Java: Don't introduce parameter for potentially matching fields if the fields are written - improved (IDEA-182284)

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<PsiElement> 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;

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() {
+        <selection>
+        if (c.x > 0) {
+            bar(c.x);
+            c.x--;
+        }</selection>
+
+        if (c.y > 0) {
+            bar(c.y);
+            c.y--;
+        }
+    }
+
+    private void bar(int i) { }
+}

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) { }
+}

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) {
+        <selection>
+        if (a.x > 0) {
+            bar(a.x);
+            a.x--;
+        }</selection>
+
+        if (b.y > 0) {
+            bar(b.y);
+            b.y--;
+        }
+    }
+
+    private void bar(int i) { }
+}

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) { }
+}

java/java-tests/testData/refactoring/extractMethod/DecrementDifferentOuterFieldsDuplicate.java

@@ -0,0 +1,21 @@
+class DecrementDifferentOuterFieldsDuplicate {
+    int x;
+    int y;
+
+    class C {
+        private void foo() {
+            <selection>
+            if (x > 0) {
+                bar(x);
+                DecrementDifferentOuterFieldsDuplicate.this.x--;
+            }</selection>
+
+            if (y > 0) {
+                bar(y);
+                DecrementDifferentOuterFieldsDuplicate.this.y--;
+            }
+        }
+    }
+
+    private void bar(int i) { }
+}

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) { }
+}

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() {
+        <selection>
+        if (C.x > 0) {
+            bar(C.x);
+            C.x--;
+        }</selection>
+
+        if (C.y > 0) {
+            bar(C.y);
+            C.y--;
+        }
+    }
+
+    private void bar(int i) { }
+}

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) { }
+}

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();
   }