From 721ed46e929944581dd31ee300455d6fadd6dd13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Grob?= Date: Thu, 7 Mar 2024 08:26:05 +0100 Subject: [PATCH 1/3] Fix RenameVariable for variables referenced through type casts --- .../org/openrewrite/java/RenameVariable.java | 44 ++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/rewrite-java/src/main/java/org/openrewrite/java/RenameVariable.java b/rewrite-java/src/main/java/org/openrewrite/java/RenameVariable.java index 09407432bf8..eaa4c801c40 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/RenameVariable.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/RenameVariable.java @@ -152,18 +152,52 @@ 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) { + J target = getTarget(fieldAccess); 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 (target instanceof J.TypeCast) { + J.TypeCast typeCast = (J.TypeCast) target; + JavaType targetType = resolveType(typeCast.getType()); + return variableNameFieldType.equals(targetType) || targetType != null && targetType.equals(variableNameFieldType.getOwner()); + } else if (target instanceof J.Identifier) { + J.Identifier fieldAccessTarget = (J.Identifier) target; + JavaType fieldAccessTargetType = resolveType(fieldAccessTarget.getType()); + return variableNameFieldType.equals(fieldAccessTarget.getFieldType()) || fieldAccessTargetType != null && fieldAccessTargetType.equals(variableNameFieldType.getOwner()); + } } return false; } + @Nullable + private J 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 tree; + } + return null; + } + return null; + } + + private JavaType resolveType(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()); From e9989ec3cbb0914c998a959f9881c77600fac742 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 7 Mar 2024 11:10:29 +0100 Subject: [PATCH 2/3] Add unit test --- .../openrewrite/java/RenameVariableTest.java | 100 ++++++++++++------ 1 file changed, 70 insertions(+), 30 deletions(-) diff --git a/rewrite-java-test/src/test/java/org/openrewrite/java/RenameVariableTest.java b/rewrite-java-test/src/test/java/org/openrewrite/java/RenameVariableTest.java index 63f2e64fe42..75ef8d00cf9 100644 --- a/rewrite-java-test/src/test/java/org/openrewrite/java/RenameVariableTest.java +++ b/rewrite-java-test/src/test/java/org/openrewrite/java/RenameVariableTest.java @@ -77,33 +77,33 @@ void renameFieldWithSameNameAsParameterWithJavaDoc() { spec -> spec.recipe(renameVariableTest("name", "_name", false)), java( """ - public class A { - private String name; - - /** - * The length of name 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 name 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 name 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 name added to the length of {@link #_name}. + * + * @param name My parameter. + */ + int fooA(String name) { + return name.length() + this._name.length(); + } + } + """ ) ); } @@ -180,7 +180,7 @@ void isParameterizedClass() { java( """ package org.openrewrite; - + public class A { private String _val; private String name; @@ -193,7 +193,7 @@ public class A { """, """ package org.openrewrite; - + public class A { private String v; private String name; @@ -820,7 +820,7 @@ public J visitVariableDeclarations(J.VariableDeclarations multiVariable, Executi """ public class B { int n; - + { n++; // do not change. int n; @@ -838,7 +838,7 @@ public int foo(int n) { """ public class B { int n; - + { n++; // do not change. int n1; @@ -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; + } + } + """ + ) + ); + } } From 1e31e787f3ddc2de646fe9f6368113506faafa01 Mon Sep 17 00:00:00 2001 From: Tim te Beek Date: Thu, 7 Mar 2024 19:12:36 +0100 Subject: [PATCH 3/3] Be explicit about null handling and types --- .../org/openrewrite/java/RenameVariable.java | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/rewrite-java/src/main/java/org/openrewrite/java/RenameVariable.java b/rewrite-java/src/main/java/org/openrewrite/java/RenameVariable.java index eaa4c801c40..b7c98f77923 100644 --- a/rewrite-java/src/main/java/org/openrewrite/java/RenameVariable.java +++ b/rewrite-java/src/main/java/org/openrewrite/java/RenameVariable.java @@ -160,23 +160,23 @@ private boolean isVariableName(Object value, J.Identifier ident) { */ private boolean fieldAccessTargetsVariable(J.FieldAccess fieldAccess) { if (renameVariable.getName().getFieldType() != null) { - J target = getTarget(fieldAccess); + Expression target = getTarget(fieldAccess); + JavaType targetType = resolveType(target.getType()); JavaType.Variable variableNameFieldType = renameVariable.getName().getFieldType(); + if (TypeUtils.isOfType(variableNameFieldType.getOwner(), targetType)) { + return true; + } if (target instanceof J.TypeCast) { - J.TypeCast typeCast = (J.TypeCast) target; - JavaType targetType = resolveType(typeCast.getType()); - return variableNameFieldType.equals(targetType) || targetType != null && targetType.equals(variableNameFieldType.getOwner()); + return TypeUtils.isOfType(variableNameFieldType, targetType); } else if (target instanceof J.Identifier) { - J.Identifier fieldAccessTarget = (J.Identifier) target; - JavaType fieldAccessTargetType = resolveType(fieldAccessTarget.getType()); - return variableNameFieldType.equals(fieldAccessTarget.getFieldType()) || fieldAccessTargetType != null && fieldAccessTargetType.equals(variableNameFieldType.getOwner()); + return TypeUtils.isOfType(variableNameFieldType, ((J.Identifier) target).getFieldType()); } } return false; } @Nullable - private J getTarget(J.FieldAccess fieldAccess) { + private Expression getTarget(J.FieldAccess fieldAccess) { Expression target = fieldAccess.getTarget(); if (target instanceof J.Identifier) { return target; @@ -187,14 +187,15 @@ private J getTarget(J.FieldAccess fieldAccess) { if (target instanceof J.Parentheses) { J tree = ((J.Parentheses) target).getTree(); if (tree instanceof J.TypeCast) { - return tree; + return (J.TypeCast) tree; } return null; } return null; } - private JavaType resolveType(JavaType type) { + @Nullable + private JavaType resolveType(@Nullable JavaType type) { return type instanceof JavaType.Parameterized ? ((JavaType.Parameterized) type).getType() : type; }