OSDir


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Static code analysis for Flink project


Hi Alex,

thanks for bringing this topic up. So far the Flink project does not use a
static code analysis tool but I think it can strongly benefit from it
(simply by looking at the reported bugs). There was a previous discussion
about enabling the ASF Sonarcube integration for Flink [1] but it was never
put into reality. There is also an integration for Travis which might be
interesting to look into [2]. I would be in favour of enabling this.

[1]
http://apache-flink-mailing-list-archive.1008284.n3.nabble.com/Add-Sonarqube-analysis-td14556.html
[2] https://docs.travis-ci.com/user/sonarcloud/

Cheers,
Till

On Tue, Jun 12, 2018 at 11:12 PM Ted Yu <yuzhihong@xxxxxxxxx> wrote:

> I took a look at some of the blocker defects.
> e.g.
>
> https://sonarcloud.io/project/issues?id=org.apache.flink%3Aflink-parent&open=AWPxETxA3e-qcckj1Sl1&resolved=false&severities=BLOCKER&types=BUG
>
> For
> ./flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/contrib/streaming/state/PredefinedOptions.java
> , the closing of DBOptions using try-with-resources is categorized as
> blocker by the analysis.
>
> I don't think that categorization is proper.
>
> We can locate the high priority defects, according to consensus, and fix
> those.
>
> Cheers
>
> On Tue, Jun 12, 2018 at 2:01 PM, <simeon.arkhipov@xxxxxxxxx> wrote:
>
> > Hello Flink community.
> >
> > I am new in Flink project and probably don't understand it a lot. Could
> > you please clarify one question to me?
> >
> > I download Flink sources and build it from scratch. I found checkstyle
> > guidelines that every Flink developer should follow which is very useful.
> > However, I didn't find anything about static analysis tools like
> Sonarcube.
> > I have looked through mailing lists archive but without success. That
> > seemed very strange to me.
> >
> > I have setup Sonarcube and run analysis on whole Flink project. After a
> > while I have got 442 bugs, 511 vulnerabilities and more than 13K Code
> > Smells issues. You can see them all here: https://sonarcloud.io/
> > dashboard?id=org.apache.flink%3Aflink-parent
> >
> > I looked through some of bugs and vulnerabilities and there are many
> > important ones (in my opinions) like these:
> > - 'other' is dereferenced. A "NullPointerException" could be thrown;
> > "other" is nullable here.
> > - Either re-interrupt this method or rethrow the "InterruptedException".
> > - Move this call to "wait()" into a synchronized block to be sure the
> > monitor on "Object" is held.
> > - Refactor this code so that the Iterator supports multiple traversal
> > - Use try-with-resources or close this "JsonGenerator" in a "finally"
> > clause. Use try-with-resources or close this "JsonGenerator" in a
> "finally"
> > clause.
> > - Cast one of the operands of this subtraction operation to a "long".
> > - Make "ZERO_CALENDAR" an instance variable.
> > - Add a "NoSuchElementException" for iteration beyond the end of the
> > collection.
> > - Replace the call to "Thread.sleep(...)" with a call to "wait(...)".
> > - Call "Optional#isPresent()" before accessing the value.
> > - Change this condition so that it does not always evaluate to "false".
> > Expression is always false.
> > - This class overrides "equals()" and should therefore also override
> > "hashCode()".
> > - "equals(Object obj)" should test argument type
> > - Not enough arguments in LOG.debug function. Not enough arguments.
> > - Remove this return statement from this finally block.
> > - "notify" may not wake up the appropriate thread.
> > - Remove the boxing to "Double".
> > - Classes should not be compared by name
> > - "buffers" is a method parameter, and should not be used for
> > synchronization.
> >
> > Are there any plans to work on static analysis support for Flink project
> > or it was intentionally agreed do not use static analysis as time
> consuming
> > and worthless?
> >
> > Thank you in advance for you replies.
> >
> > Best Regards,
> > ---
> > Alex Arkhipov
> >
> >
>