Later, we may enable more checks than just leaked resources to improve the code gradually.
- - -- --- ----- -------- ------------- Jacek Lewandowski pt., 16 cze 2023 o 13:48 Ekaterina Dimitrova <e.dimitr...@gmail.com> napisał(a): > Got so excited that I forgot to say which of the two options exactly I > meant - running the analysis only on changed files after the initial full > pass is done sounds like a good improvement to me > > On Fri, 16 Jun 2023 at 7:43, Ekaterina Dimitrova <e.dimitr...@gmail.com> > wrote: > >> I think this is a great idea and it will probably reduce the time to run >> it. Thank you! >> >> On Fri, 16 Jun 2023 at 7:40, Jacek Lewandowski < >> lewandowski.ja...@gmail.com> wrote: >> >>> Additional question is whether we want to run the checks against the >>> whole project or just against the file changes between the feature branch >>> and the target release branch? >>> >>> >>> - - -- --- ----- -------- ------------- >>> Jacek Lewandowski >>> >>> >>> pt., 16 cze 2023 o 13:09 Aleksey Yeshchenko <alek...@apple.com> >>> napisał(a): >>> >>>> Sounds like a clear improvement to me. Only once this check flagged a >>>> legitimate issue I missed, if I’m remembering correctly. All other >>>> instances have just been annoyances, forcing to add a redundant suppressed >>>> annotation. >>>> >>>> On 15 Jun 2023, at 19:01, Ekaterina Dimitrova <e.dimitr...@gmail.com> >>>> wrote: >>>> >>>> Hi everyone, >>>> Happy Thursday! >>>> Some time ago, Jacek raised the point that ant eclipse-warnings is >>>> generating too many false positives and not really working as expected. >>>> (CASSANDRA-18239) >>>> >>>> Reminder: ant eclipse-warnings is a task we run with the goal to check >>>> Cassandra code - static analysis to warn on unsafe use of Autocloseable >>>> instances; checks against two related particular compiler options >>>> >>>> While trying to upgrade ECJ compiler that we use for this task >>>> (CASSANDRA-18190) so we can switch the task from running it with JDK8 to >>>> JDK11 in preparation for dropping JDK8, I hit the following issues: >>>> - the latest version of ECJ is throwing more than 300 Potential Resource >>>> Leak warnings. I looked at 10-15, and they were all false positives. >>>> - Even if we file a bug report to the Eclipse community, JDK11 is about to >>>> be removed with the next version of the compiler >>>> >>>> So I shared this information with Jacek. He came up with a different >>>> solution: >>>> It seems we already pull through Guava CheckerFramework with an MIT >>>> license, which appears to be acceptable according to this link - >>>> https://www.apache.org/legal/resolved.html#category-a >>>> He already has an initial integration with Cassandra which shows the >>>> following: >>>> - CheckerFramework does not understand the @SuppressWarnings("resource") >>>> (there is a different one to be used), so it is immediately visible how it >>>> does not report all those false positives that eclipse-warnings does. On >>>> the flip side, I got the feedback that what it has witnessed so far is >>>> something we should investigate. >>>> - Also, there are additional annotations like @Owning that let you fix >>>> many problems at once because the tool understands that the ownership of >>>> the resources was passed to another entity; It also enables you to do >>>> something impossible with eclipse-warnings - you can tell the tool that >>>> there is another method that needs to be called to release the resources, >>>> like release, free, disconnect, etc. >>>> - the tool works with JDK8, JDK11, JDK17, and JDK20, so we can backport it >>>> even to older branches (while at the same time keeping eclipse-warnings >>>> there) >>>> - though it runs 8 minutes so, we should not run it with every test, some >>>> reorganization around ant tasks will be covered as even for >>>> eclipse-warnings it was weird to call it on every single test run locally >>>> by default >>>> >>>> >>>> If there are no concerns, we will continue replacing ant eclipse-warnings >>>> with the CheckerFramework as part of CASSANDRA-18239 and CASSANDRA-18190 >>>> in trunk. >>>> >>>> Best regards, >>>> >>>> Ekaterina >>>> >>>> >>>>