From 00efd1ff04f6f915956f47e29f44351d87fb6a11 Mon Sep 17 00:00:00 2001 From: Roman Ivanov Date: Fri, 1 Sep 2017 13:34:35 +0700 Subject: [PATCH] improve optional chain simplification --- .../SimplifyOptionalCallChainsInspection.java | 535 +++++++++++++++--- ...SimplifyStreamApiCallChainsInspection.java | 77 +-- .../afterNullCheckedAction.java | 32 ++ .../afterOrElseNull.java | 4 +- .../afterOrElseNullIfCondition.java | 32 ++ .../afterOrElseNullInverted.java | 4 +- .../optionalChains/afterOrElseSimple.java | 10 + .../beforeNullCheckedAction.java | 35 ++ .../beforeOrElseNull.java | 2 +- .../beforeOrElseNullIfCondition.java | 35 ++ .../beforeOrElseNullInverted.java | 2 +- .../beforeOrElseNullObjectReturns.java | 36 ++ .../beforeOrElseNullVariableInside.java | 36 ++ .../optionalChains/beforeOrElseSimple.java | 11 + .../SimplifyOptionalCallChains.html | 2 + 15 files changed, 680 insertions(+), 173 deletions(-) create mode 100644 java/java-tests/testData/inspection/optionalChains/afterNullCheckedAction.java rename java/java-tests/testData/inspection/{streamApiCallChains => optionalChains}/afterOrElseNull.java (78%) create mode 100644 java/java-tests/testData/inspection/optionalChains/afterOrElseNullIfCondition.java rename java/java-tests/testData/inspection/{streamApiCallChains => optionalChains}/afterOrElseNullInverted.java (78%) create mode 100644 java/java-tests/testData/inspection/optionalChains/afterOrElseSimple.java create mode 100644 java/java-tests/testData/inspection/optionalChains/beforeNullCheckedAction.java rename java/java-tests/testData/inspection/{streamApiCallChains => optionalChains}/beforeOrElseNull.java (90%) create mode 100644 java/java-tests/testData/inspection/optionalChains/beforeOrElseNullIfCondition.java rename java/java-tests/testData/inspection/{streamApiCallChains => optionalChains}/beforeOrElseNullInverted.java (90%) create mode 100644 java/java-tests/testData/inspection/optionalChains/beforeOrElseNullObjectReturns.java create mode 100644 java/java-tests/testData/inspection/optionalChains/beforeOrElseNullVariableInside.java create mode 100644 java/java-tests/testData/inspection/optionalChains/beforeOrElseSimple.java diff --git a/java/java-impl/src/com/intellij/codeInspection/SimplifyOptionalCallChainsInspection.java b/java/java-impl/src/com/intellij/codeInspection/SimplifyOptionalCallChainsInspection.java index 08e7d52dc56a..92854c5d393c 100644 --- a/java/java-impl/src/com/intellij/codeInspection/SimplifyOptionalCallChainsInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/SimplifyOptionalCallChainsInspection.java @@ -15,25 +15,29 @@ */ package com.intellij.codeInspection; +import com.intellij.codeInspection.util.LambdaGenerationUtil; import com.intellij.codeInspection.util.OptionalUtil; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.impl.PsiDiamondTypeUtil; +import com.intellij.psi.search.searches.ReferencesSearch; import com.intellij.psi.util.PsiExpressionTrimRenderer; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import com.intellij.refactoring.util.LambdaRefactoringUtil; -import com.intellij.util.ObjectUtils; import com.siyeh.ig.callMatcher.CallMatcher; -import com.siyeh.ig.psiutils.EquivalenceChecker; -import com.siyeh.ig.psiutils.ExpressionUtils; -import com.siyeh.ig.psiutils.MethodCallUtils; +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; -import java.util.Objects; +import java.util.*; import java.util.regex.Pattern; +import static com.intellij.util.ObjectUtils.tryCast; + + /** * @author Tagir Valeev */ @@ -54,88 +58,113 @@ public class SimplifyOptionalCallChainsInspection extends BaseJavaBatchLocalInsp if (!PsiUtil.isLanguageLevel8OrHigher(holder.getFile())) { return PsiElementVisitor.EMPTY_VISITOR; } - return new JavaElementVisitor() { + return new OptionalChainVisitor() { @Override - public void visitMethodCallExpression(PsiMethodCallExpression call) { - PsiExpression falseArg = null; - boolean useOrElseGet = false; - if (OPTIONAL_OR_ELSE.test(call)) { - falseArg = call.getArgumentList().getExpressions()[0]; - } - else if (OPTIONAL_OR_ELSE_GET.test(call)) { - useOrElseGet = true; - PsiLambdaExpression lambda = getLambda(call.getArgumentList().getExpressions()[0]); - if (lambda == null || lambda.getParameterList().getParametersCount() != 0) return; - falseArg = LambdaUtil.extractSingleExpressionFromBody(lambda.getBody()); - } - if (falseArg == null) return; - handleMapOrElse(call, useOrElseGet, falseArg); - handleOfNullableOrElse(call, falseArg); - } - - private void handleOfNullableOrElse(PsiMethodCallExpression call, PsiExpression falseArg) { - if (!ExpressionUtils.isNullLiteral(falseArg)) return; - PsiElement parent = PsiUtil.skipParenthesizedExprUp(call.getParent()); - if (!(parent instanceof PsiExpressionList)) return; - PsiMethodCallExpression parentCall = ObjectUtils.tryCast(parent.getParent(), PsiMethodCallExpression.class); - if (!OPTIONAL_OF_NULLABLE.test(parentCall)) return; - PsiExpression qualifier = call.getMethodExpression().getQualifierExpression(); - if (qualifier == null || - !EquivalenceChecker.getCanonicalPsiEquivalence().typesAreEquivalent(qualifier.getType(), parentCall.getType())) { - return; - } - holder.registerProblem(Objects.requireNonNull(parentCall.getMethodExpression().getReferenceNameElement()), - "Unnecessary Optional rewrapping", - new SimplifyOptionalChainFix(qualifier.getText(), "Unwrap")); - } - - private void handleMapOrElse(PsiMethodCallExpression call, boolean useOrElseGet, PsiExpression falseArg) { - PsiMethodCallExpression qualifierCall = MethodCallUtils.getQualifierMethodCall(call); - if (!OPTIONAL_MAP.test(qualifierCall)) return; - PsiLambdaExpression lambda = getLambda(qualifierCall.getArgumentList().getExpressions()[0]); - if (lambda == null) return; - PsiExpression trueArg = LambdaUtil.extractSingleExpressionFromBody(lambda.getBody()); - if (trueArg == null) return; - PsiParameter[] parameters = lambda.getParameterList().getParameters(); - if (parameters.length != 1) return; - PsiExpression qualifier = qualifierCall.getMethodExpression().getQualifierExpression(); - if (qualifier == null) return; - String opt = qualifier.getText(); - PsiParameter parameter = parameters[0]; - String proposed = OptionalUtil.generateOptionalUnwrap(opt, parameter, trueArg, falseArg, call.getType(), useOrElseGet); - String canonicalOrElse; - if (useOrElseGet && !ExpressionUtils.isSimpleExpression(falseArg)) { - canonicalOrElse = ".orElseGet(() -> " + falseArg.getText() + ")"; - } - else { - canonicalOrElse = ".orElse(" + falseArg.getText() + ")"; - } - String canonical = opt + ".map(" + LambdaUtil.createLambda(parameter, trueArg) + ")" + canonicalOrElse; - if (proposed.length() < canonical.length()) { - String displayCode; - if(proposed.equals(opt)) { - displayCode = ""; - } else if(opt.length() > 10) { - // should be a parseable expression - opt = "(($))"; - String template = OptionalUtil.generateOptionalUnwrap(opt, parameter, trueArg, falseArg, call.getType(), useOrElseGet); - displayCode = - PsiExpressionTrimRenderer.render(JavaPsiFacade.getElementFactory(holder.getProject()).createExpressionFromText(template, call)); - displayCode = displayCode.replaceFirst(Pattern.quote(opt), ".."); - } else { - displayCode = - PsiExpressionTrimRenderer.render(JavaPsiFacade.getElementFactory(holder.getProject()).createExpressionFromText(proposed, call)); - } - String message = displayCode.isEmpty() ? "Remove redundant steps from optional chain" : - "Simplify optional chain to '" + displayCode + "'"; - holder.registerProblem(Objects.requireNonNull(call.getMethodExpression().getReferenceNameElement()), - "Optional chain can be simplified", - new SimplifyOptionalChainFix(proposed, message)); - } + protected void handleSimplification(@NotNull PsiElement element, + @NotNull OptionalChainSimplification simplification) { + holder.registerProblem(element, simplification.getDescription(), new OptionalChainFix(simplification)); } }; } + private static abstract class OptionalChainVisitor extends JavaElementVisitor { + @Override + public void visitMethodCallExpression(PsiMethodCallExpression call) { + PsiExpression falseArg = null; + boolean useOrElseGet = false; + if (OPTIONAL_OR_ELSE.test(call)) { + falseArg = call.getArgumentList().getExpressions()[0]; + } + else if (OPTIONAL_OR_ELSE_GET.test(call)) { + useOrElseGet = true; + PsiLambdaExpression lambda = getLambda(call.getArgumentList().getExpressions()[0]); + if (lambda == null || lambda.getParameterList().getParametersCount() != 0) return; + falseArg = LambdaUtil.extractSingleExpressionFromBody(lambda.getBody()); + } + if (falseArg == null) return; + handleMapOrElse(call, useOrElseGet, falseArg); + handleOfNullableOrElse(call, falseArg); + handleOrElseNullConditionalReturn(call, falseArg); + handleOrElseNullConditionalAction(call, falseArg); + } + + private void handleOfNullableOrElse(PsiMethodCallExpression call, PsiExpression falseArg) { + if (!ExpressionUtils.isNullLiteral(falseArg)) return; + PsiElement parent = PsiUtil.skipParenthesizedExprUp(call.getParent()); + if (!(parent instanceof PsiExpressionList)) return; + PsiMethodCallExpression parentCall = tryCast(parent.getParent(), PsiMethodCallExpression.class); + if (!OPTIONAL_OF_NULLABLE.test(parentCall)) return; + PsiExpression qualifier = call.getMethodExpression().getQualifierExpression(); + if (qualifier == null || + !EquivalenceChecker.getCanonicalPsiEquivalence().typesAreEquivalent(qualifier.getType(), parentCall.getType())) { + return; + } + SimplifyOptionalChainFix fix = new SimplifyOptionalChainFix(qualifier.getText(), "Unwrap", "Unnecessary Optional rewrapping"); + handleSimplification(Objects.requireNonNull(parentCall.getMethodExpression().getReferenceNameElement()), fix); + } + + private void handleMapOrElse(PsiMethodCallExpression call, boolean useOrElseGet, PsiExpression falseArg) { + PsiMethodCallExpression qualifierCall = MethodCallUtils.getQualifierMethodCall(call); + if (!OPTIONAL_MAP.test(qualifierCall)) return; + PsiLambdaExpression lambda = getLambda(qualifierCall.getArgumentList().getExpressions()[0]); + if (lambda == null) return; + PsiExpression trueArg = LambdaUtil.extractSingleExpressionFromBody(lambda.getBody()); + if (trueArg == null) return; + PsiParameter[] parameters = lambda.getParameterList().getParameters(); + if (parameters.length != 1) return; + PsiExpression qualifier = qualifierCall.getMethodExpression().getQualifierExpression(); + if (qualifier == null) return; + String opt = qualifier.getText(); + PsiParameter parameter = parameters[0]; + String proposed = OptionalUtil.generateOptionalUnwrap(opt, parameter, trueArg, falseArg, call.getType(), useOrElseGet); + String canonicalOrElse; + if (useOrElseGet && !ExpressionUtils.isSimpleExpression(falseArg)) { + canonicalOrElse = ".orElseGet(() -> " + falseArg.getText() + ")"; + } + else { + canonicalOrElse = ".orElse(" + falseArg.getText() + ")"; + } + String canonical = opt + ".map(" + LambdaUtil.createLambda(parameter, trueArg) + ")" + canonicalOrElse; + if (proposed.length() < canonical.length()) { + String displayCode; + if(proposed.equals(opt)) { + displayCode = ""; + } else if(opt.length() > 10) { + // should be a parseable expression + opt = "(($))"; + String template = OptionalUtil.generateOptionalUnwrap(opt, parameter, trueArg, falseArg, call.getType(), useOrElseGet); + displayCode = + PsiExpressionTrimRenderer.render(JavaPsiFacade.getElementFactory(parameter.getProject()).createExpressionFromText(template, call)); + displayCode = displayCode.replaceFirst(Pattern.quote(opt), ".."); + } else { + displayCode = + PsiExpressionTrimRenderer.render(JavaPsiFacade.getElementFactory(parameter.getProject()).createExpressionFromText(proposed, call)); + } + String message = displayCode.isEmpty() ? "Remove redundant steps from optional chain" : + "Simplify optional chain to '" + displayCode + "'"; + SimplifyOptionalChainFix fix = new SimplifyOptionalChainFix(proposed, message, "Optional chain can be simplified"); + handleSimplification(Objects.requireNonNull(call.getMethodExpression().getReferenceNameElement()), fix); + } + } + + private void handleOrElseNullConditionalReturn(PsiMethodCallExpression call, PsiExpression falseArg) { + OrElseReturnStreamFix.Context context = OrElseReturnStreamFix.Context.extract(call, falseArg); + if (context == null) return; + OrElseReturnStreamFix fix = new OrElseReturnStreamFix(context.getDefaultExpression(), context.isSimple()); + handleSimplification(Objects.requireNonNull(call.getMethodExpression().getReferenceNameElement()), fix); + } + + private void handleOrElseNullConditionalAction(PsiMethodCallExpression call, PsiExpression falseArg) { + if (OrElseNonNullActionFix.Context.extract(call, falseArg) == null) return; + OrElseNonNullActionFix fix = new OrElseNonNullActionFix(); + handleSimplification(Objects.requireNonNull(call.getMethodExpression().getReferenceNameElement()), fix); + } + + protected abstract void handleSimplification(@NotNull PsiElement element, @NotNull OptionalChainSimplification simplification); + } + + + @Nullable private static PsiLambdaExpression getLambda(PsiExpression initializer) { PsiExpression expression = PsiUtil.skipParenthesizedExprDown(initializer); if (expression instanceof PsiLambdaExpression) { @@ -147,20 +176,41 @@ public class SimplifyOptionalCallChainsInspection extends BaseJavaBatchLocalInsp return null; } - private static class SimplifyOptionalChainFix implements LocalQuickFix { - private final String myReplacement; - private final String myMessage; - - public SimplifyOptionalChainFix(String replacement, String message) { - myReplacement = replacement; - myMessage = message; + @Nullable + private static PsiExpression extractFalseArg(@NotNull PsiMethodCallExpression call) { + if (OPTIONAL_OR_ELSE.test(call)) { + return call.getArgumentList().getExpressions()[0]; } + if (OPTIONAL_OR_ELSE_GET.test(call)) { + PsiLambdaExpression lambda = getLambda(call.getArgumentList().getExpressions()[0]); + if (lambda == null || lambda.getParameterList().getParametersCount() != 0) return null; + return LambdaUtil.extractSingleExpressionFromBody(lambda.getBody()); + } + return null; + } + + interface OptionalChainSimplification { + @NotNull + String getName(); + + @NotNull + String getDescription(); + + void applyFix(@NotNull Project project, @NotNull PsiElement element); + } + + + private static class OptionalChainFix implements LocalQuickFix { + + private final @NotNull OptionalChainSimplification mySimplification; + + OptionalChainFix(@NotNull OptionalChainSimplification simplification) {mySimplification = simplification;} @Nls @NotNull @Override public String getName() { - return myMessage; + return mySimplification.getName(); } @Nls @@ -172,7 +222,310 @@ public class SimplifyOptionalCallChainsInspection extends BaseJavaBatchLocalInsp @Override public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { - PsiMethodCallExpression call = PsiTreeUtil.getParentOfType(descriptor.getStartElement(), PsiMethodCallExpression.class); + mySimplification.applyFix(project, descriptor.getStartElement()); + } + } + + + private static class OrElseReturnStreamFix implements OptionalChainSimplification { + private final @NotNull String defaultExpression; + private final boolean myIsSimple; + + private OrElseReturnStreamFix(@NotNull PsiExpression expression, boolean simple) { + defaultExpression = PsiExpressionTrimRenderer.render(expression); + myIsSimple = simple; + } + + @NotNull + @Override + public String getName() { + String method = myIsSimple ? "orElse" : "orElseGet"; + return "Replace null check with " + method + "(" + defaultExpression + ")"; + } + + @NotNull + @Override + public String getDescription() { + return "Remove redundant null check"; + } + + @Override + public void applyFix(@NotNull Project project, @NotNull PsiElement element) { + PsiMethodCallExpression call = PsiTreeUtil.getParentOfType(element, PsiMethodCallExpression.class, false); + if (call == null) return; + PsiExpression falseArg = extractFalseArg(call); + if (!ExpressionUtils.isNullLiteral(falseArg)) return; + Context context = Context.extract(call, falseArg); + if (context == null) return; + PsiExpression receiver = context.getOrElseCall().getMethodExpression().getQualifierExpression(); + if (receiver == null) return; + String methodWithArg = context.isSimple() + ? ".orElse(" + context.getDefaultExpression().getText() + ")" + : ".orElseGet(()->" + context.getDefaultExpression().getText() + ")"; + String expressionText; + expressionText = receiver.getText() + methodWithArg; + PsiStatement finalStatement = JavaPsiFacade.getElementFactory(project).createStatementFromText("return " + expressionText + ";", receiver); + PsiStatement current = PsiTreeUtil.getParentOfType(context.getOrElseCall(), PsiStatement.class, false); + if (current == null) return; + PsiElement result = new CommentTracker().replaceAndRestoreComments(current, finalStatement); + new CommentTracker().deleteAndRestoreComments(context.getNextStatement()); + LambdaCanBeMethodReferenceInspection.replaceAllLambdasWithMethodReferences(result); + } + + /* + if(optValue != null) {return optValue;} else {return "default";} + or + return optValue == null? "default" : optValue; + */ + @Nullable + private static PsiExpression extractConditionalDefaultValue(@NotNull PsiStatement statement, @NotNull PsiVariable optValue) { + if (statement instanceof PsiIfStatement) { + PsiIfStatement ifStatement = (PsiIfStatement)statement; + PsiExpression condition = ifStatement.getCondition(); + if (condition == null) return null; + PsiExpression thenExpr = getReturnExpression(ifStatement.getThenBranch()); + PsiExpression elseExpr = getReturnExpression(ifStatement.getElseBranch()); + if (thenExpr == null || elseExpr == null) return null; + return extractConditionalDefaultValue(thenExpr, elseExpr, condition, optValue); + } + else if (statement instanceof PsiReturnStatement) { + PsiExpression returnValue = ((PsiReturnStatement)statement).getReturnValue(); + PsiConditionalExpression ternary = tryCast(PsiUtil.skipParenthesizedExprDown(returnValue), PsiConditionalExpression.class); + if (ternary == null) return null; + PsiExpression thenExpression = ternary.getThenExpression(); + PsiExpression elseExpression = ternary.getElseExpression(); + if(thenExpression == null || elseExpression == null) return null; + return extractConditionalDefaultValue(thenExpression, elseExpression, ternary.getCondition(), optValue); + } + return null; + } + + @Contract("null -> null") + @Nullable + private static PsiExpression getReturnExpression(@Nullable PsiStatement block) { + if (block == null) return null; + PsiStatement statement = ControlFlowUtils.stripBraces(block); + PsiReturnStatement returnStatement = tryCast(statement, PsiReturnStatement.class); + if (returnStatement == null) return null; + return returnStatement.getReturnValue(); + } + + @Nullable + private static PsiExpression extractConditionalDefaultValue(@NotNull PsiExpression thenExpr, + @NotNull PsiExpression elseExpr, + @NotNull PsiExpression condition, + @NotNull PsiVariable optValue) { + PsiVariable nullChecked = ExpressionUtils.getVariableFromNullComparison(condition, true); + boolean inverted = false; + if (nullChecked == null) { + nullChecked = ExpressionUtils.getVariableFromNullComparison(condition, false); + if (nullChecked == null) return null; + inverted = true; + } + if (!nullChecked.equals(optValue) || !ExpressionUtils.isReferenceTo(inverted ? thenExpr : elseExpr, optValue)) return null; + PsiExpression defaultExpression = inverted ? elseExpr : thenExpr; + if (VariableAccessUtils.variableIsUsed(optValue, defaultExpression)) return null; + return defaultExpression; + } + + private static class Context { + @NotNull private final PsiMethodCallExpression myOrElseCall; + @NotNull private final PsiExpression myDefaultExpression; + @NotNull private final PsiStatement myNextStatement; + private final boolean mySimple; + + private Context(@NotNull PsiMethodCallExpression call, + @NotNull PsiExpression defaultExpression, + @NotNull PsiStatement nextStatement, boolean simple) { + myOrElseCall = call; + myDefaultExpression = defaultExpression; + myNextStatement = nextStatement; + mySimple = simple; + } + + @NotNull + public PsiStatement getNextStatement() { + return myNextStatement; + } + + + @NotNull + public PsiMethodCallExpression getOrElseCall() { + return myOrElseCall; + } + + @NotNull + public PsiExpression getDefaultExpression() { + return myDefaultExpression; + } + + public boolean isSimple() { + return mySimple; + } + + @Nullable + static Context extract(@NotNull PsiMethodCallExpression call, @NotNull PsiExpression falseArg) { + if (!ExpressionUtils.isNullLiteral(falseArg)) return null; + PsiLocalVariable returnVar = PsiTreeUtil.getParentOfType(call, PsiLocalVariable.class, true); + if (returnVar == null) return null; + PsiStatement nextStatement = + tryCast(PsiTreeUtil.skipWhitespacesForward(returnVar.getParent()), PsiStatement.class); + if (nextStatement == null) return null; + PsiExpression defaultValue = extractConditionalDefaultValue(nextStatement, returnVar); + boolean isSimple = ExpressionUtils.isSimpleExpression(defaultValue); + if (defaultValue == null || (!isSimple && !LambdaGenerationUtil.canBeUncheckedLambda(defaultValue))) return null; + PsiType type = defaultValue.getType(); + PsiType methodCallReturnValue = call.getMethodExpression().getType(); + if(type == null || methodCallReturnValue == null || !methodCallReturnValue.isAssignableFrom(type)) return null; + return new Context(call, defaultValue, nextStatement, isSimple); + } + } + } + + + private static class OrElseNonNullActionFix implements OptionalChainSimplification { + @NotNull + @Override + public String getName() { + return "Replace null check with ifPresent()"; + } + + @NotNull + @Override + public String getDescription() { + return "Remove redundant null check"; + } + + @Override + public void applyFix(@NotNull Project project, @NotNull PsiElement element) { + PsiMethodCallExpression call = PsiTreeUtil.getParentOfType(element, PsiMethodCallExpression.class, false); + if (call == null) return; + PsiExpression falseArg = extractFalseArg(call); + if (!ExpressionUtils.isNullLiteral(falseArg)) return; + Context context = Context.extract(call, falseArg); + if(context == null) return; + PsiExpression receiver = context.getOrElseCall().getMethodExpression().getQualifierExpression(); + if(receiver == null) return; + String statementText = receiver.getText() + ".ifPresent(" + LambdaUtil.createLambda(context.getVariable(), context.getAction()) + ");"; + PsiStatement finalStatement = JavaPsiFacade.getElementFactory(project).createStatementFromText(statementText, context.getStatement()); + PsiElement result = context.getStatement().replace(finalStatement); + context.getConditionStatement().delete(); + LambdaCanBeMethodReferenceInspection.replaceAllLambdasWithMethodReferences(result); + } + + private static class Context { + private final @NotNull PsiExpression myAction; + private final @NotNull PsiStatement myConditionStatement; + private final @NotNull PsiStatement myStatement; + private final @NotNull PsiVariable myVariable; + private final @NotNull PsiMethodCallExpression myOrElseCall; + + @NotNull + public PsiExpression getAction() { + return myAction; + } + + @NotNull + public PsiStatement getConditionStatement() { + return myConditionStatement; + } + + @NotNull + public PsiStatement getStatement() { + return myStatement; + } + + @NotNull + public PsiMethodCallExpression getOrElseCall() { + return myOrElseCall; + } + + private Context(@NotNull PsiExpression action, + @NotNull PsiStatement conditionStatement, + @NotNull PsiStatement statement, + @NotNull PsiVariable variable, @NotNull PsiMethodCallExpression call) { + myAction = action; + myConditionStatement = conditionStatement; + myStatement = statement; + myVariable = variable; + myOrElseCall = call; + } + + @NotNull + public PsiVariable getVariable() { + return myVariable; + } + + @Nullable + static Context extract(@NotNull PsiMethodCallExpression orElseCall, @NotNull PsiExpression orElseArgument) { + if (!ExpressionUtils.isNullLiteral(orElseArgument)) return null; + PsiLocalVariable returnVar = PsiTreeUtil.getParentOfType(orElseCall, PsiLocalVariable.class, true); + if (returnVar == null) return null; + PsiStatement statement = PsiTreeUtil.getParentOfType(returnVar, PsiStatement.class, false); + if(statement == null) return null; + PsiStatement nextStatement = + tryCast(PsiTreeUtil.skipWhitespacesForward(returnVar.getParent()), PsiStatement.class); + if (nextStatement == null) return null; + PsiExpression lambdaExpr = extractMappingExpression(nextStatement, returnVar); + if (lambdaExpr == null || !LambdaGenerationUtil.canBeUncheckedLambda(lambdaExpr)) return null; + if(!ReferencesSearch.search(returnVar).forEach(reference -> + PsiTreeUtil.isAncestor(statement, reference.getElement(), false) || + PsiTreeUtil.isAncestor(nextStatement, reference.getElement(), false))) return null; + return new Context(lambdaExpr, nextStatement, statement, returnVar, orElseCall); + } + + + } + + + /* + if(optValue != null) { + System.out.println(optValue); + } + */ + @Nullable + private static PsiExpression extractMappingExpression(@NotNull PsiStatement statement, @NotNull PsiVariable optValue) { + PsiIfStatement ifStatement = tryCast(statement, PsiIfStatement.class); + if (ifStatement == null) return null; + if (ifStatement.getElseBranch() != null) return null; + PsiExpression condition = ifStatement.getCondition(); + if (condition == null) return null; + if (ExpressionUtils.getVariableFromNullComparison(condition, false) != optValue) return null; + + PsiStatement thenStatement = ControlFlowUtils.stripBraces(ifStatement.getThenBranch()); + PsiExpressionStatement expressionStatement = tryCast(thenStatement, PsiExpressionStatement.class); + if (expressionStatement == null) return null; + return expressionStatement.getExpression(); + } + } + + + private static class SimplifyOptionalChainFix implements OptionalChainSimplification { + private final String myReplacement; + private final String myMessage; + private final String myDescription; + + public SimplifyOptionalChainFix(String replacement, String message, String description) { + myReplacement = replacement; + myMessage = message; + myDescription = description; + } + + @NotNull + @Override + public String getName() { + return myMessage; + } + + @NotNull + @Override + public String getDescription() { + return myDescription; + } + + @Override + public void applyFix(@NotNull Project project, @NotNull PsiElement element) { + PsiMethodCallExpression call = PsiTreeUtil.getParentOfType(element, PsiMethodCallExpression.class); if (call == null) return; PsiExpression replacementExpression = JavaPsiFacade.getElementFactory(project).createExpressionFromText(myReplacement, call); PsiElement result = call.replace(replacementExpression); diff --git a/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java b/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java index 94ee11c42df5..cf8d43a38481 100644 --- a/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java @@ -83,8 +83,6 @@ public class SimplifyStreamApiCallChainsInspection extends BaseJavaBatchLocalIns instanceCall(CommonClassNames.JAVA_LANG_BOOLEAN, "equals").parameterCount(1); private static final CallMatcher STREAM_OF = staticCall(CommonClassNames.JAVA_UTIL_STREAM_STREAM, "of").parameterTypes("T"); - private static final CallMatcher OR_ELSE = - instanceCall(CommonClassNames.JAVA_UTIL_OPTIONAL, "orElse").parameterCount(1); private static final CallMatcher STREAM_MATCH = anyOf(STREAM_ANY_MATCH, STREAM_NONE_MATCH, STREAM_ALL_MATCH); @@ -97,8 +95,7 @@ public class SimplifyStreamApiCallChainsInspection extends BaseJavaBatchLocalIns ReplaceForEachMethodFix.handler(), RemoveBooleanIdentityFix.handler(), ReplaceWithPeekFix.handler(), - SimpleStreamOfFix.handler(), - OrElseReturnStreamFix.handler() + SimpleStreamOfFix.handler() ).registerAll(SimplifyMatchNegationFix.handlers()); private static final Logger LOG = Logger.getInstance(SimplifyStreamApiCallChainsInspection.class); @@ -1435,76 +1432,4 @@ public class SimplifyStreamApiCallChainsInspection extends BaseJavaBatchLocalIns }); } } - - private static class OrElseReturnStreamFix implements CallChainSimplification { - - @NotNull private final PsiLocalVariable myReturnVar; - @NotNull private final PsiExpression myReturnExpr; - @NotNull private final PsiExpression myOrElseArgument; - @NotNull private final PsiMethodCallExpression myOrElseCall; - @NotNull private final PsiExpression myDefaultExpression; - - public OrElseReturnStreamFix(@NotNull PsiLocalVariable returnVar, - @NotNull PsiExpression returnExpr, - @NotNull PsiExpression orElseArgument, - @NotNull PsiMethodCallExpression orElseCall, - @NotNull PsiExpression defaultExpression) { - myReturnVar = returnVar; - myReturnExpr = returnExpr; - myOrElseArgument = orElseArgument; - myOrElseCall = orElseCall; - myDefaultExpression = defaultExpression; - } - - @Override - public String getName() { - return "Replace 'orElse(null)' with 'orElse(" + myDefaultExpression.getText() + ")'"; - } - - @Override - public String getMessage() { - return "Can be replaced with 'orElse(" + myDefaultExpression.getText() + ")'"; - } - - @Override - public PsiElement simplify(PsiMethodCallExpression element) { - myOrElseArgument.replace(myDefaultExpression); - myReturnExpr.replace(myOrElseCall); - myReturnVar.delete(); - return myOrElseCall.getParent(); - } - - @NotNull - static CallHandler handler() { - return CallHandler.of(OR_ELSE, call -> { - PsiExpression orElseArgument = call.getArgumentList().getExpressions()[0]; - if (!ExpressionUtils.isNullLiteral(orElseArgument)) return null; - PsiLocalVariable returnVar = PsiTreeUtil.getParentOfType(call, PsiLocalVariable.class, true); - if(returnVar == null) return null; - PsiReturnStatement returnStatement = tryCast(PsiTreeUtil.skipWhitespacesForward(returnVar.getParent()), PsiReturnStatement.class); - if (returnStatement == null) return null; - PsiExpression returnExpr = returnStatement.getReturnValue(); - if (returnExpr == null) return null; - PsiConditionalExpression ternary = tryCast(PsiUtil.skipParenthesizedExprDown(returnExpr), PsiConditionalExpression.class); - if (ternary == null) return null; - PsiExpression condition = ternary.getCondition(); - PsiExpression thenExpr = ternary.getThenExpression(); - PsiExpression elseExpr = ternary.getElseExpression(); - if (thenExpr == null || elseExpr == null) return null; - PsiVariable nullChecked = ExpressionUtils.getVariableFromNullComparison(condition, true); - boolean inverted = false; - if (nullChecked == null) { - nullChecked = ExpressionUtils.getVariableFromNullComparison(condition, false); - if (nullChecked == null) return null; - inverted = true; - } - if (!nullChecked.equals(returnVar) || !ExpressionUtils.isReferenceTo(inverted ? thenExpr : elseExpr, returnVar)) return null; - PsiExpression defaultExpression = inverted ? elseExpr : thenExpr; - if (VariableAccessUtils.variableIsUsed(returnVar, defaultExpression)) return null; - //if(SideEffectChecker.mayHaveSideEffects(defaultExpression)) return null; // new expressions not allowed - return new OrElseReturnStreamFix(returnVar, returnExpr, orElseArgument, call, defaultExpression); - }); - } - - } } diff --git a/java/java-tests/testData/inspection/optionalChains/afterNullCheckedAction.java b/java/java-tests/testData/inspection/optionalChains/afterNullCheckedAction.java new file mode 100644 index 000000000000..85834927e9f3 --- /dev/null +++ b/java/java-tests/testData/inspection/optionalChains/afterNullCheckedAction.java @@ -0,0 +1,32 @@ +// "Replace null check with ifPresent()" "true" + +import java.util.*; + +public class Main { + static class Person { + String name; + int age; + + public String getName() { + return name; + } + + public int getAge() { + return age; + } + + public Person(String name, int age) { + this.name = name; + this.age = age; + } + } + + public Person work() { + List personList = Arrays.asList( + new Person("Roman", 21), + new Person("James", 25), + new Person("Kelly", 12) + ); + personList.stream().filter(p -> p.getAge() > 13).max(Comparator.comparingInt(Person::getAge)).ifPresent(System.out::println); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterOrElseNull.java b/java/java-tests/testData/inspection/optionalChains/afterOrElseNull.java similarity index 78% rename from java/java-tests/testData/inspection/streamApiCallChains/afterOrElseNull.java rename to java/java-tests/testData/inspection/optionalChains/afterOrElseNull.java index 0b641a14e5e0..26da48424c12 100644 --- a/java/java-tests/testData/inspection/streamApiCallChains/afterOrElseNull.java +++ b/java/java-tests/testData/inspection/optionalChains/afterOrElseNull.java @@ -1,4 +1,4 @@ -// "Replace 'orElse(null)' with 'orElse(new Person("noname", 0))'" "true" +// "Replace null check with orElseGet(new Person(...))" "true" import java.util.*; @@ -27,6 +27,6 @@ public class Main { new Person("James", 25), new Person("Kelly", 12) ); - return personList.stream().filter(p -> p.getAge() > 13).max(Comparator.comparingInt(Person::getAge)).orElse(new Person("noname", 0)); + return personList.stream().filter(p -> p.getAge() > 13).max(Comparator.comparingInt(Person::getAge)).orElseGet(() -> new Person("noname", 0)); } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/optionalChains/afterOrElseNullIfCondition.java b/java/java-tests/testData/inspection/optionalChains/afterOrElseNullIfCondition.java new file mode 100644 index 000000000000..26da48424c12 --- /dev/null +++ b/java/java-tests/testData/inspection/optionalChains/afterOrElseNullIfCondition.java @@ -0,0 +1,32 @@ +// "Replace null check with orElseGet(new Person(...))" "true" + +import java.util.*; + +public class Main { + static class Person { + String name; + int age; + + public String getName() { + return name; + } + + public int getAge() { + return age; + } + + public Person(String name, int age) { + this.name = name; + this.age = age; + } + } + + public Person work() { + List personList = Arrays.asList( + new Person("Roman", 21), + new Person("James", 25), + new Person("Kelly", 12) + ); + return personList.stream().filter(p -> p.getAge() > 13).max(Comparator.comparingInt(Person::getAge)).orElseGet(() -> new Person("noname", 0)); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/afterOrElseNullInverted.java b/java/java-tests/testData/inspection/optionalChains/afterOrElseNullInverted.java similarity index 78% rename from java/java-tests/testData/inspection/streamApiCallChains/afterOrElseNullInverted.java rename to java/java-tests/testData/inspection/optionalChains/afterOrElseNullInverted.java index 0b641a14e5e0..26da48424c12 100644 --- a/java/java-tests/testData/inspection/streamApiCallChains/afterOrElseNullInverted.java +++ b/java/java-tests/testData/inspection/optionalChains/afterOrElseNullInverted.java @@ -1,4 +1,4 @@ -// "Replace 'orElse(null)' with 'orElse(new Person("noname", 0))'" "true" +// "Replace null check with orElseGet(new Person(...))" "true" import java.util.*; @@ -27,6 +27,6 @@ public class Main { new Person("James", 25), new Person("Kelly", 12) ); - return personList.stream().filter(p -> p.getAge() > 13).max(Comparator.comparingInt(Person::getAge)).orElse(new Person("noname", 0)); + return personList.stream().filter(p -> p.getAge() > 13).max(Comparator.comparingInt(Person::getAge)).orElseGet(() -> new Person("noname", 0)); } } \ No newline at end of file diff --git a/java/java-tests/testData/inspection/optionalChains/afterOrElseSimple.java b/java/java-tests/testData/inspection/optionalChains/afterOrElseSimple.java new file mode 100644 index 000000000000..1da06b516890 --- /dev/null +++ b/java/java-tests/testData/inspection/optionalChains/afterOrElseSimple.java @@ -0,0 +1,10 @@ +// "Replace null check with orElse("")" "true" + +import java.util.*; + +public class Main { + + public Person work(Optional o) { + return o.orElse(""); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/optionalChains/beforeNullCheckedAction.java b/java/java-tests/testData/inspection/optionalChains/beforeNullCheckedAction.java new file mode 100644 index 000000000000..2fd2249d786b --- /dev/null +++ b/java/java-tests/testData/inspection/optionalChains/beforeNullCheckedAction.java @@ -0,0 +1,35 @@ +// "Replace null check with ifPresent()" "true" + +import java.util.*; + +public class Main { + static class Person { + String name; + int age; + + public String getName() { + return name; + } + + public int getAge() { + return age; + } + + public Person(String name, int age) { + this.name = name; + this.age = age; + } + } + + public Person work() { + List personList = Arrays.asList( + new Person("Roman", 21), + new Person("James", 25), + new Person("Kelly", 12) + ); + Person maxPerson = personList.stream().filter(p -> p.getAge() > 13).max(Comparator.comparingInt(Person::getAge)).orElse(null); + if(maxPerson != null) { + System.out.println(maxPerson); + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeOrElseNull.java b/java/java-tests/testData/inspection/optionalChains/beforeOrElseNull.java similarity index 90% rename from java/java-tests/testData/inspection/streamApiCallChains/beforeOrElseNull.java rename to java/java-tests/testData/inspection/optionalChains/beforeOrElseNull.java index 286dfc6a9f59..7a44129db658 100644 --- a/java/java-tests/testData/inspection/streamApiCallChains/beforeOrElseNull.java +++ b/java/java-tests/testData/inspection/optionalChains/beforeOrElseNull.java @@ -1,4 +1,4 @@ -// "Replace 'orElse(null)' with 'orElse(new Person("noname", 0))'" "true" +// "Replace null check with orElseGet(new Person(...))" "true" import java.util.*; diff --git a/java/java-tests/testData/inspection/optionalChains/beforeOrElseNullIfCondition.java b/java/java-tests/testData/inspection/optionalChains/beforeOrElseNullIfCondition.java new file mode 100644 index 000000000000..79253e3bdc2e --- /dev/null +++ b/java/java-tests/testData/inspection/optionalChains/beforeOrElseNullIfCondition.java @@ -0,0 +1,35 @@ +// "Replace null check with orElseGet(new Person(...))" "true" + +import java.util.*; + +public class Main { + static class Person { + String name; + int age; + + public String getName() { + return name; + } + + public int getAge() { + return age; + } + + public Person(String name, int age) { + this.name = name; + this.age = age; + } + } + + public Person work() { + List personList = Arrays.asList( + new Person("Roman", 21), + new Person("James", 25), + new Person("Kelly", 12) + ); + Person maxPerson = personList.stream().filter(p -> p.getAge() > 13).max(Comparator.comparingInt(Person::getAge)).orElse(null); + if(maxPerson != null) { + return maxPerson; + } else return new Person("noname", 0); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeOrElseNullInverted.java b/java/java-tests/testData/inspection/optionalChains/beforeOrElseNullInverted.java similarity index 90% rename from java/java-tests/testData/inspection/streamApiCallChains/beforeOrElseNullInverted.java rename to java/java-tests/testData/inspection/optionalChains/beforeOrElseNullInverted.java index 6934595268a6..302d54a19c3f 100644 --- a/java/java-tests/testData/inspection/streamApiCallChains/beforeOrElseNullInverted.java +++ b/java/java-tests/testData/inspection/optionalChains/beforeOrElseNullInverted.java @@ -1,4 +1,4 @@ -// "Replace 'orElse(null)' with 'orElse(new Person("noname", 0))'" "true" +// "Replace null check with orElseGet(new Person(...))" "true" import java.util.*; diff --git a/java/java-tests/testData/inspection/optionalChains/beforeOrElseNullObjectReturns.java b/java/java-tests/testData/inspection/optionalChains/beforeOrElseNullObjectReturns.java new file mode 100644 index 000000000000..e9d89d54b546 --- /dev/null +++ b/java/java-tests/testData/inspection/optionalChains/beforeOrElseNullObjectReturns.java @@ -0,0 +1,36 @@ +// "Replace null check with orElseGet(new Object())" "false" + +import java.util.*; + +public class Main { + static class Person { + String name; + int age; + + public String getName() { + return name; + } + + public int getAge() { + return age; + } + + public Person(String name, int age) { + this.name = name; + this.age = age; + } + + public Person(Person p) { + } + } + + public Person work() { + List personList = Arrays.asList( + new Person("Roman", 21), + new Person("James", 25), + new Person("Kelly", 12) + ); + Person maxPerson = personList.stream().filter(p -> p.getAge() > 13).max(Comparator.comparingInt(Person::getAge)).orElse(null); + return maxPerson == null ? new Object() : maxPerson; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/optionalChains/beforeOrElseNullVariableInside.java b/java/java-tests/testData/inspection/optionalChains/beforeOrElseNullVariableInside.java new file mode 100644 index 000000000000..bd656d239d66 --- /dev/null +++ b/java/java-tests/testData/inspection/optionalChains/beforeOrElseNullVariableInside.java @@ -0,0 +1,36 @@ +// "Replace null check with orElseGet(new Person(...))" "false" + +import java.util.*; + +public class Main { + static class Person { + String name; + int age; + + public String getName() { + return name; + } + + public int getAge() { + return age; + } + + public Person(String name, int age) { + this.name = name; + this.age = age; + } + + public Person(Person p) { + } + } + + public Person work() { + List personList = Arrays.asList( + new Person("Roman", 21), + new Person("James", 25), + new Person("Kelly", 12) + ); + Person maxPerson = personList.stream().filter(p -> p.getAge() > 13).max(Comparator.comparingInt(Person::getAge)).orElse(null); + return maxPerson == null ? new Person(maxPerson) : maxPerson; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/optionalChains/beforeOrElseSimple.java b/java/java-tests/testData/inspection/optionalChains/beforeOrElseSimple.java new file mode 100644 index 000000000000..e892b11f443f --- /dev/null +++ b/java/java-tests/testData/inspection/optionalChains/beforeOrElseSimple.java @@ -0,0 +1,11 @@ +// "Replace null check with orElse("")" "true" + +import java.util.*; + +public class Main { + + public Person work(Optional o) { + String v = o.orElse(null); + return v == null? "" : v; + } +} \ No newline at end of file diff --git a/resources-en/src/inspectionDescriptions/SimplifyOptionalCallChains.html b/resources-en/src/inspectionDescriptions/SimplifyOptionalCallChains.html index d330fb4e2af4..df8e1f1f0663 100644 --- a/resources-en/src/inspectionDescriptions/SimplifyOptionalCallChains.html +++ b/resources-en/src/inspectionDescriptions/SimplifyOptionalCallChains.html @@ -6,6 +6,8 @@
  • optional.map(x -> Optional.of(x.trim())).orElseGet(Optional::empty) → optional.map(String::trim)
  • optional.map(x -> (String)x).orElse(null) → (String) optional.orElse(null)
  • Optional.ofNullable(optional.orElse(null)) → optional
    • +
  • val = optional.orElse(null); val != null ? val : defaultExpr → optional.orElse(defaultExpr)
    • +
  • val = optional.orElse(null); if(val != null) expr(val) → optional.ifPresent(val -> expr(val))

    • New in 2017.2