What to do with SpotBugs findings?

Trying to get familiar with OpenMRS and also trying out Eclipse plugins I had SpotBugs work on openmrs-api where it found code that I think cannot work at all.

The class Operator uses the class ComparisonOperator which actually implements Operator. SpotBugs says: *During the initialization of a class, the class makes an active use of a subclass. That subclass will not yet be initialized at the time of this use.

The OpenMRS wiki suggests that there has been the intention to integrate FindBugs into the maven build ([https://issues.openmrs.org/browse/TRUNK-5083](Add FindBugs rules and maven plugin)) but obviously it has never happend.

Can this code (see below) really be used? Would it make sense to have another go at integrating FindBugs or SpotBugs into the maven build? What was the reason the pull request didn’t make it?

--------------code marked by SpotBugs--------------

public interface Operator {
	// comparison operators
	public static final Operator CONTAINS = ComparisonOperator.CONTAINS;

and

public interface ComparisonOperator extends Operator {
	// comparison operators
	public static final ComparisonOperator CONTAINS = new Contains();

Do you mean that the pull request on that ticket was not merged?

No, I just couldn’t find any files generated by FindBugs and then looked in the wrong pom.xml. Looking in the right pom.xml I can now see the plugin but even in debug mode I cannot find any evidence that FindBugs is executed.:frowning:

I run maven with (clean) install and was expecting the build to fail when bugs are found. Is this some kind of RTFM on my side?

I asked Google for help and it suggests that version 3.0.1 might not be compatible with Java 8 but I couldn’t find any reference to 3.0.5. So I guess it should work.

Is this of help? Java Conventions - Documentation - OpenMRS Wiki

The wiki entry together with the accepted pull request made me think that there could be some reports and build failures caused by findbugs analysis. But it wasn’t much help in understanding why nothing happend. No trace of the use of findbugs-maven-plugin in the build log.

So far I’ve figured out: The findbugs-maven-plugin in the (openmrs-core) reactor POM doesn’t do anything. Added to one of the “sub-poms” (like openmrs-api in the run below) it will be used and can (if configured) prevent successful builds: [ERROR] Failed to execute goal org.codehaus.mojo:findbugs-maven-plugin:

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for OpenMRS 2.3.0-SNAPSHOT:
[INFO] 
[INFO] OpenMRS ............................................ SUCCESS [  1.438 s]
[INFO] openmrs-tools ...................................... SUCCESS [  0.880 s]
[INFO] openmrs-test ....................................... SUCCESS [  0.105 s]
[INFO] openmrs-api ........................................ FAILURE [ 36.045 s]
[INFO] openmrs-web ........................................ SKIPPED
[INFO] openmrs-webapp ..................................... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  38.722 s
[INFO] Finished at: 2019-09-03T01:00:51+02:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.codehaus.mojo:findbugs-maven-plugin:3.0.5:check (analyze-compile) on project openmrs-api: failed with 470 bugs and 0 errors -> [Help 1]

So it seems it would be necessary to introduce the plugin into the actual POMs instead of the reactor POM. Considering the number of bugs found it wouldn’t be advisable though to prevent the build. But it would certainly be a good idea to look into the findings.

Perhaps generate a report and from that some issues?

How about just adding a maven build profile that one can use if they want such a report?

Here is an example of the profile that we use whenever we want to run integration tests: https://github.com/openmrs/openmrs-core/blob/master/pom.xml#L921-L945

which we run as: mvn clean test -Pskip-default-test -Pintegration-test