From cce68d9b4d8361d4bbb6fe7b16763625801d01e1 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 30 Sep 2025 16:02:05 +0200 Subject: [PATCH] [java-inspections] IDEA-377761 Incorrect usages of annotations. Opposite container annotations together GitOrigin-RevId: 2a9ceb46787ef8cdff3677cd1c1c72782951c9cc --- .../messages/JavaAnalysisBundle.properties | 1 + .../nullable/NullableStuffInspectionBase.java | 23 +++++++++++++++++++ .../NullableNotNullManagerImpl.java | 12 ++++++++++ .../AnnotationPackageSupport.java | 9 ++++++++ .../JSpecifyAnnotationSupport.java | 8 +++++++ .../codeInsight/NullableNotNullManager.java | 6 +++++ .../IncompatibleContainer.java | 12 ++++++++++ .../JSpecifyConformanceAnnotationTest.java | 4 +--- .../NullableStuffInspectionTest.java | 5 ++++ 9 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 java/java-tests/testData/inspection/nullableProblems/IncompatibleContainer.java diff --git a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties index eb44c8d77404..af95396d80b6 100644 --- a/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties +++ b/java/java-analysis-api/resources/messages/JavaAnalysisBundle.properties @@ -128,6 +128,7 @@ annotate.overridden.methods.parameters=Annotate overriding method parameters as anonymous.ref.loc.can.be.replaced.with.0=Anonymous #ref #loc can be replaced with {0} anonymous.ref.loc.can.be.replaced.with.lambda=Anonymous #ref #loc can be replaced with lambda assigning.a.collection.of.nullable.elements=Assigning a collection of nullable elements into a collection of non-null elements +conflicting.nullability.annotations=Conflicting nullability annotations nullable.stuff.error.overriding.nullable.with.notnull=Overriding a collection of nullable elements with a collection of non-null elements nullable.stuff.error.overriding.notnull.with.nullable=Overriding a collection of non-null elements with a collection of nullable elements comparision.between.object.and.primitive=Comparison between Object and primitive is illegal and is accepted in Java 7 only 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 62cf5f7a5cf0..841561fe0ace 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 @@ -123,6 +123,17 @@ public class NullableStuffInspectionBase extends AbstractBaseJavaLocalInspection checkParameters(constructor, holder, List.of(), manager); } } + checkConflictingContainerAnnotations(holder, aClass.getModifierList()); + } + + @Override + public void visitPackageStatement(@NotNull PsiPackageStatement statement) { + checkConflictingContainerAnnotations(holder, statement.getAnnotationList()); + } + + @Override + public void visitModule(@NotNull PsiJavaModule module) { + checkConflictingContainerAnnotations(holder, module.getModifierList()); } @Override @@ -513,6 +524,17 @@ public class NullableStuffInspectionBase extends AbstractBaseJavaLocalInspection }; } + private void checkConflictingContainerAnnotations(@NotNull ProblemsHolder holder, @Nullable PsiModifierList list) { + if (list == null || !list.hasAnnotations()) return; + NullableNotNullManager manager = NullableNotNullManager.getInstance(holder.getProject()); + List conflictingAnnotations = manager.getConflictingAnnotations(list); + if (!conflictingAnnotations.isEmpty()) { + for (PsiAnnotation annotation : conflictingAnnotations) { + reportProblem(holder, annotation, "conflicting.nullability.annotations"); + } + } + } + private void reportProblem(@NotNull ProblemsHolder holder, PsiElement anchor, String messageKey, Object... args) { reportProblem(holder, anchor, LocalQuickFix.EMPTY_ARRAY, messageKey, args); } @@ -861,6 +883,7 @@ public class NullableStuffInspectionBase extends AbstractBaseJavaLocalInspection checkSupers(method, holder, annotated, superMethods); checkParameters(method, holder, superMethods, nullableManager); checkOverriders(method, holder, annotated, nullableManager); + checkConflictingContainerAnnotations(holder, method.getModifierList()); } private void checkSupers(PsiMethod method, diff --git a/java/java-impl/src/com/intellij/codeInsight/NullableNotNullManagerImpl.java b/java/java-impl/src/com/intellij/codeInsight/NullableNotNullManagerImpl.java index cab4d2fc2ec0..afa281caba9a 100644 --- a/java/java-impl/src/com/intellij/codeInsight/NullableNotNullManagerImpl.java +++ b/java/java-impl/src/com/intellij/codeInsight/NullableNotNullManagerImpl.java @@ -416,6 +416,18 @@ public class NullableNotNullManagerImpl extends NullableNotNullManager implement } return info; } + + @Override + public @NotNull List<@NotNull PsiAnnotation> getConflictingAnnotations(@NotNull PsiAnnotationOwner owner) { + if (!owner.hasAnnotations()) return List.of(); + for (AnnotationPackageSupport support : myAnnotationSupports) { + List<@NotNull PsiAnnotation> annotations = support.getConflictingContainerAnnotations(owner); + if (!annotations.isEmpty()) { + return annotations; + } + } + return List.of(); + } private @Unmodifiable @NotNull List filterNickNames(@NotNull Nullability nullability) { return ContainerUtil.mapNotNull(getAllNullabilityNickNames(), c -> Jsr305Support.getNickNamedNullability(c) == nullability ? c.getQualifiedName() : null); diff --git a/java/java-impl/src/com/intellij/codeInsight/annoPackages/AnnotationPackageSupport.java b/java/java-impl/src/com/intellij/codeInsight/annoPackages/AnnotationPackageSupport.java index 951c7d42a29b..9416addd645f 100644 --- a/java/java-impl/src/com/intellij/codeInsight/annoPackages/AnnotationPackageSupport.java +++ b/java/java-impl/src/com/intellij/codeInsight/annoPackages/AnnotationPackageSupport.java @@ -6,6 +6,7 @@ import com.intellij.codeInsight.Nullability; import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.openapi.extensions.ExtensionPointName; import com.intellij.psi.PsiAnnotation; +import com.intellij.psi.PsiAnnotationOwner; import com.intellij.psi.PsiElement; import org.jetbrains.annotations.NotNull; @@ -34,6 +35,14 @@ public interface AnnotationPackageSupport { return ContextNullabilityInfo.constant(null); } + /** + * @param owner annotation owner of container (method, class, or package statement) + * @return list of conflicting annotations which denote different nullability; empty list if no conflicts were found + */ + default @NotNull List<@NotNull PsiAnnotation> getConflictingContainerAnnotations(@NotNull PsiAnnotationOwner owner) { + return Collections.emptyList(); + } + /** * @param nullability desired nullability * @return list of explicit annotations which denote given nullability (and may denote additional semantics). diff --git a/java/java-impl/src/com/intellij/codeInsight/annoPackages/JSpecifyAnnotationSupport.java b/java/java-impl/src/com/intellij/codeInsight/annoPackages/JSpecifyAnnotationSupport.java index e96ecc3393e1..f702d06cdbf6 100644 --- a/java/java-impl/src/com/intellij/codeInsight/annoPackages/JSpecifyAnnotationSupport.java +++ b/java/java-impl/src/com/intellij/codeInsight/annoPackages/JSpecifyAnnotationSupport.java @@ -40,6 +40,14 @@ public final class JSpecifyAnnotationSupport implements AnnotationPackageSupport .filtering(context -> !resolvesToTypeParameter(context)); } + @Override + public @NotNull List<@NotNull PsiAnnotation> getConflictingContainerAnnotations(@NotNull PsiAnnotationOwner owner) { + PsiAnnotation marked = owner.findAnnotation(DEFAULT_NOT_NULL); + PsiAnnotation unmarked = owner.findAnnotation(DEFAULT_NULLNESS_UNKNOWN); + if (marked != null && unmarked != null) return List.of(marked, unmarked); + return List.of(); + } + static boolean resolvesToTypeParameter(@NotNull PsiElement context) { PsiType targetType = context instanceof PsiMethod method ? method.getReturnType() : context instanceof PsiVariable variable ? variable.getType() : 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 b7b75a1f60b9..19e8aa5098ec 100644 --- a/java/java-psi-api/src/com/intellij/codeInsight/NullableNotNullManager.java +++ b/java/java-psi-api/src/com/intellij/codeInsight/NullableNotNullManager.java @@ -306,6 +306,12 @@ public abstract class NullableNotNullManager { protected abstract @NotNull ContextNullabilityInfo getNullityDefault(@NotNull PsiModifierListOwner container, PsiAnnotation.TargetType @NotNull [] placeTargetTypes); + /** + * @param owner annotation owner of container (method, class, or package statement) + * @return list of conflicting annotations which denote different nullability; empty list if no conflicts were found + */ + public abstract @NotNull List<@NotNull PsiAnnotation> getConflictingAnnotations(@NotNull PsiAnnotationOwner owner); + @ApiStatus.Internal @NotNull public List getNullablesWithNickNames() { diff --git a/java/java-tests/testData/inspection/nullableProblems/IncompatibleContainer.java b/java/java-tests/testData/inspection/nullableProblems/IncompatibleContainer.java new file mode 100644 index 000000000000..a119b8a92d07 --- /dev/null +++ b/java/java-tests/testData/inspection/nullableProblems/IncompatibleContainer.java @@ -0,0 +1,12 @@ +import org.jspecify.annotations.*; + +@NullMarked +@NullUnmarked +class Test { +} + +class TestMethod { + @NullMarked + @NullUnmarked + void method() {} +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/JSpecifyConformanceAnnotationTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/JSpecifyConformanceAnnotationTest.java index 357e986a6798..b978e3b95033 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/JSpecifyConformanceAnnotationTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/JSpecifyConformanceAnnotationTest.java @@ -92,9 +92,6 @@ public class JSpecifyConformanceAnnotationTest extends LightJavaCodeInsightFixtu private static boolean suppressWarning(@NotNull String message, String fileName, Integer offset) { Set> suppressed = Set.of( - Pair.create("Irrelevant.java", 44), // see: IDEA-377761 - Pair.create("Irrelevant.java", 46), // see: IDEA-377761 - Pair.create("Other.java", 72), // see: IDEA-377763 Pair.create("Other.java", 70) // see: IDEA-377763 ); @@ -230,6 +227,7 @@ public class JSpecifyConformanceAnnotationTest extends LightJavaCodeInsightFixtu "inspection.nullable.problems.at.throws", "inspection.nullable.problems.at.type.parameter", "inspection.nullable.problems.Nullable.NotNull.conflict", + "conflicting.nullability.annotations", "inspection.nullable.problems.at.wildcard", "inspection.nullable.problems.at.local.variable" -> warnings.put(anchor, "test:irrelevant-annotation:" + getAnnotationShortName(((PsiAnnotationImpl)anchor).getQualifiedName())); 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 ae1ba95095be..b7dc7792cade 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/NullableStuffInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/NullableStuffInspectionTest.java @@ -471,4 +471,9 @@ public class NullableStuffInspectionTest extends LightJavaCodeInsightFixtureTest setupTypeUseAnnotations("org.jspecify.annotations", myFixture); doTest(); } + + public void testIncompatibleContainer() { + addJSpecifyNullMarked(myFixture); + doTest(); + } } \ No newline at end of file