Improve ChangeFieldName to replace ident references as well (fixes #14)

rewrite-core/src/main/java/com/netflix/rewrite/tree/visitor/refactor/RefactorVisitor.java

@@ -19,6 +19,7 @@
 import com.netflix.rewrite.tree.*;
 import com.netflix.rewrite.tree.visitor.CursorAstVisitor;
 import com.netflix.rewrite.tree.visitor.refactor.op.AddImport;
+import com.netflix.rewrite.tree.visitor.refactor.op.ChangeFieldName;
 import com.netflix.rewrite.tree.visitor.refactor.op.DeleteStatement;
 import com.netflix.rewrite.tree.visitor.refactor.op.RemoveImport;
 
@@ -95,6 +96,10 @@ protected void deleteStatement(Statement statement) {
         andThen.get().add(new DeleteStatement(statement.getId()));
     }
 
+    protected void changeFieldName(Type.Class classType, String hasName, String toName) {
+        andThen.get().add(new ChangeFieldName(classType, hasName, toName));
+    }
+
     protected void andThen(RefactorVisitor visitor) {
         andThen.get().add(visitor);
     }

rewrite-core/src/main/java/com/netflix/rewrite/tree/visitor/refactor/op/ChangeFieldName.java

@@ -15,9 +15,9 @@
  */
 package com.netflix.rewrite.tree.visitor.refactor.op;
 
-import com.netflix.rewrite.tree.Tr;
-import com.netflix.rewrite.tree.Type;
-import com.netflix.rewrite.tree.TypeUtils;
+import com.netflix.rewrite.internal.lang.Nullable;
+import com.netflix.rewrite.tree.*;
+import com.netflix.rewrite.tree.visitor.CursorAstVisitor;
 import com.netflix.rewrite.tree.visitor.refactor.AstTransform;
 import com.netflix.rewrite.tree.visitor.refactor.RefactorVisitor;
 import lombok.RequiredArgsConstructor;
@@ -42,20 +42,75 @@ public List<AstTransform> visitVariable(Tr.VariableDecls.NamedVar variable) {
                 .getParentOrThrow() // Tr.Block
                 .getParentOrThrow() // maybe Tr.ClassDecl
                 .getTree() instanceof Tr.ClassDecl;
-        Type.Class containingClassType = TypeUtils.asClass(getCursor().enclosingClass().getType());
 
-        return maybeTransform(isField && containingClassType == classType && variable.getSimpleName().equals(hasName),
+        return maybeTransform(isField &&
+                        matchesClass(getCursor().enclosingClass().getType()) &&
+                        variable.getSimpleName().equals(hasName),
                 super.visitVariable(variable),
                 transform(variable, v -> v.withName(v.getName().withName(toName)))
         );
     }
 
     @Override
     public List<AstTransform> visitFieldAccess(Tr.FieldAccess fieldAccess) {
-        Type.Class targetType = TypeUtils.asClass(fieldAccess.getTarget().getType());
-        return maybeTransform(targetType == classType && fieldAccess.getSimpleName().equals(hasName),
+        return maybeTransform(matchesClass(fieldAccess.getTarget().getType()) &&
+                        fieldAccess.getSimpleName().equals(hasName),
                 super.visitFieldAccess(fieldAccess),
                 transform(fieldAccess, fa -> fa.withName(fa.getName().withName(toName)))
         );
     }
+
+    @Override
+    public List<AstTransform> visitIdentifier(Tr.Ident ident) {
+        return maybeTransform(ident.getSimpleName().equals(hasName) && isFieldReference(ident),
+                super.visitIdentifier(ident),
+                transform(ident, i -> i.withName(toName))
+        );
+    }
+
+    private boolean matchesClass(@Nullable Type test) {
+        Type.Class testClassType = TypeUtils.asClass(test);
+        return testClassType != null && testClassType.getFullyQualifiedName().equals(classType.getFullyQualifiedName());
+    }
+
+    private boolean isFieldReference(Tr.Ident ident) {
+        Cursor nearest = new FindVariableDefinition(ident, getCursor()).visit(getCursor().enclosingCompilationUnit());
+        return nearest != null && nearest
+                .getParentOrThrow() // maybe Tr.VariableDecls
+                .getParentOrThrow() // maybe Tr.Block
+                .getParentOrThrow() // maybe Tr.ClassDecl
+                .getTree() instanceof Tr.ClassDecl;
+    }
+
+    @RequiredArgsConstructor
+    private static class FindVariableDefinition extends CursorAstVisitor<Cursor> {
+        private final Tr.Ident ident;
+        private final Cursor referenceScope;
+
+        @Override
+        public Cursor defaultTo(Tree t) {
+            return null;
+        }
+
+        @Override
+        public Cursor visitVariable(Tr.VariableDecls.NamedVar variable) {
+            return variable.getSimpleName().equalsIgnoreCase(ident.getSimpleName()) && getCursor().isInSameNameScope(referenceScope) ?
+                    getCursor() :
+                    super.visitVariable(variable);
+        }
+
+        @Override
+        public Cursor reduce(Cursor r1, Cursor r2) {
+            if(r1 == null) {
+                return r2;
+            }
+
+            if(r2 == null) {
+                return r1;
+            }
+
+            // the definition will be the "closest" cursor, i.e. the one with the longest path in the same name scope
+            return r1.getPathAsStream().count() > r2.getPathAsStream().count() ? r1 : r2;
+        }
+    }
 }

rewrite-core/src/test/kotlin/com/netflix/rewrite/tree/visitor/refactor/op/ChangeFieldNameTest.kt

@@ -47,6 +47,47 @@ open class ChangeFieldNameTest : Parser by OpenJdkParser() {
 
     @Test
     fun changeFieldNameReferences() {
+        val b = parse("""
+            public class B {
+               int n;
+               
+               {
+                   n = 1;
+                   n /= 2;
+                   if(n + 1 == 2) {}
+                   n++;
+               }
+               
+               public int foo(int n) {
+                   return n + this.n;
+               }
+            }
+        """.trimIndent())
+
+        val fixed = b.refactor()
+                .changeFieldName(Type.Class.build("B"), "n", "n1")
+                .fix()
+
+        assertRefactored(fixed, """
+            public class B {
+               int n1;
+               
+               {
+                   n1 = 1;
+                   n1 /= 2;
+                   if(n1 + 1 == 2) {}
+                   n1++;
+               }
+               
+               public int foo(int n) {
+                   return n + this.n1;
+               }
+            }
+        """)
+    }
+
+    @Test
+    fun changeFieldNameReferencesInOtherClass() {
         val b = """
             public class B {
                int n;
@@ -60,7 +101,7 @@ open class ChangeFieldNameTest : Parser by OpenJdkParser() {
                     b.n = 1;
                 }
             }
-        """.trimIndent())
+        """.trimIndent(), b)
 
         val fixed = a.refactor()
                 .changeFieldName(Type.Class.build("B"), "n", "n1")