Upgrade errorprone to 2.39.0 (#68)

Author: gtoison

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.38.0</errorprone.version>
+		<errorprone.version>2.39.0</errorprone.version>
 		<nullaway.version>0.12.6</nullaway.version>
 		<errorprone.slf4j.version>0.1.28</errorprone.slf4j.version>
 		<picnic.errorprone.support.version>0.23.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 = 468;
+	public static final int ERRORPRONE_REPOSITORY_RULES_COUNT = 471;
 	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/AndroidJdkLibsChecker.md

@@ -3,7 +3,7 @@ only the latest or unreleased devices can handle
 
 ## Suppression
 
-WARNING: We _strongly_ recommend checking your code with Android Lint if
+WARNING: We *strongly* recommend checking your code with Android Lint if
 suppressing or disabling this check.
 
 The check can be suppressed in code that deliberately only targets newer Android

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

@@ -0,0 +1,16 @@
+Lenient format strings, such as those accepted by `Preconditions`, are often
+constructed lazily. The message is rarely needed, so it should either be cheap
+to construct or constructed only when needed. This check ensures that these
+messages are not constructed using expensive methods that are evaluated eagerly.
+
+Prefer this:
+
+```java
+checkNotNull(foo, "hello %s", name);
+```
+
+instead of this:
+
+```java
+checkNotNull(foo, String.format("hello %s", name));
+```

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

@@ -28,5 +28,7 @@ or map negative numbers onto the non-negative range:
 
 ```java
 long lng = r.nextLong();
-lng = (lng == Long.MIN_VALUE) ? 0 : Math.abs(lng);
+
+long bestForHashCodes = lng & Long.MAX_VALUE;
+long bestForMath = LongMath.saturatedAbs(lng);
 ```

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

@@ -29,3 +29,41 @@ switch statement on an enum type to either handle all values of the enum, or
 have a default statement group.
 
 [style]: https://google.github.io/styleguide/javaguide.html#s4.8.4.3-switch-default
+
+## Library skew
+
+If libraries are compiled against different versions of the same enum it's
+possible for the switch statement to encounter an enum value despite it
+otherwise being thought to be exhaustive. If there is no default branch code
+execution will simply fall out of the switch statement.
+
+Since developers may have assumed this to be impossible, it may be helpful to
+add a default branch when library skew is a concern, however, you may not want
+to give up checking to ensure that all cases are handled. Therefore if a default
+branch exists with a comment containing "skew", the default will not be
+considered for exhaustiveness. For example:
+
+```java
+enum TrafficLightColour { RED, GREEN, YELLOW }
+
+void approachIntersection(TrafficLightColour state) {
+  switch (state) {
+    case GREEN:
+      proceed();
+      break;
+    case YELLOW:
+    case RED:
+      stop();
+      break;
+    default: // In case of skew we may get an unknown value, always stop.
+      stop();
+      break;
+  }
+}
+```
+
+In this case the default branch is providing runtime safety for unknown enum
+values while also still enforcing that all known enum values are handled.
+
+Note: The [UnnecessaryDefaultInEnumSwitch](UnnecessaryDefaultInEnumSwitch.md)
+check will not classify the default as unnecessary if it has the "skew" comment.

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

@@ -14,15 +14,15 @@ misinterpreted.
 For example, consider this:
 
 ```java
-boolean d = (a && b) || c;",
-boolean e = (a || b) ? c : d;",
-int z = (x + y) << 2;",
+boolean d = (a && b) || c;
+boolean e = (a || b) ? c : d;
+int z = (x + y) << 2;
 ```
 
 Instead of this:
 
 ```java
-boolean r = a && b || c;",
-boolean e = a || b ? c : d;",
-int z = x + y << 2;",
+boolean r = a && b || c;
+boolean e = a || b ? c : d;
+int z = x + y << 2;
 ```

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

@@ -1,28 +1,34 @@
-We're trying to make `switch` statements simpler to understand at a glance.
-Misunderstanding the control flow of a `switch` block is a common source of
-bugs.
+We're trying to make `switch`es simpler to understand at a glance.
+Misunderstanding the control flow of a `switch` is a common source of bugs.
 
-### Statement `switch` statements:
+As part of this simplification, new-style arrow (`->`) switches are encouraged
+instead of old-style colon (`:`) switches. And where possible, neighboring cases
+are grouped together (e.g. `case A, B, C`).
 
-*   Have a colon between the `case` and the case's code. For example, `case
+### Old-style colon (`:`) `switch`es:
+
+*   Have a colon between the `case` and the `case`'s code. For example, `case
     HEARTS:`
 *   Because of the potential for fall-through, it takes time and cognitive load
-    to understand the control flow for each `case`
-*   When a `switch` block is large, just skimming each `case` can be toilsome
-*   Fall-though can also be conditional (see example below). In this scenario,
-    one would need to reason about all possible flows for each `case`. When
-    conditionally falling-through multiple `case`s in a row is possible, the
-    number of potential control flows can grow rapidly
-
-### Expression `switch` statements
-
-*   Have an arrow between the `case` and the case's code. For example, `case
+    to understand the control flow. When a `switch` block is large, just
+    skimming each `case` can be toilsome. Fall-through can also be conditional
+    (see example 5. below). In this scenario, one would potentially need to
+    reason about all possible flows for each `case`. When conditionally
+    falling-through multiple `case`s, the number of potential control flows can
+    grow rapidly
+*   Lexical scopes overlap, which can lead to surprising behaviors: definitions
+    of local variables from earlier `case`s are propagated down to later
+    `case`s, however the *values* that initialize those local variables do not
+    propagate in the same way
+
+### New-style arrow (`->`) `switch`es:
+
+*   Have an arrow between the `case` and the `case`'s code. For example, `case
     HEARTS ->`
-*   With an expression `switch` statement, you know at a glance that no cases
-    fall through. No control flow analysis needed
+*   No `case`s fall through; no control flow analysis needed
 *   Safely and easily reorder `case`s (within a `switch`)
-*   It's also possible to group identical cases together (`case A, B, C`) for
-    improved readability
+*   Lexical scopes are isolated between different `case`s; if you define a local
+    variable within a `case`, it can only be used within that specific `case`.
 
 ### Examples
 
@@ -48,7 +54,7 @@ private void foo(Suit suit) {
 }
 ```
 
-Which can be simplified into the following expression `switch`:
+Which can be simplified by grouping and using a new-style switch:
 
 ``` {.good}
 enum Suit {HEARTS, CLUBS, SPADES, DIAMONDS};
@@ -65,16 +71,14 @@ private void foo(Suit suit) {
 }
 ```
 
-#### 2. Return switch
+#### 2. `return switch ...`
 
-Sometimes `switch` is used with `return`. Below, even though a `case` is
-specified for each possible value of the `enum`, note that we nevertheless need
-a "should never happen" clause:
+Sometimes `switch` is used with a `return` for each `case`, like this:
 
 ``` {.bad}
 enum SideOfCoin {OBVERSE, REVERSE};
 
-private String foo(SideOfCoin sideOfCoin) {
+private String renderName(SideOfCoin sideOfCoin) {
   switch(sideOfCoin) {
     case OBVERSE:
       return "Heads";
@@ -86,43 +90,45 @@ private String foo(SideOfCoin sideOfCoin) {
 }
 ```
 
-Using an expression switch simplifies the code and removes the need for an
-explicit "should never happen" clause.
+Note that even though a `case` is present for each possible value of the `enum`,
+a boilerplate "should never happen" clause is still needed. The transformed code
+is simpler and doesn't need a "should never happen" clause.
 
 ```
 enum SideOfCoin {OBVERSE, REVERSE};
 
-private String foo(SideOfCoin sideOfCoin) {
+private String renderName(SideOfCoin sideOfCoin) {
   return switch(sideOfCoin) {
     case OBVERSE -> "Heads";
     case REVERSE -> "Tails";
   };
 }
 ```
 
-If you nevertheless wish to have an explicit "should never happen" clause, this
-can be accomplished by placing the logic under a `default` case. For example:
+If you nevertheless wish to define an explicit "should never happen" clause,
+this can be accomplished by placing the logic inside a `default` case. For
+example:
 
 ```
-
 enum SideOfCoin {OBVERSE, REVERSE};
 
 private String foo(SideOfCoin sideOfCoin) {
   return switch(sideOfCoin) {
     case OBVERSE -> "Heads";
     case REVERSE -> "Tails";
-    default -> {
-      // This should never happen
-      throw new RuntimeException("Unknown side of coin");
-    }
+    default -> throw new RuntimeException("Unknown side of coin"); // should never happen
   };
 }
 ```
 
-#### 3. Assignment switch
+When the checker detects an existing `default` that appears to be redundant, it
+may suggest a secondary auto-fix which removes the redundant `default` and its
+code (if any).
+
+#### 3. Assignment `switch`
 
-If every branch of a `switch` is making an assignment to the same variable, it
-can be re-written as an assignment switch:
+If every branch of a `switch` is making an assignment to the same variable, the
+code can be simplified into a combined assignment and `switch`:
 
 ``` {.bad}
 enum Suit {HEARTS, CLUBS, SPADES, DIAMONDS};
@@ -157,14 +163,107 @@ private void updateScore(Suit suit) {
     case HEARTS, DIAMONDS -> -1;
     case SPADES -> 2;
     case CLUBS -> 3;
-    };
+  };
+}
+```
+
+Taking this one step further: if a local variable is defined, and then
+immediately followed by a `switch` in which every `case` assigns to that same
+variable, then all three (the `switch`, the variable declaration, and the
+assignment) can be merged:
+
+``` {.bad}
+enum Suit {HEARTS, CLUBS, SPADES, DIAMONDS};
+
+private void updateStatus(Suit suit) {
+  int score;
+
+  switch(suit) {
+    case HEARTS:
+    // Fall thru
+    case DIAMONDS:
+      score = 1;
+      break;
+    case SPADES:
+      score = 2;
+      break;
+    case CLUBS:
+      score = 3;
+    }
+  ...
+
+}
+```
+
+Becomes:
+
+```
+enum Suit {HEARTS, CLUBS, SPADES, DIAMONDS};
+
+private void updateStatus(Suit suit) {
+  int score = switch(suit) {
+    case HEARTS, DIAMONDS -> 1;
+    case SPADES -> 2;
+    case CLUBS -> 3;
+  };
+  ...
 }
 ```
 
-#### 4. Complex control flows
+#### 4. Just converting to new arrow `switch`
+
+Even when the simplifications discussed above are not applicable, conversion to
+new arrow `switch`es can be automated by this checker:
+
+``` {.bad}
+enum Suit {HEARTS, CLUBS, SPADES, DIAMONDS};
+
+private void processEvent(Suit suit) {
+    switch (suit) {
+      case CLUBS:
+        String message = "hello";
+        var anotherMessage = "salut";
+        processMessages(message, anotherMessage);
+        break;
+      case DIAMONDS:
+        anotherMessage = "bonjour";
+        processMessage(anotherMessage);
+    }
+}
+```
+
+Note that the local variables referenced in multiple cases are hoisted up out of
+the `switch` statement, and `var` declarations are converted to explicit types,
+resulting in:
+
+```
+enum Suit {HEARTS, CLUBS, SPADES, DIAMONDS};
+
+private void processEvent(Suit suit) {
+    String anotherMessage;
+    switch (suit) {
+      case CLUBS -> {
+        String message = "hello";
+        anotherMessage = "salut";
+        processMessages(message, anotherMessage);
+      }
+      case DIAMONDS -> {
+        anotherMessage = "bonjour";
+        processMessage(anotherMessage);
+      }
+    }
+}
+```
+
+#### 5. Complex control flows
 
 Here's an example of a complex statement `switch` with conditional fall-through
-and complex control flows. How many potential execution paths can you spot?
+and various control flows. Unfortunately, the checker does not currently have
+the ability to automatically convert such code to new-style arrow `switch`es.
+Manually converting the code could be a good opportunity to improve its
+readability.
+
+How many potential execution paths can you spot?
 
 ``` {.bad}
 enum Suit {HEARTS, CLUBS, SPADES, DIAMONDS};