mirror of
https://gitflic.ru/project/openide/openide.git
synced 2026-08-26 03:13:40 +07:00
DFA: better equals handling
This commit is contained in:
+13
@@ -30,6 +30,7 @@ import com.intellij.psi.util.PsiUtil;
|
||||
import com.intellij.psi.util.TypeConversionUtil;
|
||||
import com.intellij.util.*;
|
||||
import com.intellij.util.containers.ContainerUtil;
|
||||
import com.siyeh.ig.bugs.EqualsWithItselfInspection;
|
||||
import com.siyeh.ig.fixes.EqualsToEqualityFix;
|
||||
import com.siyeh.ig.psiutils.*;
|
||||
import one.util.streamex.IntStreamEx;
|
||||
@@ -690,6 +691,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
|
||||
}
|
||||
|
||||
private void reportConstantBoolean(ProblemsHolder holder, PsiElement psiAnchor, boolean evaluatesToTrue) {
|
||||
if (shouldBeSuppressed(psiAnchor)) return;
|
||||
boolean isAssertion = isAssertionEffectively(psiAnchor, evaluatesToTrue);
|
||||
if (!DONT_REPORT_TRUE_ASSERT_STATEMENTS || !isAssertion) {
|
||||
List<LocalQuickFix> fixes = new ArrayList<>();
|
||||
@@ -706,6 +708,17 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool
|
||||
}
|
||||
}
|
||||
|
||||
private static boolean shouldBeSuppressed(PsiElement anchor) {
|
||||
if (!(anchor instanceof PsiExpression)) return false;
|
||||
PsiExpression expression = (PsiExpression)anchor;
|
||||
while (expression != null && BoolUtils.isNegation(expression)) {
|
||||
expression = BoolUtils.getNegated(expression);
|
||||
}
|
||||
PsiMethodCallExpression call = ObjectUtils.tryCast(expression, PsiMethodCallExpression.class);
|
||||
// Reported by "Equals with itself" inspection; avoid double reporting
|
||||
return call != null && EqualsWithItselfInspection.isEqualsWithItself(call);
|
||||
}
|
||||
|
||||
private static LocalQuickFix createReplaceWithNullCheckFix(PsiElement psiAnchor, boolean evaluatesToTrue) {
|
||||
if (evaluatesToTrue) return null;
|
||||
if (!(psiAnchor instanceof PsiMethodCallExpression) || !MethodCallUtils.isEqualsCall((PsiMethodCallExpression)psiAnchor)) return null;
|
||||
|
||||
+40
-21
@@ -16,6 +16,7 @@
|
||||
package com.intellij.codeInspection.dataFlow;
|
||||
|
||||
import com.intellij.codeInspection.dataFlow.StandardMethodContract.ValueConstraint;
|
||||
import com.intellij.codeInspection.dataFlow.value.DfaRelationValue;
|
||||
import com.intellij.codeInspection.dataFlow.value.DfaRelationValue.RelationType;
|
||||
import com.intellij.lang.injection.InjectedLanguageManager;
|
||||
import com.intellij.psi.*;
|
||||
@@ -35,6 +36,7 @@ import java.util.List;
|
||||
import java.util.function.Supplier;
|
||||
|
||||
import static com.intellij.codeInspection.dataFlow.ContractReturnValue.*;
|
||||
import static com.intellij.codeInspection.dataFlow.MethodContract.singleConditionContract;
|
||||
import static com.intellij.codeInspection.dataFlow.MethodContract.trivialContract;
|
||||
import static com.intellij.codeInspection.dataFlow.StandardMethodContract.ValueConstraint.*;
|
||||
import static com.intellij.codeInspection.dataFlow.StandardMethodContract.createConstraintArray;
|
||||
@@ -48,11 +50,11 @@ public class HardcodedContracts {
|
||||
private static final List<MethodContract> ARRAY_RANGE_CONTRACTS = ContainerUtil.immutableList(
|
||||
nonnegativeArgumentContract(1),
|
||||
nonnegativeArgumentContract(2),
|
||||
MethodContract.singleConditionContract(ContractValue.argument(1), RelationType.GT,
|
||||
ContractValue.argument(0).specialField(SpecialField.ARRAY_LENGTH), fail()),
|
||||
MethodContract.singleConditionContract(ContractValue.argument(2), RelationType.GT,
|
||||
ContractValue.argument(0).specialField(SpecialField.ARRAY_LENGTH), fail()),
|
||||
MethodContract.singleConditionContract(ContractValue.argument(1), RelationType.GT, ContractValue.argument(2), fail())
|
||||
singleConditionContract(ContractValue.argument(1), RelationType.GT, ContractValue.argument(0).specialField(SpecialField.ARRAY_LENGTH),
|
||||
fail()),
|
||||
singleConditionContract(ContractValue.argument(2), RelationType.GT, ContractValue.argument(0).specialField(SpecialField.ARRAY_LENGTH),
|
||||
fail()),
|
||||
singleConditionContract(ContractValue.argument(1), RelationType.GT, ContractValue.argument(2), fail())
|
||||
);
|
||||
|
||||
private static final CallMatcher QUEUE_POLL = instanceCall("java.util.Queue", "poll").parameterCount(0);
|
||||
@@ -103,17 +105,17 @@ public class HardcodedContracts {
|
||||
.register(instanceCall(JAVA_UTIL_MAP, "equals").parameterTypes(JAVA_LANG_OBJECT),
|
||||
ContractProvider.list(SpecialField.MAP_SIZE::getEqualsContracts))
|
||||
.register(instanceCall(JAVA_UTIL_COLLECTION, "contains").parameterCount(1),
|
||||
ContractProvider.single(() -> MethodContract.singleConditionContract(
|
||||
ContractProvider.single(() -> singleConditionContract(
|
||||
ContractValue.qualifier().specialField(SpecialField.COLLECTION_SIZE), RelationType.EQ, ContractValue.zero(),
|
||||
returnFalse())))
|
||||
.register(instanceCall(JAVA_UTIL_MAP, "containsKey", "containsValue").parameterCount(1),
|
||||
ContractProvider.single(() -> MethodContract.singleConditionContract(
|
||||
ContractProvider.single(() -> singleConditionContract(
|
||||
ContractValue.qualifier().specialField(SpecialField.MAP_SIZE), RelationType.EQ, ContractValue.zero(), returnFalse())))
|
||||
.register(instanceCall(JAVA_UTIL_LIST, "get").parameterTypes("int"),
|
||||
ContractProvider.list(() -> Arrays.asList(nonnegativeArgumentContract(0),
|
||||
specialFieldRangeContract(0, RelationType.LT, SpecialField.COLLECTION_SIZE))))
|
||||
.register(instanceCall("java.util.SortedSet", "first", "last").parameterCount(0),
|
||||
ContractProvider.single(() -> MethodContract.singleConditionContract(
|
||||
ContractProvider.single(() -> singleConditionContract(
|
||||
ContractValue.qualifier().specialField(SpecialField.COLLECTION_SIZE), RelationType.EQ,
|
||||
ContractValue.zero(), fail())))
|
||||
// All these methods take array as 1st parameter, from index as 2nd and to index as 3rd
|
||||
@@ -123,7 +125,7 @@ public class HardcodedContracts {
|
||||
.register(staticCall("org.mockito.ArgumentMatchers", "argThat").parameterCount(1),
|
||||
ContractProvider.single(() -> new StandardMethodContract(new ValueConstraint[]{ANY_VALUE}, returnAny())))
|
||||
.register(instanceCall("java.util.Queue", "peek", "poll").parameterCount(0),
|
||||
(call, paramCount) -> Arrays.asList(MethodContract.singleConditionContract(
|
||||
(call, paramCount) -> Arrays.asList(singleConditionContract(
|
||||
ContractValue.qualifier().specialField(SpecialField.COLLECTION_SIZE), RelationType.EQ,
|
||||
ContractValue.zero(), returnNull()), trivialContract(returnAny())))
|
||||
.register(anyOf(staticCall(JAVA_LANG_MATH, "max").parameterTypes("int", "int"),
|
||||
@@ -137,9 +139,11 @@ public class HardcodedContracts {
|
||||
staticCall(JAVA_LANG_LONG, "min").parameterTypes("long", "long")),
|
||||
(call, paramCount) -> mathMinMax(false))
|
||||
.register(instanceCall(JAVA_LANG_STRING, "startsWith", "endsWith", "contains"),
|
||||
ContractProvider.single(() -> MethodContract.singleConditionContract(
|
||||
ContractProvider.single(() -> singleConditionContract(
|
||||
ContractValue.qualifier().specialField(SpecialField.STRING_LENGTH), RelationType.LT,
|
||||
ContractValue.argument(0).specialField(SpecialField.STRING_LENGTH), returnFalse())));
|
||||
ContractValue.argument(0).specialField(SpecialField.STRING_LENGTH), returnFalse())))
|
||||
.register(instanceCall(JAVA_LANG_OBJECT, "equals").parameterTypes(JAVA_LANG_OBJECT),
|
||||
(call, paramCount) -> equalsContracts(call));
|
||||
|
||||
public static List<MethodContract> getHardcodedContracts(@NotNull PsiMethod method, @Nullable PsiMethodCallExpression call) {
|
||||
PsiClass owner = method.getContainingClass();
|
||||
@@ -204,34 +208,49 @@ public class HardcodedContracts {
|
||||
if (endLimited) {
|
||||
contracts.add(nonnegativeArgumentContract(1));
|
||||
contracts.add(specialFieldRangeContract(1, RelationType.LE, SpecialField.STRING_LENGTH));
|
||||
contracts.add(MethodContract
|
||||
.singleConditionContract(ContractValue.argument(0), RelationType.LE.getNegated(),
|
||||
ContractValue.argument(1), fail()));
|
||||
contracts.add(singleConditionContract(ContractValue.argument(0), RelationType.LE.getNegated(), ContractValue.argument(1), fail()));
|
||||
}
|
||||
return contracts;
|
||||
}
|
||||
|
||||
static MethodContract optionalAbsentContract(ContractReturnValue returnValue) {
|
||||
return MethodContract
|
||||
.singleConditionContract(ContractValue.qualifier(), RelationType.IS, ContractValue.optionalValue(false), returnValue);
|
||||
return singleConditionContract(ContractValue.qualifier(), RelationType.IS, ContractValue.optionalValue(false), returnValue);
|
||||
}
|
||||
|
||||
static MethodContract nonnegativeArgumentContract(int argNumber) {
|
||||
return MethodContract
|
||||
.singleConditionContract(ContractValue.argument(argNumber), RelationType.LT, ContractValue.zero(), fail());
|
||||
return singleConditionContract(ContractValue.argument(argNumber), RelationType.LT, ContractValue.zero(), fail());
|
||||
}
|
||||
|
||||
static MethodContract specialFieldRangeContract(int index, RelationType type, SpecialField specialField) {
|
||||
return MethodContract.singleConditionContract(ContractValue.argument(index), type.getNegated(),
|
||||
ContractValue.qualifier().specialField(specialField), fail());
|
||||
return singleConditionContract(ContractValue.argument(index), type.getNegated(), ContractValue.qualifier().specialField(specialField),
|
||||
fail());
|
||||
}
|
||||
|
||||
static List<MethodContract> mathMinMax(boolean isMax) {
|
||||
return Arrays.asList(MethodContract.singleConditionContract(
|
||||
return Arrays.asList(singleConditionContract(
|
||||
ContractValue.argument(0), isMax ? RelationType.GT : RelationType.LT, ContractValue.argument(1), returnParameter(0)),
|
||||
trivialContract(returnParameter(1)));
|
||||
}
|
||||
|
||||
private static List<MethodContract> equalsContracts(PsiMethodCallExpression call) {
|
||||
PsiExpression qualifier = call == null ? null : call.getMethodExpression().getQualifierExpression();
|
||||
if (qualifier != null && knownAsEqualByReference(qualifier.getType())) {
|
||||
return Arrays.asList(
|
||||
singleConditionContract(ContractValue.qualifier(), RelationType.EQ, ContractValue.argument(0), returnTrue()),
|
||||
trivialContract(returnFalse())
|
||||
);
|
||||
}
|
||||
return Arrays.asList(new StandardMethodContract(new StandardMethodContract.ValueConstraint[]{NULL_VALUE}, returnFalse()),
|
||||
singleConditionContract(ContractValue.qualifier(), DfaRelationValue.RelationType.EQ,
|
||||
ContractValue.argument(0), returnTrue()));
|
||||
}
|
||||
|
||||
private static boolean knownAsEqualByReference(PsiType type) {
|
||||
if (type instanceof PsiArrayType) return true;
|
||||
PsiClass psiClass = PsiUtil.resolveClassInClassTypeOnly(type);
|
||||
return psiClass != null && (psiClass.isEnum() || JAVA_LANG_CLASS.equals(psiClass.getQualifiedName()));
|
||||
}
|
||||
|
||||
private static boolean isJunit(String className) {
|
||||
return className.startsWith("junit.framework.") || className.startsWith("org.junit.")
|
||||
|| className.equals("org.testng.AssertJUnit");
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
import java.util.List;
|
||||
|
||||
class Test {
|
||||
enum A {X, Y, Z};
|
||||
List<String> list1, list2;
|
||||
|
||||
void test(List<String> tokens) {
|
||||
String t = "int";
|
||||
A s = A.X;
|
||||
A l = A.X;
|
||||
for (String token : tokens) {
|
||||
if ("unsigned".equals(token)) {
|
||||
s = A.Z;
|
||||
}
|
||||
else if ("signed".equals(token)) {
|
||||
s = A.Y;
|
||||
}
|
||||
else if ("short".equals(token)) {
|
||||
l = A.Y;
|
||||
}
|
||||
else if ("long".equals(token)) {
|
||||
l = A.Z;
|
||||
}
|
||||
else if (list1.contains(token) || list2.contains(token)) {
|
||||
t = token;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,18 @@
|
||||
import java.util.List;
|
||||
|
||||
class Test {
|
||||
void test(Object x, Object y) {
|
||||
if(x.equals(x)) {
|
||||
// do not report here; reported by EqualsWithItselfInspection
|
||||
System.out.println("always");
|
||||
}
|
||||
if(!x.equals(x)) {
|
||||
// do not report here; reported by EqualsWithItselfInspection
|
||||
System.out.println("never");
|
||||
}
|
||||
y = x;
|
||||
if(x.equals(y)) {
|
||||
System.out.println("always");
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -619,4 +619,6 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase {
|
||||
public void testNanComparisonWrong() { doTest(); }
|
||||
public void testConstantMethods() { doTest(); }
|
||||
public void testPolyadicEquality() { doTest(); }
|
||||
public void testEqualsInLoopNotTooComplex() { doTest(); }
|
||||
public void testEqualsWithItself() { doTest(); }
|
||||
}
|
||||
|
||||
+24
-24
@@ -53,31 +53,31 @@ public class EqualsWithItselfInspection extends BaseInspection {
|
||||
@Override
|
||||
public void visitMethodCallExpression(PsiMethodCallExpression expression) {
|
||||
super.visitMethodCallExpression(expression);
|
||||
if (!MethodCallUtils.isEqualsCall(expression) &&
|
||||
!MethodCallUtils.isEqualsIgnoreCaseCall(expression) &&
|
||||
!MethodCallUtils.isCompareToCall(expression) &&
|
||||
!MethodCallUtils.isCompareToIgnoreCaseCall(expression)) {
|
||||
return;
|
||||
if (isEqualsWithItself(expression)) {
|
||||
registerMethodCallError(expression);
|
||||
}
|
||||
final PsiReferenceExpression methodExpression = expression.getMethodExpression();
|
||||
final PsiExpressionList argumentList = expression.getArgumentList();
|
||||
final PsiExpression[] arguments = argumentList.getExpressions();
|
||||
if (arguments.length != 1) {
|
||||
return;
|
||||
}
|
||||
final PsiExpression argument = ParenthesesUtils.stripParentheses(arguments[0]);
|
||||
final PsiExpression qualifier = methodExpression.getQualifierExpression();
|
||||
if (qualifier == null) {
|
||||
if (!(argument instanceof PsiThisExpression)) {
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
if (!EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(qualifier, argument) ||
|
||||
SideEffectChecker.mayHaveSideEffects(qualifier)) {
|
||||
return;
|
||||
}
|
||||
}
|
||||
registerMethodCallError(expression);
|
||||
}
|
||||
}
|
||||
|
||||
public static boolean isEqualsWithItself(PsiMethodCallExpression expression) {
|
||||
if (!MethodCallUtils.isEqualsCall(expression) &&
|
||||
!MethodCallUtils.isEqualsIgnoreCaseCall(expression) &&
|
||||
!MethodCallUtils.isCompareToCall(expression) &&
|
||||
!MethodCallUtils.isCompareToIgnoreCaseCall(expression)) {
|
||||
return false;
|
||||
}
|
||||
final PsiReferenceExpression methodExpression = expression.getMethodExpression();
|
||||
final PsiExpressionList argumentList = expression.getArgumentList();
|
||||
final PsiExpression[] arguments = argumentList.getExpressions();
|
||||
if (arguments.length != 1) {
|
||||
return false;
|
||||
}
|
||||
final PsiExpression argument = ParenthesesUtils.stripParentheses(arguments[0]);
|
||||
final PsiExpression qualifier = methodExpression.getQualifierExpression();
|
||||
if (qualifier != null) {
|
||||
return EquivalenceChecker.getCanonicalPsiEquivalence().expressionsAreEquivalent(qualifier, argument) &&
|
||||
!SideEffectChecker.mayHaveSideEffects(qualifier);
|
||||
}
|
||||
return argument instanceof PsiThisExpression;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user