From 7e689ac90cc7bf35fc5804c73ac68981ea61d49e Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Wed, 14 Jun 2017 13:10:10 +0300 Subject: [PATCH] Java: Detect which output variables are used after the method call when extracting a method object (IDEA-152688) --- .../extractMethod/ControlFlowWrapper.java | 4 ++ .../ExtractMethodObjectProcessor.java | 2 +- .../psi/controlFlow/ControlFlowUtil.java | 13 ++++ .../BatchUpdateCausedByFormatter.java.after | 12 ++-- .../OutputVariablesUsedInLoop1.java | 29 +++++++++ .../OutputVariablesUsedInLoop1.java.after | 61 +++++++++++++++++++ .../OutputVariablesUsedInLoop2.java | 30 +++++++++ .../OutputVariablesUsedInLoop2.java.after | 57 +++++++++++++++++ .../multipleExitPoints/StaticInner.java.after | 2 +- .../UniqueObjectName.java.after | 2 +- ...ethodObjectWithMultipleExitPointsTest.java | 8 +++ 11 files changed, 211 insertions(+), 9 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop1.java create mode 100644 java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop1.java.after create mode 100644 java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop2.java create mode 100644 java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop2.java.after diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethod/ControlFlowWrapper.java b/java/java-impl/src/com/intellij/refactoring/extractMethod/ControlFlowWrapper.java index cee7c9a44806..8b5007c88d06 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ControlFlowWrapper.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ControlFlowWrapper.java @@ -147,6 +147,10 @@ public class ControlFlowWrapper { return myExitStatements; } + public List filterUsedVariables(PsiVariable[] outputVariables) { + return ControlFlowUtil.filterUsedVariables(myControlFlow, myFlowEnd, outputVariables); + } + public static class ExitStatementsNotSameException extends Exception {} 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 c886545938c9..cfa9cd86009a 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethodObject/ExtractMethodObjectProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethodObject/ExtractMethodObjectProcessor.java @@ -874,7 +874,7 @@ public class ExtractMethodObjectProcessor extends BaseRefactoringProcessor { setMethodCall((PsiMethodCallExpression)((PsiLocalVariable)replace.getDeclaredElements()[0]).getInitializer()); } - final List usedVariables = myControlFlowWrapper.getUsedVariables(); + final List usedVariables = myControlFlowWrapper.filterUsedVariables(myOutputVariables); Collection reassigned = myControlFlowWrapper.getInitializedTwice(); for (PsiVariable variable : usedVariables) { String name = variable.getName(); diff --git a/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java b/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java index b5de7ea1432d..f801e66ba5e6 100644 --- a/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java +++ b/java/java-psi-impl/src/com/intellij/psi/controlFlow/ControlFlowUtil.java @@ -361,6 +361,19 @@ public class ControlFlowUtil { return outputVariables; } + public static List filterUsedVariables(ControlFlow flow, int offset, PsiVariable[] variables) { + if (offset >= flow.getSize()) { + return Collections.emptyList(); + } + List result = new ArrayList<>(); + for (PsiVariable variable : variables) { + if (needVariableValueAt(variable, flow, offset)) { + result.add(variable); + } + } + return result; + } + public static Collection findExitPointsAndStatements(final ControlFlow flow, final int start, final int end, final IntArrayList exitPoints, final Class... classesFilter) { if (end == start) { diff --git a/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/BatchUpdateCausedByFormatter.java.after b/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/BatchUpdateCausedByFormatter.java.after index 5b04adfb008c..94a9ac937372 100644 --- a/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/BatchUpdateCausedByFormatter.java.after +++ b/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/BatchUpdateCausedByFormatter.java.after @@ -13,15 +13,15 @@ public class ABug { Inner inner = new Inner(bottles).invoke(); if (inner.is()) return null; + List errors = inner.getErrors(); + int money = inner.getMoney(); + int nCount = inner.getnCount(); + int rCount = inner.getrCount(); + int wCount = inner.getwCount(); + char[] tripel = inner.getTripel(); boolean no33pr = inner.isNo33pr(); int first = inner.getFirst(); int last = inner.getLast(); - char[] tripel = inner.getTripel(); - int rCount = inner.getrCount(); - int wCount = inner.getwCount(); - int nCount = inner.getnCount(); - int money = inner.getMoney(); - List errors = inner.getErrors(); boolean unhappy = no33pr && first || no33pr && last; diff --git a/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop1.java b/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop1.java new file mode 100644 index 000000000000..45a53082f3a2 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop1.java @@ -0,0 +1,29 @@ +class Test { + + public static class Node { + public int x; + public boolean condition; + public Node next; + } + + public static int test(Node cur) { + Node prev = null; + int total = 0; + while (cur != null) { + if (cur.condition) { + if (prev != null) { + total += prev.x; + } + prev = cur; + cur = cur.next; + } else { + if (prev != null) { + total += prev.x; + } + prev = cur; + cur = cur.next; + } + } + return total; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop1.java.after b/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop1.java.after new file mode 100644 index 000000000000..c64d794a2a14 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop1.java.after @@ -0,0 +1,61 @@ +class Test { + + public static class Node { + public int x; + public boolean condition; + public Node next; + } + + public static int test(Node cur) { + Node prev = null; + int total = 0; + while (cur != null) { + if (cur.condition) { + if (prev != null) { + total += prev.x; + } + prev = cur; + cur = cur.next; + } else { + Inner inner = new Inner(cur, prev, total).invoke(); + cur = inner.getCur(); + prev = inner.getPrev(); + total = inner.getTotal(); + } + } + return total; + } + + private static class Inner { + private Node cur; + private Node prev; + private int total; + + public Inner(Node cur, Node prev, int total) { + this.cur = cur; + this.prev = prev; + this.total = total; + } + + public Node getCur() { + return cur; + } + + public Node getPrev() { + return prev; + } + + public int getTotal() { + return total; + } + + public Inner invoke() { + if (prev != null) { + total += prev.x; + } + prev = cur; + cur = cur.next; + return this; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop2.java b/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop2.java new file mode 100644 index 000000000000..75a7f3b23b69 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop2.java @@ -0,0 +1,30 @@ +class Test { + + public static class Node { + public int x; + public boolean condition; + public Node next; + } + + public static int test(Node cur) { + Node prev = null; + int total = 0; + while (cur != null) { + if (cur.condition) { + if (prev != null) { + total += prev.x; + } + prev = cur; + cur = cur.next; + } else { + if (prev != null) { + total += prev.x; + } + prev = cur; + cur = cur.next; + return cur != null ? total + cur.x : total; + } + } + return total; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop2.java.after b/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop2.java.after new file mode 100644 index 000000000000..c00403a2a235 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/OutputVariablesUsedInLoop2.java.after @@ -0,0 +1,57 @@ +class Test { + + public static class Node { + public int x; + public boolean condition; + public Node next; + } + + public static int test(Node cur) { + Node prev = null; + int total = 0; + while (cur != null) { + if (cur.condition) { + if (prev != null) { + total += prev.x; + } + prev = cur; + cur = cur.next; + } else { + Inner inner = new Inner(cur, prev, total).invoke(); + cur = inner.getCur(); + total = inner.getTotal(); + return cur != null ? total + cur.x : total; + } + } + return total; + } + + private static class Inner { + private Node cur; + private Node prev; + private int total; + + public Inner(Node cur, Node prev, int total) { + this.cur = cur; + this.prev = prev; + this.total = total; + } + + public Node getCur() { + return cur; + } + + public int getTotal() { + return total; + } + + public Inner invoke() { + if (prev != null) { + total += prev.x; + } + prev = cur; + cur = cur.next; + return this; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/StaticInner.java.after b/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/StaticInner.java.after index 01358df3f1a8..bc9135e4e98f 100644 --- a/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/StaticInner.java.after +++ b/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/StaticInner.java.after @@ -3,8 +3,8 @@ class Test { int i = 0; Inner inner = new Inner(i).invoke(); - int k = inner.getK(); int j = inner.getJ(); + int k = inner.getK(); int m = k + j; } diff --git a/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/UniqueObjectName.java.after b/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/UniqueObjectName.java.after index 9e2abc690d7d..03f4bb263eb5 100644 --- a/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/UniqueObjectName.java.after +++ b/java/java-tests/testData/refactoring/extractMethodObject/multipleExitPoints/UniqueObjectName.java.after @@ -4,8 +4,8 @@ class A { int y = 46; int inner = 47; Inner inner1 = new Inner(y).invoke(); - y = inner1.getY(); x = inner1.getX(); + y = inner1.getY(); x = y + x + 45; boolean z = true; if (z) { diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodObjectWithMultipleExitPointsTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodObjectWithMultipleExitPointsTest.java index bbbb21950d2c..b90718c0433f 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodObjectWithMultipleExitPointsTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodObjectWithMultipleExitPointsTest.java @@ -156,6 +156,14 @@ public class ExtractMethodObjectWithMultipleExitPointsTest extends LightRefactor doTestWithIdeaCodeStyleSettings(); } + public void testOutputVariablesUsedInLoop1() throws Exception { + doTest(); + } + + public void testOutputVariablesUsedInLoop2() throws Exception { + doTest(); + } + private void doTestWithIdeaCodeStyleSettings() throws Exception { final CodeStyleSettings settings = CodeStyleSettingsManager.getSettings(getProject()); String oldPrefix = settings.FIELD_NAME_PREFIX;