From b4e18d3d4775a9576d70faf29d4b6dcd86e5df06 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Fri, 20 Mar 2020 18:21:42 +0700 Subject: [PATCH] IDEA-61551 Introduce variable: provide more options for nested scopes GitOrigin-RevId: b0819c27e58ff3f2c6d39f70a5a2ef37f1fe1361 --- .../IntroduceVariableBase.java | 92 ++++++++++++++++++- .../inplaceIntroduceVariable/inBlock1.java | 24 +++++ .../inBlock1_after.java | 25 +++++ .../inplaceIntroduceVariable/inBlock2.java | 24 +++++ .../inBlock2_after.java | 25 +++++ .../inplaceIntroduceVariable/inBlock3.java | 24 +++++ .../inBlock3_after.java | 25 +++++ .../inBlockLambda1.java | 13 +++ .../inBlockLambda1_after.java | 14 +++ .../inBlockLambda2.java | 13 +++ .../inBlockLambda2_after.java | 14 +++ .../InplaceIntroduceVariableTest.java | 20 ++++ .../messages/JavaRefactoringBundle.properties | 3 +- 13 files changed, 310 insertions(+), 6 deletions(-) create mode 100644 java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock1.java create mode 100644 java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock1_after.java create mode 100644 java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock2.java create mode 100644 java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock2_after.java create mode 100644 java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock3.java create mode 100644 java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock3_after.java create mode 100644 java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda1.java create mode 100644 java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda1_after.java create mode 100644 java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda2.java create mode 100644 java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda2_after.java 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 58156c9034aa..5e16ef3c6c85 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceVariable/IntroduceVariableBase.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceVariable/IntroduceVariableBase.java @@ -70,6 +70,7 @@ import one.util.streamex.StreamEx; import org.jetbrains.annotations.*; import java.util.*; +import java.util.stream.Collectors; /** * @author dsl @@ -122,6 +123,24 @@ public abstract class IntroduceVariableBase extends IntroduceHandlerBase { // For debug/test purposes return formatDescription(0); } + + @NotNull + private static IntroduceVariableBase.JavaReplaceChoice allOccurrencesInside(PsiElement parent, + int sameKeywordCount, + String finalKeyword) { + return new JavaReplaceChoice(ReplaceChoice.ALL, null, false) { + @Override + public PsiExpression[] filter(ExpressionOccurrenceManager manager) { + return StreamEx.of(manager.getOccurrences()).filter(expr -> PsiTreeUtil.isAncestor(parent, expr, true)) + .toArray(PsiExpression.EMPTY_ARRAY); + } + + @Override + public String formatDescription(int occurrencesCount) { + return JavaRefactoringBundle.message("replace.occurrences.inside.statement", occurrencesCount, finalKeyword, sameKeywordCount); + } + }; + } } private static final Logger LOG = Logger.getInstance(IntroduceVariableBase.class); @@ -1220,15 +1239,78 @@ public abstract class IntroduceVariableBase extends IntroduceHandlerBase { } if (myOccurrences.size() > 1 && !myCantReplaceAll) { - JavaReplaceChoice choice = occurrencesMap.containsKey(JavaReplaceChoice.NO_WRITE) - ? new JavaReplaceChoice(ReplaceChoice.ALL, JavaRefactoringBundle.message("replace.all.read.and.write"), - false) - : JavaReplaceChoice.ALL; - occurrencesMap.put(choice, myOccurrences); + if (occurrencesMap.containsKey(JavaReplaceChoice.NO_WRITE)) { + JavaReplaceChoice choice = new JavaReplaceChoice( + ReplaceChoice.ALL, JavaRefactoringBundle.message("replace.all.read.and.write"), false); + occurrencesMap.put(choice, myOccurrences); + } + else { + generateScopeBasedChoices(expr, occurrencesMap); + occurrencesMap.put(JavaReplaceChoice.ALL, myOccurrences); + } } } return occurrencesMap; } + + private void generateScopeBasedChoices(PsiExpression expr, + LinkedHashMap> occurrencesMap) { + Comparator treeOrder = (e1, e2) -> { + if (PsiTreeUtil.isAncestor(e1, e2, true)) return 1; + if (PsiTreeUtil.isAncestor(e2, e1, true)) return -1; + return 0; + }; + PsiElement physical = getPhysicalElement(expr); + TreeMap> groupByBlock = + StreamEx.of(myOccurrences).groupingBy(e -> PsiTreeUtil.findCommonParent(e, physical), + () -> new TreeMap<>(treeOrder), Collectors.toList()); + assert !groupByBlock.isEmpty(); + List currentOccurrences = new ArrayList<>(); + Map counts = new HashMap<>(); + groupByBlock.forEach((parent, occurrences) -> { + PsiElement nextParent = groupByBlock.higherKey(parent); + if (nextParent == null) return; + currentOccurrences.addAll(occurrences); + if (currentOccurrences.size() == 1) return; + PsiElement current = parent.getParent(); + String keyword = null; + while (current != nextParent) { + if (current instanceof PsiIfStatement || current instanceof PsiWhileStatement || current instanceof PsiForStatement || + current instanceof PsiTryStatement) { + keyword = current.getFirstChild().getText(); + } + else if (current instanceof PsiDoWhileStatement) { + keyword = "do-while"; + } + else if (current instanceof PsiForeachStatement) { + keyword = "for-each"; + } + else if (current instanceof PsiLambdaExpression) { + keyword = "lambda"; + } + if (keyword != null) { + break; + } + current = current.getParent(); + } + if (keyword == null && nextParent instanceof PsiIfStatement) { + PsiStatement thenBranch = ((PsiIfStatement)nextParent).getThenBranch(); + PsiStatement elseBranch = ((PsiIfStatement)nextParent).getElseBranch(); + if (PsiTreeUtil.isAncestor(thenBranch, parent, false)) { + keyword = "if-then"; + } else if (PsiTreeUtil.isAncestor(elseBranch, parent, false)) { + keyword = "else"; + } + } + if (keyword != null) { + int sameKeywordCount = counts.merge(keyword, 1, Integer::sum); + if (sameKeywordCount <= 2) { + JavaReplaceChoice choice = JavaReplaceChoice.allOccurrencesInside(parent, sameKeywordCount, keyword); + occurrencesMap.put(choice, new ArrayList<>(currentOccurrences)); + } + } + }); + } } protected static String getRefactoringName() { diff --git a/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock1.java b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock1.java new file mode 100644 index 000000000000..954453b473ee --- /dev/null +++ b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock1.java @@ -0,0 +1,24 @@ +class C { + private int bar() { + return 42; + } + public void foo() { + if (true) { + if (false) { + int i = bar() + 1; + int j = bar() - 1; + } else { + int i = bar() + 1; + int j = bar() + 1; + } + } else { + if (true) { + int i = bar() + 1; + int j = bar() + 1; + } else { + int i = bar() + 1; + int j = bar() + 1; + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock1_after.java b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock1_after.java new file mode 100644 index 000000000000..9ad226c7fa69 --- /dev/null +++ b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock1_after.java @@ -0,0 +1,25 @@ +class C { + private int bar() { + return 42; + } + public void foo() { + if (true) { + if (false) { + int i = bar() + 1; + int j = bar() - 1; + } else { + int i1 = bar() + 1; + int i = i1; + int j = i1; + } + } else { + if (true) { + int i = bar() + 1; + int j = bar() + 1; + } else { + int i = bar() + 1; + int j = bar() + 1; + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock2.java b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock2.java new file mode 100644 index 000000000000..954453b473ee --- /dev/null +++ b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock2.java @@ -0,0 +1,24 @@ +class C { + private int bar() { + return 42; + } + public void foo() { + if (true) { + if (false) { + int i = bar() + 1; + int j = bar() - 1; + } else { + int i = bar() + 1; + int j = bar() + 1; + } + } else { + if (true) { + int i = bar() + 1; + int j = bar() + 1; + } else { + int i = bar() + 1; + int j = bar() + 1; + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock2_after.java b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock2_after.java new file mode 100644 index 000000000000..9464073f07da --- /dev/null +++ b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock2_after.java @@ -0,0 +1,25 @@ +class C { + private int bar() { + return 42; + } + public void foo() { + if (true) { + int i1 = bar() + 1; + if (false) { + int i = i1; + int j = bar() - 1; + } else { + int i = i1; + int j = i1; + } + } else { + if (true) { + int i = bar() + 1; + int j = bar() + 1; + } else { + int i = bar() + 1; + int j = bar() + 1; + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock3.java b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock3.java new file mode 100644 index 000000000000..954453b473ee --- /dev/null +++ b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock3.java @@ -0,0 +1,24 @@ +class C { + private int bar() { + return 42; + } + public void foo() { + if (true) { + if (false) { + int i = bar() + 1; + int j = bar() - 1; + } else { + int i = bar() + 1; + int j = bar() + 1; + } + } else { + if (true) { + int i = bar() + 1; + int j = bar() + 1; + } else { + int i = bar() + 1; + int j = bar() + 1; + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock3_after.java b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock3_after.java new file mode 100644 index 000000000000..8b382167728a --- /dev/null +++ b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlock3_after.java @@ -0,0 +1,25 @@ +class C { + private int bar() { + return 42; + } + public void foo() { + int i1 = bar() + 1; + if (true) { + if (false) { + int i = i1; + int j = bar() - 1; + } else { + int i = i1; + int j = i1; + } + } else { + if (true) { + int i = i1; + int j = i1; + } else { + int i = i1; + int j = i1; + } + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda1.java b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda1.java new file mode 100644 index 000000000000..91b07b576e87 --- /dev/null +++ b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda1.java @@ -0,0 +1,13 @@ +import java.util.function.Supplier; + +class C { + Supplier> test(String s) { + System.out.println(s.trim()+s.trim()); + return () -> { + System.out.println(s.trim()+s.trim()); + return () -> { + return s.trim() + s.trim(); + }; + }; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda1_after.java b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda1_after.java new file mode 100644 index 000000000000..e3119b64353a --- /dev/null +++ b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda1_after.java @@ -0,0 +1,14 @@ +import java.util.function.Supplier; + +class C { + Supplier> test(String s) { + System.out.println(s.trim()+s.trim()); + return () -> { + System.out.println(s.trim()+s.trim()); + return () -> { + String trim = s.trim(); + return trim + trim; + }; + }; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda2.java b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda2.java new file mode 100644 index 000000000000..91b07b576e87 --- /dev/null +++ b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda2.java @@ -0,0 +1,13 @@ +import java.util.function.Supplier; + +class C { + Supplier> test(String s) { + System.out.println(s.trim()+s.trim()); + return () -> { + System.out.println(s.trim()+s.trim()); + return () -> { + return s.trim() + s.trim(); + }; + }; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda2_after.java b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda2_after.java new file mode 100644 index 000000000000..7ef3773498bf --- /dev/null +++ b/java/java-tests/testData/refactoring/inplaceIntroduceVariable/inBlockLambda2_after.java @@ -0,0 +1,14 @@ +import java.util.function.Supplier; + +class C { + Supplier> test(String s) { + System.out.println(s.trim()+s.trim()); + return () -> { + String trim = s.trim(); + System.out.println(trim + trim); + return () -> { + return trim + trim; + }; + }; + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/InplaceIntroduceVariableTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/InplaceIntroduceVariableTest.java index 06f2a529308c..09ea3a2e13f8 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/InplaceIntroduceVariableTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/InplaceIntroduceVariableTest.java @@ -175,6 +175,26 @@ public class InplaceIntroduceVariableTest extends AbstractJavaInplaceIntroduceTe invokeEditorAction(IdeActions.ACTION_EDITOR_ENTER); }); } + + public void testInBlock1() { + doTestReplaceChoice("Replace 0 occurrences in 'else' block"); + } + + public void testInBlock2() { + doTestReplaceChoice("Replace 0 occurrences in 'if-then' block"); + } + + public void testInBlock3() { + doTestReplaceChoice("Replace all 0 occurrences"); + } + + public void testInBlockLambda1() { + doTestReplaceChoice("Replace 0 occurrences in 'lambda' block"); + } + + public void testInBlockLambda2() { + doTestReplaceChoice("Replace 0 occurrences in outer 'lambda' block"); + } private void doTestStopEditing(Consumer pass) { String name = getTestName(true); diff --git a/java/openapi/resources/messages/JavaRefactoringBundle.properties b/java/openapi/resources/messages/JavaRefactoringBundle.properties index c4287ca8f238..6584e83dbc47 100644 --- a/java/openapi/resources/messages/JavaRefactoringBundle.properties +++ b/java/openapi/resources/messages/JavaRefactoringBundle.properties @@ -609,4 +609,5 @@ replace.inside.current.lambda=Create variable inside current lambda replace.as.separate.operation=Extract as ''{0}'' operation replace.all.read.and.write=Replace read and write occurrences (will change semantics!) replace.all.and.extract=Replace all {0} occurrences and extract as ''{1}'' operation -replace.lambda.chain.detected=Lambda chain detected \ No newline at end of file +replace.lambda.chain.detected=Lambda chain detected +replace.occurrences.inside.statement=Replace {0} occurrences in{2, choice, 1#|2# outer} ''{1}'' block