I confused Jacoco to be code analysis tool instead of code coverage one. Early morning I guess :) I was thinking more about tooling like Sonar. What do you think about that? Any pros / cons to Spotbugs?
________________________________________ From: Miklosovic, Stefan <stefan.mikloso...@netapp.com> Sent: Tuesday, November 8, 2022 11:36 To: dev@cassandra.apache.org Subject: Re: [DISCUSSION] Add SpotBugs to the build Hi David, all what is the position of Jacoco in Cassandra as this point? I see it in build.xml there are some targets and infrastructure around that. What is wrong with using Jacoco instead more heavily for static analysis? Why do we need to introduce this? Regards ________________________________________ From: David Capwell <dcapw...@apple.com> Sent: Tuesday, November 8, 2022 0:10 To: dev Subject: [DISCUSSION] Add SpotBugs to the build NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe. I was thinking that it would be good to add SpotBugs (https://spotbugs.github.io) into our build to help find bugs earlier in the life cycle. SpotBugs is LGPL but as it is used only in the build and not to within this project, then this should be fine with Apache. The motivation for adding this was from CASSANDRA-17178; the Simulator has issues with Serializable classes missing serialVersionUID (as we deal with ClassLoaders; this field is strongly recommend in general for all Serializable classes), but this project can add more value as there are a large collection of potential bugs to look out for; below are a few examples found. * Number.valueOf vs Number.parse<size>. In many parts of the code we do valueOf which returns a boxed value; we then unbox for the usage; this adds more garbage that isn’t needed * Using Number.compareTo rather than primitive compare functions (causing boxing) * Ignoring return value for functions that don’t have a side effect. This happens in a few cases where we are building a StringBuilder where we call .toString but ignore the string… then call it later on * use of putIfAbsent without looking at the return. This was found in CacheService where we add the SSTable reader to the cache and assume we win the race and start using it… rather than using the object that won the race
