diff --git a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaVariableState.java b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaVariableState.java index 935ca9d9dcbe..ae8c86692207 100644 --- a/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaVariableState.java +++ b/java/java-impl/src/com/intellij/codeInspection/dataFlow/DfaVariableState.java @@ -27,14 +27,15 @@ package com.intellij.codeInspection.dataFlow; import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInspection.dataFlow.value.DfaTypeValue; import com.intellij.codeInspection.dataFlow.value.DfaValue; -import com.intellij.psi.PsiPrimitiveType; -import com.intellij.psi.PsiVariable; +import com.intellij.codeInspection.nullable.NullableStuffInspection; +import com.intellij.psi.*; import gnu.trove.THashSet; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.Nullable; import java.util.HashSet; import java.util.Iterator; +import java.util.List; import java.util.Set; public class DfaVariableState implements Cloneable { @@ -48,8 +49,36 @@ public class DfaVariableState implements Cloneable { myVar = var; myInstanceofValues = new HashSet(); myNotInstanceofValues = new HashSet(); - myNullable = var != null && NullableNotNullManager.isNullable(var); - myVariableIsDeclaredNotNull = var != null && NullableNotNullManager.isNotNull(var); + myNullable = var != null && (NullableNotNullManager.isNullable(var) || isNullableInitialized(var, true)); + myVariableIsDeclaredNotNull = var != null && (NullableNotNullManager.isNotNull(var) || isNullableInitialized(var, false)); + } + + private static boolean isNullableInitialized(PsiVariable var, boolean nullable) { + if (!var.hasModifierProperty(PsiModifier.FINAL) || !(var instanceof PsiField)) { + return false; + } + + List initializers = NullableStuffInspection.findAllConstructorInitializers((PsiField)var); + if (!nullable && initializers.isEmpty()) { + return false; + } + + for (PsiExpression expression : initializers) { + if (!(expression instanceof PsiReferenceExpression)) { + return false; + } + PsiElement target = ((PsiReferenceExpression)expression).resolve(); + if (!(target instanceof PsiParameter)) { + return false; + } + if (nullable && NullableNotNullManager.isNullable((PsiParameter)target)) { + return true; + } + if (!nullable && !NullableNotNullManager.isNotNull((PsiParameter)target)) { + return false; + } + } + return true; } protected DfaVariableState(final DfaVariableState toClone) { 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 aef21b63a3bf..a37eaa2c0e68 100644 --- a/java/java-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/nullable/NullableStuffInspection.java @@ -42,7 +42,6 @@ import com.intellij.psi.util.*; import com.intellij.refactoring.psi.PropertyUtils; import com.intellij.util.ArrayUtil; import com.intellij.util.Processor; -import com.intellij.util.Query; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -50,8 +49,11 @@ import javax.swing.*; import java.awt.*; import java.awt.event.ActionEvent; import java.awt.event.ActionListener; +import java.util.ArrayList; import java.util.List; +import static com.intellij.util.containers.ContainerUtil.addIfNotNull; + public class NullableStuffInspection extends BaseLocalInspectionTool { // deprecated fields remain to minimize changes to users inspection profiles (which are often located in version control). @Deprecated @SuppressWarnings({"WeakerAccess"}) public boolean REPORT_NULLABLE_METHOD_OVERRIDES_NOTNULL = true; @@ -176,38 +178,11 @@ public class NullableStuffInspection extends BaseLocalInspectionTool { } } - if (containingClass != null) { - 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; - } + for (PsiExpression rhs : findAllConstructorInitializers(field)) { + if (rhs instanceof PsiReferenceExpression) { + PsiElement target = ((PsiReferenceExpression)rhs).resolve(); + if (target instanceof PsiParameter) { + PsiParameter parameter = (PsiParameter)target; if (REPORT_NOT_ANNOTATED_GETTER && !AnnotationUtil.isAnnotated(parameter, manager.getAllAnnotations()) && !TypeConversionUtil.isPrimitiveAndNotNull(parameter.getType())) { final PsiIdentifier nameIdentifier2 = parameter.getNameIdentifier(); assert nameIdentifier2 != null : parameter; @@ -215,7 +190,7 @@ public class NullableStuffInspection extends BaseLocalInspectionTool { .message("inspection.nullable.problems.annotated.field.constructor.parameter.not.annotated", StringUtil.getShortName(anno)), ProblemHighlightType.GENERIC_ERROR_OR_WARNING, new AddAnnotationFix(anno, parameter, ArrayUtil.toStringArray(annoToRemove))); - return true; + continue; } if (annotated.isDeclaredNotNull && manager.isNullable(parameter, false)) { final PsiIdentifier nameIdentifier2 = parameter.getNameIdentifier(); @@ -235,9 +210,9 @@ public class NullableStuffInspection extends BaseLocalInspectionTool { ProblemHighlightType.GENERIC_ERROR_OR_WARNING, new AddAnnotationFix(anno, parameter, ArrayUtil.toStringArray(annoToRemove))); } - return true; + } - }); + } } } } @@ -247,19 +222,6 @@ public class NullableStuffInspection extends BaseLocalInspectionTool { LOG.assertTrue(parameter.isPhysical(), setter.getText()); } - @Nullable - 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()); @@ -521,4 +483,42 @@ public class NullableStuffInspection extends BaseLocalInspectionTool { REPORT_NULLS_PASSED_TO_NON_ANNOTATED_METHOD = myReportNullsPassedToNonAnnotatedParameter.isSelected(); } } + + public static List findAllConstructorInitializers(PsiField field) { + final List result = new ArrayList(); + addIfNotNull(result, field.getInitializer()); + + PsiClass containingClass = field.getContainingClass(); + if (containingClass != null) { + LocalSearchScope scope = new LocalSearchScope(containingClass.getConstructors()); + ReferencesSearch.search(field, scope, false).forEach(new Processor() { + @Override + public boolean process(PsiReference reference) { + final PsiElement element = reference.getElement(); + if (element instanceof PsiReferenceExpression) { + final PsiAssignmentExpression assignment = getAssignmentExpressionIfOnAssignmentLhs(element); + final PsiMethod method = PsiTreeUtil.getParentOfType(assignment, PsiMethod.class); + if (method != null && method.isConstructor() && assignment != null) { + addIfNotNull(result, assignment.getRExpression()); + } + } + return true; + } + }); + } + return result; + } + + @Nullable + private static PsiAssignmentExpression getAssignmentExpressionIfOnAssignmentLhs(PsiElement 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; + } } diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/FinalFieldsInitializedByAnnotatedParameters.java b/java/java-tests/testData/inspection/dataFlow/fixture/FinalFieldsInitializedByAnnotatedParameters.java new file mode 100644 index 000000000000..d06035d16656 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/FinalFieldsInitializedByAnnotatedParameters.java @@ -0,0 +1,21 @@ +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +public class Doo { + private final Object myA; + private final Object myB; + + public Doo(@Nullable Object myA, @NotNull Object myB) { + this.myA = myA; + this.myB = myB; + } + + int foo() { + if (myB != null && + myA.equals(myB)) { + return 2; + } + + return myA.hashCode(); + } +} diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java index 8202b8371805..cd6c67d36e57 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/DataFlowInspectionFixtureTest.java @@ -65,5 +65,6 @@ public class DataFlowInspectionFixtureTest extends JavaCodeInsightFixtureTestCas public void testStringTernaryAlwaysTrue() throws Throwable { doTest(); } public void testBoxing128() throws Throwable { doTest(); } + public void testFinalFieldsInitializedByAnnotatedParameters() throws Throwable { doTest(); } }