Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SuppressFBWarnings is ignored #558

Open
victorherraiz-santander opened this issue Mar 1, 2023 · 10 comments
Open

SuppressFBWarnings is ignored #558

victorherraiz-santander opened this issue Mar 1, 2023 · 10 comments
Assignees

Comments

@victorherraiz-santander
Copy link

victorherraiz-santander commented Mar 1, 2023

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
Copy link
Member

hazendaz commented Mar 1, 2023

@victorherraiz-santander Did that work previously on 4.7.3.0?

@victorherraiz-santander
Copy link
Author

4.7.3.0 behaves as expected and suppress the warning

@hazendaz
Copy link
Member

hazendaz commented Mar 2, 2023

@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...

@hazendaz hazendaz self-assigned this Mar 2, 2023
@victorherraiz-santander
Copy link
Author

victorherraiz-santander commented Mar 2, 2023

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
Copy link
Member

@victorherraiz-santander

Diff between 4.7.3.0 and master can be seen here.

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
Copy link
Author

victorherraiz-santander commented Apr 12, 2023

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
Copy link

@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
Copy link
Member

Is this a spotbugs issue then rather than maven plugin?

@victorherraiz
Copy link

victorherraiz commented Dec 15, 2023

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
Copy link
Member

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.

mches added a commit to markitect-dev/markitect-liquibase that referenced this issue Aug 1, 2024
mches added a commit to markitect-dev/markitect-liquibase that referenced this issue Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants