Upgrade errorprone to 2.36.0

.github/workflows/build.yml

@@ -24,25 +24,35 @@ jobs:
         run: mvn -B test
   integration-tests:
     name: Integration tests
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
     needs: unit-tests
     strategy:
+      fail-fast: false
       matrix:
         include:
           # 9.9 LTS 
           - SONAR_SERVER_VERSION: 9.9.3.79811
             SONAR_PLUGIN_API_VERSION: 9.14.0.375
             SONAR_PLUGIN_API_GROUPID: org.sonarsource.api.plugin
             SONAR_SERVER_JAVA_VERSION: 17
+            os: ubuntu-latest
           # 10.x 
           - SONAR_SERVER_VERSION: 10.4.0.87286
             SONAR_PLUGIN_API_VERSION: 10.6.0.2114
             SONAR_PLUGIN_API_GROUPID: org.sonarsource.api.plugin
             SONAR_SERVER_JAVA_VERSION: 17
-          - SONAR_SERVER_VERSION: 10.6.0.92116
-            SONAR_PLUGIN_API_VERSION: 10.7.0.2191
+            os: ubuntu-latest
+          - SONAR_SERVER_VERSION: 10.7.0.96327
+            SONAR_PLUGIN_API_VERSION: 10.11.0.2468
+            SONAR_PLUGIN_API_GROUPID: org.sonarsource.api.plugin
+            SONAR_SERVER_JAVA_VERSION: 17
+            os: ubuntu-latest
+          - SONAR_SERVER_VERSION: 24.12.0.100206
+            SONAR_PLUGIN_API_VERSION: 10.14.0.2599
             SONAR_PLUGIN_API_GROUPID: org.sonarsource.api.plugin
             SONAR_SERVER_JAVA_VERSION: 17
+            os: ubuntu-latest
+
     steps:
       - uses: actions/checkout@v4
       - name: Set up JDK
@@ -60,10 +70,22 @@ jobs:
           key: ${{ runner.os }}-sonar-${{ matrix.SONAR_SERVER_VERSION }}
           restore-keys: ${{ runner.os }}-sonar-${{ matrix.SONAR_SERVER_VERSION }}
       - name: Build and Test
-        run: |
-          mvn -B verify \
-          -Dsonar.server.version=${{ matrix.SONAR_SERVER_VERSION }} \
-          -Dsonar-plugin-api.version=${{ matrix.SONAR_PLUGIN_API_VERSION }} \
+        env:
+          SONAR_SCANNER_JAVA_OPTS: >
+            --add-exports jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED
+            --add-exports jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED
+            --add-exports jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED
+            --add-exports jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED
+            --add-exports jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED
+            --add-exports jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED
+            --add-exports jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED
+            --add-exports jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+            --add-opens jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED
+            --add-opens jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED
+        run: >
+          mvn -B verify
+          -Dsonar.server.version=${{ matrix.SONAR_SERVER_VERSION }}
+          -Dsonar-plugin-api.version=${{ matrix.SONAR_PLUGIN_API_VERSION }}
           -Dsonar-plugin-api.groupId=${{ matrix.SONAR_PLUGIN_API_GROUPID }}
   build:
     name: Build

.gitignore

@@ -3,3 +3,6 @@
 /.classpath
 /.project
 /dependency-reduced-pom.xml
+**/org.eclipse.jdt.core.prefs
+**/.settings
+**/.factorypath

README.md

