[java-dfa] Remove useless fixes when value is known to be always null

Fixes IDEA-289497 'NullPointerException' recommendations contradiction

GitOrigin-RevId: 07be3f2ee5ce03bd7380b563806fcad03fb2a0f5
This commit is contained in:
Tagir Valeev
2022-03-03 10:49:26 +00:00
committed by intellij-monorepo-bot
parent 3f8722ac48
commit 4a9b9b3cc2
13 changed files with 58 additions and 48 deletions
@@ -235,7 +235,10 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
return Collections.emptyList();
}
protected @NotNull List<LocalQuickFix> createNPEFixes(@Nullable PsiExpression qualifier, PsiExpression expression, boolean onTheFly) {
protected @NotNull List<LocalQuickFix> createNPEFixes(@Nullable PsiExpression qualifier,
PsiExpression expression,
boolean onTheFly,
boolean alwaysNull) {
return Collections.emptyList();
}
@@ -613,17 +616,19 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
}
// Expression of null type: could be failed LVTI, skip it to avoid confusion
if (expression != null && !nullLiteral && PsiType.NULL.equals(expression.getType())) continue;
boolean alwaysNull = problem.isAlwaysNull(expressions);
NullabilityProblemKind.innerClassNPE.ifMyProblem(problem, newExpression -> {
List<LocalQuickFix> fixes = createNPEFixes(newExpression.getQualifier(), newExpression, reporter.isOnTheFly());
List<LocalQuickFix> fixes = createNPEFixes(newExpression.getQualifier(), newExpression, reporter.isOnTheFly(), alwaysNull);
reporter
.registerProblem(getElementToHighlight(newExpression), problem.getMessage(expressions), fixes.toArray(LocalQuickFix.EMPTY_ARRAY));
});
NullabilityProblemKind.callMethodRefNPE.ifMyProblem(problem, methodRef ->
reporter.registerProblem(methodRef, JavaAnalysisBundle.message("dataflow.message.npe.methodref.invocation"),
createMethodReferenceNPEFixes(methodRef, reporter.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY)));
NullabilityProblemKind.callNPE.ifMyProblem(problem, call -> reportCallMayProduceNpe(reporter, problem.getMessage(expressions), call));
NullabilityProblemKind.callNPE.ifMyProblem(problem, call ->
reportCallMayProduceNpe(reporter, problem.getMessage(expressions), call, alwaysNull));
NullabilityProblemKind.passingToNotNullParameter.ifMyProblem(problem, expr -> {
List<LocalQuickFix> fixes = createNPEFixes(expression, expression, reporter.isOnTheFly());
List<LocalQuickFix> fixes = createNPEFixes(expression, expression, reporter.isOnTheFly(), alwaysNull);
reporter.registerProblem(expression, problem.getMessage(expressions), fixes.toArray(LocalQuickFix.EMPTY_ARRAY));
});
NullabilityProblemKind.passingToNotNullMethodRefParameter.ifMyProblem(problem, methodRef -> {
@@ -635,14 +640,14 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
reporter.registerProblem(methodRef, JavaAnalysisBundle.message("dataflow.message.unboxing.nullable.argument.methodref"), fixes);
});
NullabilityProblemKind.arrayAccessNPE.ifMyProblem(problem, arrayAccess -> {
LocalQuickFix[] fixes =
createNPEFixes(arrayAccess.getArrayExpression(), arrayAccess, reporter.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY);
LocalQuickFix[] fixes = createNPEFixes(arrayAccess.getArrayExpression(), arrayAccess, reporter.isOnTheFly(),
alwaysNull).toArray(LocalQuickFix.EMPTY_ARRAY);
reporter.registerProblem(arrayAccess, problem.getMessage(expressions), fixes);
});
NullabilityProblemKind.fieldAccessNPE.ifMyProblem(problem, element -> {
PsiElement parent = element.getParent();
PsiExpression fieldAccess = parent instanceof PsiReferenceExpression ? (PsiExpression)parent : element;
LocalQuickFix[] fix = createNPEFixes(element, fieldAccess, reporter.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY);
LocalQuickFix[] fix = createNPEFixes(element, fieldAccess, reporter.isOnTheFly(), alwaysNull).toArray(LocalQuickFix.EMPTY_ARRAY);
reporter.registerProblem(element, problem.getMessage(expressions), fix);
});
NullabilityProblemKind.unboxingNullable.ifMyProblem(problem, element -> {
@@ -664,9 +669,9 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
NullabilityProblemKind.passingToNonAnnotatedMethodRefParameter.ifMyProblem(
problem, methodRef -> reportNullableArgumentPassedToNonAnnotatedMethodRef(reporter, expressions, problem, methodRef));
NullabilityProblemKind.passingToNonAnnotatedParameter.ifMyProblem(
problem, top -> reportNullableArgumentsPassedToNonAnnotated(reporter, problem.getMessage(expressions), expression, top));
problem, top -> reportNullableArgumentsPassedToNonAnnotated(reporter, problem.getMessage(expressions), expression, top, alwaysNull));
NullabilityProblemKind.assigningToNonAnnotatedField.ifMyProblem(
problem, top -> reportNullableAssignedToNonAnnotatedField(reporter, top, expression, problem.getMessage(expressions)));
problem, top -> reportNullableAssignedToNonAnnotatedField(reporter, top, expression, problem.getMessage(expressions), alwaysNull));
}
}
}
@@ -675,7 +680,8 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
NullabilityProblem<?> problem,
PsiExpression expr,
Map<PsiExpression, ConstantResult> expressions) {
LocalQuickFix[] fixes = createNPEFixes(expr, expr, reporter.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY);
LocalQuickFix[] fixes = createNPEFixes(expr, expr, reporter.isOnTheFly(), problem.isAlwaysNull(expressions))
.toArray(LocalQuickFix.EMPTY_ARRAY);
reporter.registerProblem(expr, problem.getMessage(expressions), fixes);
}
@@ -791,10 +797,10 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
private void reportNullableArgumentsPassedToNonAnnotated(ProblemReporter reporter,
@InspectionMessage String message,
PsiExpression expression,
PsiExpression top) {
PsiExpression top, boolean alwaysNull) {
PsiParameter parameter = MethodCallUtils.getParameterForArgument(top);
if (parameter != null && BaseIntentionAction.canModify(parameter) && AnnotationUtil.isAnnotatingApplicable(parameter)) {
List<LocalQuickFix> fixes = createNPEFixes(expression, top, reporter.isOnTheFly());
List<LocalQuickFix> fixes = createNPEFixes(expression, top, reporter.isOnTheFly(), alwaysNull);
fixes.add(AddAnnotationPsiFix.createAddNullableFix(parameter));
reporter.registerProblem(expression, message, fixes.toArray(LocalQuickFix.EMPTY_ARRAY));
}
@@ -803,10 +809,11 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
private void reportNullableAssignedToNonAnnotatedField(ProblemReporter reporter,
PsiExpression top,
PsiExpression expression,
@InspectionMessage String message) {
@InspectionMessage String message,
boolean alwaysNull) {
PsiField field = getAssignedField(top);
if (field != null) {
List<LocalQuickFix> fixes = createNPEFixes(expression, top, reporter.isOnTheFly());
List<LocalQuickFix> fixes = createNPEFixes(expression, top, reporter.isOnTheFly(), alwaysNull);
fixes.add(AddAnnotationPsiFix.createAddNullableFix(field));
reporter.registerProblem(expression, message, fixes.toArray(LocalQuickFix.EMPTY_ARRAY));
}
@@ -822,10 +829,13 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
return null;
}
private void reportCallMayProduceNpe(ProblemReporter reporter, @InspectionMessage String message, PsiMethodCallExpression callExpression) {
private void reportCallMayProduceNpe(ProblemReporter reporter, @InspectionMessage String message, PsiMethodCallExpression callExpression,
boolean alwaysNull) {
PsiReferenceExpression methodExpression = callExpression.getMethodExpression();
List<LocalQuickFix> fixes = createNPEFixes(methodExpression.getQualifierExpression(), callExpression, reporter.isOnTheFly());
ContainerUtil.addIfNotNull(fixes, ReplaceWithObjectsEqualsFix.createFix(callExpression, methodExpression));
List<LocalQuickFix> fixes = createNPEFixes(methodExpression.getQualifierExpression(), callExpression, reporter.isOnTheFly(), alwaysNull);
if (!alwaysNull) {
ContainerUtil.addIfNotNull(fixes, ReplaceWithObjectsEqualsFix.createFix(callExpression, methodExpression));
}
PsiElement toHighlight = getElementToHighlight(callExpression);
reporter.registerProblem(toHighlight, message, fixes.toArray(LocalQuickFix.EMPTY_ARRAY));
@@ -1051,14 +1061,14 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
final PsiExpression anchor = problem.getAnchor();
PsiExpression expr = problem.getDereferencedExpression();
boolean exactlyNull = isNullLiteralExpression(expr) || expressions.get(expr) == ConstantResult.NULL;
boolean exactlyNull = problem.isAlwaysNull(expressions);
if (!REPORT_UNSOUND_WARNINGS && !exactlyNull) continue;
if (nullability == Nullability.NOT_NULL) {
String presentable = NullableStuffInspectionBase.getPresentableAnnoName(anno);
final String text = exactlyNull
? JavaAnalysisBundle.message("dataflow.message.return.null.from.notnull", presentable)
: JavaAnalysisBundle.message("dataflow.message.return.nullable.from.notnull", presentable);
reporter.registerProblem(expr, text, createNPEFixes(expr, expr, reporter.isOnTheFly()).toArray(LocalQuickFix.EMPTY_ARRAY));
reporter.registerProblem(expr, text, createNPEFixes(expr, expr, reporter.isOnTheFly(), exactlyNull).toArray(LocalQuickFix.EMPTY_ARRAY));
}
else if (AnnotationUtil.isAnnotatingApplicable(anchor)) {
final String defaultNullable = manager.getDefaultNullable();
@@ -1222,10 +1232,6 @@ public abstract class DataFlowInspectionBase extends AbstractBaseJavaLocalInspec
return field.hasInitializer() && PsiUtil.isConstantExpression(field.getInitializer());
}
private static boolean isNullLiteralExpression(PsiElement expr) {
return expr instanceof PsiExpression && ExpressionUtils.isNullLiteral((PsiExpression)expr);
}
private @Nullable LocalQuickFix createSimplifyBooleanExpressionFix(PsiElement element, final boolean value) {
LocalQuickFixOnPsiElement fix = createSimplifyBooleanFix(element, value);
if (fix == null) return null;
@@ -575,19 +575,20 @@ public final class NullabilityProblemKind<T extends PsiElement> {
return myFromUnknown;
}
public boolean isAlwaysNull(@NotNull Map<PsiExpression, DataFlowInspectionBase.ConstantResult> expressions) {
PsiExpression expression = PsiUtil.skipParenthesizedExprDown(getDereferencedExpression());
return expression != null &&
(ExpressionUtils.isNullLiteral(expression) || expressions.get(expression) == DataFlowInspectionBase.ConstantResult.NULL);
}
@NotNull
public @InspectionMessage String getMessage(Map<PsiExpression, DataFlowInspectionBase.ConstantResult> expressions) {
public @InspectionMessage String getMessage(@NotNull Map<PsiExpression, DataFlowInspectionBase.ConstantResult> expressions) {
if (myKind.myAlwaysNullMessage == null || myKind.myNormalMessage == null) {
throw new IllegalStateException("This problem kind has no message associated: " + myKind);
}
String suffix = myFromUnknown ? JavaAnalysisBundle.message("dataflow.message.unknown.nullability") : "";
PsiExpression expression = PsiUtil.skipParenthesizedExprDown(getDereferencedExpression());
if (expression != null) {
if (ExpressionUtils.isNullLiteral(expression) || expressions.get(expression) == DataFlowInspectionBase.ConstantResult.NULL) {
return myKind.myAlwaysNullMessage.get() + suffix;
}
}
return myKind.myNormalMessage.get() + suffix;
Supplier<@Nls String> msg = isAlwaysNull(expressions) ? myKind.myAlwaysNullMessage : myKind.myNormalMessage;
return msg.get() + suffix;
}
@NotNull
@@ -170,7 +170,10 @@ public class DataFlowInspection extends DataFlowInspectionBase {
@Override
@NotNull
protected List<LocalQuickFix> createNPEFixes(@Nullable PsiExpression qualifier, PsiExpression expression, boolean onTheFly) {
protected List<LocalQuickFix> createNPEFixes(@Nullable PsiExpression qualifier,
PsiExpression expression,
boolean onTheFly,
boolean alwaysNull) {
qualifier = PsiUtil.deparenthesizeExpression(qualifier);
final List<LocalQuickFix> fixes = new SmartList<>();
@@ -182,7 +185,7 @@ public class DataFlowInspection extends DataFlowInspectionBase {
if (isVolatileFieldReference(qualifier)) {
ContainerUtil.addIfNotNull(fixes, createIntroduceVariableFix());
}
else if (!ExpressionUtils.isNullLiteral(qualifier) && !SideEffectChecker.mayHaveSideEffects(qualifier)) {
else if (!alwaysNull && !SideEffectChecker.mayHaveSideEffects(qualifier)) {
String suffix = " != null";
if (PsiUtil.getLanguageLevel(qualifier).isAtLeast(LanguageLevel.JDK_1_4) && CodeBlockSurrounder.canSurround(expression)) {
String replacement = ParenthesesUtils.getText(qualifier, ParenthesesUtils.EQUALITY_PRECEDENCE) + suffix;
@@ -198,7 +201,7 @@ public class DataFlowInspection extends DataFlowInspectionBase {
}
}
if (!ExpressionUtils.isNullLiteral(qualifier) && PsiUtil.isLanguageLevel7OrHigher(qualifier)) {
if (!alwaysNull && PsiUtil.isLanguageLevel7OrHigher(qualifier)) {
fixes.add(new SurroundWithRequireNonNullFix(qualifier));
}
@@ -1,6 +1,6 @@
// "Assert 'myFoo != null'" "true"
class A{
private final String myFoo = null;
private final String myFoo = Math.random() > 0.5 ? "" : null;
String myBar;
{
@@ -3,9 +3,9 @@ import java.util.function.Supplier;
class A{
void test(){
Object container = null;
Object container = Math.random() > 0.5 ? "" : null;
Supplier<String> r = () -> {
if (container == null) {
if (Math.random() > 0.5) {
assert container != null;
return container.toString();
} else {
@@ -1,5 +1,5 @@
// "Assert 'myFoo != null'" "true"
class A{
private final String myFoo = null;
private final String myFoo = Math.random() > 0.5 ? "" : null;
String myBar = myFoo.su<caret>bstring(0);
}
@@ -3,7 +3,7 @@ import java.util.function.Supplier;
class A{
void test(){
Object container = null;
Supplier<String> r = () -> container == null ? container.toS<caret>tring() : "";
Object container = Math.random() > 0.5 ? "" : null;
Supplier<String> r = () -> Math.random() > 0.5 ? container.toS<caret>tring() : "";
}
}
@@ -4,7 +4,7 @@ import org.jetbrains.annotations.NotNull;
class A{
void test(@NotNull List l) {
final List list = null;
final List list = Math.random() > 0.5 ? new List() : null;
test(list != null ? list : null<caret>);
}
}
@@ -2,7 +2,7 @@
class A{
void test(){
List list = null;
List list = Math.random() > 0.5 ? new List() : null;
Object o = list != null ? list.get(0) : null<caret>;
}
}
@@ -4,7 +4,7 @@ import org.jetbrains.annotations.NotNull;
class A{
void test(@NotNull List l) {
final List list = null;
final List list = Math.random() > 0.5 ? new List() : null;
test(li<caret>st);
}
}
@@ -2,7 +2,7 @@
class A{
void test(){
List list = null;
List list = Math.random() > 0.5 ? new List() : null;
Object o = list.ge<caret>t(0);
}
}
@@ -1,7 +1,7 @@
// "Surround with 'if (i != null)'" "true"
class A {
void foo(){
String i = null;
void foo(int x){
String i = x > 0 ? "" : null;
if (i != null<caret>) {
i.hashCode();
}
@@ -1,7 +1,7 @@
// "Surround with 'if (i != null)'" "true"
class A {
void foo(){
String i = null;
void foo(int x){
String i = x > 0 ? "" : null;
i.has<caret>hCode();
}
}