From a42ef743390a2626cab5b35bc309292d894bdcc1 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Tue, 26 Oct 2010 21:54:52 +0200 Subject: [PATCH] IDEA-15184 (Inspection "@NotNull/@Nullable" problems misses @NotNull getter for @Nullable field) IDEA-60156 (Inspection: Detect assignment to @NotNull field from unannotated parameter) --- .../nullable/NullableStuffInspection.java | 144 ++++++++++++++---- .../getterSetterProblems/expected.xml | 34 ++++- .../getterSetterProblems/src/Test.java | 20 +++ .../src/messages/InspectionsBundle.properties | 4 + 4 files changed, 175 insertions(+), 27 deletions(-) diff --git a/java/java-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspection.java b/java/java-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspection.java index a53fb5e699d0..3d2fa931a3a1 100644 --- a/java/java-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspection.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2009 JetBrains s.r.o. + * Copyright 2000-2010 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,11 +26,12 @@ import com.intellij.psi.*; import com.intellij.psi.codeStyle.JavaCodeStyleManager; import com.intellij.psi.codeStyle.VariableKind; import com.intellij.psi.search.GlobalSearchScope; +import com.intellij.psi.search.LocalSearchScope; import com.intellij.psi.search.searches.OverridingMethodsSearch; -import com.intellij.psi.util.MethodSignatureBackedByPsiMethod; -import com.intellij.psi.util.PropertyUtil; -import com.intellij.psi.util.PsiUtil; -import com.intellij.psi.util.TypeConversionUtil; +import com.intellij.psi.search.searches.ReferencesSearch; +import com.intellij.psi.util.*; +import com.intellij.util.Processor; +import com.intellij.util.Query; import org.jetbrains.annotations.NotNull; import javax.swing.*; @@ -73,41 +74,132 @@ public class NullableStuffInspection extends BaseLocalInspectionTool { @Override public void visitField(PsiField field) { if (!PsiUtil.isLanguageLevel5OrHigher(field)) return; - Annotated annotated = check(field, holder, field.getType()); - if (annotated.isDeclaredNotNull || annotated.isDeclaredNullable) { + final PsiType type = field.getType(); + final Annotated annotated = check(field, holder, type); + if (TypeConversionUtil.isPrimitiveAndNotNull(type)) { + return; + } + if (annotated.isDeclaredNotNull ^ annotated.isDeclaredNullable) { final String anno = annotated.isDeclaredNotNull ? AnnotationUtil.NOT_NULL : AnnotationUtil.NULLABLE; final String annoToRemove = annotated.isDeclaredNotNull ? AnnotationUtil.NULLABLE : AnnotationUtil.NOT_NULL; final String simpleName = annotated.isDeclaredNotNull ? AnnotationUtil.NOT_NULL_SIMPLE_NAME : AnnotationUtil.NULLABLE_SIMPLE_NAME; final String propName = JavaCodeStyleManager.getInstance(field.getProject()).variableNameToPropertyName(field.getName(), VariableKind.FIELD); final boolean isStatic = field.hasModifierProperty(PsiModifier.STATIC); - if (REPORT_NOT_ANNOTATED_GETTER) { - final PsiMethod getter = PropertyUtil.findPropertyGetter(field.getContainingClass(), propName, isStatic, false); - if (getter != null && !AnnotationUtil.isAnnotated(getter, AnnotationUtil.ALL_ANNOTATIONS) && !TypeConversionUtil.isPrimitiveAndNotNull(getter.getReturnType())) { - holder.registerProblem(getter.getNameIdentifier(), - InspectionsBundle.message("inspection.nullable.problems.annotated.field.getter.not.annotated", simpleName), - ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - new AnnotateMethodFix(anno, annoToRemove)); + final PsiMethod getter = PropertyUtil.findPropertyGetter(field.getContainingClass(), propName, isStatic, false); + if (getter != null) { + if (REPORT_NOT_ANNOTATED_GETTER) { + if (!AnnotationUtil.isAnnotated(getter, AnnotationUtil.ALL_ANNOTATIONS) && + !TypeConversionUtil.isPrimitiveAndNotNull(getter.getReturnType())) { + holder.registerProblem(getter.getNameIdentifier(), InspectionsBundle + .message("inspection.nullable.problems.annotated.field.getter.not.annotated", simpleName), + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, new AnnotateMethodFix(anno, annoToRemove)); + } + } + if (annotated.isDeclaredNotNull && AnnotationUtil.isAnnotated(getter, AnnotationUtil.NULLABLE, false)) { + holder.registerProblem(getter.getNameIdentifier(), InspectionsBundle.message( + "inspection.nullable.problems.annotated.field.getter.conflict", simpleName, AnnotationUtil.NULLABLE_SIMPLE_NAME), + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, new AnnotateMethodFix(anno, annoToRemove)); + } else if (annotated.isDeclaredNullable && AnnotationUtil.isAnnotated(getter, AnnotationUtil.NOT_NULL, false)) { + holder.registerProblem(getter.getNameIdentifier(), InspectionsBundle.message( + "inspection.nullable.problems.annotated.field.getter.conflict", simpleName, AnnotationUtil.NOT_NULL_SIMPLE_NAME), + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, new AnnotateMethodFix(anno, annoToRemove)); } } - if (REPORT_NOT_ANNOTATED_SETTER_PARAMETER) { - final PsiMethod setter = PropertyUtil.findPropertySetter(field.getContainingClass(), propName, isStatic, false); - if (setter != null) { - final PsiParameter[] parameters = setter.getParameterList().getParameters(); - assert parameters.length == 1; - final PsiParameter parameter = parameters[0]; - if (!AnnotationUtil.isAnnotated(parameter, AnnotationUtil.ALL_ANNOTATIONS) && !TypeConversionUtil.isPrimitiveAndNotNull(parameter.getType())) { - holder.registerProblem(parameter.getNameIdentifier(), - InspectionsBundle.message("inspection.nullable.problems.annotated.field.setter.parameter.not.annotated", simpleName), - ProblemHighlightType.GENERIC_ERROR_OR_WARNING, - new AddAnnotationFix(anno, parameter, annoToRemove)); - } + final PsiClass containingClass = field.getContainingClass(); + final PsiMethod setter = PropertyUtil.findPropertySetter(containingClass, propName, isStatic, false); + if (setter != null) { + final PsiParameter[] parameters = setter.getParameterList().getParameters(); + assert parameters.length == 1; + final PsiParameter parameter = parameters[0]; + if (REPORT_NOT_ANNOTATED_SETTER_PARAMETER && !AnnotationUtil.isAnnotated(parameter, AnnotationUtil.ALL_ANNOTATIONS) && !TypeConversionUtil.isPrimitiveAndNotNull(parameter.getType())) { + holder.registerProblem(parameter.getNameIdentifier(), + InspectionsBundle.message("inspection.nullable.problems.annotated.field.setter.parameter.not.annotated", simpleName), + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + new AddAnnotationFix(anno, parameter, annoToRemove)); } + if (annotated.isDeclaredNotNull && AnnotationUtil.isAnnotated(parameter, AnnotationUtil.NULLABLE, false)) { + holder.registerProblem(parameter.getNameIdentifier(), InspectionsBundle.message( + "inspection.nullable.problems.annotated.field.setter.parameter.conflict", simpleName, AnnotationUtil.NULLABLE_SIMPLE_NAME), + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + new AddAnnotationFix(anno, parameter, annoToRemove)); + } else if (annotated.isDeclaredNullable && AnnotationUtil.isAnnotated(parameter, AnnotationUtil.NOT_NULL, false)) { + holder.registerProblem(parameter.getNameIdentifier(), InspectionsBundle.message( + "inspection.nullable.problems.annotated.field.setter.parameter.conflict", simpleName, AnnotationUtil.NOT_NULL_SIMPLE_NAME), + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + new AddAnnotationFix(anno, parameter, annoToRemove)); + } + if (containingClass == null) { + return; + } + final PsiMethod[] constructors = containingClass.getConstructors(); + final Query search = ReferencesSearch.search(field, new LocalSearchScope(constructors), false); + search.forEach(new Processor() { + @Override + public boolean process(PsiReference reference) { + final PsiElement element = reference.getElement(); + if (!(element instanceof PsiReferenceExpression)) { + return true; + } + PsiReferenceExpression referenceExpression = (PsiReferenceExpression)element; + final PsiAssignmentExpression assignmentExpression = getAssignmentExpressionIfOnAssignmentLefthand(referenceExpression); + final PsiMethod method = PsiTreeUtil.getParentOfType(assignmentExpression, PsiMethod.class); + if (method == null || !method.isConstructor()) { + return true; + } + if (assignmentExpression == null) { + return true; + } + final PsiExpression rhs = assignmentExpression.getRExpression(); + if (!(rhs instanceof PsiReferenceExpression)) { + return true; + } + PsiReferenceExpression expression = (PsiReferenceExpression)rhs; + final PsiElement target = expression.resolve(); + if (!(target instanceof PsiParameter)) { + return true; + } + final PsiParameter parameter = (PsiParameter)target; + if (!method.equals(parameter.getDeclarationScope())) { + return true; + } + if (REPORT_NOT_ANNOTATED_SETTER_PARAMETER && !AnnotationUtil.isAnnotated(parameter, AnnotationUtil.ALL_ANNOTATIONS)) { + holder.registerProblem(parameter.getNameIdentifier(), InspectionsBundle + .message("inspection.nullable.problems.annotated.field.constructor.parameter.not.annotated", simpleName), + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, new AddAnnotationFix(anno, parameter, annoToRemove)); + return true; + } + if (annotated.isDeclaredNotNull && AnnotationUtil.isAnnotated(parameter, AnnotationUtil.NULLABLE, false)) { + holder.registerProblem(parameter.getNameIdentifier(), InspectionsBundle.message( + "inspection.nullable.problems.annotated.field.constructor.parameter.conflict", simpleName, AnnotationUtil.NULLABLE_SIMPLE_NAME), + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + new AddAnnotationFix(anno, parameter, annoToRemove)); + } else if (annotated.isDeclaredNullable && AnnotationUtil.isAnnotated(parameter, AnnotationUtil.NOT_NULL, false)) { + holder.registerProblem(parameter.getNameIdentifier(), InspectionsBundle.message( + "inspection.nullable.problems.annotated.field.constructor.parameter.conflict", simpleName, AnnotationUtil.NOT_NULL_SIMPLE_NAME), + ProblemHighlightType.GENERIC_ERROR_OR_WARNING, + new AddAnnotationFix(anno, parameter, annoToRemove)); + } + return true; + } + }); } } } + public PsiAssignmentExpression getAssignmentExpressionIfOnAssignmentLefthand(PsiExpression expression) { + PsiElement parent = PsiTreeUtil.skipParentsOfType(expression, PsiParenthesizedExpression.class); + if (!(parent instanceof PsiAssignmentExpression)) { + return null; + } + final PsiAssignmentExpression assignmentExpression = (PsiAssignmentExpression)parent; + if (!PsiTreeUtil.isAncestor(assignmentExpression.getLExpression(), expression, false)) { + return null; + } + return assignmentExpression; + } + @Override public void visitParameter(PsiParameter parameter) { if (!PsiUtil.isLanguageLevel5OrHigher(parameter)) return; check(parameter, holder, parameter.getType()); diff --git a/java/java-tests/testData/inspection/nullableProblems/getterSetterProblems/expected.xml b/java/java-tests/testData/inspection/nullableProblems/getterSetterProblems/expected.xml index d0cec5c3076f..07441b9e8513 100644 --- a/java/java-tests/testData/inspection/nullableProblems/getterSetterProblems/expected.xml +++ b/java/java-tests/testData/inspection/nullableProblems/getterSetterProblems/expected.xml @@ -30,4 +30,36 @@ @NotNull/@Nullable problems Getter for @NotNull field might be annotated @NotNull itself - \ No newline at end of file + + + Test.java + 25 + @NotNull/@Nullable problems + Constructor parameter for @NotNull field might be annotated @NotNull itself + + + + + Test.java + 38 + @NotNull/@Nullable problems + Setter parameter for @NotNull field might be annotated @NotNull itself + + + + + Test.java + 34 + @NotNull/@Nullable problems + Getter for @NotNull field might be annotated @NotNull itself + + + + + Test.java + 29 + @NotNull/@Nullable problems + Constructor parameter for @NotNull field might be annotated @NotNull itself + + + diff --git a/java/java-tests/testData/inspection/nullableProblems/getterSetterProblems/src/Test.java b/java/java-tests/testData/inspection/nullableProblems/getterSetterProblems/src/Test.java index c025ca8bfeaf..a908c2c615ca 100644 --- a/java/java-tests/testData/inspection/nullableProblems/getterSetterProblems/src/Test.java +++ b/java/java-tests/testData/inspection/nullableProblems/getterSetterProblems/src/Test.java @@ -18,4 +18,24 @@ class B { public boolean getBug() { return Boolean.valueOf(bug); } +} +class C { + @NotNull C c; + + C(C c) { + this.c = c; + } + + C(@Nullable C c, int i) { + this.c = c; + } + + @Nullable + public C getC() { + return c; + } + + public void setC(@Nullable C c) { + this.c = c; + } } \ No newline at end of file diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index 75d197bdb4dc..a2fd36b34f3c 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -107,7 +107,11 @@ inspection.nullable.problems.Nullable.NotNull.conflict=Cannot annotate with both inspection.nullable.problems.Nullable.method.overrides.NotNull=Method annotated with @Nullable must not override @NotNull method inspection.nullable.problems.method.overrides.NotNull=Not annotated method overrides method annotated with @NotNull inspection.nullable.problems.annotated.field.getter.not.annotated=Getter for @{0} field might be annotated @{0} itself +inspection.nullable.problems.annotated.field.getter.conflict=Getter for @{0} field is annotated @{1} inspection.nullable.problems.annotated.field.setter.parameter.not.annotated=Setter parameter for @{0} field might be annotated @{0} itself +inspection.nullable.problems.annotated.field.setter.parameter.conflict=Setter parameter for @{0} field is annotated @{1} +inspection.nullable.problems.annotated.field.constructor.parameter.not.annotated=Constructor parameter for @{0} field might be annotated @{0} itself +inspection.nullable.problems.annotated.field.constructor.parameter.conflict=Constructor parameter for @{0} field is annotated @{1} inspection.nullable.problems.NotNull.parameter.overrides.Nullable=Parameter annotated @NonNull must not override @Nullable parameter inspection.test.only.problems.display.name=@TestOnly method call in production code