diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/ClassGetClassInspection.java b/java/java-analysis-impl/src/com/intellij/codeInspection/ClassGetClassInspection.java new file mode 100644 index 000000000000..af776386de16 --- /dev/null +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/ClassGetClassInspection.java @@ -0,0 +1,78 @@ +// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.intellij.codeInspection; + +import com.intellij.openapi.project.Project; +import com.intellij.psi.*; +import com.intellij.psi.util.PsiTreeUtil; +import com.siyeh.ig.callMatcher.CallMatcher; +import com.siyeh.ig.psiutils.CommentTracker; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; + +import java.util.Objects; + +/** + * @author Tagir Valeev + */ +public class ClassGetClassInspection extends AbstractBaseJavaLocalInspectionTool { + private static final CallMatcher OBJECT_GET_CLASS = + CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_OBJECT, "getClass").parameterCount(0); + + @NotNull + @Override + public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { + return new JavaElementVisitor() { + @Override + public void visitMethodCallExpression(PsiMethodCallExpression call) { + if (!OBJECT_GET_CLASS.test(call)) return; + // Sometimes people use xyz.getClass() for implicit NPE check. While it's a questionable code style + // do not warn about such case + if (call.getParent() instanceof PsiExpressionStatement) return; + PsiExpression qualifier = call.getMethodExpression().getQualifierExpression(); + if (qualifier == null) return; + PsiType type = qualifier.getType(); + if (!(type instanceof PsiClassType)) return; + if (!((PsiClassType)type).rawType().equalsToText(CommonClassNames.JAVA_LANG_CLASS)) return; + holder.registerProblem(Objects.requireNonNull(call.getMethodExpression().getReferenceNameElement()), + InspectionsBundle.message("inspection.class.getclass.message"), + new RemoveGetClassCallFix(), new ReplaceWithClassClassFix()); + } + }; + } + + private static class RemoveGetClassCallFix implements LocalQuickFix { + @Nls(capitalization = Nls.Capitalization.Sentence) + @NotNull + @Override + public String getFamilyName() { + return InspectionsBundle.message("inspection.class.getclass.fix.remove.name"); + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiMethodCallExpression call = PsiTreeUtil.getParentOfType(descriptor.getStartElement(), PsiMethodCallExpression.class); + if (call == null) return; + PsiExpression qualifier = call.getMethodExpression().getQualifierExpression(); + if (qualifier == null) return; + CommentTracker ct = new CommentTracker(); + ct.replaceAndRestoreComments(call, ct.markUnchanged(qualifier)); + } + } + + private static class ReplaceWithClassClassFix implements LocalQuickFix { + @Nls(capitalization = Nls.Capitalization.Sentence) + @NotNull + @Override + public String getFamilyName() { + return InspectionsBundle.message("inspection.class.getclass.fix.replace.name"); + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiMethodCallExpression call = PsiTreeUtil.getParentOfType(descriptor.getStartElement(), PsiMethodCallExpression.class); + if (call == null) return; + CommentTracker ct = new CommentTracker(); + ct.replaceAndRestoreComments(call, "java.lang.Class.class"); + } + } +} diff --git a/java/java-impl/src/META-INF/JavaPlugin.xml b/java/java-impl/src/META-INF/JavaPlugin.xml index 7e2178656c4c..544e3f18c899 100644 --- a/java/java-impl/src/META-INF/JavaPlugin.xml +++ b/java/java-impl/src/META-INF/JavaPlugin.xml @@ -794,6 +794,11 @@ enabledByDefault="true" level="WARNING" key="inspection.suspicious.list.remove.display.name" bundle="messages.InspectionsBundle" implementationClass="com.intellij.codeInspection.SuspiciousListRemoveInLoopInspection"/> + + +

Reports when getClass() method is called on java.lang.Class instance. This is usually a mistake as the result is + always equivalent to Class.class. If it's mistake then the getClass() call should be removed and qualifier should be used + directly. If the behavior is intended, then it's better to write Class.class explicitly to avoid confusion. +

+ +

