diff --git a/java/java-impl/src/com/intellij/codeInspection/ReplaceNullCheckInspection.java b/java/java-impl/src/com/intellij/codeInspection/ReplaceNullCheckInspection.java index af0f111e75ef..cdbafe6205a4 100644 --- a/java/java-impl/src/com/intellij/codeInspection/ReplaceNullCheckInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/ReplaceNullCheckInspection.java @@ -56,9 +56,9 @@ public class ReplaceNullCheckInspection extends AbstractBaseJavaLocalInspectionT public void visitIfStatement(PsiIfStatement ifStatement) { NotNullContext context = NotNullContext.from(ifStatement); if(context == null) return; - String method = getMethodWithClass(context.getExpressionToReplace(), context.isStream()); + String method = getMethodWithClass(context.myExpressionToReplace, context.myIsStream); - PsiStatement nextToDelete = context.getNextToDelete(); + PsiStatement nextToDelete = context.myNextToDelete; int maybeImplicitElseLength = nextToDelete != null ? nextToDelete.getTextLength() : 0; boolean isInfoLevel = noWarningReplacementBigger && ifStatement.getTextLength() + maybeImplicitElseLength - context.getLenAfterReplace() < MINIMAL_WARN_DELTA_SIZE; ProblemHighlightType highlight = getHighlight(context, isInfoLevel); @@ -68,7 +68,7 @@ public class ReplaceNullCheckInspection extends AbstractBaseJavaLocalInspectionT @NotNull private ProblemHighlightType getHighlight(NotNullContext context, boolean isInfoLevel) { - if(context.isStream()) { + if(context.myIsStream) { return ProblemHighlightType.GENERIC_ERROR_OR_WARNING; } if (isInfoLevel) { @@ -81,11 +81,11 @@ public class ReplaceNullCheckInspection extends AbstractBaseJavaLocalInspectionT public void visitConditionalExpression(PsiConditionalExpression ternary) { TernaryNotNullContext context = TernaryNotNullContext.from(ternary); if(context == null) return; - String method = getMethodWithClass(context.getNonNullExpr(), false); - String name = context.getVariable().getName(); + String method = getMethodWithClass(context.myNullExpr, false); + String name = context.myReferenceExpression.getText(); boolean replacementShorter = name != null - && context.getNonNullExpr().getTextLength() + method.length() + name.length() < context.getTernary().getTextLength() + MINIMAL_WARN_DELTA_SIZE; + && context.myNullExpr.getTextLength() + method.length() + name.length() < context.myTernary.getTextLength() + MINIMAL_WARN_DELTA_SIZE; boolean isInfoLevel = noWarningReplacementBigger && replacementShorter; ProblemHighlightType highlightType = isInfoLevel ? ProblemHighlightType.INFORMATION : ProblemHighlightType.GENERIC_ERROR_OR_WARNING; holder.registerProblem(ternary, InspectionsBundle.message("inspection.require.non.null.message", method), @@ -124,27 +124,27 @@ public class ReplaceNullCheckInspection extends AbstractBaseJavaLocalInspectionT NotNullContext context = NotNullContext.from((PsiIfStatement)element); if (context == null) return; CommentTracker tracker = new CommentTracker(); - PsiExpression expression = context.getExpressionToReplace(); - if(!context.isStream()) { - PsiExpression requireCall = createRequireExpression(tracker, expression, project, context.getVariable(), context.getReference()); - context.getReference().replace(requireCall); + PsiExpression expression = context.myExpressionToReplace; + if(!context.myIsStream) { + PsiExpression requireCall = createRequireExpression(tracker, expression, project, context.myReference, context.myDiff); + context.myDiff.replace(requireCall); } else { PsiElementFactory factory = JavaPsiFacade.getElementFactory(project); - String streamOfNullableText = CommonClassNames.JAVA_UTIL_STREAM_STREAM + ".ofNullable(" + context.getVariable().getName() + ")"; + String streamOfNullableText = CommonClassNames.JAVA_UTIL_STREAM_STREAM + ".ofNullable(" + context.myReference.getText() + ")"; PsiExpression streamOfNullable = factory.createExpressionFromText(streamOfNullableText, expression); expression.replace(streamOfNullable); } - result = tracker.replaceAndRestoreComments(context.getIfStatement(), context.getNullBranchStmt()); - if (context.getNextToDelete() != null) { - context.getNextToDelete().delete(); + result = tracker.replaceAndRestoreComments(context.myIfStatement, context.myNullBranchStmt); + if (context.myNextToDelete != null) { + context.myNextToDelete.delete(); } } else if(element instanceof PsiConditionalExpression) { TernaryNotNullContext context = TernaryNotNullContext.from((PsiConditionalExpression)element); if(context == null) return; CommentTracker tracker = new CommentTracker(); PsiExpression requireCall = - createRequireExpression(tracker, context.getNonNullExpr(), project, context.getVariable(), context.getNonNullExpr()); - result = tracker.replace(context.getTernary(), requireCall); + createRequireExpression(tracker, context.myNullExpr, project, context.myReferenceExpression, context.myNullExpr); + result = tracker.replace(context.myTernary, requireCall); } else return; LambdaCanBeMethodReferenceInspection.replaceAllLambdasWithMethodReferences(result); CodeStyleManager.getInstance(project).reformat(JavaCodeStyleManager.getInstance(project).shortenClassReferences(result)); @@ -155,14 +155,14 @@ public class ReplaceNullCheckInspection extends AbstractBaseJavaLocalInspectionT private static PsiExpression createRequireExpression(@NotNull CommentTracker tracker, @NotNull PsiExpression expression, @NotNull Project project, - @NotNull PsiVariable variable, + @NotNull PsiReferenceExpression nullableReference, @NotNull PsiElement context) { boolean isSimple = ExpressionUtils.isSimpleExpression(expression); String expr = tracker.text(expression); if (!isSimple) { expr = "()->" + expr; } - String varName = variable.getName(); + String varName = nullableReference.getText(); String requireCallText = CommonClassNames.JAVA_UTIL_OBJECTS + "." + getMethod(expression) + "(" + varName + "," + expr + ")"; PsiElementFactory factory = JavaPsiFacade.getElementFactory(project); return factory.createExpressionFromText(requireCallText, context); @@ -170,70 +170,33 @@ public class ReplaceNullCheckInspection extends AbstractBaseJavaLocalInspectionT private static class NotNullContext { private final @NotNull PsiExpression myExpressionToReplace; - private final @NotNull PsiExpression myReference; + private final @NotNull PsiExpression myDiff; private final @NotNull PsiStatement myNullBranchStmt; - private final @NotNull PsiVariable myVariable; + private final @NotNull PsiReferenceExpression myReference; private final @NotNull PsiIfStatement myIfStatement; private final @Nullable PsiStatement myNextToDelete; private final boolean myIsStream; private NotNullContext(@NotNull PsiExpression expressionToReplace, - @NotNull PsiExpression reference, + @NotNull PsiExpression diff, @NotNull PsiStatement nullBranchStmt, - @NotNull PsiVariable variable, + @NotNull PsiReferenceExpression reference, @NotNull PsiIfStatement statement, @Nullable PsiStatement nextToDelete, boolean isStream) { myExpressionToReplace = expressionToReplace; - myReference = reference; + myDiff = diff; myNullBranchStmt = nullBranchStmt; - myVariable = variable; + myReference = reference; myIfStatement = statement; myNextToDelete = nextToDelete; myIsStream = isStream; } - @NotNull - public PsiExpression getExpressionToReplace() { - return myExpressionToReplace; - } - - @NotNull - public PsiVariable getVariable() { - return myVariable; - } - - @NotNull - public PsiIfStatement getIfStatement() { - return myIfStatement; - } - - @Nullable - public PsiStatement getNextToDelete() { - return myNextToDelete; - } - - - - @NotNull - public PsiExpression getReference() { - return myReference; - } - - @NotNull - public PsiStatement getNullBranchStmt() { - return myNullBranchStmt; - } - - public boolean isStream() { - return myIsStream; - } - - int getLenAfterReplace() { - int lengthAfterReplace = getExpressionToReplace().getTextLength() + getMethodWithClass(getExpressionToReplace(), isStream()).length(); - if(!isStream()) { - lengthAfterReplace += getNullBranchStmt().getTextLength() + 6; + int lengthAfterReplace = myExpressionToReplace.getTextLength() + getMethodWithClass(myExpressionToReplace, myIsStream).length(); + if(!myIsStream) { + lengthAfterReplace += myNullBranchStmt.getTextLength() + 6; } return lengthAfterReplace; } @@ -244,8 +207,11 @@ public class ReplaceNullCheckInspection extends AbstractBaseJavaLocalInspectionT if(condition == null) return null; PsiBinaryExpression binOp = tryCast(condition, PsiBinaryExpression.class); if(binOp == null) return null; - PsiVariable variable = extractVariable(binOp); - if (variable == null) return null; + PsiExpression value = ExpressionUtils.getValueComparedWithNull(binOp); + PsiReferenceExpression referenceExpression = tryCast(value, PsiReferenceExpression.class); + if(referenceExpression == null) return null; + PsiVariable variable = tryCast(referenceExpression.resolve(), PsiVariable.class); + if(variable == null) return null; if(ClassUtils.isPrimitive(variable.getType())) return null; boolean inverted = binOp.getOperationTokenType() == JavaTokenType.NE; @@ -254,22 +220,23 @@ public class ReplaceNullCheckInspection extends AbstractBaseJavaLocalInspectionT if(elseBranch != null) { PsiStatement nullBranch = inverted? thenBranch : elseBranch; PsiStatement nonNullBranch = inverted? elseBranch : thenBranch; - return extractContext(ifStatement, variable, nullBranch, nonNullBranch, null); + return extractContext(ifStatement, variable, referenceExpression, nullBranch, nonNullBranch, null); } else { PsiReturnStatement nextReturn = tryCast(PsiTreeUtil.skipWhitespacesAndCommentsForward(ifStatement), PsiReturnStatement.class); if (nextReturn == null) return null; if (thenBranch instanceof PsiReturnStatement) { PsiStatement nullBranch = inverted? thenBranch : nextReturn; PsiStatement nonNullBranch = inverted? nextReturn : thenBranch; - return extractContext(ifStatement, variable, nullBranch, nonNullBranch, nextReturn); + return extractContext(ifStatement, variable, referenceExpression, nullBranch, nonNullBranch, nextReturn); } } return null; } - @Contract("_, _, null, _, _ -> null") + @Contract("_, _, _, null, _, _ -> null") private static NotNullContext extractContext(@NotNull PsiIfStatement ifStatement, @NotNull PsiVariable variable, + @NotNull PsiReferenceExpression reference, @Nullable PsiStatement nullBranch, @Nullable PsiStatement nonNullBranch, @Nullable PsiReturnStatement toDelete) { @@ -286,7 +253,7 @@ public class ReplaceNullCheckInspection extends AbstractBaseJavaLocalInspectionT if(!STREAM_EMPTY.test(nonNullCall) || !STREAM_OF.test(nullCall)) return null; PsiExpression maybeRef = nullCall.getArgumentList().getExpressions()[0]; if (!ExpressionUtils.isReferenceTo(maybeRef, variable)) return null; - return new NotNullContext(nullCall, maybeRef, nullBranch, variable, ifStatement, null, true); + return new NotNullContext(nullCall, maybeRef, nullBranch, reference, ifStatement, null, true); } nullDiff = qualifierDiff.getLeft(); nonNullDiff = qualifierDiff.getRight(); @@ -294,7 +261,7 @@ public class ReplaceNullCheckInspection extends AbstractBaseJavaLocalInspectionT } if(NullnessUtil.getExpressionNullness(nonNullDiff, true) != Nullness.NOT_NULL) return null; if(!LambdaGenerationUtil.canBeUncheckedLambda(nonNullDiff)) return null; - return new NotNullContext(nonNullDiff, nullDiff, nullBranch, variable, ifStatement, toDelete, false); + return new NotNullContext(nonNullDiff, nullDiff, nullBranch, reference, ifStatement, toDelete, false); } } @@ -308,56 +275,37 @@ public class ReplaceNullCheckInspection extends AbstractBaseJavaLocalInspectionT return isStream ? "Stream.ofNullable" : "Objects." + getMethod(expression); } - @Nullable - private static PsiVariable extractVariable(@NotNull PsiBinaryExpression binOp) { - PsiExpression value = ExpressionUtils.getValueComparedWithNull(binOp); - PsiReferenceExpression referenceExpression = tryCast(value, PsiReferenceExpression.class); - if(referenceExpression == null) return null; - PsiVariable variable = tryCast(referenceExpression.resolve(), PsiVariable.class); - if(variable == null) return null; - return variable; - } private static class TernaryNotNullContext { private final @NotNull PsiConditionalExpression myTernary; - private final @NotNull PsiExpression myNonNullExpr; - private final @NotNull PsiVariable myVariable; + private final @NotNull PsiExpression myNullExpr; + private final @NotNull PsiReferenceExpression myReferenceExpression; - private TernaryNotNullContext(@NotNull PsiConditionalExpression ternary, @NotNull PsiExpression expr, @NotNull PsiVariable variable) { + private TernaryNotNullContext(@NotNull PsiConditionalExpression ternary, + @NotNull PsiExpression nullExpr, + @NotNull PsiReferenceExpression expression) { myTernary = ternary; - myNonNullExpr = expr; - myVariable = variable; - } - - @NotNull - public PsiExpression getNonNullExpr() { - return myNonNullExpr; - } - - @NotNull - public PsiConditionalExpression getTernary() { - return myTernary; - } - - @NotNull - public PsiVariable getVariable() { - return myVariable; + myNullExpr = nullExpr; + myReferenceExpression = expression; } @Nullable static TernaryNotNullContext from(@NotNull PsiConditionalExpression ternary) { PsiBinaryExpression binOp = tryCast(ternary.getCondition(), PsiBinaryExpression.class); if(binOp == null) return null; - PsiVariable variable = extractVariable(binOp); + PsiExpression value = ExpressionUtils.getValueComparedWithNull(binOp); + PsiReferenceExpression referenceExpression = tryCast(value, PsiReferenceExpression.class); + if(referenceExpression == null) return null; + PsiVariable variable = tryCast(referenceExpression.resolve(), PsiVariable.class); if(variable == null) return null; boolean negated = binOp.getOperationTokenType() == JavaTokenType.NE; - PsiExpression nonNullBranch = negated ? ternary.getElseExpression() : ternary.getThenExpression(); + PsiExpression nullBranch = negated ? ternary.getElseExpression() : ternary.getThenExpression(); if(ClassUtils.isPrimitive(variable.getType())) return null; - PsiExpression nullBranch = negated ? ternary.getThenExpression() : ternary.getElseExpression(); - if(!ExpressionUtils.isReferenceTo(nullBranch, variable)) return null; - if(NullnessUtil.getExpressionNullness(nonNullBranch, true) != Nullness.NOT_NULL) return null; - if(!LambdaGenerationUtil.canBeUncheckedLambda(nonNullBranch)) return null; - return new TernaryNotNullContext(ternary, nonNullBranch, variable); + PsiExpression nonNullBranch = negated ? ternary.getThenExpression() : ternary.getElseExpression(); + if(!ExpressionUtils.isReferenceTo(nonNullBranch, variable)) return null; + if(NullnessUtil.getExpressionNullness(nullBranch, true) != Nullness.NOT_NULL) return null; + if(!LambdaGenerationUtil.canBeUncheckedLambda(nullBranch)) return null; + return new TernaryNotNullContext(ternary, nullBranch, referenceExpression); } } diff --git a/java/java-tests/testData/inspection/requireNonNull/afterStreamEmptyOfField.java b/java/java-tests/testData/inspection/requireNonNull/afterStreamEmptyOfField.java new file mode 100644 index 000000000000..ff22720ff196 --- /dev/null +++ b/java/java-tests/testData/inspection/requireNonNull/afterStreamEmptyOfField.java @@ -0,0 +1,16 @@ +// "Replace condition with Stream.ofNullable" "true" + +import java.util.*; +import java.util.stream.Stream; + +class Other { + Object val = new Object(); + +} + +class Test { + + public Stream test(Other o) { + return Stream.ofNullable(o.val); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/afterTernaryField.java b/java/java-tests/testData/inspection/requireNonNull/afterTernaryField.java new file mode 100644 index 000000000000..532557f0fe38 --- /dev/null +++ b/java/java-tests/testData/inspection/requireNonNull/afterTernaryField.java @@ -0,0 +1,11 @@ +// "Replace condition with Objects.requireNonNullElse" "true" + +import java.util.*; + +class X { + String y; + + static String test(X x) { + return Objects.requireNonNullElse(x.y, "foo"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/beforeStreamEmptyOfField.java b/java/java-tests/testData/inspection/requireNonNull/beforeStreamEmptyOfField.java new file mode 100644 index 000000000000..d8cdd818cf8d --- /dev/null +++ b/java/java-tests/testData/inspection/requireNonNull/beforeStreamEmptyOfField.java @@ -0,0 +1,20 @@ +// "Replace condition with Stream.ofNullable" "true" + +import java.util.*; +import java.util.stream.Stream; + +class Other { + Object val = new Object(); + +} + +class Test { + + public Stream test(Other o) { + if (o.val == null) { + return Stream.empty(); + } else { + return Stream.ofNullable(o.val); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/beforeTernaryField.java b/java/java-tests/testData/inspection/requireNonNull/beforeTernaryField.java new file mode 100644 index 000000000000..18058b45bd03 --- /dev/null +++ b/java/java-tests/testData/inspection/requireNonNull/beforeTernaryField.java @@ -0,0 +1,11 @@ +// "Replace condition with Objects.requireNonNullElse" "true" + +import java.util.*; + +class X { + String y; + + static String test(X x) { + return x.y == null ? "foo" : x.y; + } +} \ No newline at end of file