From cd4a117a49ef362b6e37626522d9827bdf724b48 Mon Sep 17 00:00:00 2001 From: Roman Ivanov Date: Mon, 25 Sep 2017 09:41:15 +0700 Subject: [PATCH] support complex statements inside null check : RequireNonNullInspection --- .../RequireNonNullInspection.java | 306 +++++++++++++----- .../requireNonNull/afterIfAssignment.java | 14 + .../requireNonNull/afterIfCall.java | 14 + .../requireNonNull/afterNonSimpleElse.java | 2 +- .../requireNonNull/afterSimpeReturn.java | 13 + .../afterSimpeReturnWithoutElse.java | 9 + .../afterTernaryInvertedNonSimpleCheck.java | 2 +- .../requireNonNull/afterTernaryOnly.java | 13 + .../afterTernarySimpleCheck.java | 2 +- .../requireNonNull/beforeIfAssignment.java | 18 ++ .../requireNonNull/beforeIfCall.java | 18 ++ .../requireNonNull/beforeNonSimpleElse.java | 4 +- .../requireNonNull/beforeSimpeReturn.java | 17 + ...java => beforeSimpeReturnWithoutElse.java} | 5 +- .../requireNonNull/beforeTernaryNullable.java | 15 + .../requireNonNull/beforeTernaryOnly.java | 13 + .../src/messages/InspectionsBundle.properties | 2 + .../RequireNonNull.html | 2 +- resources/src/META-INF/IdeaPlugin.xml | 4 +- 19 files changed, 391 insertions(+), 82 deletions(-) create mode 100644 java/java-tests/testData/inspection/requireNonNull/afterIfAssignment.java create mode 100644 java/java-tests/testData/inspection/requireNonNull/afterIfCall.java create mode 100644 java/java-tests/testData/inspection/requireNonNull/afterSimpeReturn.java create mode 100644 java/java-tests/testData/inspection/requireNonNull/afterSimpeReturnWithoutElse.java create mode 100644 java/java-tests/testData/inspection/requireNonNull/afterTernaryOnly.java create mode 100644 java/java-tests/testData/inspection/requireNonNull/beforeIfAssignment.java create mode 100644 java/java-tests/testData/inspection/requireNonNull/beforeIfCall.java create mode 100644 java/java-tests/testData/inspection/requireNonNull/beforeSimpeReturn.java rename java/java-tests/testData/inspection/requireNonNull/{beforeSimpleCheck.java => beforeSimpeReturnWithoutElse.java} (68%) create mode 100644 java/java-tests/testData/inspection/requireNonNull/beforeTernaryNullable.java create mode 100644 java/java-tests/testData/inspection/requireNonNull/beforeTernaryOnly.java diff --git a/java/java-impl/src/com/intellij/codeInspection/RequireNonNullInspection.java b/java/java-impl/src/com/intellij/codeInspection/RequireNonNullInspection.java index fc7687c2164a..a074682691ba 100644 --- a/java/java-impl/src/com/intellij/codeInspection/RequireNonNullInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/RequireNonNullInspection.java @@ -22,12 +22,10 @@ import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.codeStyle.JavaCodeStyleManager; -import com.intellij.psi.tree.IElementType; +import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; -import com.siyeh.ig.psiutils.ClassUtils; -import com.siyeh.ig.psiutils.CommentTracker; -import com.siyeh.ig.psiutils.ControlFlowUtils; -import com.siyeh.ig.psiutils.ExpressionUtils; +import com.siyeh.ig.psiutils.*; +import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -35,6 +33,8 @@ import org.jetbrains.annotations.Nullable; import static com.intellij.util.ObjectUtils.tryCast; public class RequireNonNullInspection extends BaseJavaBatchLocalInspectionTool { + private static final EquivalenceChecker ourEquivalence = EquivalenceChecker.getCanonicalPsiEquivalence(); + @NotNull @Override public PsiElementVisitor buildVisitor(@NotNull ProblemsHolder holder, boolean isOnTheFly) { @@ -47,16 +47,18 @@ public class RequireNonNullInspection extends BaseJavaBatchLocalInspectionTool { public void visitIfStatement(PsiIfStatement ifStatement) { NotNullContext context = NotNullContext.from(ifStatement); if(context == null) return; - String method = context.getMethod(); - holder.registerProblem(ifStatement, "Replace condition with Objects." + method, new ReplaceWithRequireNonNullFix(method)); + String method = getMethod(context.getExpression()); + holder.registerProblem(ifStatement, InspectionsBundle.message("inspection.require.non.null.message", method), + new ReplaceWithRequireNonNullFix(method)); } @Override - public void visitAssignmentExpression(PsiAssignmentExpression assignment) { - NotNullContext context = NotNullContext.from(assignment); + public void visitConditionalExpression(PsiConditionalExpression ternary) { + TernaryNotNullContext context = TernaryNotNullContext.from(ternary); if(context == null) return; - String method = context.getMethod(); - holder.registerProblem(assignment, "Replace condition with Objects." + method, new ReplaceWithRequireNonNullFix(method)); + String method = getMethod(context.getNonNullExpr()); + holder.registerProblem(ternary, InspectionsBundle.message("inspection.require.non.null.message", method), + new ReplaceWithRequireNonNullFix(method)); } }; } @@ -77,108 +79,266 @@ public class RequireNonNullInspection extends BaseJavaBatchLocalInspectionTool { @NotNull @Override public String getFamilyName() { - return "Replace null checked assignment with Objects static method"; + return "Replace null checked condition with Objects static method"; } @Override public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { PsiElement element = descriptor.getStartElement(); - final NotNullContext context; - boolean isExpr = false; + final PsiElement result; if(element instanceof PsiIfStatement) { - context = NotNullContext.from((PsiIfStatement)element); - } else if(element instanceof PsiAssignmentExpression) { - context = NotNullContext.from((PsiAssignmentExpression)element); - isExpr = true; + NotNullContext context = NotNullContext.from((PsiIfStatement)element); + if (context == null) return; + CommentTracker tracker = new CommentTracker(); + PsiExpression expression = context.getExpression(); + PsiExpression requireCall = createRequireExpression(tracker, expression, project, context.getVariable(), context.getReference()); + context.getReference().replace(requireCall); + + result = tracker.replaceAndRestoreComments(context.getIfStatement(), context.getNullBranchStmt()); + if (context.getNextToDelete() != null) { + context.getNextToDelete().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); } else return; - if(context == null) return; - CommentTracker tracker = new CommentTracker(); - boolean isSimple = ExpressionUtils.isSimpleExpression(context.getExpression()); - String expr = tracker.text(context.getExpression()); - if(!isSimple) { - expr = "()->" + expr; - } - String varName = context.getVariable().getName(); - String maybeSemicolon = isExpr ? "" : ";"; - String replacement = varName + "=" + CommonClassNames.JAVA_UTIL_OBJECTS + "." + myMethod + "(" + varName + "," + expr + ")" + maybeSemicolon; - PsiElement result = tracker.replaceAndRestoreComments(element, replacement); LambdaCanBeMethodReferenceInspection.replaceAllLambdasWithMethodReferences(result); CodeStyleManager.getInstance(project).reformat(JavaCodeStyleManager.getInstance(project).shortenClassReferences(result)); } } - private static class NotNullContext { - private final PsiExpression myExpression; - private final PsiVariable myVariable; + @NotNull + private static PsiExpression createRequireExpression(@NotNull CommentTracker tracker, + @NotNull PsiExpression expression, + @NotNull Project project, + @NotNull PsiVariable variable, + @NotNull PsiElement context) { + boolean isSimple = ExpressionUtils.isSimpleExpression(expression); + String expr = tracker.text(expression); + if (!isSimple) { + expr = "()->" + expr; + } + String varName = variable.getName(); + String requireCallText = CommonClassNames.JAVA_UTIL_OBJECTS + "." + getMethod(expression) + "(" + varName + "," + expr + ")"; + PsiElementFactory factory = JavaPsiFacade.getElementFactory(project); + return factory.createExpressionFromText(requireCallText, context); + } - private NotNullContext(PsiExpression expression, PsiVariable variable) { + private static class NotNullContext { + private final @NotNull PsiExpression myExpression; + private final @NotNull PsiExpression myReference; + private final @NotNull PsiStatement myNullBranchStmt; + private final @NotNull PsiVariable myVariable; + private final @NotNull PsiIfStatement myIfStatement; + private final @Nullable PsiStatement myNextToDelete; + + private NotNullContext(@NotNull PsiExpression expression, + @NotNull PsiExpression reference, + @NotNull PsiStatement nullBranchStmt, + @NotNull PsiVariable variable, + @NotNull PsiIfStatement statement, + @Nullable PsiStatement nextToDelete) { myExpression = expression; + myReference = reference; + myNullBranchStmt = nullBranchStmt; myVariable = variable; + myIfStatement = statement; + myNextToDelete = nextToDelete; } + @NotNull public PsiExpression getExpression() { return myExpression; } + @NotNull public PsiVariable getVariable() { return myVariable; } - public String getMethod() { - boolean isSimple = ExpressionUtils.isSimpleExpression(myExpression); - return isSimple? "requireNonNullElse" : "requireNonNullElseGet"; + @NotNull + public PsiIfStatement getIfStatement() { + return myIfStatement; } @Nullable - static NotNullContext from(PsiIfStatement ifStatement) { - if(ifStatement.getElseBranch() != null) return null; - PsiStatement statement = ControlFlowUtils.stripBraces(ifStatement.getThenBranch()); + public PsiStatement getNextToDelete() { + return myNextToDelete; + } + + + + @NotNull + public PsiExpression getReference() { + return myReference; + } + + @NotNull + public PsiStatement getNullBranchStmt() { + return myNullBranchStmt; + } + + + @Nullable + static NotNullContext from(@NotNull PsiIfStatement ifStatement) { PsiExpression condition = ifStatement.getCondition(); if(condition == null) return null; PsiBinaryExpression binOp = tryCast(condition, PsiBinaryExpression.class); if(binOp == 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; + PsiVariable variable = extractVariable(binOp); + if (variable == null) return null; if(ClassUtils.isPrimitive(variable.getType())) return null; - PsiExpressionStatement expressionStatement = tryCast(statement, PsiExpressionStatement.class); - if(expressionStatement == null) return null; - PsiExpression maybeNonNull = ExpressionUtils.getAssignmentTo(expressionStatement.getExpression(), variable); - if(NullnessUtil.getExpressionNullness(maybeNonNull) != Nullness.NOT_NULL) return null; - if(!LambdaGenerationUtil.canBeUncheckedLambda(maybeNonNull)) return null; - return new NotNullContext(maybeNonNull, variable); + + boolean inverted = binOp.getOperationTokenType() == JavaTokenType.NE; + PsiStatement elseBranch = ControlFlowUtils.stripBraces(ifStatement.getElseBranch()); + PsiStatement thenBranch = ControlFlowUtils.stripBraces(ifStatement.getThenBranch()); + if(elseBranch != null) { + PsiStatement nullBranch = inverted? thenBranch : elseBranch; + PsiStatement nonNullBranch = inverted? elseBranch : thenBranch; + return extractContext(ifStatement, variable, 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 null; + } + + @Contract("_, _, null, _, _ -> null") + private static NotNullContext extractContext(@NotNull PsiIfStatement ifStatement, + @NotNull PsiVariable variable, + @Nullable PsiStatement nullBranch, + @Nullable PsiStatement nonNullBranch, + @Nullable PsiReturnStatement toDelete) { + if(nullBranch == null) return null; + EquivalenceChecker.Match match = ourEquivalence.statementsMatch(nullBranch, nonNullBranch); + PsiExpression nullDiff = tryCast(match.getLeftDiff(), PsiExpression.class); + PsiExpression nonNullDiff = tryCast(match.getRightDiff(), PsiExpression.class); + if(!ExpressionUtils.isReferenceTo(nullDiff, variable)) { + TopmostQualifierDiff qualifierDiff = TopmostQualifierDiff.from(nullDiff, nonNullDiff); + if(qualifierDiff == null) return null; + nullDiff = qualifierDiff.getLeft(); + nonNullDiff = qualifierDiff.getRight(); + if(!ExpressionUtils.isReferenceTo(nullDiff, variable)) return null; + } + if(NullnessUtil.getExpressionNullness(nonNullDiff) != Nullness.NOT_NULL) return null; + if(!LambdaGenerationUtil.canBeUncheckedLambda(nonNullDiff)) return null; + return new NotNullContext(nonNullDiff, nullDiff, nullBranch, variable, ifStatement, toDelete); + } + } + + static String getMethod(PsiExpression expression) { + return ExpressionUtils.isSimpleExpression(expression) ? "requireNonNullElse" : "requireNonNullElseGet"; + } + + @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 TernaryNotNullContext(@NotNull PsiConditionalExpression ternary, @NotNull PsiExpression expr, @NotNull PsiVariable variable) { + myTernary = ternary; + myNonNullExpr = expr; + myVariable = variable; + } + + @NotNull + public PsiExpression getNonNullExpr() { + return myNonNullExpr; + } + + @NotNull + public PsiConditionalExpression getTernary() { + return myTernary; + } + + @NotNull + public PsiVariable getVariable() { + return myVariable; } @Nullable - static NotNullContext from(PsiAssignmentExpression assignment) { - PsiConditionalExpression ternary = tryCast(assignment.getRExpression(), PsiConditionalExpression.class); - if(ternary == null) return null; - PsiReferenceExpression reference = tryCast(assignment.getLExpression(), PsiReferenceExpression.class); - if(reference == null) return null; - PsiVariable variable = tryCast(reference.resolve(), PsiVariable.class); - + static TernaryNotNullContext from(@NotNull PsiConditionalExpression ternary) { PsiBinaryExpression binOp = tryCast(ternary.getCondition(), PsiBinaryExpression.class); if(binOp == null) return null; - final boolean negated; - IElementType tokenType = binOp.getOperationTokenType(); - if(tokenType == JavaTokenType.NE) { - negated = true; - } else if(tokenType == JavaTokenType.EQEQ) { - negated = false; - } else return null; - if (!ExpressionUtils.isNullLiteral(ExpressionUtils.getOtherOperand(binOp, variable))) return null; - - + PsiVariable variable = extractVariable(binOp); + if(variable == null) return null; + boolean negated = binOp.getOperationTokenType() == JavaTokenType.NE; + PsiExpression nonNullBranch = 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) != Nullness.NOT_NULL) return null; + if(!LambdaGenerationUtil.canBeUncheckedLambda(nonNullBranch)) return null; + return new TernaryNotNullContext(ternary, nonNullBranch, variable); + } + } - PsiExpression main = negated? ternary.getThenExpression() : ternary.getElseExpression(); - if(!ExpressionUtils.isReferenceTo(main, variable)) return null; - PsiExpression alternative = negated? ternary.getElseExpression() : ternary.getThenExpression(); - if(NullnessUtil.getExpressionNullness(alternative) != Nullness.NOT_NULL) return null; - if(!LambdaGenerationUtil.canBeUncheckedLambda(alternative)) return null; - return new NotNullContext(alternative, variable); + /** + * Represents difference between o1.m1().m2() and o2.m1().m2() + * Relies that call chain and arguments are exactly the same + */ + private static class TopmostQualifierDiff { + private final @Nullable PsiExpression myLeft; + private final @Nullable PsiExpression myRight; + + private TopmostQualifierDiff(@Nullable PsiExpression left, @Nullable PsiExpression right) { + myLeft = left; + myRight = right; + } + + @Nullable + public PsiExpression getRight() { + return myRight; + } + + @Nullable + public PsiExpression getLeft() { + return myLeft; + } + + @Nullable + static TopmostQualifierDiff from(@Nullable PsiExpression left, @Nullable PsiExpression right) { + PsiMethodCallExpression leftCall = tryCast(left, PsiMethodCallExpression.class); + PsiMethodCallExpression rightCall = tryCast(right, PsiMethodCallExpression.class); + if(leftCall == null || rightCall == null) return null; + while(true) { + PsiReferenceExpression leftMethodExpression = leftCall.getMethodExpression(); + PsiReferenceExpression rightMethodExpression = rightCall.getMethodExpression(); + if(tryCast(leftMethodExpression.resolve(), PsiMethod.class) != tryCast(rightMethodExpression.resolve(), PsiMethod.class)) return null; + PsiExpression[] leftExpressions = leftCall.getArgumentList().getExpressions(); + PsiExpression[] rightExpressions = rightCall.getArgumentList().getExpressions(); + int length = leftExpressions.length; + if(length != rightExpressions.length) return null; + for (int i = 0; i < length; i++) { + if(!ourEquivalence.expressionsAreEquivalent(leftExpressions[i], rightExpressions[i])) return null; + } + PsiExpression leftQualifier = leftMethodExpression.getQualifierExpression(); + leftCall = tryCast(leftQualifier, PsiMethodCallExpression.class); + PsiExpression rightQualifier = rightMethodExpression.getQualifierExpression(); + rightCall = tryCast(rightQualifier, PsiMethodCallExpression.class); + if(leftCall == null || rightCall == null) { + return new TopmostQualifierDiff(leftQualifier, rightQualifier); + } + } } } } diff --git a/java/java-tests/testData/inspection/requireNonNull/afterIfAssignment.java b/java/java-tests/testData/inspection/requireNonNull/afterIfAssignment.java new file mode 100644 index 000000000000..1586afa36450 --- /dev/null +++ b/java/java-tests/testData/inspection/requireNonNull/afterIfAssignment.java @@ -0,0 +1,14 @@ +// "Replace condition with Objects.requireNonNullElseGet" "true" + +import java.util.*; + +class Util { + String method(String s) {return s;} +} + +class Test { + public void test(Object o) { + Object another; + another = Objects.requireNonNullElseGet(o, Object::new); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/afterIfCall.java b/java/java-tests/testData/inspection/requireNonNull/afterIfCall.java new file mode 100644 index 000000000000..fda24990ef2f --- /dev/null +++ b/java/java-tests/testData/inspection/requireNonNull/afterIfCall.java @@ -0,0 +1,14 @@ +// "Replace condition with Objects.requireNonNullElse" "true" + +import java.util.*; + +class Util { + String method(String s) {return s;} +} + +class Test { + public void test(String o) { + Object another; + Util.method(Objects.requireNonNullElse(o, "Hello")); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/afterNonSimpleElse.java b/java/java-tests/testData/inspection/requireNonNull/afterNonSimpleElse.java index 593efe48f553..53f248d476df 100644 --- a/java/java-tests/testData/inspection/requireNonNull/afterNonSimpleElse.java +++ b/java/java-tests/testData/inspection/requireNonNull/afterNonSimpleElse.java @@ -4,6 +4,6 @@ import java.util.*; class Test { public void test(Object o) { - o = Objects.requireNonNullElseGet(o, Object::new); + return Objects.requireNonNullElseGet(o, Object::new); } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/afterSimpeReturn.java b/java/java-tests/testData/inspection/requireNonNull/afterSimpeReturn.java new file mode 100644 index 000000000000..1c87dea5ab7a --- /dev/null +++ b/java/java-tests/testData/inspection/requireNonNull/afterSimpeReturn.java @@ -0,0 +1,13 @@ +// "Replace condition with Objects.requireNonNullElse" "true" + +import java.util.*; + +class Util { + String method(String s) {return s;} +} + +class Test { + public void test(String o) { + return Util.method(Objects.requireNonNullElse(o, "Adasdasdas").trim()).trim(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/afterSimpeReturnWithoutElse.java b/java/java-tests/testData/inspection/requireNonNull/afterSimpeReturnWithoutElse.java new file mode 100644 index 000000000000..c8518e6233e8 --- /dev/null +++ b/java/java-tests/testData/inspection/requireNonNull/afterSimpeReturnWithoutElse.java @@ -0,0 +1,9 @@ +// "Replace condition with Objects.requireNonNullElse" "true" + +import java.util.*; + +class Test { + public void test(String o) { + return Objects.requireNonNullElse(o, ""); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/afterTernaryInvertedNonSimpleCheck.java b/java/java-tests/testData/inspection/requireNonNull/afterTernaryInvertedNonSimpleCheck.java index 593efe48f553..1a71fe9522c1 100644 --- a/java/java-tests/testData/inspection/requireNonNull/afterTernaryInvertedNonSimpleCheck.java +++ b/java/java-tests/testData/inspection/requireNonNull/afterTernaryInvertedNonSimpleCheck.java @@ -4,6 +4,6 @@ import java.util.*; class Test { public void test(Object o) { - o = Objects.requireNonNullElseGet(o, Object::new); + o = Objects.requireNonNullElseGet(o, Object::new); } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/afterTernaryOnly.java b/java/java-tests/testData/inspection/requireNonNull/afterTernaryOnly.java new file mode 100644 index 000000000000..d9371a3dff13 --- /dev/null +++ b/java/java-tests/testData/inspection/requireNonNull/afterTernaryOnly.java @@ -0,0 +1,13 @@ +// "Replace condition with Objects.requireNonNullElse" "true" + +import java.util.*; + +class Test { + void work(Object o) { + + } + + public void test(Object o) { + work(Objects.requireNonNullElse(o, "")); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/afterTernarySimpleCheck.java b/java/java-tests/testData/inspection/requireNonNull/afterTernarySimpleCheck.java index 75afa6cd2271..786374d10410 100644 --- a/java/java-tests/testData/inspection/requireNonNull/afterTernarySimpleCheck.java +++ b/java/java-tests/testData/inspection/requireNonNull/afterTernarySimpleCheck.java @@ -4,6 +4,6 @@ import java.util.*; class Test { public void test(Object o) { - o = Objects.requireNonNullElse(o, ""); + o = Objects.requireNonNullElse(o, ""); } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/beforeIfAssignment.java b/java/java-tests/testData/inspection/requireNonNull/beforeIfAssignment.java new file mode 100644 index 000000000000..469b2e1126e7 --- /dev/null +++ b/java/java-tests/testData/inspection/requireNonNull/beforeIfAssignment.java @@ -0,0 +1,18 @@ +// "Replace condition with Objects.requireNonNullElseGet" "true" + +import java.util.*; + +class Util { + String method(String s) {return s;} +} + +class Test { + public void test(Object o) { + Object another; + if(o == null) { + another = new Object(); + } else { + another = o; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/beforeIfCall.java b/java/java-tests/testData/inspection/requireNonNull/beforeIfCall.java new file mode 100644 index 000000000000..d82c4fe5fe48 --- /dev/null +++ b/java/java-tests/testData/inspection/requireNonNull/beforeIfCall.java @@ -0,0 +1,18 @@ +// "Replace condition with Objects.requireNonNullElse" "true" + +import java.util.*; + +class Util { + String method(String s) {return s;} +} + +class Test { + public void test(String o) { + Object another; + if(o == null) { + Util.method("Hello"); + } else { + Util.method(o); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/beforeNonSimpleElse.java b/java/java-tests/testData/inspection/requireNonNull/beforeNonSimpleElse.java index 68907558a909..57dc20ff8943 100644 --- a/java/java-tests/testData/inspection/requireNonNull/beforeNonSimpleElse.java +++ b/java/java-tests/testData/inspection/requireNonNull/beforeNonSimpleElse.java @@ -5,7 +5,9 @@ import java.util.*; class Test { public void test(Object o) { if(o == null) { - o = new Object(); + return new Object(); + } else { + return o; } } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/beforeSimpeReturn.java b/java/java-tests/testData/inspection/requireNonNull/beforeSimpeReturn.java new file mode 100644 index 000000000000..343e5239d87a --- /dev/null +++ b/java/java-tests/testData/inspection/requireNonNull/beforeSimpeReturn.java @@ -0,0 +1,17 @@ +// "Replace condition with Objects.requireNonNullElse" "true" + +import java.util.*; + +class Util { + String method(String s) {return s;} +} + +class Test { + public void test(String o) { + if(o == null) { + return Util.method("Adasdasdas".trim()).trim(); + } else { + return Util.method(o.trim()).trim(); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/beforeSimpleCheck.java b/java/java-tests/testData/inspection/requireNonNull/beforeSimpeReturnWithoutElse.java similarity index 68% rename from java/java-tests/testData/inspection/requireNonNull/beforeSimpleCheck.java rename to java/java-tests/testData/inspection/requireNonNull/beforeSimpeReturnWithoutElse.java index cdcc6b718aa7..f4e52b2c5d4b 100644 --- a/java/java-tests/testData/inspection/requireNonNull/beforeSimpleCheck.java +++ b/java/java-tests/testData/inspection/requireNonNull/beforeSimpeReturnWithoutElse.java @@ -3,9 +3,10 @@ import java.util.*; class Test { - public void test(Object o) { + public void test(String o) { if(o == null) { - o = ""; + return ""; } + return o; } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/beforeTernaryNullable.java b/java/java-tests/testData/inspection/requireNonNull/beforeTernaryNullable.java new file mode 100644 index 000000000000..dc241dfca416 --- /dev/null +++ b/java/java-tests/testData/inspection/requireNonNull/beforeTernaryNullable.java @@ -0,0 +1,15 @@ +// "Replace condition with Objects.requireNonNullElse" "false" + +import java.util.*; + +class Test { + void work(Object o) {} + + private static Object nullable() { + return null; + } + + public void test(Object o) { + work(o == null? "" : nullable()); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/requireNonNull/beforeTernaryOnly.java b/java/java-tests/testData/inspection/requireNonNull/beforeTernaryOnly.java new file mode 100644 index 000000000000..0575b2665a25 --- /dev/null +++ b/java/java-tests/testData/inspection/requireNonNull/beforeTernaryOnly.java @@ -0,0 +1,13 @@ +// "Replace condition with Objects.requireNonNullElse" "true" + +import java.util.*; + +class Test { + void work(Object o) { + + } + + public void test(Object o) { + work(o == null? "" : o); + } +} \ No newline at end of file diff --git a/platform/platform-resources-en/src/messages/InspectionsBundle.properties b/platform/platform-resources-en/src/messages/InspectionsBundle.properties index 63e0738dc114..5b61e2698b89 100644 --- a/platform/platform-resources-en/src/messages/InspectionsBundle.properties +++ b/platform/platform-resources-en/src/messages/InspectionsBundle.properties @@ -800,6 +800,8 @@ inspection.simplify.collector.message=Can be simplified using ''{0}'' collector inspection.simplify.collector.fix.family.name=Simplify cascaded collector inspection.simplify.collector.fix.name=Use ''Collectors.{0}'' collector +inspection.require.non.null.message=Replace condition with Objects.{0} + inspection.redundant.stream.optional.call.message=Redundant ''{0}'' call inspection.redundant.stream.optional.call.explanation.filter=predicate is always true inspection.redundant.stream.optional.call.explanation.sorted=subsequent ''{0}'' call makes sorting useless diff --git a/resources-en/src/inspectionDescriptions/RequireNonNull.html b/resources-en/src/inspectionDescriptions/RequireNonNull.html index 429a9e722e7e..0fc8ad233cc8 100644 --- a/resources-en/src/inspectionDescriptions/RequireNonNull.html +++ b/resources-en/src/inspectionDescriptions/RequireNonNull.html @@ -2,7 +2,7 @@ This inspection suggests to replace null check with Objects static method. Example: -if(o == null) { o = ""; } can be replaced with o = Objects.requireNonNullElse(o, ""); +

if(o == null) { o = ""; }

can be replaced with

o = Objects.requireNonNullElse(o, "");

New in 2017.3

\ No newline at end of file diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index 3134ac9a15bd..cdfe9d8600a5 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -969,9 +969,9 @@ displayName="Null value for Optional type"/> + displayName="Replace null checked condition with Objects static method"/>