Skip to content

Commit

Permalink
Merge pull request #377 from openrewrite/update-replace-duplicate-str…
Browse files Browse the repository at this point in the history 
…ing-to-use-variable-name-to-track-name

Updating ReplaceDuplicateStringLiterals recipe to use VariableNameUtils to track variable names
  • Loading branch information
@lkerford
lkerford authored Oct 22, 2024
2 parents 619b623 + f4a2a38 commit b19a745
Show file tree
Hide file tree
Showing 2 changed files with 117 additions and 9 deletions.
26 changes: 17 additions & 9 deletions src/main/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiterals.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

import java.time.Duration;
import java.util.*;
import java.util.stream.Collectors;

import static java.util.Arrays.asList;
import static java.util.Collections.emptyList;
Expand Down Expand Up @@ -97,8 +98,9 @@ public J visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ct
if (duplicateLiteralsMap.isEmpty()) {
return classDecl;
}
Set<String> variableNames = duplicateLiteralInfo.getVariableNames();
Map<String, String> fieldValueToFieldName = duplicateLiteralInfo.getFieldValueToFieldName();
Set<String> variableNames = VariableNameUtils.findNamesInScope(getCursor()).stream()
.filter(i -> !fieldValueToFieldName.containsValue(i)).collect(Collectors.toSet());
String classFqn = classDecl.getType().getFullyQualifiedName();
Map<J.Literal, String> replacements = new HashMap<>();
for (Map.Entry<String, List<J.Literal>> entry : duplicateLiteralsMap.entrySet()) {
Expand All @@ -107,7 +109,13 @@ public J visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ct
String classFieldName = fieldValueToFieldName.get(valueOfLiteral);
String variableName;
if (classFieldName != null) {
variableName = getNameWithoutShadow(classFieldName, variableNames);
String maybeVariableName = getNameWithoutShadow(classFieldName, variableNames);
if (duplicateLiteralInfo.existingFieldValueToFieldName.get(maybeVariableName) != null) {
variableNames.add(maybeVariableName);
maybeVariableName = getNameWithoutShadow(classFieldName, variableNames);
}

variableName = maybeVariableName;
if (StringUtils.isBlank(variableName)) {
continue;
}
Expand Down Expand Up @@ -199,14 +207,14 @@ private static boolean isPrivateStaticFinalVariable(J.VariableDeclarations.Named

@Value
private static class DuplicateLiteralInfo {
Set<String> variableNames;
Map<String, String> fieldValueToFieldName;
Map<String, String> existingFieldValueToFieldName;

@NonFinal
Map<String, List<J.Literal>> duplicateLiterals;

public static DuplicateLiteralInfo find(J.ClassDeclaration inClass) {
DuplicateLiteralInfo result = new DuplicateLiteralInfo(new LinkedHashSet<>(), new LinkedHashMap<>(), new HashMap<>());
DuplicateLiteralInfo result = new DuplicateLiteralInfo(new LinkedHashMap<>(), new LinkedHashMap<>(), new HashMap<>());
new JavaIsoVisitor<Integer>() {

@Override
Expand All @@ -221,11 +229,11 @@ public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations
Cursor parentScope = getCursor().dropParentUntil(is -> is instanceof J.ClassDeclaration || is instanceof J.MethodDeclaration);
boolean privateStaticFinalVariable = isPrivateStaticFinalVariable(variable);
// `private static final String`(s) are handled separately by `FindExistingPrivateStaticFinalFields`.
if (parentScope.getValue() instanceof J.MethodDeclaration ||
parentScope.getValue() instanceof J.ClassDeclaration &&
!(privateStaticFinalVariable && v.getInitializer() instanceof J.Literal &&
((J.Literal) v.getInitializer()).getValue() instanceof String)) {
result.variableNames.add(v.getSimpleName());
if (v.getInitializer() instanceof J.Literal &&
(parentScope.getValue() instanceof J.MethodDeclaration || parentScope.getValue() instanceof J.ClassDeclaration) &&
!(privateStaticFinalVariable && ((J.Literal) v.getInitializer()).getValue() instanceof String)) {
String value = (((J.Literal) v.getInitializer()).getValue()).toString();
result.existingFieldValueToFieldName.put(v.getSimpleName(), value);
}
if (parentScope.getValue() instanceof J.ClassDeclaration &&
privateStaticFinalVariable && v.getInitializer() instanceof J.Literal &&
Expand Down
100 changes: 100 additions & 0 deletions src/test/java/org/openrewrite/staticanalysis/ReplaceDuplicateStringLiteralsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,106 @@ class A {
);
}

@Test
void enumCollidesWithNewStringLiteral() {
rewriteRun(
//language=java
java(
"""
package org.foo;
enum TYPES {
VALUE, NUMBER, TEXT
}
class A {
final String val1 = "types";
final String val2 = "types";
final String val3 = "types";
TYPES type = TYPES.VALUE;
}
""",
"""
package org.foo;
enum TYPES {
VALUE, NUMBER, TEXT
}
class A {
private static final String TYPES_1 = "types";
final String val1 = TYPES_1;
final String val2 = TYPES_1;
final String val3 = TYPES_1;
TYPES type = TYPES.VALUE;
}
"""
)
);
}

@Test
void fieldNameCollidesWithNewStringLiteral() {
rewriteRun(
//language=java
java(
"""
package org.foo;
class A {
final String val1 = "value";
final String val2 = "value";
final String val3 = "value";
final int VALUE = 1;
}
""",
"""
package org.foo;
class A {
private static final String VALUE_1 = "value";
final String val1 = VALUE_1;
final String val2 = VALUE_1;
final String val3 = VALUE_1;
final int VALUE = 1;
}
"""
)
);
}

@Test
void staticImportCollidesWithNewStringLiteral() {
rewriteRun(
//language=java
java(
"""
package org.foo;
import static java.lang.Long.MAX_VALUE;
class A {
final String val1 = "max_value";
final String val2 = "max_value";
final String val3 = "max_value";
final long value = MAX_VALUE;
}
""",
"""
package org.foo;
import static java.lang.Long.MAX_VALUE;
class A {
private static final String MAX_VALUE_1 = "max_value";
final String val1 = MAX_VALUE_1;
final String val2 = MAX_VALUE_1;
final String val3 = MAX_VALUE_1;
final long value = MAX_VALUE;
}
"""
)
);
}

@Test
void generatedNameIsVeryLong() {
rewriteRun(
Expand Down

0 comments on commit b19a745

Please sign in to comment.