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
