IG: add quick fixes and handle abstract classes better

in "Comparable implemented but 'equals()' not overridden" inspection

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ComparableImplementedButEqualsNotOverriddenInspection.java

@@ -15,34 +15,119 @@
  */
 package com.siyeh.ig.bugs;
 
+import com.intellij.codeInspection.ProblemDescriptor;
 import com.intellij.openapi.project.Project;
-import com.intellij.psi.CommonClassNames;
-import com.intellij.psi.JavaPsiFacade;
-import com.intellij.psi.PsiClass;
-import com.intellij.psi.PsiMethod;
-import com.intellij.psi.search.GlobalSearchScope;
+import com.intellij.openapi.util.text.StringUtil;
+import com.intellij.psi.*;
+import com.intellij.psi.codeStyle.CodeStyleManager;
+import com.intellij.psi.javadoc.PsiDocComment;
+import com.intellij.psi.javadoc.PsiDocToken;
 import com.intellij.psi.util.MethodSignatureUtil;
+import com.intellij.psi.util.PsiUtil;
 import com.siyeh.HardcodedMethodConstants;
 import com.siyeh.InspectionGadgetsBundle;
 import com.siyeh.ig.BaseInspection;
 import com.siyeh.ig.BaseInspectionVisitor;
-import com.siyeh.ig.psiutils.MethodUtils;
+import com.siyeh.ig.InspectionGadgetsFix;
+import com.siyeh.ig.psiutils.ClassUtils;
+import org.jetbrains.annotations.Nls;
 import org.jetbrains.annotations.NotNull;
 
