From de21a9ec30ab2ef26f133afb16ce476d914ddd6e Mon Sep 17 00:00:00 2001 From: Joan Viladrosa Date: Wed, 18 Oct 2023 15:06:38 +0200 Subject: [PATCH 1/3] rename when same id in different scopes --- .../RenameLocalVariablesToCamelCase.java | 7 +- .../RenamePrivateFieldsToCamelCase.java | 8 ++- .../staticanalysis/RenameToCamelCase.java | 40 +++++++++++- .../RenameLocalVariablesToCamelCaseTest.java | 64 +++++++++++++++++++ 4 files changed, 113 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCase.java b/src/main/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCase.java index 0251b1819a..cc876a6eaa 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCase.java +++ b/src/main/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCase.java @@ -77,7 +77,8 @@ protected boolean shouldRename(Set hasNameKey, J.VariableDeclarations.Na if (toName.isEmpty() || !Character.isAlphabetic(toName.charAt(0))) { return false; } - return !hasNameKey.contains(toName); + Set keys = computeAllKeys(toName, variable); + return keys.stream().noneMatch(hasNameKey::contains); } @Override @@ -94,7 +95,7 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m if (!LOWER_CAMEL.matches(name) && name.length() > 1) { renameVariable(v, LOWER_CAMEL.format(name)); } else { - hasNameKey(name); + hasNameKey(computeKey(name, v)); } } return mv; @@ -140,7 +141,7 @@ private boolean isDeclaredInCatch() { @Override public J.Identifier visitIdentifier(J.Identifier identifier, ExecutionContext ctx) { - hasNameKey(identifier.getSimpleName()); + hasNameKey(computeKey(identifier.getSimpleName(), identifier)); return identifier; } diff --git a/src/main/java/org/openrewrite/staticanalysis/RenamePrivateFieldsToCamelCase.java b/src/main/java/org/openrewrite/staticanalysis/RenamePrivateFieldsToCamelCase.java index 0de3a7755a..0e2d5a3116 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RenamePrivateFieldsToCamelCase.java +++ b/src/main/java/org/openrewrite/staticanalysis/RenamePrivateFieldsToCamelCase.java @@ -75,7 +75,11 @@ protected boolean shouldRename(Set hasNameKey, J.VariableDeclarations.Na if (toName.isEmpty() || !Character.isAlphabetic(toName.charAt(0))) { return false; } - return !hasNameKey.contains(toName) && !hasNameKey.contains(variable.getSimpleName()); + return hasNameKey.stream().noneMatch(key -> + key.equals(toName) || + key.endsWith(" " + toName) || + key.endsWith(" " + variable.getSimpleName()) + ); } @SuppressWarnings("all") @@ -110,7 +114,7 @@ public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations String toName = LOWER_CAMEL.format(variable.getSimpleName()); renameVariable(variable, toName); } else { - hasNameKey(variable.getSimpleName()); + hasNameKey(computeKey(variable.getSimpleName(), variable)); } return super.visitVariable(variable, ctx); diff --git a/src/main/java/org/openrewrite/staticanalysis/RenameToCamelCase.java b/src/main/java/org/openrewrite/staticanalysis/RenameToCamelCase.java index e694fd7b26..e81192bdec 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RenameToCamelCase.java +++ b/src/main/java/org/openrewrite/staticanalysis/RenameToCamelCase.java @@ -22,6 +22,7 @@ import org.openrewrite.java.RenameVariable; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaSourceFile; +import org.openrewrite.java.tree.JavaType; import java.util.HashSet; import java.util.LinkedHashMap; @@ -43,7 +44,7 @@ public abstract class RenameToCamelCase extends JavaIsoVisitor String toName = entry.getValue(); if (shouldRename(hasNameSet, variable, toName)) { cu = (JavaSourceFile) new RenameVariable<>(variable, toName).visitNonNull(cu, ctx); - hasNameSet.add(toName); + hasNameSet.add(computeKey(toName, variable)); } } return cu; @@ -65,4 +66,41 @@ protected void hasNameKey(String variableName) { .computeMessageIfAbsent("HAS_NAME_KEY", k -> new HashSet<>()) .add(variableName); } + + protected String computeKey(String identifier, J context) { + JavaType.Variable fieldType = getFieldType(context); + if (fieldType != null && fieldType.getOwner() != null) { + return fieldType.getOwner() + " " + identifier; + } + return identifier; + } + + protected Set computeAllKeys(String identifier, J context) { + Set keys = new HashSet<>(); + keys.add(identifier); + JavaType.Variable fieldType = getFieldType(context); + if (fieldType != null && fieldType.getOwner() != null) { + keys.add(fieldType.getOwner() + " " + identifier); + if (fieldType.getOwner() instanceof JavaType.Method) { + // Add all enclosing classes + JavaType.FullyQualified declaringType = ((JavaType.Method) fieldType.getOwner()).getDeclaringType(); + while (declaringType != null) { + keys.add(declaringType + " " + identifier); + declaringType = declaringType.getOwningClass(); + } + } + } + return keys; + } + + @Nullable + protected JavaType.Variable getFieldType(J tree) { + if (tree instanceof J.Identifier) { + return ((J.Identifier) tree).getFieldType(); + } + if (tree instanceof J.VariableDeclarations.NamedVariable) { + return ((J.VariableDeclarations.NamedVariable) tree).getVariableType(); + } + return null; + } } diff --git a/src/test/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCaseTest.java b/src/test/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCaseTest.java index ffa96572cf..dc40fb394e 100644 --- a/src/test/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCaseTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCaseTest.java @@ -372,4 +372,68 @@ public void doSomething() { ) ); } + + @Test + @Issue("https://github.com/openrewrite/rewrite-static-analysis/issues/171") + void renameMultipleOcurrencesDifferentScope() { + rewriteRun( + java( + """ + class Test { + void test() { + String ID; + } + void test2() { + String ID; + } + } + """, + """ + class Test { + void test() { + String id; + } + void test2() { + String id; + } + } + """ + ) + ); + } + + @Test + void doNotRenameIfInParent() { + rewriteRun( + java( + """ + class Test { + String id; + void test() { + String ID; + } + } + """ + ) + ); + } + + @Test + void doNotRenameIfInParentFromInnerClass() { + rewriteRun( + java( + """ + class Test { + String id; + class InnerClass { + void test() { + String ID; + } + } + } + """ + ) + ); + } + } From 85653efba76dd5ddc4c65ca96a712ffc086f49f0 Mon Sep 17 00:00:00 2001 From: Joan Viladrosa Date: Wed, 18 Oct 2023 15:11:28 +0200 Subject: [PATCH 2/3] missing case --- .../staticanalysis/RenamePrivateFieldsToCamelCase.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/openrewrite/staticanalysis/RenamePrivateFieldsToCamelCase.java b/src/main/java/org/openrewrite/staticanalysis/RenamePrivateFieldsToCamelCase.java index 0e2d5a3116..144c303b63 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RenamePrivateFieldsToCamelCase.java +++ b/src/main/java/org/openrewrite/staticanalysis/RenamePrivateFieldsToCamelCase.java @@ -77,6 +77,7 @@ protected boolean shouldRename(Set hasNameKey, J.VariableDeclarations.Na } return hasNameKey.stream().noneMatch(key -> key.equals(toName) || + key.equals(variable.getSimpleName()) || key.endsWith(" " + toName) || key.endsWith(" " + variable.getSimpleName()) ); From c2cc145f7097c6a2e9385533570f7dcc94481d05 Mon Sep 17 00:00:00 2001 From: Joan Viladrosa Date: Wed, 18 Oct 2023 15:13:43 +0200 Subject: [PATCH 3/3] moved method to children class --- .../RenameLocalVariablesToCamelCase.java | 20 +++++++++++++++++++ .../staticanalysis/RenameToCamelCase.java | 18 ----------------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCase.java b/src/main/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCase.java index cc876a6eaa..66df86ea34 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCase.java +++ b/src/main/java/org/openrewrite/staticanalysis/RenameLocalVariablesToCamelCase.java @@ -21,9 +21,11 @@ import org.openrewrite.TreeVisitor; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaSourceFile; +import org.openrewrite.java.tree.JavaType; import java.time.Duration; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Set; @@ -168,6 +170,24 @@ private Cursor getCursorToParentScope(Cursor cursor) { is instanceof JavaSourceFile ); } + + private Set computeAllKeys(String identifier, J context) { + Set keys = new HashSet<>(); + keys.add(identifier); + JavaType.Variable fieldType = getFieldType(context); + if (fieldType != null && fieldType.getOwner() != null) { + keys.add(fieldType.getOwner() + " " + identifier); + if (fieldType.getOwner() instanceof JavaType.Method) { + // Add all enclosing classes + JavaType.FullyQualified declaringType = ((JavaType.Method) fieldType.getOwner()).getDeclaringType(); + while (declaringType != null) { + keys.add(declaringType + " " + identifier); + declaringType = declaringType.getOwningClass(); + } + } + } + return keys; + } }; } } diff --git a/src/main/java/org/openrewrite/staticanalysis/RenameToCamelCase.java b/src/main/java/org/openrewrite/staticanalysis/RenameToCamelCase.java index e81192bdec..a877aa1d01 100644 --- a/src/main/java/org/openrewrite/staticanalysis/RenameToCamelCase.java +++ b/src/main/java/org/openrewrite/staticanalysis/RenameToCamelCase.java @@ -75,24 +75,6 @@ protected String computeKey(String identifier, J context) { return identifier; } - protected Set computeAllKeys(String identifier, J context) { - Set keys = new HashSet<>(); - keys.add(identifier); - JavaType.Variable fieldType = getFieldType(context); - if (fieldType != null && fieldType.getOwner() != null) { - keys.add(fieldType.getOwner() + " " + identifier); - if (fieldType.getOwner() instanceof JavaType.Method) { - // Add all enclosing classes - JavaType.FullyQualified declaringType = ((JavaType.Method) fieldType.getOwner()).getDeclaringType(); - while (declaringType != null) { - keys.add(declaringType + " " + identifier); - declaringType = declaringType.getOwningClass(); - } - } - } - return keys; - } - @Nullable protected JavaType.Variable getFieldType(J tree) { if (tree instanceof J.Identifier) {