Warn about throwIfUnchecked(unchecked), which could be just `throw …

…unchecked`.

This was prompted by google/guava#7434, in which the correct fix was `throwIfUnchecked(unchecked.getCause())`. (Compare [one other instance of that](https://github.com/googleapis/google-auth-library-java/blob/f154edb3d8503d29f0020b6904dfa40e034ded93/oauth2_http/java/com/google/auth/oauth2/ServiceAccountJwtAccessCredentials.java#L373).) But most hits in practice appear to be `catch (RuntimeException e) { throwIfUnchecked(e); }`, which isn't a bug, just unnecessary code (which can typically be further simplified sometimes as far as to remove the `try`-`catch` block entirely, as suggested by the Google-internal `RethrowException` check).

This check could be written with Refaster (unknown commit), _but_:

- That would prevent it from running at compile time.
- That would in practice prevent us from open-sourcing it.
- We'd need to use placeholders if we want to continue to remove the code after `throwIfUnchecked`, and placeholders can have slow runtime and won't remove code that has comments. (This check doesn't remove comments, either, but at least it will remove the code around them.)
- Refaster can't handle `RuntimeException | Error e`, which comes up a couple times in our depot.
- To cover all the cases we can correctly, the Refaster templates need to be written in just the right order (expression templates before block templates), and they end up about as complicated as this plain old Error Prone check.

PiperOrigin-RevId: 686901145

core/src/main/java/com/google/errorprone/bugpatterns/ThrowIfUncheckedKnownUnchecked.java

@@ -0,0 +1,105 @@
+/*
+ * Copyright 2016 The Error Prone Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package com.google.errorprone.bugpatterns;
+
+import static com.google.errorprone.BugPattern.SeverityLevel.WARNING;
+import static com.google.errorprone.matchers.Description.NO_MATCH;
+import static com.google.errorprone.matchers.Matchers.allOf;
+import static com.google.errorprone.matchers.Matchers.anyOf;
+import static com.google.errorprone.matchers.Matchers.argument;
+import static com.google.errorprone.matchers.Matchers.argumentCount;
+import static com.google.errorprone.matchers.method.MethodMatchers.staticMethod;
+import static com.sun.source.tree.Tree.Kind.BLOCK;
+import static com.sun.source.tree.Tree.Kind.EXPRESSION_STATEMENT;
+
+import com.google.errorprone.BugPattern;
+import com.google.errorprone.VisitorState;
+import com.google.errorprone.bugpatterns.BugChecker.MethodInvocationTreeMatcher;
+import com.google.errorprone.fixes.SuggestedFix;
+import com.google.errorprone.matchers.Description;
+import com.google.errorprone.matchers.Matcher;
+import com.google.errorprone.util.ASTHelpers;
+import com.sun.source.tree.BlockTree;
+import com.sun.source.tree.ExpressionTree;
+import com.sun.source.tree.MethodInvocationTree;
+import com.sun.tools.javac.code.Symtab;
+import com.sun.tools.javac.code.Type;
+import com.sun.tools.javac.code.Types;
+import javax.lang.model.type.UnionType;
+
+/** A BugPattern; see the summary. */
+@BugPattern(
+    summary = "`throwIfUnchecked(knownUnchecked)` is equivalent to `throw knownUnchecked`.",
+    severity = WARNING)
+public class ThrowIfUncheckedKnownUnchecked extends BugChecker
+    implements MethodInvocationTreeMatcher {
+
+  private static final Matcher<MethodInvocationTree> IS_THROW_IF_UNCHECKED =
+      allOf(
+          anyOf(
+              staticMethod().onClass("com.google.common.base.Throwables").named("throwIfUnchecked"),
+              staticMethod()
+                  .onClass("com.google.common.base.Throwables")
+                  .named("propagateIfPossible")),
+          argumentCount(1));
+
+  private static final Matcher<ExpressionTree> IS_KNOWN_UNCHECKED =
+      new Matcher<ExpressionTree>() {
+        @Override
+        public boolean matches(ExpressionTree tree, VisitorState state) {
+          Type type = ASTHelpers.getType(tree);
+          if (type.isUnion()) {
+            return ((UnionType) type)
+                .getAlternatives().stream().allMatch(t -> isKnownUnchecked(state, (Type) t));
+          } else {
+            return isKnownUnchecked(state, type);
+          }
+        }
+
+        boolean isKnownUnchecked(VisitorState state, Type type) {
+          Types types = state.getTypes();
+          Symtab symtab = state.getSymtab();
+          // Check erasure for generics.
+          // TODO(cpovirk): Is that necessary here or in ThrowIfUncheckedKnownChecked?
+          type = types.erasure(type);
+          return types.isSubtype(type, symtab.errorType)
+              || types.isSubtype(type, symtab.runtimeExceptionType);
+        }
+      };
+
+  @Override
+  public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) {
+    if (IS_THROW_IF_UNCHECKED.matches(tree, state)
+        && argument(0, IS_KNOWN_UNCHECKED).matches(tree, state)) {
+      var fix = SuggestedFix.builder();
+      fix.replace(tree, "throw " + state.getSourceForNode(tree.getArguments().get(0)));
+      /*
+       * Changing to `throw ...` make the compiler recognize everything afterward in the block as
+       * unreachable. To avoid build errors from that, we remove everything afterward.
+       *
+       * We might still produce build errors if code *after* the block becomes unreachable (because
+       * it's now possible to fall out of this block). That seems tolerable.
+       */
+      var parent = state.getPath().getParentPath().getLeaf();
+      var grandparent = state.getPath().getParentPath().getParentPath().getLeaf();
+      if (parent.getKind() == EXPRESSION_STATEMENT && grandparent.getKind() == BLOCK) {
+        ((BlockTree) grandparent)
+            .getStatements().stream().dropWhile(t -> t != parent).skip(1).forEach(fix::delete);
+      }
+      return describeMatch(tree, fix.build());
+    }
+    return NO_MATCH;
+  }
+}

