spotbugs / spotbugs-maven-plugin

Maven Mojo Plug-In to generate reports based on the SpotBugs Analyzer
https://spotbugs.github.io/spotbugs-maven-plugin/
Apache License 2.0
69 stars 51 forks source link

SuppressFBWarnings is ignored #558

Open victorherraiz-santander opened 1 year ago

victorherraiz-santander commented 1 year ago

SuppressFBWarnings is ignored in versions 4.7.3.1 and 4.7.3.2.

        @SuppressFBWarnings(value = "PREDICTABLE_RANDOM", justification = "This is just a demo")
        private static final Random RANDOM = new Random();

output:

[INFO] BugInstance size is 1
[INFO] Error size is 0
[INFO] Total bugs: 1
[ERROR] Medium: This random generator (java.util.Random) is predictable [uk.co.package.reference.ReferenceApplication$ReferenceRestController] At ReferenceApplication.java:[line 35] PREDICTABLE_RANDOM
hazendaz commented 1 year ago

@victorherraiz-santander Did that work previously on 4.7.3.0?

victorherraiz-santander commented 1 year ago

4.7.3.0 behaves as expected and suppress the warning

hazendaz commented 1 year ago

@victorherraiz-santander Thanks, will look into it as soon as I can. Nothing is coming to mind at the moment as to the issue, still same spotbugs...

victorherraiz-santander commented 1 year ago

Thank you!

These are the versions that work for me:

        <spotbugs.version>4.7.3</spotbugs.version>
        <spotbugs-maven-plugin.version>4.7.3.0</spotbugs-maven-plugin.version>
        <findsecbugs-plugin.version>1.12.0</findsecbugs-plugin.version>

And this is the plugin declaration:

            <plugin>
                <groupId>com.github.spotbugs</groupId>
                <artifactId>spotbugs-maven-plugin</artifactId>
                <version>${spotbugs-maven-plugin.version}</version>
                <dependencies>
                    <dependency>
                        <groupId>com.github.spotbugs</groupId>
                        <artifactId>spotbugs</artifactId>
                        <version>${spotbugs.version}</version>
                    </dependency>
                    <dependency>
                        <groupId>uk.co.package.springboot</groupId>
                        <artifactId>spotbugs-configuration</artifactId>
                        <version>1.3.2-SNAPSHOT</version>
                    </dependency>
                </dependencies>
                <configuration>
                    <effort>Max</effort>
                    <failThreshold>Medium</failThreshold>
                    <sarifOutput>true</sarifOutput>
                    <excludeFilterFile>uk/co/package/spotbugs/exclude-filter-file.xml</excludeFilterFile>
                    <plugins>
                        <plugin>
                            <groupId>com.h3xstream.findsecbugs</groupId>
                            <artifactId>findsecbugs-plugin</artifactId>
                            <version>${findsecbugs-plugin.version}</version>
                        </plugin>
                    </plugins>
                </configuration>
                <executions>
                    <execution>
                        <id>spotbugs-check</id>
                        <goals>
                            <goal>check</goal>
                        </goals>
                    </execution>
                </executions>
            </plugin>

If I increase just the plugin versions it fails

hazendaz commented 1 year ago

@victorherraiz-santander

Diff between 4.7.3.0 and master can be seen here.

https://github.com/spotbugs/spotbugs-maven-plugin/compare/spotbugs-maven-plugin-4.7.3.0...spotbugs

Only specific changes I can gather are that sarif originally ignored the xml report.

Spotbugs claims to support as many reports as needed. The xml one is default. I wonder if allowing both to be created has not impacted sarif usage. Can you check your output and see if there is additionally xml output that may contain the specific issue?

Also would it be possible to write an integration test for us that demonstrates this specific issue to better help solve it?

I'd like to get a 4.7.3.3 out soon as groovy just fixed a defect affecting this project that I've now allowed so I'll wait until I hear back from you to potentially get this included.

victorherraiz-santander commented 1 year ago

Sorry for the delay.

Both, xml and sarif, show the same info.

I will try adding an integration test to replicate the issue.

I also tested with 4.7.3.4 and the issue is still there.

As you can see I am using the same version of spotbugs with different plugin <spotbugs.version>4.7.3</spotbugs.version>

victorherraiz commented 5 months ago

@hazendaz The issue is still there for 4.8.2.0 with 4.8.2

Moving @SuppressFBWarnings("PREDICTABLE_RANDOM") from field to class, it works. But it is not desirable. It's like field suppresion is not working for some reason.

hazendaz commented 5 months ago

Is this a spotbugs issue then rather than maven plugin?

victorherraiz commented 5 months ago

Using spotbugs 4.7.3 in 4.7.3.1 or even in 4.8.2.0 does not solve the problem. And using a new spotbugs version in 4.7.3.0 does not reproduce the issue. Then, I think spotbugs is not the issue.

I saw that there are several dependency updates on 4.7.3.1. I tried to downgrade most of them but groovy, the issue still there. I will take a look to the options pass to the ant task when I have some free time.

hazendaz commented 5 months ago

So you are saying it was working prior to 4.7.0? Possibly something in there. I've been trying to be a bit more aggressive on this repo and unlike in the past where this was my only part of spotbugs, I'm now involved in most of the other projects now too. We did seem to lose some regular folks working on things so its down to just really 3 of us and 2 others that have stepped back a little. Unfortunately on maven, its mostly been me so any help you can provide would be great. Any integration tests added would help too. In fact, I pretty much go with all is well when those work given there are so many.