From 473d383cbe33b4d4e0420a54745955cb65e189f8 Mon Sep 17 00:00:00 2001 From: peter Date: Sat, 3 Jun 2017 18:05:29 +0200 Subject: [PATCH] dfa: prefer unknown nullity from super over scoped nullity default (IDEA-167062) --- .../codeInspection/dataFlow/NullnessUtil.java | 10 +--------- .../dataFlow/StandardInstructionVisitor.java | 5 +++-- .../dataFlow/value/DfaValueFactory.java | 19 +++++++++++++++++-- .../intellij/codeInsight/AnnotationUtil.java | 2 +- ...ullabilityDefaultVsMethodImplementing.java | 10 ++++++++++ .../DataFlowInspectionTest.java | 9 +++++++++ 6 files changed, 41 insertions(+), 14 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/NullabilityDefaultVsMethodImplementing.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullnessUtil.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullnessUtil.java index e918d50bb218..1359dd4d8bb1 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullnessUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullnessUtil.java @@ -19,7 +19,6 @@ import com.intellij.codeInsight.daemon.ImplicitUsageProvider; import com.intellij.codeInsight.daemon.impl.analysis.JavaGenericsUtil; import com.intellij.codeInspection.dataFlow.value.DfaVariableValue; import com.intellij.openapi.extensions.Extensions; -import com.intellij.patterns.ElementPattern; import com.intellij.psi.*; import com.intellij.psi.search.GlobalSearchScope; import com.intellij.psi.search.PsiSearchHelper; @@ -35,13 +34,7 @@ import org.jetbrains.annotations.Nullable; import java.util.List; -import static com.intellij.patterns.PsiJavaPatterns.psiMember; -import static com.intellij.patterns.PsiJavaPatterns.psiParameter; -import static com.intellij.patterns.StandardPatterns.or; - public class NullnessUtil { - private static final ElementPattern MEMBER_OR_METHOD_PARAMETER = - or(psiMember(), psiParameter().withSuperParent(2, psiMember())); static Boolean calcCanBeNull(DfaVariableValue value) { PsiModifierListOwner var = value.getPsiVariable(); @@ -50,8 +43,7 @@ public class NullnessUtil { return toBoolean(nullability); } - Nullness defaultNullability = - value.getFactory().isUnknownMembersAreNullable() && MEMBER_OR_METHOD_PARAMETER.accepts(var) ? Nullness.NULLABLE : Nullness.UNKNOWN; + Nullness defaultNullability = value.getFactory().suggestNullabilityForNonAnnotatedMember(var); if (var instanceof PsiParameter && var.getParent() instanceof PsiForeachStatement) { PsiExpression iteratedValue = ((PsiForeachStatement)var.getParent()).getIteratedValue(); diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index f9e5ec7972fc..b8d0bbdf8804 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -579,8 +579,9 @@ public class StandardInstructionVisitor extends InstructionVisitor { if (type != null && !(type instanceof PsiPrimitiveType)) { Nullness nullability = myReturnTypeNullability.get(instruction); - if (nullability == Nullness.UNKNOWN && factory.isUnknownMembersAreNullable()) { - nullability = Nullness.NULLABLE; + PsiMethod targetMethod = instruction.getTargetMethod(); + if (nullability == Nullness.UNKNOWN && targetMethod != null) { + nullability = factory.suggestNullabilityForNonAnnotatedMember(targetMethod); } return factory.createTypeValue(type, nullability); } 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 e7312f9d8356..216cf1152c57 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 @@ -16,10 +16,12 @@ package com.intellij.codeInspection.dataFlow.value; +import com.intellij.codeInsight.AnnotationUtil; import com.intellij.codeInspection.dataFlow.*; import com.intellij.codeInspection.dataFlow.rangeSet.LongRangeSet; import com.intellij.codeInspection.dataFlow.value.DfaRelationValue.RelationType; import com.intellij.openapi.util.Pair; +import com.intellij.patterns.ElementPattern; import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.util.containers.ContainerUtil; @@ -31,6 +33,10 @@ import org.jetbrains.annotations.Nullable; import java.util.List; import java.util.Map; +import static com.intellij.patterns.PsiJavaPatterns.psiMember; +import static com.intellij.patterns.PsiJavaPatterns.psiParameter; +import static com.intellij.patterns.StandardPatterns.or; + public class DfaValueFactory { private final List myValues = ContainerUtil.newArrayList(); private final Map, Boolean> myAssignableCache = ContainerUtil.newHashMap(); @@ -57,8 +63,17 @@ public class DfaValueFactory { return myHonorFieldInitializers; } - public boolean isUnknownMembersAreNullable() { - return myUnknownMembersAreNullable; + private static final ElementPattern MEMBER_OR_METHOD_PARAMETER = + or(psiMember(), psiParameter().withSuperParent(2, psiMember())); + + + @NotNull + public Nullness suggestNullabilityForNonAnnotatedMember(@NotNull PsiModifierListOwner member) { + if (myUnknownMembersAreNullable && MEMBER_OR_METHOD_PARAMETER.accepts(member) && AnnotationUtil.getSuperAnnotationOwners(member).isEmpty()) { + return Nullness.NULLABLE; + } + + return Nullness.UNKNOWN; } @NotNull diff --git a/java/java-psi-api/src/com/intellij/codeInsight/AnnotationUtil.java b/java/java-psi-api/src/com/intellij/codeInsight/AnnotationUtil.java index f56a7628fa06..d84ff358f983 100644 --- a/java/java-psi-api/src/com/intellij/codeInsight/AnnotationUtil.java +++ b/java/java-psi-api/src/com/intellij/codeInsight/AnnotationUtil.java @@ -156,7 +156,7 @@ public class AnnotationUtil { return result == null ? PsiAnnotation.EMPTY_ARRAY : result.toArray(new PsiAnnotation[result.size()]); } - public static List getSuperAnnotationOwners(final T element) { + public static List getSuperAnnotationOwners(@NotNull T element) { return CachedValuesManager.getCachedValue(element, () -> { Set result = ContainerUtil.newLinkedHashSet(); if (element instanceof PsiMethod) { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/NullabilityDefaultVsMethodImplementing.java b/java/java-tests/testData/inspection/dataFlow/fixture/NullabilityDefaultVsMethodImplementing.java new file mode 100644 index 000000000000..36d4591e6eb3 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/NullabilityDefaultVsMethodImplementing.java @@ -0,0 +1,10 @@ +interface UnknownInterface { + void foo(String s); +} + +@javax.annotation.ParametersAreNonnullByDefault +class ImplWithNotNull implements UnknownInterface { + public void foo(String s) { + System.out.println(s.hashCode()); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java index 0fe879ee77f4..ee1d16172c03 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java @@ -352,6 +352,15 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { doTest(); } + public void testNullabilityDefaultVsMethodImplementing() { + addJavaxDefaultNullabilityAnnotations(myFixture); + + DataFlowInspection inspection = new DataFlowInspection(); + inspection.TREAT_UNKNOWN_MEMBERS_AS_NULLABLE = true; + myFixture.enableInspections(inspection); + myFixture.testHighlighting(true, false, true, getTestName(false) + ".java"); + } + public static void addJavaxDefaultNullabilityAnnotations(final JavaCodeInsightTestFixture fixture) { fixture.addClass("package javax.annotation;" + "@javax.annotation.meta.TypeQualifierDefault(java.lang.annotation.ElementType.PARAMETER) @javax.annotation.Nonnull " +