openrewrite / rewrite

Automated mass refactoring of source code.
https://docs.openrewrite.org
Apache License 2.0
2.09k stars 312 forks source link

RenamePrivateFieldsToCamelCase modifies field and then does not rename its variable references #2285

Closed pway99 closed 1 year ago

pway99 commented 1 year ago

Issue discovered with zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/callback/ZkAsyncCallbacks.java

Problem

Nested class Identifiers referencing the variable to be renamed not transformed.

Root cause:

RenamePrivateFieldsToCamelCase assumes RenameVariable will transform nested class identifers referencing the variable in a manner similar to the RenameVariableTest#renameInnerClassVariables.

Notes:

  • nested class identifiers referencing a named variable of thier parent class are not in the same name scope as the variable being renamed
  • The RenameVariableTest recipe is an anonymous inner class visiting ClassDeclarations then invoking RenameVariable for each matching variable within the class
  • RenameVariableTest#renameInnerClassVariables demonstrates the expected RenamePrivateFieldsToCamelCase behavior even though RenameLocalVariables will only rename identifiers whithin the same name scope as the target variable.

Expected transformation:

before:

class A {
    private static String MY_STRING = "VAR";
    void doSomething() {
        MY_STRING.toLowerCase();
        AB.INNER_STRING.toLowerCase();
    }

    private static class AB {
        private static String INNER_STRING = "var";
        void doSomething() {
            MY_STRING.toLowerCase();
        }
    }
}

after:

class A {
    private static String myString = "VAR";
    void doSomething() {
        myString.toLowerCase();
        AB.INNER_STRING.toLowerCase();
    }

    private static class AB {
        private static String INNER_STRING = "var";
        void doSomething() {
            myString.toLowerCase();
        }
    }
}

Example diff

      import org.slf4j.LoggerFactory;

public class ZkAsyncCallbacks {
-  private static Logger LOG = LoggerFactory.getLogger(ZkAsyncCallbacks.class);
+  private static Logger log = LoggerFactory.getLogger(ZkAsyncCallbacks.class);

  public static class GetDataCallbackHandler extends DefaultCallback implements DataCallback {
    public byte[] _data;

Recipes in example diff:

pway99 commented 1 year ago

fixed by: https://github.com/openrewrite/rewrite/pull/2292