DFA: better equals handling

This commit is contained in:
Tagir Valeev
2018-06-25 13:49:36 +07:00
parent b3a0e44a1e
commit b4d16557b3
6 changed files with 126 additions and 45 deletions
@@ -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;
@@ -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(); }
}
@@ -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;
}
}