From 3145def978902002be212ce87587a8d034e75104 Mon Sep 17 00:00:00 2001 From: Roman Ivanov Date: Wed, 16 Aug 2017 12:17:34 +0700 Subject: [PATCH] add fix to simplify orElse(null) and return after --- ...SimplifyStreamApiCallChainsInspection.java | 77 ++++++++++++++++++- .../streamApiCallChains/afterOrElseNull.java | 32 ++++++++ .../afterOrElseNullInverted.java | 32 ++++++++ .../streamApiCallChains/beforeOrElseNull.java | 33 ++++++++ .../beforeOrElseNullInverted.java | 33 ++++++++ 5 files changed, 206 insertions(+), 1 deletion(-) create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/afterOrElseNull.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/afterOrElseNullInverted.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/beforeOrElseNull.java create mode 100644 java/java-tests/testData/inspection/streamApiCallChains/beforeOrElseNullInverted.java diff --git a/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java b/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java index cf8d43a38481..94ee11c42df5 100644 --- a/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/SimplifyStreamApiCallChainsInspection.java @@ -83,6 +83,8 @@ 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); @@ -95,7 +97,8 @@ public class SimplifyStreamApiCallChainsInspection extends BaseJavaBatchLocalIns ReplaceForEachMethodFix.handler(), RemoveBooleanIdentityFix.handler(), ReplaceWithPeekFix.handler(), - SimpleStreamOfFix.handler() + SimpleStreamOfFix.handler(), + OrElseReturnStreamFix.handler() ).registerAll(SimplifyMatchNegationFix.handlers()); private static final Logger LOG = Logger.getInstance(SimplifyStreamApiCallChainsInspection.class); @@ -1432,4 +1435,76 @@ 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/streamApiCallChains/afterOrElseNull.java b/java/java-tests/testData/inspection/streamApiCallChains/afterOrElseNull.java new file mode 100644 index 000000000000..0b641a14e5e0 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterOrElseNull.java @@ -0,0 +1,32 @@ +// "Replace 'orElse(null)' with 'orElse(new Person("noname", 0))'" "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)).orElse(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/streamApiCallChains/afterOrElseNullInverted.java new file mode 100644 index 000000000000..0b641a14e5e0 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/afterOrElseNullInverted.java @@ -0,0 +1,32 @@ +// "Replace 'orElse(null)' with 'orElse(new Person("noname", 0))'" "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)).orElse(new Person("noname", 0)); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeOrElseNull.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeOrElseNull.java new file mode 100644 index 000000000000..286dfc6a9f59 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeOrElseNull.java @@ -0,0 +1,33 @@ +// "Replace 'orElse(null)' with 'orElse(new Person("noname", 0))'" "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); + return maxPerson == null? new Person("noname", 0): maxPerson; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/streamApiCallChains/beforeOrElseNullInverted.java b/java/java-tests/testData/inspection/streamApiCallChains/beforeOrElseNullInverted.java new file mode 100644 index 000000000000..6934595268a6 --- /dev/null +++ b/java/java-tests/testData/inspection/streamApiCallChains/beforeOrElseNullInverted.java @@ -0,0 +1,33 @@ +// "Replace 'orElse(null)' with 'orElse(new Person("noname", 0))'" "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); + return maxPerson != null? maxPerson : new Person("noname", 0); + } +} \ No newline at end of file