Skip to content

Commit

Permalink
Fix RenameVariable for variables referenced through type casts (openr…
Browse files Browse the repository at this point in the history 
…ewrite#4082)

* Fix RenameVariable for variables referenced through type casts

* Add unit test

* Be explicit about null handling and types

---------

Co-authored-by: Tim te Beek <[email protected]>
  • Loading branch information
@renegrob @timtebeek
2 people authored and U808771 committed Mar 14, 2024
1 parent 23e461f commit c30f2e8
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 35 deletions.
100 changes: 70 additions & 30 deletions rewrite-java-test/src/test/java/org/openrewrite/java/RenameVariableTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,33 +77,33 @@ void renameFieldWithSameNameAsParameterWithJavaDoc() {
spec -> spec.recipe(renameVariableTest("name", "_name", false)),
java(
"""
public class A {
private String name;
/**
* The length of <code>name</code> added to the length of {@link #name}.
*
* @param name My parameter.
*/
int fooA(String name) {
return name.length() + this.name.length();
}
}
""",
"""
public class A {
private String _name;
/**
* The length of <code>name</code> added to the length of {@link #_name}.
*
* @param name My parameter.
*/
int fooA(String name) {
return name.length() + this._name.length();
}
}
public class A {
private String name;
/**
* The length of <code>name</code> added to the length of {@link #name}.
*
* @param name My parameter.
*/
int fooA(String name) {
return name.length() + this.name.length();
}
}
""",
"""
public class A {
private String _name;
/**
* The length of <code>name</code> added to the length of {@link #_name}.
*
* @param name My parameter.
*/
int fooA(String name) {
return name.length() + this._name.length();
}
}
"""
)
);
}
Expand Down Expand Up @@ -180,7 +180,7 @@ void isParameterizedClass() {
java(
"""
package org.openrewrite;
public class A<T> {
private String _val;
private String name;
Expand All @@ -193,7 +193,7 @@ public class A<T> {
""",
"""
package org.openrewrite;
public class A<T> {
private String v;
private String name;
Expand Down Expand Up @@ -820,7 +820,7 @@ public J visitVariableDeclarations(J.VariableDeclarations multiVariable, Executi
"""
public class B {
int n;
{
n++; // do not change.
int n;
Expand All @@ -838,7 +838,7 @@ public int foo(int n) {
"""
public class B {
int n;
{
n++; // do not change.
int n1;
Expand Down Expand Up @@ -955,4 +955,44 @@ class A {
)
);
}

@Test
@Issue("https://github.com/openrewrite/rewrite/issues/4059")
void renameTypeCastedField() {
rewriteRun(
spec -> spec.recipe(renameVariableTest("objArray", "objects", false)),
java(
"""
import java.util.Arrays;
public class A {
private Object[] objArray;
public boolean isEqualTo(Object object) {
return Arrays.equals(objArray, ((A) object).objArray);
}
public int length() {
return objArray.length;
}
}
""",
"""
import java.util.Arrays;
public class A {
private Object[] objects;
public boolean isEqualTo(Object object) {
return Arrays.equals(objects, ((A) object).objects);
}
public int length() {
return objects.length;
}
}
"""
)
);
}
}
45 changes: 40 additions & 5 deletions rewrite-java/src/main/java/org/openrewrite/java/RenameVariable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,18 +152,53 @@ private boolean isVariableName(Object value, J.Identifier ident) {
* FieldAccess targets the variable if its target is an Identifier and either
* its target FieldType equals variable.Name.FieldType
* or its target Type equals variable.Name.FieldType.Owner
* or if FieldAccess targets a TypCast and either
* its type equals variable.Name.FieldType
* or its type equals variable.Name.FieldType.Owner.
* In case the FieldAccess targets another FieldAccess, the target is followed
* until it is either an Identifier or a TypeCast.
*/
private boolean fieldAccessTargetsVariable(J.FieldAccess fieldAccess) {
if (renameVariable.getName().getFieldType() != null && fieldAccess.getTarget() instanceof J.Identifier) {
J.Identifier fieldAccessTarget = (J.Identifier) fieldAccess.getTarget();
if (renameVariable.getName().getFieldType() != null) {
Expression target = getTarget(fieldAccess);
JavaType targetType = resolveType(target.getType());
JavaType.Variable variableNameFieldType = renameVariable.getName().getFieldType();
JavaType fieldAccessTargetType = fieldAccessTarget.getType() instanceof JavaType.Parameterized ? ((JavaType.Parameterized) fieldAccessTarget.getType()).getType() : fieldAccessTarget.getType();
return variableNameFieldType.equals(fieldAccessTarget.getFieldType()) ||
(fieldAccessTargetType != null && fieldAccessTargetType.equals(variableNameFieldType.getOwner()));
if (TypeUtils.isOfType(variableNameFieldType.getOwner(), targetType)) {
return true;
}
if (target instanceof J.TypeCast) {
return TypeUtils.isOfType(variableNameFieldType, targetType);
} else if (target instanceof J.Identifier) {
return TypeUtils.isOfType(variableNameFieldType, ((J.Identifier) target).getFieldType());
}
}
return false;
}

@Nullable
private Expression getTarget(J.FieldAccess fieldAccess) {
Expression target = fieldAccess.getTarget();
if (target instanceof J.Identifier) {
return target;
}
if (target instanceof J.FieldAccess) {
return getTarget((J.FieldAccess) target);
}
if (target instanceof J.Parentheses<?>) {
J tree = ((J.Parentheses<?>) target).getTree();
if (tree instanceof J.TypeCast) {
return (J.TypeCast) tree;
}
return null;
}
return null;
}

@Nullable
private JavaType resolveType(@Nullable JavaType type) {
return type instanceof JavaType.Parameterized ? ((JavaType.Parameterized) type).getType() : type;
}

@Override
public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable namedVariable, P p) {
boolean nameEquals = namedVariable.getSimpleName().equals(renameVariable.getSimpleName());
Expand Down

0 comments on commit c30f2e8

Please sign in to comment.