From 2b105754603997b69bf8c4274c783011b6a00dd2 Mon Sep 17 00:00:00 2001 From: Error Prone Team Date: Wed, 17 Mar 2021 10:12:05 -0700 Subject: [PATCH] Rethrow ReflectiveOperationException as LinkageError instead of AssertionError. PiperOrigin-RevId: 363446325 --- ...ctiveOperationExceptionAsLinkageError.java | 81 +++++++++++++++++++ .../scanner/BuiltInCheckerSuppliers.java | 2 + ...eOperationExceptionAsLinkageErrorTest.java | 44 ++++++++++ ...nExceptionAsLinkageErrorNegativeCases.java | 69 ++++++++++++++++ ...nExceptionAsLinkageErrorPositiveCases.java | 64 +++++++++++++++ 5 files changed, 260 insertions(+) create mode 100644 core/src/main/java/com/google/errorprone/bugpatterns/RethrowReflectiveOperationExceptionAsLinkageError.java create mode 100644 core/src/test/java/com/google/errorprone/bugpatterns/RethrowReflectiveOperationExceptionAsLinkageErrorTest.java create mode 100644 core/src/test/java/com/google/errorprone/bugpatterns/testdata/RethrowReflectiveOperationExceptionAsLinkageErrorNegativeCases.java create mode 100644 core/src/test/java/com/google/errorprone/bugpatterns/testdata/RethrowReflectiveOperationExceptionAsLinkageErrorPositiveCases.java diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/RethrowReflectiveOperationExceptionAsLinkageError.java b/core/src/main/java/com/google/errorprone/bugpatterns/RethrowReflectiveOperationExceptionAsLinkageError.java new file mode 100644 index 00000000000..daedebf40ba --- /dev/null +++ b/core/src/main/java/com/google/errorprone/bugpatterns/RethrowReflectiveOperationExceptionAsLinkageError.java @@ -0,0 +1,81 @@ +/* + * Copyright 2021 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.constructor; + +import com.google.common.collect.Iterables; +import com.google.errorprone.BugPattern; +import com.google.errorprone.VisitorState; +import com.google.errorprone.bugpatterns.BugChecker.ThrowTreeMatcher; +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.ExpressionTree; +import com.sun.source.tree.NewClassTree; +import com.sun.source.tree.ThrowTree; +import com.sun.tools.javac.code.Symbol; +import java.util.List; +import javax.lang.model.element.ElementKind; + +/** A {@link BugChecker}; see the associated {@link BugPattern} annotation for details. */ +@BugPattern( + name = "RethrowReflectiveOperationExceptionAsLinkageError", + summary = "Prefer LinkageError for rethrowing ReflectiveOperationException as unchecked", + severity = WARNING) +public class RethrowReflectiveOperationExceptionAsLinkageError extends BugChecker + implements ThrowTreeMatcher { + private static final String ASSERTION_ERROR = "java.lang.AssertionError"; + private static final String REFLECTIVE_OPERATION_EXCEPTION = + "java.lang.ReflectiveOperationException"; + private static final Matcher MATCHER = constructor().forClass(ASSERTION_ERROR); + + @Override + public Description matchThrow(ThrowTree throwTree, VisitorState state) { + if (!MATCHER.matches(throwTree.getExpression(), state)) { + return NO_MATCH; + } + NewClassTree newClassTree = (NewClassTree) throwTree.getExpression(); + List arguments = newClassTree.getArguments(); + if (arguments.isEmpty() || arguments.size() > 2) { + return NO_MATCH; + } + Symbol cause = ASTHelpers.getSymbol(Iterables.getLast(arguments)); + if (cause == null || !isReflectiveOperationException(state, cause)) { + return NO_MATCH; + } + String message = + arguments.size() == 1 + ? String.format("%s.getMessage()", cause.getSimpleName()) + : state.getSourceForNode(arguments.get(0)); + return describeMatch( + newClassTree, + SuggestedFix.replace( + newClassTree, + String.format("new LinkageError(%s, %s)", message, cause.getSimpleName()))); + } + + private static boolean isReflectiveOperationException(VisitorState state, Symbol symbol) { + return state + .getTypes() + .isSameType(symbol.asType(), state.getTypeFromString(REFLECTIVE_OPERATION_EXCEPTION)) + && symbol.getKind().equals(ElementKind.EXCEPTION_PARAMETER); + } +} diff --git a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java index 4d93d9c5669..c6ddfa99216 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -282,6 +282,7 @@ import com.google.errorprone.bugpatterns.RemovedInJDK11; import com.google.errorprone.bugpatterns.RequiredModifiersChecker; import com.google.errorprone.bugpatterns.RestrictedApiChecker; +import com.google.errorprone.bugpatterns.RethrowReflectiveOperationExceptionAsLinkageError; import com.google.errorprone.bugpatterns.ReturnValueIgnored; import com.google.errorprone.bugpatterns.ReturnsNullCollection; import com.google.errorprone.bugpatterns.RxReturnValueIgnored; @@ -877,6 +878,7 @@ public static ScannerSupplier errorChecks() { QualifierOrScopeOnInjectMethod.class, ReachabilityFenceUsage.class, ReferenceEquality.class, + RethrowReflectiveOperationExceptionAsLinkageError.class, ReturnFromVoid.class, RxReturnValueIgnored.class, SameNameButDifferent.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/RethrowReflectiveOperationExceptionAsLinkageErrorTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/RethrowReflectiveOperationExceptionAsLinkageErrorTest.java new file mode 100644 index 00000000000..bf1bad8fcff --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/RethrowReflectiveOperationExceptionAsLinkageErrorTest.java @@ -0,0 +1,44 @@ +/* + * Copyright 2021 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.CompilationTestHelper; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** {@link RethrowReflectiveOperationExceptionAsLinkageError}Test */ +@RunWith(JUnit4.class) +public class RethrowReflectiveOperationExceptionAsLinkageErrorTest { + private final CompilationTestHelper testHelper = + CompilationTestHelper.newInstance( + RethrowReflectiveOperationExceptionAsLinkageError.class, getClass()); + + @Test + public void positive() { + testHelper + .addSourceFile("RethrowReflectiveOperationExceptionAsLinkageErrorPositiveCases.java") + .doTest(); + } + + @Test + public void negative() { + testHelper + .addSourceFile("RethrowReflectiveOperationExceptionAsLinkageErrorNegativeCases.java") + .doTest(); + } +} diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/RethrowReflectiveOperationExceptionAsLinkageErrorNegativeCases.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/RethrowReflectiveOperationExceptionAsLinkageErrorNegativeCases.java new file mode 100644 index 00000000000..6573602ee12 --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/RethrowReflectiveOperationExceptionAsLinkageErrorNegativeCases.java @@ -0,0 +1,69 @@ +/* + * Copyright 2021 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.testdata; + +import java.io.IOException; + +public class RethrowReflectiveOperationExceptionAsLinkageErrorNegativeCases { + void assertionErrorNonStringConstructor() { + try { + throw new ReflectiveOperationException(); + } catch (ReflectiveOperationException e) { + throw new AssertionError(1); + } + } + + void assertionErrorNoArgConstructor() { + try { + throw new ReflectiveOperationException(); + } catch (ReflectiveOperationException e) { + throw new AssertionError(); + } + } + + void noThrowAssertionError() { + try { + throw new ReflectiveOperationException(); + } catch (ReflectiveOperationException e) { + throw new IllegalStateException(e); + } + } + + void noCatchReflectiveOperationException() { + try { + throw new ReflectiveOperationException(); + } catch (Exception e) { + throw new AssertionError(e); + } + } + + void multiCatchExceptions() { + try { + int a = 100; + if (a < 100) { + throw new IOException("Test"); + } + throw new ReflectiveOperationException(); + } catch (IOException | ReflectiveOperationException e) { + throw new AssertionError(e); + } + } + + void throwNewReflectiveOperationException() { + throw new AssertionError(new ReflectiveOperationException("Test")); + } +} diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/testdata/RethrowReflectiveOperationExceptionAsLinkageErrorPositiveCases.java b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/RethrowReflectiveOperationExceptionAsLinkageErrorPositiveCases.java new file mode 100644 index 00000000000..3727561186a --- /dev/null +++ b/core/src/test/java/com/google/errorprone/bugpatterns/testdata/RethrowReflectiveOperationExceptionAsLinkageErrorPositiveCases.java @@ -0,0 +1,64 @@ +/* + * Copyright 2021 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.testdata; + +public class RethrowReflectiveOperationExceptionAsLinkageErrorPositiveCases { + void assertionErrorExceptionConstructor() { + try { + throw new ReflectiveOperationException(); + } catch (ReflectiveOperationException e) { + // BUG: Diagnostic contains: throw new LinkageError(e.getMessage(), e); + throw new AssertionError(e); + } + } + + void assertionErrorStringConstructor() { + try { + throw new ReflectiveOperationException(); + } catch (ReflectiveOperationException e) { + // BUG: Diagnostic contains: throw new LinkageError("Test", e); + throw new AssertionError("Test", e); + } + } + + void assertionErrorStringFormatConstructor() { + try { + throw new ReflectiveOperationException(); + } catch (ReflectiveOperationException e) { + // BUG: Diagnostic contains: throw new LinkageError(String.format("Test"), e); + throw new AssertionError(String.format("Test"), e); + } + } + + void multiLineCatchBlock() { + try { + throw new ReflectiveOperationException(); + } catch (ReflectiveOperationException e1) { + int a = 100; + if (a < 100) { + try { + throw new ReflectiveOperationException(); + } catch (ReflectiveOperationException e2) { + // BUG: Diagnostic contains: throw new LinkageError(e2.getMessage(), e2); + throw new AssertionError(e2); + } + } + // BUG: Diagnostic contains: throw new LinkageError(e1.getMessage(), e1); + throw new AssertionError(e1); + } + } +}