diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/DoNotCallChecker.java b/core/src/main/java/com/google/errorprone/bugpatterns/DoNotCallChecker.java index c5d942fc354..e8d9d7bfb67 100644 --- a/core/src/main/java/com/google/errorprone/bugpatterns/DoNotCallChecker.java +++ b/core/src/main/java/com/google/errorprone/bugpatterns/DoNotCallChecker.java @@ -130,6 +130,11 @@ public class DoNotCallChecker extends BugChecker .onExactClass("java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock") .named("newCondition"), "ReadLocks do not support conditions.") + .put( + instanceMethod().onExactClass("java.lang.StackTraceElement").named("getClass"), + "Calling getClass on StackTraceElement returns the Class object for" + + " StackTraceElement, you probably meant to retrieve the class containing the" + + " execution point represented by this stack trace element using getClassName") .buildOrThrow(); static final String DO_NOT_CALL = "com.google.errorprone.annotations.DoNotCall"; diff --git a/core/src/main/java/com/google/errorprone/bugpatterns/StackTraceElementGetClass.java b/core/src/main/java/com/google/errorprone/bugpatterns/StackTraceElementGetClass.java deleted file mode 100644 index 3691dfe44d4..00000000000 --- a/core/src/main/java/com/google/errorprone/bugpatterns/StackTraceElementGetClass.java +++ /dev/null @@ -1,85 +0,0 @@ -/* - * 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.matchers.Matchers.instanceMethod; - -import com.google.errorprone.BugPattern; -import com.google.errorprone.BugPattern.SeverityLevel; -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.ExpressionStatementTree; -import com.sun.source.tree.ExpressionTree; -import com.sun.source.tree.MethodInvocationTree; -import com.sun.source.tree.Tree; - -/** - * Checks for usages of {@code StackTraceElement#getClass} method. - * - *

{@code StackTraceElement#getClass} returns the Class object for {@code StackTraceElement}. In - * almost all the scenarios this is not intended and is a potential source of bugs. The most common - * usage of this method is to retrieve the name of the class where exception occurred, in such cases - * {@code StackTraceElement#getClassName} can be used instead. In case Class object for {@code - * StackTraceElement} is required it can be obtained using {code StackTraceElement#class} method. - */ -@BugPattern( - name = "StackTraceElementGetClass", - summary = - "Calling getClass on StackTraceElement returns the Class object for StackTraceElement, you" - + " probably meant to retrieve the class containing the execution point represented by" - + " this stack trace element.", - severity = SeverityLevel.ERROR) -public class StackTraceElementGetClass extends BugChecker implements MethodInvocationTreeMatcher { - - private static final Matcher GET_CLASS_MATCHER = - instanceMethod().onExactClass("java.lang.StackTraceElement").named("getClass"); - private static final Matcher GET_NAME_MATCHER = - instanceMethod().onExactClass("java.lang.Class").named("getName"); - private static final Matcher GET_SIMPLE_NAME_MATCHER = - instanceMethod().onExactClass("java.lang.Class").named("getSimpleName"); - - @Override - public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState state) { - if (!GET_CLASS_MATCHER.matches(tree, state)) { - return Description.NO_MATCH; - } - Description.Builder descriptionBuilder = buildDescription(tree); - Tree parentTree = state.getPath().getParentPath().getLeaf(); - if (parentTree instanceof ExpressionTree - && (GET_NAME_MATCHER.matches((ExpressionTree) parentTree, state) - || GET_SIMPLE_NAME_MATCHER.matches((ExpressionTree) parentTree, state))) { - SuggestedFix.Builder fixBuilder = - SuggestedFix.builder() - .replace( - parentTree, - state.getSourceForNode(ASTHelpers.getReceiver(tree)) + ".getClassName"); - if (GET_SIMPLE_NAME_MATCHER.matches((ExpressionTree) parentTree, state)) { - fixBuilder.setShortDescription( - "Replace with getClassName. WARNING: This returns the fully-qualified name of class."); - } - descriptionBuilder.addFix(fixBuilder.build()); - } - if (!(parentTree instanceof ExpressionStatementTree)) { - descriptionBuilder.addFix(SuggestedFix.replace(tree, "StackTraceElement.class")); - } - return descriptionBuilder.build(); - } -} 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 d58bed58b1d..aeca710d72e 100644 --- a/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java +++ b/core/src/main/java/com/google/errorprone/scanner/BuiltInCheckerSuppliers.java @@ -307,7 +307,6 @@ import com.google.errorprone.bugpatterns.ShortCircuitBoolean; import com.google.errorprone.bugpatterns.ShouldHaveEvenArgs; import com.google.errorprone.bugpatterns.SizeGreaterThanOrEqualsZero; -import com.google.errorprone.bugpatterns.StackTraceElementGetClass; import com.google.errorprone.bugpatterns.StaticAssignmentInConstructor; import com.google.errorprone.bugpatterns.StaticMockMember; import com.google.errorprone.bugpatterns.StaticQualifiedUsingExpression; @@ -726,7 +725,6 @@ public static ScannerSupplier errorChecks() { SelfEquals.class, ShouldHaveEvenArgs.class, SizeGreaterThanOrEqualsZero.class, - StackTraceElementGetClass.class, StreamToString.class, StringBuilderInitWithChar.class, SubstringOfZero.class, diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/DoNotCallCheckerTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/DoNotCallCheckerTest.java index 472b55f7298..559162cab8b 100644 --- a/core/src/test/java/com/google/errorprone/bugpatterns/DoNotCallCheckerTest.java +++ b/core/src/test/java/com/google/errorprone/bugpatterns/DoNotCallCheckerTest.java @@ -552,4 +552,22 @@ public void typeArgs_dontCrash() { "}") .doTest(); } + + @Test + public void positive_getSimpleName_refactoredToGetClassName() { + testHelper + .addSourceLines( + "Test.java", + "class Test{", + " void f() {", + " try {", + " throw new Exception();", + " } catch (Exception ex) {", + " // BUG: Diagnostic contains: getClassName", + " ex.getStackTrace()[0].getClass().getSimpleName();", + " }", + " }", + "}") + .doTest(); + } } diff --git a/core/src/test/java/com/google/errorprone/bugpatterns/StackTraceElementGetClassTest.java b/core/src/test/java/com/google/errorprone/bugpatterns/StackTraceElementGetClassTest.java deleted file mode 100644 index 1859e32e890..00000000000 --- a/core/src/test/java/com/google/errorprone/bugpatterns/StackTraceElementGetClassTest.java +++ /dev/null @@ -1,141 +0,0 @@ -/* - * 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.BugCheckerRefactoringTestHelper; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Tests for {@link StackTraceElementGetClass}. */ -@RunWith(JUnit4.class) -public class StackTraceElementGetClassTest { - private final BugCheckerRefactoringTestHelper helper = - BugCheckerRefactoringTestHelper.newInstance(StackTraceElementGetClass.class, getClass()); - - @Test - public void positive_getName_refactoredToGetClassName() { - helper - .addInputLines( - "Test.java", - "class Test{", - " void f() {", - " try {", - " throw new Exception();", - " }", - " catch(Exception ex) {", - " ex.getStackTrace()[0].getClass().getName();", - " }", - " }", - "}") - .addOutputLines( - "Test.java", - "class Test{", - " void f() {", - " try {", - " throw new Exception();", - " }", - " catch(Exception ex) {", - " ex.getStackTrace()[0].getClassName();", - " }", - " }", - "}") - .doTest(); - } - - @Test - public void positive_getSimpleName_refactoredToGetClassName() { - helper - .addInputLines( - "Test.java", - "class Test{", - " void f() {", - " try {", - " throw new Exception();", - " }", - " catch(Exception ex) {", - " ex.getStackTrace()[0].getClass().getSimpleName();", - " }", - " }", - "}") - .addOutputLines( - "Test.java", - "class Test{", - " void f() {", - " try {", - " throw new Exception();", - " }", - " catch(Exception ex) {", - " ex.getStackTrace()[0].getClassName();", - " }", - " }", - "}") - .doTest(); - } - - @Test - public void positive_getClass_refactoredToClass() { - helper - .addInputLines( - "Test.java", - "import java.lang.reflect.Field;", - "class Test{", - " void f() throws Exception {", - " try {", - " throw new Exception();", - " }", - " catch(Exception ex) {", - " Field f = " - + " ex.getStackTrace()[0].getClass().getDeclaredField(\"declaringClass\");", - " }", - " }", - "}") - .addOutputLines( - "Test.java", - "import java.lang.reflect.Field;", - "class Test{", - " void f() throws Exception {", - " try {", - " throw new Exception();", - " }", - " catch(Exception ex) {", - " Field f = StackTraceElement.class.getDeclaredField(\"declaringClass\");", - " }", - " }", - "}") - .doTest(); - } - - @Test - public void negative_unchanged() { - helper - .addInputLines( - "Test.java", - "class Test{", - " void f() {", - " try {", - " throw new Exception();", - " }", - " catch(Exception ex) {", - " ex.getStackTrace()[0].getClassName();", - " }", - " }", - "}") - .expectUnchanged() - .doTest(); - } -} diff --git a/docs/bugpattern/StackTraceElementGetClass.md b/docs/bugpattern/StackTraceElementGetClass.md deleted file mode 100644 index a34725d08b3..00000000000 --- a/docs/bugpattern/StackTraceElementGetClass.md +++ /dev/null @@ -1,7 +0,0 @@ -`StackTraceElementGetClass#getClass` returns the Class object for -`StackTraceElement`. In almost all the scenarios this is not intended and is a -potential source of bugs. The most common usage of this method is to retrieve -the name of the class where exception occurred, in such cases -`StackTraceElement#getClassName` can be used instead. In case Class object for -`StackTraceElement` is required it can be obtained using -`StackTraceElement#class` method.