From 5e2fa7be19452105769b660a0ce7b1a4dd9a0dba Mon Sep 17 00:00:00 2001 From: Anna Kozlova Date: Fri, 5 Dec 2014 13:45:53 +0100 Subject: [PATCH] extract method: new signature detector (IDEA-66281); initial --- .../util/duplicates/DuplicatesFinder.java | 24 +- .../extractMethod/ExtractMethodProcessor.java | 31 +- .../ExtractMethodSignatureSuggester.java | 328 ++++++++++++++++++ .../ExtractMethodObjectProcessor.java | 2 + .../duplicates/ConstantMatchProvider.java | 2 +- .../util/duplicates/DuplicatesImpl.java | 14 +- .../util/duplicates/MatchProvider.java | 5 +- .../duplicates/MethodDuplicatesHandler.java | 23 +- .../MethodDuplicatesMatchProvider.java | 2 +- ...ignatureLeaveSameExpressionsUntouched.java | 13 + ...reLeaveSameExpressionsUntouched_after.java | 15 + .../SuggestChangeSignatureOneParam.java | 11 + ...eSignatureOneParamMultipleTimesInside.java | 13 + ...tureOneParamMultipleTimesInside_after.java | 15 + .../SuggestChangeSignatureOneParam_after.java | 14 + .../SuggestChangeSignatureSameParamNames.java | 13 + ...stChangeSignatureSameParamNames_after.java | 15 + ...angeSignatureWithChangedParameterName.java | 13 + ...gnatureWithChangedParameterName_after.java | 16 + .../refactoring/ExtractMethodTest.java | 48 ++- 20 files changed, 585 insertions(+), 32 deletions(-) create mode 100644 java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodSignatureSuggester.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureLeaveSameExpressionsUntouched.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureLeaveSameExpressionsUntouched_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParam.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParamMultipleTimesInside.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParamMultipleTimesInside_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParam_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureSameParamNames.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureSameParamNames_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureWithChangedParameterName.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureWithChangedParameterName_after.java diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java index 846e1fd5d0ee..38c0cf4abbf4 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesFinder.java @@ -97,7 +97,18 @@ public class DuplicatesFinder { } + public InputVariables getParameters() { + return myParameters; + } + public PsiElement[] getPattern() { + return myPattern; + } + + @Nullable + public ReturnValue getReturnValue() { + return myReturnValue; + } public List findDuplicates(PsiElement scope) { annotatePattern(); @@ -164,9 +175,7 @@ public class DuplicatesFinder { @Nullable private Match isDuplicateFragment(PsiElement candidate, boolean ignoreParameterTypesAndPostVariableUsages) { - for (PsiElement pattern : myPattern) { - if (PsiTreeUtil.isAncestor(pattern, candidate, false)) return null; - } + if (isSelf(candidate)) return null; PsiElement sibling = candidate; ArrayList candidates = new ArrayList(); for (final PsiElement element : myPattern) { @@ -206,6 +215,15 @@ public class DuplicatesFinder { return match; } + protected boolean isSelf(PsiElement candidate) { + for (PsiElement pattern : myPattern) { + if (PsiTreeUtil.isAncestor(pattern, candidate, false)) { + return true; + } + } + return false; + } + private boolean checkPostVariableUsages(final ArrayList candidates, final Match match) { final PsiElement codeFragment = ControlFlowUtil.findCodeFragment(candidates.get(0)); try { diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java index 0fc07c490148..3af0c948d209 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java @@ -47,7 +47,10 @@ import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.openapi.wm.WindowManager; import com.intellij.psi.*; -import com.intellij.psi.codeStyle.*; +import com.intellij.psi.codeStyle.CodeStyleManager; +import com.intellij.psi.codeStyle.CodeStyleSettingsManager; +import com.intellij.psi.codeStyle.JavaCodeStyleManager; +import com.intellij.psi.codeStyle.VariableKind; import com.intellij.psi.controlFlow.ControlFlowUtil; import com.intellij.psi.impl.source.codeStyle.JavaCodeStyleManagerImpl; import com.intellij.psi.scope.processor.VariablesProcessor; @@ -725,6 +728,11 @@ public class ExtractMethodProcessor implements MatchProvider { myVariableDatum[i].passAsParameter = false; } + @TestOnly + public void changeParamName(int i, String param) { + myVariableDatum[i].name = param; + } + /** * Invoked in command and in atomic action */ @@ -1663,15 +1671,28 @@ public class ExtractMethodProcessor implements MatchProvider { return myExtractedMethod; } - public boolean hasDuplicates() { - final List duplicates = getDuplicates(); - return duplicates != null && !duplicates.isEmpty(); + public Boolean hasDuplicates() { + List duplicates = getDuplicates(); + if (duplicates != null && !duplicates.isEmpty()) { + return true; + } + final ExtractMethodSignatureSuggester suggester = new ExtractMethodSignatureSuggester(myProject, myExtractedMethod, myMethodCall, myVariableDatum); + duplicates = suggester.getDuplicates(myExtractedMethod, myMethodCall); + if (duplicates != null && !duplicates.isEmpty()) { + myDuplicates = duplicates; + myExtractedMethod = suggester.getExtractedMethod(); + myMethodCall = suggester.getMethodCall(); + myVariableDatum = suggester.getVariableData(); + return null; + } + return false; } public boolean hasDuplicates(Set files) { final DuplicatesFinder finder = initDuplicates(); - if (hasDuplicates()) return true; + final Boolean hasDuplicates = hasDuplicates(); + if (hasDuplicates == null || hasDuplicates) return true; if (finder != null) { final PsiManager psiManager = PsiManager.getInstance(myProject); for (VirtualFile file : files) { diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodSignatureSuggester.java b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodSignatureSuggester.java new file mode 100644 index 000000000000..eecedb1f4f4f --- /dev/null +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodSignatureSuggester.java @@ -0,0 +1,328 @@ +/* + * Copyright 2000-2014 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.extractMethod; + +import com.intellij.codeInsight.JavaPsiEquivalenceUtil; +import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.command.WriteCommandAction; +import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.project.Project; +import com.intellij.openapi.ui.Messages; +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.search.LocalSearchScope; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.PsiUtil; +import com.intellij.refactoring.util.RefactoringUtil; +import com.intellij.refactoring.util.VariableData; +import com.intellij.refactoring.util.duplicates.DuplicatesFinder; +import com.intellij.refactoring.util.duplicates.Match; +import com.intellij.refactoring.util.duplicates.MethodDuplicatesHandler; +import com.intellij.util.text.UniqueNameGenerator; +import gnu.trove.THashMap; +import gnu.trove.THashSet; +import gnu.trove.TObjectHashingStrategy; +import org.jetbrains.annotations.Nullable; + +import java.util.*; + +public class ExtractMethodSignatureSuggester { + private static final Logger LOG = Logger.getInstance("#" + ExtractMethodSignatureSuggester.class.getName()); + private static final TObjectHashingStrategy ourEquivalenceStrategy = new TObjectHashingStrategy() { + @Override + public int computeHashCode(PsiExpression object) { + return RefactoringUtil.unparenthesizeExpression(object).getClass().hashCode(); + } + + @Override + public boolean equals(PsiExpression o1, PsiExpression o2) { + return JavaPsiEquivalenceUtil + .areExpressionsEquivalent(RefactoringUtil.unparenthesizeExpression(o1), RefactoringUtil.unparenthesizeExpression(o2)); + } + }; + + private Project myProject; + private PsiElementFactory myElementFactory; + + private PsiMethod myExtractedMethod; + private PsiMethodCallExpression myMethodCall; + private VariableData[] myVariableData; + + public ExtractMethodSignatureSuggester(Project project, + PsiMethod extractedMethod, + PsiMethodCallExpression methodCall, + VariableData[] variableDatum) { + myProject = project; + myElementFactory = JavaPsiFacade.getElementFactory(project); + + myExtractedMethod = (PsiMethod)extractedMethod.copy(); + myMethodCall = methodCall; + myVariableData = variableDatum; + } + + public List getDuplicates(final PsiMethod method, final PsiMethodCallExpression methodCall) { + final List duplicates = findDuplicatesSignature(method); + if (duplicates != null && !duplicates.isEmpty()) { + if (ApplicationManager.getApplication().isUnitTestMode() || + Messages.showYesNoDialog(myProject, "No exact duplicates found.\nWould you like to apply suggested changes to replace " + duplicates.size() + " duplicates?", "Extract Parameters to Replace Duplicates", + Messages.getQuestionIcon()) == Messages.YES) { + WriteCommandAction.runWriteCommandAction(myProject, new Runnable() { + @Override + public void run() { + myMethodCall = (PsiMethodCallExpression)methodCall.replace(myMethodCall); + myExtractedMethod = (PsiMethod)method.replace(myExtractedMethod); + } + }); + + final DuplicatesFinder finder = MethodDuplicatesHandler.createDuplicatesFinder(myExtractedMethod); + if (finder != null) { + final List datas = finder.getParameters().getInputVariables(); + myVariableData = datas.toArray(new VariableData[datas.size()]); + return finder.findDuplicates(myExtractedMethod.getContainingClass()); + } + } + } + return null; + } + + + public PsiMethod getExtractedMethod() { + return myExtractedMethod; + } + + public PsiMethodCallExpression getMethodCall() { + return myMethodCall; + } + + public VariableData[] getVariableData() { + return myVariableData; + } + + @Nullable + public List findDuplicatesSignature(final PsiMethod method) { + final List copies = new ArrayList(); + final InputVariables variables = detectTopLevelExpressionsToReplaceWithParameters(copies); + if (variables == null) { + return null; + } + + final DuplicatesFinder defaultFinder = MethodDuplicatesHandler.createDuplicatesFinder(myExtractedMethod); + if (defaultFinder == null) { + return null; + } + + final DuplicatesFinder finder = new DuplicatesFinder(defaultFinder.getPattern(), variables, defaultFinder.getReturnValue(), new ArrayList()) { + @Override + protected boolean isSelf(PsiElement candidate) { + return PsiTreeUtil.isAncestor(method, candidate, true); + } + }; + List duplicates = finder.findDuplicates(method.getContainingClass()); + + if (duplicates != null && !duplicates.isEmpty()) { + restoreRenamedParams(copies); + inlineSameArguments(method, copies, variables, duplicates); + myMethodCall = (PsiMethodCallExpression)myMethodCall.copy(); + for (PsiExpression expression : copies) { + myMethodCall.getArgumentList().add(expression); + } + return duplicates; + } + else { + return null; + } + } + + private void inlineSameArguments(PsiMethod method, List copies, InputVariables variables, List duplicates) { + final List variableDatum = variables.getInputVariables(); + final Map toInline = new HashMap(); + final int strongParamsCound = method.getParameterList().getParametersCount(); + for (int i = strongParamsCound; i < variableDatum.size(); i++) { + VariableData variableData = variableDatum.get(i); + final THashSet map = new THashSet(ourEquivalenceStrategy); + if (!collectParamValues(duplicates, variableData, map)) { + continue; + } + + final PsiExpression currentExpression = copies.get(i - strongParamsCound); + map.add(currentExpression); + + if (map.size() == 1) { + toInline.put(variableData.variable, currentExpression); + } + } + + if (!toInline.isEmpty()) { + copies.removeAll(toInline.values()); + inlineArgumentsInMethodBody(toInline); + removeRedundantParametersFromMethodSignature(toInline); + } + } + + private void removeRedundantParametersFromMethodSignature(Map param2ExprMap) { + for (PsiParameter parameter : myExtractedMethod.getParameterList().getParameters()) { + if (param2ExprMap.containsKey(parameter)) { + parameter.delete(); + } + } + } + + private void inlineArgumentsInMethodBody(final Map param2ExprMap) { + final Map replacement = new HashMap(); + myExtractedMethod.accept(new JavaRecursiveElementWalkingVisitor() { + @Override + public void visitReferenceExpression(PsiReferenceExpression expression) { + super.visitReferenceExpression(expression); + final PsiElement resolve = expression.resolve(); + if (resolve instanceof PsiVariable) { + final PsiExpression toInlineExpr = param2ExprMap.get((PsiVariable)resolve); + if (toInlineExpr != null) { + replacement.put(expression, toInlineExpr); + } + } + } + }); + for (PsiExpression expression : replacement.keySet()) { + expression.replace(replacement.get(expression)); + } + } + + private static boolean collectParamValues(List duplicates, VariableData variableData, THashSet map) { + for (Match duplicate : duplicates) { + final List values = duplicate.getParameterValues(variableData.variable); + if (values == null || values.isEmpty()) { + return false; + } + boolean found = false; + for (PsiElement value : values) { + if (value instanceof PsiExpression) { + map.add((PsiExpression)value); + found = true; + break; + } + } + if (!found) return false; + } + return true; + } + + private void restoreRenamedParams(List copies) { + final Map renameMap = new HashMap(); + for (VariableData data : myVariableData) { + if (!data.name.equals(data.variable.getName())) { + renameMap.put(data.name, data.variable); + } + } + + if (!renameMap.isEmpty()) { + for (PsiExpression currentExpression : copies) { + final Map params = new HashMap(); + currentExpression.accept(new JavaRecursiveElementWalkingVisitor() { + @Override + public void visitReferenceExpression(PsiReferenceExpression expression) { + super.visitReferenceExpression(expression); + final PsiElement resolve = expression.resolve(); + if (resolve instanceof PsiParameter && myExtractedMethod.equals(((PsiParameter)resolve).getDeclarationScope())) { + final String name = ((PsiParameter)resolve).getName(); + final PsiVariable variable = renameMap.get(name); + if (renameMap.containsKey(name)) { + params.put(expression, variable); + } + } + } + }); + for (PsiReferenceExpression expression : params.keySet()) { + final PsiVariable var = params.get(expression); + expression.replace(myElementFactory.createExpressionFromText(var.getName(), expression)); + } + } + } + } + + + @Nullable + private InputVariables detectTopLevelExpressionsToReplaceWithParameters(List copies) { + final PsiParameter[] parameters = myExtractedMethod.getParameterList().getParameters(); + final List inputVariables = new ArrayList(Arrays.asList(parameters)); + final PsiCodeBlock body = myExtractedMethod.getBody(); + LOG.assertTrue(body != null); + final PsiStatement[] pattern = body.getStatements(); + final List exprs = new ArrayList(); + for (PsiStatement statement : pattern) { + if (statement instanceof PsiExpressionStatement) { + final PsiExpression expression = ((PsiExpressionStatement)statement).getExpression(); + if (expression instanceof PsiIfStatement || expression instanceof PsiLoopStatement) { + continue; + } + } + statement.accept(new JavaRecursiveElementWalkingVisitor() { + @Override + public void visitCallExpression(PsiCallExpression callExpression) { + final PsiExpressionList list = callExpression.getArgumentList(); + if (list != null) { + for (PsiExpression expression : list.getExpressions()) { + if (expression instanceof PsiReferenceExpression) { + final PsiElement resolve = ((PsiReferenceExpression)expression).resolve(); + if (resolve instanceof PsiField) { + exprs.add(expression); + } + } else { + exprs.add(expression); + } + } + } + } + }); + } + + if (exprs.isEmpty()) { + return null; + } + + final UniqueNameGenerator uniqueNameGenerator = new UniqueNameGenerator(); + for (PsiParameter parameter : parameters) { + uniqueNameGenerator.addExistingName(parameter.getName()); + } + final THashMap unique = new THashMap(ourEquivalenceStrategy); + final Map replacement = new HashMap(); + for (PsiExpression expr : exprs) { + String name = unique.get(expr); + if (name == null) { + + final PsiType type = GenericsUtil.getVariableTypeByExpressionType(expr.getType()); + if (type == null || type == PsiType.NULL || PsiUtil.resolveClassInType(type) instanceof PsiAnonymousClass) return null; + + copies.add(myElementFactory.createExpressionFromText(expr.getText(), body)); + + final SuggestedNameInfo info = JavaCodeStyleManager.getInstance(myProject).suggestVariableName(VariableKind.PARAMETER, null, expr, null); + name = uniqueNameGenerator.generateUniqueName(info.names[0]); + + final PsiParameter parameter = (PsiParameter)myExtractedMethod.getParameterList().add(myElementFactory.createParameter(name, type)); + inputVariables.add(parameter); + unique.put(expr, name); + } + replacement.put(expr, name); + } + + for (PsiExpression expression : replacement.keySet()) { + expression.replace(myElementFactory.createExpressionFromText(replacement.get(expression), null)); + } + + return new InputVariables(inputVariables, myExtractedMethod.getProject(), new LocalSearchScope(myExtractedMethod), false); + } +} diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethodObject/ExtractMethodObjectProcessor.java b/java/java-impl/src/com/intellij/refactoring/extractMethodObject/ExtractMethodObjectProcessor.java index 2a8741b88451..c7079345e98b 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethodObject/ExtractMethodObjectProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethodObject/ExtractMethodObjectProcessor.java @@ -28,6 +28,7 @@ import com.intellij.openapi.editor.Editor; import com.intellij.openapi.project.Project; import com.intellij.openapi.ui.DialogWrapper; import com.intellij.openapi.util.Comparing; +import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.text.StringUtil; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; @@ -61,6 +62,7 @@ import com.intellij.util.IncorrectOperationException; import com.intellij.util.VisibilityUtil; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import javax.swing.*; import java.util.*; diff --git a/java/java-impl/src/com/intellij/refactoring/util/duplicates/ConstantMatchProvider.java b/java/java-impl/src/com/intellij/refactoring/util/duplicates/ConstantMatchProvider.java index 482cb6409b6c..7c66aaebcf03 100644 --- a/java/java-impl/src/com/intellij/refactoring/util/duplicates/ConstantMatchProvider.java +++ b/java/java-impl/src/com/intellij/refactoring/util/duplicates/ConstantMatchProvider.java @@ -58,7 +58,7 @@ class ConstantMatchProvider implements MatchProvider { } @Override - public boolean hasDuplicates() { + public Boolean hasDuplicates() { return !myMatches.isEmpty(); } diff --git a/java/java-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesImpl.java b/java/java-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesImpl.java index 5e5b620bcd31..38c798b5549d 100644 --- a/java/java-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesImpl.java +++ b/java/java-impl/src/com/intellij/refactoring/util/duplicates/DuplicatesImpl.java @@ -59,13 +59,17 @@ public class DuplicatesImpl { private DuplicatesImpl() {} public static void invoke(@NotNull final Project project, @NotNull Editor editor, @NotNull MatchProvider provider) { + invoke(project, editor, provider, true); + } + + public static void invoke(@NotNull final Project project, @NotNull Editor editor, @NotNull MatchProvider provider, boolean skipPromptWhenOne) { final List duplicates = provider.getDuplicates(); int idx = 0; final Ref showAll = new Ref(); final String confirmDuplicatePrompt = getConfirmationPrompt(provider, duplicates); for (final Match match : duplicates) { if (!match.getMatchStart().isValid() || !match.getMatchEnd().isValid()) continue; - if (replaceMatch(project, provider, match, editor, ++idx, duplicates.size(), showAll, confirmDuplicatePrompt, true)) return; + if (replaceMatch(project, provider, match, editor, ++idx, duplicates.size(), showAll, confirmDuplicatePrompt, skipPromptWhenOne)) return; } } @@ -186,18 +190,18 @@ public class DuplicatesImpl { } public static void processDuplicates(@NotNull MatchProvider provider, @NotNull Project project, @NotNull Editor editor) { - boolean hasDuplicates = provider.hasDuplicates(); - if (hasDuplicates) { + Boolean hasDuplicates = provider.hasDuplicates(); + if (hasDuplicates == null || hasDuplicates.booleanValue()) { List duplicates = provider.getDuplicates(); if (duplicates.size() == 1) { previewMatch(project, duplicates.get(0), editor); } - final int answer = ApplicationManager.getApplication().isUnitTestMode() ? Messages.YES : Messages.showYesNoDialog(project, + final int answer = ApplicationManager.getApplication().isUnitTestMode() || hasDuplicates == null ? Messages.YES : Messages.showYesNoDialog(project, RefactoringBundle.message("0.has.detected.1.code.fragments.in.this.file.that.can.be.replaced.with.a.call.to.extracted.method", ApplicationNamesInfo.getInstance().getProductName(), duplicates.size()), "Process Duplicates", Messages.getQuestionIcon()); if (answer == Messages.YES) { - invoke(project, editor, provider); + invoke(project, editor, provider, hasDuplicates != null); } } } diff --git a/java/java-impl/src/com/intellij/refactoring/util/duplicates/MatchProvider.java b/java/java-impl/src/com/intellij/refactoring/util/duplicates/MatchProvider.java index abe57ef465ab..8c067a215ef3 100644 --- a/java/java-impl/src/com/intellij/refactoring/util/duplicates/MatchProvider.java +++ b/java/java-impl/src/com/intellij/refactoring/util/duplicates/MatchProvider.java @@ -30,7 +30,10 @@ public interface MatchProvider { List getDuplicates(); - boolean hasDuplicates(); + /** + * @return null if no confirmation prompt is expected + */ + @Nullable Boolean hasDuplicates(); @Nullable String getConfirmDuplicatePrompt(Match match); diff --git a/java/java-impl/src/com/intellij/refactoring/util/duplicates/MethodDuplicatesHandler.java b/java/java-impl/src/com/intellij/refactoring/util/duplicates/MethodDuplicatesHandler.java index d908d35ec3cd..78c4ed2ab216 100644 --- a/java/java-impl/src/com/intellij/refactoring/util/duplicates/MethodDuplicatesHandler.java +++ b/java/java-impl/src/com/intellij/refactoring/util/duplicates/MethodDuplicatesHandler.java @@ -257,6 +257,16 @@ public class MethodDuplicatesHandler implements RefactoringActionHandler { } public static List hasDuplicates(final PsiFile file, final PsiMember member) { + final DuplicatesFinder duplicatesFinder = createDuplicatesFinder(member); + if (duplicatesFinder == null) { + return Collections.emptyList(); + } + + return duplicatesFinder.findDuplicates(file); + } + + @Nullable + public static DuplicatesFinder createDuplicatesFinder(PsiMember member) { PsiElement[] pattern; ReturnValue matchedReturnValue = null; if (member instanceof PsiMethod) { @@ -288,17 +298,14 @@ public class MethodDuplicatesHandler implements RefactoringActionHandler { pattern = new PsiElement[]{((PsiField)member).getInitializer()}; } if (pattern.length == 0) { - return Collections.emptyList(); + return null; } final List inputVariables = member instanceof PsiMethod ? Arrays.asList(((PsiMethod)member).getParameterList().getParameters()) : new ArrayList(); - final DuplicatesFinder duplicatesFinder = - new DuplicatesFinder(pattern, - new InputVariables(inputVariables, member.getProject(), new LocalSearchScope(pattern), false), - matchedReturnValue, - new ArrayList()); - - return duplicatesFinder.findDuplicates(file); + return new DuplicatesFinder(pattern, + new InputVariables(inputVariables, member.getProject(), new LocalSearchScope(pattern), false), + matchedReturnValue, + new ArrayList()); } static String getStatusMessage(final int duplicatesNo) { diff --git a/java/java-impl/src/com/intellij/refactoring/util/duplicates/MethodDuplicatesMatchProvider.java b/java/java-impl/src/com/intellij/refactoring/util/duplicates/MethodDuplicatesMatchProvider.java index 7686126427b6..f7b06702bce0 100644 --- a/java/java-impl/src/com/intellij/refactoring/util/duplicates/MethodDuplicatesMatchProvider.java +++ b/java/java-impl/src/com/intellij/refactoring/util/duplicates/MethodDuplicatesMatchProvider.java @@ -149,7 +149,7 @@ class MethodDuplicatesMatchProvider implements MatchProvider { } @Override - public boolean hasDuplicates() { + public Boolean hasDuplicates() { return myDuplicates.isEmpty(); } diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureLeaveSameExpressionsUntouched.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureLeaveSameExpressionsUntouched.java new file mode 100644 index 000000000000..ad1e6d5f632c --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureLeaveSameExpressionsUntouched.java @@ -0,0 +1,13 @@ +public class Test { + { + int x = 0; + + System.out.println("foo"); + System.out.println("bazz"); + System.out.println(x); + + System.out.println("bar"); + System.out.println("bazz"); + System.out.println(x); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureLeaveSameExpressionsUntouched_after.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureLeaveSameExpressionsUntouched_after.java new file mode 100644 index 000000000000..db07aa54c3a8 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureLeaveSameExpressionsUntouched_after.java @@ -0,0 +1,15 @@ +public class Test { + { + int x = 0; + + newMethod(x, "foo"); + + newMethod(x, "bar"); + } + + private void newMethod(int x, String foo) { + System.out.println(foo); + System.out.println("bazz"); + System.out.println(x); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParam.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParam.java new file mode 100644 index 000000000000..47681845c571 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParam.java @@ -0,0 +1,11 @@ +public class Test { + { + int x = 0; + + System.out.println("foo"); + System.out.println(x); + + System.out.println("bar"); + System.out.println(x); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParamMultipleTimesInside.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParamMultipleTimesInside.java new file mode 100644 index 000000000000..a345233a9c37 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParamMultipleTimesInside.java @@ -0,0 +1,13 @@ +public class Test { + { + int x = 0; + + System.out.println("foo"); + System.out.println("foo"); + System.out.println(x); + + System.out.println("bar"); + System.out.println("bar"); + System.out.println(x); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParamMultipleTimesInside_after.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParamMultipleTimesInside_after.java new file mode 100644 index 000000000000..c6d3c785933b --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParamMultipleTimesInside_after.java @@ -0,0 +1,15 @@ +public class Test { + { + int x = 0; + + newMethod(x, "foo"); + + newMethod(x, "bar"); + } + + private void newMethod(int x, String foo) { + System.out.println(foo); + System.out.println(foo); + System.out.println(x); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParam_after.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParam_after.java new file mode 100644 index 000000000000..ef16acc2489b --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureOneParam_after.java @@ -0,0 +1,14 @@ +public class Test { + { + int x = 0; + + newMethod(x, "foo"); + + newMethod(x, "bar"); + } + + private void newMethod(int x, String foo) { + System.out.println(foo); + System.out.println(x); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureSameParamNames.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureSameParamNames.java new file mode 100644 index 000000000000..ebb9e5910770 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureSameParamNames.java @@ -0,0 +1,13 @@ +public class Test { + { + int x = 0; + + System.out.println(1); + System.out.println(2); + System.out.println(x); + + System.out.println(3); + System.out.println(4); + System.out.println(x); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureSameParamNames_after.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureSameParamNames_after.java new file mode 100644 index 000000000000..47d1ed9ed8c6 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureSameParamNames_after.java @@ -0,0 +1,15 @@ +public class Test { + { + int x = 0; + + newMethod(x, 1, 2); + + newMethod(x, 3, 4); + } + + private void newMethod(int x, int x2, int x3) { + System.out.println(x2); + System.out.println(x3); + System.out.println(x); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureWithChangedParameterName.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureWithChangedParameterName.java new file mode 100644 index 000000000000..2445f2264db8 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureWithChangedParameterName.java @@ -0,0 +1,13 @@ +public class Test { + { + int x = 0; + + + System.out.println(x); + System.out.println(x + 1); + + + System.out.println(x); + System.out.println(x + 2); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureWithChangedParameterName_after.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureWithChangedParameterName_after.java new file mode 100644 index 000000000000..b3e5880154f3 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureWithChangedParameterName_after.java @@ -0,0 +1,16 @@ +public class Test { + { + int x = 0; + + + newMethod(x, x + 1); + + + newMethod(x, x + 2); + } + + private void newMethod(int p, int x) { + System.out.println(p); + System.out.println(x); + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java b/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java index 13b7300eb00a..09e9863d6fc2 100644 --- a/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/refactoring/ExtractMethodTest.java @@ -19,6 +19,7 @@ import com.intellij.JavaTestUtil; import com.intellij.codeInsight.CodeInsightUtil; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.Pair; import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleSettings; import com.intellij.psi.codeStyle.CodeStyleSettingsManager; @@ -33,6 +34,7 @@ import com.intellij.util.IncorrectOperationException; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; +import java.util.ArrayList; import java.util.List; public class ExtractMethodTest extends LightCodeInsightTestCase { @@ -609,6 +611,29 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { doDuplicatesTest(); } + public void testSuggestChangeSignatureOneParam() throws Exception { + doDuplicatesTest(); + } + + public void testSuggestChangeSignatureOneParamMultipleTimesInside() throws Exception { + doDuplicatesTest(); + } + + public void testSuggestChangeSignatureLeaveSameExpressionsUntouched() throws Exception { + doDuplicatesTest(); + } + + public void testSuggestChangeSignatureSameParamNames() throws Exception { + doDuplicatesTest(); + } + + public void testSuggestChangeSignatureWithChangedParameterName() throws Exception { + configureByFile(BASE_PATH + getTestName(false) + ".java"); + boolean success = performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, null, false, "p"); + assertTrue(success); + checkResultByFile(BASE_PATH + getTestName(false) + "_after.java"); + } + public void testTargetAnonymous() throws Exception { doTest(); } @@ -682,7 +707,7 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { settings.ELSE_ON_NEW_LINE = true; settings.CATCH_ON_NEW_LINE = myCatchOnNewLine; configureByFile(BASE_PATH + getTestName(false) + ".java"); - boolean success = performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, type, false); + boolean success = performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, type, false, null); assertTrue(success); checkResultByFile(BASE_PATH + getTestName(false) + "_after.java"); } @@ -692,7 +717,7 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { settings.ELSE_ON_NEW_LINE = true; settings.CATCH_ON_NEW_LINE = myCatchOnNewLine; configureByFile(BASE_PATH + getTestName(false) + ".java"); - boolean success = performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, null, true); + boolean success = performExtractMethod(true, true, getEditor(), getFile(), getProject(), false, null, true, null); assertTrue(success); checkResultByFile(BASE_PATH + getTestName(false) + "_after.java"); } @@ -752,7 +777,7 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { final boolean extractChainedConstructor, int... disabledParams) throws PrepareFailedException, IncorrectOperationException { - return performExtractMethod(doRefactor, replaceAllDuplicates, editor, file, project, extractChainedConstructor, null, false, disabledParams); + return performExtractMethod(doRefactor, replaceAllDuplicates, editor, file, project, extractChainedConstructor, null, false, null, disabledParams); } public static boolean performExtractMethod(boolean doRefactor, @@ -763,6 +788,7 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { final boolean extractChainedConstructor, PsiType returnType, boolean makeStatic, + String newNameOfFirstParam, int... disabledParams) throws PrepareFailedException, IncorrectOperationException { int startOffset = editor.getSelectionModel().getSelectionStart(); @@ -801,15 +827,21 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { processor.doNotPassParameter(param); } } + if (newNameOfFirstParam != null) { + processor.changeParamName(0, newNameOfFirstParam); + } ExtractMethodHandler.run(project, editor, processor); } if (replaceAllDuplicates) { - final List duplicates = processor.getDuplicates(); - for (final Match match : duplicates) { - if (!match.getMatchStart().isValid() || !match.getMatchEnd().isValid()) continue; - PsiDocumentManager.getInstance(project).commitAllDocuments(); - processor.processMatch(match); + final Boolean hasDuplicates = processor.hasDuplicates(); + if (hasDuplicates == null || hasDuplicates.booleanValue()) { + final List duplicates = processor.getDuplicates(); + for (final Match match : duplicates) { + if (!match.getMatchStart().isValid() || !match.getMatchEnd().isValid()) continue; + PsiDocumentManager.getInstance(project).commitAllDocuments(); + processor.processMatch(match); + } } }