I can help a little bit with fixing ErrorProne issues as I am already seeing them some with bazel builds
On Mon, Dec 24, 2018 at 9:53 AM Ian Turton <ijtur...@gmail.com> wrote: > Sounds good to me, I'm a little tied up just now, but I might be able to > help later. > > Ian > > On Mon, 24 Dec 2018, 15:38 Andrea Aime <andrea.a...@geo-solutions.it > wrote: > >> Hi all, >> I was wondering if it might be time, and or it might be useful, to add >> some static code analysis in our build >> chain, as a profile that build servers can run (especially Travis). >> >> Looking at the code quality related tools we added in the last year, the >> code formatting compliance checks, >> breaking the build at misbehavior, have worked, while the OWASP checks, >> being a daily check, have largely >> been ignored. >> >> With this in mind, I think static analysis should be of the kind that can >> break the build, or setting it up might >> end up being a waste of time. >> >> A second observation is about what the static analysis checks, it should >> be a set of obvious bugs and not >> too many that getting the build in compliant state would be an impossible >> undertaking, otherwise we'd >> not be able to get started. >> >> Looking around I've found basically PMD, which is a source code analyzer >> and somewhat old, and two >> bytecode analyzer, spotbugs (the successor of the dead findbugs) and >> Google's errorprone. >> >> PMD <https://pmd.github.io/pmd-6.10.0/index.html> checks source code >> directly, has maven integration and it's well configurable. >> The default setup is not usable imho, as it finds over 1000 violations >> just in gt-main, with all sorts >> of issues in the mix, however it's easy to configure and lowering down to >> priority=1 >> there is only a single thread safety related failure in main (an >> improperly implemented double checked >> locking, not using a volatile field as the synch variable). >> Upping the level of the checks to priority 2 makes the code report a ton >> of "using new Boolean(...)" instead of "Boolean.valueOf" >> which does not seem that important to me.... (well, if it's used in a >> tight loop it's gonna kill performance, ok, >> but believe there should be worse issues to care for). A sample build >> failure report at priority 2 is attached. >> Going up to level 3 throws in a ton of checks that border on trivial, I >> would not go there. >> If you want to try out, I have a branch here: >> https://github.com/aaime/geotools/tree/pmd >> To use it, do a "mvn clean pmd:check -Ppmd" (mind the profile, otherwise >> the configuration won't be used) >> >> Spotbugs <https://spotbugs.readthedocs.io/en/latest/> can break the >> build, but seems to be mostly geared towards doing reports. The maven >> integration >> can be configured to list the types of checks one want, but I cannot find >> the list of all bug analyzers to >> configure it with a smaller number. >> Even with the lowest analysis effort and the highest confidence effort >> setups I get 284 errors reported >> on gt-main only (report attached). >> If you want to try it out, I have a branch here: >> https://github.com/aaime/geotools/tree/spotbugs >> To use it, do a "min install -DskipTests" (did not manage to get it into >> a profile yet, that part would be easy though). >> >> ErrorProne <https://github.com/google/error-prone> is a compiler >> plugin/javac replacement that adds more checks during the compile phase. >> As per Google's philosophy, it's opinionated and leaves little room to >> configuration, on the brighter side, >> on our code base it's reporting relatively few issues, 42 on the gt-main >> module (report attached to this mail), >> and relatively obvious ones. >> One downside, it seems to have issues >> <https://github.com/google/error-prone/issues/1205> with JDK 11, >> however, we can activate it only with JDK 8 if needs be. >> If you want to try it out, I have a branch here: >> https://github.com/aaime/geotools/tree/errorprone >> To use it, do a "min install -DskipTests -Perrorprone" >> >> Personally I haven't liked spotbugs, seems too hard to configure, but if >> someone manages to reduce >> the errors reported a few important ones I might change my mind about it >> :-D >> >> PMD is quick and easy, although there are very few issues categorized at >> priority 1, High, in the documentation >> <https://pmd.github.io/pmd-6.10.0/pmd_rules_java.html>, >> going to level 2 seems to introduce already a bunch that are debatable, >> but not too bad. I would not go priority level 3. >> Also, if there is no one helping, PMD seems at priority "1" seems to be >> suitable for a "one man job". >> >> ErrorProne seems to have a better balance, not too many issues reported >> and the ones that I see appear >> to be "reasonable", downside, pretty much no configuration, so if we >> stumble into something we don't like, >> well, we're toast. And it's still reporting enough issues that I'm not >> sure I can do it alone. >> >> Opinions? >> Anyone "excited" about the topic enough to help? >> >> Cheers >> Andrea >> >> == GeoServer Professional Services from the experts! Visit >> http://goo.gl/it488V for more information. == Ing. Andrea Aime @geowolf >> Technical Lead GeoSolutions S.A.S. Via di Montramito 3/A 55054 Massarosa >> (LU) phone: +39 0584 962313 fax: +39 0584 1660272 mob: +39 339 8844549 >> http://www.geo-solutions.it http://twitter.com/geosolutions_it >> ------------------------------------------------------- *Con riferimento >> alla normativa sul trattamento dei dati personali (Reg. UE 2016/679 - >> Regolamento generale sulla protezione dei dati “GDPR”), si precisa che ogni >> circostanza inerente alla presente email (il suo contenuto, gli eventuali >> allegati, etc.) è un dato la cui conoscenza è riservata al/i solo/i >> destinatario/i indicati dallo scrivente. Se il messaggio Le è giunto per >> errore, è tenuta/o a cancellarlo, ogni altra operazione è illecita. Le >> sarei comunque grato se potesse darmene notizia. This email is intended >> only for the person or entity to which it is addressed and may contain >> information that is privileged, confidential or otherwise protected from >> disclosure. We remind that - as provided by European Regulation 2016/679 >> “GDPR” - copying, dissemination or use of this e-mail or the information >> herein by anyone other than the intended recipient is prohibited. If you >> have received this email by mistake, please notify us immediately by >> telephone or e-mail.* >> _______________________________________________ >> GeoTools-Devel mailing list >> GeoTools-Devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/geotools-devel >> > _______________________________________________ > GeoTools-Devel mailing list > GeoTools-Devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/geotools-devel > -- -- http://schwehr.org
_______________________________________________ GeoTools-Devel mailing list GeoTools-Devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geotools-devel
