From fdfa078e77b9a4a21910468339f5ba9183d7cc7b Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Tue, 7 Mar 2017 10:51:47 +0700 Subject: [PATCH] Extract chained operation: more specific message in menu item; collection support --- .../chainCall/ChainCallExtractor.java | 22 +++++- .../CollectionChainCallExtractor.java | 49 ++++++++++++++ .../CompletionStageChainCallExtractor.java | 25 ++----- .../chainCall/StreamChainCallExtractor.java | 7 ++ .../IntroduceVariableBase.java | 67 +++++++++++++------ .../com/siyeh/ig/psiutils/StreamApiUtil.java | 52 ++++++++++---- resources/src/META-INF/IdeaPlugin.xml | 1 + 7 files changed, 168 insertions(+), 55 deletions(-) create mode 100644 java/java-impl/src/com/intellij/refactoring/chainCall/CollectionChainCallExtractor.java diff --git a/java/java-impl/src/com/intellij/refactoring/chainCall/ChainCallExtractor.java b/java/java-impl/src/com/intellij/refactoring/chainCall/ChainCallExtractor.java index 1beb7b73866d..9d1a40874fd7 100644 --- a/java/java-impl/src/com/intellij/refactoring/chainCall/ChainCallExtractor.java +++ b/java/java-impl/src/com/intellij/refactoring/chainCall/ChainCallExtractor.java @@ -16,12 +16,14 @@ package com.intellij.refactoring.chainCall; import com.intellij.codeInspection.LambdaCanBeMethodReferenceInspection; +import com.intellij.codeInspection.util.OptionalUtil; import com.intellij.openapi.extensions.ExtensionPointName; import com.intellij.openapi.project.Project; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.util.PsiTreeUtil; import com.intellij.refactoring.util.LambdaRefactoringUtil; +import com.intellij.refactoring.util.RefactoringUtil; import com.intellij.util.ArrayUtil; import com.siyeh.ig.psiutils.ExpressionUtils; import org.jetbrains.annotations.Contract; @@ -55,7 +57,25 @@ public interface ChainCallExtractor { * @return chain call. Result is correct only if {@link #canExtractChainCall} was checked before * for given expression and expressionType */ - String buildChainCall(PsiVariable variable, PsiExpression expression, PsiType expressionType); + default String buildChainCall(PsiVariable variable, PsiExpression expression, PsiType expressionType) { + if(expression instanceof PsiArrayInitializerExpression) { + expression = RefactoringUtil.convertInitializerToNormalExpression(expression, expressionType); + } + String typeArgument = OptionalUtil.getMapTypeArgument(expression, expressionType); + return "." + typeArgument + getMethodName(variable, expression, expressionType) + + "(" + variable.getName() + "->" + expression.getText() + ")"; + } + + /** + * Returns a name of the method to be extracted + * + * @param variable variable to be used as chain call input + * @param expression mapping expression + * @param expressionType target expression type + * @return chain call. Result is correct only if {@link #canExtractChainCall} was checked before + * for given expression and expressionType + */ + String getMethodName(PsiVariable variable, PsiExpression expression, PsiType expressionType); /** * Returns new name for the existing call from which element is to be extracted. Sometimes it should be renamed diff --git a/java/java-impl/src/com/intellij/refactoring/chainCall/CollectionChainCallExtractor.java b/java/java-impl/src/com/intellij/refactoring/chainCall/CollectionChainCallExtractor.java new file mode 100644 index 000000000000..90c6a9915ab0 --- /dev/null +++ b/java/java-impl/src/com/intellij/refactoring/chainCall/CollectionChainCallExtractor.java @@ -0,0 +1,49 @@ +/* + * Copyright 2000-2017 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.intellij.refactoring.chainCall; + +import com.intellij.psi.*; +import com.intellij.psi.util.InheritanceUtil; +import com.siyeh.ig.psiutils.StreamApiUtil; +import org.jetbrains.annotations.NotNull; + +/** + * @author Tagir Valeev + */ +public class CollectionChainCallExtractor implements ChainCallExtractor { + @Override + public boolean canExtractChainCall(@NotNull PsiMethodCallExpression call, PsiExpression expression, PsiType expressionType) { + PsiReferenceExpression methodExpression = call.getMethodExpression(); + if (!StreamApiUtil.isSupportedStreamElement(expressionType) || + !"forEach".equals(methodExpression.getReferenceName()) || + methodExpression.getQualifierExpression() == null || + !InheritanceUtil.isInheritor(methodExpression.getQualifierExpression().getType(), CommonClassNames.JAVA_UTIL_COLLECTION)) { + return false; + } + PsiMethod method = call.resolveMethod(); + return method != null && method.getParameterList().getParametersCount() == 1; + } + + @Override + public String getMethodName(PsiVariable variable, PsiExpression expression, PsiType expressionType) { + return "stream()." + StreamApiUtil.getMapOperationName(variable.getType(), expressionType); + } + + @Override + public String buildChainCall(PsiVariable variable, PsiExpression expression, PsiType expressionType) { + return ".stream()" + StreamApiUtil.generateMapOperation(variable, expressionType, expression); + } +} diff --git a/java/java-impl/src/com/intellij/refactoring/chainCall/CompletionStageChainCallExtractor.java b/java/java-impl/src/com/intellij/refactoring/chainCall/CompletionStageChainCallExtractor.java index b35c31d7481c..88afaaedaaf3 100644 --- a/java/java-impl/src/com/intellij/refactoring/chainCall/CompletionStageChainCallExtractor.java +++ b/java/java-impl/src/com/intellij/refactoring/chainCall/CompletionStageChainCallExtractor.java @@ -15,37 +15,26 @@ */ package com.intellij.refactoring.chainCall; -import com.intellij.codeInspection.util.OptionalUtil; import com.intellij.psi.*; -import com.intellij.psi.util.InheritanceUtil; -import com.intellij.refactoring.util.RefactoringUtil; +import com.siyeh.ig.callMatcher.CallMatcher; import org.jetbrains.annotations.NotNull; /** * @author Tagir Valeev */ public class CompletionStageChainCallExtractor implements ChainCallExtractor { + private static final CallMatcher NEXT_CALL = + CallMatcher.instanceCall("java.util.concurrent.CompletionStage", "thenApply", "thenAccept", "thenCompose").parameterCount(1); + @Override public boolean canExtractChainCall(@NotNull PsiMethodCallExpression call, PsiExpression expression, PsiType expressionType) { if (expressionType instanceof PsiPrimitiveType) return false; - String methodName = call.getMethodExpression().getReferenceName(); - if (!"thenApply".equals(methodName) && !"thenAccept".equals(methodName) && !"thenCompose".equals(methodName)) { - return false; - } if (call.getMethodExpression().getQualifierExpression() == null) return false; - PsiMethod method = call.resolveMethod(); - return method != null && - method.getParameterList().getParametersCount() == 1 && - InheritanceUtil.isInheritor(method.getContainingClass(), "java.util.concurrent.CompletionStage"); + return NEXT_CALL.test(call); } @Override - public String buildChainCall(PsiVariable variable, PsiExpression expression, PsiType expressionType) { - if(expression instanceof PsiArrayInitializerExpression) { - expression = RefactoringUtil.convertInitializerToNormalExpression(expression, expressionType); - } - String typeArgument = OptionalUtil.getMapTypeArgument(expression, expressionType); - return "." + typeArgument + "thenApply" + - "(" + variable.getName() + "->" + expression.getText() + ")"; + public String getMethodName(PsiVariable variable, PsiExpression expression, PsiType expressionType) { + return "thenApply"; } } diff --git a/java/java-impl/src/com/intellij/refactoring/chainCall/StreamChainCallExtractor.java b/java/java-impl/src/com/intellij/refactoring/chainCall/StreamChainCallExtractor.java index e38a5733c037..a86872b70377 100644 --- a/java/java-impl/src/com/intellij/refactoring/chainCall/StreamChainCallExtractor.java +++ b/java/java-impl/src/com/intellij/refactoring/chainCall/StreamChainCallExtractor.java @@ -61,6 +61,13 @@ public class StreamChainCallExtractor implements ChainCallExtractor { return methodName; } + @Override + public String getMethodName(PsiVariable variable, PsiExpression expression, PsiType expressionType) { + String shortcutMappingMethod = StreamApiUtil.getShortcutMappingMethod(variable, expressionType, expression); + if(shortcutMappingMethod != null) return shortcutMappingMethod; + return StreamApiUtil.getMapOperationName(variable.getType(), expressionType); + } + @Override public String buildChainCall(PsiVariable variable, PsiExpression expression, PsiType expressionType) { return StreamApiUtil.generateMapOperation(variable, expressionType, expression); diff --git a/java/java-impl/src/com/intellij/refactoring/introduceVariable/IntroduceVariableBase.java b/java/java-impl/src/com/intellij/refactoring/introduceVariable/IntroduceVariableBase.java index 05f745235aee..fc781c3e5c1a 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceVariable/IntroduceVariableBase.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceVariable/IntroduceVariableBase.java @@ -87,15 +87,27 @@ import java.util.*; * Date: Nov 15, 2002 */ public abstract class IntroduceVariableBase extends IntroduceHandlerBase { - public enum JavaReplaceChoice implements OccurrencesChooser.BaseReplaceChoice { - NO("Replace this occurrence only"), NO_WRITE("Replace all occurrences but write"), ALL("Replace all {0} occurrences"), - NO_CHAIN("Create variable inside current lambda"), CHAIN("Extract as separate step"), - CHAIN_ALL("Replace all {0} occurrences and extract as separate step"); + public static class JavaReplaceChoice implements OccurrencesChooser.BaseReplaceChoice { + public static final JavaReplaceChoice NO = new JavaReplaceChoice(OccurrencesChooser.ReplaceChoice.NO); + public static final JavaReplaceChoice NO_WRITE = new JavaReplaceChoice(OccurrencesChooser.ReplaceChoice.NO_WRITE); + public static final JavaReplaceChoice ALL = new JavaReplaceChoice(OccurrencesChooser.ReplaceChoice.ALL); + public static final JavaReplaceChoice NO_CHAIN = new JavaReplaceChoice("Create variable inside current lambda", false, false, false); + public static final JavaReplaceChoice CHAIN = new JavaReplaceChoice("Extract as a separate operation", false, false, true); + public static final JavaReplaceChoice CHAIN_ALL = + new JavaReplaceChoice("Replace all occurrences and extract as a separate operation", true, true, true); private final String myDescription; + private final boolean myAll, myMultiple, myChain; - JavaReplaceChoice(String description) { + JavaReplaceChoice(OccurrencesChooser.ReplaceChoice choice) { + this(choice.getDescription(), choice.isAll(), choice.isMultiple(), false); + } + + JavaReplaceChoice(String description, boolean all, boolean multiple, boolean chain) { myDescription = description; + myAll = all; + myMultiple = multiple; + myChain = chain; } public String getDescription() { @@ -104,12 +116,16 @@ public abstract class IntroduceVariableBase extends IntroduceHandlerBase { @Override public boolean isMultiple() { - return this == NO_WRITE || this == ALL || this == CHAIN_ALL; + return myMultiple; } @Override public boolean isAll() { - return this == ALL || this == CHAIN_ALL; + return myAll; + } + + public boolean isChain() { + return myChain; } @Override @@ -673,7 +689,7 @@ public abstract class IntroduceVariableBase extends IntroduceHandlerBase { .filter(occurrence -> !(expr.equals(occurrence) && expr.getParent() instanceof PsiExpressionStatement)) .filter(occurrence -> allChoice || (noWriteChoice && !PsiUtil.isAccessedForWriting(occurrence)) || expr.equals(occurrence)) .toArray(PsiExpression[]::new); - if (choice == JavaReplaceChoice.CHAIN || choice == JavaReplaceChoice.CHAIN_ALL) { + if (choice.isChain()) { myInplaceIntroducer = new ChainCallInplaceIntroducer(project, settings, chosenAnchor, @@ -747,7 +763,7 @@ public abstract class IntroduceVariableBase extends IntroduceHandlerBase { if (choice != null) { callback.pass(choice); } else { - String title = occurrencesInfo.myChainCallCandidate && occurrences.length == 1 + String title = occurrencesInfo.myChainMethodName != null && occurrences.length == 1 ? "Lambda chain detected" : OccurrencesChooser.DEFAULT_CHOOSER_TITLE; OccurrencesChooser.simpleChooser(editor).showChooser(callback, occurrencesMap, title); @@ -1250,7 +1266,7 @@ public abstract class IntroduceVariableBase extends IntroduceHandlerBase { boolean myCantReplaceAll; boolean myCantReplaceAllButWrite; boolean myHasWriteAccess; - boolean myChainCallCandidate; + final String myChainMethodName; public OccurrencesInfo(PsiExpression[] occurrences) { myOccurrences = Arrays.asList(occurrences); @@ -1268,35 +1284,42 @@ public abstract class IntroduceVariableBase extends IntroduceHandlerBase { } } myHasWriteAccess = myOccurrences.size() > myNonWrite.size() && myOccurrences.size() > 1; - myChainCallCandidate = CHAIN_ALLOWED && isChainCallCandidate(); + myChainMethodName = CHAIN_ALLOWED ? getChainCallExtractor() : null; } - private boolean isChainCallCandidate() { - if (myHasWriteAccess || myOccurrences.isEmpty()) return false; - // The whole lambda body selected + private String getChainCallExtractor() { + if (myHasWriteAccess || myOccurrences.isEmpty()) return null; PsiExpression expression = myOccurrences.get(0); - if (myOccurrences.size() == 1 && expression.getParent() instanceof PsiLambdaExpression) return false; + // The whole lambda body selected + if (myOccurrences.size() == 1 && expression.getParent() instanceof PsiLambdaExpression) return null; PsiElement parent = PsiTreeUtil.findCommonParent(myOccurrences); - if (parent == null) return false; + if (parent == null) return null; PsiType type = expression.getType(); PsiLambdaExpression lambda = PsiTreeUtil.getParentOfType(parent, PsiLambdaExpression.class, true, PsiStatement.class); - if (ChainCallExtractor.findExtractor(lambda, expression, type) == null) return false; + ChainCallExtractor extractor = ChainCallExtractor.findExtractor(lambda, expression, type); + if (extractor == null) return null; PsiParameter parameter = lambda.getParameterList().getParameters()[0]; - return ReferencesSearch.search(parameter).forEach((Processor)ref -> - myOccurrences.stream().anyMatch(expr -> PsiTreeUtil.isAncestor(expr, ref.getElement(), false))); + if (!ReferencesSearch.search(parameter).forEach((Processor)ref -> + myOccurrences.stream().anyMatch(expr -> PsiTreeUtil.isAncestor(expr, ref.getElement(), false)))) { + return null; + } + return extractor.getMethodName(parameter, expression, type); } @NotNull LinkedHashMap> buildOccurrencesMap(PsiExpression expr) { final LinkedHashMap> occurrencesMap = ContainerUtil.newLinkedHashMap(); - if (myChainCallCandidate) { + if (myChainMethodName != null) { if (myOccurrences.size() > 1 && !myCantReplaceAll) { occurrencesMap.put(JavaReplaceChoice.NO, Collections.singletonList(expr)); occurrencesMap.put(JavaReplaceChoice.ALL, myOccurrences); - occurrencesMap.put(JavaReplaceChoice.CHAIN_ALL, myOccurrences); + occurrencesMap.put( + new JavaReplaceChoice("Replace all {0} occurrences and extract as ''" + myChainMethodName + "'' operation", true, true, true), + myOccurrences); } else { occurrencesMap.put(JavaReplaceChoice.NO_CHAIN, Collections.singletonList(expr)); - occurrencesMap.put(JavaReplaceChoice.CHAIN, Collections.singletonList(expr)); + occurrencesMap.put(new JavaReplaceChoice("Extract as ''" + myChainMethodName + "'' operation", false, false, true), + Collections.singletonList(expr)); } } else { occurrencesMap.put(JavaReplaceChoice.NO, Collections.singletonList(expr)); diff --git a/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/StreamApiUtil.java b/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/StreamApiUtil.java index 7687fd40d53a..1c8582022006 100644 --- a/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/StreamApiUtil.java +++ b/plugins/InspectionGadgets/src/com/siyeh/ig/psiutils/StreamApiUtil.java @@ -23,6 +23,8 @@ import org.jetbrains.annotations.Contract; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import java.util.Objects; + /** * @author Tagir Valeev */ @@ -87,21 +89,9 @@ public class StreamApiUtil { @NotNull public static String generateMapOperation(PsiVariable variable, @Nullable PsiType outType, PsiElement mapper) { + String shortcutMappingMethod = getShortcutMappingMethod(variable, outType, mapper); + if (shortcutMappingMethod != null) return shortcutMappingMethod.isEmpty() ? "" : "." + shortcutMappingMethod + "()"; PsiType inType = variable.getType(); - if (mapper instanceof PsiExpression && ExpressionUtils.isReferenceTo((PsiExpression)mapper, variable)) { - if (!(outType instanceof PsiPrimitiveType)) { - return inType instanceof PsiPrimitiveType ? ".boxed()" : ""; - } - if(outType.equals(inType)) { - return ""; - } - if (PsiType.LONG.equals(outType) && PsiType.INT.equals(inType)) { - return ".asLongStream()"; - } - if (PsiType.DOUBLE.equals(outType) && (PsiType.LONG.equals(inType) || PsiType.INT.equals(inType))) { - return ".asDoubleStream()"; - } - } String operationName = getMapOperationName(inType, outType); if(outType != null && mapper instanceof PsiArrayInitializerExpression) { mapper = RefactoringUtil.convertInitializerToNormalExpression((PsiExpression)mapper, outType); @@ -111,6 +101,40 @@ public class StreamApiUtil { "(" + variable.getName() + "->" + mapper.getText() + ")"; } + /** + * Returns the shortcut mapping method name + * + * @param variable mapper input variable + * @param outType output type of the mapper + * @param mapper mapper code + * @return shortcut mapping name ("boxed", "asLongStream", "asDoubleStream") if applicable, empty string if it's + * ditto mapping (no mapping is necessary at all) and null if no shortcut is applicable for given mapper + */ + @Nullable + public static String getShortcutMappingMethod(PsiVariable variable, @Nullable PsiType outType, PsiElement mapper) { + if (!(mapper instanceof PsiExpression)) return null; + PsiExpression expression = PsiUtil.skipParenthesizedExprDown(((PsiExpression)mapper)); + if (expression instanceof PsiTypeCastExpression && Objects.equals(expression.getType(), outType)) { + expression = ((PsiTypeCastExpression)expression).getOperand(); + } + if (ExpressionUtils.isReferenceTo(expression, variable)) { + PsiType inType = variable.getType(); + if (!(outType instanceof PsiPrimitiveType)) { + return inType instanceof PsiPrimitiveType ? "boxed" : ""; + } + if (outType.equals(inType)) { + return ""; + } + if (PsiType.LONG.equals(outType) && PsiType.INT.equals(inType)) { + return "asLongStream"; + } + if (PsiType.DOUBLE.equals(outType) && (PsiType.LONG.equals(inType) || PsiType.INT.equals(inType))) { + return "asDoubleStream"; + } + } + return null; + } + @NotNull public static String getMapOperationName(PsiType inType, @Nullable PsiType outType) { if(outType instanceof PsiPrimitiveType) { diff --git a/resources/src/META-INF/IdeaPlugin.xml b/resources/src/META-INF/IdeaPlugin.xml index 9fedb89710bc..cbf61b5ab8fd 100644 --- a/resources/src/META-INF/IdeaPlugin.xml +++ b/resources/src/META-INF/IdeaPlugin.xml @@ -1900,6 +1900,7 @@ +