diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java index 1a676cde8112..e83126b62b61 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/TrackingRunner.java @@ -91,6 +91,10 @@ public class TrackingRunner extends StandardDataFlowRunner { public abstract static class DfaProblemType { public abstract String toString(); + + CauseItem[] findCauses(PsiExpression expression, MemoryStateChange history) { + return new CauseItem[0]; + } } public static class CauseItem { @@ -243,11 +247,38 @@ public class TrackingRunner extends StandardDataFlowRunner { } public static class CastDfaProblemType extends DfaProblemType { + @Override + public CauseItem[] findCauses(PsiExpression expression, MemoryStateChange history) { + if (expression instanceof PsiTypeCastExpression) { + PsiType expressionType = expression.getType(); + MemoryStateChange operandPush = history.findExpressionPush(((PsiTypeCastExpression)expression).getOperand()); + if (operandPush != null) { + return new CauseItem[]{findTypeCause(operandPush, expressionType, false)}; + } + } + return new CauseItem[0]; + } + public String toString() { return "cast may fail"; } } + public static class NullableDfaProblemType extends DfaProblemType { + @Override + public CauseItem[] findCauses(PsiExpression expression, MemoryStateChange history) { + Pair nullability = history.findFact(history.myTopOfStack, DfaFactType.NULLABILITY); + if (nullability.second == DfaNullability.NULLABLE || nullability.second == DfaNullability.NULL) { + return new CauseItem[]{findNullabilityCause(history, nullability.first, nullability.second)}; + } + return new CauseItem[0]; + } + + public String toString() { + return "may be null"; + } + } + static class PossibleExecutionDfaProblemType extends DfaProblemType { boolean myComplete = true; @@ -265,6 +296,11 @@ public class TrackingRunner extends StandardDataFlowRunner { myValue = value; } + @Override + public CauseItem[] findCauses(PsiExpression expression, MemoryStateChange history) { + return findConstantValueCause(expression, history, myValue); + } + @Override public String toString() { return "value is always " + myValue; @@ -286,7 +322,6 @@ public class TrackingRunner extends StandardDataFlowRunner { /* TODO: 1. Find causes of other warnings: - Cause for possible NPE Cause for AIOOBE Cause for "Contract always fails" Cause for "modifying an immutable collection" @@ -297,6 +332,7 @@ public class TrackingRunner extends StandardDataFlowRunner { Warning caused by polyadic math Warning caused by narrowing conversion Warning caused by unary minus + Warning caused by final field initializer TODO: 3. Check how it works with Inliners (notably: Stream API) Ternary operators @@ -307,18 +343,7 @@ public class TrackingRunner extends StandardDataFlowRunner { private static CauseItem findCauseChain(PsiExpression expression, MemoryStateChange history, DfaProblemType type) { CauseItem root = new CauseItem(type, expression); if (history.getExpression() != expression) return root; - if (type instanceof ValueDfaProblemType) { - Object expectedValue = ((ValueDfaProblemType)type).myValue; - CauseItem[] causes = findConstantValueCause(expression, history, expectedValue); - root.addChildren(causes); - } - if (type instanceof CastDfaProblemType && expression instanceof PsiTypeCastExpression) { - PsiType expressionType = expression.getType(); - MemoryStateChange operandPush = history.findExpressionPush(((PsiTypeCastExpression)expression).getOperand()); - if (operandPush != null) { - root.addChildren(findTypeCause(operandPush, expressionType, false)); - } - } + root.addChildren(type.findCauses(expression, history)); return root; } @@ -590,9 +615,25 @@ public class TrackingRunner extends StandardDataFlowRunner { private static CauseItem findNullabilityCause(MemoryStateChange factUse, MemoryStateChange factDef, DfaNullability nullability) { PsiExpression expression = factUse.getExpression(); if (factDef != null && expression != null) { + DfaValue value = factUse.myTopOfStack; + if (factDef.myInstruction instanceof AssignInstruction && factDef.myTopOfStack == value) { + PsiExpression rExpression = PsiUtil.skipParenthesizedExprDown(((AssignInstruction)factDef.myInstruction).getRExpression()); + while (rExpression instanceof PsiTypeCastExpression) { + rExpression = PsiUtil.skipParenthesizedExprDown(((PsiTypeCastExpression)rExpression).getOperand()); + } + if (rExpression != null) { + MemoryStateChange rValuePush = factDef.findExpressionPush(rExpression); + if (rValuePush != null) { + CauseItem assignmentItem = new CauseItem("'" + value + "' was assigned", rExpression); + Pair rValueFact = rValuePush.findFact(rValuePush.myTopOfStack, DfaFactType.NULLABILITY); + assignmentItem.addChildren(findNullabilityCause(rValuePush, rValueFact.first, nullability)); + return assignmentItem; + } + } + } PsiExpression defExpression = factDef.getExpression(); if (defExpression != null) { - return new CauseItem(expression.getText() + " is known to be '" + nullability.getPresentationName() + "' from #ref", defExpression); + return new CauseItem("'" + expression.getText() + "' is known to be '" + nullability.getPresentationName() + "' from #ref", defExpression); } } if (expression instanceof PsiMethodCallExpression) { @@ -660,8 +701,13 @@ public class TrackingRunner extends StandardDataFlowRunner { else { message = memberName + " '" + name + "' is annotated as '" + nullability.getPresentationName() + "'"; } - if (owner.getContainingFile() == anchor.getContainingFile()) { + if (info.getAnnotation().getContainingFile() == anchor.getContainingFile()) { + anchor = info.getAnnotation(); + } else if (owner.getContainingFile() == anchor.getContainingFile()) { anchor = owner.getNavigationElement(); + if (anchor instanceof PsiNameIdentifierOwner) { + anchor = ((PsiNameIdentifierOwner)anchor).getNameIdentifier(); + } } return new CauseItem(message, anchor); } diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java index 72222a7f4edf..e73d2200e4bb 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspection.java @@ -166,6 +166,10 @@ public class DataFlowInspection extends DataFlowInspectionBase { if (!ExpressionUtils.isNullLiteral(qualifier) && PsiUtil.isLanguageLevel7OrHigher(qualifier)) { fixes.add(new SurroundWithRequireNonNullFix(qualifier)); } + + if (!ExpressionUtils.isNullLiteral(qualifier)) { + ContainerUtil.addIfNotNull(fixes, createExplainFix(qualifier, new TrackingRunner.NullableDfaProblemType())); + } ContainerUtil.addIfNotNull(fixes, DfaOptionalSupport.registerReplaceOptionalOfWithOfNullableFix(qualifier)); } diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/NotNullParameter.java b/java/java-tests/testData/inspection/dataFlow/tracker/NotNullParameter.java index a155f9113db7..a90c78783e89 100644 --- a/java/java-tests/testData/inspection/dataFlow/tracker/NotNullParameter.java +++ b/java/java-tests/testData/inspection/dataFlow/tracker/NotNullParameter.java @@ -1,6 +1,6 @@ /* Value is always false (null == s) - Parameter 's' is annotated as 'non-null' (@NotNull String s) + Parameter 's' is annotated as 'non-null' (@NotNull) */ import org.jetbrains.annotations.NotNull; diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/NpeAnnotation.java b/java/java-tests/testData/inspection/dataFlow/tracker/NpeAnnotation.java new file mode 100644 index 000000000000..959ae680bb56 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/NpeAnnotation.java @@ -0,0 +1,16 @@ +/* +May be null (s) + 's' was assigned (loadString()) + Method 'loadString' is annotated as 'nullable' (@Nullable) + */ + +import org.jetbrains.annotations.Nullable; + +class Test { + void test() { + String s = loadString(); + System.out.println(s.trim()); + } + + native @Nullable String loadString(); +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/NpeSimple.java b/java/java-tests/testData/inspection/dataFlow/tracker/NpeSimple.java new file mode 100644 index 000000000000..e5ac9d9706f9 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/NpeSimple.java @@ -0,0 +1,14 @@ +/* +May be null (s) + An execution might exist where: + 's' is known to be 'null' from line #9 (s == null) + */ + +class Test { + void test(String s) { + if (s == null) { + System.out.println(s); + } + System.out.println(s.trim()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/tracker/NpeWithCast.java b/java/java-tests/testData/inspection/dataFlow/tracker/NpeWithCast.java new file mode 100644 index 000000000000..0b29d755e77b --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/tracker/NpeWithCast.java @@ -0,0 +1,17 @@ +/* +May be null (foo) + 'foo' was assigned (getFoo()) + Method 'getFoo' is annotated as 'nullable' (@Nullable) + */ + +import org.jetbrains.annotations.Nullable; + +class Test { + + void test(Object x) { + String foo = (String)getFoo(); + System.out.println(foo.trim()); + } + + @Nullable native Object getFoo(); +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java index 7354cee13933..e6a5170a1421 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTrackerTest.java @@ -94,6 +94,11 @@ public class DataFlowInspectionTrackerTest extends LightCodeInsightFixtureTestCa if (expression instanceof PsiTypeCastExpression) { return new TrackingRunner.CastDfaProblemType(); } + PsiElement parent = expression.getParent(); + if (parent instanceof PsiReferenceExpression) { + // Test possible NPE in qualifiers only + return new TrackingRunner.NullableDfaProblemType(); + } CommonDataflow.DataflowResult result = CommonDataflow.getDataflowResult(expression); assertNotNull("No common dataflow result for expression: " + selectedText, result); Set values = result.getExpressionValues(expression); @@ -141,4 +146,7 @@ public class DataFlowInspectionTrackerTest extends LightCodeInsightFixtureTestCa public void testAndChainCause() { doTest(); } public void testAndChainDependentCause() { doTest(); } public void testOrChainCause() { doTest(); } + public void testNpeSimple() { doTest(); } + public void testNpeAnnotation() { doTest(); } + public void testNpeWithCast() { doTest(); } }