@@ -34,6 +34,8 @@ When running on JDK 16 or newer add the following options due to [JEP 396](https
 
 See [.mvn/jvm.config](sonar-erroraway-sonar-plugin/src/test/resources/projects/simple/.mvn/jvm.config) for a way to do it with Maven and [gradle.properties](sonar-erroraway-sonar-plugin/src/test/resources/projects/simple/gradle.properties) for a way to do it with Gradle
 
+From SonarQybe 10.6 the scanner also auto provisions a JRE and runs the analysis off that JVM. Since the JRE does not include the required compiler module, this needs to be disabled with `sonar.scanner.skipJreProvisioning=true`.
+
 When these options are not set you will receive errors: 
 ```
 Exception in thread "main" java.util.ServiceConfigurationError: com.google.errorprone.bugpatterns.BugChecker: Provider ... could not be instantiated

pom.xml

@@ -41,7 +41,7 @@
 		<sonar-java-frontend.version>8.2.0.36672</sonar-java-frontend.version>
 		<sonar-orchestrator.version>4.7.0.1861</sonar-orchestrator.version>
 
-		<errorprone.version>2.31.0</errorprone.version>
+		<errorprone.version>2.36.0</errorprone.version>
 		<nullaway.version>0.11.2</nullaway.version>
 		<errorprone.slf4j.version>0.1.20</errorprone.slf4j.version>
 		<picnic.errorprone.support.version>0.18.0</picnic.errorprone.support.version>

sonar-erroraway-lib/src/main/java/com/github/erroraway/rules/ErrorAwayRulesMapping.java

@@ -34,7 +34,7 @@ public final class ErrorAwayRulesMapping {
 	public static final String ERRORPRONE_SLF4J_REPOSITORY = "errorprone-slf4j";
 	public static final String PICNIC_REPOSITORY = "picnic-errorprone";
 
-	public static final int ERRORPRONE_REPOSITORY_RULES_COUNT = 447;
+	public static final int ERRORPRONE_REPOSITORY_RULES_COUNT = 455;
 	public static final int NULLAWAY_REPOSITORY_RULES_COUNT = 1;
 	public static final int ERRORPRONE_SLF4J_REPOSITORY_RULES_COUNT = 8;
 	public static final int PICNIC_REPOSITORY_RULES_COUNT = 39;

sonar-erroraway-sonar-plugin/src/main/resources/errorprone/bugpattern/ArrayRecordComponent.md

@@ -0,0 +1,12 @@
+There are two main problems with having a component of a record be an array.
+
+1.  By default, the generated `equals` and `hashCode` will just call `equals` or
+    `hashCode` on the array. Two distinct arrays are never considered equal by
+    `equals` even if their contents are the same. The generated `toString` is
+    similarly not useful, since it will be something like `[B@723279cf`.
+
+2.  Arrays are mutable, but records should not be mutable. A client of a record
+    with an array component can change the contents of the array.
+
+Instead of an array component, consider something like `ImmutableList<String>`,
+or, for primitive arrays, something like `ByteString` or `ImmutableIntArray`.

sonar-erroraway-sonar-plugin/src/main/resources/errorprone/bugpattern/DuplicateBranches.md

@@ -0,0 +1,32 @@
+Branching constructs (`if` statements, `conditional` expressions) should contain
+difference code in the two branches. Repeating identical code in both branches
+is usually a bug.
+
+For example:
+
+```java
+condition ? same : same
+```
+
+```java
+if (condition) {
+  same();
+} else {
+  same();
+}
+```
+
+this usually indicates a typo where one of the branches was supposed to contain
+different logic:
+
+```java
+condition ? something : somethingElse
+```
+
+```java
+if (condition) {
+  doSomething();
+} else {
+  doSomethingElse();
+}
+```

sonar-erroraway-sonar-plugin/src/main/resources/errorprone/bugpattern/EmptyCatch.md

@@ -0,0 +1,30 @@
+The [Google Java Style Guide §6.2][style] states:
+
+> It is very rarely correct to do nothing in response to a caught exception.
+> (Typical responses are to log it, or if it is considered "impossible", rethrow
+> it as an AssertionError.)
+>
+> When it truly is appropriate to take no action whatsoever in a catch block,
+> the reason this is justified is explained in a comment.
+
+When writing tests that expect an exception to be thrown, prefer using
+[`Assert.assertThrows`][assertthrows] instead of writing a try-catch. That is,
+prefer this:
+
+```java
+assertThrows(NoSuchElementException.class, () -> emptyStack.pop());
+```
+
+instead of this:
+
+```java
+try {
+  emptyStack.pop();
+  fail();
+} catch (NoSuchElementException expected) {
+}
+```
+
+[style]: https://google.github.io/styleguide/javaguide.html#s6.2-caught-exceptions
+
+[assertthrows]: https://junit.org/junit4/javadoc/latest/org/junit/Assert.html#assertThrows(java.lang.Class,%20org.junit.function.ThrowingRunnable)

sonar-erroraway-sonar-plugin/src/main/resources/errorprone/bugpattern/FutureTransformAsync.md

@@ -0,0 +1,6 @@
+The usage of `transformAsync` is not necessary when all the return values of the
+transformation function are immediate futures. In this case, the usage of
+`transform` is preferred.
+
+Note that `transform` cannot be used if the body of the transformation function
+throws checked exceptions.

sonar-erroraway-sonar-plugin/src/main/resources/errorprone/bugpattern/MemberName.md → sonar-erroraway-sonar-plugin/src/main/resources/errorprone/bugpattern/IdentifierName.md

@@ -1,4 +1,4 @@
-The [Google Java Style Guide §5.2][style] provides rules for naming nembers.
+The [Google Java Style Guide §5.2][style] provides rules for naming identifiers.
 
 ## Test methods
 

sonar-erroraway-sonar-plugin/src/main/resources/errorprone/bugpattern/MisleadingEscapedSpace.md

@@ -0,0 +1,22 @@
+When Java introduced text blocks as a feature, it also introduced a new string
+escape sequence `\s`. This escape sequence is another way to write a normal
+space, but it has the advantage that it can be used at the end of a line in a
+text block, where a normal space would be stripped.
+
+This new escape sequence can easily be confused with the regex `\s`, which is a
+metacharacter that matches any kind of whitespace character. To write that
+metacharacter in a Java string, you must still write `\\s`: an escaped backslash
+followed by an `s`.
+
+There is little reason to ever write the Java escape `\s` except at the end of a
+line. Either use a normal space, or switch to `\\s` if you are trying to write
+the regex metacharacter.
+
+```java
+// Each line here is five characters long.
+String colors = """
+    one \s
+    two \s
+    three
+    """;
+```

sonar-erroraway-sonar-plugin/src/main/resources/errorprone/bugpattern/MissingFail.md

@@ -1,4 +1,4 @@
-When testing for exceptions in junit, it is easy to forget the call to `fail()`:
+When testing for exceptions in JUnit, it is easy to forget the call to `fail()`:
 
 ```java
 try {
@@ -16,7 +16,7 @@ import static org.junit.Assert.fail;
 
 try {
   someOperationThatShouldThrow();
-  fail()
+  fail();
 } catch (SomeException expected) {
   assertThat(expected).hasMessage("Operation failed");
 }
@@ -81,5 +81,5 @@ characteristics are present:
 
 *   A field assignment in the catch block.
 *   A call to `assertTrue/False(boolean variable or field)` in the catch block.
-*   The last statement in the `try` block is an `assert*()` (that is not a noop:
-    `assertFalse(false)`, `assertTrue(true))` or `Mockito.verify()` call.
+*   The last statement in the `try` block is an `assert*()` (that is not a
+    noop): `assertFalse(false)`, `assertTrue(true))` or `Mockito.verify()` call.

sonar-erroraway-sonar-plugin/src/main/resources/errorprone/bugpattern/OptionalNotPresent.md

@@ -1,8 +1,8 @@
 Calling `get()` on an `Optional` that is not present will result in a
 `NoSuchElementException`.
 
-This check detects cases where `get()` is called whent the optional is
-definitely not present, e.g.:
+This check detects cases where `get()` is called when the optional is definitely
+not present, e.g.:
 
 ```java
 if (!o.isPresent()) {

sonar-erroraway-sonar-plugin/src/main/resources/errorprone/bugpattern/RedundantControlFlow.md

@@ -0,0 +1,54 @@
+Unnecessary control flow statements may be misleading in some contexts.
+
+For instance, this method to find groups of prime order has a bug:
+
+```java
+static ImmutableList<Group> filterGroupsOfPrimeOrder(Iterable<Group> groups) {
+  ImmutableList.Builder<Group> filtered = ImmutableList.builder();
+  for (Group group : groups) {
+    for (int i = 2; i < group.order(); i++) {
+      if (group.order() % i == 0) {
+        continue;
+      }
+    }
+    filtered.add(group);
+  }
+  return filtered.build();
+}
+```
+
+The `continue` statement only breaks out of the innermost loop, so the input is
+returned unchanged.
+
+The most readable alternative is probably to avoid a nested loop entirely:
+
+```java
+static ImmutableList<Group> filterGroupsOfPrimeOrder(Iterable<Group> groups) {
+  ImmutableList.Builder<Group> filtered = ImmutableList.builder();
+  for (Group group : groups) {
+    if (!isPrime(group.order())) {
+      continue;
+    }
+    filtered.add(group);
+  }
+  return filtered.build();
+}
+```
+
+A labelled break statement would also be correct, but is quite uncommon:
+
+```java
+static ImmutableList<Group> filterGroupsOfPrimeOrder(Iterable<Group> groups) {
+  ImmutableList.Builder<Group> filtered = ImmutableList.builder();
+  outer:
+  for (Group group : groups) {
+    for (int i = 2; i < group.order(); i++) {
+      if (group.order() % i == 0) {
+        continue outer;
+      }
+    }
+    filtered.add(group);
+  }
+  return filtered.build();
+}
+```

sonar-erroraway-sonar-plugin/src/main/resources/errorprone/bugpattern/StringConcatToTextBlock.md

@@ -0,0 +1,56 @@
+Using [text blocks] for strings that span multiple lines can make code easier to
+read.
+
+For example, prefer this:
+
+```java
+String message =
+    """
+    'The time has come,' the Walrus said,
+    'To talk of many things:
+    Of shoes -- and ships -- and sealing-wax --
+    Of cabbages -- and kings --
+    And why the sea is boiling hot --
+    And whether pigs have wings.'
+    """;
+```
+
+instead of this:
+
+```java
+String message =
+    "'The time has come,' the Walrus said,\n"
+        + "'To talk of many things:\n"
+        + "Of shoes -- and ships -- and sealing-wax --\n"
+        + "Of cabbages -- and kings --\n"
+        + "And why the sea is boiling hot --\n"
+        + "And whether pigs have wings.'\n";
+```
+
+## Trailing newlines
+
+If the string should not contain a trailing newline, use a `\` to escape the
+final newline in the text block. That is, these two strings are equivalent:
+
+```java
+String s = "hello\n" + "world";
+```
+
+```java
+String s =
+    """
+    hello
+    world\
+    """;
+```
+
+The suggested fixes for this check preserve the exact contents of the original
+string, so if the original string doesn't include a trailing newline the fix
+will use a `\` to escape the last newline.
+
+If the whitespace in the string isn't significant, for example because the
+string value will be parsed by a parser that doesn't care about the trailing
+newlines, consider removing the final `\` to improve the readability of the
+string.
+
+[text blocks]: https://docs.oracle.com/en/java/javase/23/text-blocks/index.html

sonar-erroraway-sonar-plugin/src/test/java/com/github/erroraway/sonarqube/it/ErrorAwayIT.java

@@ -88,6 +88,10 @@ private static String getSonarWebPort() {
 		return System.getProperty("sonar.web.port", "9000");
 	}
 
+	private String getScannerJvmOptions() {
+		return "--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED --add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED";
+	}
+
 	@AfterAll
 	public static void stopOrchestrator() {
 		ORCHESTRATOR.stop();
@@ -119,7 +123,9 @@ void analyzeSimpleMavenProject() {
 				.setProperty("sonar.login", "admin")
 				.setProperty("sonar.password", "admin")
 				.setProperty("sonar.web.port", getSonarWebPort())
-				.setGoals("clean package org.sonarsource.scanner.maven:sonar-maven-plugin:sonar -Dsonar.plugins.downloadOnlyRequired=false");
+				.setProperty("sonar.scanner.javaOpts", getScannerJvmOptions())
+				.setProperty("sonar.scanner.skipJreProvisioning", "true")
+				.setGoals("clean package org.sonarsource.scanner.maven:sonar-maven-plugin:sonar");
 
 		ORCHESTRATOR.executeBuild(build);
 
@@ -137,9 +143,10 @@ void analyzeSimpleGradleProject() {
 				.setProperty("sonar.login", "admin")
 				.setProperty("sonar.password", "admin")
 				.setProperty("sonar.web.port", getSonarWebPort())
+				.setProperty("sonar.scanner.javaOpts", getScannerJvmOptions())
+				.setProperty("sonar.scanner.skipJreProvisioning", "true")
 				.setTasks("clean", "build")
 				.addArgument("--stacktrace")
-				.addArgument("-Dsonar.plugins.downloadOnlyRequired=false")
 				.addSonarTask();
 
 		ORCHESTRATOR.executeBuild(build);