diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowRunner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowRunner.java index 38c242bc8e2a..e74e40373c31 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowRunner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowRunner.java @@ -161,10 +161,11 @@ public class DataFlowRunner { if (instruction instanceof BranchingInstruction) { BranchingInstruction branching = (BranchingInstruction)instruction; - if (processedStates.get(branching).contains(instructionState.getMemoryState())) { + Collection processed = processedStates.get(branching); + if (processed.contains(instructionState.getMemoryState())) { continue; } - if (processedStates.get(branching).size() > MAX_STATES_PER_BRANCH) { + if (processed.size() > MAX_STATES_PER_BRANCH) { LOG.debug("Too complex because too many different possible states"); return RunnerResult.TOO_COMPLEX; // Too complex :( } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaInstructionState.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaInstructionState.java index 71cb1f6ccdf6..2af73f66c74c 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaInstructionState.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaInstructionState.java @@ -102,6 +102,9 @@ class StateQueue { if (nextStates == null) { nextStates = merger.mergeByType(); } + if (nextStates == null) { + nextStates = merger.mergeByUnknowns(); + } if (nextStates == null) break; memoryStates = nextStates; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java index 6ff44b4cfa6a..bc2b3ade12a9 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java @@ -136,11 +136,18 @@ public class DfaMemoryStateImpl implements DfaMemoryState { } public int hashCode() { - return (((getNonTrivialEqClasses().hashCode() * 31 + - getDistinctClassPairs().hashCode()) * 31 + - myStack.hashCode()) * 31 + - myUnknownVariables.hashCode()) * 31 + - myVariableStates.hashCode(); + return getPartialHashCode(true); + } + + int getPartialHashCode(boolean unknowns) { + int hash = ((getNonTrivialEqClasses().hashCode() * 31 + + getDistinctClassPairs().hashCode()) * 31 + + myStack.hashCode()) * 31 + + myVariableStates.hashCode(); + if (unknowns) { + hash = hash * 31 + myUnknownVariables.hashCode(); + } + return hash; } @SuppressWarnings({"HardCodedStringLiteral"}) 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 756d907cda0e..709b0fb95903 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StateMerger.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StateMerger.java @@ -38,7 +38,6 @@ import java.util.Set; class StateMerger { private final List myStates; private final MultiMap,DfaMemoryStateImpl> myStatesByEq = new MultiMap, DfaMemoryStateImpl>(); - private final MultiMap,DfaMemoryStateImpl> myStatesByInstanceof = new MultiMap, DfaMemoryStateImpl>(); private final Map> myVarValues = ContainerUtil.newIdentityHashMap(); public StateMerger(List states) { @@ -54,12 +53,6 @@ class StateMerger { } } myVarValues.put(state, varValues); - - for (DfaVariableValue value : state.getChangedVariable()) { - for (DfaPsiType instanceofValue : state.getVariableState(value).myInstanceofValues) { - myStatesByInstanceof.putValue(Pair.create(value, instanceofValue), state); - } - } } } @@ -82,7 +75,8 @@ class StateMerger { DfaMemoryStateImpl copy = copyWithoutVar(state, var).createCopy(); complementaryStates.add(state); - postProcessMergedState(var, copy, complementaryStates); + mergeNullableState(var, copy, complementaryStates); + mergeUnknowns(copy, complementaryStates); return getMergeResult(copy, complementaryStates); } @@ -90,13 +84,17 @@ class StateMerger { return null; } - private static void postProcessMergedState(DfaVariableValue var, - DfaMemoryStateImpl mergedState, - Collection complementaryStates) { + private static void mergeUnknowns(DfaMemoryStateImpl mergedState, Collection complementaryStates) { for (DfaMemoryStateImpl removedState : complementaryStates) { for (DfaVariableValue unknownVar : removedState.getUnknownVariables()) { mergedState.doFlush(unknownVar, true); } + } + } + private static void mergeNullableState(DfaVariableValue var, + DfaMemoryStateImpl mergedState, + Collection complementaryStates) { + for (DfaMemoryStateImpl removedState : complementaryStates) { if (removedState.getVariableState(var).isNullable()) { mergedState.setVariableState(var, mergedState.getVariableState(var).withNullability(Nullness.NULLABLE)); } @@ -115,8 +113,50 @@ class StateMerger { return result; } + @Nullable + public List mergeByUnknowns() { + MultiMap byHash = new MultiMap(); + for (DfaMemoryStateImpl state : myStates) { + ProgressManager.checkCanceled(); + byHash.putValue(state.getPartialHashCode(false), state); + } + + for (Integer key : byHash.keySet()) { + Collection similarStates = byHash.get(key); + if (similarStates.size() < 2) continue; + + for (final DfaMemoryStateImpl state1 : similarStates) { + ProgressManager.checkCanceled(); + List complementary = ContainerUtil.filter(similarStates, new Condition() { + @Override + public boolean value(DfaMemoryStateImpl state2) { + return state1.equalsSuperficially(state2) && state1.equalsByRelations(state2) && state1.equalsByVariableStates(state2); + } + }); + if (complementary.size() > 1) { + DfaMemoryStateImpl copy = state1.createCopy(); + mergeUnknowns(copy, complementary); + return getMergeResult(copy, ContainerUtil.newHashSet(complementary)); + } + } + + } + + return null; + } + @Nullable public List mergeByType() { + MultiMap,DfaMemoryStateImpl> byInstanceof = new MultiMap, DfaMemoryStateImpl>(); + for (final DfaMemoryStateImpl state : myStates) { + ProgressManager.checkCanceled(); + for (DfaVariableValue value : state.getChangedVariable()) { + for (DfaPsiType instanceofValue : state.getVariableState(value).myInstanceofValues) { + byInstanceof.putValue(Pair.create(value, instanceofValue), state); + } + } + } + for (final DfaMemoryStateImpl state : myStates) { ProgressManager.checkCanceled(); @@ -124,7 +164,7 @@ class StateMerger { for (final DfaPsiType notInstanceof : state.getVariableState(var).myNotInstanceofValues) { final DfaVariableState varStateWithoutType = getVarStateWithoutType(state, var, notInstanceof); List complementaryStates = ContainerUtil.filter( - myStatesByInstanceof.get(Pair.create(var, notInstanceof)), + byInstanceof.get(Pair.create(var, notInstanceof)), new Condition() { @Override public boolean value(DfaMemoryStateImpl another) { @@ -142,7 +182,8 @@ class StateMerger { copy.setVariableState(var, varStateWithoutType); complementaryStates.add(state); - postProcessMergedState(var, copy, complementaryStates); + mergeNullableState(var, copy, complementaryStates); + mergeUnknowns(copy, complementaryStates); return getMergeResult(copy, ContainerUtil.newHashSet(complementaryStates)); } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ManyDisjunctiveFieldAssignmentsInLoopNotComplex.java b/java/java-tests/testData/inspection/dataFlow/fixture/ManyDisjunctiveFieldAssignmentsInLoopNotComplex.java new file mode 100644 index 000000000000..ecf1d65a2ec0 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ManyDisjunctiveFieldAssignmentsInLoopNotComplex.java @@ -0,0 +1,32 @@ +import org.jetbrains.annotations.NotNull; + +class Some { + String field1; + String field2; + String field3; + String field4; + String field5; + String field6; + String field7; + String field8; + String field9; + + void a(String[] lines) { + for (String line : lines) { + if (line.startsWith("1")) field1 = someString(); + else if (line.startsWith("2")) field2 = someString(); + else if (line.startsWith("3")) field3 = someString(); + else if (line.startsWith("4")) field4 = someString(); + else if (line.startsWith("5")) field5 = someString(); + else if (line.startsWith("6")) field6 = someString(); + else if (line.startsWith("7")) field7 = someString(); + else if (line.startsWith("8")) field8 = someString(); + else if (line.startsWith("9")) field9 = someString(); + } + } + + @NotNull String someString() { return ""; } + +} + + diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java index e62dbadfee45..220240d04046 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java @@ -303,6 +303,8 @@ public class DataFlowInspectionTest extends LightCodeInsightFixtureTestCase { public void testManySequentialInstanceofsNotComplex() { doTest(); } public void testLongDisjunctionsNotComplex() { doTest(); } public void testWhileNotComplex() { doTest(); } + public void testManyDisjunctiveFieldAssignmentsInLoopNotComplex() { doTest(); } + public void testVariablesDiverge() { doTest(); } public void _testNullCheckBeforeInstanceof() { doTest(); }