From 97e82d7a5df165819c474a1bf3e11edef2e6c06e Mon Sep 17 00:00:00 2001 From: peter Date: Thu, 4 Jul 2013 18:25:17 +0200 Subject: [PATCH] IDEA-109222 Nullity is not inferred correctly. --- .../dataFlow/ControlFlowAnalyzer.java | 12 ++--- .../dataFlow/value/DfaValueFactory.java | 2 +- .../dataFlow/value/DfaVariableValue.java | 49 ++++++++++++------- .../fixture/HonorGetterAnnotation.java | 19 +++++++ .../DataFlowInspectionTest.java | 2 + 5 files changed, 59 insertions(+), 25 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/HonorGetterAnnotation.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java index 3802bcc4d9e7..2dd724f60100 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/ControlFlowAnalyzer.java @@ -1599,15 +1599,16 @@ class ControlFlowAnalyzer extends JavaElementVisitor { return null; } - PsiVariable var = resolveToVariable(refExpr); + PsiElement target = refExpr.resolve(); + PsiVariable var = getAccessedVariable(target); if (var == null) { return null; } - boolean isCall = expression instanceof PsiMethodCallExpression; + PsiMethod accessMethod = target instanceof PsiMethod ? (PsiMethod)target : null; PsiExpression qualifier = refExpr.getQualifierExpression(); if (qualifier == null) { - DfaVariableValue result = myFactory.getVarFactory().createVariableValue(var, refExpr.getType(), false, null, isCall); + DfaVariableValue result = myFactory.getVarFactory().createVariableValue(var, refExpr.getType(), false, null, accessMethod); if (var instanceof PsiField) { myFields.add(result); } @@ -1617,15 +1618,14 @@ class ControlFlowAnalyzer extends JavaElementVisitor { if (DfaPsiUtil.isFinalField(var) || DfaPsiUtil.isPlainMutableField(var)) { DfaVariableValue qualifierValue = createChainedVariableValue(qualifier); if (qualifierValue != null) { - return myFactory.getVarFactory().createVariableValue(var, refExpr.getType(), false, qualifierValue, isCall || qualifierValue.isViaMethods()); + return myFactory.getVarFactory().createVariableValue(var, refExpr.getType(), false, qualifierValue, accessMethod); } } return null; } @Nullable - private static PsiVariable resolveToVariable(PsiReferenceExpression refExpr) { - PsiElement target = refExpr.resolve(); + private static PsiVariable getAccessedVariable(final PsiElement target) { if (target instanceof PsiVariable) { return (PsiVariable)target; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java index dcc4a3f290d9..1ea146ec3ed6 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaValueFactory.java @@ -136,7 +136,7 @@ public class DfaValueFactory { } if (!variable.hasModifierProperty(PsiModifier.VOLATILE) && isEffectivelyUnqualified(referenceExpression)) { - return getVarFactory().createVariableValue(variable, referenceExpression.getType(), false, null, false); + return getVarFactory().createVariableValue(variable, referenceExpression.getType(), false, null, null); } return null; diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java index 39541bf6ca60..695c0e61c37b 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/value/DfaVariableValue.java @@ -49,15 +49,15 @@ public class DfaVariableValue extends DfaValue { } public DfaVariableValue createVariableValue(PsiVariable myVariable, boolean isNegated) { - return createVariableValue(myVariable, myVariable.getType(), isNegated, null, false); + return createVariableValue(myVariable, myVariable.getType(), isNegated, null, null); } @NotNull public DfaVariableValue createVariableValue(PsiVariable myVariable, - @Nullable PsiType varType, boolean isNegated, @Nullable DfaVariableValue qualifier, boolean viaMethods) { + @Nullable PsiType varType, boolean isNegated, @Nullable DfaVariableValue qualifier, @Nullable PsiMethod accessMethod) { mySharedInstance.myVariable = myVariable; mySharedInstance.myIsNegated = isNegated; mySharedInstance.myQualifier = qualifier; - mySharedInstance.myViaMethods = viaMethods; + mySharedInstance.myAccessMethod = accessMethod; String id = mySharedInstance.toString(); ArrayList conditions = myStringToObject.get(id); @@ -71,7 +71,7 @@ public class DfaVariableValue extends DfaValue { } } - DfaVariableValue result = new DfaVariableValue(myVariable, varType, isNegated, myFactory, qualifier, viaMethods); + DfaVariableValue result = new DfaVariableValue(myVariable, varType, isNegated, myFactory, qualifier, accessMethod); if (qualifier != null) { myQualifiersToChainedVariables.putValue(qualifier, result); } @@ -92,18 +92,18 @@ public class DfaVariableValue extends DfaValue { private PsiVariable myVariable; private PsiType myVarType; + private PsiMethod myAccessMethod; @Nullable private DfaVariableValue myQualifier; private boolean myIsNegated; - private boolean myViaMethods; private Nullness myInherentNullability; - private DfaVariableValue(PsiVariable variable, PsiType varType, boolean isNegated, DfaValueFactory factory, @Nullable DfaVariableValue qualifier, boolean viaMethods) { + private DfaVariableValue(PsiVariable variable, PsiType varType, boolean isNegated, DfaValueFactory factory, @Nullable DfaVariableValue qualifier, PsiMethod accessMethod) { super(factory); myVariable = variable; myIsNegated = isNegated; myQualifier = qualifier; - myViaMethods = viaMethods; myVarType = varType; + myAccessMethod = accessMethod; } private DfaVariableValue(DfaValueFactory factory) { @@ -128,7 +128,7 @@ public class DfaVariableValue extends DfaValue { @Override public DfaVariableValue createNegated() { - return myFactory.getVarFactory().createVariableValue(myVariable, myVarType, !myIsNegated, myQualifier, myViaMethods); + return myFactory.getVarFactory().createVariableValue(myVariable, myVarType, !myIsNegated, myQualifier, myAccessMethod); } @SuppressWarnings({"HardCodedStringLiteral"}) @@ -140,7 +140,7 @@ public class DfaVariableValue extends DfaValue { private boolean hardEquals(DfaVariableValue aVar) { return aVar.myVariable == myVariable && aVar.myIsNegated == myIsNegated && - aVar.myViaMethods == myViaMethods && + aVar.myAccessMethod == myAccessMethod && (myQualifier == null ? aVar.myQualifier == null : myQualifier.hardEquals(aVar.myQualifier)); } @@ -150,7 +150,7 @@ public class DfaVariableValue extends DfaValue { } public boolean isViaMethods() { - return myViaMethods; + return myAccessMethod != null || myQualifier != null && myQualifier.isViaMethods(); } public Nullness getInherentNullability() { @@ -158,19 +158,32 @@ public class DfaVariableValue extends DfaValue { return myInherentNullability; } + return myInherentNullability = calcInherentNullability(); + } + + private Nullness calcInherentNullability() { + PsiMethod accessMethod = myAccessMethod; + Nullness nullability = DfaPsiUtil.getElementNullability(getVariableType(), accessMethod); + if (nullability != Nullness.UNKNOWN) { + return nullability; + } + PsiVariable var = getPsiVariable(); - Nullness nullability = DfaPsiUtil.getElementNullability(getVariableType(), var); - if (nullability == Nullness.UNKNOWN && var != null) { + nullability = DfaPsiUtil.getElementNullability(getVariableType(), var); + if (nullability != Nullness.UNKNOWN) { + return nullability; + } + + if (var != null) { if (DfaPsiUtil.isNullableInitialized(var, true)) { - nullability = Nullness.NULLABLE; - } else if (DfaPsiUtil.isNullableInitialized(var, false)) { - nullability = Nullness.NOT_NULL; + return Nullness.NULLABLE; + } + if (DfaPsiUtil.isNullableInitialized(var, false)) { + return Nullness.NOT_NULL; } } - myInherentNullability = nullability; - - return nullability; + return Nullness.UNKNOWN; } public boolean isLocalVariable() { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/HonorGetterAnnotation.java b/java/java-tests/testData/inspection/dataFlow/fixture/HonorGetterAnnotation.java new file mode 100644 index 000000000000..fa72072aed8c --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/HonorGetterAnnotation.java @@ -0,0 +1,19 @@ +import org.jetbrains.annotations.Nullable; + +public class Goo { + Permission permission; + + { + Object category = permission.getCategory(); + System.out.println(category.hashCode()); + } +} + +class Permission { + Object category; + + + @Nullable Object getCategory() { + return category; + } +} \ 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 6141ea53df20..c15658137c0d 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionTest.java @@ -171,6 +171,8 @@ public class DataFlowInspectionTest extends LightCodeInsightFixtureTestCase { public void testEqualsHasNoSideEffects() { doTest(); } + public void testHonorGetterAnnotation() { doTest(); } + public void testIsNullCheck() throws Exception { ConditionCheckManager.getInstance(myModule.getProject()).getIsNullCheckMethods().add( buildConditionChecker("Value", "isNull", ConditionChecker.Type.IS_NULL_METHOD,