FYI, now that the ErrorProne migration is complete [1] it's a good time to discuss whether FindBugs should be removed or not.
I haven't compared the set of checks we are doing with FindBugs vs ErrorProne. If ErrorProne can replace the checks from FindBugs then I'm in favor of getting rid of it. One of the main gripes with FindBugs is that is not well maintained. SpotBugs is a new fork of FindBugs which is more active; if we do decide there's value in FindBugs perhaps we should at least move to the newer fork: https://spotbugs.github.io/ [1] https://lists.apache.org/thread.html/0ae24dc4e88e6c38407b74ad4d093985a525f3d3eea094e054ed4cfa@%3Cdev.beam.apache.org%3E On Wed, May 30, 2018 at 9:42 AM Kenneth Knowles <k...@google.com> wrote: > Awesome! > > In the meantime I've tried out Gradle + Checker and unfortunately > compilation hung. It could be due to any subset of Gradle, Checker, > Errorprone. I would not expect a performance problem, since Checker is > "pluggable type systems" and type checking is a very fast sort of analysis. > Also I didn't immediately find the configuration to ignore generated code. > I didn't have a lot of time so I didn't dig further. > > Kenn > > On Wed, May 30, 2018 at 9:28 AM Pablo Estrada <pabl...@google.com> wrote: > >> Thank you guys : D >> >> On Wed, May 30, 2018 at 9:20 AM Scott Wegner <sweg...@google.com> wrote: >> >>> Sorry to revive an old thread, but I wanted to give a shout-out and say >>> thank you to Ismaël and Tim who have been quickly chipping away at the >>> ErrorProne backlog. We started with 47 ErrorProne JIRA's [1], and in two >>> weeks we're down to just 17 [2]. Thanks! >>> >>> [1] >>> *https://issues.apache.org/jira/issues/?jql=project%20%3D%20BEAM%20AND%20labels%20%3D%20errorprone >>> <https://issues.apache.org/jira/issues/?jql=project%20%3D%20BEAM%20AND%20labels%20%3D%20errorprone> >>> * >>> [2] >>> https://issues.apache.org/jira/issues/?jql=project%20%3D%20BEAM%20AND%20status%20in%20(Open%2C%20%22In%20Progress%22)%20AND%20labels%20%3D%20errorprone >>> >>> >>> On Fri, May 18, 2018 at 7:12 AM Ismaël Mejía <ieme...@gmail.com> wrote: >>> >>>> As part of the error-prone effort Tim has been also cleaning other >>>> static >>>> analysis warnings as reported by IntelliJ's Inspect -> Analyze code. I >>>> think this is a good moment to grok some of those too e.g. scoping, >>>> unused >>>> variables, redundancies, etc. So I hope the others taking part this work >>>> try to tackle a chunk of those as well. >>>> >>>> Extra note. Of course IntelliJ's code analysis should be judged a bit, >>>> there are always fake positives or undesirable changes. >>>> >>>> On Fri, May 18, 2018 at 7:56 AM Jean-Baptiste Onofré <j...@nanthrax.net> >>>> wrote: >>>> >>>> > Thanks Tim. >>>> >>>> > I think we will be able to remove findbugs after some run/check using >>>> ErrorProne and see the gaps. >>>> >>>> > Regards >>>> > JB >>>> > Le 18 mai 2018, à 07:49, Tim Robertson <timrobertson...@gmail.com> a >>>> écrit: >>>> >>>> >> Thank you all. >>>> >>>> >> I think this is clear. Removing findbugs can happen at a future >>>> point. >>>> >>>> >> @Scott - I've been working through the java IO error prone issues >>>> (some >>>> already merged, some with open PRs now) so will take those IO Jiras. I >>>> will >>>> enable failOnWarning, address dependency issues for findbugs and tackle >>>> the >>>> error prone warnings. >>>> >>>> >>>> >> On Fri, May 18, 2018 at 1:07 AM, Scott Wegner <sweg...@google.com> >>>> wrote: >>>> >>>> >>> +0.02173913 >>>> >>>> >>> I'm happy to replace FindBugs with ErrorProne, but we need to first >>>> upgrade ErrorProne analyzer warnings to errors. Currently the codebase >>>> is >>>> full of warning spam, and there's no enforcement preventing future >>>> violations from being added. >>>> >>>> >>> I've done the work for enforcing ErrorProne analysis on >>>> java-sdk-core >>>> [1], and I've sharded out the rest of the Java components in JIRA issues >>>> [2] (45 total). Fixing the issues is relatively straightforward, and >>>> I've >>>> tried to provide enough guidance to make them as starter tasks (example: >>>> [3]). Teng Peng has already started on Spark [4] (thanks!) >>>> >>>> >>> [1] https://github.com/apache/beam/pull/5319 >>>> >>> [2] >>>> >>>> https://issues.apache.org/jira/issues/?jql=project%20%3D%20BEAM%20AND%20status%20%3D%20Open%20AND%20labels%20%3D%20errorprone >>>> >>> [3] https://issues.apache.org/jira/browse/BEAM-4347 >>>> >>> [4] https://issues.apache.org/jira/browse/BEAM-4318 >>>> >>>> >>> On Thu, May 17, 2018 at 2:00 PM Ismaël Mejía <ieme...@gmail.com> >>>> wrote: >>>> >>>> >>>> +0.7 also. Findbugs support for more recent versions of Java is >>>> lacking and >>>> >>>> the maintenance seems frozen in time. >>>> >>>> >>>> As a possible plan b can we identify the missing important >>>> validations >>>> to >>>> >>>> identify how much we lose and if it is considerable, maybe we can >>>> create a >>>> >>>> minimal configuration for those, and eventually migrate from >>>> findbugs >>>> to >>>> >>>> spotbugs (https://github.com/spotbugs/spotbugs/) that seems at >>>> least >>>> to be >>>> >>>> maintained and the most active findbugs fork. >>>> >>>> >>>> >>>> On Thu, May 17, 2018 at 9:31 PM Kenneth Knowles <k...@google.com> >>>> wrote: >>>> >>>> >>>> > +0.7 I think we should work to remove findbugs. Errorprone covers >>>> most of >>>> >>>> the same stuff but better and faster. >>>> >>>> >>>> > The one thing I'm not sure about is nullness analysis. Findbugs >>>> has >>>> some >>>> >>>> serious limitations there but it really improves code quality and >>>> prevents >>>> >>>> blunders. I'm not sure errorprone covers that. I know the Checker >>>> analyzer >>>> >>>> has a full solution that makes NPE impossible as in most modern >>>> languages. >>>> >>>> Maybe that is easy to plug in. The core Java SDK is a good >>>> candidate >>>> for >>>> >>>> the first place to do it since it is affects everything else. >>>> >>>> >>>> > On Thu, May 17, 2018 at 12:02 PM Tim Robertson < >>>> timrobertson...@gmail.com> >>>> >>>> wrote: >>>> >>>> >>>> >> Hi all, >>>> >>>> >> [bringing a side thread discussion from slack to here] >>>> >>>> >>>> >> We're tackling error-prone warnings now and we aim to fail the >>>> build on >>>> >>>> warnings raised [1]. >>>> >>>> >>>> >> Enabling failOnWarning also fails the build on findbugs >>>> warnings. >>>> >>>> Currently I see places where these arise from missing a >>>> dependency on >>>> >>>> findbugs_annotations and I asked on slack the best way to introduce >>>> this >>>> >>>> globally in gradle. >>>> >>>> >>>> >> In that discussion the idea was floated to consider removing >>>> findbugs >>>> >>>> completely given it is older, has licensing considerations and is >>>> not >>>> >>>> released regularly. >>>> >>>> >>>> >> What do people think about this idea please? >>>> >>>> >>>> >> Thanks, >>>> >>>> >> Tim >>>> >>>> >> [1] >>>> >>>> >>>> https://lists.apache.org/thread.html/95aae2785c3cd728c2d3378cbdff2a7ba19caffcd4faa2049d2e2f46@%3Cdev.beam.apache.org%3E >>>> >>> -- >> Got feedback? go/pabloem-feedback >> <https://goto.google.com/pabloem-feedback> >> >