core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java

@@ -380,6 +380,7 @@
 import com.google.errorprone.bugpatterns.ThreadLocalUsage;
 import com.google.errorprone.bugpatterns.ThreeLetterTimeZoneID;
 import com.google.errorprone.bugpatterns.ThrowIfUncheckedKnownChecked;
+import com.google.errorprone.bugpatterns.ThrowIfUncheckedKnownUnchecked;
 import com.google.errorprone.bugpatterns.ThrowNull;
 import com.google.errorprone.bugpatterns.ThrowSpecificExceptions;
 import com.google.errorprone.bugpatterns.ThrowsUncheckedException;
@@ -1086,6 +1087,7 @@ public static ScannerSupplier warningChecks() {
           ThreadLocalUsage.class,
           ThreadPriorityCheck.class,
           ThreeLetterTimeZoneID.class,
+          ThrowIfUncheckedKnownUnchecked.class,
           TimeUnitConversionChecker.class,
           ToStringReturnsNull.class,
           TraditionalSwitchExpression.class,

core/src/test/java/com/google/errorprone/bugpatterns/ThrowIfUncheckedKnownUncheckedTest.java

@@ -0,0 +1,157 @@
+/*
+ * Copyright 2024 The Error Prone Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+ * in compliance with the License. You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package com.google.errorprone.bugpatterns;
+
+import com.google.errorprone.BugCheckerRefactoringTestHelper;
+import com.google.errorprone.CompilationTestHelper;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class ThrowIfUncheckedKnownUncheckedTest {
+  private final CompilationTestHelper compilationHelper =
+      CompilationTestHelper.newInstance(ThrowIfUncheckedKnownUnchecked.class, getClass());
+  private final BugCheckerRefactoringTestHelper refactoringHelper =
+      BugCheckerRefactoringTestHelper.newInstance(ThrowIfUncheckedKnownUnchecked.class, getClass());
+
+  @Test
+  public void knownRuntimeException() {
+    compilationHelper
+        .addSourceLines(
+            "Foo.java",
+            """
+            import static com.google.common.base.Throwables.throwIfUnchecked;
+
+            class Foo {
+              void x(IllegalArgumentException e) {
+                // BUG: Diagnostic contains:
+                throwIfUnchecked(e);
+              }
+            }
+            """)
+        .doTest();
+  }
+
+  @Test
+  public void knownError() {
+    compilationHelper
+        .addSourceLines(
+            "Foo.java",
+            """
+            import static com.google.common.base.Throwables.throwIfUnchecked;
+
+            class Foo {
+              void x(LinkageError e) {
+                // BUG: Diagnostic contains:
+                throwIfUnchecked(e);
+              }
+            }
+            """)
+        .doTest();
+  }
+
+  @Test
+  public void knownUncheckedMulticatch() {
+    compilationHelper
+        .addSourceLines(
+            "Foo.java",
+            """
+            import static com.google.common.base.Throwables.throwIfUnchecked;
+
+            class Foo {
+              void x() {
+                try {
+                } catch (RuntimeException | Error e) {
+                  // BUG: Diagnostic contains:
+                  throwIfUnchecked(e);
+                }
+              }
+            }
+            """)
+        .doTest();
+  }
+
+  @Test
+  public void knownCheckedException() {
+    compilationHelper
+        .addSourceLines(
+            "Foo.java",
+            """
+            import static com.google.common.base.Throwables.throwIfUnchecked;
+
+            import java.io.IOException;
+
+            class Foo {
+              void x(IOException e) {
+                throwIfUnchecked(e);
+              }
+            }
+            """)
+        .doTest();
+  }
+
+  @Test
+  public void unknownType() {
+    compilationHelper
+        .addSourceLines(
+            "Foo.java",
+            """
+            import static com.google.common.base.Throwables.throwIfUnchecked;
+
+            class Foo {
+              void x(Exception e) {
+                throwIfUnchecked(e);
+              }
+            }
+            """)
+        .doTest();
+  }
+
+  @Test
+  public void refactoring() {
+    refactoringHelper
+        .addInputLines(
+            "in/Foo.java",
+            """
+            import static com.google.common.base.Throwables.throwIfUnchecked;
+
+            class Foo {
+              void x(IllegalArgumentException e) {
+                log(e);
+                throwIfUnchecked(e);
+                throw new RuntimeException(e);
+              }
+
+              void log(Throwable t) {}
+            }
+            """)
+        .addOutputLines(
+            "out/Foo.java",
+            """
+            import static com.google.common.base.Throwables.throwIfUnchecked;
+
+            class Foo {
+              void x(IllegalArgumentException e) {
+                log(e);
+                throw e;
+              }
+
+              void log(Throwable t) {}
+            }
+            """)
+        .doTest();
+  }
+}