From b5b3ceda3dfd3c143d4460a9ca94af398015a9c5 Mon Sep 17 00:00:00 2001 From: Alexandr Suhinin Date: Mon, 20 Mar 2023 11:54:15 +0200 Subject: [PATCH] [extract method] IDEA-315737: enable extract method on virtual expressions GitOrigin-RevId: a6ca3dea5d3ab7ec8237c21a06e366e3e5c68a89 --- .../newImpl/ExtractMethodAnalyzer.kt | 2 +- .../inplace/DuplicatesMethodExtractor.kt | 6 ++++- .../refactoring/IntroduceVariableUtil.java | 22 ++++++++++--------- .../ExtractVirtualExpressionFromPolyadic.java | 5 +++++ ...ctVirtualExpressionFromPolyadic_after.java | 12 ++++++++++ ...ExtractVirtualExpressionFromSubstring.java | 6 +++++ ...tVirtualExpressionFromSubstring_after.java | 13 +++++++++++ .../ExtractMethodAndDuplicatesInplaceTest.kt | 8 +++++++ 8 files changed, 62 insertions(+), 12 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromPolyadic.java create mode 100644 java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromPolyadic_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromSubstring.java create mode 100644 java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromSubstring_after.java diff --git a/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/ExtractMethodAnalyzer.kt b/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/ExtractMethodAnalyzer.kt index a8d24b41c4ae..e6a44ec748d5 100644 --- a/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/ExtractMethodAnalyzer.kt +++ b/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/ExtractMethodAnalyzer.kt @@ -58,7 +58,7 @@ fun findExtractOptions(elements: List): ExtractOptions { throw ExtractException(JavaRefactoringBundle.message("extract.method.error.many.exits"), flowOutput.statements + listOfNotNull(outputVariable)) } - val targetClass = PsiTreeUtil.getParentOfType(ExtractMethodHelper.getValidParentOf(elements.first()), PsiClass::class.java) + val targetClass = PsiTreeUtil.getContextOfType(elements.first(), PsiClass::class.java) ?: throw ExtractException(JavaRefactoringBundle.message("extract.method.error.class.not.found"), elements.first().containingFile) var extractOptions = ExtractOptions(targetClass, elements, flowOutput, dataOutput) diff --git a/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/inplace/DuplicatesMethodExtractor.kt b/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/inplace/DuplicatesMethodExtractor.kt index ea933d6009da..f02f23e3f90c 100644 --- a/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/inplace/DuplicatesMethodExtractor.kt +++ b/java/java-impl-refactorings/src/com/intellij/refactoring/extractMethod/newImpl/inplace/DuplicatesMethodExtractor.kt @@ -24,6 +24,7 @@ import com.intellij.refactoring.extractMethod.newImpl.ExtractMethodPipeline.addM import com.intellij.refactoring.extractMethod.newImpl.JavaDuplicatesFinder.Companion.textRangeOf import com.intellij.refactoring.extractMethod.newImpl.structures.ExtractOptions import com.intellij.refactoring.extractMethod.newImpl.structures.InputParameter +import com.intellij.refactoring.introduceField.ElementToWorkOn import com.intellij.refactoring.util.duplicates.DuplicatesImpl import com.intellij.ui.ReplacePromptDialog import com.siyeh.ig.psiutils.SideEffectChecker.mayHaveSideEffects @@ -40,7 +41,10 @@ class DuplicatesMethodExtractor(val extractOptions: ExtractOptions, val targetCl JavaDuplicatesFinder.linkCopiedClassMembersWithOrigin(file) val copiedFile = file.copy() as PsiFile val copiedClass = PsiTreeUtil.findSameElementInCopy(targetClass, copiedFile) - val copiedElements = elements.map { PsiTreeUtil.findSameElementInCopy(it, copiedFile) } + val expression = elements.singleOrNull() as? PsiExpression + val virtualExpressionRange = expression?.getUserData(ElementToWorkOn.TEXT_RANGE)?.textRange + val range = virtualExpressionRange ?:TextRange(elements.first().textRange.startOffset, elements.last().textRange.endOffset) + val copiedElements = ExtractSelector().suggestElementsToExtract(copiedFile, range) val extractOptions = findExtractOptions(copiedClass, copiedElements, methodName, makeStatic) return DuplicatesMethodExtractor(extractOptions, targetClass, elements) } diff --git a/java/java-impl/src/com/intellij/refactoring/IntroduceVariableUtil.java b/java/java-impl/src/com/intellij/refactoring/IntroduceVariableUtil.java index 2776c7dbafd9..3516f453be33 100644 --- a/java/java-impl/src/com/intellij/refactoring/IntroduceVariableUtil.java +++ b/java/java-impl/src/com/intellij/refactoring/IntroduceVariableUtil.java @@ -188,8 +188,8 @@ public final class IntroduceVariableUtil { tempExpr.putUserData(ElementToWorkOn.PREFIX, prefix); tempExpr.putUserData(ElementToWorkOn.SUFFIX, suffix); - final RangeMarker rangeMarker = - FileDocumentManager.getInstance().getDocument(file.getVirtualFile()).createRangeMarker(startOffset, endOffset); + Document document = PsiDocumentManager.getInstance(project).getDocument(file); + RangeMarker rangeMarker = document != null ? document.createRangeMarker(startOffset, endOffset) : null; tempExpr.putUserData(ElementToWorkOn.TEXT_RANGE, rangeMarker); if (parent != null) { @@ -216,7 +216,7 @@ public final class IntroduceVariableUtil { final String fakeInitializer = "intellijidearulezzz"; final int[] refIdx = new int[1]; - final PsiElement toBeExpression = createReplacement(fakeInitializer, project, prefix, suffix, parent, rangeMarker, refIdx); + final PsiElement toBeExpression = createReplacement(fakeInitializer, project, prefix, suffix, parent, TextRange.create(startOffset, endOffset), refIdx); if (ErrorUtil.containsDeepError(toBeExpression)) return null; if (literalExpression != null && toBeExpression instanceof PsiExpression) { PsiType type = ((PsiExpression)toBeExpression).getType(); @@ -339,22 +339,22 @@ public final class IntroduceVariableUtil { return null; } - public static PsiElement createReplacement(final @NonNls String refText, final Project project, + private static PsiElement createReplacement(final @NonNls String refText, final Project project, final String prefix, final String suffix, - final PsiElement parent, final RangeMarker rangeMarker, int[] refIdx) { + final PsiElement parent, final TextRange textRange, int[] refIdx) { String text = refText; if (parent != null) { final String allText = parent.getContainingFile().getText(); final TextRange parentRange = parent.getTextRange(); - LOG.assertTrue(parentRange.getStartOffset() <= rangeMarker.getStartOffset(), parent + "; prefix:" + prefix + "; suffix:" + suffix); - String beg = allText.substring(parentRange.getStartOffset(), rangeMarker.getStartOffset()); + LOG.assertTrue(parentRange.getStartOffset() <= textRange.getStartOffset(), parent + "; prefix:" + prefix + "; suffix:" + suffix); + String beg = allText.substring(parentRange.getStartOffset(), textRange.getStartOffset()); //noinspection SSBasedInspection (suggested replacement breaks behavior) if (StringUtil.stripQuotesAroundValue(beg).trim().isEmpty() && prefix == null) beg = ""; - LOG.assertTrue(rangeMarker.getEndOffset() <= parentRange.getEndOffset(), parent + "; prefix:" + prefix + "; suffix:" + suffix); - String end = allText.substring(rangeMarker.getEndOffset(), parentRange.getEndOffset()); + LOG.assertTrue(textRange.getEndOffset() <= parentRange.getEndOffset(), parent + "; prefix:" + prefix + "; suffix:" + suffix); + String end = allText.substring(textRange.getEndOffset(), parentRange.getEndOffset()); //noinspection SSBasedInspection (suggested replacement breaks behavior) if (StringUtil.stripQuotesAroundValue(end).trim().isEmpty() && suffix == null) end = ""; @@ -459,7 +459,9 @@ public final class IntroduceVariableUtil { final RangeMarker rangeMarker = expr1.getUserData(ElementToWorkOn.TEXT_RANGE); LOG.assertTrue(parent != null, expr1); - return parent.replace(createReplacement(ref.getText(), project, prefix, suffix, parent, rangeMarker, new int[1])); + LOG.assertTrue(rangeMarker != null, expr1); + final TextRange textRange = rangeMarker.getTextRange(); + return parent.replace(createReplacement(ref.getText(), project, prefix, suffix, parent, textRange, new int[1])); } } } diff --git a/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromPolyadic.java b/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromPolyadic.java new file mode 100644 index 000000000000..9c4b4199e70c --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromPolyadic.java @@ -0,0 +1,5 @@ +class Test { + void test(){ + System.out.println("one" + "two" + "three"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromPolyadic_after.java b/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromPolyadic_after.java new file mode 100644 index 000000000000..ef6cd7994452 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromPolyadic_after.java @@ -0,0 +1,12 @@ +import org.jetbrains.annotations.NotNull; + +class Test { + void test(){ + System.out.println(getString() + "three"); + } + + @NotNull + private static String getString() { + return "one" + "two"; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromSubstring.java b/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromSubstring.java new file mode 100644 index 000000000000..9f168a9137e8 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromSubstring.java @@ -0,0 +1,6 @@ +class Test { + void test(){ + System.out.println("one" + "two" + "three"); + System.out.println("one two three"); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromSubstring_after.java b/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromSubstring_after.java new file mode 100644 index 000000000000..7788445a19c4 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodAndDuplicatesInplace/ExtractVirtualExpressionFromSubstring_after.java @@ -0,0 +1,13 @@ +import org.jetbrains.annotations.NotNull; + +class Test { + void test(){ + System.out.println("one" + getTwo() + "three"); + System.out.println("one " + getTwo() + " three"); + } + + @NotNull + private static String getTwo() { + return "two"; + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodAndDuplicatesInplaceTest.kt b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodAndDuplicatesInplaceTest.kt index 4fdf6d081fd5..c047a3d1fbf9 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodAndDuplicatesInplaceTest.kt +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodAndDuplicatesInplaceTest.kt @@ -468,6 +468,14 @@ class ExtractMethodAndDuplicatesInplaceTest: LightJavaCodeInsightTestCase() { doTest() } + fun testExtractVirtualExpressionFromPolyadic(){ + doTest() + } + + fun testExtractVirtualExpressionFromSubstring(){ + doTest() + } + fun testRefactoringListener(){ templateTest { configureByFile("$BASE_PATH/${getTestName(false)}.java")