From 01b72f856a8942cd266ffb241c495cb1a45a84ec Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 27 Nov 2018 16:22:33 +0700 Subject: [PATCH] DFA: branch-specific reporting (IDEA-203016) --- .../dataFlow/DataFlowInspectionBase.java | 60 ++-- .../dataFlow/DataFlowInstructionVisitor.java | 2 +- .../dataFlow/NullabilityProblemKind.java | 302 ++++++++++++------ .../dataFlow/StandardInstructionVisitor.java | 2 +- .../afterTernary.java | 8 +- .../beforeTernary.java | 8 +- .../quickFix/surroundWithIf/afterTernary.java | 10 +- .../surroundWithIf/beforeTernary.java | 8 +- ...yComponentAndMethodAnnotationConflict.java | 13 +- .../dataFlow/fixture/OptionalInlining.java | 4 +- .../fixture/StreamTypeAnnoInlining.java | 4 +- .../SwitchExpressionsNullabilityJava12.java | 18 ++ .../dataFlow/fixture/TernaryNullability.java | 16 + .../DataFlowInspection12Test.java | 1 + .../DataFlowInspectionTest.java | 1 + 15 files changed, 308 insertions(+), 149 deletions(-) create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/SwitchExpressionsNullabilityJava12.java create mode 100644 java/java-tests/testData/inspection/dataFlow/fixture/TernaryNullability.java 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 0401bbdaee1e..8f8a97b27afd 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 @@ -42,6 +42,7 @@ import org.jetbrains.annotations.Nullable; import javax.swing.*; import java.text.MessageFormat; import java.util.*; +import java.util.function.Consumer; import static com.intellij.util.ObjectUtils.tryCast; @@ -448,28 +449,31 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool private void reportNullabilityProblems(ProblemReporter reporter, DataFlowInstructionVisitor visitor) { Map expressions = visitor.getConstantExpressions(); - visitor.problems().forEach(problem -> { + List> problems = NullabilityProblemKind.postprocessNullabilityProblems(visitor.problems().toList()); + for (NullabilityProblem problem : problems) { + PsiExpression expression = problem.getDereferencedExpression(); + Consumer standardReporter = expr -> reportNullabilityProblem(reporter, problem, expression, expressions); NullabilityProblemKind.innerClassNPE.ifMyProblem(problem, newExpression -> { List fixes = createNPEFixes(newExpression.getQualifier(), newExpression, reporter.isOnTheFly()); - reporter.registerProblem(getElementToHighlight(newExpression), problem.getMessage(expressions), fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); + reporter + .registerProblem(getElementToHighlight(newExpression), problem.getMessage(expressions), fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); }); NullabilityProblemKind.callMethodRefNPE.ifMyProblem(problem, methodRef -> reporter.registerProblem(methodRef, InspectionsBundle.message("dataflow.message.npe.methodref.invocation"), - createMethodReferenceNPEFixes(methodRef, reporter.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY))); + createMethodReferenceNPEFixes(methodRef, reporter.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY))); NullabilityProblemKind.callNPE.ifMyProblem(problem, call -> reportCallMayProduceNpe(reporter, problem.getMessage(expressions), call)); NullabilityProblemKind.passingToNotNullParameter.ifMyProblem(problem, expr -> { - PsiExpression expression = PsiUtil.skipParenthesizedExprDown(expr); List fixes = createNPEFixes(expression, expression, reporter.isOnTheFly()); - reporter.registerProblem(expr, problem.getMessage(expressions), fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); + reporter.registerProblem(expression, problem.getMessage(expressions), fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); }); NullabilityProblemKind.passingToNotNullMethodRefParameter.ifMyProblem(problem, methodRef -> { LocalQuickFix[] fixes = createMethodReferenceNPEFixes(methodRef, reporter.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY); reporter.registerProblem(methodRef, InspectionsBundle.message("dataflow.message.passing.nullable.argument.methodref"), fixes); }); - NullabilityProblemKind.arrayAccessNPE.ifMyProblem(problem, expression -> { + NullabilityProblemKind.arrayAccessNPE.ifMyProblem(problem, arrayAccess -> { LocalQuickFix[] fixes = - createNPEFixes(expression.getArrayExpression(), expression, reporter.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY); - reporter.registerProblem(expression, problem.getMessage(expressions), fixes); + createNPEFixes(arrayAccess.getArrayExpression(), arrayAccess, reporter.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY); + reporter.registerProblem(arrayAccess, problem.getMessage(expressions), fixes); }); NullabilityProblemKind.fieldAccessNPE.ifMyProblem(problem, element -> { PsiElement parent = element.getParent(); @@ -478,24 +482,25 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool reporter.registerProblem(element, problem.getMessage(expressions), fix); }); NullabilityProblemKind.unboxingNullable.ifMyProblem(problem, element -> { - if (element instanceof PsiTypeCastExpression && ((PsiTypeCastExpression)element).getType() instanceof PsiPrimitiveType) { - element = Objects.requireNonNull(((PsiTypeCastExpression)element).getOperand()); + PsiExpression anchor = expression; + if (anchor instanceof PsiTypeCastExpression && anchor.getType() instanceof PsiPrimitiveType) { + anchor = Objects.requireNonNull(((PsiTypeCastExpression)anchor).getOperand()); } - reporter.registerProblem(element, problem.getMessage(expressions)); + reporter.registerProblem(anchor, problem.getMessage(expressions)); }); NullabilityProblemKind.nullableFunctionReturn.ifMyProblem( - problem, expr -> reporter.registerProblem(expr, problem.getMessage(expressions))); - NullabilityProblemKind.assigningToNotNull.ifMyProblem(problem, expr -> reportNullabilityProblem(reporter, problem, expr, expressions)); - NullabilityProblemKind.storingToNotNullArray.ifMyProblem(problem, expr -> reportNullabilityProblem(reporter, problem, expr, expressions)); + problem, expr -> reporter.registerProblem(expression == null ? expr : expression, problem.getMessage(expressions))); + NullabilityProblemKind.assigningToNotNull.ifMyProblem(problem, standardReporter); + NullabilityProblemKind.storingToNotNullArray.ifMyProblem(problem, standardReporter); if (SUGGEST_NULLABLE_ANNOTATIONS) { NullabilityProblemKind.passingToNonAnnotatedMethodRefParameter.ifMyProblem( problem, methodRef -> reporter.registerProblem(methodRef, problem.getMessage(expressions))); NullabilityProblemKind.passingToNonAnnotatedParameter.ifMyProblem( - problem, expression -> reportNullableArgumentsPassedToNonAnnotated(reporter, problem.getMessage(expressions), expression)); + problem, top -> reportNullableArgumentsPassedToNonAnnotated(reporter, problem.getMessage(expressions), expression, top)); NullabilityProblemKind.assigningToNonAnnotatedField.ifMyProblem( - problem, expression -> reportNullableAssignedToNonAnnotatedField(reporter, expression, problem.getMessage(expressions))); + problem, top -> reportNullableAssignedToNonAnnotatedField(reporter, top, expression, problem.getMessage(expressions))); } - }); + } } private void reportNullabilityProblem(ProblemReporter reporter, @@ -604,20 +609,25 @@ public class DataFlowInspectionBase extends AbstractBaseJavaLocalInspectionTool }); } - private void reportNullableArgumentsPassedToNonAnnotated(ProblemReporter reporter, String message, PsiExpression anchor) { - PsiExpression expression = PsiUtil.skipParenthesizedExprDown(anchor); - PsiParameter parameter = MethodCallUtils.getParameterForArgument(anchor); + private void reportNullableArgumentsPassedToNonAnnotated(ProblemReporter reporter, + String message, + PsiExpression expression, + PsiExpression top) { + PsiParameter parameter = MethodCallUtils.getParameterForArgument(top); if (parameter != null && BaseIntentionAction.canModify(parameter) && AnnotationUtil.isAnnotatingApplicable(parameter)) { - List fixes = createNPEFixes(expression, expression, reporter.isOnTheFly()); + List fixes = createNPEFixes(expression, top, reporter.isOnTheFly()); fixes.add(AddAnnotationPsiFix.createAddNullableFix(parameter)); - reporter.registerProblem(anchor, message, fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); + reporter.registerProblem(expression, message, fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); } } - private void reportNullableAssignedToNonAnnotatedField(ProblemReporter reporter, PsiExpression expression, String message) { - PsiField field = getAssignedField(expression); + private void reportNullableAssignedToNonAnnotatedField(ProblemReporter reporter, + PsiExpression top, + PsiExpression expression, + String message) { + PsiField field = getAssignedField(top); if (field != null) { - List fixes = createNPEFixes(expression, expression, reporter.isOnTheFly()); + List fixes = createNPEFixes(expression, top, reporter.isOnTheFly()); fixes.add(AddAnnotationPsiFix.createAddNullableFix(field)); reporter.registerProblem(expression, message, fixes.toArray(LocalQuickFix.EMPTY_ARRAY)); } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java index d784455569f5..9b9c4f4cdf68 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/DataFlowInstructionVisitor.java @@ -261,7 +261,7 @@ final class DataFlowInstructionVisitor extends StandardInstructionVisitor { @Override protected boolean checkNotNullable(DfaMemoryState state, DfaValue value, @Nullable NullabilityProblemKind.NullabilityProblem problem) { - if (NullabilityProblemKind.nullableReturn.isMyProblem(problem) && !state.isNotNull(value)) { + if (problem != null && problem.getKind() == NullabilityProblemKind.nullableReturn && !state.isNotNull(value)) { myAlwaysReturnsNotNull = false; } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullabilityProblemKind.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullabilityProblemKind.java index 567a4800525a..8cbeb3e8f4a1 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullabilityProblemKind.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/NullabilityProblemKind.java @@ -7,7 +7,6 @@ import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.PsiTypesUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.psi.util.TypeConversionUtil; -import com.intellij.util.ObjectUtils; import com.siyeh.ig.psiutils.ExpectedTypeUtils; import com.siyeh.ig.psiutils.ExpressionUtils; import com.siyeh.ig.psiutils.MethodCallUtils; @@ -18,11 +17,12 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.PropertyKey; -import java.util.Map; -import java.util.Objects; +import java.util.*; import java.util.function.Consumer; +import java.util.function.Function; import static com.intellij.codeInspection.InspectionsBundle.BUNDLE; +import static com.intellij.util.ObjectUtils.tryCast; /** * Represents a kind of nullability problem @@ -127,16 +127,6 @@ public class NullabilityProblemKind { return problem != null && problem.myKind == this ? (NullabilityProblem)problem : null; } - /** - * Returns true if the kind of supplied problem is the same as this kind - * - * @param problem problem to check - * @return true if the kind of supplied problem is the same as this kind - */ - public final boolean isMyProblem(@Nullable NullabilityProblem problem) { - return problem != null && problem.myKind == this; - } - /** * Executes given consumer if the supplied problem has the same kind as this kind * @@ -199,74 +189,18 @@ public class NullabilityProblemKind { } } if (parent instanceof PsiAssignmentExpression) { - PsiAssignmentExpression assignment = (PsiAssignmentExpression)parent; - IElementType tokenType = assignment.getOperationTokenType(); - if (assignment.getRExpression() == context) { - PsiExpression lho = PsiUtil.skipParenthesizedExprDown(assignment.getLExpression()); - if (lho != null) { - PsiType type = lho.getType(); - if (tokenType.equals(JavaTokenType.PLUSEQ) && TypeUtils.isJavaLangString(type)) { - return null; - } - if (type instanceof PsiPrimitiveType) { - return createUnboxingProblem(context, expression); - } - Nullability nullability = Nullability.UNKNOWN; - PsiVariable target = null; - if (lho instanceof PsiReferenceExpression) { - target = ObjectUtils.tryCast(((PsiReferenceExpression)lho).resolve(), PsiVariable.class); - if (target != null) { - nullability = DfaPsiUtil.getElementNullability(type, target); - } - } else { - nullability = DfaPsiUtil.getTypeNullability(type); - } - boolean forceDeclaredNullity = !(target instanceof PsiParameter && target.getParent() instanceof PsiParameterList); - if (forceDeclaredNullity && nullability == Nullability.NOT_NULL) { - return (lho instanceof PsiArrayAccessExpression ? storingToNotNullArray : assigningToNotNull).problem(context, expression); - } - if (nullability == Nullability.UNKNOWN && lho instanceof PsiReferenceExpression) { - PsiField field = ObjectUtils.tryCast(((PsiReferenceExpression)lho).resolve(), PsiField.class); - if (field != null && !field.hasModifierProperty(PsiModifier.FINAL)) { - return assigningToNonAnnotatedField.problem(context, expression); - } - } - } - } + return getAssignmentProblem((PsiAssignmentExpression)parent, expression, context); } if (parent instanceof PsiExpressionList) { - if (parent.getParent() instanceof PsiSwitchLabelStatementBase) { - return fieldAccessNPE.problem(context, expression); - } - PsiParameter parameter = MethodCallUtils.getParameterForArgument(context); - if (parameter != null) { - if (parameter.getType() instanceof PsiPrimitiveType) { - return createUnboxingProblem(context, expression); - } - PsiElement grandParent = parent.getParent(); - if (grandParent instanceof PsiAnonymousClass) { - grandParent = grandParent.getParent(); - } - if (grandParent instanceof PsiCall) { - PsiSubstitutor substitutor = ((PsiCall)grandParent).resolveMethodGenerics().getSubstitutor(); - Nullability nullability = DfaPsiUtil.getElementNullability(substitutor.substitute(parameter.getType()), parameter); - if (nullability == Nullability.NOT_NULL) { - return passingToNotNullParameter.problem(context, expression); - } - if (nullability == Nullability.UNKNOWN) { - return passingToNonAnnotatedParameter.problem(context, expression); - } - } - } + return getExpressionListProblem((PsiExpressionList)parent, expression, context); } - if (parent instanceof PsiIfStatement || - parent instanceof PsiWhileStatement || - parent instanceof PsiDoWhileStatement || - parent instanceof PsiUnaryExpression || - parent instanceof PsiConditionalExpression || - parent instanceof PsiTypeCastExpression || - parent instanceof PsiForStatement && ((PsiForStatement)parent).getCondition() == context || - parent instanceof PsiAssertStatement && ((PsiAssertStatement)parent).getAssertCondition() == context) { + if (parent instanceof PsiArrayInitializerExpression) { + return getArrayInitializerProblem((PsiArrayInitializerExpression)parent, expression, context); + } + if (parent instanceof PsiIfStatement || parent instanceof PsiWhileStatement || parent instanceof PsiDoWhileStatement || + parent instanceof PsiUnaryExpression || parent instanceof PsiConditionalExpression || parent instanceof PsiTypeCastExpression || + (parent instanceof PsiForStatement && ((PsiForStatement)parent).getCondition() == context) || + (parent instanceof PsiAssertStatement && ((PsiAssertStatement)parent).getAssertCondition() == context)) { return createUnboxingProblem(context, expression); } if (parent instanceof PsiSwitchBlock) { @@ -290,27 +224,6 @@ public class NullabilityProblemKind { return createUnboxingProblem(context, expression); } } - if (parent instanceof PsiArrayInitializerExpression) { - PsiType type = ((PsiArrayInitializerExpression)parent).getType(); - if (type instanceof PsiArrayType) { - PsiType componentType = ((PsiArrayType)type).getComponentType(); - if (TypeConversionUtil.isPrimitiveAndNotNull(componentType)) { - return createUnboxingProblem(context, expression); - } - Nullability nullability = DfaPsiUtil.getTypeNullability(componentType); - if (nullability == Nullability.UNKNOWN) { - if (parent.getParent() instanceof PsiNewExpression) { - PsiType expectedType = ExpectedTypeUtils.findExpectedType((PsiExpression)parent.getParent(), false); - if (expectedType instanceof PsiArrayType) { - nullability = DfaPsiUtil.getTypeNullability(((PsiArrayType)expectedType).getComponentType()); - } - } - } - if (nullability == Nullability.NOT_NULL) { - return storingToNotNullArray.problem(context, expression); - } - } - } if (parent instanceof PsiArrayAccessExpression) { PsiArrayAccessExpression arrayAccessExpression = (PsiArrayAccessExpression)parent; if (arrayAccessExpression.getArrayExpression() == context) { @@ -321,6 +234,103 @@ public class NullabilityProblemKind { return null; } + @Nullable + private static NullabilityProblem getExpressionListProblem(@NotNull PsiExpressionList expressionList, + @NotNull PsiExpression expression, + @NotNull PsiExpression context) { + if (expressionList.getParent() instanceof PsiSwitchLabelStatementBase) { + return fieldAccessNPE.problem(context, expression); + } + PsiParameter parameter = MethodCallUtils.getParameterForArgument(context); + if (parameter != null) { + if (parameter.getType() instanceof PsiPrimitiveType) { + return createUnboxingProblem(context, expression); + } + PsiElement grandParent = expressionList.getParent(); + if (grandParent instanceof PsiAnonymousClass) { + grandParent = grandParent.getParent(); + } + if (grandParent instanceof PsiCall) { + PsiSubstitutor substitutor = ((PsiCall)grandParent).resolveMethodGenerics().getSubstitutor(); + Nullability nullability = DfaPsiUtil.getElementNullability(substitutor.substitute(parameter.getType()), parameter); + if (nullability == Nullability.NOT_NULL) { + return passingToNotNullParameter.problem(context, expression); + } + if (nullability == Nullability.UNKNOWN) { + return passingToNonAnnotatedParameter.problem(context, expression); + } + } + } + return null; + } + + @Nullable + private static NullabilityProblem getArrayInitializerProblem(@NotNull PsiArrayInitializerExpression initializer, + @NotNull PsiExpression expression, + @NotNull PsiExpression context) { + PsiType type = initializer.getType(); + if (type instanceof PsiArrayType) { + PsiType componentType = ((PsiArrayType)type).getComponentType(); + if (TypeConversionUtil.isPrimitiveAndNotNull(componentType)) { + return createUnboxingProblem(context, expression); + } + Nullability nullability = DfaPsiUtil.getTypeNullability(componentType); + if (nullability == Nullability.UNKNOWN) { + if (initializer.getParent() instanceof PsiNewExpression) { + PsiType expectedType = ExpectedTypeUtils.findExpectedType((PsiExpression)initializer.getParent(), false); + if (expectedType instanceof PsiArrayType) { + nullability = DfaPsiUtil.getTypeNullability(((PsiArrayType)expectedType).getComponentType()); + } + } + } + if (nullability == Nullability.NOT_NULL) { + return storingToNotNullArray.problem(context, expression); + } + } + return null; + } + + @Nullable + private static NullabilityProblem getAssignmentProblem(@NotNull PsiAssignmentExpression assignment, + @NotNull PsiExpression expression, + @NotNull PsiExpression context) { + IElementType tokenType = assignment.getOperationTokenType(); + if (assignment.getRExpression() == context) { + PsiExpression lho = PsiUtil.skipParenthesizedExprDown(assignment.getLExpression()); + if (lho != null) { + PsiType type = lho.getType(); + if (tokenType.equals(JavaTokenType.PLUSEQ) && TypeUtils.isJavaLangString(type)) { + return null; + } + if (type instanceof PsiPrimitiveType) { + return createUnboxingProblem(context, expression); + } + Nullability nullability = Nullability.UNKNOWN; + PsiVariable target = null; + if (lho instanceof PsiReferenceExpression) { + target = tryCast(((PsiReferenceExpression)lho).resolve(), PsiVariable.class); + if (target != null) { + nullability = DfaPsiUtil.getElementNullability(type, target); + } + } + else { + nullability = DfaPsiUtil.getTypeNullability(type); + } + boolean forceDeclaredNullity = !(target instanceof PsiParameter && target.getParent() instanceof PsiParameterList); + if (forceDeclaredNullity && nullability == Nullability.NOT_NULL) { + return (lho instanceof PsiArrayAccessExpression ? storingToNotNullArray : assigningToNotNull).problem(context, expression); + } + if (nullability == Nullability.UNKNOWN && lho instanceof PsiReferenceExpression) { + PsiField field = tryCast(((PsiReferenceExpression)lho).resolve(), PsiField.class); + if (field != null && !field.hasModifierProperty(PsiModifier.FINAL)) { + return assigningToNonAnnotatedField.problem(context, expression); + } + } + } + } + return null; + } + /** * Looks for top expression with the same nullability as given expression. That is: skips casts or conditionals, which don't unbox; * goes up from switch expression breaks or expression-branches. @@ -329,7 +339,7 @@ public class NullabilityProblemKind { * @return the top expression */ @NotNull - private static PsiExpression findTopExpression(@NotNull PsiExpression expression) { + static PsiExpression findTopExpression(@NotNull PsiExpression expression) { PsiExpression context = expression; while (true) { PsiElement parent = context.getParent(); @@ -368,16 +378,90 @@ public class NullabilityProblemKind { return unboxingNullable.problem(context, expression); } + static List> postprocessNullabilityProblems(Collection> problems) { + List> unchanged = new ArrayList<>(); + Map> expressionToProblem = new HashMap<>(); + for (NullabilityProblem problem : problems) { + PsiExpression expression = problem.getDereferencedExpression(); + NullabilityProblemKind kind = problem.getKind(); + if (expression == null) { + unchanged.add(problem); + continue; + } + if (innerClassNPE == kind || callNPE == kind || arrayAccessNPE == kind || fieldAccessNPE == kind) { + // Qualifier-problems are reported on top-expression level for now as it's rare case to have + // something complex in qualifier and we highlight not the qualifier itself, but something else (e.g. called method name) + unchanged.add(problem.withExpression(findTopExpression(expression))); + continue; + } + // Merge ternary problems reported for both branches into single problem + while (true) { + PsiExpression top = skipParenthesesAndObjectCastsUp(expression); + PsiConditionalExpression ternary = tryCast(top.getParent(), PsiConditionalExpression.class); + if (ternary != null) { + PsiExpression otherBranch = null; + if (ternary.getThenExpression() == top) { + otherBranch = ternary.getElseExpression(); + } + else if (ternary.getElseExpression() == top) { + otherBranch = ternary.getThenExpression(); + } + if (otherBranch != null) { + otherBranch = skipParenthesesAndObjectCastsDown(otherBranch); + NullabilityProblem otherBranchProblem = expressionToProblem.remove(otherBranch); + if (otherBranchProblem != null) { + expression = ternary; + problem = problem.withExpression(ternary); + continue; + } + } + } + break; + } + expressionToProblem.put(expression, problem); + } + return StreamEx.of(unchanged, expressionToProblem.values()).toFlatList(Function.identity()); + } + + private static PsiExpression skipParenthesesAndObjectCastsDown(PsiExpression expression) { + while (true) { + if (expression instanceof PsiParenthesizedExpression) { + expression = ((PsiParenthesizedExpression)expression).getExpression(); + } + else if (expression instanceof PsiTypeCastExpression && !(expression.getType() instanceof PsiPrimitiveType)) { + expression = ((PsiTypeCastExpression)expression).getOperand(); + } + else { + return expression; + } + } + } + + @NotNull + private static PsiExpression skipParenthesesAndObjectCastsUp(PsiExpression expression) { + PsiExpression top = expression; + while (true) { + PsiElement parent = top.getParent(); + if (parent instanceof PsiParenthesizedExpression || + (parent instanceof PsiTypeCastExpression && !(((PsiTypeCastExpression)parent).getType() instanceof PsiPrimitiveType))) { + top = (PsiExpression)parent; + } + else { + return top; + } + } + } + /** * Represents a concrete nullability problem on PSI which consists of PSI element (anchor) and {@link NullabilityProblemKind}. * @param a type of anchor element */ public static final class NullabilityProblem { - private final @NotNull NullabilityProblemKind myKind; + private final @NotNull NullabilityProblemKind myKind; private final @NotNull T myAnchor; private final @Nullable PsiExpression myDereferencedExpression; - NullabilityProblem(@NotNull NullabilityProblemKind kind, @NotNull T anchor, @Nullable PsiExpression dereferencedExpression) { + NullabilityProblem(@NotNull NullabilityProblemKind kind, @NotNull T anchor, @Nullable PsiExpression dereferencedExpression) { myKind = kind; myAnchor = anchor; myDereferencedExpression = dereferencedExpression; @@ -419,22 +503,32 @@ public class NullabilityProblemKind { return myKind.myNormalMessage; } + @NotNull + public NullabilityProblemKind getKind() { + return myKind; + } + @Override public boolean equals(Object o) { if (this == o) return true; if (!(o instanceof NullabilityProblem)) return false; NullabilityProblem problem = (NullabilityProblem)o; - return myKind.equals(problem.myKind) && myAnchor.equals(problem.myAnchor); + return myKind.equals(problem.myKind) && myAnchor.equals(problem.myAnchor) && + Objects.equals(myDereferencedExpression, problem.myDereferencedExpression); } @Override public int hashCode() { - return Objects.hash(myKind, myAnchor); + return Objects.hash(myKind, myAnchor, myDereferencedExpression); } @Override public String toString() { return "[" + myKind + "] " + myAnchor.getText(); } + + public NullabilityProblem withExpression(PsiExpression expression) { + return expression == myDereferencedExpression ? this : new NullabilityProblem<>(myKind, myAnchor, expression); + } } } diff --git a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java index 512b5eaad382..19c5c1cf76f3 100644 --- a/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java +++ b/java/java-analysis-impl/src/com/intellij/codeInspection/dataFlow/StandardInstructionVisitor.java @@ -482,7 +482,7 @@ public class StandardInstructionVisitor extends InstructionVisitor { return ((DfaFactMapValue)value).withFact(DfaFactType.NULLABILITY, DfaNullability.NOT_NULL); } if (ok) return value; - if (memState.isNull(value) && NullabilityProblemKind.nullableFunctionReturn.isMyProblem(problem)) { + if (memState.isNull(value) && problem != null && problem.getKind() == NullabilityProblemKind.nullableFunctionReturn) { return value.getFactory().getFactValue(DfaFactType.NULLABILITY, DfaNullability.NOT_NULL); } if (value instanceof DfaVariableValue) { diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/afterTernary.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/afterTernary.java index e2f7e3e2b49a..722b8f16f307 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/afterTernary.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/afterTernary.java @@ -1,8 +1,12 @@ -// "Replace with '(b ? null : "foo") != null ?:'" "true" +// "Replace with '(b ? foo(1) : foo(2)) != null ?:'" "true" class A { void bar(String s) {} void foo(boolean b){ - bar((b ? null : "foo") != null ? b ? null : "foo" : null); + bar((b ? foo(1) : foo(2)) != null ? b ? foo(1) : foo(2) : null); + } + + static String foo(int x) { + return x > 0 ? "pos" : x < 0 ? "neg" : null; } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/beforeTernary.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/beforeTernary.java index 413b3d1b3f95..c1b13bf791f7 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/beforeTernary.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/replaceWithTernaryOperator/beforeTernary.java @@ -1,8 +1,12 @@ -// "Replace with '(b ? null : "foo") != null ?:'" "true" +// "Replace with '(b ? foo(1) : foo(2)) != null ?:'" "true" class A { void bar(String s) {} void foo(boolean b){ - bar(b ? null : "foo"); + bar(b ? foo(1) : foo(2)); + } + + static String foo(int x) { + return x > 0 ? "pos" : x < 0 ? "neg" : null; } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/afterTernary.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/afterTernary.java index 862486a4b834..709c5515cf11 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/afterTernary.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/afterTernary.java @@ -1,10 +1,14 @@ -// "Surround with 'if ((b ? null : "foo") != null)'" "true" +// "Surround with 'if ((b ? foo(1) : foo(2)) != null)'" "true" class A { void bar(String s) {} void foo(boolean b){ - if ((b ? null : "foo") != null) { - bar(b ? null : "foo"); + if ((b ? foo(1) : foo(2)) != null) { + bar(b ? foo(1) : foo(2)); } } + + static String foo(int x) { + return x > 0 ? "pos" : x < 0 ? "neg" : null; + } } \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/beforeTernary.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/beforeTernary.java index 6e7bc5706bf5..1eb3936895e5 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/beforeTernary.java +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/surroundWithIf/beforeTernary.java @@ -1,8 +1,12 @@ -// "Surround with 'if ((b ? null : "foo") != null)'" "true" +// "Surround with 'if ((b ? foo(1) : foo(2)) != null)'" "true" class A { void bar(String s) {} void foo(boolean b){ - bar(b ? null : "foo"); + bar(b ? foo(1) : foo(2)); + } + + static String foo(int x) { + return x > 0 ? "pos" : x < 0 ? "neg" : null; } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/ArrayComponentAndMethodAnnotationConflict.java b/java/java-tests/testData/inspection/dataFlow/fixture/ArrayComponentAndMethodAnnotationConflict.java index 71c08d67d7ae..82e041dcd8ab 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/ArrayComponentAndMethodAnnotationConflict.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/ArrayComponentAndMethodAnnotationConflict.java @@ -19,17 +19,20 @@ class FooImpl implements Foo { return null; } else if (Math.random() > 0.5) { - @NotNull Object @Nullable [] arr1 = new Object[]{null, new Object(), Math.random() > 0.5 ? new Object() : null}; - @NotNull Object @Nullable [] arr2 = {null, new Object(), Math.random() > 0.5 ? new Object() : null}; - return new Object[]{null, new Object(), Math.random() > 0.5 ? new Object() : null}; + @NotNull Object @Nullable [] arr1 = new Object[]{null, new Object(), Math.random() > 0.5 ? new Object() : null, foo()}; + @NotNull Object @Nullable [] arr2 = {null, new Object(), Math.random() > 0.5 ? new Object() : null, foo()}; + return new Object[]{null, new Object(), Math.random() > 0.5 ? new Object() : null, foo()}; } - return new @NotNull Object @Nullable []{null, new Object(), Math.random() > 0.5 ? new Object() : null}; + return new @NotNull Object @Nullable []{null, new Object(), Math.random() > 0.5 ? new Object() : null, foo()}; } void test() { @NotNull Object @Nullable [] array = getNullableArrayOfNotNullObjects(); assert array != null; array[0] = null; - array[1] = Math.random() > 0.5 ? null : "foo"; + array[1] = Math.random() > 0.5 ? null : "foo"; + array[3] = foo(); } + + native @Nullable Object foo(); } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java index b1bd8c4277ae..85fafc1c047d 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/OptionalInlining.java @@ -134,7 +134,7 @@ public class OptionalInlining { System.out.println("impossible"); } if(opt.isPresent()) { - if(opt.transform(x -> x.isEmpty() ? null : x).toJavaUtil().isPresent()) { + if(opt.transform(x -> x.isEmpty() ? null : x).toJavaUtil().isPresent()) { System.out.println("Always"); } if(opt.toJavaUtil().map(x -> x.isEmpty() ? null : x).isPresent()) { @@ -157,7 +157,7 @@ public class OptionalInlining { opt.flatMap(null); opt.flatMap(x -> null); opt.flatMap(this::nullableOptionalMethod); - opt.flatMap(x -> x.isEmpty() ? null : Optional.of(x)); + opt.flatMap(x -> x.isEmpty() ? null : Optional.of(x)); opt.flatMap(x -> { if (x.isEmpty()) { return null; diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/StreamTypeAnnoInlining.java b/java/java-tests/testData/inspection/dataFlow/fixture/StreamTypeAnnoInlining.java index 46a5eb730d72..59b123cea8e0 100644 --- a/java/java-tests/testData/inspection/dataFlow/fixture/StreamTypeAnnoInlining.java +++ b/java/java-tests/testData/inspection/dataFlow/fixture/StreamTypeAnnoInlining.java @@ -8,12 +8,12 @@ import java.util.stream.*; public class StreamTypeAnnoInlining { void testToArray() { @NotNull Object @NotNull[] foo0 = Stream.of("a", "b") - .map(x-> "a".equals(x) ? null : x.toUpperCase()) + .map(x-> "a".equals(x) ? null : x.toUpperCase()) .toArray(); // IDEA-194697 @NotNull String @NotNull[] foo = Stream.of("a", "b") - .map(x-> "a".equals(x) ? null : x.toUpperCase()) + .map(x-> "a".equals(x) ? null : x.toUpperCase()) .toArray(String[]::new); @NotNull String @NotNull[] foo1 = Stream.of("b", "c") diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/SwitchExpressionsNullabilityJava12.java b/java/java-tests/testData/inspection/dataFlow/fixture/SwitchExpressionsNullabilityJava12.java new file mode 100644 index 000000000000..9f79b8a3d6ac --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/SwitchExpressionsNullabilityJava12.java @@ -0,0 +1,18 @@ +import org.jetbrains.annotations.*; + +public class SwitchExpressionsNullabilityJava12 { + void cons(@NotNull String str) {} + + void test(@Nullable String a, @Nullable String b, int i, boolean f) { + cons(((String)(switch(i) { + case 1 -> a; + case 2 -> "foo"; + case 3 -> null; + case 4 -> { + System.out.println("four!"); + break f ? i : b; + } + default -> "bar"; + }))); + } +} diff --git a/java/java-tests/testData/inspection/dataFlow/fixture/TernaryNullability.java b/java/java-tests/testData/inspection/dataFlow/fixture/TernaryNullability.java new file mode 100644 index 000000000000..1ffdc2122a24 --- /dev/null +++ b/java/java-tests/testData/inspection/dataFlow/fixture/TernaryNullability.java @@ -0,0 +1,16 @@ +import org.jetbrains.annotations.*; + +final class MyClass { + void cons(Object x) {} + + void test(@Nullable Object a, @Nullable Object b, @Nullable Object c, boolean f1, boolean f2) { + cons(f1 ? a : b); + cons(f1 ? a : new Object()); + cons(f1 ? new Object() : a); + cons(f1 ? f2 ? a : (Object)b : new Object()); + cons(f1 ? f2 ? a : new Object() : b); + cons(f1 ? f2 ? new Object() : a : b); + cons(f1 ? f2 ? a : b : ((Object)c)); + cons(f1 ? f2 ? a : b : f2 ? c : new Object()); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection12Test.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection12Test.java index 11e4d17a7bda..839b10b2f124 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection12Test.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspection12Test.java @@ -40,4 +40,5 @@ public class DataFlowInspection12Test extends DataFlowInspectionTestCase { public void testSwitchStatementsJava12() { doTest(); } public void testSwitchExpressionsJava12() { doTest(); } + public void testSwitchExpressionsNullabilityJava12() { doTest(); } } \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java index 9cf057cedc1b..c315879a4919 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/DataFlowInspectionTest.java @@ -669,4 +669,5 @@ public class DataFlowInspectionTest extends DataFlowInspectionTestCase { public void testAssertNullEphemeral() { doTest(); } public void testNotNullAnonymousConstructor() { doTest(); } public void testCaughtNPE() { doTest(); } + public void testTernaryNullability() { doTest(); } }