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

Findbugs is reporting false positive bugs SA_LOCAL_SELF_COMPARISON when using instanceof pattern matching #876

Open
mpet opened this issue Nov 29, 2023 · 13 comments

Comments

@mpet
Copy link

mpet commented Nov 29, 2023

Issue Description

When running sonar rules on java code that has the following construct:

https://docs.oracle.com/en/java/javase/17/language/pattern-matching-instanceof-operator.html#GUID-843060B5-240C-4F47-A7B0-95C42E5B08A7

Sonarqube rule: findbugs:SA_LOCAL_SELF_COMPARISON

gives a false positive.

Environment

Component Version
SonarQube Community EditionVersion 9.9 (build 65466)
Sonar-FindBugs FindBugs 4.2.3 / SpotBugs 4.7.3
Maven 3.8.5
Gradle
Java 17

Code (If needed)

private static X509Certificate readCertificatePEMFile(String certificatePemFilePath)
            throws IOException, CertificateException {
        if (certificatePemFilePath != null) {
            File certificatePemFile = getCertificatePEMFile(certificatePemFilePath);
            if (certificatePemFile.exists() && certificatePemFile.canRead()) {
                try (InputStream inStream = new FileInputStream(certificatePemFile)) {
                    try (PEMParser pemParser = new PEMParser(new InputStreamReader(inStream))) {
                        Object object = pemParser.readObject();
                        if (object instanceof X509CertificateHolder x509CertificateHolder) {
                            return new JcaX509CertificateConverter().getCertificate(x509CertificateHolder);
                        }
                    }
                }
            }
        }
        throw new MsranJcatException(String.format("error CA trust certificate pem file!%n{}", certificatePemFilePath));
    }
@hazendaz
Copy link
Member

@mpet Can you try the latest and confirm if issue persists?

@gtoison
Copy link
Contributor

gtoison commented Dec 3, 2023

This is the same issue as spotbugs/spotbugs#1992 most probably

@gtoison gtoison closed this as completed Dec 3, 2023
@gtoison gtoison reopened this Dec 3, 2023
@mpet
Copy link
Author

mpet commented Dec 5, 2023

@hazendaz I will verify with:

com.github.spotbugs spotbugs-maven-plugin 4.8.2.0

@mpet
Copy link
Author

mpet commented Dec 5, 2023

When I tested with this version it is ok. Do @hazendaz or @gtoison know in which version this was fixed?

@gtoison
Copy link
Contributor

gtoison commented Dec 5, 2023

My understanding (based on spotbugs/spotbugs#1435 (comment) and spotbugs/spotbugs#1992) is that the issue was actually caused by compilers (ecj, javac) producing some redudant bytecode.

Maybe while testing with the gradle plugin you also changed the compiler?

It is not clear to me what was fixed (nor where or when). From what I can see only a unit test reproducing the issue was added to the SpotBugs project, but I might have missed something

@mpet
Copy link
Author

mpet commented Dec 5, 2023

I am a bit confused now since when I run with the spotbugs maven plugin,spotbugs-maven-plugin, on the below code:
https://github.com/manikmagar/java14-examples/tree/master

Both spotbugs versions 4.7.3.0 and 4.8.2.0

image

image

But I get a different bug reported?!

@gtoison
Copy link
Contributor

gtoison commented Dec 6, 2023

I think you need to share a way of reproducing the problem or we won't be able to help.
My understanding is that the issue is with the compiler, because some compilers produce redundant bytecode.
Sharing a maven project is not enough because there's no way to tell what compiler was used: even though the above project is java 14 based on the pom, you might be running maven on top of a jdk 21.
Similarly your initial question says you are on java 17 but I'm not reproducing the problem, so I'm probably missing some crucial details.

@mpet
Copy link
Author

mpet commented Dec 6, 2023

This is really confusing since the messge from the people admin the SQ server was:
"Currently we are using:
FindBugs 4.2.3 / SpotBugs 4.7.3"
is it not only one plugin for both?

@mpet
Copy link
Author

mpet commented Dec 6, 2023

@gtoison yes I understand I need to be able to reproduce the problem. I do not have access to the SQ instance since I am not admin. That is why I tried with sonar maven plugin since that was the best option.
Btw example is called jaav14 I changed to use 17 for it.

Not sure this is related to compiler since the same compiler is used in both cases:
https://www.azul.com/downloads/#zulu and it is 17.

What is needed from SQ to show what is happening?

@gtoison
Copy link
Contributor

gtoison commented Dec 6, 2023

FindBugs is the original name of the project. SonarSource made a plugin to use it in SonarQube.
When FindBugs was renamed SpotBugs the plugin kept using the FindBugs name, because there was no practical way to rename a SonarQube plugin.

So 4.2.3 is the version of the SonarQube plugin and it uses SpotBugs 4.7.3 to perform the analysis.

The plugin does not run on the server, it runs wherever the project is built. Normally after the project is compiled and the tests executed sonar runs the analysis (still on the computer running the build) and then only ships the findings (issues, coverage, etc.) to the SonarQube server.
So the false positive issue is happening on the computer running the build and the SonarQube server is just reporting it.

@mpet
Copy link
Author

mpet commented Dec 7, 2023

@gtoison we have been running it on windows and linux but the jdk 17 is the same from Azul.
I got this info from SQ admin.

image

@gtoison
Copy link
Contributor

gtoison commented Dec 11, 2023

I think that you are referring to the JDK running your SonarQube server, but that shouldn't matter here.

wborn added a commit to wborn/openhab-webui that referenced this issue Jan 8, 2024
This cleanup includes:

* Fix deprecations
* Fix JavaDocs
* Remove redundant toString calls
* Remove redundant semicolons
* Simplify boolean expressions
* Use diamond operator
* Use enhanced for loops
* Use instanceof pattern matching
* Use isEmpty instead of 0 comparisons
* Use lambdas
* Use static inner classes
* Use StandardCharsets

Also adds the SA_LOCAL_SELF_COMPARISON suppression similar as used in other repositories for spotbugs/sonar-findbugs#876.

Signed-off-by: Wouter Born <[email protected]>
wborn added a commit to wborn/openhab-webui that referenced this issue Jan 8, 2024
This cleanup includes:

* Fix deprecations
* Fix JavaDocs
* Remove redundant toString calls
* Remove redundant semicolons
* Simplify boolean expressions
* Use diamond operator
* Use enhanced for loops
* Use instanceof pattern matching
* Use isEmpty instead of 0 comparisons
* Use lambdas
* Use static inner classes
* Use StandardCharsets

Also adds the SA_LOCAL_SELF_COMPARISON suppression similar as used in other repositories for spotbugs/sonar-findbugs#876.

Signed-off-by: Wouter Born <[email protected]>
@gtoison
Copy link
Contributor

gtoison commented Feb 12, 2024

@mpet I think you might be running into an issue with Eclipse's compiler producing redundant bytecode. When SpotBugs analyzes that bytecode it finds the SA_LOCAL_SELF_COMPARISON which is indeed in the compiled .class file.
It seems that this is addressed and should be fixed in upcoming versions of ECJ
See spotbugs/spotbugs#1992

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