RSPEC-2444 SonarQube 8.x not reporting issues while version 4.5.x does

SonarQube Server / Community Build
, ,
1

SonarSource Code Analyzers Rules Explorer rule is not reporting issues in example code in SonarQube 8.8 while it did in SonarQube 4.5.x

  • versions used (SonarQube, Scanner, language analyzer)
    SonarQube 8.8, compile using Maven
mvn --version
Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
Maven home: /usr/local/Cellar/maven/3.6.3_1/libexec
Java version: 1.8.0_282, vendor: AdoptOpenJDK, runtime: xxx/adopt-openj9-1.8.0_282/Contents/Home/jre
Default locale: en_US, platform encoding: UTF-8
OS name: "mac os x", version: "10.16", arch: "x86_64", family: "Mac"
  • minimal code sample to reproduce (with analysis parameter, and potential instructions to compile).

Test.java

package test;

import java.util.Properties;

public class Test {
    private static Properties fPreferences = null;

    private static Properties getPreferences() {
        if (fPreferences == null) {
            fPreferences = new Properties(); // Noncompliant
            fPreferences.put("loading", "true");
            fPreferences.put("filterstack", "true");
            readPreferences();
        }
        return fPreferences;
    }

    private static void readPreferences() {
    }

    public Object doSomething() {
        return fPreferences.get("loading");
    }
}

pom.xml

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <groupId>test</groupId>
    <version>1.0.0-SNAPSHOT</version>
    <artifactId>test</artifactId>
    <packaging>jar</packaging>
</project>
2

Rule from FindBugs plugin “Incorrect lazy initialization and update of static field” does report issue in this code.

4

Hello @pgrabowski

Version 4.5.x is more than 5 years old, a lot happened since then!
More specifically, my guess is that the change you are looking for is: SONARJAVA-2569.

The idea is that in order to remove noise, this rule triggers an issue only when the class contains a synchronized method. It is also stated in the rule description.

I hope it clarifies the situation.
Quentin

6

Hi @Quentin
I didn’t notice that description of rule was updated.
For our case old behaviour of the rule was completely correct and helped us spot issues.
After the change rule seams useless.
I don’t understand why rule now requires an other synchronised method to treat class as multithread class.
Example class is valid multithread, issue would be here if two threads would do Test.getPreferences().get(“loading”), there could be a race when one thread passes null check and other would get half initialised fPreferences field.

Can you point me out to discussion that lead to changes in this rule? Jira link doesn’t have much information.

Regards,
Piotr Grabowski

7

I did not manage to find the discussion related to this change either. My guess is that we have to know when we are in a multi-threaded context, without this restriction, the rule generates too many issues, the real problems will be hidden amongst false positives.

That being said, I kind of agree that it seems a bit too restrictive, however, I am not sure how we can do better in this situation, without being too noisy.

8

Thanks for reply.
I wanted to see full discussion to get more context about what type of noise that rule generated.
For us how this rule behaviour was correct and I believe that other people would also benefit from it. If rule generates too much noise for code base that doesn’t have multi thread use maybe there should be configuration option for it ‘is code base multi thread’ or similar.
Can you still have configuration options for rules?

9

After reviewing the whole situation, I agree that there is something to improve for this rule, and relaxing the constraint to have a synchronized method seems to be the best way to go.

I created a ticket (SONARJAVA-3802) summarizing the situation, could you have a look at it and tell me if it makes sense to you?

Thanks for raising the point.

Best,
Quentin

1 Like
10

Thanks Quentin.
Ticket you created is exactly what I had in mind.

1 Like
11

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.