IDEA-197241 Track content equality instead of reference equality for Strings in DFA

Also fixes IDEA-197195 Code Inspection shows wrong results when comparing same String but with different references, one from string pool and one from heap
This commit is contained in:
Tagir Valeev
2018-08-13 17:07:56 +07:00
parent 2c0d0b16a6
commit 594243cb64
7 changed files with 74 additions and 6 deletions
@@ -97,7 +97,7 @@ public class HardcodedContracts {
ContractProvider.list(SpecialField.COLLECTION_SIZE::getEmptyContracts))
.register(instanceCall(JAVA_UTIL_MAP, "isEmpty").parameterCount(0),
ContractProvider.list(SpecialField.MAP_SIZE::getEmptyContracts))
.register(instanceCall(JAVA_LANG_STRING, "equals", "equalsIgnoreCase").parameterCount(1),
.register(instanceCall(JAVA_LANG_STRING, "equalsIgnoreCase").parameterCount(1),
ContractProvider.list(SpecialField.STRING_LENGTH::getEqualsContracts))
.register(anyOf(instanceCall(JAVA_UTIL_SET, "equals").parameterTypes(JAVA_LANG_OBJECT),
instanceCall(JAVA_UTIL_LIST, "equals").parameterTypes(JAVA_LANG_OBJECT)),
@@ -238,7 +238,8 @@ public class HardcodedContracts {
private static List<MethodContract> equalsContracts(PsiMethodCallExpression call) {
PsiExpression qualifier = call == null ? null : call.getMethodExpression().getQualifierExpression();
if (qualifier != null && knownAsEqualByReference(qualifier.getType())) {
if (qualifier != null && (knownAsEqualByReference(qualifier.getType()) ||
TypeUtils.isJavaLangString(qualifier.getType()))) {
return Arrays.asList(
singleConditionContract(ContractValue.qualifier(), RelationType.EQ, ContractValue.argument(0), returnTrue()),
trivialContract(returnFalse())
@@ -646,6 +646,19 @@ public class StandardInstructionVisitor extends InstructionVisitor {
DfaValue dfaLeft,
RelationType relationType) {
DfaValueFactory factory = runner.getFactory();
if((relationType == RelationType.EQ || relationType == RelationType.NE) &&
isStringComparison(instruction.getExpression()) &&
!memState.isNull(dfaLeft) && !memState.isNull(dfaRight)) {
ArrayList<DfaInstructionState> states = new ArrayList<>(2);
DfaMemoryState equality = memState.createCopy();
if (equality.applyCondition(factory.createCondition(dfaLeft, RelationType.EQ, dfaRight))) {
states.add(makeBooleanResult(instruction, runner, equality, ThreeState.UNSURE));
}
if (memState.applyCondition(factory.createCondition(dfaLeft, RelationType.NE, dfaRight))) {
states.add(makeBooleanResult(instruction, runner, memState, ThreeState.fromBoolean(relationType == RelationType.NE)));
}
return states.toArray(DfaInstructionState.EMPTY_ARRAY);
}
RelationType[] relations = splitRelation(relationType);
ArrayList<DfaInstructionState> states = new ArrayList<>(relations.length);
@@ -676,6 +689,15 @@ public class StandardInstructionVisitor extends InstructionVisitor {
return states.toArray(DfaInstructionState.EMPTY_ARRAY);
}
private static boolean isStringComparison(PsiExpression expression) {
if (expression instanceof PsiBinaryExpression) {
PsiExpression left = ((PsiBinaryExpression)expression).getLOperand();
PsiExpression right = ((PsiBinaryExpression)expression).getROperand();
return right != null && (TypeUtils.isJavaLangString(left.getType()) || TypeUtils.isJavaLangString(right.getType()));
}
return false;
}
@NotNull
private static RelationType[] splitRelation(RelationType relationType) {
switch (relationType) {
@@ -50,7 +50,7 @@ class ContractReturnValues {
}
void testNullToEmpty(String s, String s1) {
if(nullToEmpty(s) != s) {
if(!nullToEmpty(s).equals(s)) {
System.out.println(<warning descr="Condition 's == null' is always 'true'">s == null</warning>);
}
if(<warning descr="Condition 'nullToEmpty(s1) == null' is always 'false'">nullToEmpty(s1) == null</warning>) {
@@ -1,5 +1,5 @@
abstract class Foo {
protected String bar;
protected Object bar;
}
class FF extends Foo {
@@ -80,7 +80,7 @@ public class OptionalInlining {
if (<warning descr="Condition 's1.equals(\"xz\")' is always 'false'">s1.equals("xz")</warning>) {
System.out.println("never");
}
String abc = opt.filter(s -> s == "xyz").orElse("abc"); // s.equals("xyz") does not work yet :(
String abc = opt.filter(s -> s.equals("xyz")).orElse("abc");
if (<warning descr="Condition 'abc.equals(\"123\")' is always 'false'">abc.equals("123")</warning>) {
System.out.println("never");
}
@@ -184,7 +184,8 @@ public class OptionalInlining {
}
void testIntermediate(Optional<String> opt) {
if (<warning descr="Condition 'opt.filter(x -> x == \"foo\").filter(x -> x == \"bar\").isPresent()' is always 'false'">opt.filter(x -> x == "foo").filter(x -> <warning descr="Condition 'x == \"bar\"' is always 'false'">x == "bar"</warning>).isPresent()</warning>) {
if (<warning descr="Condition 'opt.filter(x -> x.equals(\"foo\")) .filter(\"bar\"::equals).isPresent()' is always 'false'">opt.filter(x -> x.equals("foo"))
.filter(<warning descr="Method reference result is always 'false'">"bar"::equals</warning>).isPresent()</warning>) {
System.out.println("never");
}
}
@@ -0,0 +1,43 @@
class StringEquality {
void ifChain(String s) {
if (s.equals("foo")) {
}
else if (s.equals("bar")) {
}else if(<warning descr="Condition '\"foo\".equals(s)' is always 'false'">"foo".equals(s)</warning>) {
}
}
void switchAfterIf(String s) {
if (s.equals("foo")) {
return;
}
switch(s) {
case "bar":
case "baz":
<warning descr="Switch label 'case \"foo\":' is unreachable">case "foo":</warning>
}
}
void lengths(String s, String s1) {
if(<warning descr="Condition 's.equals(s1) && s.length() != s1.length()' is always 'false'">s.equals(s1) && <warning descr="Condition 's.length() != s1.length()' is always 'false' when reached">s.length() != s1.length()</warning></warning>) {}
if(<warning descr="Condition 's.length() != s1.length() && s.equals(s1)' is always 'false'">s.length() != s1.length() && <warning descr="Condition 's.equals(s1)' is always 'false' when reached">s.equals(s1)</warning></warning>) {}
}
// IDEA-197195
void foo() {
String v = "Foo";
String vv = "FooFoo";
String vvv = vv.substring(3);
System.out.println(<warning descr="Result of 'v.equals(vv)' is always 'false'">v.equals(vv)</warning>);
System.out.println(<warning descr="Condition 'v == vv' is always 'false'">v == vv</warning>);
System.out.println(<warning descr="Result of 'v.equals(vvv)' is always 'true'">v.equals(vvv)</warning>);
System.out.println(v == vvv); // Unsure: strings are equal by content, but DFA does not know whether they are equal by reference
System.out.println(<warning descr="Result of 'vv.equals(vvv)' is always 'false'">vv.equals(vvv)</warning>);
System.out.println(<warning descr="Condition 'vv == vvv' is always 'false'">vv == vvv</warning>);
}
}
@@ -637,4 +637,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase {
public void testNullFlushed() { doTest(); }
public void testBooleanMergeInLoop() { doTest(); }
public void testVoidIsAlwaysNull() { doTest(); }
public void testStringEquality() { doTest(); }
}