From e03e71ce9f6708e472f56b11c2d6541f7c8a82e7 Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Fri, 15 Jun 2018 15:40:26 +0300 Subject: [PATCH] Java: Improved highlighting mode in the inspection "Replace with Objects.equals()" (IDEA-193783) --- .../siyeh/InspectionGadgetsBundle.properties | 2 +- ...alsReplaceableByObjectsCallInspection.java | 24 +++---- .../EqualsReplaceableByObjectsCall.html | 12 ++-- .../EqualsReplaceableByObjectsCall.java | 62 +++++++++---------- ...ualsReplaceableByObjectsCallCheckNull.java | 40 ++++++------ ...EqualsReplaceableByObjectsCallFixTest.java | 24 +++---- ...eplaceableByObjectsCallInspectionTest.java | 4 +- 7 files changed, 84 insertions(+), 84 deletions(-) diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties index f5f2b2bd43f3..d4df74581350 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/InspectionGadgetsBundle.properties @@ -2091,7 +2091,7 @@ dangling.javadoc.delete.quickfix=Remove dangling comment equals.replaceable.by.objects.call.display.name='equals()' expression replaceable by 'Objects.equals()' expression equals.replaceable.by.objects.call.problem.descriptor=#ref replaceable by 'Objects.equals()' expression #loc equals.replaceable.by.objects.call.quickfix=Replace with 'Objects.equals()' expression -equals.replaceable.by.objects.check.not.null.option=Report only null safe 'equals' calls +equals.replaceable.by.objects.check.not.null.option=Highlight expressions like 'a != null \\&\\& a.equals(b)' array.objects.equals.display.name='Objects.equals()' called on arrays array.objects.equals.problem.descriptor=Objects.#ref() on arrays should probably be 'Arrays.equals()' #loc array.objects.deep.equals.problem.descriptor=Objects.#ref() on arrays should probably be 'Arrays.deepEquals()' #loc diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/EqualsReplaceableByObjectsCallInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/EqualsReplaceableByObjectsCallInspection.java index 82fb4efc12ab..81f292d1e124 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/EqualsReplaceableByObjectsCallInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/migration/EqualsReplaceableByObjectsCallInspection.java @@ -16,6 +16,7 @@ package com.siyeh.ig.migration; import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.codeInspection.ProblemHighlightType; import com.intellij.codeInspection.ui.SingleCheckboxOptionsPanel; import com.intellij.openapi.project.Project; import com.intellij.psi.*; @@ -42,7 +43,7 @@ import javax.swing.*; * @author Bas Leijdekkers */ public class EqualsReplaceableByObjectsCallInspection extends BaseInspection { - public boolean checkNotNull = true; + public boolean checkNotNull; private static final EquivalenceChecker EQUIVALENCE = new NoSideEffectExpressionEquivalenceChecker(); @@ -141,16 +142,14 @@ public class EqualsReplaceableByObjectsCallInspection extends BaseInspection { return; } } - if (!checkNotNull) { - if (qualifierExpression == null) { - return; - } - final PsiExpression argumentExpression = getArgumentExpression(expression); - if (argumentExpression == null) { - return; - } - registerError(expression, qualifierExpression.getText(), argumentExpression.getText(), true); + if (qualifierExpression == null) { + return; } + final PsiExpression argumentExpression = getArgumentExpression(expression); + if (argumentExpression == null) { + return; + } + registerError(expression, ProblemHighlightType.INFORMATION, qualifierExpression.getText(), argumentExpression.getText(), true); } private boolean processNotNullCheck(PsiBinaryExpression expression) { @@ -219,7 +218,10 @@ public class EqualsReplaceableByObjectsCallInspection extends BaseInspection { final PsiExpression argumentExpression = getArgumentExpression(methodCallExpression); if (argumentExpression != null) { final PsiExpression expressionToReplace = checkEqualityBefore(expression, equal, qualifierExpression, argumentExpression); - registerError(expressionToReplace, nullCheckedExpression.getText(), argumentExpression.getText(), Boolean.valueOf(equal)); + ProblemHighlightType highlightType = checkNotNull || expression != expressionToReplace ? + ProblemHighlightType.GENERIC_ERROR_OR_WARNING : ProblemHighlightType.INFORMATION; + registerError(expressionToReplace, highlightType, + nullCheckedExpression.getText(), argumentExpression.getText(), Boolean.valueOf(equal)); return true; } } diff --git a/plugins/InspectionGadgets/src/inspectionDescriptions/EqualsReplaceableByObjectsCall.html b/plugins/InspectionGadgets/src/inspectionDescriptions/EqualsReplaceableByObjectsCall.html index 80c33db1a5d0..7568b89561de 100644 --- a/plugins/InspectionGadgets/src/inspectionDescriptions/EqualsReplaceableByObjectsCall.html +++ b/plugins/InspectionGadgets/src/inspectionDescriptions/EqualsReplaceableByObjectsCall.html @@ -2,12 +2,16 @@ Reports expressions that can be replaced with a call to java.util.Objects.equals(), which is available since JDK 1.7. For example: -
a != null && a.equals(b)
 +

