Skip to content

Rename variables if the name matches class name #5454

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

radoslaw-panuszewski
Copy link
Contributor

@radoslaw-panuszewski radoslaw-panuszewski commented May 19, 2025

What's your motivation?

When I rename a type in IntelliJ IDEA (for example SomeType to AnotherType) it will automatically rename variables with the same name as the class, but starting with lowercase character (so someType to anotherType).

This PR introduces the above feature to org.openrewrite.java.ChangeType recipe.

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@github-project-automation github-project-automation bot moved this to In Progress in OpenRewrite May 19, 2025
@radoslaw-panuszewski radoslaw-panuszewski force-pushed the rename-variables-when-changing-type branch 2 times, most recently from b83c9b8 to 071fb76 Compare May 19, 2025 19:02
@radoslaw-panuszewski radoslaw-panuszewski force-pushed the rename-variables-when-changing-type branch from 071fb76 to 8551095 Compare May 19, 2025 19:07
@radoslaw-panuszewski radoslaw-panuszewski marked this pull request as ready for review May 19, 2025 19:12
Copy link
Member

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat addition, thanks! Couple small requests, but otherwise looks good.

Comment on lines +1595 to +1596
public A2 method(A2 a2) {
return a2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat idea indeed to rename these as well!

@timtebeek timtebeek moved this from In Progress to Ready to Review in OpenRewrite May 20, 2025
@timtebeek timtebeek added the enhancement New feature or request label May 20, 2025
@radoslaw-panuszewski
Copy link
Contributor Author

Hey @timtebeek, could you ping rest of the team to take a look? 😉

@timtebeek
Copy link
Member

Got back over the weekend, thanks! Having a look now.

Did a quick extra check that we only touch variables where the type changed:

    @Test
    void changeMethodVariableName() {
        rewriteRun(
          spec -> spec.recipe(new ChangeType("a.A1", "a.A2", true)),
          java(
            """
              package a;
              public class A1 {
              }
              """
          ),
          java(
            """
              package a;
              public class A2 {
              }
              """
          ),
          java(
            """
              package org.foo;
              
              import a.A1;
              import a.A2;
              
              public class Example {
                  public A1 method1(A1 a1) {
                      return a1;
                  }
                  public A2 method2(A2 a1) {
                      return a1; // Unchanged
                  }
              }
              """,
            """
              package org.foo;
              
              import a.A2;
              
              public class Example {
                  public A2 method1(A2 a2) {
                      return a2;
                  }
                  public A2 method2(A2 a1) {
                      return a1; // Unchanged
                  }
              }
              """
          )
        );
    }

@radoslaw-panuszewski
Copy link
Contributor Author

Hi @timtebeek, did you take a look? I would love to proceed with this PR 😉

Copy link
Member

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks both; took a little longer to figure out what seemed "off" at the earlier implementation. Figured it out just now and applied changes to make better use of RenameVariable and keep all changes inside visitVariable. Previously only the printed name of the variable was changed, but the underlying type informations was not updated from the same point. That might then lead to problems down the line. With the changes made just now I'm more confident to merge this, as it's such a frequently used recipe. Thanks again for the suggestion, tests and initial implementation!

@radoslaw-panuszewski
Copy link
Contributor Author

Hi @timtebeek, thanks for the changes! The only thing I don't understand is the commit Do not expect Kotlin changes just yet. Does it mean this recipe won't rename variables in Kotlin? That would be bad news for my company, which have most of the code written in Kotlin 😞

Or maybe the "just yet" means that more commits will follow? 😛

@timtebeek
Copy link
Member

I've not yet worked out why we're not seeing change for Kotlin, and my IDE was throwing up some unrelated issues in trying to figure it out. It might be that RenameVariable visitor, or something else still. I'm definitely open to fixing that, but perhaps best done on a separate PR such that we have something to work from already. Appreciate any help debugging that addition!

@timtebeek timtebeek merged commit f37ee62 into openrewrite:main May 31, 2025
2 checks passed
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite May 31, 2025
@timtebeek
Copy link
Member

Looks to fail on
image

@timtebeek
Copy link
Member

Fixed in 3b1f9d1; thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants