Skip to content
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

Fix RenameVariable for variables referenced through type casts #4082

Merged
merged 3 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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;
}
}
"""
)
);
}
}
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;
renegrob marked this conversation as resolved.
Show resolved Hide resolved
}
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