Hi, Generally speaking I would be in favour of adding some reasonable static code analysis, however this requires a lot of effort and can not be done at once. Also if such checks are firing incorrectly too often it would be annoying to manually suppress such warnings/errors.
> I don't really see a benefit in enabling it /continuously/. On the other hand, I do not see a point of not running it as a part of travis CI and non-fast mvn build (`mvn clean install -DskipTests` should perform such static checks). If such rules are not enforced by a tool, then there is really no point in talking about them - they will be very quickly ignored and abandoned. Piotrek > On 13 Jun 2018, at 10:38, Chesnay Schepler <ches...@apache.org> wrote: > > I don't really see a benefit in enabling it /continuously/. > This wouldn't be part of the build or CI processes, as we can't fail the > builds since it happens too often that issues are improperly categorized. > > Wading through these lists is time-consuming and I very much doubt that we > will do that with a high or even regular frequency. > We would always require 1-2 committers to commit to this process. > Thus there's no benefit in running sonarcube beyond these irregular checks as > we'd just be wasting processing time. > > I suggest to keep it as a manual process. > > On 13.06.2018 09:35, Till Rohrmann wrote: >> 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 <yuzhih...@gmail.com> 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.arkhi...@gmail.com> 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 >>>> >>>> >