+import java.util.Arrays;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+
 public class ComparableImplementedButEqualsNotOverriddenInspection extends BaseInspection {
 
   @Override
   @NotNull
   public String getDisplayName() {
-    return InspectionGadgetsBundle.message(
-      "comparable.implemented.but.equals.not.overridden.display.name");
+    return InspectionGadgetsBundle.message("comparable.implemented.but.equals.not.overridden.display.name");
   }
 
   @Override
   @NotNull
   protected String buildErrorString(Object... infos) {
-    return InspectionGadgetsBundle.message(
-      "comparable.implemented.but.equals.not.overridden.problem.descriptor");
+    return InspectionGadgetsBundle.message("comparable.implemented.but.equals.not.overridden.problem.descriptor");
+  }
+
+  @NotNull
+  @Override
+  protected InspectionGadgetsFix[] buildFixes(Object... infos) {
+    return new InspectionGadgetsFix[] {
+      new GenerateEqualsMethodFix(),
+      new AddNoteFix()
+    };
+  }
+
+  private static class GenerateEqualsMethodFix extends InspectionGadgetsFix {
+    @Nls
+    @NotNull
+    @Override
+    public String getFamilyName() {
+      return "Generate 'equals()' method";
+    }
+
+    @Override
+    protected void doFix(Project project, ProblemDescriptor descriptor) {
+      final PsiClass aClass = (PsiClass)descriptor.getPsiElement().getParent();
+      final StringBuilder methodText = new StringBuilder();
+      if (PsiUtil.isLanguageLevel5OrHigher(aClass)) {
+        methodText.append("@java.lang.Override ");
+      }
+      methodText.append("public ");
+      methodText.append("boolean equals(Object o) {\n");
+      methodText.append("if (!(o instanceof ").append(aClass.getName()).append("))").append("return false;");
+      methodText.append("return compareTo((").append(aClass.getName()).append(")o)==0;\n");
+      methodText.append("}");
+      final PsiMethod method =
+        JavaPsiFacade.getElementFactory(project).createMethodFromText(methodText.toString(), aClass, PsiUtil.getLanguageLevel(aClass));
+      final PsiElement newMethod = aClass.add(method);
+      CodeStyleManager.getInstance(project).reformat(newMethod);
+    }
+  }
+
+  private static class AddNoteFix extends InspectionGadgetsFix {
+
+    private static final Pattern PARAM_PATTERN = Pattern.compile("\\*[ \t]+@");
+
+    @Nls
+    @NotNull
+    @Override
+    public String getFamilyName() {
+      return "Add 'ordering inconsistent with equals' JavaDoc note";
+    }
+
+    @Override
+    protected void doFix(Project project, ProblemDescriptor descriptor) {
+      final PsiClass aClass = (PsiClass)descriptor.getPsiElement().getParent();
+      final PsiDocComment comment = aClass.getDocComment();
+      final PsiElementFactory factory = JavaPsiFacade.getElementFactory(project);
+      if (comment == null) {
+        final PsiDocComment newComment = factory.createDocCommentFromText(
+          "/**\n" +
+          "* Note: this class has a natural ordering that is inconsistent with equals.\n" +
+          "*/", aClass);
+        aClass.addBefore(newComment, aClass.getFirstChild());
+      }
+      else {
+        final String text = comment.getText();
+        final Matcher matcher = PARAM_PATTERN.matcher(text);
+        String newCommentText;
+        if (matcher.find()) {
+          newCommentText = text.substring(0, matcher.start()) +
+                           " * Note: this class has a natural ordering that is inconsistent with equals.\n" +
+                           text.substring(matcher.start());
+        }
+        else {
+          newCommentText = text.substring(0, text.length() - 2) +
+                           " * Note: this class has a natural ordering that is inconsistent with equals.\n*/";
+        }
+        final PsiDocComment newComment = factory.createDocCommentFromText(newCommentText);
+        comment.replace(newComment);
+      }
+    }
   }
 
   @Override
@@ -56,43 +141,68 @@ public class ComparableImplementedButEqualsNotOverriddenInspection extends BaseI
     public void visitClass(PsiClass aClass) {
       super.visitClass(aClass);
       if (aClass.isInterface()) {
+        // the problem can't be fixed for an interface, so let's not report it
         return;
       }
-      final PsiMethod[] methods = aClass.findMethodsByName(HardcodedMethodConstants.COMPARE_TO, false);
-      if (methods.length == 0) {
-        return;
-      }
-      final Project project = aClass.getProject();
-      final JavaPsiFacade psiFacade = JavaPsiFacade.getInstance(project);
-      final GlobalSearchScope scope = aClass.getResolveScope();
       final PsiClass comparableClass =
-        psiFacade.findClass(CommonClassNames.JAVA_LANG_COMPARABLE,
-                            scope);
-      if (comparableClass == null) {
+        JavaPsiFacade.getInstance(aClass.getProject()).findClass(CommonClassNames.JAVA_LANG_COMPARABLE, aClass.getResolveScope());
+      if (comparableClass == null || !aClass.isInheritor(comparableClass, true)) {
         return;
       }
-      if (!aClass.isInheritor(comparableClass, true)) {
+      final PsiMethod[] comparableMethods = comparableClass.getMethods();
+      if (comparableMethods.length != 1) { // incorrect/broken jdk
         return;
       }
-      final PsiMethod compareToMethod = comparableClass.getMethods()[0];
-      boolean foundCompareTo = false;
-      for (PsiMethod method : methods) {
-        if (MethodSignatureUtil.isSuperMethod(compareToMethod, method)) {
-          foundCompareTo = true;
-          break;
+      final PsiMethod comparableMethod = MethodSignatureUtil.findMethodBySuperMethod(aClass, comparableMethods[0], false);
+      if (comparableMethod == null || comparableMethod.hasModifierProperty(PsiModifier.ABSTRACT) ||
+        comparableMethod.getBody() == null) {
+        return;
+      }
+      final PsiClass objectClass = ClassUtils.findObjectClass(aClass);
+      if (objectClass == null) {
+        return;
+      }
+      final PsiMethod[] equalsMethods = objectClass.findMethodsByName(HardcodedMethodConstants.EQUALS, false);
+      if (equalsMethods.length != 1) { // incorrect/broken jdk
+        return;
+      }
+      final PsiMethod equalsMethod = MethodSignatureUtil.findMethodBySuperMethod(aClass, equalsMethods[0], false);
+      if (equalsMethod != null && !equalsMethod.hasModifierProperty(PsiModifier.ABSTRACT)) {
+        return;
+      }
+      final String docCommentText = collapseWhitespace(getActualCommentText(aClass.getDocComment()));
+      if (StringUtil.containsIgnoreCase(docCommentText, "this class has a natural ordering that is inconsistent with equals")) {
+        // see Comparable.compareTo() javadoc
+        return;
+      }
+      registerClassError(aClass, aClass);
+    }
+
+    private static String getActualCommentText(PsiDocComment comment) {
+      if (comment == null) return "";
+      return Arrays.stream(comment.getChildren())
+        .filter(e -> (e instanceof PsiDocToken) && ((PsiDocToken)e).getTokenType() == JavaDocTokenType.DOC_COMMENT_DATA)
+        .map(PsiElement::getText)
+        .collect(Collectors.joining());
+    }
+
+    private static String collapseWhitespace(String s) {
+      final StringBuilder result = new StringBuilder();
+      boolean space = false;
+      for (int i = 0, length = s.length(); i < length; i++) {
+        char ch = s.charAt(i);
+        if (StringUtil.isWhiteSpace(ch)) {
+          if (!space) {
+            result.append(' ');
+            space = true;
+          }
+        }
+        else {
+          result.append(ch);
+          space = false;
         }
       }
-      if (!foundCompareTo) {
-        return;
-      }
-      final PsiMethod[] equalsMethods = aClass.findMethodsByName(
-        HardcodedMethodConstants.EQUALS, false);
-      for (PsiMethod equalsMethod : equalsMethods) {
-        if (MethodUtils.isEquals(equalsMethod)) {
-          return;
-        }
-      }
-      registerClassError(aClass);
+      return result.toString();
     }
   }
 }

plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/comparable_implemented_but_equals_not_overridden/AbstractClass1.java

@@ -0,0 +1,8 @@
+abstract class <warning descr="Class 'AbstractClass1' implements 'java.lang.Comparable' but does not override 'equals()'">AbstractClass1</warning> implements Comparable<AbstractClass1> {
+
+  int field = 1;
+
+  public int compareTo(AbstractClass1 other) {
+    return field > other.field ? 1 : (field == other.field ? 0 : -1);
+  }
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/comparable_implemented_but_equals_not_overridden/AbstractClass2.java

@@ -0,0 +1,4 @@
+abstract class AbstractClass2 implements Comparable<AbstractClass2> {
+
+  public abstract int compareTo(AbstractClass2 other);
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/comparable_implemented_but_equals_not_overridden/AbstractClass3.java

@@ -0,0 +1,10 @@
+abstract class <warning descr="Class 'AbstractClass3' implements 'java.lang.Comparable' but does not override 'equals()'">AbstractClass3</warning> implements Comparable<AbstractClass3> {
+
+  int field;
+
+  public int compareTo(AbstractClass3 other) {
+    return field > other.field ? 1 : (field == other.field ? 0 : -1);
+  }
+
+  public abstract boolean equals(Object other);
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/comparable_implemented_but_equals_not_overridden/Note.java

@@ -0,0 +1,11 @@
+/**
+ * Note: This     class has a natural
+ * ordering that is INCONSISTENT with equals.
+ */
+class Note implements Comparable<Note> {
+
+  @Override
+  public int compareTo(Note other) {
+    return 0;
+  }
+}

plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/ComparableImplementedButEqualsNotOverriddenInspectionTest.java

@@ -26,6 +26,10 @@ public class ComparableImplementedButEqualsNotOverriddenInspectionTest extends L
 
   public void testInterfaceImplementingComparable() { doTest(); }
   public void testSimple() { doTest(); }
+  public void testAbstractClass1() { doTest(); }
+  public void testAbstractClass2() { doTest(); }
+  public void testAbstractClass3() { doTest(); }
+  public void testNote() { doTest(); }
 
   @Nullable
   @Override