diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ComparableImplementedButEqualsNotOverriddenInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ComparableImplementedButEqualsNotOverriddenInspection.java
index 1686f5b5bb6a..863ce5652cac 100644
--- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ComparableImplementedButEqualsNotOverriddenInspection.java
+++ b/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();
}
}
}
diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/comparable_implemented_but_equals_not_overridden/AbstractClass1.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/comparable_implemented_but_equals_not_overridden/AbstractClass1.java
new file mode 100644
index 000000000000..95bdc82ff4c6
--- /dev/null
+++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/comparable_implemented_but_equals_not_overridden/AbstractClass1.java
@@ -0,0 +1,8 @@
+abstract class AbstractClass1 implements Comparable {
+
+ int field = 1;
+
+ public int compareTo(AbstractClass1 other) {
+ return field > other.field ? 1 : (field == other.field ? 0 : -1);
+ }
+}
\ No newline at end of file
diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/comparable_implemented_but_equals_not_overridden/AbstractClass2.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/comparable_implemented_but_equals_not_overridden/AbstractClass2.java
new file mode 100644
index 000000000000..7b99a07e178c
--- /dev/null
+++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/comparable_implemented_but_equals_not_overridden/AbstractClass2.java
@@ -0,0 +1,4 @@
+abstract class AbstractClass2 implements Comparable {
+
+ public abstract int compareTo(AbstractClass2 other);
+}
\ No newline at end of file
diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/comparable_implemented_but_equals_not_overridden/AbstractClass3.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/comparable_implemented_but_equals_not_overridden/AbstractClass3.java
new file mode 100644
index 000000000000..9c9f5f9bfe83
--- /dev/null
+++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/comparable_implemented_but_equals_not_overridden/AbstractClass3.java
@@ -0,0 +1,10 @@
+abstract class AbstractClass3 implements Comparable {
+
+ int field;
+
+ public int compareTo(AbstractClass3 other) {
+ return field > other.field ? 1 : (field == other.field ? 0 : -1);
+ }
+
+ public abstract boolean equals(Object other);
+}
\ No newline at end of file
diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/comparable_implemented_but_equals_not_overridden/Note.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/comparable_implemented_but_equals_not_overridden/Note.java
new file mode 100644
index 000000000000..ea8235218929
--- /dev/null
+++ b/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 {
+
+ @Override
+ public int compareTo(Note other) {
+ return 0;
+ }
+}
\ No newline at end of file
diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/ComparableImplementedButEqualsNotOverriddenInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/ComparableImplementedButEqualsNotOverriddenInspectionTest.java
index 11f392fbfd32..c1ab7d827162 100644
--- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/ComparableImplementedButEqualsNotOverriddenInspectionTest.java
+++ b/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