Upgrade errorprone to 2.44.0

.github/workflows/build.yml

@@ -10,7 +10,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        java: [ '17', '21' ]
+        java: [ '21' ]
     name: Unit tests Java ${{ matrix.Java }}
     steps:
       - uses: actions/checkout@v4

pom.xml

@@ -41,7 +41,7 @@
 		<sonar-java-frontend.version>8.2.0.36672</sonar-java-frontend.version>
 		<sonar-orchestrator.version>5.6.0.2578</sonar-orchestrator.version>
 
-		<errorprone.version>2.39.0</errorprone.version>
+		<errorprone.version>2.44.0</errorprone.version>
 		<nullaway.version>0.12.7</nullaway.version>
 		<errorprone.slf4j.version>0.1.29</errorprone.slf4j.version>
 		<picnic.errorprone.support.version>0.23.0</picnic.errorprone.support.version>
@@ -56,6 +56,7 @@
 		<junit.version>5.10.1</junit.version>
 		<mockito.version>4.5.1</mockito.version>
 		<assertj.version>3.21.0</assertj.version>
+		<jacoco.version>0.8.14</jacoco.version>
 
 		<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
 
@@ -95,7 +96,7 @@
 			<plugin>
 				<groupId>org.jacoco</groupId>
 				<artifactId>jacoco-maven-plugin</artifactId>
-				<version>0.8.11</version>
+				<version>${jacoco.version}</version>
 				<executions>
 					<execution>
 						<id>prepare-agent</id>

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 = 471;
+	public static final int ERRORPRONE_REPOSITORY_RULES_COUNT = 476;
 	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 = 45;

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

@@ -146,6 +146,6 @@ if (set.contains(hi)) {
 }
 ```
 
-[equalstester]: https://static.javadoc.io/com.google.guava/guava-testlib/19.0/com/google/common/testing/EqualsTester.html
-[objeq]: https://docs.oracle.com/javase/7/docs/api/java/lang/Object.html#equals(java.lang.Object)
-[av]: https://github.com/google/auto/blob/master/value/userguide/index.md
+[equalstester]: https://www.javadoc.io/doc/com.google.guava/guava-testlib/latest/com/google/common/testing/EqualsTester.html
+[objeq]: https://docs.oracle.com/en/java/javase/25/docs/api/java.base/java/lang/Object.html#equals(java.lang.Object)
+[av]: https://github.com/google/auto/blob/main/value/userguide/index.md

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

@@ -1,7 +1,7 @@
 Methods that return `java.util.concurrent.Future` and its subclasses generally
 indicate errors by returning a future that eventually fails.
 
-If you don’t check the return value of these methods, you will never find out if
+If you don't check the return value of these methods, you will never find out if
 they threw an exception.
 
 Nested futures can also result in missed cancellation signals or suppressed

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

@@ -1,5 +1,7 @@
-A `main` method must be `public`, `static`, and return `void` (see
-[JLS §12.1.4]).
+## Pre Java 25
+
+Prior to Java 25, a `main` method must be `public`, `static`, and return `void`
+(see [JLS §12.1.4]).
 
 For example, the following method is confusing, because it is an overload of a
 valid `main` method (it has the same name and signature), but is not a valid
@@ -18,7 +20,25 @@ $ java T.java
 error: 'main' method is not declared 'public static'
 ```
 
-[JLS §12.1.4]: https://docs.oracle.com/javase/specs/jls/se11/html/jls-12.html#jls-12.1.4
+## Java 25 and later
+
+For Java 25 and later, a `main` method must return `void` (see [JLS §12.1.4]).
+The `public` and `static` requirements have been dropped.
+
+For example, the following method is confusing, because it is an overload of a
+valid `main` method (it has the same name and arguments), but does not return
+`void`:
+
+```java
+class Test {
+  public static int main(String[] args) {
+    System.err.println("hello world");
+    return 0;
+  }
+}
+```
+
+[JLS §12.1.4]: https://docs.oracle.com/javase/specs/jls/se25/html/jls-12.html#jls-12.1.4
 
 TIP: If you're declaring a method that isn't intended to be used as the main
 method of your program, prefer to use a name other than `main`. It's confusing

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

@@ -1,4 +1,4 @@
 The constructor is annotated with @Inject(optional=true), or it is annotated
 with @Inject and a binding annotation. This will cause a Guice runtime error.
 
