From 2e8c2a6308cce9dccb4a1e387215f2b6fa554699 Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Thu, 5 Oct 2017 21:27:29 +0300 Subject: [PATCH] Java: Support parametrized duplicates matching for expressions (IDEA-179924, IDEA-180107) --- .../extractMethod/ParametrizedDuplicates.java | 79 ++++++++++++++++++- .../SuggestChangeSignaturePlusOneFolding.java | 15 ++++ ...stChangeSignaturePlusOneFolding_after.java | 20 +++++ ...SuggestChangeSignatureVoidCallFolding.java | 15 ++++ ...tChangeSignatureVoidCallFolding_after.java | 20 +++++ .../java/refactoring/ExtractMethodTest.java | 8 ++ 6 files changed, 153 insertions(+), 4 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignaturePlusOneFolding.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignaturePlusOneFolding_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureVoidCallFolding.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureVoidCallFolding_after.java diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethod/ParametrizedDuplicates.java b/java/java-impl/src/com/intellij/refactoring/extractMethod/ParametrizedDuplicates.java index 9f43bb3ab7dc..09c1fa04ec08 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ParametrizedDuplicates.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ParametrizedDuplicates.java @@ -18,6 +18,7 @@ package com.intellij.refactoring.extractMethod; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.project.Project; import com.intellij.openapi.util.Pair; +import com.intellij.openapi.util.UserDataHolderBase; import com.intellij.psi.*; import com.intellij.psi.codeStyle.JavaCodeStyleManager; import com.intellij.psi.codeStyle.SuggestedNameInfo; @@ -56,16 +57,26 @@ public class ParametrizedDuplicates { private VariableData[] myVariableData; private ParametrizedDuplicates(PsiElement[] pattern) { + LOG.assertTrue(pattern.length != 0, "pattern length"); if (pattern[0] instanceof PsiStatement) { - Project project = pattern[0].getProject(); - PsiElement[] copy = IntroduceParameterHandler.getElementsInCopy(project, pattern[0].getContainingFile(), pattern); + PsiElement[] copy = copyElements(pattern); myElements = wrapWithCodeBlock(copy); } + else if (pattern[0] instanceof PsiExpression) { + PsiElement[] copy = copyElements(pattern); + PsiExpression wrapped = wrapExpressionWithCodeBlock(copy); + myElements = wrapped != null ? new PsiElement[]{wrapped} : PsiElement.EMPTY_ARRAY; + } else { myElements = PsiElement.EMPTY_ARRAY; } } + private static PsiElement[] copyElements(@NotNull PsiElement[] pattern) { + Project project = pattern[0].getProject(); + return IntroduceParameterHandler.getElementsInCopy(project, pattern[0].getContainingFile(), pattern); + } + @Nullable public static ParametrizedDuplicates findDuplicates(@NotNull ExtractMethodProcessor originalProcessor) { PsiElement[] pattern = originalProcessor.myElements; @@ -128,6 +139,9 @@ public class ParametrizedDuplicates { } private boolean initMatches(@NotNull List matches) { + if (myElements.length == 0) { + return false; + } myOccurrencesList = new ArrayList<>(); Map occurrencesMap = new THashMap<>(); Set badMatches = new THashSet<>(); @@ -144,6 +158,7 @@ public class ParametrizedDuplicates { } } } + myOccurrencesList.sort(Comparator.comparing(occurrences -> occurrences.myFirstOffset)); if (!badMatches.isEmpty()) { matches = new ArrayList<>(matches); @@ -267,6 +282,55 @@ public class ParametrizedDuplicates { return Arrays.copyOfRange(elementsInCopy, 1, elementsInCopy.length - 1); } + @Nullable + private static PsiExpression wrapExpressionWithCodeBlock(@NotNull PsiElement[] copy) { + if (copy.length != 1 || !(copy[0] instanceof PsiExpression)) return null; + + PsiExpression expression = (PsiExpression)copy[0]; + PsiType type = expression.getType(); + if (type == null || PsiType.NULL.equals(type)) return null; + + PsiElement parent = expression.getParent(); + PsiElementFactory factory = JavaPsiFacade.getElementFactory(expression.getProject()); + PsiClass parentClass = PsiTreeUtil.getParentOfType(expression, PsiClass.class); + if (parentClass == null) return null; + + PsiElement parentClassStart = parentClass.getLBrace(); + if (parentClassStart == null) return null; + + // It's syntactically correct to write "new Object() {void foo(){}}.foo()" - see JLS 15.9.5 + String wrapperBodyText = (PsiType.VOID.equals(type) ? "" : "return ") + expression.getText() + ";"; + String wrapperClassImmediateCallText = "new " + CommonClassNames.JAVA_LANG_OBJECT + "() { " + + type.getCanonicalText() + " wrapperMethod() {" + wrapperBodyText + "} " + + "}.wrapperMethod()"; + PsiExpression wrapperClassImmediateCall = factory.createExpressionFromText(wrapperClassImmediateCallText, parent); + wrapperClassImmediateCall = (PsiExpression)expression.replace(wrapperClassImmediateCall); + PsiMethod method = PsiTreeUtil.findChildOfType(wrapperClassImmediateCall, PsiMethod.class); + LOG.assertTrue(method != null, "wrapper class method is null"); + + PsiCodeBlock body = method.getBody(); + LOG.assertTrue(body != null, "wrapper class method's body is null"); + + PsiStatement[] statements = body.getStatements(); + LOG.assertTrue(statements.length == 1, "wrapper class method's body statement count"); + PsiStatement bodyStatement = statements[0]; + + PsiExpression wrapped = null; + if (PsiType.VOID.equals(type) && bodyStatement instanceof PsiExpressionStatement) { + wrapped = ((PsiExpressionStatement)bodyStatement).getExpression(); + } + else if (bodyStatement instanceof PsiReturnStatement) { + wrapped = ((PsiReturnStatement)bodyStatement).getReturnValue(); + } + else { + LOG.error("Unexpected statement in expression code block " + bodyStatement); + } + if (expression instanceof UserDataHolderBase && wrapped instanceof UserDataHolderBase) { + ((UserDataHolderBase)expression).copyUserDataTo((UserDataHolderBase)wrapped); + } + return wrapped; + } + @NotNull private Map createParameterDeclarations(@NotNull ExtractMethodProcessor originalProcessor, @NotNull Map expressionsMapping) { @@ -275,7 +339,12 @@ public class ParametrizedDuplicates { Map parameterDeclarations = new THashMap<>(); UniqueNameGenerator generator = originalProcessor.getParameterNameGenerator(myElements[0]); PsiElementFactory factory = JavaPsiFacade.getElementFactory(project); - PsiElement parent = myElements[0].getParent(); + PsiStatement statement = + myElements[0] instanceof PsiStatement ? (PsiStatement)myElements[0] : PsiTreeUtil.getParentOfType(myElements[0], PsiStatement.class); + LOG.assertTrue(statement != null, "first statement is null"); + PsiElement parent = statement.getParent(); + LOG.assertTrue(parent instanceof PsiCodeBlock, "first statement's parent isn't a code block"); + for (Occurrences occurrences : myOccurrencesList) { ExtractedParameter parameter = occurrences.myParameters.get(0); PsiExpression patternUsage = parameter.myPattern.getUsage(); @@ -287,7 +356,7 @@ public class ParametrizedDuplicates { String declarationText = parameter.myType.getCanonicalText() + " " + parameterName + " = " + usageText + ";"; PsiDeclarationStatement paramDeclaration = (PsiDeclarationStatement)factory.createStatementFromText(declarationText, parent); - paramDeclaration = (PsiDeclarationStatement)parent.addBefore(paramDeclaration, myElements[0]); + paramDeclaration = (PsiDeclarationStatement)parent.addBefore(paramDeclaration, statement); PsiLocalVariable localVariable = (PsiLocalVariable)paramDeclaration.getDeclaredElements()[0]; parameterDeclarations.put(localVariable, occurrences); @@ -375,10 +444,12 @@ public class ParametrizedDuplicates { private static class Occurrences { @NotNull private final Set myPatterns; @NotNull private final List myParameters; + private final int myFirstOffset; public Occurrences(ExtractedParameter parameter) { myPatterns = parameter.myUsages.keySet(); myParameters = new ArrayList<>(); + myFirstOffset = myPatterns.stream().mapToInt(PsiElement::getTextOffset).min().orElse(0); } public void add(ExtractedParameter parameter) { diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignaturePlusOneFolding.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignaturePlusOneFolding.java new file mode 100644 index 000000000000..33d842ba2cd1 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignaturePlusOneFolding.java @@ -0,0 +1,15 @@ +import java.util.ArrayList; +import java.util.List; + +public class C { + List list = new ArrayList<>(); + + void foo(int index) { + list.get(index); + + list.get(index + 1); + } + void bar() { + list.get(1-2); + } +} diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignaturePlusOneFolding_after.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignaturePlusOneFolding_after.java new file mode 100644 index 000000000000..848aa27c6438 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignaturePlusOneFolding_after.java @@ -0,0 +1,20 @@ +import java.util.ArrayList; +import java.util.List; + +public class C { + List list = new ArrayList<>(); + + void foo(int index) { + newMethod(index); + + newMethod(index + 1); + } + + private String newMethod(int i) { + return list.get(i); + } + + void bar() { + newMethod(1-2); + } +} diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureVoidCallFolding.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureVoidCallFolding.java new file mode 100644 index 000000000000..4bc80ee7a5ac --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureVoidCallFolding.java @@ -0,0 +1,15 @@ +import java.util.ArrayList; +import java.util.List; + +public class C { + List list = new ArrayList<>(); + + void foo(int index) { + System.out.println(list.get(index)); + + System.out.println(list.get(index + 1)); + } + void bar() { + System.out.println(list.get(1-2)); + } +} diff --git a/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureVoidCallFolding_after.java b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureVoidCallFolding_after.java new file mode 100644 index 000000000000..dfb8115b9cba --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/SuggestChangeSignatureVoidCallFolding_after.java @@ -0,0 +1,20 @@ +import java.util.ArrayList; +import java.util.List; + +public class C { + List list = new ArrayList<>(); + + void foo(int index) { + newMethod(index); + + newMethod(index + 1); + } + + private void newMethod(int i) { + System.out.println(list.get(i)); + } + + void bar() { + newMethod(1-2); + } +} diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java index c41c1b483693..9ff2953bac29 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java @@ -803,6 +803,14 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { doDuplicatesTest(); } + public void testSuggestChangeSignaturePlusOneFolding() throws Exception { + doDuplicatesTest(); + } + + public void testSuggestChangeSignatureVoidCallFolding() throws Exception { + doDuplicatesTest(); + } + public void testSuggestChangeSignatureWithOutputVariables() throws Exception { doDuplicatesTest(); }