+ 

a == b || a != null && a.equals(b)
+
+ 
a != null ? a.equals(b) : b == null
 +

- If a.equals(b) isn't preceded with a != null, replacing it with Objects.equals(a, b) - still makes sense, but slightly changes the semantics. For the cases where only equivalent replacement is needed, there's - the check box Report only null safe 'equals' calls to enforce that. + Replacing expressions like 

a != null && a.equals(b)
with + 
Objects.equals(a, b)
 slightly changes the semantics, + but if that's what you need there's a check box for that in the inspection settings.

This inspection only applies to projects and modules configured to use a language level of 7 or higher. diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/equals_replaceable_by_objects_call/EqualsReplaceableByObjectsCall.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/equals_replaceable_by_objects_call/EqualsReplaceableByObjectsCall.java index c333c3672bde..7b7125ecee41 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/equals_replaceable_by_objects_call/EqualsReplaceableByObjectsCall.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/equals_replaceable_by_objects_call/EqualsReplaceableByObjectsCall.java @@ -1,41 +1,41 @@ class EqualsReplaceableByObjectsCall { void yyy(Object a, Object b) { - boolean c = (a != null) && a.equals(b); + boolean c = (a != null) && a.equals(b); boolean d = (a != b) && (a == null || !a.equals(b)); boolean e = ((a) == (b)) || ((a) != (null) && (a).equals((b))); } void ignoreNullityCheck(Object a, Object b) { - boolean c = a.equals(b); + boolean c = a.equals(b); } void bar(T x, T y, T z) { - boolean b = !x.s.equals(y.s); - boolean c = x.s != null && x.s.equals(y.s); - boolean d = y.s != null && x.s.equals(y.s); - boolean e = x.s == null || !x.s.equals(y.s); + boolean b = !x.s.equals(y.s); + boolean c = x.s != null && x.s.equals(y.s); + boolean d = y.s != null && x.s.equals(y.s); + boolean e = x.s == null || !x.s.equals(y.s); boolean f = x.s != y.s && (x.s == null || !x.s.equals(y.s)); - boolean g = x.s != y.s || (x.s == null || !x.s.equals(y.s)); - boolean h = x.s != y.s && (z.s == null || !x.s.equals(y.s)); + boolean g = x.s != y.s || (x.s == null || !x.s.equals(y.s)); + boolean h = x.s != y.s && (z.s == null || !x.s.equals(y.s)); } void baz(T x, T y) { - boolean b = x.copy().equals(y.copy()); - boolean c = x != null && x.equals(y.copy()); - boolean d = x == null || !x.equals(y.copy()); - boolean e = x.copy() != null && x.copy().equals(y); - boolean f = x.copy() == null || !x.copy().equals(y); - boolean g = x.copy().s != null && x.copy().s.equals(y.s); - boolean h = x.copy().s == null || !x.copy().s.equals(y.s); - boolean i = x.s == y.copy().s || (x).s != null && (x.s).equals(y.copy().s); - boolean j = x.s != y.copy().s && ((x).s == null || !(x.s).equals(y.copy().s)); + boolean b = x.copy().equals(y.copy()); + boolean c = x != null && x.equals(y.copy()); + boolean d = x == null || !x.equals(y.copy()); + boolean e = x.copy() != null && x.copy().equals(y); + boolean f = x.copy() == null || !x.copy().equals(y); + boolean g = x.copy().s != null && x.copy().s.equals(y.s); + boolean h = x.copy().s == null || !x.copy().s.equals(y.s); + boolean i = x.s == y.copy().s || (x).s != null && (x.s).equals(y.copy().s); + boolean j = x.s != y.copy().s && ((x).s == null || !(x.s).equals(y.copy().s)); } void arr(T[] a, T[] b, int i) { - boolean c = a[i] != null && a[i].equals(b[i]); + boolean c = a[i] != null && a[i].equals(b[i]); boolean d = a[i] == null ? b[i] == null : a[i].equals(b[i]); - boolean e = a[i++] != null && a[i++].equals(b[i++]); - boolean f = a[--i] != null && a[--i].equals(b[--i]); + boolean e = a[i++] != null && a[i++].equals(b[i++]); + boolean f = a[--i] != null && a[--i].equals(b[--i]); } static class T { @@ -64,10 +64,10 @@ class EqualsReplaceableByObjectsCall { } } static boolean ab1(String s) { - return A.b.equals(s); + return A.b.equals(s); } static boolean ab2(String s) { - return A.b != null && A.b.equals(s); + return A.b != null && A.b.equals(s); } static boolean ab3(String s) { return A.b == s || A.b != null && A.b.equals(s); @@ -101,7 +101,7 @@ class EqualsReplaceableByObjectsCall { static final String NULL_CONSTANT = null; boolean nullConstant(Object o) { - return NULL_CONSTANT.equals(o); + return NULL_CONSTANT.equals(o); } static final Object NEW_CONSTANT = new Object(); @@ -139,28 +139,28 @@ class EqualsReplaceableByObjectsCall { return (s != null) ? (!s.equals(t.s)) : !(t.s == null); } boolean notMatches1(Ternary t) { - return s == null ? t.s != null : s.equals(t.s); + return s == null ? t.s != null : s.equals(t.s); } boolean notMatches2(Ternary t) { - return s != null ? t.s != null : s.equals(t.s); + return s != null ? t.s != null : s.equals(t.s); } boolean notMatches3(Ternary t) { - return ((s != null) ? !(s.equals(t.s)) : (t.s == null)); + return ((s != null) ? !(s.equals(t.s)) : (t.s == null)); } boolean notMatches4(Ternary t) { - return s == null ? t.s == null : !s.equals(t.s); + return s == null ? t.s == null : !s.equals(t.s); } boolean notMatches5(Ternary t) { - return s != null ? s.equals(t.s) : t.s != null; + return s != null ? s.equals(t.s) : t.s != null; } boolean notMatches6(Ternary t) { - return s == null ? !s.equals(t.s) : t.s != null; + return s == null ? !s.equals(t.s) : t.s != null; } boolean notMatches7(Ternary t) { - return s == null ? t.s == null : t.s.equals(s); + return s == null ? t.s == null : t.s.equals(s); } boolean notMatches8(Ternary t) { - return (s != null) ? (t.s.equals(s)) : (t.s == null); + return (s != null) ? (t.s.equals(s)) : (t.s == null); } } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/equals_replaceable_by_objects_call/EqualsReplaceableByObjectsCallCheckNull.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/equals_replaceable_by_objects_call/EqualsReplaceableByObjectsCallCheckNull.java index 9dc195c0fc67..83ed055c8b1e 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/equals_replaceable_by_objects_call/EqualsReplaceableByObjectsCallCheckNull.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/migration/equals_replaceable_by_objects_call/EqualsReplaceableByObjectsCallCheckNull.java @@ -6,27 +6,27 @@ class EqualsReplaceableByObjectsCall { } void ignoreNullityCheck(Object a, Object b) { - boolean c = a.equals(b); + boolean c = a.equals(b); } void bar(T x, T y, T z) { - boolean b = !x.s.equals(y.s); + boolean b = !x.s.equals(y.s); boolean c = x.s != null && x.s.equals(y.s); - boolean d = y.s != null && x.s.equals(y.s); + boolean d = y.s != null && x.s.equals(y.s); boolean e = x.s == null || !x.s.equals(y.s); boolean f = x.s != y.s && (x.s == null || !x.s.equals(y.s)); boolean g = x.s != y.s || (x.s == null || !x.s.equals(y.s)); - boolean h = x.s != y.s && (z.s == null || !x.s.equals(y.s)); + boolean h = x.s != y.s && (z.s == null || !x.s.equals(y.s)); } void baz(T x, T y) { - boolean b = x.copy().equals(y.copy()); + boolean b = x.copy().equals(y.copy()); boolean c = x != null && x.equals(y.copy()); boolean d = x == null || !x.equals(y.copy()); - boolean e = x.copy() != null && x.copy().equals(y); - boolean f = x.copy() == null || !x.copy().equals(y); - boolean g = x.copy().s != null && x.copy().s.equals(y.s); - boolean h = x.copy().s == null || !x.copy().s.equals(y.s); + boolean e = x.copy() != null && x.copy().equals(y); + boolean f = x.copy() == null || !x.copy().equals(y); + boolean g = x.copy().s != null && x.copy().s.equals(y.s); + boolean h = x.copy().s == null || !x.copy().s.equals(y.s); boolean i = x.s == y.copy().s || (x).s != null && (x.s).equals(y.copy().s); boolean j = x.s != y.copy().s && ((x).s == null || !(x.s).equals(y.copy().s)); } @@ -34,8 +34,8 @@ class EqualsReplaceableByObjectsCall { void arr(T[] a, T[] b, int i) { boolean c = a[i] != null && a[i].equals(b[i]); boolean d = a[i] == null ? b[i] == null : a[i].equals(b[i]); - boolean e = a[i++] != null && a[i++].equals(b[i++]); - boolean f = a[--i] != null && a[--i].equals(b[--i]); + boolean e = a[i++] != null && a[i++].equals(b[i++]); + boolean f = a[--i] != null && a[--i].equals(b[--i]); } static class T { @@ -64,7 +64,7 @@ class EqualsReplaceableByObjectsCall { } } static boolean ab1(String s) { - return A.b.equals(s); + return A.b.equals(s); } static boolean ab2(String s) { return A.b != null && A.b.equals(s); @@ -91,28 +91,28 @@ class EqualsReplaceableByObjectsCall { return (s != null) ? (!s.equals(t.s)) : !(t.s == null); } boolean notMatches1(Ternary t) { - return s == null ? t.s != null : s.equals(t.s); + return s == null ? t.s != null : s.equals(t.s); } boolean notMatches2(Ternary t) { - return s != null ? t.s != null : s.equals(t.s); + return s != null ? t.s != null : s.equals(t.s); } boolean notMatches3(Ternary t) { - return ((s != null) ? !(s.equals(t.s)) : (t.s == null)); + return ((s != null) ? !(s.equals(t.s)) : (t.s == null)); } boolean notMatches4(Ternary t) { - return s == null ? t.s == null : !s.equals(t.s); + return s == null ? t.s == null : !s.equals(t.s); } boolean notMatches5(Ternary t) { - return s != null ? s.equals(t.s) : t.s != null; + return s != null ? s.equals(t.s) : t.s != null; } boolean notMatches6(Ternary t) { - return s == null ? !s.equals(t.s) : t.s != null; + return s == null ? !s.equals(t.s) : t.s != null; } boolean notMatches7(Ternary t) { - return s == null ? t.s == null : t.s.equals(s); + return s == null ? t.s == null : t.s.equals(s); } boolean notMatches8(Ternary t) { - return (s != null) ? (t.s.equals(s)) : (t.s == null); + return (s != null) ? (t.s.equals(s)) : (t.s == null); } } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/migration/EqualsReplaceableByObjectsCallFixTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/migration/EqualsReplaceableByObjectsCallFixTest.java index 3354a856ff7e..79cc1288fa46 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/migration/EqualsReplaceableByObjectsCallFixTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/fixes/migration/EqualsReplaceableByObjectsCallFixTest.java @@ -27,16 +27,14 @@ import com.siyeh.ig.migration.EqualsReplaceableByObjectsCallInspection; */ public class EqualsReplaceableByObjectsCallFixTest extends IGQuickFixesTestCase { - private EqualsReplaceableByObjectsCallInspection myInspection; + public void testSimpleEquals() { doTest(); } + public void testSimpleNotEquals() { doTest(); } - public void testSimpleEquals() { doTestNoNullCheck(); } - public void testSimpleNotEquals() { doTestNoNullCheck(); } - - public void testQualifiedArgument() { doTestNoNullCheck(); } + public void testQualifiedArgument() { doTest(); } public void testQualifiedReciever() { doTest(); } - public void testExpressionReciever() { doTestNoNullCheck(); } - public void testExpressionArgument() { doTestNoNullCheck(); } + public void testExpressionReciever() { doTest(); } + public void testExpressionArgument() { doTest(); } public void testExpressionArgument2() { doTest(); } public void testLongEquals() { doTest(); } @@ -44,8 +42,8 @@ public class EqualsReplaceableByObjectsCallFixTest extends IGQuickFixesTestCase public void testShortEquals() { doTest(); } public void testShortNotEquals() { doTest(); } - public void testSuperEquals() { doTestNoNullCheck(); } - public void testThisEquals() { doTestNoNullCheck(); } + public void testSuperEquals() { doTest(); } + public void testThisEquals() { doTest(); } public void testQualifiedThisNotEqual() { doTest(); } public void testQualifiedSuperEqual() { doTest(); } @@ -68,14 +66,8 @@ public class EqualsReplaceableByObjectsCallFixTest extends IGQuickFixesTestCase @Override public void setUp() throws Exception { super.setUp(); - myInspection = new EqualsReplaceableByObjectsCallInspection(); - myFixture.enableInspections(myInspection); + myFixture.enableInspections(new EqualsReplaceableByObjectsCallInspection()); myRelativePath = "migration/equals_replaceable_by_objects_call"; myDefaultHint = InspectionGadgetsBundle.message("equals.replaceable.by.objects.call.quickfix"); } - - private void doTestNoNullCheck() { - myInspection.checkNotNull = false; - doTest(); - } } diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/migration/EqualsReplaceableByObjectsCallInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/migration/EqualsReplaceableByObjectsCallInspectionTest.java index b13d9ebc1bae..14d386975fc0 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/migration/EqualsReplaceableByObjectsCallInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/migration/EqualsReplaceableByObjectsCallInspectionTest.java @@ -54,7 +54,9 @@ public class EqualsReplaceableByObjectsCallInspectionTest extends LightInspectio boolean oldNotNull = myInspection.checkNotNull; try { myInspection.checkNotNull = checkNotNull; - doTest(); + + myFixture.configureByFile(getTestName(false) + ".java"); + myFixture.testHighlighting(true, true, false); } finally { myInspection.checkNotNull = oldNotNull;