-See [https://code.google.com/p/google-guice/wiki/InjectionPoints] for details.
+For more information, see https://github.com/google/guice/wiki/InjectionPoints.

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

@@ -6,8 +6,8 @@ That is, prefer this:
 
 ```java
 boolean isDigits(String string) {
-  for (int i = 0; i < s.length(); i++) {
-    char c = s.charAt(i);
+  for (int i = 0; i < string.length(); i++) {
+    char c = string.charAt(i);
     if (!Character.isDigit(c)) {
       return false;
     }

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

@@ -1,8 +1,8 @@
-If a test subject and the argument to `isEqualTo` are the same instance (e.g.
-`assertThat(x).isEqualTo(x)`), then the assertion will always pass. Truth
-implements `isEqualTo` using [`Objects#equal`] , which tests its arguments for
-reference equality and returns true without calling `equals()` if both arguments
-are the same instance.
+When using Truth, if a test subject and the argument to `isEqualTo` are the same
+instance (for example `assertThat(x).isEqualTo(x)`), then the assertion will
+always pass. Truth implements `isEqualTo` using [`Objects#equal`] , which tests
+its arguments for reference equality and returns true without calling `equals()`
+if both arguments are the same instance.
 
 JUnit's `assertEquals` (and similar) methods are implemented in terms of
 `Object#equals`. However, this is not explicitly documented, so isn't a
@@ -14,4 +14,8 @@ To test the implementation of an `equals` method, use
 [Guava's EqualsTester][javadoc], or explicitly call `equals` as part of the
 test.
 
+In our experience, `assertThat(x).isEqualTo(x)` and similar are *more likely to
+be typos* than assertions about an `equals` method. This alone is sufficient
+motivation to choose a dedicated approach for testing `equals` implementations.
+
 [javadoc]: https://static.javadoc.io/com.google.guava/guava-testlib/21.0/com/google/common/testing/EqualsTester.html

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

@@ -1,5 +1,5 @@
-Suppressions for `unchecked` or `rawtypes` warnings should have an accompanying
-comment to explain why the javac warning is safe to ignore.
+`@SuppressWarnings` should have an accompanying comment to explain why the javac
+warning is safe to ignore.
 
 Rather than just suppressing the warning:
 

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

@@ -1,7 +1,7 @@
 Null-checking `System.console()` is not a reliable way to detect if the console
 is connected to a terminal.
 
-See
+See JDK 22
 [Release Note: JLine As The Default Console Provider](https://bugs.openjdk.org/browse/JDK-8309155):
 
 > `System.console()` now returns a `Console` object when the standard streams
@@ -15,6 +15,18 @@ See
 > A new method `Console.isTerminal()` has been added to test if console is
 > connected to a terminal.
 
+and JDK 25 release note
+[Release Note: Default Console Implementation No Longer Based On JLine](https://bugs.openjdk.org/browse/JDK-8351576):
+
+> The default Console obtained via `System.console()` is no longer based on
+> JLine. Since JDK 20, the JDK has included a JLine-based Console
+> implementation, offering a richer user experience and better support for
+> virtual terminal environments, such as IDEs. This implementation was initially
+> opt-in via a system property in JDK 20 and JDK 21 and became the default in
+> JDK 22. However, maintaining the JLine-based Console proved challenging. As a
+> result, in JDK 25, it has reverted to being opt-in, as it was in JDK 20 and
+> JDK 21.
+
 To prepare for this change while remaining compatible with JDK versions prior to
 JDK 22, consider using reflection to call `Console#isTerminal` on JDK versions
 that support it:
@@ -23,7 +35,7 @@ that support it:
   @SuppressWarnings("SystemConsoleNull") // https://errorprone.info/bugpattern/SystemConsoleNull
   private static boolean systemConsoleIsTerminal() {
     Console systemConsole = System.console();
-    if (Runtime.version().feature() < 22) {
+    if (Runtime.version().feature() < 22 || systemConsole == null) {
       return systemConsole != null;
     }
     try {

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

@@ -4,26 +4,29 @@ examples of error prone issues below:
 #### Constructors
 
 The constructors don't validate the parameters at all, they just "trust" it
-100%.
+100%. \
+This is also true for the static method `Locale.of`, introduced in JDK 19.
 
 For example:
 
-```
-Locale locale = new Locale("en_AU");
-toString()        : "en_au"
-getLanguage()     : "en_au"
-locale.getCountry : ""
-
-locale = new Locale("somethingBad#!34, too long, and clearly not a locale ID");
-toString()    : "somethingbad#!34, too long, and clearly not a locale id"
-getLanguage() : "somethingbad#!34, too long, and clearly not a locale id"
-getCountry()  : ""
+```java
+Locale locale = new Locale("en_AU"); // or Locale.of("en_AU")
+locale.toString();    // "en_au"
+locale.getLanguage(); // "en_au"
+locale.getCountry();  // ""
+
+locale = new Locale("somethingBad#!34, long, and clearly not a locale ID");
+// or Locale.of("somethingBad#!34, long, and clearly not a locale ID")
+locale.toString();    // "somethingbad#!34, long, and clearly not a locale id"
+locale.getLanguage(); // "somethingbad#!34, long, and clearly not a locale id"
+locale.getCountry();  // ""
 ```
 
 As you can see, the full string is interpreted as language, and the country is
 empty.
 
-For `new Locale("zh", "tw", "#Hant")` you get:
+For `new Locale("zh", "tw", "#Hant")` and `Locale.of("zh", "tw", "#Hant")` you
+get:
 
 ```
 toString()    : zh_TW_#Hant
@@ -51,20 +54,70 @@ There's no reliable way of getting a correct result through a `Locale`
 constructor, so we should prefer using `Locale.forLanguageTag()` (and the IETF
 BCP 47 format) for correctness.
 
-**Note:** You might see a `.replace("_", "-")` appended to a suggested fix for
+**Note:** You might see a `.replace('_', '-')` appended to a suggested fix for
 the error prone checker for this bug pattern. This is sanitization measure to
 handle the fact that `Locale.forLanguageTag()` accepts the "minus form" of a tag
 (`en-US`) but not the "underscore form" (`en_US`). It will silently default to
 `Locale.ROOT` if the latter form is passed in.
 
+**Note:** This error-prone rule cannot reliably fix constructors and static
+method `Locale.of` with two or three parameters, because a proper fix requires
+more context.
+
+If the initial code started with a `String` that was split at `'_'` or `'-'`,
+just to be used for locale, the right fix is to use `toLanguageTag()`.
+
+```java
+// Initial code
+void someMethod(String localeId) {
+  String[] parts = localeId.split("_");
+  Locale locale = switch (parts.size) {
+    case 1 -> new Locale(part[0]), // or Locale.of
+    case 2 -> new Locale(part[0], part[1]), // or Locale.of
+    case 3 -> new Locale(part[0], part[1], part[2]), // or Locale.of
+  }
+  // use the locale
+}
+
+// Fixed code
+void someMethod(String localeId) {
+  Locale locale = Locale.forLanguageTag.replace('_', '-');
+  // use the locale
+}
+```
+
+If the initial code started separate "pieces" (language, region, variant) the
+right fix is to use a `Locale.Builder()`.
+
+```java
+// Initial code
+void someMethod(@NotNull String langId, String regionId) {
+  Locale locale (regionId == null)
+      ? new Locale(langId) // or Locale.of
+      : new Locale(langId, regionId); // or Locale.of
+  // use the locale
+}
+
+// Fixed code
+void someMethod(@NotNull String langId, String regionId) {
+  Locale.Builder builder = new Locale.Builder();
+  builder.setLanguage(langId);
+  if (regionId == null) {
+    builder.setCountry(regionId);
+  }
+  Locale locale = builder.build();
+  // use the locale
+}
+```
+
 #### toString()
 
-This poses the inverse of the constructor problem
+This poses the inverse of the constructor problem.
 
-```
+```java
 Locale myLocale = Locale.forLanguageTag("zh-hant-tw")
 String myLocaleStr = myLocale.toString() // zh_TW_#Hant
-Locale derivedLocale = ??? // Not clean way to get a correct locale from this string
+Locale derivedLocale = ??? // Not clean way to get a correct locale from myLocaleStr
 ```
 
 The `toString()` implementation for `Locale` isn't necessarily incorrect in

sonar-erroraway-maven-plugin/src/main/resources/errorprone/bugpattern/javadoc/NotJavadoc.md

@@ -2,6 +2,8 @@ This comment starts with `/**`, but isn't actually Javadoc.
 
 Javadoc comments
 [must precede a class, field, constructor, or method declaration](https://www.oracle.com/technical-resources/articles/java/javadoc-tool.html#format).
+Javadoc cannot be applied to classes or methods nested within a method, as those
+elements never form part of an API.
 
 Using `/**` comments in locations where Javadoc is not supported is confusing
 and unnecessary.

sonar-erroraway-maven-plugin/src/main/resources/errorprone/bugpattern/nullness/NullNeedsCastForVarargs.md

@@ -0,0 +1,2 @@
+Note: The Truth methods covered by this check still accept a literal null in
+Truth 1.4.5, but we expect to change that in a forthcoming version.