From 770acf5b8ff882d8313a0c766bba9cb838f4158e Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 29 May 2018 17:55:18 +0700 Subject: [PATCH] IDEA-191905 Optional.get without Optional.isPresent can suggest quick fix when Optional.get returns Optional --- ...OptionalGetWithoutIsPresentInspection.java | 88 ++++++++++++++++++- .../inspection/optionalGet/afterFlatMap.java | 37 ++++++++ .../inspection/optionalGet/beforeFlatMap.java | 37 ++++++++ ...lGetWithoutIsPresentInspectionFixTest.java | 33 +++++++ 4 files changed, 191 insertions(+), 4 deletions(-) create mode 100644 java/java-tests/testData/inspection/optionalGet/afterFlatMap.java create mode 100644 java/java-tests/testData/inspection/optionalGet/beforeFlatMap.java create mode 100644 java/java-tests/testSrc/com/intellij/java/codeInspection/OptionalGetWithoutIsPresentInspectionFixTest.java diff --git a/java/java-impl/src/com/intellij/codeInspection/java18api/OptionalGetWithoutIsPresentInspection.java b/java/java-impl/src/com/intellij/codeInspection/java18api/OptionalGetWithoutIsPresentInspection.java index bc4a56f98c70..5b6b7a5a8b64 100644 --- a/java/java-impl/src/com/intellij/codeInspection/java18api/OptionalGetWithoutIsPresentInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/java18api/OptionalGetWithoutIsPresentInspection.java @@ -2,18 +2,28 @@ package com.intellij.codeInspection.java18api; import com.intellij.codeInsight.PsiEquivalenceUtil; -import com.intellij.codeInspection.AbstractBaseJavaLocalInspectionTool; -import com.intellij.codeInspection.InspectionsBundle; -import com.intellij.codeInspection.ProblemsHolder; +import com.intellij.codeInspection.*; import com.intellij.codeInspection.dataFlow.CommonDataflow; import com.intellij.codeInspection.dataFlow.DfaFactType; import com.intellij.codeInspection.dataFlow.DfaOptionalSupport; +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.codeStyle.JavaCodeStyleManager; +import com.intellij.psi.codeStyle.SuggestedNameInfo; +import com.intellij.psi.codeStyle.VariableKind; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; +import com.intellij.util.IncorrectOperationException; +import com.siyeh.ig.psiutils.CommentTracker; +import com.siyeh.ig.psiutils.ExpressionUtils; import com.siyeh.ig.psiutils.TypeUtils; +import org.jetbrains.annotations.Nls; import org.jetbrains.annotations.NotNull; +import java.util.Objects; + public class OptionalGetWithoutIsPresentInspection extends AbstractBaseJavaLocalInspectionTool { @NotNull @Override @@ -37,7 +47,8 @@ public class OptionalGetWithoutIsPresentInspection extends AbstractBaseJavaLocal result.getExpressionFact(qualifier, DfaFactType.OPTIONAL_PRESENCE) == null && !isPresentCallWithSameQualifierExists(qualifier)) { holder.registerProblem(nameElement, - InspectionsBundle.message("inspection.optional.get.without.is.present.message", optionalClass.getName())); + InspectionsBundle.message("inspection.optional.get.without.is.present.message", optionalClass.getName()), + tryCreateFix(call)); } } } @@ -60,4 +71,73 @@ public class OptionalGetWithoutIsPresentInspection extends AbstractBaseJavaLocal } }; } + + private static LocalQuickFix tryCreateFix(PsiMethodCallExpression call) { + PsiExpression qualifier = call.getMethodExpression().getQualifierExpression(); + if (qualifier == null) return null; + PsiClass optionalClass = PsiUtil.resolveClassInClassTypeOnly(qualifier.getType()); + if (optionalClass == null || !CommonClassNames.JAVA_UTIL_OPTIONAL.equals(optionalClass.getQualifiedName())) return null; + PsiType optionalElementType = OptionalUtil.getOptionalElementType(qualifier.getType()); + if (optionalElementType == null) return null; + PsiMethodCallExpression nextCall = ExpressionUtils.getCallForQualifier(call); + if (nextCall != null) { + if (optionalClass.equals(PsiUtil.resolveClassInClassTypeOnly(nextCall.getType()))) { + if (!LambdaGenerationUtil.canBeUncheckedLambda(nextCall)) { + // Probably qualifier accesses non-final vars or throws exception: we will replace qualifier, so this is not a problem + PsiMethodCallExpression copy = (PsiMethodCallExpression)nextCall.copy(); + PsiExpression copyQualifier = Objects.requireNonNull(copy.getMethodExpression().getQualifierExpression()); + try { + copyQualifier.replace(JavaPsiFacade.getElementFactory(call.getProject()) + .createExpressionFromText("((" + optionalElementType.getCanonicalText() + ")null)", + copyQualifier)); + } + catch (IncorrectOperationException e) { + return null; + } + if (!LambdaGenerationUtil.canBeUncheckedLambda(copy)) { + return null; + } + } + return new UseFlatMapFix(); + } + } + return null; + } + + private static class UseFlatMapFix implements LocalQuickFix { + @Nls(capitalization = Nls.Capitalization.Sentence) + @NotNull + @Override + public String getFamilyName() { + return "Use 'flatMap'"; + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + PsiMethodCallExpression call = PsiTreeUtil.getParentOfType(descriptor.getStartElement(), PsiMethodCallExpression.class); + if (call == null) return; + PsiExpression qualifier = call.getMethodExpression().getQualifierExpression(); + if (qualifier == null) return; + PsiType elementType = OptionalUtil.getOptionalElementType(qualifier.getType()); + PsiMethodCallExpression nextCall = ExpressionUtils.getCallForQualifier(call); + if (nextCall == null) return; + JavaCodeStyleManager manager = JavaCodeStyleManager.getInstance(project); + SuggestedNameInfo info = manager.suggestVariableName(VariableKind.PARAMETER, null, qualifier, elementType, true); + String name = info.names.length == 0 ? "value" : info.names[0]; + name = manager.suggestUniqueVariableName(name, call, true); + CommentTracker ct = new CommentTracker(); + PsiReferenceExpression methodExpression = nextCall.getMethodExpression(); + ct.markRangeUnchanged(Objects.requireNonNull(methodExpression.getQualifierExpression()).getNextSibling(), + methodExpression.getLastChild()); + ct.markRangeUnchanged(methodExpression.getNextSibling(), nextCall.getLastChild()); + PsiMethodCallExpression newNextCall = (PsiMethodCallExpression)nextCall.copy(); + PsiExpression newQualifier = Objects.requireNonNull(newNextCall.getMethodExpression().getQualifierExpression()); + newQualifier.replace(JavaPsiFacade.getElementFactory(project).createExpressionFromText(name, newNextCall)); + String lambda = name + "->" + newNextCall.getText(); + String replacement = ct.text(qualifier) + ".flatMap(" + lambda + ")"; + PsiMethodCallExpression result = (PsiMethodCallExpression)ct.replaceAndRestoreComments(nextCall, replacement); + LambdaCanBeMethodReferenceInspection.replaceLambdaWithMethodReference( + (PsiLambdaExpression)result.getArgumentList().getExpressions()[0]); + } + } } diff --git a/java/java-tests/testData/inspection/optionalGet/afterFlatMap.java b/java/java-tests/testData/inspection/optionalGet/afterFlatMap.java new file mode 100644 index 000000000000..b2dca0462fa2 --- /dev/null +++ b/java/java-tests/testData/inspection/optionalGet/afterFlatMap.java @@ -0,0 +1,37 @@ +// "Fix all 'Optional.get() is called without isPresent() check' problems in file" "true" +import java.util.Optional; + +class Test { + class PropertyHolder { + T property; + + Optional getProperty() { + return Optional.ofNullable(property); + } + + Optional getProperty(String s) { + return Optional.ofNullable(property); + } + Optional getPropertyEx() throws Exception { + return Optional.ofNullable(property); + } + } + + class Smth { + PropertyHolder propertyHolder; + + Optional> getPropertyHolder() { + return Optional.ofNullable(propertyHolder); + } + + Optional> getPropertyHolderEx() throws Exception { + return Optional.ofNullable(propertyHolder); + } + } + + void test() throws Exception { + Optional property = new Smth().getPropertyHolder().flatMap(PropertyHolder::getProperty); + Optional property1 = new Smth().getPropertyHolderEx().flatMap(propertyHolderEx -> propertyHolderEx.getProperty("foo")); + Optional property2 = new Smth().getPropertyHolderEx().get().getPropertyEx(); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/inspection/optionalGet/beforeFlatMap.java b/java/java-tests/testData/inspection/optionalGet/beforeFlatMap.java new file mode 100644 index 000000000000..b55af2649795 --- /dev/null +++ b/java/java-tests/testData/inspection/optionalGet/beforeFlatMap.java @@ -0,0 +1,37 @@ +// "Fix all 'Optional.get() is called without isPresent() check' problems in file" "true" +import java.util.Optional; + +class Test { + class PropertyHolder { + T property; + + Optional getProperty() { + return Optional.ofNullable(property); + } + + Optional getProperty(String s) { + return Optional.ofNullable(property); + } + Optional getPropertyEx() throws Exception { + return Optional.ofNullable(property); + } + } + + class Smth { + PropertyHolder propertyHolder; + + Optional> getPropertyHolder() { + return Optional.ofNullable(propertyHolder); + } + + Optional> getPropertyHolderEx() throws Exception { + return Optional.ofNullable(propertyHolder); + } + } + + void test() throws Exception { + Optional property = new Smth().getPropertyHolder().get().getProperty(); + Optional property1 = new Smth().getPropertyHolderEx().get().getProperty("foo"); + Optional property2 = new Smth().getPropertyHolderEx().get().getPropertyEx(); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/codeInspection/OptionalGetWithoutIsPresentInspectionFixTest.java b/java/java-tests/testSrc/com/intellij/java/codeInspection/OptionalGetWithoutIsPresentInspectionFixTest.java new file mode 100644 index 000000000000..c6093e9babe8 --- /dev/null +++ b/java/java-tests/testSrc/com/intellij/java/codeInspection/OptionalGetWithoutIsPresentInspectionFixTest.java @@ -0,0 +1,33 @@ +// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +package com.intellij.java.codeInspection; + +import com.intellij.codeInsight.daemon.quickFix.LightQuickFixParameterizedTestCase; +import com.intellij.codeInspection.LocalInspectionTool; +import com.intellij.codeInspection.java18api.OptionalGetWithoutIsPresentInspection; +import com.intellij.testFramework.LightProjectDescriptor; +import org.jetbrains.annotations.NotNull; + +import static com.intellij.testFramework.fixtures.LightCodeInsightFixtureTestCase.JAVA_8; + +public class OptionalGetWithoutIsPresentInspectionFixTest extends LightQuickFixParameterizedTestCase { + public void test() { doAllTests(); } + + @NotNull + @Override + protected LightProjectDescriptor getProjectDescriptor() { + return JAVA_8; + } + + @NotNull + @Override + protected LocalInspectionTool[] configureLocalInspectionTools() { + return new LocalInspectionTool[]{ + new OptionalGetWithoutIsPresentInspection() + }; + } + + @Override + protected String getBasePath() { + return "/inspection/optionalGet"; + } +}