IDEA-140536 Wrong warning: constructor @NotNull parameter should be @Nullable when field is @Nullable

This commit is contained in:
peter
2015-05-21 12:16:39 +02:00
parent 9425fa701c
commit aff4db09b0
3 changed files with 75 additions and 36 deletions
@@ -32,11 +32,9 @@ import com.intellij.psi.codeStyle.JavaCodeStyleManager;
import com.intellij.psi.codeStyle.VariableKind;
import com.intellij.psi.search.GlobalSearchScope;
import com.intellij.psi.search.searches.OverridingMethodsSearch;
import com.intellij.psi.search.searches.ReferencesSearch;
import com.intellij.psi.util.*;
import com.intellij.util.ArrayUtil;
import com.intellij.util.Function;
import com.intellij.util.Processor;
import com.intellij.util.containers.ContainerUtil;
import org.jdom.Element;
import org.jetbrains.annotations.NotNull;
@@ -213,7 +211,7 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo
assert parameters.length == 1 : setter.getText();
final PsiParameter parameter = parameters[0];
LOG.assertTrue(parameter != null, setter.getText());
AddAnnotationPsiFix addAnnoFix = new AddAnnotationPsiFix(anno, parameter, PsiNameValuePair.EMPTY_ARRAY, ArrayUtil.toStringArray(annoToRemove));
AddAnnotationPsiFix addAnnoFix = createAddAnnotationFix(anno, annoToRemove, parameter);
if (REPORT_NOT_ANNOTATED_GETTER && !manager.hasNullability(parameter) && !TypeConversionUtil.isPrimitiveAndNotNull(parameter.getType())) {
final PsiIdentifier nameIdentifier1 = parameter.getNameIdentifier();
assertValidElement(setter, parameter, nameIdentifier1);
@@ -237,6 +235,11 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo
}
}
@NotNull
private static AddAnnotationPsiFix createAddAnnotationFix(String anno, List<String> annoToRemove, PsiParameter parameter) {
return new AddAnnotationPsiFix(anno, parameter, PsiNameValuePair.EMPTY_ARRAY, ArrayUtil.toStringArray(annoToRemove));
}
private static void assertValidElement(PsiMethod setter, PsiParameter parameter, PsiIdentifier nameIdentifier1) {
LOG.assertTrue(nameIdentifier1 != null && nameIdentifier1.isPhysical(), setter.getText());
LOG.assertTrue(parameter.isPhysical(), setter.getText());
@@ -259,46 +262,49 @@ public class NullableStuffInspectionBase extends BaseJavaBatchLocalInspectionToo
Annotated annotated,
NullableNotNullManager manager,
String anno, List<String> annoToRemove, @NotNull ProblemsHolder holder) {
for (PsiExpression rhs : DfaPsiUtil.findAllConstructorInitializers(field)) {
List<PsiExpression> initializers = DfaPsiUtil.findAllConstructorInitializers(field);
if (initializers.isEmpty()) return;
List<PsiParameter> notNullParams = ContainerUtil.newArrayList();
boolean isFinal = field.hasModifierProperty(PsiModifier.FINAL);
for (PsiExpression rhs : initializers) {
if (rhs instanceof PsiReferenceExpression) {
PsiElement target = ((PsiReferenceExpression)rhs).resolve();
if (target instanceof PsiParameter && target.isPhysical()) {
PsiParameter parameter = (PsiParameter)target;
AddAnnotationPsiFix
fix = new AddAnnotationPsiFix(anno, parameter, PsiNameValuePair.EMPTY_ARRAY, ArrayUtil.toStringArray(annoToRemove));
if (REPORT_NOT_ANNOTATED_GETTER && !manager.hasNullability(parameter) && !TypeConversionUtil
.isPrimitiveAndNotNull(parameter.getType())) {
final PsiIdentifier nameIdentifier2 = parameter.getNameIdentifier();
assert nameIdentifier2 != null : parameter;
assert nameIdentifier2.isPhysical() : parameter;
holder.registerProblem(nameIdentifier2, InspectionsBundle
.message("inspection.nullable.problems.annotated.field.constructor.parameter.not.annotated",
getPresentableAnnoName(field)),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, fix);
continue;
}
if (annotated.isDeclaredNullable && isNotNullNotInferred(parameter, false, false)) {
boolean usedAsQualifier = !ReferencesSearch.search(parameter).forEach(new Processor<PsiReference>() {
@Override
public boolean process(PsiReference reference) {
final PsiElement element = reference.getElement();
return !(element instanceof PsiReferenceExpression && element.getParent() instanceof PsiReferenceExpression);
}
});
if (!usedAsQualifier) {
final PsiIdentifier nameIdentifier2 = parameter.getNameIdentifier();
assert nameIdentifier2 != null : parameter;
holder.registerProblem(nameIdentifier2, InspectionsBundle.message(
"inspection.nullable.problems.annotated.field.constructor.parameter.conflict", getPresentableAnnoName(field),
getPresentableAnnoName(parameter)),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
fix);
if (REPORT_NOT_ANNOTATED_GETTER && !manager.hasNullability(parameter) && !TypeConversionUtil.isPrimitiveAndNotNull(parameter.getType())) {
final PsiIdentifier nameIdentifier = parameter.getNameIdentifier();
if (nameIdentifier != null && nameIdentifier.isPhysical()) {
holder.registerProblem(
nameIdentifier,
InspectionsBundle.message("inspection.nullable.problems.annotated.field.constructor.parameter.not.annotated", getPresentableAnnoName(field)),
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, createAddAnnotationFix(anno, annoToRemove, parameter));
continue;
}
}
if (isFinal && annotated.isDeclaredNullable && isNotNullNotInferred(parameter, false, false)) {
notNullParams.add(parameter);
}
}
}
}
if (notNullParams.size() != initializers.size()) {
// it's not the case that the field is final and @Nullable and always initialized via @NotNull parameters
// so there might be other initializers that could justify it being nullable
// so don't highlight field and constructor parameter annotation inconsistency
return;
}
PsiIdentifier nameIdentifier = field.getNameIdentifier();
if (nameIdentifier.isPhysical()) {
holder.registerProblem(nameIdentifier, "@" + getPresentableAnnoName(field) + " field is always initialized not-null",
ProblemHighlightType.GENERIC_ERROR_OR_WARNING, new AddNotNullAnnotationFix(field));
}
}
@NotNull
@@ -5,7 +5,7 @@ class Test {
@Nullable private final String baseFile1;
public Test(@NotNull String <warning descr="Constructor parameter for @Nullable field is annotated @NotNull">baseFile</warning>) {
public Test(@NotNull String baseFile) {
this.baseFile = baseFile;
this.baseFile1 = null;
}
@@ -18,4 +18,38 @@ class Test {
this.baseFile = null;
}
}
}
}
class Test2 {
@Nullable Object member;
public Test2(@NotNull Object member) {
this.member = member;
}
public void setMember(@Nullable Object member) {
this.member = member;
}
}
class Test3 {
@Nullable final Object <warning descr="@Nullable field is always initialized not-null">member</warning>;
public Test3(@NotNull Object member) {
this.member = member;
}
}
class Test4 {
@Nullable Object member;
public Test4(@NotNull Object member) {
this.member = member;
}
public Test4(int a) {
this.member = null;
}
}
@@ -149,7 +149,6 @@ inspection.nullable.problems.annotated.field.getter.conflict=Getter for @{0} fie
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 @{0} must not override @{1} parameter
inspection.nullable.problems.NotNull.parameter.overrides.not.annotated=Parameter annotated @{0} should not override non-annotated parameter
inspection.nullable.problems.parameter.overrides.NotNull=Not annotated parameter overrides @{0} parameter