IDEA-190984 Warn if getClass() is called on Class instance

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");
+    }
+  }
+}

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"/>
+    <localInspection groupPath="Java" language="JAVA" shortName="ClassGetClass"
+                     groupBundle="messages.InspectionsBundle" groupKey="group.names.probable.bugs"
+                     enabledByDefault="true" level="WARNING"
+                     key="inspection.class.getclass.display.name" bundle="messages.InspectionsBundle"
+                     implementationClass="com.intellij.codeInspection.ClassGetClassInspection"/>
     <localInspection groupPath="Java,Java language level migration aids" language="JAVA" shortName="FoldExpressionIntoStream"
                      groupBundle="messages.InspectionsBundle" groupKey="group.names.language.level.specific.issues.and.migration.aids8"
                      enabledByDefault="true" level="INFORMATION"

java/java-impl/src/inspectionDescriptions/ClassGetClass.html

@@ -0,0 +1,10 @@
+<html>
+<body>
+<p>Reports when <b>getClass()</b> method is called on <b>java.lang.Class</b> instance. This is usually a mistake as the result is
+  always equivalent to <b>Class.class</b>. If it's mistake then the <b>getClass()</b> call should be removed and qualifier should be used
+  directly. If the behavior is intended, then it's better to write <b>Class.class</b> explicitly to avoid confusion.
+</p>
+<!-- tooltip end -->
+<p><small>New in 2018.2</small></p>
+</body>
+</html>

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());
+  }
+}

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());
+  }
+}

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.getCl<caret>ass().getName());
+  }
+}

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.getCla<caret>ss();
+    System.out.println(clazz.getName());
+  }
+}

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.getCl<caret>ass().getName());
+  }
+}

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";
+  }
+}

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'