From ff8a4f1e494b2beac59d9608e6269c5ffdfaf969 Mon Sep 17 00:00:00 2001 From: peter Date: Wed, 12 Sep 2012 14:00:20 +0200 Subject: [PATCH] don't intermix final fields with different qualifiers in dfa (IDEA-91417) --- .../dataFlow/DfaMemoryStateImpl.java | 32 +++++++++---------- .../codeInspection/dataFlow/DfaUtil.java | 4 +-- .../dataFlow/DfaVariableState.java | 7 ---- .../dataFlow/StandardInstructionVisitor.java | 6 ++-- .../dataFlow/value/DfaBoxedValue.java | 30 ++++++----------- .../dataFlow/value/DfaVariableValue.java | 6 ++++ .../FinalFieldsDifferentInstances.java | 21 ++++++++++++ .../DataFlowInspectionFixtureTest.java | 1 + 8 files changed, 59 insertions(+), 48 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/FinalFieldsDifferentInstances.java diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java index 2ba233ea60ab..09f4545d96d8 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaMemoryStateImpl.java @@ -73,6 +73,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState { public DfaMemoryStateImpl createCopy() { DfaMemoryStateImpl newState = createNew(); + //noinspection unchecked newState.myStack = (Stack)myStack.clone(); newState.myDistinctClasses = new TLongHashSet(myDistinctClasses.toArray()); newState.myEqClasses = new ArrayList(); @@ -359,7 +360,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState { return cacheable((DfaConstValue)valueToWrap); } if (valueToWrap instanceof DfaVariableValue) { - if (PsiType.BOOLEAN.equals(((DfaVariableValue)valueToWrap).getPsiVariable().getType())) return true; + if (PsiType.BOOLEAN.equals(((DfaVariableValue)valueToWrap).getVariableType())) return true; for (DfaValue value : getEqClassesFor(valueToWrap)) { if (value instanceof DfaConstValue && cacheable((DfaConstValue)value)) return true; } @@ -405,8 +406,8 @@ public class DfaMemoryStateImpl implements DfaMemoryState { SortedIntSet c1 = myEqClasses.get(c1Index); SortedIntSet c2 = myEqClasses.get(c2Index); - Set vars = new THashSet(); - Set negatedvars = new THashSet(); + Set vars = ContainerUtil.newTroveSet(); + Set negatedVars = ContainerUtil.newTroveSet(); int[] cs = new int[c1.size() + c2.size()]; c1.set(0, cs, 0, c1.size()); c2.set(0, cs, c1.size(), c2.size()); @@ -419,13 +420,15 @@ public class DfaMemoryStateImpl implements DfaMemoryState { if (dfaValue instanceof DfaConstValue) nConst++; if (dfaValue instanceof DfaVariableValue) { DfaVariableValue variableValue = (DfaVariableValue)dfaValue; - PsiVariable variable = variableValue.getPsiVariable(); - Set set = variableValue.isNegated() ? negatedvars : vars; - set.add(variable); + if (variableValue.isNegated()) { + negatedVars.add(variableValue.createNegated()); + } else { + vars.add(variableValue); + } } if (nConst > 1) return false; } - if (ContainerUtil.intersects(vars, negatedvars)) return false; + if (ContainerUtil.intersects(vars, negatedVars)) return false; TLongArrayList c2Pairs = new TLongArrayList(); long[] distincts = myDistinctClasses.toArray(); @@ -461,7 +464,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState { } private static int low(long l) { - return (int)(l & 0xFFFFFFFF); + return (int)l; } private static int high(long l) { @@ -631,8 +634,8 @@ public class DfaMemoryStateImpl implements DfaMemoryState { } } if (dfaLeft instanceof DfaVariableValue) { - PsiVariable psiVariable = ((DfaVariableValue)dfaLeft).getPsiVariable(); - if (TypeConversionUtil.isPrimitiveWrapper(psiVariable.getType()) + PsiType type = ((DfaVariableValue)dfaLeft).getVariableType(); + if (TypeConversionUtil.isPrimitiveWrapper(type) && (!isNegated // from the fact (wrappers are not the same) does not follow (unboxed values are not equals) || dfaRight instanceof DfaConstValue || dfaRight instanceof DfaBoxedValue && ((DfaBoxedValue)dfaRight).getWrappedValue() instanceof DfaConstValue) ){ @@ -640,7 +643,7 @@ public class DfaMemoryStateImpl implements DfaMemoryState { dfaRight = myFactory.getBoxedFactory().createUnboxed(dfaRight); result &= applyRelation(dfaLeft, dfaRight, isNegated); } - else if (TypeConversionUtil.isPrimitiveAndNotNull(psiVariable.getType())){ + else if (TypeConversionUtil.isPrimitiveAndNotNull(type)){ dfaLeft = myFactory.getBoxedFactory().createBoxed(dfaLeft); dfaRight = myFactory.getBoxedFactory().createBoxed(dfaRight); if (dfaLeft != null && dfaRight != null) { @@ -697,11 +700,8 @@ public class DfaMemoryStateImpl implements DfaMemoryState { } public boolean applyNotNull(DfaValue value) { - if (value instanceof DfaVariableValue) { - PsiVariable variable = ((DfaVariableValue)value).getPsiVariable(); - if (variable != null && variable.getType() instanceof PsiPrimitiveType) { - return true; - } + if (value instanceof DfaVariableValue && ((DfaVariableValue)value).getVariableType() instanceof PsiPrimitiveType) { + return true; } return checkNotNullable(value) && applyCondition(compareToNull(value, true)); diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaUtil.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaUtil.java index a31bdafc1e04..e60714128b79 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaUtil.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaUtil.java @@ -253,12 +253,12 @@ public class DfaUtil { ValuableDataFlowRunner.ValuableDfaVariableState state = (ValuableDataFlowRunner.ValuableDfaVariableState)entry.getValue(); DfaVariableValue variableValue = entry.getKey(); final PsiExpression psiExpression = state.myExpression; - if (psiExpression != null) { + if (psiExpression != null && variableValue.getQualifier() == null) { myValues.put(variableValue.getPsiVariable(), psiExpression); } } DfaValue value = instruction.getValue(); - if (value instanceof DfaVariableValue) { + if (value instanceof DfaVariableValue && ((DfaVariableValue)value).getQualifier() == null) { if (memState.isNotNull((DfaVariableValue)value)) { myNotNulls.add(((DfaVariableValue)value).getPsiVariable()); } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaVariableState.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaVariableState.java index 71c2da6cf150..8fbf4c3bfe8a 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaVariableState.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaVariableState.java @@ -43,10 +43,8 @@ public class DfaVariableState implements Cloneable { private final Set myNotInstanceofValues; private boolean myNullable = false; private final boolean myVariableIsDeclaredNotNull; - private final PsiVariable myVar; public DfaVariableState(@Nullable PsiVariable var) { - myVar = var; myInstanceofValues = new HashSet(); myNotInstanceofValues = new HashSet(); myNullable = var != null && (NullableNotNullManager.isNullable(var) || isNullableInitialized(var, true)); @@ -82,7 +80,6 @@ public class DfaVariableState implements Cloneable { } protected DfaVariableState(final DfaVariableState toClone) { - myVar = toClone.myVar; myInstanceofValues = new THashSet(toClone.myInstanceofValues); myNotInstanceofValues = new THashSet(toClone.myNotInstanceofValues); myNullable = toClone.myNullable; @@ -178,10 +175,6 @@ public class DfaVariableState implements Cloneable { myNullable = nullable; } - public PsiVariable getVariable() { - return myVar; - } - public void setValue(DfaValue value) { } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index f73bcf4b67ce..0af557f5bcd7 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -127,7 +127,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { if (qualifier instanceof DfaVariableValue) { final DfaNotNullValue.Factory factory = runner.getFactory().getNotNullFactory(); - memState.setVarValue((DfaVariableValue)qualifier, factory.create(((DfaVariableValue)qualifier).getPsiVariable().getType())); + memState.setVarValue((DfaVariableValue)qualifier, factory.create(((DfaVariableValue)qualifier).getVariableType())); } } @@ -168,7 +168,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { if (args.length <= parametersNotNull.length && revIdx < parametersNotNull.length && parametersNotNull[revIdx] && !memState.applyNotNull(arg)) { onPassingNullParameter(runner, args[revIdx]); if (arg instanceof DfaVariableValue) { - memState.setVarValue((DfaVariableValue)arg, factory.create(((DfaVariableValue)arg).getPsiVariable().getType())); + memState.setVarValue((DfaVariableValue)arg, factory.create(((DfaVariableValue)arg).getVariableType())); } } } @@ -183,7 +183,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { onInstructionProducesNPE(instruction, runner); } if (qualifier instanceof DfaVariableValue) { - memState.setVarValue((DfaVariableValue)qualifier, factory.create(((DfaVariableValue)qualifier).getPsiVariable().getType())); + memState.setVarValue((DfaVariableValue)qualifier, factory.create(((DfaVariableValue)qualifier).getVariableType())); } } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaBoxedValue.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaBoxedValue.java index 8925f8d34c77..c401bf255c90 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaBoxedValue.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaBoxedValue.java @@ -15,9 +15,8 @@ */ package com.intellij.codeInspection.dataFlow.value; -import com.intellij.psi.PsiVariable; +import com.intellij.util.containers.ContainerUtil; import com.intellij.util.containers.HashMap; -import gnu.trove.THashMap; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -43,7 +42,6 @@ public class DfaBoxedValue extends DfaValue { public static class Factory { private final Map cachedValues = new HashMap(); - private final Map cachedNegatedValues = new HashMap(); private final DfaValueFactory myFactory; public Factory(DfaValueFactory factory) { @@ -55,19 +53,16 @@ public class DfaBoxedValue extends DfaValue { if (valueToWrap instanceof DfaUnboxedValue) return ((DfaUnboxedValue)valueToWrap).getVariable(); Object o = valueToWrap instanceof DfaConstValue ? ((DfaConstValue)valueToWrap).getValue() - : valueToWrap instanceof DfaVariableValue ? ((DfaVariableValue)valueToWrap).getPsiVariable() : null; + : valueToWrap instanceof DfaVariableValue ? valueToWrap : null; if (o == null) return null; - Map map = valueToWrap instanceof DfaVariableValue && ((DfaVariableValue)valueToWrap).isNegated() ? cachedNegatedValues : cachedValues; - DfaBoxedValue boxedValue = map.get(o); + DfaBoxedValue boxedValue = cachedValues.get(o); if (boxedValue == null) { - boxedValue = new DfaBoxedValue(valueToWrap, myFactory); - map.put(o, boxedValue); + cachedValues.put(o, boxedValue = new DfaBoxedValue(valueToWrap, myFactory)); } return boxedValue; } - private final Map cachedUnboxedValues = new THashMap(); - private final Map cachedNegatedUnboxedValues = new THashMap(); + private final Map cachedUnboxedValues = ContainerUtil.newTroveMap(); @NotNull public DfaValue createUnboxed(DfaValue value) { @@ -78,20 +73,15 @@ public class DfaBoxedValue extends DfaValue { if (value == value.myFactory.getConstFactory().getNull()) return DfaUnknownValue.getInstance(); return value; } - DfaValue result; if (value instanceof DfaVariableValue) { - PsiVariable var = ((DfaVariableValue)value).getPsiVariable(); - Map map = ((DfaVariableValue)value).isNegated() ? cachedNegatedUnboxedValues : cachedUnboxedValues; - result = map.get(var); + DfaVariableValue var = (DfaVariableValue)value; + DfaUnboxedValue result = cachedUnboxedValues.get(var); if (result == null) { - result = new DfaUnboxedValue((DfaVariableValue)value, myFactory); - map.put(var, (DfaUnboxedValue)result); + cachedUnboxedValues.put(var, result = new DfaUnboxedValue(var, myFactory)); } + return result; } - else { - result = DfaUnknownValue.getInstance(); - } - return result; + return DfaUnknownValue.getInstance(); } } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java index bddc40d1c32a..5a6e39adfaa8 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java @@ -24,6 +24,7 @@ */ package com.intellij.codeInspection.dataFlow.value; +import com.intellij.psi.PsiType; import com.intellij.psi.PsiVariable; import com.intellij.util.containers.HashMap; import com.intellij.util.containers.MultiMap; @@ -110,6 +111,11 @@ public class DfaVariableValue extends DfaValue { return myVariable; } + @Nullable + public PsiType getVariableType() { + return myVariable == null ? null : myVariable.getType(); + } + public boolean isNegated() { return myIsNegated; } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/FinalFieldsDifferentInstances.java b/java/java-tests/testData/inspection/dataFlow/fixture/FinalFieldsDifferentInstances.java new file mode 100644 index 000000000000..ef403e764bf9 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/FinalFieldsDifferentInstances.java @@ -0,0 +1,21 @@ +public class BrokenAlignment { + + private static boolean dominates(final WatchRequestImpl request, final WatchRequestImpl other) { + if (request.myToWatchRecursively) { + return other.myRootPath.startsWith(request.myRootPath); + } + + return !other.myToWatchRecursively && request.myRootPath.equals(other.myRootPath); + } + + private static class WatchRequestImpl { + private final boolean myToWatchRecursively; + private final String myRootPath = ""; + + private WatchRequestImpl(boolean toWatchRecursively) { + myToWatchRecursively = toWatchRecursively; + } + + } + +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java index a5f80d7392b2..1b1cf077eb94 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java @@ -80,6 +80,7 @@ public class DataFlowInspectionFixtureTest extends JavaCodeInsightFixtureTestCas public void testNotGreaterIsNotEquals() throws Throwable { doTest(); } public void testChainedFinalFieldsDfa() throws Throwable { doTest(); } + public void testFinalFieldsDifferentInstances() throws Throwable { doTest(); } public void testChainedFinalFieldAccessorsDfa() throws Throwable { doTest(); } }