diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ExtractableExpressionPart.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ExtractableExpressionPart.java index 4024972bfbd8..2b83d1ab28a2 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ExtractableExpressionPart.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/duplicates/ExtractableExpressionPart.java @@ -122,7 +122,7 @@ public class ExtractableExpressionPart { @Nullable static ExtractableExpressionPart matchVariable(@NotNull PsiReferenceExpression expression, @Nullable List scope) { PsiElement resolved = expression.resolve(); - if (resolved instanceof PsiField && isUnqualifiedModification(expression)) { + if (resolved instanceof PsiField && isModification(expression)) { return null; } if (resolved instanceof PsiVariable && (scope == null || !DuplicatesFinder.isUnder(resolved, scope))) { @@ -132,23 +132,20 @@ public class ExtractableExpressionPart { return null; } - private static boolean isUnqualifiedModification(@NotNull PsiReferenceExpression expression) { - PsiExpression qualifier = expression.getQualifierExpression(); - if (qualifier == null || qualifier instanceof PsiQualifiedExpression) { // 'this.' and 'super.' with fields are just like unqualified - PsiElement parent = PsiUtil.skipParenthesizedExprUp(expression.getParent()); - if (parent instanceof PsiAssignmentExpression) { - PsiAssignmentExpression assignment = (PsiAssignmentExpression)parent; - if (PsiTreeUtil.isAncestor(assignment.getLExpression(), expression, false)) { - return true; - } + private static boolean isModification(@NotNull PsiReferenceExpression expression) { + PsiElement parent = PsiUtil.skipParenthesizedExprUp(expression.getParent()); + if (parent instanceof PsiAssignmentExpression) { + PsiAssignmentExpression assignment = (PsiAssignmentExpression)parent; + if (PsiTreeUtil.isAncestor(assignment.getLExpression(), expression, false)) { + return true; } - else if (parent instanceof PsiUnaryExpression) { - PsiUnaryExpression unary = (PsiUnaryExpression)parent; - IElementType tokenType = unary.getOperationTokenType(); - if ((tokenType.equals(JavaTokenType.PLUSPLUS) || tokenType.equals(JavaTokenType.MINUSMINUS)) && - PsiTreeUtil.isAncestor(unary.getOperand(), expression, false)) { - return true; - } + } + else if (parent instanceof PsiUnaryExpression) { + PsiUnaryExpression unary = (PsiUnaryExpression)parent; + IElementType tokenType = unary.getOperationTokenType(); + if ((tokenType.equals(JavaTokenType.PLUSPLUS) || tokenType.equals(JavaTokenType.MINUSMINUS)) && + PsiTreeUtil.isAncestor(unary.getOperand(), expression, false)) { + return true; } } return false; diff --git a/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentChainedFieldsDuplicate.java b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentChainedFieldsDuplicate.java new file mode 100644 index 000000000000..17003b7082a2 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentChainedFieldsDuplicate.java @@ -0,0 +1,22 @@ +class DecrementDifferentChainedFieldsDuplicate { + static class C { + int x; + int y; + } + public static final C c; + + private void foo() { + + if (c.x > 0) { + bar(c.x); + c.x--; + } + + if (c.y > 0) { + bar(c.y); + c.y--; + } + } + + private void bar(int i) { } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentChainedFieldsDuplicate_after.java b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentChainedFieldsDuplicate_after.java new file mode 100644 index 000000000000..2d68632257ff --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentChainedFieldsDuplicate_after.java @@ -0,0 +1,26 @@ +class DecrementDifferentChainedFieldsDuplicate { + static class C { + int x; + int y; + } + public static final C c; + + private void foo() { + + newMethod(); + + if (c.y > 0) { + bar(c.y); + c.y--; + } + } + + private void newMethod() { + if (c.x > 0) { + bar(c.x); + c.x--; + } + } + + private void bar(int i) { } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentInnerFieldsDuplicate.java b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentInnerFieldsDuplicate.java new file mode 100644 index 000000000000..0ee6e3ef76bb --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentInnerFieldsDuplicate.java @@ -0,0 +1,21 @@ +class DecrementDifferentInnerFieldsDuplicate { + class C { + int x; + int y; + } + + private void foo(C a, C b) { + + if (a.x > 0) { + bar(a.x); + a.x--; + } + + if (b.y > 0) { + bar(b.y); + b.y--; + } + } + + private void bar(int i) { } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentInnerFieldsDuplicate_after.java b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentInnerFieldsDuplicate_after.java new file mode 100644 index 000000000000..476c262a699a --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentInnerFieldsDuplicate_after.java @@ -0,0 +1,25 @@ +class DecrementDifferentInnerFieldsDuplicate { + class C { + int x; + int y; + } + + private void foo(C a, C b) { + + newMethod(a); + + if (b.y > 0) { + bar(b.y); + b.y--; + } + } + + private void newMethod(C a) { + if (a.x > 0) { + bar(a.x); + a.x--; + } + } + + private void bar(int i) { } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentOuterFieldsDuplicate.java b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentOuterFieldsDuplicate.java new file mode 100644 index 000000000000..94c564b59d5a --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentOuterFieldsDuplicate.java @@ -0,0 +1,21 @@ +class DecrementDifferentOuterFieldsDuplicate { + int x; + int y; + + class C { + private void foo() { + + if (x > 0) { + bar(x); + DecrementDifferentOuterFieldsDuplicate.this.x--; + } + + if (y > 0) { + bar(y); + DecrementDifferentOuterFieldsDuplicate.this.y--; + } + } + } + + private void bar(int i) { } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentOuterFieldsDuplicate_after.java b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentOuterFieldsDuplicate_after.java new file mode 100644 index 000000000000..cca7398cf88c --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentOuterFieldsDuplicate_after.java @@ -0,0 +1,25 @@ +class DecrementDifferentOuterFieldsDuplicate { + int x; + int y; + + class C { + private void foo() { + + newMethod(); + + if (y > 0) { + bar(y); + DecrementDifferentOuterFieldsDuplicate.this.y--; + } + } + + private void newMethod() { + if (x > 0) { + bar(x); + DecrementDifferentOuterFieldsDuplicate.this.x--; + } + } + } + + private void bar(int i) { } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentStaticFieldsDuplicate.java b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentStaticFieldsDuplicate.java new file mode 100644 index 000000000000..376a16dc9dab --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentStaticFieldsDuplicate.java @@ -0,0 +1,21 @@ +class DecrementDifferentStaticFieldsDuplicate { + static class C { + static int x; + static int y; + } + + private void foo() { + + if (C.x > 0) { + bar(C.x); + C.x--; + } + + if (C.y > 0) { + bar(C.y); + C.y--; + } + } + + private void bar(int i) { } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentStaticFieldsDuplicate_after.java b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentStaticFieldsDuplicate_after.java new file mode 100644 index 000000000000..5423d3bc95bb --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/DecrementDifferentStaticFieldsDuplicate_after.java @@ -0,0 +1,25 @@ +class DecrementDifferentStaticFieldsDuplicate { + static class C { + static int x; + static int y; + } + + private void foo() { + + newMethod(); + + if (C.y > 0) { + bar(C.y); + C.y--; + } + } + + private void newMethod() { + if (C.x > 0) { + bar(C.x); + C.x--; + } + } + + private void bar(int i) { } +} \ No newline at end of file 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 8344470a94f4..578b1880aa5e 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java @@ -1185,6 +1185,22 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { doDuplicatesTest(); } + public void testDecrementDifferentStaticFieldsDuplicate() throws Exception { + doDuplicatesTest(); + } + + public void testDecrementDifferentOuterFieldsDuplicate() throws Exception { + doDuplicatesTest(); + } + + public void testDecrementDifferentInnerFieldsDuplicate() throws Exception { + doDuplicatesTest(); + } + + public void testDecrementDifferentChainedFieldsDuplicate() throws Exception { + doDuplicatesTest(); + } + public void testArgumentFoldingWholeStatement() throws Exception { doDuplicatesTest(); }