From 1d74e0af9d6fa8ada40ca6bff9cd9fd3aa51065f Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 11 Nov 2016 13:14:55 +0700 Subject: [PATCH] IDEA-163767 Simplify optional.isPresent() inspection doesn't suggest simplify trivial case --- .../OptionalIsPresentInspection.java | 28 +++++++++++-------- .../codeInspection/util/OptionalUtil.java | 9 ++++-- .../afterOptionalReturnSelfOrEmpty.java | 9 ++++++ .../beforeOptionalReturnSelf.java | 14 ++++++++++ .../beforeOptionalReturnSelfOrEmpty.java | 12 ++++++++ 5 files changed, 59 insertions(+), 13 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterOptionalReturnSelfOrEmpty.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeOptionalReturnSelf.java create mode 100644 java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeOptionalReturnSelfOrEmpty.java diff --git a/java/java-impl/src/com/intellij/codeInspection/OptionalIsPresentInspection.java b/java/java-impl/src/com/intellij/codeInspection/OptionalIsPresentInspection.java index 7a8688d851bd..9efa61efd284 100644 --- a/java/java-impl/src/com/intellij/codeInspection/OptionalIsPresentInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/OptionalIsPresentInspection.java @@ -163,9 +163,10 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo return ((PsiReferenceExpression)qualifier).isReferenceTo(variable); } - @Contract("null, _ -> false") - static boolean isOptionalLambdaCandidate(PsiExpression lambdaCandidate, PsiVariable optionalVariable) { + @Contract("_, null, _ -> false") + static boolean isOptionalLambdaCandidate(PsiVariable optionalVariable, PsiExpression lambdaCandidate, PsiExpression falseExpression) { if (lambdaCandidate == null) return false; + if (ExpressionUtils.isReferenceTo(lambdaCandidate, optionalVariable) && OptionalUtil.isOptionalEmptyCall(falseExpression)) return true; if (!ExceptionUtil.getThrownCheckedExceptions(lambdaCandidate).isEmpty()) return false; Ref hasOptionalReference = new Ref<>(Boolean.FALSE); return PsiTreeUtil.processElements(lambdaCandidate, e -> { @@ -203,11 +204,15 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo PsiExpression trueValue, PsiExpression falseValue, PsiType targetType) { + if (ExpressionUtils.isReferenceTo(trueValue, optionalVariable) && OptionalUtil.isOptionalEmptyCall(falseValue)) { + trueValue = + factory.createExpressionFromText(CommonClassNames.JAVA_UTIL_OPTIONAL + ".of(" + optionalVariable.getName() + ".get())", trueValue); + } + if (ExpressionUtils.isReferenceTo(falseValue, optionalVariable)) { + falseValue = factory.createExpressionFromText(CommonClassNames.JAVA_UTIL_OPTIONAL + ".empty()", falseValue); + } String lambdaText = generateOptionalLambda(factory, optionalVariable, trueValue); PsiLambdaExpression lambda = (PsiLambdaExpression)factory.createExpressionFromText(lambdaText, trueValue); - if(ExpressionUtils.isReferenceTo(falseValue, optionalVariable)) { - falseValue = factory.createExpressionFromText(CommonClassNames.JAVA_UTIL_OPTIONAL+".empty()", falseValue); - } return OptionalUtil.generateOptionalUnwrap(optionalVariable.getName(), lambda.getParameterList().getParameters()[0], (PsiExpression)lambda.getBody(), falseValue, targetType, true); } @@ -289,7 +294,7 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo if (!ExpressionUtils.isSimpleExpression(falseValue) && !LambdaGenerationUtil.canBeUncheckedLambda(falseValue)) return false; PsiExpression trueValue = ((PsiReturnStatement)trueElement).getReturnValue(); - return isOptionalLambdaCandidate(trueValue, optionalVariable); + return isOptionalLambdaCandidate(optionalVariable, trueValue, falseValue); } @Override @@ -316,7 +321,7 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo falseAssignment == null || !EquivalenceChecker.getCanonicalPsiEquivalence() .expressionsAreEquivalent(trueAssignment.getLExpression(), falseAssignment.getLExpression()) || - !isOptionalLambdaCandidate(trueAssignment.getRExpression(), optionalVariable)) { + !isOptionalLambdaCandidate(optionalVariable, trueAssignment.getRExpression(), falseAssignment.getLExpression())) { return false; } return ExpressionUtils.isSimpleExpression(falseAssignment.getRExpression()) || @@ -344,9 +349,10 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo @Override public boolean isApplicable(PsiVariable optionalVariable, PsiElement trueElement, PsiElement falseElement) { if(!(trueElement instanceof PsiExpression) || !(falseElement instanceof PsiExpression)) return false; - return isOptionalLambdaCandidate((PsiExpression)trueElement, optionalVariable) && - (ExpressionUtils.isSimpleExpression((PsiExpression)falseElement) || - LambdaGenerationUtil.canBeUncheckedLambda((PsiExpression)falseElement)); + PsiExpression trueExpression = (PsiExpression)trueElement; + PsiExpression falseExpression = (PsiExpression)falseElement; + return isOptionalLambdaCandidate(optionalVariable, trueExpression, falseExpression) && + (ExpressionUtils.isSimpleExpression(falseExpression) || LambdaGenerationUtil.canBeUncheckedLambda(falseExpression)); } @Override @@ -368,7 +374,7 @@ public class OptionalIsPresentInspection extends BaseJavaBatchLocalInspectionToo if (falseElement != null && !(falseElement instanceof PsiEmptyStatement)) return false; if (!(trueElement instanceof PsiExpressionStatement)) return false; PsiExpression expression = ((PsiExpressionStatement)trueElement).getExpression(); - return isOptionalLambdaCandidate(expression, optionalVariable); + return isOptionalLambdaCandidate(optionalVariable, expression, null); } @Override diff --git a/java/java-impl/src/com/intellij/codeInspection/util/OptionalUtil.java b/java/java-impl/src/com/intellij/codeInspection/util/OptionalUtil.java index 86a854e47089..421be5ac900a 100644 --- a/java/java-impl/src/com/intellij/codeInspection/util/OptionalUtil.java +++ b/java/java-impl/src/com/intellij/codeInspection/util/OptionalUtil.java @@ -113,8 +113,7 @@ public class OptionalUtil { condition.getThenExpression(), falseExpression, targetType, useOrElseGet); } } - if(falseExpression instanceof PsiMethodCallExpression && - MethodCallUtils.isCallToStaticMethod((PsiMethodCallExpression)falseExpression, CommonClassNames.JAVA_UTIL_OPTIONAL, "empty", 0)) { + if(isOptionalEmptyCall(falseExpression)) { // simplify "qualifier.map(x -> Optional.of(x)).orElse(Optional.empty())" to "qualifier" if (trueExpression instanceof PsiMethodCallExpression && MethodCallUtils.isCallToStaticMethod((PsiMethodCallExpression)trueExpression, CommonClassNames.JAVA_UTIL_OPTIONAL, "of", 1)) { @@ -138,6 +137,12 @@ public class OptionalUtil { } } + @Contract("null -> false") + public static boolean isOptionalEmptyCall(PsiExpression expression) { + return expression instanceof PsiMethodCallExpression && + MethodCallUtils.isCallToStaticMethod((PsiMethodCallExpression)expression, CommonClassNames.JAVA_UTIL_OPTIONAL, "empty", 0); + } + @NotNull public static String getMapTypeArgument(PsiExpression expression, PsiType type) { if (!(type instanceof PsiClassType)) return ""; diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterOptionalReturnSelfOrEmpty.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterOptionalReturnSelfOrEmpty.java new file mode 100644 index 000000000000..1e2a51eab313 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/afterOptionalReturnSelfOrEmpty.java @@ -0,0 +1,9 @@ +// "Replace Optional.isPresent() condition with functional style expression" "true" + +import java.util.*; + +public class Main { + Optional foo(Optional first) { + return first; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeOptionalReturnSelf.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeOptionalReturnSelf.java new file mode 100644 index 000000000000..ced3cad5af70 --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeOptionalReturnSelf.java @@ -0,0 +1,14 @@ +// "Replace Optional.isPresent() condition with functional style expression" "false" + +import java.util.*; + +public class Main { + Optional foo(Optional first) { + // could be replaced in Java-9 with return first.or(() -> "xyz"); + // but the only option in Java-8 is return first.map(Optional::of).orElseGet(() -> Optional.of("xyz")) which is weird + if (first.isPresent()) { + return first; + } + return Optional.of("xyz"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeOptionalReturnSelfOrEmpty.java b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeOptionalReturnSelfOrEmpty.java new file mode 100644 index 000000000000..ac49d516cbfe --- /dev/null +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/quickFix/optionalIsPresent/beforeOptionalReturnSelfOrEmpty.java @@ -0,0 +1,12 @@ +// "Replace Optional.isPresent() condition with functional style expression" "true" + +import java.util.*; + +public class Main { + Optional foo(Optional first) { + if (first.isPresent()) { + return first; + } + return Optional.empty(); + } +} \ No newline at end of file