From ee554a876cc08aba01d8d63dfac19c978dc9df06 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 8 Jun 2018 17:26:08 +0700 Subject: [PATCH] NullableNotNullManager: findEffectiveNullabilityAnnotationInfo, findEffectiveNullability Removed isContainer* methods; more nullability refactoring --- .../dataFlow/DataFlowInspectionBase.java | 17 ++--- .../codeInspection/dataFlow/DfaPsiUtil.java | 20 +++--- .../NullParameterConstraintChecker.java | 3 +- .../codeInspection/dataFlow/Nullness.java | 24 ------- .../nullable/NullableStuffInspectionBase.java | 12 ++-- .../slicer/JavaSliceNullnessAnalyzer.java | 13 ++-- .../NullabilityAnnotationInfo.java | 14 ++++ .../codeInsight/NullableNotNullManager.java | 71 ++++++++++--------- .../ObjectsRequireNonNullIntention.java | 9 +-- 9 files changed, 90 insertions(+), 93 deletions(-) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java index 6529261ce967..20f5ea39897f 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInspectionBase.java @@ -2,9 +2,7 @@ package com.intellij.codeInspection.dataFlow; -import com.intellij.codeInsight.AnnotationUtil; -import com.intellij.codeInsight.ExpressionUtil; -import com.intellij.codeInsight.NullableNotNullManager; +import com.intellij.codeInsight.*; import com.intellij.codeInsight.daemon.GroupNames; import com.intellij.codeInsight.intention.AddAnnotationPsiFix; import com.intellij.codeInspection.*; @@ -763,15 +761,15 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool final PsiMethod method = getScopeMethod(block); if (method == null) return; NullableNotNullManager manager = NullableNotNullManager.getInstance(holder.getProject()); - PsiAnnotation anno = manager.findEffectiveNullabilityAnnotation(method); - Nullness annoNullness = Nullness.fromAnnotation(anno); - if (annoNullness == Nullness.NULLABLE) { - assert anno != null; + NullabilityAnnotationInfo info = manager.findEffectiveNullabilityAnnotationInfo(method); + PsiAnnotation anno = info == null ? null : info.getAnnotation(); + Nullability annoNullness = info == null ? Nullability.UNKNOWN : info.getNullability(); + if (annoNullness == Nullability.NULLABLE) { if (!AnnotationUtil.isInferredAnnotation(anno)) return; if (DfaPsiUtil.getTypeNullability(method.getReturnType()) == Nullness.NULLABLE) return; } - if (annoNullness != Nullness.NOT_NULL && (!SUGGEST_NULLABLE_ANNOTATIONS || block.getParent() instanceof PsiLambdaExpression)) return; + if (annoNullness != Nullability.NOT_NULL && (!SUGGEST_NULLABLE_ANNOTATIONS || block.getParent() instanceof PsiLambdaExpression)) return; PsiType returnType = method.getReturnType(); // no warnings in void lambdas, where the expression is not returned anyway @@ -784,8 +782,7 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool final PsiExpression expr = problem.getAnchor(); if (!reportedAnchors.add(expr)) continue; - if (annoNullness == Nullness.NOT_NULL) { - assert anno != null; + if (annoNullness == Nullability.NOT_NULL) { String presentable = NullableStuffInspectionBase.getPresentableAnnoName(anno); final String text = isNullLiteralExpression(expr) ? InspectionsBundle.message("dataflow.message.return.null.from.notnull", presentable) diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaPsiUtil.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaPsiUtil.java index 043994426c51..db445f0b9b9a 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaPsiUtil.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DfaPsiUtil.java @@ -2,6 +2,8 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInsight.AnnotationUtil; +import com.intellij.codeInsight.Nullability; +import com.intellij.codeInsight.NullabilityAnnotationInfo; import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil; import com.intellij.codeInsight.daemon.impl.analysis.JavaGenericsUtil; @@ -84,9 +86,9 @@ public class DfaPsiUtil { return Nullness.NOT_NULL; } - Nullness fromAnnotation = getNullabilityFromAnnotation(owner, ignoreParameterNullabilityInference); - if (fromAnnotation != Nullness.UNKNOWN) { - return fromAnnotation; + Nullability fromAnnotation = getNullabilityFromAnnotation(owner, ignoreParameterNullabilityInference); + if (fromAnnotation != Nullability.UNKNOWN) { + return Nullness.fromNullability(fromAnnotation); } if (owner instanceof PsiMethod && isMapMethodWithUnknownNullity((PsiMethod)owner)) { @@ -111,14 +113,14 @@ public class DfaPsiUtil { } @NotNull - private static Nullness getNullabilityFromAnnotation(PsiModifierListOwner owner, boolean ignoreParameterNullabilityInference) { + private static Nullability getNullabilityFromAnnotation(PsiModifierListOwner owner, boolean ignoreParameterNullabilityInference) { NullableNotNullManager manager = NullableNotNullManager.getInstance(owner.getProject()); - PsiAnnotation annotation = manager.findEffectiveNullabilityAnnotation(owner); - if (annotation == null || - (ignoreParameterNullabilityInference && owner instanceof PsiParameter && AnnotationUtil.isInferredAnnotation(annotation))) { - return Nullness.UNKNOWN; + NullabilityAnnotationInfo info = manager.findEffectiveNullabilityAnnotationInfo(owner); + if (info == null || + ignoreParameterNullabilityInference && owner instanceof PsiParameter && AnnotationUtil.isInferredAnnotation(info.getAnnotation())) { + return Nullability.UNKNOWN; } - return Nullness.fromAnnotation(annotation); + return info.getNullability(); } private static boolean isMapMethodWithUnknownNullity(@NotNull PsiMethod method) { diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullParameterConstraintChecker.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullParameterConstraintChecker.java index 9f553d018ccb..7697c1d6b3ef 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullParameterConstraintChecker.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullParameterConstraintChecker.java @@ -1,6 +1,7 @@ // 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.codeInspection.dataFlow; +import com.intellij.codeInsight.Nullability; import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInspection.dataFlow.instructions.AssignInstruction; import com.intellij.codeInspection.dataFlow.instructions.Instruction; @@ -53,7 +54,7 @@ class NullParameterConstraintChecker extends DataFlowRunner { for (int index = 0; index < parameters.length; index++) { PsiParameter parameter = parameters[index]; if (!(parameter.getType() instanceof PsiPrimitiveType) && - NullableNotNullManager.getInstance(method.getProject()).findEffectiveNullabilityAnnotation(parameter) == null && + NullableNotNullManager.getInstance(method.getProject()).findEffectiveNullability(parameter) == Nullability.UNKNOWN && JavaNullMethodArgumentUtil.hasNullArgument(method, index)) { nullableParameters.add(parameter); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/Nullness.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/Nullness.java index 075cc018d03d..70c7a8b24fb5 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/Nullness.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/Nullness.java @@ -16,12 +16,7 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInsight.Nullability; -import com.intellij.codeInsight.NullableNotNullManager; -import com.intellij.psi.PsiAnnotation; -import com.intellij.psi.PsiModifierListOwner; import org.jetbrains.annotations.Contract; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; /** * @deprecated use {@link Nullability} @@ -30,25 +25,6 @@ import org.jetbrains.annotations.Nullable; public enum Nullness { NOT_NULL, NULLABLE, UNKNOWN; - /** - * Convert nullability annotation returned by {@link NullableNotNullManager#findEffectiveNullabilityAnnotation(PsiModifierListOwner)} - * to {@code Nullness} value - * - * @param annotation annotation to convert - * @return Nullness value - */ - @NotNull - public static Nullness fromAnnotation(@Nullable PsiAnnotation annotation) { - if (annotation == null) return UNKNOWN; - if (NullableNotNullManager.isNullableAnnotation(annotation) || NullableNotNullManager.isContainerNullableAnnotation(annotation)) { - return NULLABLE; - } - if (NullableNotNullManager.isNotNullAnnotation(annotation) || NullableNotNullManager.isContainerNotNullAnnotation(annotation)) { - return NOT_NULL; - } - return UNKNOWN; - } - @Contract(pure = true) public Nullability toNullability() { switch (this) { 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 9ebbf758302f..4e44aa6f2b91 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 @@ -3,6 +3,7 @@ package com.intellij.codeInspection.nullable; import com.intellij.codeInsight.AnnotationTargetUtil; import com.intellij.codeInsight.AnnotationUtil; +import com.intellij.codeInsight.NullabilityAnnotationInfo; import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInsight.daemon.GroupNames; import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil; @@ -513,10 +514,11 @@ public class NullableStuffInspectionBase extends AbstractBaseJavaLocalInspection } private static void checkNotNullFieldsInitialized(PsiField field, NullableNotNullManager manager, @NotNull ProblemsHolder holder) { - PsiAnnotation annotation = manager.getNotNullAnnotation(field, false); - if (annotation == null || HighlightControlFlowUtil.isFieldInitializedAfterObjectConstruction(field)) return; + NullabilityAnnotationInfo info = manager.findEffectiveNullabilityAnnotationInfo(field); + if (info == null || HighlightControlFlowUtil.isFieldInitializedAfterObjectConstruction(field)) return; - boolean byDefault = manager.isContainerAnnotation(annotation); + boolean byDefault = info.isContainer(); + PsiAnnotation annotation = info.getAnnotation(); PsiJavaCodeReferenceElement name = annotation.getNameReferenceElement(); holder.registerProblem(annotation.isPhysical() && !byDefault ? annotation : field.getNameIdentifier(), (byDefault && name != null ? "@" + name.getReferenceName() : "Not-null") + " fields must be initialized"); @@ -578,8 +580,8 @@ public class NullableStuffInspectionBase extends AbstractBaseJavaLocalInspection @NotNull private static String getPresentableAnnoName(@NotNull PsiModifierListOwner owner) { NullableNotNullManager manager = NullableNotNullManager.getInstance(owner.getProject()); - PsiAnnotation anno = manager.findEffectiveNullabilityAnnotation(owner); - String name = anno == null ? null : anno.getQualifiedName(); + NullabilityAnnotationInfo info = manager.findEffectiveNullabilityAnnotationInfo(owner); + String name = info == null ? null : info.getAnnotation().getQualifiedName(); if (name == null) { return "???"; } diff --git a/java/java-impl/src/com/intellij/slicer/JavaSliceNullnessAnalyzer.java b/java/java-impl/src/com/intellij/slicer/JavaSliceNullnessAnalyzer.java index fdbab3663918..1e1aed62e041 100644 --- a/java/java-impl/src/com/intellij/slicer/JavaSliceNullnessAnalyzer.java +++ b/java/java-impl/src/com/intellij/slicer/JavaSliceNullnessAnalyzer.java @@ -15,6 +15,7 @@ */ package com.intellij.slicer; +import com.intellij.codeInsight.Nullability; import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInspection.dataFlow.DfaUtil; import com.intellij.codeInspection.dataFlow.Nullness; @@ -45,9 +46,8 @@ public class JavaSliceNullnessAnalyzer extends SliceNullnessAnalyzerBase { if (value instanceof PsiMethodCallExpression) { PsiMethod method = ((PsiMethodCallExpression)value).resolveMethod(); if (method != null) { - PsiAnnotation annotation = NullableNotNullManager.getInstance(method.getProject()).findEffectiveNullabilityAnnotation(method); - Nullness nullness = Nullness.fromAnnotation(annotation); - if (nullness != Nullness.UNKNOWN) return nullness; + Nullability nullability = NullableNotNullManager.getInstance(method.getProject()).findEffectiveNullability(method); + return Nullness.fromNullability(nullability); } } if (value instanceof PsiPolyadicExpression && ((PsiPolyadicExpression)value).getOperationTokenType() == JavaTokenType.PLUS) { @@ -75,12 +75,13 @@ public class JavaSliceNullnessAnalyzer extends SliceNullnessAnalyzerBase { } } + if (value instanceof PsiEnumConstant) return Nullness.NOT_NULL; + if (value instanceof PsiModifierListOwner) { - if (NullableNotNullManager.isNotNull((PsiModifierListOwner)value)) return Nullness.NOT_NULL; - if (NullableNotNullManager.isNullable((PsiModifierListOwner)value)) return Nullness.NULLABLE; + return Nullness + .fromNullability(NullableNotNullManager.getInstance(value.getProject()).findEffectiveNullability(((PsiModifierListOwner)value))); } - if (value instanceof PsiEnumConstant) return Nullness.NOT_NULL; return Nullness.UNKNOWN; } } diff --git a/java/java-psi-api/src/com/intellij/codeInsight/NullabilityAnnotationInfo.java b/java/java-psi-api/src/com/intellij/codeInsight/NullabilityAnnotationInfo.java index a3bf19511a47..54eb1ef611c4 100644 --- a/java/java-psi-api/src/com/intellij/codeInsight/NullabilityAnnotationInfo.java +++ b/java/java-psi-api/src/com/intellij/codeInsight/NullabilityAnnotationInfo.java @@ -40,4 +40,18 @@ public class NullabilityAnnotationInfo { public boolean isContainer() { return myContainer; } + + /** + * @return true if this annotation is an external annotation + */ + public boolean isExternal() { + return AnnotationUtil.isExternalAnnotation(myAnnotation); + } + + /** + * @return true if this annotation is an inferred annotation + */ + public boolean isInferred() { + return AnnotationUtil.isInferredAnnotation(myAnnotation); + } } 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 586d096feb8b..24b5db0a7544 100644 --- a/java/java-psi-api/src/com/intellij/codeInsight/NullableNotNullManager.java +++ b/java/java-psi-api/src/com/intellij/codeInsight/NullableNotNullManager.java @@ -83,7 +83,8 @@ public abstract class NullableNotNullManager { if (annotation == null) { return null; } - if (isContainerAnnotation(annotation)) { + PsiAnnotation.TargetType[] acceptAnyTarget = PsiAnnotation.TargetType.values(); + if (checkNullityDefault(annotation, acceptAnyTarget, false) != null) { return null; } return annotation.getQualifiedName(); @@ -94,11 +95,6 @@ public abstract class NullableNotNullManager { return findNullityAnnotationWithDefault(owner, checkBases, true); } - public boolean isContainerAnnotation(@NotNull PsiAnnotation anno) { - PsiAnnotation.TargetType[] acceptAnyTarget = PsiAnnotation.TargetType.values(); - return checkNullityDefault(anno, acceptAnyTarget, false) != null; - } - public abstract void setDefaultNullable(@NotNull String defaultNullable); @NotNull @@ -199,17 +195,25 @@ public abstract class NullableNotNullManager { } /** - * Returns nullability annotation (either nullable, or non-nullable) which has effect for given element. - * To figure out its exact effect can be subsequently checked using {@link #isNullableAnnotation(PsiAnnotation)}, - * {@link #isNotNullAnnotation(PsiAnnotation)}, {@link #isContainerNullableAnnotation(PsiAnnotation)} or - * {@link #isContainerNotNullAnnotation(PsiAnnotation)}. May return explicit, inferred, external or - * default nullability annotation for class/package. + * Returns effective nullability derived from annotations for given element. * - * @param owner - * @return effective nullability annotation, or null if not found. + * @param owner element to find a nullability for + * @return effective nullability + */ + @NotNull + public Nullability findEffectiveNullability(@NotNull PsiModifierListOwner owner) { + NullabilityAnnotationInfo info = findEffectiveNullabilityAnnotationInfo(owner); + return info == null ? Nullability.UNKNOWN : info.getNullability(); + } + + /** + * Returns nullability annotation info which has effect for given element. + * + * @param owner element to find an annotation for + * @return effective nullability annotation info, or null if not found. */ @Nullable - public PsiAnnotation findEffectiveNullabilityAnnotation(@NotNull PsiModifierListOwner owner) { + public NullabilityAnnotationInfo findEffectiveNullabilityAnnotationInfo(@NotNull PsiModifierListOwner owner) { PsiType type = getOwnerType(owner); if (type == null || TypeConversionUtil.isPrimitiveAndNotNull(type)) return null; @@ -218,8 +222,9 @@ public abstract class NullableNotNullManager { } @Nullable - private PsiAnnotation doFindEffectiveNullabilityAnnotation(@NotNull PsiModifierListOwner owner) { - Set annotationNames = ContainerUtil.newHashSet(getNullablesWithNickNames()); + private NullabilityAnnotationInfo doFindEffectiveNullabilityAnnotation(@NotNull PsiModifierListOwner owner) { + List nullables = getNullablesWithNickNames(); + Set annotationNames = ContainerUtil.newHashSet(nullables); annotationNames.addAll(getNotNullsWithNickNames()); Set extraAnnotations = DEFAULT_ALL.stream().filter(anno -> !annotationNames.contains(anno)).collect(Collectors.toSet()); annotationNames.addAll(extraAnnotations); @@ -231,20 +236,30 @@ public abstract class NullableNotNullManager { // return null in this case return null; } - return annotation; + return new NullabilityAnnotationInfo(annotation, + nullables.contains(annotation.getQualifiedName()) ? Nullability.NULLABLE : Nullability.NOT_NULL, + false); } if (owner instanceof PsiParameter) { List superParameters = getSuperAnnotationOwners((PsiParameter)owner); if (!superParameters.isEmpty()) { - PsiAnnotation superParameterAnnotation = takeAnnotationFromSuperParameters((PsiParameter)owner, superParameters); - return superParameterAnnotation != null && isContainerNotNullAnnotation(superParameterAnnotation) ? superParameterAnnotation : null; + for (PsiParameter parameter: superParameters) { + PsiAnnotation plain = findPlainAnnotation(parameter, false, annotationNames); + // Plain not null annotation is not inherited + if (plain != null) return null; + NullabilityAnnotationInfo defaultInfo = findNullityDefaultInHierarchy(parameter); + if (defaultInfo != null) { + return defaultInfo.getNullability() == Nullability.NOT_NULL ? defaultInfo : null; + } + } + return null; } } - NullabilityAnnotationInfo nullityDefault = findNullityDefaultInHierarchy(owner); - if (nullityDefault != null && (nullityDefault.getNullability() == Nullability.NULLABLE || !hasHardcodedContracts(owner))) { - return nullityDefault.getAnnotation(); + NullabilityAnnotationInfo defaultInfo = findNullityDefaultInHierarchy(owner); + if (defaultInfo != null && (defaultInfo.getNullability() == Nullability.NULLABLE || !hasHardcodedContracts(owner))) { + return defaultInfo; } return null; } @@ -411,16 +426,4 @@ public abstract class NullableNotNullManager { public static boolean isNotNullAnnotation(@NotNull PsiAnnotation annotation) { return getInstance(annotation.getProject()).getNotNullsWithNickNames().contains(annotation.getQualifiedName()); } - - public static boolean isContainerNullableAnnotation(@NotNull PsiAnnotation annotation) { - PsiAnnotation.TargetType[] acceptAnyTarget = PsiAnnotation.TargetType.values(); - NullabilityAnnotationInfo nullityDefault = getInstance(annotation.getProject()).checkNullityDefault(annotation, acceptAnyTarget, false); - return nullityDefault != null && nullityDefault.getNullability() == Nullability.NULLABLE; - } - - public static boolean isContainerNotNullAnnotation(@NotNull PsiAnnotation annotation) { - PsiAnnotation.TargetType[] acceptAnyTarget = PsiAnnotation.TargetType.values(); - NullabilityAnnotationInfo nullityDefault = getInstance(annotation.getProject()).checkNullityDefault(annotation, acceptAnyTarget, false); - return nullityDefault != null && nullityDefault.getNullability() == Nullability.NOT_NULL; - } } \ No newline at end of file diff --git a/plugins/IntentionPowerPak/src/com/siyeh/ipp/asserttoif/ObjectsRequireNonNullIntention.java b/plugins/IntentionPowerPak/src/com/siyeh/ipp/asserttoif/ObjectsRequireNonNullIntention.java index 569ddd1c1755..95dff45e8959 100644 --- a/plugins/IntentionPowerPak/src/com/siyeh/ipp/asserttoif/ObjectsRequireNonNullIntention.java +++ b/plugins/IntentionPowerPak/src/com/siyeh/ipp/asserttoif/ObjectsRequireNonNullIntention.java @@ -16,8 +16,9 @@ package com.siyeh.ipp.asserttoif; import com.intellij.codeInsight.AnnotationUtil; +import com.intellij.codeInsight.Nullability; +import com.intellij.codeInsight.NullabilityAnnotationInfo; import com.intellij.codeInsight.NullableNotNullManager; -import com.intellij.codeInspection.dataFlow.Nullness; import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; @@ -110,9 +111,9 @@ public class ObjectsRequireNonNullIntention extends Intention { if (ClassUtils.findClass("java.util.Objects", element) == null) { return false; } - final PsiAnnotation annotation = NullableNotNullManager.getInstance(variable.getProject()).findEffectiveNullabilityAnnotation(variable); - if (annotation != null && Nullness.fromAnnotation(annotation) == Nullness.NOT_NULL && - !AnnotationUtil.isExternalAnnotation(annotation) && !AnnotationUtil.isInferredAnnotation(annotation)) { + final NullabilityAnnotationInfo + info = NullableNotNullManager.getInstance(variable.getProject()).findEffectiveNullabilityAnnotationInfo(variable); + if (info != null && info.getNullability() == Nullability.NOT_NULL && !info.isExternal() && !info.isInferred()) { return true; } final PsiStatement referenceStatement = PsiTreeUtil.getParentOfType(referenceExpression, PsiStatement.class);