From d0de6284510e87fc5b2073bd920a1b98a7c37d28 Mon Sep 17 00:00:00 2001 From: Ekaterina Tuzova Date: Thu, 18 Apr 2013 15:47:29 +0400 Subject: [PATCH] added Replace duplicates in Extract Method --- .../com/jetbrains/python/PyBundle.properties | 1 + .../extractmethod/PyDuplicatesFinder.java | 97 +++++++++++++++++ .../extractmethod/PyExtractMethodUtil.java | 101 +++++++++++++++--- .../extractmethod/DuplicateMultiLine.after.py | 8 ++ .../DuplicateMultiLine.before.py | 5 + .../DuplicateSingleLine.after.py | 10 ++ .../DuplicateSingleLine.before.py | 5 + .../refactoring/PyExtractMethodTest.java | 8 ++ 8 files changed, 221 insertions(+), 14 deletions(-) create mode 100644 python/src/com/jetbrains/python/refactoring/extractmethod/PyDuplicatesFinder.java create mode 100644 python/testData/refactoring/extractmethod/DuplicateMultiLine.after.py create mode 100644 python/testData/refactoring/extractmethod/DuplicateMultiLine.before.py create mode 100644 python/testData/refactoring/extractmethod/DuplicateSingleLine.after.py create mode 100644 python/testData/refactoring/extractmethod/DuplicateSingleLine.before.py diff --git a/python/src/com/jetbrains/python/PyBundle.properties b/python/src/com/jetbrains/python/PyBundle.properties index 5361946381aa..f47f0c3e3725 100644 --- a/python/src/com/jetbrains/python/PyBundle.properties +++ b/python/src/com/jetbrains/python/PyBundle.properties @@ -536,6 +536,7 @@ refactoring.extract.method.error.cannot.perform.refactoring.when.execution.flow. refactoring.extract.method.error.cannot.perform.refactoring.when.from.import.inside=Cannot perform refactoring with from import statement inside code block refactoring.extract.method.error.cannot.perform.refactoring.using.selected.elements=Cannot perform extract method using selected element(s) refactoring.extract.method.error.name.clash=Method name clashes with already existing name +refactoring.extract.method.error.cannot.perform.refactoring.with.local=Cannot perform refactoring from expression with local variables modifications and return instructions inside code fragment # extract superclass refactoring.extract.super.target.path.outside.roots=Target directory is outside the project.
Must be within content roots diff --git a/python/src/com/jetbrains/python/refactoring/extractmethod/PyDuplicatesFinder.java b/python/src/com/jetbrains/python/refactoring/extractmethod/PyDuplicatesFinder.java new file mode 100644 index 000000000000..1d070dd34b90 --- /dev/null +++ b/python/src/com/jetbrains/python/refactoring/extractmethod/PyDuplicatesFinder.java @@ -0,0 +1,97 @@ +package com.jetbrains.python.refactoring.extractmethod; + +import com.intellij.codeInsight.PsiEquivalenceUtil; +import com.intellij.openapi.util.Pair; +import com.intellij.psi.PsiComment; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiWhiteSpace; +import com.intellij.psi.util.PsiTreeUtil; +import com.jetbrains.python.psi.PyFunction; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.ArrayList; +import java.util.List; + +/** + * User : ktisha + */ +public class PyDuplicatesFinder { + private final ArrayList myPattern; + + public PyDuplicatesFinder(@NotNull final PsiElement statement1, @NotNull final PsiElement statement2) { + myPattern = new ArrayList(); + PsiElement sibling = statement1; + + do { + myPattern.add(sibling); + if (sibling == statement2) break; + sibling = PsiTreeUtil.skipSiblingsForward(sibling, PsiWhiteSpace.class, PsiComment.class); + } while (sibling != null); + } + + public List> findDuplicates(@Nullable final PsiElement scope, @NotNull final PyFunction generatedMethod) { + final ArrayList> result = new ArrayList>(); + if (scope != null) { + findPatternOccurrences(result, scope, generatedMethod); + } + return result; + } + + private void findPatternOccurrences(@NotNull final List> array, @NotNull final PsiElement scope, + @NotNull final PyFunction generatedMethod) { + if (scope == generatedMethod) return; + final PsiElement[] children = scope.getChildren(); + for (PsiElement child : children) { + final Pair match = isDuplicateFragment(child); + if (match != null) { + array.add(match); + continue; + } + findPatternOccurrences(array, child, generatedMethod); + } + } + + @Nullable + private Pair isDuplicateFragment(@NotNull final PsiElement candidate) { + for (PsiElement pattern : myPattern) { + if (PsiTreeUtil.isAncestor(pattern, candidate, false)) return null; + } + PsiElement sibling = candidate; + final ArrayList candidates = new ArrayList(); + for (int i = 0; i != myPattern.size(); ++i) { + if (sibling == null) return null; + + candidates.add(sibling); + sibling = PsiTreeUtil.skipSiblingsForward(sibling, PsiWhiteSpace.class, PsiComment.class); + } + if (myPattern.size() != candidates.size()) return null; + if (candidates.size() <= 0) return null; + final Pair match = new Pair(candidates.get(0), candidates.get(candidates.size() - 1)); + for (int i = 0; i < myPattern.size(); i++) { + if (!matchPattern(myPattern.get(i), candidates.get(i))) return null; + } + return match; + } + + private static boolean matchPattern(@Nullable final PsiElement pattern, + @Nullable final PsiElement candidate) { + if (pattern == null || candidate == null) return pattern == candidate; + final PsiElement[] children1 = PsiEquivalenceUtil.getFilteredChildren(pattern, null, true); + final PsiElement[] children2 = PsiEquivalenceUtil.getFilteredChildren(candidate, null, true); + if (children1.length != children2.length) return false; + + for (int i = 0; i < children1.length; i++) { + PsiElement child1 = children1[i]; + PsiElement child2 = children2[i]; + if (!matchPattern(child1, child2)) return false; + } + + if (children1.length == 0) { + if (!pattern.textMatches(candidate)) return false; + } + + return true; + } + +} diff --git a/python/src/com/jetbrains/python/refactoring/extractmethod/PyExtractMethodUtil.java b/python/src/com/jetbrains/python/refactoring/extractmethod/PyExtractMethodUtil.java index b71249e7e40a..a24aeaf6a779 100644 --- a/python/src/com/jetbrains/python/refactoring/extractmethod/PyExtractMethodUtil.java +++ b/python/src/com/jetbrains/python/refactoring/extractmethod/PyExtractMethodUtil.java @@ -1,20 +1,27 @@ package com.jetbrains.python.refactoring.extractmethod; +import com.intellij.codeInsight.CodeInsightUtilBase; import com.intellij.codeInsight.codeFragment.CodeFragment; +import com.intellij.codeInsight.highlighting.HighlightManager; +import com.intellij.find.FindManager; import com.intellij.lang.LanguageNamesValidation; import com.intellij.lang.refactoring.NamesValidator; import com.intellij.openapi.application.ApplicationManager; +import com.intellij.openapi.application.ApplicationNamesInfo; import com.intellij.openapi.command.CommandProcessor; import com.intellij.openapi.editor.Editor; +import com.intellij.openapi.editor.LogicalPosition; +import com.intellij.openapi.editor.ScrollType; +import com.intellij.openapi.editor.colors.EditorColors; +import com.intellij.openapi.editor.colors.EditorColorsManager; +import com.intellij.openapi.editor.markup.TextAttributes; import com.intellij.openapi.project.Project; +import com.intellij.openapi.ui.DialogWrapper; import com.intellij.openapi.ui.Messages; import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.TextRange; import com.intellij.openapi.util.text.StringUtil; -import com.intellij.psi.PsiElement; -import com.intellij.psi.PsiNamedElement; -import com.intellij.psi.PsiRecursiveElementVisitor; -import com.intellij.psi.PsiWhiteSpace; +import com.intellij.psi.*; import com.intellij.psi.codeStyle.CodeStyleManager; import com.intellij.psi.impl.source.codeStyle.CodeEditUtil; import com.intellij.psi.util.PsiTreeUtil; @@ -26,6 +33,7 @@ import com.intellij.refactoring.extractMethod.ExtractMethodValidator; import com.intellij.refactoring.listeners.RefactoringElementListenerComposite; import com.intellij.refactoring.rename.RenameUtil; import com.intellij.refactoring.util.CommonRefactoringUtil; +import com.intellij.ui.ReplacePromptDialog; import com.intellij.usageView.UsageInfo; import com.intellij.util.Function; import com.intellij.util.IncorrectOperationException; @@ -56,19 +64,19 @@ public class PyExtractMethodUtil { private PyExtractMethodUtil() { } - public static void extractFromStatements(final Project project, - final Editor editor, - final PyCodeFragment fragment, - final PsiElement statement1, - final PsiElement statement2) { + public static void extractFromStatements(@NotNull final Project project, + @NotNull final Editor editor, + @NotNull final PyCodeFragment fragment, + @NotNull final PsiElement statement1, + @NotNull final PsiElement statement2) { if (!fragment.getOutputVariables().isEmpty() && fragment.isReturnInstructionInside()) { CommonRefactoringUtil.showErrorHint(project, editor, - "Cannot perform refactoring from expression with local variables modifications and return instructions inside code fragment", + PyBundle.message("refactoring.extract.method.error.cannot.perform.refactoring.with.local"), RefactoringBundle.message("error.title"), "refactoring.extractMethod"); return; } - PyFunction function = PsiTreeUtil.getParentOfType(statement1, PyFunction.class); + final PyFunction function = PsiTreeUtil.getParentOfType(statement1, PyFunction.class); final PyUtil.MethodFlags flags = function == null ? null : PyUtil.MethodFlags.of(function); final boolean isClassMethod = flags != null && flags.isClassMethod(); final boolean isStaticMethod = flags != null && flags.isStaticMethod(); @@ -90,6 +98,8 @@ public class PyExtractMethodUtil { final String methodName = data.first; final AbstractVariableData[] variableData = data.second; + final PyDuplicatesFinder finder = new PyDuplicatesFinder(statement1, statement2); + if (fragment.getOutputVariables().isEmpty()) { CommandProcessor.getInstance().executeCommand(project, new Runnable() { public void run() { @@ -123,6 +133,8 @@ public class PyExtractMethodUtil { // Replace statements with call callElement = replaceElements(elementsRange, callElement); + callElement = CodeInsightUtilBase.forcePsiPostprocessAndRestoreElement(callElement); + processDuplicates(callElement, generatedMethod, finder, editor); // Set editor setSelectionAndCaret(editor, callElement); @@ -174,6 +186,8 @@ public class PyExtractMethodUtil { // replace statements with call callElement = replaceElements(elementsRange, callElement); + callElement = CodeInsightUtilBase.forcePsiPostprocessAndRestoreElement(callElement); + processDuplicates(callElement, generatedMethod, finder, editor); // Set editor setSelectionAndCaret(editor, callElement); @@ -184,7 +198,66 @@ public class PyExtractMethodUtil { } } - private static void processGlobalWrites(@NotNull PyFunction function, @NotNull PyCodeFragment fragment) { + private static void processDuplicates(@NotNull final PsiElement callElement, + @NotNull final PyFunction generatedMethod, + @NotNull final PyDuplicatesFinder finder, + @NotNull final Editor editor) { + final ScopeOwner owner = ScopeUtil.getScopeOwner(callElement); + if (owner instanceof PsiFile) return; + final List> duplicates = finder.findDuplicates(owner, generatedMethod); + + if (duplicates.size() > 0) { + final String message = 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()); + final boolean isUnittest = ApplicationManager.getApplication().isUnitTestMode(); + final int exitCode = !isUnittest ? Messages.showYesNoDialog(callElement.getProject(), message, + RefactoringBundle.message("refactoring.extract.method.dialog.title"), Messages.getInformationIcon()) : + DialogWrapper.OK_EXIT_CODE; + if (exitCode == DialogWrapper.OK_EXIT_CODE) { + boolean replaceAll = false; + for (Pair match : duplicates) { + final List elementsRange = PyPsiUtils.collectElements(match.getFirst(), match.getSecond()); + if (!replaceAll) { + highlightInEditor(callElement.getProject(), match, editor); + + int promptResult = FindManager.PromptResult.ALL; + if (!isUnittest) { + ReplacePromptDialog promptDialog = new ReplacePromptDialog(false, RefactoringBundle.message("replace.fragment"), callElement.getProject()); + promptDialog.show(); + promptResult = promptDialog.getExitCode(); + } + if (promptResult == FindManager.PromptResult.SKIP) continue; + if (promptResult == FindManager.PromptResult.CANCEL) break; + + if (promptResult == FindManager.PromptResult.OK) { + replaceElements(elementsRange, callElement); + } + else if (promptResult == FindManager.PromptResult.ALL) { + replaceElements(elementsRange, callElement); + replaceAll = true; + } + } + else { + replaceElements(elementsRange, callElement); + } + } + } + } + } + + private static void highlightInEditor(@NotNull final Project project, @NotNull final Pair pair, + @NotNull final Editor editor) { + final HighlightManager highlightManager = HighlightManager.getInstance(project); + final EditorColorsManager colorsManager = EditorColorsManager.getInstance(); + final TextAttributes attributes = colorsManager.getGlobalScheme().getAttributes(EditorColors.SEARCH_RESULT_ATTRIBUTES); + final int startOffset = pair.getFirst().getTextRange().getStartOffset(); + final int endOffset = pair.getSecond().getTextRange().getEndOffset(); + highlightManager.addRangeHighlight(editor, startOffset, endOffset, attributes, true, null); + final LogicalPosition logicalPosition = editor.offsetToLogicalPosition(startOffset); + editor.getScrollingModel().scrollTo(logicalPosition, ScrollType.MAKE_VISIBLE); + } + + private static void processGlobalWrites(@NotNull final PyFunction function, @NotNull final PyCodeFragment fragment) { final Set globalWrites = fragment.getGlobalWrites(); final Set newGlobalNames = new LinkedHashSet(); final Scope scope = ControlFlowCache.getScope(function); @@ -239,7 +312,7 @@ public class PyExtractMethodUtil { builder.append("."); } - public static void extractFromExpression(final Project project, + public static void extractFromExpression(@NotNull final Project project, final Editor editor, final PyCodeFragment fragment, final PsiElement expression) { @@ -301,7 +374,7 @@ public class PyExtractMethodUtil { final PyElement generated = generator.createFromText(LanguageLevel.getDefault(), PyElement.class, builder.toString()); PsiElement callElement = null; if (generated instanceof PyReturnStatement) { - callElement = fragment.isReturnInstructionInside() ? generated : ((PyReturnStatement)generated).getExpression(); + callElement = ((PyReturnStatement)generated).getExpression(); } else if (generated instanceof PyExpressionStatement) { callElement = ((PyExpressionStatement)generated).getExpression(); diff --git a/python/testData/refactoring/extractmethod/DuplicateMultiLine.after.py b/python/testData/refactoring/extractmethod/DuplicateMultiLine.after.py new file mode 100644 index 000000000000..09ecace3e138 --- /dev/null +++ b/python/testData/refactoring/extractmethod/DuplicateMultiLine.after.py @@ -0,0 +1,8 @@ +def foo(): + a = 1 + print a + + +def bar(): + foo() + foo() diff --git a/python/testData/refactoring/extractmethod/DuplicateMultiLine.before.py b/python/testData/refactoring/extractmethod/DuplicateMultiLine.before.py new file mode 100644 index 000000000000..80210d251693 --- /dev/null +++ b/python/testData/refactoring/extractmethod/DuplicateMultiLine.before.py @@ -0,0 +1,5 @@ +def bar(): + a = 1 + print a + a = 1 + print a diff --git a/python/testData/refactoring/extractmethod/DuplicateSingleLine.after.py b/python/testData/refactoring/extractmethod/DuplicateSingleLine.after.py new file mode 100644 index 000000000000..5fb885b56fa9 --- /dev/null +++ b/python/testData/refactoring/extractmethod/DuplicateSingleLine.after.py @@ -0,0 +1,10 @@ +def foo(): + a = 1 + return a + + +def bar(): + a = foo() + print a + a = foo() + print a diff --git a/python/testData/refactoring/extractmethod/DuplicateSingleLine.before.py b/python/testData/refactoring/extractmethod/DuplicateSingleLine.before.py new file mode 100644 index 000000000000..a5b087f49efa --- /dev/null +++ b/python/testData/refactoring/extractmethod/DuplicateSingleLine.before.py @@ -0,0 +1,5 @@ +def bar(): + a = 1 + print a + a = 1 + print a diff --git a/python/testSrc/com/jetbrains/python/refactoring/PyExtractMethodTest.java b/python/testSrc/com/jetbrains/python/refactoring/PyExtractMethodTest.java index cc6d3e019c76..0290902bdf3a 100644 --- a/python/testSrc/com/jetbrains/python/refactoring/PyExtractMethodTest.java +++ b/python/testSrc/com/jetbrains/python/refactoring/PyExtractMethodTest.java @@ -250,4 +250,12 @@ public class PyExtractMethodTest extends LightMarkedTestCase { public void testYieldFrom33() { doTest("bar", LanguageLevel.PYTHON33); } + + public void testDuplicateSingleLine() { + doTest("foo"); + } + + public void testDuplicateMultiLine() { + doTest("foo"); + } }