IDEA-192527 warning persists on field when overriding @Nonnull custom annotation with a @Nonnull(When.Maybe) annotation

1. understand When.Maybe 2. stop at the first default annotation, of any nullity
2026-09-27 10:03:11 +07:00 · 2018-05-28 20:59:01 +02:00
parent 5f592ac0a2
commit c22dda7b02
7 changed files with 154 additions and 60 deletions
@@ -131,8 +131,7 @@ public class InferredAnnotationsManagerImpl extends InferredAnnotationsManager {
      return null;
    }

-    if (NullableNotNullManager.findNullabilityDefaultInHierarchy(method, true) != null ||
-        NullableNotNullManager.findNullabilityDefaultInHierarchy(method, false) != null) {
+    if (manager.findNullityDefaultInHierarchy(method) != null) {
      return null;
    }

@@ -20,12 +20,10 @@ import com.intellij.util.containers.ContainerUtil;
 import one.util.streamex.StreamEx;
 import org.jdom.Element;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.jetbrains.jps.model.serialization.java.compiler.JpsJavaCompilerNotNullableSerializer;

-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.List;
+import java.util.*;

@State(name = "NullableNotNullManager")
 public class NullableNotNullManagerImpl extends NullableNotNullManager implements PersistentStateComponent<Element> {
@@ -117,6 +115,45 @@ public class NullableNotNullManagerImpl extends NullableNotNullManager implement
    return result;
  }

+  @Override
+  protected NullityDefault isJsr305Default(@NotNull PsiAnnotation annotation, @NotNull PsiAnnotation.TargetType[] placeTargetTypes) {
+    PsiClass declaration = resolveAnnotationType(annotation);
+    PsiModifierList modList = declaration == null ? null : declaration.getModifierList();
+    if (modList == null) return null;
+
+    PsiAnnotation tqDefault = AnnotationUtil.findAnnotation(declaration, true, "javax.annotation.meta.TypeQualifierDefault");
+    if (tqDefault == null) return null;
+
+    Set<PsiAnnotation.TargetType> required = AnnotationTargetUtil.extractRequiredAnnotationTargets(tqDefault.findAttributeValue(null));
+    if (required == null || (!required.isEmpty() && !ContainerUtil.intersects(required, Arrays.asList(placeTargetTypes)))) return null;
+    
+    for (PsiAnnotation qualifier : modList.getAnnotations()) {
+      Nullness nullness = getJsr305QualifierNullness(qualifier);
+      if (nullness != null) {
+        return new NullityDefault(annotation, nullness == Nullness.NULLABLE);
+      }
+    }
+    return null;
+  }
+
+  @Nullable
+  private static PsiClass resolveAnnotationType(@NotNull PsiAnnotation annotation) {
+    PsiJavaCodeReferenceElement element = annotation.getNameReferenceElement();
+    PsiElement declaration = element == null ? null : element.resolve();
+    if (!(declaration instanceof PsiClass) || !((PsiClass)declaration).isAnnotationType()) return null;
+    return (PsiClass)declaration;
+  }
+
+  @Nullable
+  private Nullness getJsr305QualifierNullness(@NotNull PsiAnnotation qualifier) {
+    String qName = qualifier.getQualifiedName();
+    if (qName == null || !qName.startsWith("javax.annotation.")) return null;
+
+    if (qName.equals(JAVAX_ANNOTATION_NULLABLE) && getNullables().contains(qName)) return Nullness.NULLABLE;
+    if (qName.equals(JAVAX_ANNOTATION_NONNULL)) return extractNullityFromWhenValue(qualifier);
+    return null;
+  }
+
  private static boolean isNullabilityNickName(@NotNull PsiClass candidate) {
    String qname = candidate.getQualifiedName();
    if (qname == null || qname.startsWith("javax.annotation.")) return false;
@@ -4,6 +4,7 @@ package com.intellij.codeInsight;
 import com.intellij.psi.*;
 import com.intellij.psi.util.PsiUtil;
 import com.intellij.util.containers.ContainerUtil;
+import org.jetbrains.annotations.Nullable;

 import java.util.Set;

@@ -14,30 +15,39 @@ class CheckerFrameworkNullityUtil {
  private static final String DEFAULT_QUALIFIER = "org.checkerframework.framework.qual.DefaultQualifier";
  private static final String DEFAULT_QUALIFIERS = "org.checkerframework.framework.qual.DefaultQualifiers";

-  static boolean isCheckerDefault(PsiAnnotation anno, boolean nullable, PsiAnnotation.TargetType[] types) {
+  @Nullable
+  static NullityDefault isCheckerDefault(PsiAnnotation anno, PsiAnnotation.TargetType[] types) {
    String qName = anno.getQualifiedName();
    if (DEFAULT_QUALIFIER.equals(qName)) {
      PsiAnnotationMemberValue value = anno.findAttributeValue(PsiAnnotation.DEFAULT_REFERENCED_METHOD_NAME);
-      return value instanceof PsiClassObjectAccessExpression &&
-             isNullityAnnotationReference(nullable, (PsiClassObjectAccessExpression)value) &&
-             hasAppropriateTarget(types, anno.findAttributeValue("locations"));
+      if (value instanceof PsiClassObjectAccessExpression &&
+          hasAppropriateTarget(types, anno.findAttributeValue("locations"))) {
+        PsiClass valueClass = PsiUtil.resolveClassInClassTypeOnly(((PsiClassObjectAccessExpression)value).getOperand().getType());
+        if (valueClass != null) {
+          NullableNotNullManager instance = NullableNotNullManager.getInstance(value.getProject());
+          if (instance.getNullables().contains(valueClass.getQualifiedName())) {
+            return new NullityDefault(anno, true);
+          }
+          if (instance.getNotNulls().contains(valueClass.getQualifiedName())) {
+            return new NullityDefault(anno, true);
+          }
+        }
+      }
+      return null;
    }
    
    if (DEFAULT_QUALIFIERS.equals(qName)) {
      PsiAnnotationMemberValue value = anno.findAttributeValue(PsiAnnotation.DEFAULT_REFERENCED_METHOD_NAME);
      for (PsiAnnotationMemberValue initializer : AnnotationUtil.arrayAttributeValues(value)) {
-        if (initializer instanceof PsiAnnotation && isCheckerDefault((PsiAnnotation)initializer, nullable, types)) {
-          return true;
+        if (initializer instanceof PsiAnnotation) {
+          NullityDefault result = isCheckerDefault((PsiAnnotation)initializer, types);
+          if (result != null) {
+            return result;
+          }
        }
      }
    }
-    return false;
-  }
-
-  private static boolean isNullityAnnotationReference(boolean nullable, PsiClassObjectAccessExpression value) {
-    PsiClass valueClass = PsiUtil.resolveClassInClassTypeOnly(value.getOperand().getType());
-    NullableNotNullManager instance = NullableNotNullManager.getInstance(value.getProject());
-    return valueClass != null && (nullable ? instance.getNullables() : instance.getNotNulls()).contains(valueClass.getQualifiedName());
+    return null;
  }

  private static boolean hasAppropriateTarget(PsiAnnotation.TargetType[] types, PsiAnnotationMemberValue locations) {
@@ -29,7 +29,7 @@ public abstract class NullableNotNullManager {
  @SuppressWarnings("deprecation") public final JDOMExternalizableStringList myNullables = new JDOMExternalizableStringList();
  @SuppressWarnings("deprecation") public final JDOMExternalizableStringList myNotNulls = new JDOMExternalizableStringList();

-  private static final String JAVAX_ANNOTATION_NULLABLE = "javax.annotation.Nullable";
+  protected static final String JAVAX_ANNOTATION_NULLABLE = "javax.annotation.Nullable";
  protected static final String JAVAX_ANNOTATION_NONNULL = "javax.annotation.Nonnull";

  static final String[] DEFAULT_NULLABLES = {
@@ -109,12 +109,12 @@ public abstract class NullableNotNullManager {

  @Nullable
  public PsiAnnotation getNullableAnnotation(@NotNull PsiModifierListOwner owner, boolean checkBases) {
-    return findNullabilityAnnotationWithDefault(owner, checkBases, true);
+    return findNullityAnnotationWithDefault(owner, checkBases, true);
  }

  public boolean isContainerAnnotation(@NotNull PsiAnnotation anno) {
    PsiAnnotation.TargetType[] acceptAnyTarget = PsiAnnotation.TargetType.values();
-    return isNullabilityDefault(anno, true, acceptAnyTarget, false) || isNullabilityDefault(anno, false, acceptAnyTarget, false);
+    return checkNullityDefault(anno, acceptAnyTarget, false) != null;
  }

  public void setDefaultNullable(@NotNull String defaultNullable) {
@@ -129,7 +129,7 @@ public abstract class NullableNotNullManager {

  @Nullable
  public PsiAnnotation getNotNullAnnotation(@NotNull PsiModifierListOwner owner, boolean checkBases) {
-    return findNullabilityAnnotationWithDefault(owner, checkBases, false);
+    return findNullityAnnotationWithDefault(owner, checkBases, false);
  }

  @Nullable
@@ -192,8 +192,8 @@ public abstract class NullableNotNullManager {
  }

  @Nullable
-  private PsiAnnotation findNullabilityAnnotationWithDefault(@NotNull PsiModifierListOwner owner, boolean checkBases, boolean nullable) {
-    PsiAnnotation annotation = findPlainNullabilityAnnotation(owner, checkBases);
+  private PsiAnnotation findNullityAnnotationWithDefault(@NotNull PsiModifierListOwner owner, boolean checkBases, boolean nullable) {
+    PsiAnnotation annotation = findPlainNullityAnnotation(owner, checkBases);
    if (annotation != null) {
      String qName = annotation.getQualifiedName();
      if (qName == null) return null;
@@ -225,20 +225,21 @@ public abstract class NullableNotNullManager {
      }
    }

-    return findNullabilityDefaultInHierarchy(owner, nullable);
+    NullityDefault nullityDefault = findNullityDefaultInHierarchy(owner);
+    return nullityDefault != null && nullityDefault.isNullableDefault == nullable ? nullityDefault.annotation : null;
  }

  private PsiAnnotation takeAnnotationFromSuperParameters(@NotNull PsiParameter owner, final List<PsiParameter> superOwners) {
    return RecursionManager.doPreventingRecursion(owner, true, () -> {
      for (PsiParameter superOwner : superOwners) {
-        PsiAnnotation anno = findNullabilityAnnotationWithDefault(superOwner, false, false);
+        PsiAnnotation anno = findNullityAnnotationWithDefault(superOwner, false, false);
        if (anno != null) return anno;
      }
      return null;
    });
  }

-  private PsiAnnotation findPlainNullabilityAnnotation(@NotNull PsiModifierListOwner owner, boolean checkBases) {
+  private PsiAnnotation findPlainNullityAnnotation(@NotNull PsiModifierListOwner owner, boolean checkBases) {
    Set<String> qNames = ContainerUtil.newHashSet(getNullablesWithNickNames());
    qNames.addAll(getNotNullsWithNickNames());
    PsiAnnotation memberAnno = checkBases && owner instanceof PsiMethod
@@ -292,30 +293,30 @@ public abstract class NullableNotNullManager {
  }

  public boolean isNullable(@NotNull PsiModifierListOwner owner, boolean checkBases) {
-    return findNullabilityAnnotationWithDefault(owner, checkBases, true) != null;
+    return findNullityAnnotationWithDefault(owner, checkBases, true) != null;
  }

  public boolean isNotNull(@NotNull PsiModifierListOwner owner, boolean checkBases) {
-    return findNullabilityAnnotationWithDefault(owner, checkBases, false) != null;
+    return findNullityAnnotationWithDefault(owner, checkBases, false) != null;
  }

  @Nullable
-  static PsiAnnotation findNullabilityDefaultInHierarchy(PsiModifierListOwner owner, boolean nullable) {
+  NullityDefault findNullityDefaultInHierarchy(@NotNull PsiModifierListOwner owner) {
    PsiAnnotation.TargetType[] placeTargetTypes = AnnotationTargetUtil.getTargetsForLocation(owner.getModifierList());

    PsiElement element = owner.getParent();
    while (element != null) {
      if (element instanceof PsiModifierListOwner) {
-        PsiAnnotation annotation = getNullabilityDefault((PsiModifierListOwner)element, nullable, placeTargetTypes, false);
-        if (annotation != null) {
-          return annotation;
+        NullityDefault result = getNullityDefault((PsiModifierListOwner)element, placeTargetTypes, false);
+        if (result != null) {
+          return result;
        }
      }

      if (element instanceof PsiClassOwner) {
        String packageName = ((PsiClassOwner)element).getPackageName();
-        return findNullabilityDefaultOnPackage(nullable, placeTargetTypes,
-                                               JavaPsiFacade.getInstance(element.getProject()).findPackage(packageName));
+        return findNullityDefaultOnPackage(placeTargetTypes,
+                                           JavaPsiFacade.getInstance(element.getProject()).findPackage(packageName));
      }

      element = element.getContext();
@@ -323,12 +324,11 @@ public abstract class NullableNotNullManager {
    return null;
  }

-  private static PsiAnnotation findNullabilityDefaultOnPackage(boolean nullable,
-                                                               PsiAnnotation.TargetType[] placeTargetTypes,
-                                                               @Nullable PsiPackage psiPackage) {
+  @Nullable
+  private NullityDefault findNullityDefaultOnPackage(PsiAnnotation.TargetType[] placeTargetTypes, @Nullable PsiPackage psiPackage) {
    boolean superPackage = false;
    while (psiPackage != null) {
-      PsiAnnotation onPkg = getNullabilityDefault(psiPackage, nullable, placeTargetTypes, superPackage);
+      NullityDefault onPkg = getNullityDefault(psiPackage, placeTargetTypes, superPackage);
      if (onPkg != null) return onPkg;
      superPackage = true;
      psiPackage = psiPackage.getParentPackage();
@@ -336,36 +336,27 @@ public abstract class NullableNotNullManager {
    return null;
  }

-  private static PsiAnnotation getNullabilityDefault(@NotNull PsiModifierListOwner container, boolean nullable, PsiAnnotation.TargetType[] placeTargetTypes, boolean superPackage) {
+  @Nullable
+  private NullityDefault getNullityDefault(PsiModifierListOwner container, PsiAnnotation.TargetType[] placeTargetTypes, boolean superPackage) {
    PsiModifierList modifierList = container.getModifierList();
    if (modifierList == null) return null;
    for (PsiAnnotation annotation : modifierList.getAnnotations()) {
-      if (isNullabilityDefault(annotation, nullable, placeTargetTypes, superPackage)) {
-        return annotation;
+      NullityDefault result = checkNullityDefault(annotation, placeTargetTypes, superPackage);
+      if (result != null) {
+        return result;
      }
    }
    return null;
  }

-  private static boolean isNullabilityDefault(@NotNull PsiAnnotation annotation, boolean nullable, PsiAnnotation.TargetType[] placeTargetTypes, boolean superPackage) {
-    if (!superPackage && isJsr305Default(annotation, nullable, placeTargetTypes)) return true;
-    return CheckerFrameworkNullityUtil.isCheckerDefault(annotation, nullable, placeTargetTypes);
+  @Nullable
+  private NullityDefault checkNullityDefault(PsiAnnotation annotation, PsiAnnotation.TargetType[] placeTargetTypes, boolean superPackage) {
+    NullityDefault jsr = superPackage ? null : isJsr305Default(annotation, placeTargetTypes);
+    return jsr != null ? jsr : CheckerFrameworkNullityUtil.isCheckerDefault(annotation, placeTargetTypes);
  }

-  private static boolean isJsr305Default(PsiAnnotation annotation, boolean nullable, PsiAnnotation.TargetType[] placeTargetTypes) {
-    PsiJavaCodeReferenceElement element = annotation.getNameReferenceElement();
-    PsiElement declaration = element == null ? null : element.resolve();
-    if (!(declaration instanceof PsiClass)) return false;
-
-    String fqn = nullable ? JAVAX_ANNOTATION_NULLABLE : JAVAX_ANNOTATION_NONNULL;
-    if (!isAnnotated((PsiClass)declaration, fqn, CHECK_TYPE)) return false;
-
-    PsiAnnotation tqDefault = findAnnotation((PsiClass)declaration, true, "javax.annotation.meta.TypeQualifierDefault");
-    if (tqDefault == null) return false;
-
-    Set<PsiAnnotation.TargetType> required = AnnotationTargetUtil.extractRequiredAnnotationTargets(tqDefault.findAttributeValue(null));
-    return required != null && (required.isEmpty() || ContainerUtil.intersects(required, Arrays.asList(placeTargetTypes)));
-  }
+  @Nullable
+  protected abstract NullityDefault isJsr305Default(@NotNull PsiAnnotation annotation, @NotNull PsiAnnotation.TargetType[] placeTargetTypes);

  @NotNull
  public List<String> getNullables() {
@@ -0,0 +1,18 @@
+// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
+package com.intellij.codeInsight;
+
+import com.intellij.psi.PsiAnnotation;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * @author peter
+ */
+class NullityDefault {
+  @NotNull final PsiAnnotation annotation;
+  final boolean isNullableDefault;
+
+  NullityDefault(@NotNull PsiAnnotation annotation, boolean isNullableDefault) {
+    this.annotation = annotation;
+    this.isNullableDefault = isNullableDefault;
+  }
+}
@@ -0,0 +1,29 @@
+package foo;
+
+import javax.annotation.*;
+import javax.annotation.meta.TypeQualifierDefault;
+import javax.annotation.meta.When;
+import java.lang.annotation.ElementType;
+
+@Nonnull
+@TypeQualifierDefault({ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER, ElementType.TYPE_USE})
+@interface NonnullByDefault {}
+
+@Nonnull(when = When.MAYBE)
+@TypeQualifierDefault({ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER, ElementType.TYPE_USE})
+@interface NullableByDefault { }
+
+@NullableByDefault
+class Bug {
+
+  // This is emitting a warning when it shouldn't be.
+  private String id;
+
+  public String getId() {
+    return id;
+  }
+
+  public void setId(String id) {
+    this.id = id;
+  }
+}
@@ -226,6 +226,16 @@ public class NullableStuffInspectionTest extends LightCodeInsightFixtureTestCase
    myFixture.checkHighlighting(true, false, true);
  }

+  public void testNullableDefaultOnClassVsNonnullOnPackage() {
+    DataFlowInspectionTest.addJavaxNullabilityAnnotations(myFixture);
+    DataFlowInspectionTest.addJavaxDefaultNullabilityAnnotations(myFixture);
+    myFixture.addFileToProject("foo/package-info.java", "@NonnullByDefault package foo;");
+
+    myFixture.configureFromExistingVirtualFile(myFixture.copyFileToProject(getTestName(false) + ".java", "foo/Classes.java"));
+    myFixture.enableInspections(myInspection);
+    myFixture.checkHighlighting(true, false, true);
+  }
+
  public void testBeanValidationNotNull() {
    myFixture.addClass("package javax.annotation.constraints; public @interface NotNull{}");
    DataFlowInspection8Test.setCustomAnnotations(getProject(), getTestRootDisposable(), "javax.annotation.constraints.NotNull", "javax.annotation.constraints.Nullable");