dfa: check that states to be merged are really complementary (IDEA-136917)

diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StateMerger.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StateMerger.java
index 86c87a7af0463d528c90aa2fe4763640b0b0bfc0..fb866355d0526389fecc3f5d147024d71469f044 100644 (file)
--- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StateMerger.java
+++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StateMerger.java
@@ -23,6 +23,7 @@ import com.intellij.psi.JavaTokenType;
 import com.intellij.util.Function;
 import com.intellij.util.containers.ContainerUtil;
 import com.intellij.util.containers.MultiMap;
+import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 import java.util.*;
@@ -51,15 +52,18 @@ class StateMerger {
       if (statesWithNegations.isEmpty()) continue;
 
       ProgressManager.checkCanceled();
-      
-      MultiMap<Set<Fact>, DfaMemoryStateImpl> statesByUnrelatedFacts = MultiMap.createLinked();
-      for (DfaMemoryStateImpl state : ContainerUtil.concat(statesByFact.get(fact), statesWithNegations)) {
-        statesByUnrelatedFacts.putValue(getUnrelatedFacts(fact, state), state);
-      }
+
+      MultiMap<Set<Fact>, DfaMemoryStateImpl> statesByUnrelatedFacts1 = mapByUnrelatedFacts(fact, statesByFact.get(fact));
+      MultiMap<Set<Fact>, DfaMemoryStateImpl> statesByUnrelatedFacts2 = mapByUnrelatedFacts(fact, statesWithNegations);
 
       Replacements replacements = new Replacements(states);
-      for (Set<Fact> key : statesByUnrelatedFacts.keySet()) {
-        final Collection<DfaMemoryStateImpl> group = statesByUnrelatedFacts.get(key);
+      for (Set<Fact> key : statesByUnrelatedFacts1.keySet()) {
+        final Collection<DfaMemoryStateImpl> group1 = statesByUnrelatedFacts1.get(key);
+        final Collection<DfaMemoryStateImpl> group2 = statesByUnrelatedFacts2.get(key);
+        if (group1.isEmpty() || group2.isEmpty()) continue;
+
+        final Collection<DfaMemoryStateImpl> group = ContainerUtil.newArrayList(ContainerUtil.concat(group1, group2));
+        
         final Set<DfaVariableValue> unknowns = getAllUnknownVariables(group);
         replacements.stripAndMerge(group, new Function<DfaMemoryStateImpl, DfaMemoryStateImpl>() {
           @Override
@@ -79,6 +83,16 @@ class StateMerger {
     return null;
   }
 
+  @NotNull
+  private MultiMap<Set<Fact>, DfaMemoryStateImpl> mapByUnrelatedFacts(Fact fact,
+                                                                      Collection<DfaMemoryStateImpl> states1) {
+    MultiMap<Set<Fact>, DfaMemoryStateImpl> statesByUnrelatedFacts1 = MultiMap.createLinked();
+    for (DfaMemoryStateImpl state : states1) {
+      statesByUnrelatedFacts1.putValue(getUnrelatedFacts(fact, state), state);
+    }
+    return statesByUnrelatedFacts1;
+  }
+
   private LinkedHashSet<Fact> getUnrelatedFacts(final Fact fact, DfaMemoryStateImpl state) {
     return new LinkedHashSet<Fact>(ContainerUtil.filter(getFacts(state), new Condition<Fact>() {
       @Override
@@ -358,9 +372,20 @@ class StateMerger {
     boolean invalidatesFact(Fact another) {
       if (another.myType != myType) return false;
       if (myType == FactType.equality) {
-        return myVar == another.myVar || myVar == another.myArg;
+        return aboutSame(myVar, another.myVar) || aboutSame(myVar, another.myArg);
+      }
+      return aboutSame(myVar, another.myVar) && aboutSame(myArg, another.myArg);
+    }
+    
+    static boolean aboutSame(Object v1, Object v2) {
+      return normalize(v1) == normalize(v2);
+    }
+
+    static Object normalize(Object value) {
+      if (value instanceof DfaVariableValue && ((DfaVariableValue)value).isNegated()) {
+        return ((DfaVariableValue)value).createNegated();
       }
-      return myVar == another.myVar && myArg == another.myArg;
+      return value;
     }
 
     void removeFromState(DfaMemoryStateImpl state) {

diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/CheckComplementarityWhenMerge.java b/java/java-tests/testData/inspection/dataFlow/fixture/CheckComplementarityWhenMerge.java
new file mode 100644 (file)
index 0000000..275a3a8
--- /dev/null
+++ b/java/java-tests/testData/inspection/dataFlow/fixture/CheckComplementarityWhenMerge.java
@@ -0,0 +1,22 @@
+import org.jetbrains.annotations.*;
+
+import java.util.Collection;
+
+class NullableTest {
+
+  public void foo(@Nullable String a, Collection<D> test) {
+    boolean filterA = a == null;
+    for (D i : test) {
+      if ("c".equals(i.getE()) || filterA || bar(a)) {
+        System.out.println("ok");
+      }
+    }
+  }
+
+  private native boolean bar(@NotNull String c);
+
+  private interface D {
+    String getE();
+  }
+
+}
\ No newline at end of file

diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java
index acc64ef5f9a0aeb0c47b21f2d2b5f44479ef223c..84d9a3df52330366b1a1e980760a735c8abaad4d 100644 (file)
--- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java
+++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java
@@ -255,6 +255,7 @@ public class DataFlowInspectionTest extends LightCodeInsightFixtureTestCase {
 
   public void testVariablesDiverge() { doTest(); }
   public void testMergeByNullability() { doTest(); }
+  public void testCheckComplementarityWhenMerge() { doTest(); }
   public void testDontForgetInstanceofInfoWhenMerging() { doTest(); }
   public void testDontForgetEqInfoWhenMergingByType() { doTest(); }
   public void testDontMakeNullableAfterInstanceof() { doTest(); }