New in 2018.2

+ + \ No newline at end of file diff --git a/java/java-tests/testData/inspection/classGetClass/afterClassGetClass.java b/java/java-tests/testData/inspection/classGetClass/afterClassGetClass.java new file mode 100644 index 000000000000..eaac02911500 --- /dev/null +++ b/java/java-tests/testData/inspection/classGetClass/afterClassGetClass.java @@ -0,0 +1,6 @@ +// "Remove 'getClass()' call" "true" +class Test { + void test(Class clazz) { + System.out.println(clazz.getName()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/classGetClass/afterClassGetClassReplace.java b/java/java-tests/testData/inspection/classGetClass/afterClassGetClassReplace.java new file mode 100644 index 000000000000..d42d7c1b8794 --- /dev/null +++ b/java/java-tests/testData/inspection/classGetClass/afterClassGetClassReplace.java @@ -0,0 +1,6 @@ +// "Replace with 'Class.class'" "true" +class Test { + void test(Class clazz) { + System.out.println(Class.class.getName()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/classGetClass/beforeClassGetClass.java b/java/java-tests/testData/inspection/classGetClass/beforeClassGetClass.java new file mode 100644 index 000000000000..21534da1705c --- /dev/null +++ b/java/java-tests/testData/inspection/classGetClass/beforeClassGetClass.java @@ -0,0 +1,6 @@ +// "Remove 'getClass()' call" "true" +class Test { + void test(Class clazz) { + System.out.println(clazz.getClass().getName()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/classGetClass/beforeClassGetClassNullCheck.java b/java/java-tests/testData/inspection/classGetClass/beforeClassGetClassNullCheck.java new file mode 100644 index 000000000000..2cd69b6252e6 --- /dev/null +++ b/java/java-tests/testData/inspection/classGetClass/beforeClassGetClassNullCheck.java @@ -0,0 +1,8 @@ +// "Remove 'getClass()' call" "false" +class Test { + void test(Class clazz) { + // implicit null check + clazz.getClass(); + System.out.println(clazz.getName()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/classGetClass/beforeClassGetClassReplace.java b/java/java-tests/testData/inspection/classGetClass/beforeClassGetClassReplace.java new file mode 100644 index 000000000000..e7a786cdf7fa --- /dev/null +++ b/java/java-tests/testData/inspection/classGetClass/beforeClassGetClassReplace.java @@ -0,0 +1,6 @@ +// "Replace with 'Class.class'" "true" +class Test { + void test(Class clazz) { + System.out.println(clazz.getClass().getName()); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/ClassGetClassInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/ClassGetClassInspectionTest.java new file mode 100644 index 000000000000..5f4d6ac66025 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/ClassGetClassInspectionTest.java @@ -0,0 +1,27 @@ +// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.intellij.java.codeInspection; + +import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase; +import com.intellij.codeInspection.ClassGetClassInspection; +import com.intellij.codeInspection.LocalInspectionTool; +import org.jetbrains.annotations.NotNull; + +/** + * @author Tagir Valeev + */ +public class ClassGetClassInspectionTest extends LightQuickFixParameterizedTestCase { + @NotNull + @Override + protected LocalInspectionTool[] configureLocalInspectionTools() { + return new LocalInspectionTool[]{new ClassGetClassInspection()}; + } + + public void test() { + doAllTests(); + } + + @Override + protected String getBasePath() { + return "/inspection/classGetClass"; + } +} diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index aa8e9c8e4630..9c62f3eefe7b 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -973,3 +973,8 @@ inspection.cast.can.be.removed.narrowing.variable.type.fix.name=Change type of ' inspection.wrapper.type.may.be.primitive.name=Type may be primitive inspection.wrapper.type.may.be.primitive.fix.name=Convert wrapper type to primitive + +inspection.class.getclass.display.name=Class.getClass() call +inspection.class.getclass.message='getClass()' is called on Class instance +inspection.class.getclass.fix.remove.name=Remove 'getClass()' call +inspection.class.getclass.fix.replace.name=Replace with 'Class.class'