String equality: fix some corner cases; check dependent variables on equality

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java

@@ -11,6 +11,7 @@ import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiTypesUtil;
 import com.intellij.util.ThreeState;
 import com.intellij.util.containers.ContainerUtil;
+import com.siyeh.ig.psiutils.TypeUtils;
 import one.util.streamex.StreamEx;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -39,7 +40,9 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor {
   @Override
   public DfaInstructionState[] visitAssign(AssignInstruction instruction, DataFlowRunner runner, DfaMemoryState memState) {
     PsiExpression left = instruction.getLExpression();
-    if (left != null && !Boolean.FALSE.equals(mySameValueAssigned.get(left))) {
+    if (left != null && !Boolean.FALSE.equals(mySameValueAssigned.get(left)) && !TypeUtils.isJavaLangString(left.getType())) {
+      // Reporting strings is skipped because string reassignment might be intentionally used to deduplicate the heap objects
+      // (we compare strings by contents)
       if (!left.isPhysical()) {
         if (LOG.isDebugEnabled()) {
           LOG.debug("Non-physical element in assignment instruction: " + left.getParent().getText(), new Throwable());

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java

@@ -35,6 +35,7 @@ import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.containers.Stack;
 import gnu.trove.TIntObjectHashMap;
 import gnu.trove.TIntObjectProcedure;
+import one.util.streamex.EntryStream;
 import one.util.streamex.StreamEx;
 import org.jetbrains.annotations.Contract;
 import org.jetbrains.annotations.NotNull;
@@ -859,7 +860,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
     }
 
     if (isEffectivelyNaN(dfaLeft) || isEffectivelyNaN(dfaRight)) {
-      applyEquivalenceRelation(dfaRelation, dfaLeft, dfaRight);
+      applyEquivalenceRelation(relationType, dfaLeft, dfaRight);
       return relationType == RelationType.NE;
     }
     if ((canBeNaN(dfaLeft) && !isNull(dfaRight)) || (canBeNaN(dfaRight) && !isNull(dfaLeft))) {
@@ -869,11 +870,11 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
         return !dfaRelation.isNonEquality();
       }
 
-      applyEquivalenceRelation(dfaRelation, dfaLeft, dfaRight);
+      applyEquivalenceRelation(relationType, dfaLeft, dfaRight);
       return true;
     }
 
-    return applyEquivalenceRelation(dfaRelation, dfaLeft, dfaRight);
+    return applyEquivalenceRelation(relationType, dfaLeft, dfaRight);
   }
 
   private void updateVarStateOnComparison(@NotNull DfaVariableValue dfaVar, DfaValue value) {
@@ -897,9 +898,9 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
     }
   }
 
-  private boolean applyEquivalenceRelation(@NotNull DfaRelationValue dfaRelation, DfaValue dfaLeft, DfaValue dfaRight) {
-    boolean isNegated = dfaRelation.isNonEquality();
-    if (!isNegated && !dfaRelation.isEquality()) {
+  private boolean applyEquivalenceRelation(RelationType type, DfaValue dfaLeft, DfaValue dfaRight) {
+    boolean isNegated = type == RelationType.NE || type == RelationType.GT || type == RelationType.LT;
+    if (!isNegated && type != RelationType.EQ) {
       return true;
     }
 
@@ -920,11 +921,14 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
       }
     }
 
-    if (dfaRelation.getRelation() == RelationType.LT) {
+    if (type == RelationType.LT) {
       if (!applyLessThanRelation(dfaLeft, dfaRight)) return false;
-    } else if (dfaRelation.getRelation() == RelationType.GT) {
+    } else if (type == RelationType.GT) {
       if (!applyLessThanRelation(dfaRight, dfaLeft)) return false;
     } else {
+      if (!isNegated && !applyDependentFieldsEquivalence(dfaLeft, dfaRight)) {
+        return false;
+      }
       if (!applyRelation(dfaLeft, dfaRight, isNegated)) return false;
     }
     if (!checkCompareWithBooleanLiteral(dfaLeft, dfaRight, isNegated)) {
@@ -938,6 +942,33 @@ public class DfaMemoryStateImpl implements DfaMemoryState {
     return true;
   }
 
+  @NotNull
+  private EntryStream<? extends DfaValue, ? extends DfaValue> getDependentPairs(DfaValue left, DfaValue right) {
+    if (left instanceof DfaVariableValue) {
+      List<DfaVariableValue> leftVars = ((DfaVariableValue)left).getDependentVariables();
+      if (right instanceof DfaVariableValue) {
+        List<DfaVariableValue> rightVars = ((DfaVariableValue)right).getDependentVariables();
+        return StreamEx.of(leftVars).mapToEntry(leftVar -> StreamEx.of(rightVars)
+          .findFirst(rightVar -> leftVar.getSource().equals(rightVar.getSource())).orElse(null))
+          .nonNullValues()
+          .filterKeyValue((leftVar, rightVar) -> getEqClassIndex(leftVar) != -1 || getEqClassIndex(rightVar) != -1);
+      }
+      if (right instanceof DfaConstValue) {
+        return StreamEx.of(leftVars).filter(leftVar -> leftVar.getSource() instanceof SpecialField)
+          .mapToEntry(leftVar -> ((SpecialField)leftVar.getSource()).createValue(myFactory, right));
+      }
+    }
+    if (left instanceof DfaConstValue && right instanceof DfaVariableValue) {
+      return getDependentPairs(right, left);
+    }
+    return EntryStream.empty();
+  }
+
+  private boolean applyDependentFieldsEquivalence(@NotNull DfaValue left, @NotNull DfaValue right) {
+    return getDependentPairs(left, right)
+      .allMatch(pair -> applyCondition(myFactory.createCondition(pair.getKey(), RelationType.EQ, pair.getValue())));
+  }
+
   private boolean applyBoxedRelation(@NotNull DfaVariableValue dfaLeft, DfaValue dfaRight, boolean negated) {
     if (!TypeConversionUtil.isPrimitiveAndNotNull(dfaLeft.getVariableType())) return true;
 

java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java

@@ -693,7 +693,7 @@ public class StandardInstructionVisitor extends InstructionVisitor {
     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 right != null && (TypeUtils.isJavaLangString(left.getType()) && TypeUtils.isJavaLangString(right.getType()));
     }
     return false;
   }

java/java-tests/testData/inspection/dataFlow/fixture/AdvancedArrayAccess.java

@@ -128,27 +128,27 @@ class AdvancedArrayAccess {
   void testLocalRewritten() {
     String[] arr = {"foo", "bar", "baz"};
     arr[0] = "qux";
-    String result = "";
+    int result = 0;
     if(<warning descr="Condition 'arr[1].equals(\"bar\")' is always 'true'">arr[1].equals("bar")</warning>) {
-      result = "yes";
+      result = 1;
     }
     if(<warning descr="Condition 'arr[2].equals(\"bar\")' is always 'false'">arr[2].equals("bar")</warning>) {
-      result = "yes";
+      result = 1;
     }
     if(<warning descr="Condition 'arr[0].equals(\"foo\")' is always 'false'">arr[0].equals("foo")</warning>) {
-      result = "no";
+      result = 2;
     }
     System.out.println(result);
     arr = new String[] {"bar", "baz", "foo"};
     arr[2] = "qux";
     if(<warning descr="Condition 'arr[1].equals(\"bar\")' is always 'false'">arr[1].equals("bar")</warning>) {
-      result = "no";
+      result = 2;
     }
     if(<warning descr="Condition 'arr[2].equals(\"qux\")' is always 'true'">arr[2].equals("qux")</warning>) {
-      <warning descr="Variable is already assigned to this value">result</warning> = "yes";
+      <warning descr="Variable is already assigned to this value">result</warning> = 1;
     }
     if(<warning descr="Condition 'arr[0].equals(\"bar\")' is always 'true'">arr[0].equals("bar")</warning>) {
-      result = "no";
+      result = 2;
     }
     System.out.println(result);
   }

java/java-tests/testData/inspection/dataFlow/fixture/FieldEquality.java

@@ -0,0 +1,10 @@
+class Point {
+  int x, y;
+
+  void check(Point other) {
+    if(<warning descr="Condition 'x != other.x && this == other' is always 'false'">x != other.x && <warning descr="Condition 'this == other' is always 'false' when reached">this == other</warning></warning>) {
+      System.out.println("Impossible");
+    }
+  }
+
+}

java/java-tests/testData/inspection/dataFlow/fixture/StringEquality.java

@@ -53,4 +53,28 @@ class StringEquality {
     }
     return false;
   }
+
+  static final String SENTINEL = "foo";
+
+  void test(Object o) {
+    if(o == SENTINEL) {
+      System.out.println("oops");
+    } else {
+      System.out.println(((Number)o).longValue());
+    }
+  }
+
+  String internFoo(String s) {
+    if (s.equals("foo")) {
+      // "foo" is often used, intern it
+      s = "foo";
+    }
+    return s;
+  }
+
+  void length(String s) {
+    if(!s.startsWith("--") || <warning descr="Condition 's.equals(\".\")' is always 'false' when reached">s.equals(".")</warning>) {
+      System.out.println("invalid parameter");
+    }
+  }
 }

java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java

@@ -638,4 +638,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase {
   public void testBooleanMergeInLoop() { doTest(); }
   public void testVoidIsAlwaysNull() { doTest(); }
   public void testStringEquality() { doTest(); }
+  public void testFieldEquality() { doTest(); }
 }