diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java index 7bed63984254..86ca9e7beffa 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/HardcodedContracts.java @@ -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 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()) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index b10190ed8263..c2ead5812587 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -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 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 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) { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ContractReturnValues.java b/java/java-tests/testData/inspection/dataFlow/fixture/ContractReturnValues.java index 0141029af57b..77b6ca51a223 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/ContractReturnValues.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ContractReturnValues.java @@ -50,7 +50,7 @@ class ContractReturnValues { } void testNullToEmpty(String s, String s1) { - if(nullToEmpty(s) != s) { + if(!nullToEmpty(s).equals(s)) { System.out.println(s == null); } if(nullToEmpty(s1) == null) { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/EffectivelyUnqualified.java b/java/java-tests/testData/inspection/dataFlow/fixture/EffectivelyUnqualified.java index 86ed56676e26..feabfda393a3 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/EffectivelyUnqualified.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/EffectivelyUnqualified.java @@ -1,5 +1,5 @@ abstract class Foo { - protected String bar; + protected Object bar; } class FF extends Foo { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java index ab1926e1497f..2e7ffc84fd7d 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java @@ -80,7 +80,7 @@ public class OptionalInlining { if (s1.equals("xz")) { 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 (abc.equals("123")) { System.out.println("never"); } @@ -184,7 +184,8 @@ public class OptionalInlining { } void testIntermediate(Optional opt) { - if ( x == \"bar\").isPresent()' is always 'false'">opt.filter(x -> x == "foo").filter(x -> x == "bar").isPresent()) { + if (opt.filter(x -> x.equals("foo")) + .filter("bar"::equals).isPresent()) { System.out.println("never"); } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/StringEquality.java b/java/java-tests/testData/inspection/dataFlow/fixture/StringEquality.java new file mode 100644 index 000000000000..d76e84fa0717 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/StringEquality.java @@ -0,0 +1,43 @@ +class StringEquality { + void ifChain(String s) { + if (s.equals("foo")) { + + } + else if (s.equals("bar")) { + + }else if("foo".equals(s)) { + + } + } + + void switchAfterIf(String s) { + if (s.equals("foo")) { + return; + } + switch(s) { + case "bar": + case "baz": + case "foo": + } + } + + void lengths(String s, String s1) { + if(s.equals(s1) && s.length() != s1.length()) {} + if(s.length() != s1.length() && s.equals(s1)) {} + } + + // IDEA-197195 + void foo() { + String v = "Foo"; + String vv = "FooFoo"; + String vvv = vv.substring(3); + System.out.println(v.equals(vv)); + System.out.println(v == vv); + + System.out.println(v.equals(vvv)); + 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(vv.equals(vvv)); + System.out.println(vv == vvv); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java index 94e21d8ec0a9..f5ff0dc1c482 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java @@ -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(); } }