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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Rename variables if the name matches class name #5454

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8551095
Rename variables if the name matches class name
radoslaw-panuszewski May 19, 2025
8e132e2
Prevent variable name conflicts; support unicode characters
radoslaw-panuszewski May 20, 2025
0820d5c
Fix tests in rewrite-kotlin
radoslaw-panuszewski May 20, 2025
f2f61a6
Add test verifying that random variables with matching name are not r…
radoslaw-panuszewski May 20, 2025
5bad96a
Use 𐐅/𐐭 instead of Ą/ą in test since Ą is in BMP
radoslaw-panuszewski May 21, 2025
9c4ea8f
Revert unicode characters handling in decapitalize method as it seems…
radoslaw-panuszewski May 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 46 additions & 2 deletions rewrite-java-test/src/test/java/org/openrewrite/java/ChangeTypeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1592,8 +1592,8 @@ public A1 method(A1 a1) {
import a.A2;

public class Example {
public A2 method(A2 a1) {
return a1;
public A2 method(A2 a2) {
return a2;
Comment on lines +1595 to +1596
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!

}
}
"""
Expand Down Expand Up @@ -1630,6 +1630,50 @@ public class Test {
);
}


@Test
void doNotRenameRandomVariablesMatchingClassName() {
rewriteRun(
spec -> spec.recipe(new ChangeType("a.A1", "a.A2", false)),
java(
"""
package a;
public class A1 {
}
""",
"""
package a;
public class A2 {
}
"""
),
java(
"""
package org.foo;

import a.A1;

public class Example {
public String method(A1 a, String a1) {
return a1;
}
}
""",
"""
package org.foo;

import a.A2;

public class Example {
public String method(A2 a, String a1) {
return a1;
}
}
"""
)
);
}

@Test
void updateVariableType() {
rewriteRun(
Expand Down
34 changes: 34 additions & 0 deletions rewrite-java/src/main/java/org/openrewrite/java/ChangeType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.jspecify.annotations.Nullable;
import org.openrewrite.*;
import org.openrewrite.internal.ListUtils;
import org.openrewrite.java.VariableNameUtils.GenerationStrategy;
import org.openrewrite.java.search.UsesType;
import org.openrewrite.java.tree.*;
import org.openrewrite.marker.Markers;
Expand Down Expand Up @@ -368,6 +369,23 @@ public J visitIdentifier(J.Identifier ident, ExecutionContext ctx) {
}
}

// Rename variable if it matches class name (starting with a lowercase character)
if (ident.getSimpleName().equals(decapitalize(className))) {
if (targetType instanceof JavaType.FullyQualified) {
String newName = VariableNameUtils.generateVariableName(
decapitalize(((JavaType.FullyQualified) targetType).getClassName()),
getCursor(),
GenerationStrategy.INCREMENT_NUMBER
);

ident = ident.withSimpleName(newName);

if (ident.getFieldType() != null) {
ident = ident.withFieldType(ident.getFieldType().withName(newName));
}
}
}

// Recreate any static imports as needed
if (sf != null) {
for (J.Import anImport : sf.getImports()) {
Expand All @@ -387,6 +405,15 @@ public J visitIdentifier(J.Identifier ident, ExecutionContext ctx) {
return visitAndCast(ident, ctx, super::visitIdentifier);
}

@Override
public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext executionContext) {
J.VariableDeclarations.NamedVariable v = (J.VariableDeclarations.NamedVariable) super.visitVariable(variable, executionContext);
if (v.getVariableType() != null && !v.getSimpleName().equals(v.getVariableType().getName())) {
return v.withVariableType(v.getVariableType().withName(v.getSimpleName()));
}
Comment on lines +411 to +413
Copy link
Member

Choose a reason for hiding this comment

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

Would we be replacing any variable here? Or only when it matched the old variable name? Want to make sure there's no unexpected changes, as the recipe is used so often, and the tests are very minimal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This piece of code only updates variable type name if the underlying Identifier has been renamed earlier.

I've added a test case doNotRenameRandomVariablesMatchingClassName to verify that random variables with matching name are not renamed.

Copy link
Member

Choose a reason for hiding this comment

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

Great thanks! I'm rushing out the door here, but will ask the wider team to review while I'm out over the next few days.

return v;
}

@Override
public J visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
if (method.getMethodType() != null && method.getMethodType().hasFlags(Flag.Static)) {
Expand Down Expand Up @@ -778,4 +805,11 @@ private static boolean hasSameFQN(J.Import import_, JavaType targetType) {

return fqn != null && fqn.equals(curFqn);
}

private static String decapitalize(@Nullable String string) {
if (string != null && !string.isEmpty()) {
return Character.toLowerCase(string.charAt(0)) + string.substring(1);
}
return "";
}
}
6 changes: 3 additions & 3 deletions rewrite-kotlin/src/test/java/org/openrewrite/kotlin/ChangeTypeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ fun test(original: Original<String>) { }

import x.y.Target

fun test(original: Target<String>) { }
fun test(target: Target<String>) { }
"""
)
);
Expand Down Expand Up @@ -231,7 +231,7 @@ fun test(original: MyAlias<String>) { }

import x.y.Target as MyAlias

fun test(original: MyAlias<String>) { }
fun test(target: MyAlias<String>) { }
"""
)
);
Expand Down Expand Up @@ -263,7 +263,7 @@ fun test(original: a.b.Original<String>) { }

import x.y.Target

fun test(original: Target<String>) { }
fun test(target: Target<String>) { }
"""
)
);
Expand Down