From 4713328ffee3fa0205a606eb8c2d4de6b45eafab Mon Sep 17 00:00:00 2001 From: peter Date: Wed, 28 Jun 2017 15:43:42 +0200 Subject: [PATCH] resolve member/type annotation nullable/notnull conflict (IDEA-174042) --- .../codeInsight/NullableNotNullManager.java | 30 +++++++++++++++++-- ...yComponentAndMethodAnnotationConflict.java | 25 ++++++++++++++++ .../DataFlowInspection8Test.java | 13 ++++++-- 3 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/ArrayComponentAndMethodAnnotationConflict.java diff --git a/java/java-psi-api/src/com/intellij/codeInsight/NullableNotNullManager.java b/java/java-psi-api/src/com/intellij/codeInsight/NullableNotNullManager.java index a1ff985f7a1e..2fc9b4f24b9f 100644 --- a/java/java-psi-api/src/com/intellij/codeInsight/NullableNotNullManager.java +++ b/java/java-psi-api/src/com/intellij/codeInsight/NullableNotNullManager.java @@ -245,11 +245,35 @@ public abstract class NullableNotNullManager { private PsiAnnotation findPlainNullabilityAnnotation(@NotNull PsiModifierListOwner owner, boolean checkBases) { Set qNames = ContainerUtil.newHashSet(getNullablesWithNickNames()); qNames.addAll(getNotNullsWithNickNames()); - return checkBases && owner instanceof PsiMethod - ? AnnotationUtil.findAnnotationInHierarchy(owner, qNames) - : AnnotationUtil.findAnnotation(owner, qNames); + PsiAnnotation memberAnno = checkBases && owner instanceof PsiMethod + ? AnnotationUtil.findAnnotationInHierarchy(owner, qNames) + : AnnotationUtil.findAnnotation(owner, qNames); + if (memberAnno != null) { + if (owner instanceof PsiMethod) { + return preferTypeAnnotation(memberAnno, ((PsiMethod)owner).getReturnType()); + } + if (owner instanceof PsiVariable) { + return preferTypeAnnotation(memberAnno, ((PsiVariable)owner).getType()); + } + } + return memberAnno; } + private static PsiAnnotation preferTypeAnnotation(@NotNull PsiAnnotation memberAnno, @Nullable PsiType type) { + if (type != null) { + for (PsiAnnotation typeAnno : type.getApplicableAnnotations()) { + if (areDifferentNullityAnnotations(memberAnno, typeAnno)) { + return typeAnno; + } + } + } + return memberAnno; + } + + private static boolean areDifferentNullityAnnotations(@NotNull PsiAnnotation memberAnno, PsiAnnotation typeAnno) { + return isNullableAnnotation(typeAnno) && isNotNullAnnotation(memberAnno) || + isNullableAnnotation(memberAnno) && isNotNullAnnotation(typeAnno); + } @NotNull protected List getNullablesWithNickNames() { diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ArrayComponentAndMethodAnnotationConflict.java b/java/java-tests/testData/inspection/dataFlow/fixture/ArrayComponentAndMethodAnnotationConflict.java new file mode 100644 index 000000000000..b9314775b96b --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ArrayComponentAndMethodAnnotationConflict.java @@ -0,0 +1,25 @@ +import withTypeUse.NotNull; +import withTypeUse.Nullable; + +interface Foo { + @Nullable Object @NotNull [] getNotNullArrayOfNullableObjects(); + @NotNull Object @Nullable [] getNullableArrayOfNotNullObjects(); +} + +class FooImpl implements Foo { + + @Override + public @Nullable Object @NotNull [] getNotNullArrayOfNullableObjects() { + return null; + } + + @Override + public @NotNull Object @Nullable [] getNullableArrayOfNotNullObjects() { + if (Math.random() > 0.5) { + return null; + } + else { + return new Object[]{null, new Object()}; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java index ab159899eebc..cee5d8aff3ce 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java @@ -133,8 +133,12 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase { } static void setupTypeUseAnnotations(String pkg, JavaCodeInsightTestFixture fixture) { - fixture.addClass("package " + pkg + ";\n\nimport java.lang.annotation.*;\n\n@Target({ElementType.TYPE_USE}) public @interface Nullable { }"); - fixture.addClass("package " + pkg + ";\n\nimport java.lang.annotation.*;\n\n@Target({ElementType.TYPE_USE}) public @interface NotNull { }"); + setupCustomAnnotations(pkg, "{ElementType.TYPE_USE}", fixture); + } + + private static void setupCustomAnnotations(String pkg, String target, JavaCodeInsightTestFixture fixture) { + fixture.addClass("package " + pkg + ";\n\nimport java.lang.annotation.*;\n\n@Target(" + target + ") public @interface Nullable { }"); + fixture.addClass("package " + pkg + ";\n\nimport java.lang.annotation.*;\n\n@Target(" + target + ") public @interface NotNull { }"); setCustomAnnotations(fixture.getProject(), fixture.getTestRootDisposable(), pkg + ".NotNull", pkg + ".Nullable"); } @@ -151,4 +155,9 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase { public void testCapturedWildcardNotNull() { doTest(); } public void testVarargNotNull() { doTestWithCustomAnnotations(); } + public void testArrayComponentAndMethodAnnotationConflict() { + setupCustomAnnotations("withTypeUse", "{ElementType.METHOD, ElementType.TYPE_USE}", myFixture); + doTest(); + } + } \ No newline at end of file