Upgrade SpotBugs to 4.9.1 (#1160)

.github/actions/sonar-update-center/.github/workflows/integrationTest.yml

@@ -42,7 +42,7 @@ jobs:
         id: integrated-test
         with:
           prop-file: findbugs.properties
-          description: Use SpotBugs 4.9.0, sb-contrib 7.6.8, and findsecbugs 1.13.0
+          description: Use SpotBugs 4.9.1, sb-contrib 7.6.8, and findsecbugs 1.13.0
           minimal-supported-sq-version: 9.9
           changelog-url: /~https://github.com/spotbugs/sonar-findbugs/releases/tag/4.3.0
           download-url: https://repo.maven.apache.org/maven2/com/github/spotbugs/sonar-findbugs-plugin/4.3.0/sonar-findbugs-plugin-4.3.0.jar

.github/workflows/build.yml

@@ -107,7 +107,7 @@ jobs:
         uses: ./.github/actions/sonar-update-center
         with:
           prop-file: findbugs.properties
-          description: Use SpotBugs 4.9.0, sb-contrib 7.6.8, and findsecbugs 1.13.0
+          description: Use SpotBugs 4.9.1, sb-contrib 7.6.8, and findsecbugs 1.13.0
           minimal-supported-sq-version: 9.9
           latest-supported-sq-version: LATEST
           changelog-url: /~https://github.com/spotbugs/sonar-findbugs/releases/tag/${{ github.event.release.tag_name }}

.github/workflows/sonar-update-center.yml

@@ -55,7 +55,7 @@ jobs:
         id: integration-test
         with:
           prop-file: findbugs.properties
-          description: Use SpotBugs 4.9.0, sb-contrib 7.6.8, and findsecbugs 1.13.0
+          description: Use SpotBugs 4.9.1, sb-contrib 7.6.8, and findsecbugs 1.13.0
           minimal-supported-sq-version: 9.9
           changelog-url: /~https://github.com/spotbugs/sonar-findbugs/releases/tag/4.3.0
           download-url: https://repo.maven.apache.org/maven2/com/github/spotbugs/sonar-findbugs-plugin/4.3.0/sonar-findbugs-plugin-4.3.0.jar

README.md

@@ -1,6 +1,6 @@
 # SonarQube Spotbugs Plugin
 [![.github/workflows/build.yml](/~https://github.com/spotbugs/sonar-findbugs/actions/workflows/build.yml/badge.svg)](/~https://github.com/spotbugs/sonar-findbugs/actions/workflows/build.yml)
-![FindBugs Rules](https://img.shields.io/badge/SpotBugs_rules-944-brightgreen.svg?maxAge=2592000)
+![FindBugs Rules](https://img.shields.io/badge/SpotBugs_rules-948-brightgreen.svg?maxAge=2592000)
 [![Coverage Status](https://sonarcloud.io/api/project_badges/measure?project=com.github.spotbugs%3Asonar-findbugs-plugin&metric=coverage)](https://sonarcloud.io/component_measures?id=com.github.spotbugs:sonar-findbugs-plugin&metric=coverage)
 
 ## Description / Features
@@ -76,4 +76,4 @@ Findbugs Plugin version|Embedded SpotBugs/Findbugs version|Embedded Findsecbugs
 4.2.9                  | 4.8.4 (SpotBugs)                 | 1.13.0                     | 7.6.4 (sb-contrib)        | 1.8|7.9~|5.10.1.16922
 4.2.10                 | 4.8.6 (SpotBugs)                 | 1.13.0                     | 7.6.4 (sb-contrib)        | 1.8|7.9~|5.10.1.16922
 4.3.0                  | 4.8.6 (SpotBugs)                 | 1.13.0                     | 7.6.4 (sb-contrib)        |  17|9.9~|8.0.1.36337
-4.3.1-SNAPSHOT         | 4.9.0 (SpotBugs)                 | 1.13.0                     | 7.6.7 (sb-contrib)        |  17|9.9~|8.0.1.36337
+4.3.1-SNAPSHOT         | 4.9.1 (SpotBugs)                 | 1.13.0                     | 7.6.7 (sb-contrib)        |  17|9.9~|8.0.1.36337

generate_profiles/BuildXmlFiles.groovy

@@ -8,13 +8,13 @@ import groovy.json.JsonSlurper;
 
 @Grapes([
 
-    @Grab(group='com.github.spotbugs', module='spotbugs', version='4.9.0'),
+    @Grab(group='com.github.spotbugs', module='spotbugs', version='4.9.1'),
     @Grab(group='com.mebigfatguy.sb-contrib', module='sb-contrib', version='7.6.7'),
     @Grab(group='com.h3xstream.findsecbugs' , module='findsecbugs-plugin', version='1.13.0')]
 )
 
 
-FB = new Plugin(groupId: 'com.github.spotbugs', artifactId: 'spotbugs', version: '4.9.0')
+FB = new Plugin(groupId: 'com.github.spotbugs', artifactId: 'spotbugs', version: '4.9.1')
 CONTRIB = new Plugin(groupId: 'com.mebigfatguy.sb-contrib', artifactId: 'sb-contrib', version: '7.6.7')
 FSB = new Plugin(groupId: 'com.h3xstream.findsecbugs', artifactId: 'findsecbugs-plugin', version: '1.13.0')
 

src/main/java/org/sonar/plugins/findbugs/rules/FindbugsRulesDefinition.java

@@ -28,7 +28,7 @@ public final class FindbugsRulesDefinition implements RulesDefinition {
 
   public static final String REPOSITORY_KEY = "findbugs";
   public static final String REPOSITORY_NAME = "FindBugs";
-  public static final int RULE_COUNT = 485;
+  public static final int RULE_COUNT = 489;
   public static final int DEACTIVED_RULE_COUNT = 6;
 
   @Override

src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-and-fb-contrib.xml

@@ -1439,6 +1439,9 @@
   <Match>
     <Bug pattern='AA_ASSERTION_OF_ARGUMENTS' />
   </Match>
+  <Match>
+    <Bug pattern='HSM_HIDING_METHOD' />
+  </Match>
   <Match>
     <Bug pattern='PI_DO_NOT_REUSE_PUBLIC_IDENTIFIERS_CLASS_NAMES' />
   </Match>
@@ -1454,6 +1457,15 @@
   <Match>
     <Bug pattern='ENV_USE_PROPERTY_INSTEAD_OF_ENV' />
   </Match>
+  <Match>
+    <Bug pattern='AT_NONATOMIC_64BIT_PRIMITIVE' />
+  </Match>
+  <Match>
+    <Bug pattern='AT_STALE_THREAD_WRITE_OF_PRIMITIVE' />
+  </Match>
+  <Match>
+    <Bug pattern='AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE' />
+  </Match>
   <Match>
     <Bug pattern='ISB_INEFFICIENT_STRING_BUFFERING' />
   </Match>

src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-only.xml

@@ -1439,6 +1439,9 @@
   <Match>
     <Bug pattern='AA_ASSERTION_OF_ARGUMENTS' />
   </Match>
+  <Match>
+    <Bug pattern='HSM_HIDING_METHOD' />
+  </Match>
   <Match>
     <Bug pattern='PI_DO_NOT_REUSE_PUBLIC_IDENTIFIERS_CLASS_NAMES' />
   </Match>
@@ -1454,4 +1457,13 @@
   <Match>
     <Bug pattern='ENV_USE_PROPERTY_INSTEAD_OF_ENV' />
   </Match>
+  <Match>
+    <Bug pattern='AT_NONATOMIC_64BIT_PRIMITIVE' />
+  </Match>
+  <Match>
+    <Bug pattern='AT_STALE_THREAD_WRITE_OF_PRIMITIVE' />
+  </Match>
+  <Match>
+    <Bug pattern='AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE' />
+  </Match>
 </FindBugsFilter>

src/main/resources/org/sonar/plugins/findbugs/rules-findbugs.xml

@@ -6003,6 +6003,23 @@ Using floating-point variables should not be used as loop counters, as they are
         &lt;/p&gt;</description>
     <tag>bad-practice</tag>
   </rule>
+  <rule key='HSM_HIDING_METHOD' priority='MAJOR'>
+    <name>Correctness - Method hiding should be avoided.</name>
+    <configKey>HSM_HIDING_METHOD</configKey>
+    <description>Hiding happens when a subclass defines a static method
+        with same header (signature plus return type) as in any of the super classes.
+        In the event of method hiding the invoked method is determined based on the specific qualified name or
+        method invocation expression used at the calling site.
+        The results are often unexpected, although the Java language provides unambiguous rules for method invocation for method hiding.
+        Moreover, method hiding and method overriding is often confused by programmers.
+        Consequently, programmers should avoid the method hiding.
+        Programmer should declare the respective method non-static or restrict it as private to eradicate the problem.
+        &lt;p&gt;
+        See SEI CERT rule &lt;a href="https://wiki.sei.cmu.edu/confluence/display/java/MET07-J.+Never+declare+a+class+method+that+hides+a+method+declared+in+a+superclass+or+superinterface"&gt;MET07-J. Never declare a class method that hides a method declared in a superclass or superinterface&lt;/a&gt;.
+        &lt;/p&gt;</description>
+    <tag>correctness</tag>
+    <tag>bug</tag>
+  </rule>
   <rule key='PI_DO_NOT_REUSE_PUBLIC_IDENTIFIERS_CLASS_NAMES' priority='MAJOR'>
     <name>Bad practice - Do not reuse public identifiers from JSL as class name</name>
     <configKey>PI_DO_NOT_REUSE_PUBLIC_IDENTIFIERS_CLASS_NAMES</configKey>
@@ -6137,4 +6154,54 @@ Using floating-point variables should not be used as loop counters, as they are
       &lt;/p&gt;</description>
     <tag>bad-practice</tag>
   </rule>
+  <rule key='AT_NONATOMIC_64BIT_PRIMITIVE' priority='MAJOR'>
+    <name>Multi-threading - This write of this 64-bit primitive variable may not atomic</name>
+    <configKey>AT_NONATOMIC_64BIT_PRIMITIVE</configKey>
+    <description>&lt;p&gt;
+          The long and the double are 64-bit primitive types, and depending on the Java Virtual Machine implementation assigning a value to them can be treated as two separate 32-bit writes, and as such it's not atomic.
+          See &lt;a href="https://docs.oracle.com/javase/specs/jls/se8/html/jls-17.html#jls-17.7"&gt;JSL 17.7. Non-Atomic Treatment of double and long&lt;/a&gt;.
+          See SEI CERT rule &lt;a href="https://wiki.sei.cmu.edu/confluence/display/java/VNA05-J.+Ensure+atomicity+when+reading+and+writing+64-bit+values"&gt;VNA05-J. Ensure atomicity when reading and writing 64-bit values&lt;/a&gt; for more info.
+
+          This bug can be ignored in platforms which guarantee that 64-bit long and double type read and write operations are atomic.
+        &lt;/p&gt;
+        &lt;p&gt;
+          To fix it, declare the variable volatile, change the type of the field to the corresponding atomic type from &lt;code&gt;java.lang.concurrent.atomic&lt;\code&gt; or correctly synchronize the code.
+          Declaring the variable volatile may not be enough in some cases: e.g. when the variable is assigned a value which depends on the current value or on the result of nonatomic compound operations.
+        &lt;/p&gt;</description>
+    <tag>multi-threading</tag>
+    <tag>bug</tag>
+  </rule>
+  <rule key='AT_STALE_THREAD_WRITE_OF_PRIMITIVE' priority='MAJOR'>
+    <name>Multi-threading - This write of this shared primitive variable may not be visible to other threads</name>
+    <configKey>AT_STALE_THREAD_WRITE_OF_PRIMITIVE</configKey>
+    <description>&lt;p&gt;
+          &lt;a href="https://wiki.sei.cmu.edu/confluence/display/java/VNA00-J.+Ensure+visibility+when+accessing+shared+primitive+variables"&gt;SEI CERT rule VNA00-J&lt;/a&gt;
+          describes that reading a shared primitive variable in one thread may not yield the value of the most recent write to the variable from another thread.
+          Consequently, the thread may observe a stale value of the shared variable.
+        &lt;/p&gt;
+        &lt;p&gt;
+          To fix it, declare the variable volatile, change the type of the field to the corresponding atomic type from &lt;code&gt;java.lang.concurrent.atomic&lt;\code&gt; or correctly synchronize the code.
+          Declaring the variable volatile may not be enough in some cases: e.g. when the variable is assigned a value which depends on the current value or on the result of nonatomic compound operations.
+          This guarantees that 64-bit primitive long and double variables are accessed atomically.
+        &lt;/p&gt;</description>
+    <tag>multi-threading</tag>
+    <tag>bug</tag>
+  </rule>
+  <rule key='AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE' priority='MAJOR'>
+    <name>Multi-threading - Operation on shared variable is not atomic</name>
+    <configKey>AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE</configKey>
+    <description>&lt;p&gt;
+          This write of a variable shared between functions depends on the current value of the variable (either because it's a compound operation - e.g. +=, ++ - or it simply depends on the current value), as such it consists of more than one discrete operation.
+          These operations are not atomic in themselves and need further synchronization.
+          See &lt;a href="https://wiki.sei.cmu.edu/confluence/display/java/VNA02-J.+Ensure+that+compound+operations+on+shared+variables+are+atomic"&gt;SEI CERT rule VNA02-J&lt;/a&gt;.
+
+          Simply declaring a variable volatile fails to guarantee the atomicity of compound operations on the variable,
+          but synchronizing the writes on top of declaring the variable volatile for read operations is sufficient.
+        &lt;p&gt;
+          To solve this issue, synchronize compound operations and other write operations depending on the previous value,
+          use read-write locks, or declare the shared variable with an atomic type.
+        &lt;/p&gt;</description>
+    <tag>multi-threading</tag>
+    <tag>bug</tag>
+  </rule>
 </rules>

src/test/java/org/sonar/plugins/findbugs/FindbugsProfileImporterTest.java

@@ -132,7 +132,7 @@ void shouldImportCategories() {
     BuiltInQualityProfile profile = context.profile(Java.KEY, TEST_PROFILE);
     Collection<BuiltInActiveRule> results = profile.rules();
 
-    assertThat(results).hasSize(159);
+    assertThat(results).hasSize(160);
     assertThat(findActiveRule(profile, FindbugsRulesDefinition.REPOSITORY_KEY, "BC_IMPOSSIBLE_DOWNCAST")).isNotNull();
   }
 
@@ -182,7 +182,7 @@ void testImportingUncorrectXmlFile() {
   @ParameterizedTest
   @CsvSource({
       "/org/sonar/plugins/findbugs/findbugsXmlWithUnknownRule.xml,1",
-      "/org/sonar/plugins/findbugs/findbugsXmlWithUnknownCategory.xml,159",
+      "/org/sonar/plugins/findbugs/findbugsXmlWithUnknownCategory.xml,160",
       "/org/sonar/plugins/findbugs/findbugsXmlWithUnknownCode.xml,12"})
   void profileImport(String profilePath, int expectedSize) {
   	NewBuiltInQualityProfile newProfile = context.createBuiltInQualityProfile(TEST_PROFILE, Java.KEY);