From 9c5cbeae3e30aedf5ab01903d131cd68c23c36f2 Mon Sep 17 00:00:00 2001 From: peter Date: Fri, 18 Aug 2017 11:33:06 +0200 Subject: [PATCH] check that primitive types can't be nullity-annotated (IDEA-176629) --- .../nullable/NullableStuffInspectionBase.java | 66 ++++++++++++++----- .../AnnotatingPrimitivesAmbiguous.java | 5 ++ .../AnnotatingPrimitivesTypeUse.java | 5 ++ .../DataFlowInspection8Test.java | 8 ++- .../NullableStuffInspectionTest.java | 13 +++- 5 files changed, 78 insertions(+), 19 deletions(-) create mode 100644 java/java-tests/testData/inspection/nullableProblems/AnnotatingPrimitivesAmbiguous.java create mode 100644 java/java-tests/testData/inspection/nullableProblems/AnnotatingPrimitivesTypeUse.java diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionBase.java index 49454c02bb24..80fc6f08bcd3 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspectionBase.java @@ -15,6 +15,7 @@ */ package com.intellij.codeInspection.nullable; +import com.intellij.codeInsight.AnnotationTargetUtil; import com.intellij.codeInsight.AnnotationUtil; import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInsight.daemon.GroupNames; @@ -47,6 +48,7 @@ import org.jetbrains.annotations.Nullable; import javax.swing.*; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.Set; import static com.intellij.patterns.PsiJavaPatterns.psiElement; @@ -140,6 +142,31 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo check(parameter, holder, parameter.getType()); } + @Override + public void visitTypeElement(PsiTypeElement type) { + NullableNotNullManager manager = NullableNotNullManager.getInstance(type.getProject()); + List annotations = getExclusiveAnnotations(type); + + checkType(null, holder, type.getType(), + ContainerUtil.find(annotations, a -> manager.getNotNulls().contains(a.getQualifiedName())), + ContainerUtil.find(annotations, a -> manager.getNullables().contains(a.getQualifiedName()))); + } + + private List getExclusiveAnnotations(PsiTypeElement type) { + List annotations = ContainerUtil.newArrayList(type.getAnnotations()); + PsiTypeElement topMost = Objects.requireNonNull(SyntaxTraverser.psiApi().parents(type).filter(PsiTypeElement.class).last()); + PsiElement parent = topMost.getParent(); + if (parent instanceof PsiModifierListOwner && type.getType().equals(topMost.getType().getDeepComponentType())) { + PsiModifierList modifierList = ((PsiModifierListOwner)parent).getModifierList(); + if (modifierList != null) { + PsiAnnotation.TargetType[] targets = ArrayUtil.remove(AnnotationTargetUtil.getTargetsForLocation(modifierList), PsiAnnotation.TargetType.TYPE_USE); + annotations.addAll(ContainerUtil.filter(modifierList.getAnnotations(), + a -> AnnotationTargetUtil.isTypeAnnotation(a) && AnnotationTargetUtil.findAnnotationTarget(a, targets) == null)); + } + } + return annotations; + } + @Override public void visitAnnotation(PsiAnnotation annotation) { if (!AnnotationUtil.NOT_NULL.equals(annotation.getQualifiedName())) return; @@ -508,24 +535,31 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo this.isDeclaredNullable = isDeclaredNullable; } } - private static Annotated check(final PsiModifierListOwner parameter, final ProblemsHolder holder, PsiType type) { + private static Annotated check(final PsiModifierListOwner owner, final ProblemsHolder holder, PsiType type) { final NullableNotNullManager manager = NullableNotNullManager.getInstance(holder.getProject()); - PsiAnnotation isDeclaredNotNull = AnnotationUtil.findAnnotation(parameter, manager.getNotNulls()); - PsiAnnotation isDeclaredNullable = AnnotationUtil.findAnnotation(parameter, manager.getNullables()); - if (isDeclaredNullable != null && isDeclaredNotNull != null) { - reportNullableNotNullConflict(holder, parameter, isDeclaredNullable, isDeclaredNotNull); - } - if ((isDeclaredNotNull != null || isDeclaredNullable != null) && type != null && TypeConversionUtil.isPrimitive(type.getCanonicalText())) { - PsiAnnotation annotation = isDeclaredNotNull == null ? isDeclaredNullable : isDeclaredNotNull; - reportPrimitiveType(holder, annotation, annotation, parameter); - } - if (parameter instanceof PsiParameter) { - checkLoopParameterNullability(holder, isDeclaredNotNull, isDeclaredNullable, DfaPsiUtil.inferParameterNullability((PsiParameter)parameter)); - } + PsiAnnotation isDeclaredNotNull = AnnotationUtil.findAnnotation(owner, manager.getNotNulls()); + PsiAnnotation isDeclaredNullable = AnnotationUtil.findAnnotation(owner, manager.getNullables()); + checkType(owner, holder, type, isDeclaredNotNull, isDeclaredNullable); return new Annotated(isDeclaredNotNull != null,isDeclaredNullable != null); } + private static void checkType(@Nullable PsiModifierListOwner listOwner, + ProblemsHolder holder, + PsiType type, + @Nullable PsiAnnotation notNull, @Nullable PsiAnnotation nullable) { + if (nullable != null && notNull != null) { + reportNullableNotNullConflict(holder, listOwner, nullable, notNull); + } + if ((notNull != null || nullable != null) && type != null && TypeConversionUtil.isPrimitive(type.getCanonicalText())) { + PsiAnnotation annotation = notNull == null ? nullable : notNull; + reportPrimitiveType(holder, annotation, listOwner); + } + if (listOwner instanceof PsiParameter) { + checkLoopParameterNullability(holder, notNull, nullable, DfaPsiUtil.inferParameterNullability((PsiParameter)listOwner)); + } + } + private static void checkLoopParameterNullability(ProblemsHolder holder, @Nullable PsiAnnotation notNull, @Nullable PsiAnnotation nullable, Nullness expectedNullability) { if (notNull != null && expectedNullability == Nullness.NULLABLE) { holder.registerProblem(notNull, "Parameter can be null", @@ -537,9 +571,9 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo } } - private static void reportPrimitiveType(final ProblemsHolder holder, final PsiElement psiElement, final PsiAnnotation annotation, - final PsiModifierListOwner listOwner) { - holder.registerProblem(psiElement.isPhysical() ? psiElement : listOwner.getNavigationElement(), + private static void reportPrimitiveType(ProblemsHolder holder, PsiAnnotation annotation, + @Nullable PsiModifierListOwner listOwner) { + holder.registerProblem(!annotation.isPhysical() && listOwner != null ? listOwner.getNavigationElement() : annotation, InspectionsBundle.message("inspection.nullable.problems.primitive.type.annotation"), ProblemHighlightType.GENERIC_ERROR_OR_WARNING, new RemoveAnnotationQuickFix(annotation, listOwner)); } diff --git a/java/java-tests/testData/inspection/nullableProblems/AnnotatingPrimitivesAmbiguous.java b/java/java-tests/testData/inspection/nullableProblems/AnnotatingPrimitivesAmbiguous.java new file mode 100644 index 000000000000..8b0ce23370b2 --- /dev/null +++ b/java/java-tests/testData/inspection/nullableProblems/AnnotatingPrimitivesAmbiguous.java @@ -0,0 +1,5 @@ +class Y { + public static @withTypeUse.Nullable byte @withTypeUse.Nullable [] getData3() { + return null; + } +} diff --git a/java/java-tests/testData/inspection/nullableProblems/AnnotatingPrimitivesTypeUse.java b/java/java-tests/testData/inspection/nullableProblems/AnnotatingPrimitivesTypeUse.java new file mode 100644 index 000000000000..a48ff124ca0d --- /dev/null +++ b/java/java-tests/testData/inspection/nullableProblems/AnnotatingPrimitivesTypeUse.java @@ -0,0 +1,5 @@ +class Y { + public static @typeUse.Nullable byte @typeUse.Nullable [] getData2() { + return null; + } +} 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 e0f0c280f496..72da2e80e1ee 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection8Test.java @@ -176,10 +176,14 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase { public void testIgnoreNullabilityOnPrimitiveCast() { doTestWithCustomAnnotations();} public void testArrayComponentAndMethodAnnotationConflict() { - setupCustomAnnotations("withTypeUse", "{ElementType.METHOD, ElementType.TYPE_USE}", myFixture); + setupAmbiguousAnnotations("withTypeUse", myFixture); doTest(); } + static void setupAmbiguousAnnotations(String pkg, JavaCodeInsightTestFixture fixture) { + setupCustomAnnotations(pkg, "{ElementType.METHOD, ElementType.TYPE_USE}", fixture); + } + public void testLambdaInlining() { doTest(); } public void testOptionalInlining() { @@ -190,7 +194,7 @@ public class DataFlowInspection8Test extends DataFlowInspectionTestCase { public void testStreamKnownSource() { doTest(); } public void testMethodVsExpressionTypeAnnotationConflict() { - setupCustomAnnotations("withTypeUse", "{ElementType.METHOD, ElementType.TYPE_USE}", myFixture); + setupAmbiguousAnnotations("withTypeUse", myFixture); doTest(); } diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/NullableStuffInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/NullableStuffInspectionTest.java index 646a91679ecc..cd4720760097 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/NullableStuffInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/NullableStuffInspectionTest.java @@ -65,7 +65,18 @@ public class NullableStuffInspectionTest extends LightCodeInsightFixtureTestCase super.tearDown(); } - public void testProblems() { doTest(); } + public void testProblems() { doTest();} + + public void testAnnotatingPrimitivesTypeUse() { + DataFlowInspection8Test.setupTypeUseAnnotations("typeUse", myFixture); + doTest(); + } + + public void testAnnotatingPrimitivesAmbiguous() { + DataFlowInspection8Test.setupAmbiguousAnnotations("withTypeUse", myFixture); + doTest(); + } + public void testProblems2() { doTest(); } public void testNullableFieldNotnullParam() { doTest(); } public void testNotNullFieldNullableParam() { doTest(); }