From 1b7782b591b09804b964f99a3f763e316ae5baee Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 30 Mar 2018 11:56:40 +0700 Subject: [PATCH] EqualsWhichDoesntCheckParameterClass: support lambdas/method references Fixes IDEA-189199 'equals()' should check the class of its parameter: false-positive on method reference --- ...chDoesntCheckParameterClassInspection.java | 146 ++++++++++--- .../ig/bugs/ParameterClassCheckVisitor.java | 193 ------------------ .../EqualsWhichDoesntCheckParameterClass.java | 26 +++ ...esntCheckParameterClassInspectionTest.java | 8 + 4 files changed, 147 insertions(+), 226 deletions(-) delete mode 100644 plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ParameterClassCheckVisitor.java diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/EqualsWhichDoesntCheckParameterClassInspection.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/EqualsWhichDoesntCheckParameterClassInspection.java index f553a16a48c0..8a98a3f4e35c 100644 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/EqualsWhichDoesntCheckParameterClassInspection.java +++ b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/EqualsWhichDoesntCheckParameterClassInspection.java @@ -16,16 +16,28 @@ package com.siyeh.ig.bugs; import com.intellij.psi.*; +import com.intellij.psi.util.PsiTreeUtil; +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.callMatcher.CallMatcher; import com.siyeh.ig.psiutils.ControlFlowUtils; import com.siyeh.ig.psiutils.ExpressionUtils; import com.siyeh.ig.psiutils.MethodUtils; -import org.jetbrains.annotations.NonNls; +import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; +import static com.intellij.util.ObjectUtils.tryCast; + public class EqualsWhichDoesntCheckParameterClassInspection extends BaseInspection { + private static final CallMatcher REFLECTION_EQUALS = + CallMatcher.staticCall("org.apache.commons.lang.builder.EqualsBuilder", "reflectionEquals"); + private static final CallMatcher CLASS_IS_INSTANCE = + CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_CLASS, "isInstance").parameterCount(1); + private static final CallMatcher OBJECT_GET_CLASS = + CallMatcher.instanceCall(CommonClassNames.JAVA_LANG_OBJECT, "getClass").parameterCount(0); @Override @NotNull @@ -90,7 +102,7 @@ public class EqualsWhichDoesntCheckParameterClassInspection extends BaseInspecti if (Boolean.FALSE.equals(constant)) { return true; // incomplete code } - if (isEqualsBuilderReflectionEquals(returnValue)) { + if (REFLECTION_EQUALS.matches(returnValue)) { return true; } if (isIdentityEquals(returnValue, parameter)) { @@ -110,43 +122,111 @@ public class EqualsWhichDoesntCheckParameterClassInspection extends BaseInspecti } private static boolean isIdentityEquals(PsiExpression lhs, PsiExpression rhs, PsiParameter parameter) { - if (!(lhs instanceof PsiReferenceExpression)) { - return false; - } - final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)lhs; - final PsiElement target = referenceExpression.resolve(); - if (target != parameter) { - return false; - } - if (!(rhs instanceof PsiThisExpression)) { - return false; - } - final PsiThisExpression thisExpression = (PsiThisExpression)rhs; - return thisExpression.getQualifier() == null; + return ExpressionUtils.isReferenceTo(lhs, parameter) && + rhs instanceof PsiThisExpression && + ((PsiThisExpression)rhs).getQualifier() == null; + } + } + + private static class ParameterClassCheckVisitor extends JavaRecursiveElementWalkingVisitor { + private final PsiParameter myParameter; + private boolean myChecked; + + ParameterClassCheckVisitor(@NotNull PsiParameter parameter) { + myParameter = parameter; } - private static boolean isEqualsBuilderReflectionEquals(PsiExpression expression) { - if (!(expression instanceof PsiMethodCallExpression)) { - return false; + private void makeChecked() { + myChecked = true; + stopWalking(); + } + + @Contract("null -> false") + private boolean isParameterReference(PsiExpression operand) { + PsiReferenceExpression ref = tryCast(PsiUtil.skipParenthesizedExprDown(operand), PsiReferenceExpression.class); + if (ref == null) return false; + PsiParameter target = tryCast(ref.resolve(), PsiParameter.class); + if (target == myParameter) return true; + if (target == null) return false; + return target.getParent() instanceof PsiParameterList && target.getParent().getParent() instanceof PsiLambdaExpression; + } + + private boolean isGetInstanceCall(PsiMethodCallExpression call) { + if (!CLASS_IS_INSTANCE.test(call)) return false; + final PsiExpression arg = call.getArgumentList().getExpressions()[0]; + return isParameterReference(arg); + } + + private boolean isGetClassCall(PsiMethodCallExpression call) { + if (!OBJECT_GET_CLASS.test(call)) return false; + final PsiExpression qualifier = call.getMethodExpression().getQualifierExpression(); + return isParameterReference(qualifier); + } + + private boolean isCallToSuperEquals(PsiMethodCallExpression call) { + final PsiReferenceExpression methodExpression = call.getMethodExpression(); + final PsiExpression qualifierExpression = methodExpression.getQualifierExpression(); + if (!(qualifierExpression instanceof PsiSuperExpression)) return false; + final String name = methodExpression.getReferenceName(); + if (!HardcodedMethodConstants.EQUALS.equals(name)) return false; + final PsiExpression[] arguments = call.getArgumentList().getExpressions(); + if (arguments.length != 1) return false; + return isParameterReference(arguments[0]); + } + + @Override + public void visitMethodCallExpression(@NotNull PsiMethodCallExpression expression) { + super.visitMethodCallExpression(expression); + if (isGetClassCall(expression) || isGetInstanceCall(expression) || isCallToSuperEquals(expression)) { + makeChecked(); } - final PsiMethodCallExpression methodCallExpression = (PsiMethodCallExpression)expression; - final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); - @NonNls final String referenceName = methodExpression.getReferenceName(); - if (!"reflectionEquals".equals(referenceName)) { - return false; + } + + @Override + public void visitMethodReferenceExpression(PsiMethodReferenceExpression expression) { + super.visitMethodReferenceExpression(expression); + if (CLASS_IS_INSTANCE.methodReferenceMatches(expression)) { + makeChecked(); } - final PsiExpression qualifier = methodExpression.getQualifierExpression(); - if (!(qualifier instanceof PsiReferenceExpression)) { - return false; + } + + @Override + public void visitInstanceOfExpression(@NotNull PsiInstanceOfExpression expression) { + super.visitInstanceOfExpression(expression); + if (isParameterReference(expression.getOperand())) { + makeChecked(); } - final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)qualifier; - final PsiElement target = referenceExpression.resolve(); - if (!(target instanceof PsiClass)) { - return false; + } + + @Override + public void visitTypeCastExpression(PsiTypeCastExpression expression) { + super.visitTypeCastExpression(expression); + final PsiExpression operand = expression.getOperand(); + if (!isParameterReference(operand)) return; + final PsiTryStatement statement = PsiTreeUtil.getParentOfType(expression, PsiTryStatement.class); + if (statement == null) return; + final PsiParameter[] parameters = statement.getCatchBlockParameters(); + if (parameters.length < 2) return; + boolean nullPointerExceptionFound = false; + boolean classCastExceptionFound = false; + for (PsiParameter parameter : parameters) { + final PsiType type = parameter.getType(); + if (type.equalsToText("java.lang.NullPointerException")) { + nullPointerExceptionFound = true; + if (classCastExceptionFound) break; + } + else if (type.equalsToText("java.lang.ClassCastException")) { + classCastExceptionFound = true; + if (nullPointerExceptionFound) break; + } } - final PsiClass aClass = (PsiClass)target; - final String className = aClass.getQualifiedName(); - return "org.apache.commons.lang.builder.EqualsBuilder".equals(className); + if (classCastExceptionFound && nullPointerExceptionFound) { + makeChecked(); + } + } + + public boolean isChecked() { + return myChecked; } } } \ No newline at end of file diff --git a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ParameterClassCheckVisitor.java b/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ParameterClassCheckVisitor.java deleted file mode 100644 index 16ed75ade3da..000000000000 --- a/plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/bugs/ParameterClassCheckVisitor.java +++ /dev/null @@ -1,193 +0,0 @@ -/* - * Copyright 2003-2018 Dave Griffith, Bas Leijdekkers - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.siyeh.ig.bugs; - -import com.intellij.psi.*; -import com.intellij.psi.util.PsiTreeUtil; -import com.siyeh.HardcodedMethodConstants; -import org.jetbrains.annotations.NotNull; - -class ParameterClassCheckVisitor extends JavaRecursiveElementWalkingVisitor { - - private final PsiParameter parameter; - - private boolean checked; - - ParameterClassCheckVisitor(@NotNull PsiParameter parameter) { - this.parameter = parameter; - } - - @Override - public void visitElement(@NotNull PsiElement element) { - if (!checked) { - super.visitElement(element); - } - } - - @Override - public void visitMethodCallExpression( - @NotNull PsiMethodCallExpression expression) { - if (checked) { - return; - } - super.visitMethodCallExpression(expression); - if (isGetClassCall(expression) || isGetInstanceCall(expression) || isCallToSuperEquals(expression)) { - checked = true; - } - } - - private boolean isGetInstanceCall(PsiMethodCallExpression methodCallExpression) { - final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); - final String methodName = methodExpression.getReferenceName(); - if (!HardcodedMethodConstants.IS_INSTANCE.equals(methodName)) { - return false; - } - final PsiMethod method = methodCallExpression.resolveMethod(); - if (method == null) { - return false; - } - final PsiClass aClass = method.getContainingClass(); - if (aClass == null) { - return false; - } - final String className = aClass.getQualifiedName(); - if (!CommonClassNames.JAVA_LANG_CLASS.equals(className)) { - return false; - } - final PsiExpressionList argumentList = methodCallExpression.getArgumentList(); - final PsiExpression[] expressions = argumentList.getExpressions(); - if (expressions.length != 1) { - return false; - } - final PsiExpression expression = expressions[0]; - return isParameterReference(expression); - } - - private boolean isGetClassCall(PsiMethodCallExpression methodCallExpression) { - final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); - final String methodName = methodExpression.getReferenceName(); - if (!HardcodedMethodConstants.GET_CLASS.equals(methodName) || !methodCallExpression.getArgumentList().isEmpty()) { - return false; - } - final PsiMethod method = methodCallExpression.resolveMethod(); - if (method == null) { - return false; - } - final PsiClass aClass = method.getContainingClass(); - if (aClass == null) { - return false; - } - final String className = aClass.getQualifiedName(); - if (!CommonClassNames.JAVA_LANG_OBJECT.equals(className)) { - return false; - } - final PsiExpression qualifier = methodExpression.getQualifierExpression(); - return isParameterReference(qualifier); - } - - private boolean isCallToSuperEquals(PsiMethodCallExpression methodCallExpression) { - final PsiReferenceExpression methodExpression = methodCallExpression.getMethodExpression(); - final PsiExpression qualifierExpression = methodExpression.getQualifierExpression(); - if (!(qualifierExpression instanceof PsiSuperExpression)) { - return false; - } - final String name = methodExpression.getReferenceName(); - if (!HardcodedMethodConstants.EQUALS.equals(name)) { - return false; - } - final PsiExpressionList argumentList = methodCallExpression.getArgumentList(); - final PsiExpression[] arguments = argumentList.getExpressions(); - if (arguments.length != 1) { - return false; - } - final PsiExpression argument = arguments[0]; - if (!(argument instanceof PsiReferenceExpression)) { - return false; - } - final PsiReferenceExpression referenceExpression = (PsiReferenceExpression)argument; - final PsiElement target = referenceExpression.resolve(); - return parameter.equals(target); - } - - @Override - public void visitInstanceOfExpression(@NotNull PsiInstanceOfExpression expression) { - if (checked) { - return; - } - super.visitInstanceOfExpression(expression); - final PsiExpression operand = expression.getOperand(); - if (isParameterReference(operand)) { - checked = true; - } - } - - @Override - public void visitTypeCastExpression( - PsiTypeCastExpression expression) { - if (checked) { - return; - } - super.visitTypeCastExpression(expression); - final PsiExpression operand = expression.getOperand(); - if (!isParameterReference(operand)) { - return; - } - final PsiTryStatement statement = PsiTreeUtil.getParentOfType(expression, PsiTryStatement.class); - if (statement == null) { - return; - } - final PsiParameter[] parameters = statement.getCatchBlockParameters(); - if (parameters.length < 2) { - return; - } - boolean nullPointerExceptionFound = false; - boolean classCastExceptionFound = false; - for (PsiParameter parameter : parameters) { - final PsiType type = parameter.getType(); - if (type.equalsToText("java.lang.NullPointerException")) { - nullPointerExceptionFound = true; - if (classCastExceptionFound) { - break; - } - } - else if (type.equalsToText("java.lang.ClassCastException")) { - classCastExceptionFound = true; - if (nullPointerExceptionFound) { - break; - } - } - } - if (classCastExceptionFound && nullPointerExceptionFound) { - checked = true; - } - } - - private boolean isParameterReference(PsiExpression operand) { - if (operand == null) { - return false; - } - if (!(operand instanceof PsiReferenceExpression)) { - return false; - } - final PsiReferenceExpression expression = (PsiReferenceExpression)operand; - final PsiElement referent = expression.resolve(); - return referent != null && referent.equals(parameter); - } - - public boolean isChecked() { - return checked; - } -} \ No newline at end of file diff --git a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/equals_which_doesnt_check_parameter_class/EqualsWhichDoesntCheckParameterClass.java b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/equals_which_doesnt_check_parameter_class/EqualsWhichDoesntCheckParameterClass.java index 935c5d1ffde4..e3bdef7c5dd8 100644 --- a/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/equals_which_doesnt_check_parameter_class/EqualsWhichDoesntCheckParameterClass.java +++ b/plugins/InspectionGadgets/test/com/siyeh/igtest/bugs/equals_which_doesnt_check_parameter_class/EqualsWhichDoesntCheckParameterClass.java @@ -1,5 +1,7 @@ package com.siyeh.igtest.bugs.equals_which_doesnt_check_parameter; +import java.util.Optional; + public class EqualsWhichDoesntCheckParameterClass { private int foo; @@ -102,3 +104,27 @@ class Six { return this == obj; } } +class Cell { + int x, y; + + @Override + public boolean equals(Object obj) { + return Optional.ofNullable(obj) + .filter(Cell.class::isInstance) + .map(that -> (Cell)that) + .filter(that -> x == that.x && y == that.y) + .isPresent(); + } +} +class Cell2 { + int x, y; + + @Override + public boolean equals(Object obj) { + return Optional.ofNullable(obj) + .filter(x -> x instanceof Cell2) + .map(that -> (Cell2)that) + .filter(that -> x == that.x && y == that.y) + .isPresent(); + } +} \ No newline at end of file diff --git a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/EqualsWhichDoesntCheckParameterClassInspectionTest.java b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/EqualsWhichDoesntCheckParameterClassInspectionTest.java index b525d6622da0..fb3343bdbab9 100644 --- a/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/EqualsWhichDoesntCheckParameterClassInspectionTest.java +++ b/plugins/InspectionGadgets/testsrc/com/siyeh/ig/bugs/EqualsWhichDoesntCheckParameterClassInspectionTest.java @@ -16,7 +16,9 @@ package com.siyeh.ig.bugs; import com.intellij.codeInspection.InspectionProfileEntry; +import com.intellij.testFramework.LightProjectDescriptor; import com.siyeh.ig.LightInspectionTestCase; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; public class EqualsWhichDoesntCheckParameterClassInspectionTest extends LightInspectionTestCase { @@ -25,6 +27,12 @@ public class EqualsWhichDoesntCheckParameterClassInspectionTest extends LightIns doTest(); } + @NotNull + @Override + protected LightProjectDescriptor getProjectDescriptor() { + return JAVA_8; + } + @Nullable @Override protected InspectionProfileEntry getInspection() {