From c3733021a6cecbde0e06f1b1241429eededf1ee9 Mon Sep 17 00:00:00 2001 From: Sam Snyder Date: Thu, 23 Jul 2026 00:42:41 -0700 Subject: [PATCH] Add recipes for migrating off a variety of legacy collection types --- recipe-writing-lessons.md | 37 +++ .../ReplaceHashtableWithHashMap.java | 71 +++++ .../ReplaceLegacyCollection.java | 268 ++++++++++++++++ .../ReplaceStringBufferWithStringBuilder.java | 53 ++++ .../ReplaceVectorWithArrayList.java | 74 +++++ .../LegacySynchronizedTypesNotMigrated.java | 53 ++++ .../resources/META-INF/rewrite/recipes.csv | 4 + .../META-INF/rewrite/static-analysis.yml | 16 + .../ReplaceHashtableWithHashMapTest.java | 182 +++++++++++ ...laceStringBufferWithStringBuilderTest.java | 236 ++++++++++++++ .../ReplaceVectorWithArrayListTest.java | 287 ++++++++++++++++++ 11 files changed, 1281 insertions(+) create mode 100644 src/main/java/org/openrewrite/staticanalysis/ReplaceHashtableWithHashMap.java create mode 100644 src/main/java/org/openrewrite/staticanalysis/ReplaceLegacyCollection.java create mode 100644 src/main/java/org/openrewrite/staticanalysis/ReplaceStringBufferWithStringBuilder.java create mode 100644 src/main/java/org/openrewrite/staticanalysis/ReplaceVectorWithArrayList.java create mode 100644 src/main/java/org/openrewrite/staticanalysis/table/LegacySynchronizedTypesNotMigrated.java create mode 100644 src/test/java/org/openrewrite/staticanalysis/ReplaceHashtableWithHashMapTest.java create mode 100644 src/test/java/org/openrewrite/staticanalysis/ReplaceStringBufferWithStringBuilderTest.java create mode 100644 src/test/java/org/openrewrite/staticanalysis/ReplaceVectorWithArrayListTest.java diff --git a/recipe-writing-lessons.md b/recipe-writing-lessons.md index fed24c726..8da5868b7 100644 --- a/recipe-writing-lessons.md +++ b/recipe-writing-lessons.md @@ -282,3 +282,40 @@ Common collections: - `common-static-analysis.yml` - General static analysis fixes - `java-best-practices.yml` - Java-specific best practices - `static-analysis.yml` - Broader static analysis recipes + +## Data-flow-guarded local type replacement (ModernizeCollections) + +When replacing a legacy synchronized type with an unsynchronized one (`Hashtable`→`HashMap`, +`Vector`→`ArrayList`, `Stack`→`Deque`, `StringBuffer`→`StringBuilder`), the transformation is only +sound when the instance is a local variable that never escapes its method. Model this after +`ReplaceStackWithDeque`: a `DataFlowSpec` whose source is the variable initializer and whose sinks +mark escape. Key learnings: + +- **Escape = the reference itself leaves, not a derived value.** Check the sink node's *direct* + parent, not `firstEnclosing(J.Return.class)`. `return v.size()` puts `v` inside a `J.Return`, but + only `size()` is returned — `v` is the method-call receiver and does not escape. The escape sinks + that matter: `parent instanceof J.Return`, `v` in a `J.MethodInvocation`/`J.NewClass` argument list, + and `v` as the RHS of a `J.Assignment` whose target is a field. +- **The rewrite-analysis flow models already handle closure capture precisely.** A buffer captured by + `items.forEach(x -> sb.append(x))` stays confined (converted), while `Runnable r = () -> sb.append(x); + new Thread(r).start()` is correctly detected as escaping (not converted). No hand-rolled lambda guard + is needed. +- **Restrict to local variables.** Use `variable.getVariableType().getOwner() instanceof JavaType.Method` + to exclude fields (which are shared state). Also skip multi-variable declarations + (`Vector a = ..., b = ...`) — the shared type expression cannot be changed for just one variable. +- **Guard type-specific methods.** `Hashtable` (`contains`/`elements`/`keys`) and `Vector` + (`addElement`/`elementAt`/…) expose methods absent from `HashMap`/`ArrayList`; skip when used. + `Vector(int, int)` has no `ArrayList` constructor. `StringBuilder`/`StringBuffer` APIs are identical. +- **Fragment `ChangeType` leaves the old import; re-type the uses too.** Running `ChangeType` on just + the declaration (type expression + initializer) does not re-type the variable's *uses* (`v.add(...)`), + so those keep the old `JavaType`/method type, `typesInUse` still counts the legacy type, and + `maybeRemoveImport` cannot drop the import (`ReplaceStackWithDeque` has this wart). Blanket + `ChangeType` over the whole method is unsafe when the method references non-converted instances of the + same type (a field, an escaping local). The clean fix is precise per-variable re-typing: record the + declaration's `JavaType.Variable` in a `CompilationUnit`-scoped `IdentityHashMap`, then in + `visitIdentifier` re-type any identifier whose `getFieldType()` is that variable, and in + `visitMethodInvocation` re-map the `JavaType.Method` (declaring type **and return type**, so chained + calls like `sb.append(a).append(b)` resolve) whenever the receiver is already the replacement type but + the method type still declares the legacy type. With every reference re-typed, `maybeRemoveImport` + removes the import with no downstream cleanup needed. `ReplaceSynchronizedType` is the shared base that + implements this for `Hashtable`/`Vector`/`StringBuffer`. diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceHashtableWithHashMap.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceHashtableWithHashMap.java new file mode 100644 index 000000000..707775717 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceHashtableWithHashMap.java @@ -0,0 +1,71 @@ +/* + * Copyright 2026 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import lombok.Getter; + +import java.time.Duration; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +import static java.util.Collections.singleton; + +public class ReplaceHashtableWithHashMap extends ReplaceLegacyCollection { + + // Hashtable (Dictionary) methods that HashMap does not provide; if any is used the type cannot be swapped. + private static final Set INCOMPATIBLE_METHODS = new HashSet<>(Arrays.asList( + "contains", "elements", "keys")); + + @Getter + final String displayName = "Replace `java.util.Hashtable` with `java.util.HashMap`"; + + @Getter + final String description = "`Hashtable` synchronizes every operation, which adds overhead in the common " + + "single-threaded case. This recipe replaces a local `Hashtable` with a `HashMap` when data flow " + + "analysis can prove the `Hashtable` never escapes its method (it is not returned, assigned to a field, " + + "or passed as an argument), so no other thread can observe it and the synchronization is redundant. " + + "Fields, escaping variables, and `Hashtable`-specific method usages (`contains`, `elements`, `keys`) " + + "are left untouched. `HashMap` permits `null` keys and values, so it accepts every input `Hashtable` " + + "did."; + + @Getter + final Set tags = singleton("RSPEC-S1149"); + + @Getter + final Duration estimatedEffortPerOccurrence = Duration.ofMinutes(5); + + @Override + String getLegacyType() { + return "java.util.Hashtable"; + } + + @Override + String getReplacementType() { + return "java.util.HashMap"; + } + + @Override + Set getIncompatibleMethods() { + return INCOMPATIBLE_METHODS; + } + + @Override + Set getIncompatibleSupertypes() { + // HashMap does not extend Dictionary, so a value flowing into a Dictionary-typed target cannot be converted. + return singleton("java.util.Dictionary"); + } +} diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceLegacyCollection.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceLegacyCollection.java new file mode 100644 index 000000000..38c91c4ca --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceLegacyCollection.java @@ -0,0 +1,268 @@ +/* + * Copyright 2026 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import org.openrewrite.Cursor; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Preconditions; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.analysis.dataflow.DataFlowNode; +import org.openrewrite.analysis.dataflow.DataFlowSpec; +import org.openrewrite.analysis.dataflow.FindLocalFlowPaths; +import org.openrewrite.internal.ListUtils; +import org.openrewrite.java.ChangeType; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.search.UsesType; +import org.openrewrite.java.tree.*; +import org.openrewrite.staticanalysis.table.LegacySynchronizedTypesNotMigrated; + +import java.util.IdentityHashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; + +import static java.util.Collections.emptySet; + +/** + * Base for recipes that replace a legacy synchronized type (`Hashtable`, `Vector`, `StringBuffer`) + * with its unsynchronized counterpart when data flow analysis proves the instance is a local variable + * that never escapes its method. The whole variable — its declaration, initializer, and every use + * (including chained calls) — is re-typed so that no reference to the legacy type is left behind and + * its import can be removed. + */ +abstract class ReplaceLegacyCollection extends Recipe { + + abstract String getLegacyType(); + + abstract String getReplacementType(); + + java.util.Set getIncompatibleMethods() { + return emptySet(); + } + + boolean isIncompatibleConstructor(J.NewClass newClass) { + return false; + } + + /** + * Supertypes of the legacy type that the replacement does not also implement, so that the + * value cannot flow into a variable declared with one of them (e.g. `Dictionary` for `Hashtable`). + * The legacy type itself is always treated as incompatible. + */ + java.util.Set getIncompatibleSupertypes() { + return emptySet(); + } + + transient LegacySynchronizedTypesNotMigrated notMigrated = new LegacySynchronizedTypesNotMigrated(this); + + @Override + public TreeVisitor getVisitor() { + String legacyType = getLegacyType(); + String replacementType = getReplacementType(); + String replacementSimpleName = replacementType.substring(replacementType.lastIndexOf('.') + 1); + java.util.Set incompatibleMethods = getIncompatibleMethods(); + java.util.Set incompatibleTargets = new java.util.HashSet<>(getIncompatibleSupertypes()); + incompatibleTargets.add(legacyType); + return Preconditions.check(new UsesType<>(legacyType, false), new JavaIsoVisitor() { + @Override + public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext ctx) { + J.VariableDeclarations.NamedVariable v = super.visitVariable(variable, ctx); + + Expression initializer = v.getInitializer(); + JavaType.Variable variableType = v.getVariableType(); + boolean isField = variableType != null && !(variableType.getOwner() instanceof JavaType.Method); + boolean newLegacyInstance = initializer instanceof J.NewClass && TypeUtils.isOfClassType(initializer.getType(), legacyType); + boolean legacyField = isField && variableType != null && TypeUtils.isOfClassType(variableType.getType(), legacyType); + if (!newLegacyInstance && !legacyField) { + return v; + } + + String reason; + if (isField) { + // Fields are shared state by nature; never migrated. + reason = "declared as a field"; + } else { + // A shared type expression across multiple variables cannot be changed for just one of them. + J.VariableDeclarations declaration = getCursor().firstEnclosing(J.VariableDeclarations.class); + J.MethodDeclaration method = getCursor().firstEnclosing(J.MethodDeclaration.class); + if (declaration != null && declaration.getVariables().size() > 1) { + reason = "declared alongside other variables"; + } else if (isIncompatibleConstructor((J.NewClass) initializer)) { + reason = "uses a constructor with no " + replacementSimpleName + " equivalent"; + } else if (method != null && usesIncompatibleMethod(method, variable.getSimpleName(), legacyType, incompatibleMethods)) { + reason = "calls a method with no " + replacementSimpleName + " equivalent"; + } else { + DataFlowSpec escapes = new DataFlowSpec() { + @Override + public boolean isSource(DataFlowNode srcNode) { + return initializer == srcNode.getCursor().getValue(); + } + + @Override + public boolean isSink(DataFlowNode sinkNode) { + return escapesMethod(sinkNode.getCursor(), incompatibleTargets); + } + }; + if (FindLocalFlowPaths.noneMatch(getCursor(), escapes)) { + Expression newInitializer = (Expression) new ChangeType(legacyType, replacementType, false) + .getVisitor().visitNonNull(initializer, ctx, getCursor().getParentOrThrow()); + JavaType newType = newInitializer.getType(); + JavaType.Variable newVariableType = variableType.withType(newType); + v = v.withInitializer(newInitializer) + .withVariableType(newVariableType) + .withName(v.getName().withType(newType).withFieldType(newVariableType)); + // Register the variable so its uses (which reference the old JavaType.Variable) are re-typed. + Map retype = getCursor().getNearestMessage("retype"); + if (retype == null) { + retype = new IdentityHashMap<>(); + getCursor().putMessageOnFirstEnclosing(J.CompilationUnit.class, "retype", retype); + } + retype.put(variableType, newVariableType); + getCursor().putMessageOnFirstEnclosing(J.VariableDeclarations.class, "replace", true); + return v; + } + reason = "escapes the method"; + } + } + + JavaSourceFile cu = getCursor().firstEnclosing(JavaSourceFile.class); + J.ClassDeclaration clazz = getCursor().firstEnclosing(J.ClassDeclaration.class); + notMigrated.insertRow(ctx, new LegacySynchronizedTypesNotMigrated.Row( + cu == null ? "" : cu.getSourcePath().toString(), + clazz != null && clazz.getType() != null ? clazz.getType().getFullyQualifiedName() : "", + legacyType, + reason)); + return v; + } + + @Override + public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) { + J.VariableDeclarations mv = super.visitVariableDeclarations(multiVariable, ctx); + if (getCursor().getMessage("replace", false)) { + mv = mv.withTypeExpression((TypeTree) new ChangeType(legacyType, replacementType, false) + .getVisitor().visit(mv.getTypeExpression(), ctx, getCursor().getParentOrThrow())); + maybeAddImport(replacementType); + maybeRemoveImport(legacyType); + } + return mv; + } + + @Override + public J.Identifier visitIdentifier(J.Identifier identifier, ExecutionContext ctx) { + Map retype = getCursor().getNearestMessage("retype"); + if (retype != null && identifier.getFieldType() != null && retype.containsKey(identifier.getFieldType())) { + JavaType.Variable newVariableType = retype.get(identifier.getFieldType()); + return identifier.withType(newVariableType.getType()).withFieldType(newVariableType); + } + return identifier; + } + + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + J.MethodInvocation mi = super.visitMethodInvocation(method, ctx); + JavaType.Method methodType = mi.getMethodType(); + // Once the receiver has been re-typed to the replacement, its still-legacy method type + // (and, for chained calls, its return type) must follow so no legacy reference remains. + if (methodType != null && mi.getSelect() != null && + TypeUtils.isOfClassType(mi.getSelect().getType(), replacementType) && + TypeUtils.isOfClassType(methodType.getDeclaringType(), legacyType)) { + JavaType.Method newMethodType = remapMethodType(methodType, legacyType, replacementType); + mi = mi.withMethodType(newMethodType).withName(mi.getName().withType(newMethodType)); + } + return mi; + } + }); + } + + private static boolean escapesMethod(Cursor cursor, java.util.Set incompatibleTargets) { + Object value = cursor.getValue(); + Object parent = cursor.getParentTreeCursor().getValue(); + // Contexts where the value leaves the method, or where its static type is relied upon in a way + // the replacement type may not satisfy (cast, array element, either branch of a ternary). + if (parent instanceof J.Return || parent instanceof J.TypeCast || + parent instanceof J.NewArray || parent instanceof J.Ternary) { + return true; + } + if (parent instanceof J.MethodInvocation) { + return ((J.MethodInvocation) parent).getArguments().contains(value); + } + if (parent instanceof J.NewClass) { + return ((J.NewClass) parent).getArguments().contains(value); + } + if (parent instanceof J.Assignment) { + J.Assignment assignment = (J.Assignment) parent; + Expression target = assignment.getVariable(); + return assignment.getAssignment() == value && + (target instanceof J.FieldAccess || + (target instanceof J.Identifier && ((J.Identifier) target).getFieldType() != null)); + } + if (parent instanceof J.VariableDeclarations.NamedVariable) { + // Aliased into another local: unsafe only if the replacement type cannot satisfy that + // variable's declared type (e.g. `Vector v2 = v;` or `Dictionary d = table;`). + J.VariableDeclarations.NamedVariable target = (J.VariableDeclarations.NamedVariable) parent; + // Only an aliasing reference (`v2 = v`), never this variable's own `new` initializer (the source). + if (value instanceof J.Identifier && target.getInitializer() == value) { + for (String incompatible : incompatibleTargets) { + if (TypeUtils.isOfClassType(target.getType(), incompatible)) { + return true; + } + } + } + } + return false; + } + + private static boolean usesIncompatibleMethod(J.MethodDeclaration enclosing, String variableName, + String legacyType, java.util.Set incompatibleMethods) { + if (incompatibleMethods.isEmpty()) { + return false; + } + return new JavaIsoVisitor() { + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, AtomicBoolean found) { + if (found.get()) { + return method; + } + if (method.getSelect() instanceof J.Identifier && + variableName.equals(((J.Identifier) method.getSelect()).getSimpleName()) && + TypeUtils.isOfClassType(method.getSelect().getType(), legacyType) && + incompatibleMethods.contains(method.getSimpleName())) { + found.set(true); + return method; + } + return super.visitMethodInvocation(method, found); + } + }.reduce(enclosing, new AtomicBoolean(false)).get(); + } + + private static JavaType.Method remapMethodType(JavaType.Method methodType, String legacyType, String replacementType) { + return methodType + .withDeclaringType((JavaType.FullyQualified) remapType(methodType.getDeclaringType(), legacyType, replacementType)) + .withReturnType(remapType(methodType.getReturnType(), legacyType, replacementType)) + .withParameterTypes(ListUtils.map(methodType.getParameterTypes(), p -> remapType(p, legacyType, replacementType))); + } + + private static JavaType remapType(JavaType type, String legacyType, String replacementType) { + if (!TypeUtils.isOfClassType(type, legacyType)) { + return type; + } + JavaType.FullyQualified replacement = TypeUtils.asFullyQualified(JavaType.buildType(replacementType)); + if (replacement != null && type instanceof JavaType.Parameterized) { + return new JavaType.Parameterized(null, replacement, ((JavaType.Parameterized) type).getTypeParameters()); + } + return replacement == null ? type : replacement; + } +} diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceStringBufferWithStringBuilder.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceStringBufferWithStringBuilder.java new file mode 100644 index 000000000..0d5901b29 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceStringBufferWithStringBuilder.java @@ -0,0 +1,53 @@ +/* + * Copyright 2026 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import lombok.Getter; + +import java.time.Duration; +import java.util.Set; + +import static java.util.Collections.singleton; + +public class ReplaceStringBufferWithStringBuilder extends ReplaceLegacyCollection { + + @Getter + final String displayName = "Replace `java.lang.StringBuffer` with `java.lang.StringBuilder`"; + + @Getter + final String description = "`StringBuffer` synchronizes every operation, which adds overhead in the common " + + "single-threaded case. `StringBuilder` exposes the identical API without the synchronization. This " + + "recipe replaces a local `StringBuffer` with a `StringBuilder` when data flow analysis can prove the " + + "`StringBuffer` never escapes its method (it is not returned, assigned to a field, or passed as an " + + "argument), so no other thread can observe it and the synchronization is redundant. Fields and " + + "escaping variables are left untouched."; + + @Getter + final Set tags = singleton("RSPEC-S1149"); + + @Getter + final Duration estimatedEffortPerOccurrence = Duration.ofMinutes(5); + + @Override + String getLegacyType() { + return "java.lang.StringBuffer"; + } + + @Override + String getReplacementType() { + return "java.lang.StringBuilder"; + } +} diff --git a/src/main/java/org/openrewrite/staticanalysis/ReplaceVectorWithArrayList.java b/src/main/java/org/openrewrite/staticanalysis/ReplaceVectorWithArrayList.java new file mode 100644 index 000000000..5dc2d76c3 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/ReplaceVectorWithArrayList.java @@ -0,0 +1,74 @@ +/* + * Copyright 2026 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import lombok.Getter; +import org.openrewrite.java.tree.J; + +import java.time.Duration; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +import static java.util.Collections.singleton; + +public class ReplaceVectorWithArrayList extends ReplaceLegacyCollection { + + // Vector methods that ArrayList does not provide; if any is used the type cannot be swapped. + // Note: `ensureCapacity` and `trimToSize` exist on both, so they are intentionally omitted. + private static final Set INCOMPATIBLE_METHODS = new HashSet<>(Arrays.asList( + "addElement", "capacity", "copyInto", "elementAt", "elements", "firstElement", + "insertElementAt", "lastElement", "removeAllElements", "removeElement", + "removeElementAt", "setElementAt", "setSize")); + + @Getter + final String displayName = "Replace `java.util.Vector` with `java.util.ArrayList`"; + + @Getter + final String description = "`Vector` synchronizes every operation, which adds overhead in the common " + + "single-threaded case. This recipe replaces a local `Vector` with an `ArrayList` when data flow " + + "analysis can prove the `Vector` never escapes its method (it is not returned, assigned to a field, " + + "or passed as an argument), so no other thread can observe it and the synchronization is redundant. " + + "Fields, escaping variables, `Vector`-specific method usages (like `elementAt` or `addElement`), and " + + "the `Vector(int, int)` constructor are left untouched."; + + @Getter + final Set tags = singleton("RSPEC-S1149"); + + @Getter + final Duration estimatedEffortPerOccurrence = Duration.ofMinutes(5); + + @Override + String getLegacyType() { + return "java.util.Vector"; + } + + @Override + String getReplacementType() { + return "java.util.ArrayList"; + } + + @Override + Set getIncompatibleMethods() { + return INCOMPATIBLE_METHODS; + } + + @Override + boolean isIncompatibleConstructor(J.NewClass newClass) { + // Vector(int initialCapacity, int capacityIncrement) has no ArrayList equivalent. + return newClass.getArguments().stream().filter(a -> !(a instanceof J.Empty)).count() >= 2; + } +} diff --git a/src/main/java/org/openrewrite/staticanalysis/table/LegacySynchronizedTypesNotMigrated.java b/src/main/java/org/openrewrite/staticanalysis/table/LegacySynchronizedTypesNotMigrated.java new file mode 100644 index 000000000..70a7fdbde --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/table/LegacySynchronizedTypesNotMigrated.java @@ -0,0 +1,53 @@ +/* + * Copyright 2026 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis.table; + +import com.fasterxml.jackson.annotation.JsonIgnoreType; +import lombok.Value; +import org.openrewrite.Column; +import org.openrewrite.DataTable; +import org.openrewrite.Recipe; + +@JsonIgnoreType +public class LegacySynchronizedTypesNotMigrated extends DataTable { + + public LegacySynchronizedTypesNotMigrated(Recipe recipe) { + super(recipe, + "Legacy synchronized types not migrated", + "Instances of a legacy synchronized type (`Hashtable`, `Vector`, `Stack`, `StringBuffer`) that " + + "were found but left unchanged because they could not be proven safe to modernize."); + } + + @Value + public static class Row { + + @Column(displayName = "Source path", + description = "The path to the source file containing the unmigrated reference.") + String sourcePath; + + @Column(displayName = "Class", + description = "The fully qualified name of the class containing the reference.") + String enclosingClass; + + @Column(displayName = "Unmigrated type", + description = "The fully qualified name of the legacy synchronized type that was found but not migrated.") + String unmigratedType; + + @Column(displayName = "Reason", + description = "Why the instance was left unchanged.") + String reason; + } +} diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index 49891d0f4..36303c9ff 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/src/main/resources/META-INF/rewrite/recipes.csv @@ -71,6 +71,7 @@ maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanaly maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.MethodNameCasing,Standardize method name casing,"Fixes method names that do not follow standard naming conventions. For example, `String getFoo_bar()` would be adjusted to `String getFooBar()` and `int DoSomething()` would be adjusted to `int doSomething()`. Following a consistent casing convention for method names improves code readability and helps developers quickly distinguish methods from classes or constants.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,"[{""name"":""includeTestSources"",""type"":""Boolean"",""displayName"":""Apply recipe to test source set"",""description"":""Changes only apply to main by default. `includeTestSources` will apply the recipe to `test` source files.""},{""name"":""renamePublicMethods"",""type"":""Boolean"",""displayName"":""Rename public methods"",""description"":""Changes are not applied to public methods unless specified.""}]", maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.MinimumSwitchCases,`switch` statements should have at least 3 `case` clauses,"`switch` statements are useful when many code paths branch depending on the value of a single expression. For just one or two code paths, the code will be more readable with `if` statements. Using `switch` for trivial branching adds unnecessary syntactic overhead and obscures the simplicity of the logic.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.MissingOverrideAnnotation,Add missing `@Override` to overriding and implementing methods,"Adds `@Override` to methods overriding superclass methods or implementing interface methods. Annotating methods improves readability by showing the author's intent to override. Additionally, when annotated, the compiler will emit an error when a signature of the overridden method does not match the superclass method.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,"[{""name"":""ignoreAnonymousClassMethods"",""type"":""Boolean"",""displayName"":""Ignore methods in anonymous classes"",""description"":""When enabled, ignore missing annotations on methods which override methods when the class definition is within an anonymous class.""}]", +maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ModernizeCollections,Modernize collections,"Replace the legacy synchronized types `Hashtable`, `Vector`, `Stack`, and `StringBuffer` with their modern unsynchronized counterparts `HashMap`, `ArrayList`, `Deque`/`ArrayDeque`, and `StringBuilder`. Each replacement is only applied when data flow analysis can prove the instance is a local variable that never escapes its method, so the synchronization it provided is redundant.",5,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,,"[{""name"":""org.openrewrite.staticanalysis.table.LegacySynchronizedTypesNotMigrated"",""displayName"":""Legacy synchronized types not migrated"",""instanceName"":""Legacy synchronized types not migrated"",""description"":""Instances of a legacy synchronized type (`Hashtable`, `Vector`, `Stack`, `StringBuffer`) that were found but left unchanged because they could not be proven safe to modernize."",""columns"":[{""name"":""sourcePath"",""type"":""String"",""displayName"":""Source path"",""description"":""The path to the source file containing the unmigrated reference.""},{""name"":""enclosingClass"",""type"":""String"",""displayName"":""Class"",""description"":""The fully qualified name of the class containing the reference.""},{""name"":""unmigratedType"",""type"":""String"",""displayName"":""Unmigrated type"",""description"":""The fully qualified name of the legacy synchronized type that was found but not migrated.""},{""name"":""reason"",""type"":""String"",""displayName"":""Reason"",""description"":""Why the instance was left unchanged.""}]}]" maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ModifierOrder,Modifier order,Modifiers should be declared in the correct order as recommended by the JLS. Ordering modifiers consistently reduces cognitive load for developers who are accustomed to the standard sequence.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.MoveConditionsToWhile,Convert `while (true)` with initial `if` break to loop condition,Simplifies `while (true)` loops where the first statement is an `if` statement that only contains a `break`. The condition is inverted and moved to the loop condition for better readability.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.MultipleVariableDeclarations,No multiple variable declarations,"Places each variable declaration in its own statement and on its own line. Using one variable declaration per line encourages commenting and can increase readability. Multi-variable declarations also make it harder to track individual types and initializers, increasing the risk of subtle errors.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, @@ -143,6 +144,7 @@ However, although it's not intuitive, allocating a right-sized array ahead of ti H2 achieved significant performance gains by [switching to empty arrays instead pre-sized ones](https://github.com/h2database/h2database/issues/311).",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceDeprecatedRuntimeExecMethods,Replace deprecated `Runtime#exec()` methods,Replace `Runtime#exec(String)` methods to use `exec(String[])` instead because the former is deprecated after Java 18 and is no longer recommended for use by the Java documentation.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceDuplicateStringLiterals,Replace duplicate `String` literals,"Replaces `String` literals with a length of 5 or greater repeated a minimum of 3 times. Qualified `String` literals include final Strings, method invocations, and new class invocations. Adds a new `private static final String` or uses an existing equivalent class field. A new variable name will be generated based on the literal value if an existing field does not exist. The generated name will append a numeric value to the variable name if a name already exists in the compilation unit. Centralizing repeated string values into constants makes refactoring safer and reduces the risk of inconsistent updates.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,"[{""name"":""includeTestSources"",""type"":""Boolean"",""displayName"":""Apply recipe to test source set"",""description"":""Changes only apply to main by default. `includeTestSources` will apply the recipe to `test` source files.""}]", +maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceHashtableWithHashMap,Replace `java.util.Hashtable` with `java.util.HashMap`,"`Hashtable` synchronizes every operation, which adds overhead in the common single-threaded case. This recipe replaces a local `Hashtable` with a `HashMap` when data flow analysis can prove the `Hashtable` never escapes its method (it is not returned, assigned to a field, or passed as an argument), so no other thread can observe it and the synchronization is redundant. Fields, escaping variables, and `Hashtable`-specific method usages (`contains`, `elements`, `keys`) are left untouched. `HashMap` permits `null` keys and values, so it accepts every input `Hashtable` did.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,,"[{""name"":""org.openrewrite.staticanalysis.table.LegacySynchronizedTypesNotMigrated"",""displayName"":""Legacy synchronized types not migrated"",""instanceName"":""Legacy synchronized types not migrated"",""description"":""Instances of a legacy synchronized type (`Hashtable`, `Vector`, `Stack`, `StringBuffer`) that were found but left unchanged because they could not be proven safe to modernize."",""columns"":[{""name"":""sourcePath"",""type"":""String"",""displayName"":""Source path"",""description"":""The path to the source file containing the unmigrated reference.""},{""name"":""enclosingClass"",""type"":""String"",""displayName"":""Class"",""description"":""The fully qualified name of the class containing the reference.""},{""name"":""unmigratedType"",""type"":""String"",""displayName"":""Unmigrated type"",""description"":""The fully qualified name of the legacy synchronized type that was found but not migrated.""},{""name"":""reason"",""type"":""String"",""displayName"":""Reason"",""description"":""Why the instance was left unchanged.""}]}]" maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceLambdaWithMethodReference,Use method references in lambda,"Replaces the single statement lambdas `o -> o instanceOf X`, `o -> (A) o`, `o -> System.out.println(o)`, `o -> o != null`, `o -> o == null` with the equivalent method reference. Method references are often more concise and readable than their lambda equivalents, making the code's intent clearer at a glance.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceOptionalIsPresentWithIfPresent,Replace `Optional#isPresent()` with `Optional#ifPresent()`,Replace `Optional#isPresent()` with `Optional#ifPresent()`. Please note that this recipe is only suitable for if-blocks that lack an Else-block and have a single condition applied.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceRedundantFormatWithPrintf,Replace redundant String format invocations that are wrapped with PrintStream operations,"Replaces `PrintStream.print(String.format(format, ...args))` with `PrintStream.printf(format, ...args)` (and for `println`, appends a newline to the format string).",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, @@ -150,12 +152,14 @@ maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanaly > A more complete and consistent set of LIFO stack operations is provided by the Deque interface and its implementations, which should be used in preference to this class. `Stack` inherits from `Vector`, which carries unnecessary synchronization overhead in single-threaded contexts and exposes non-stack operations like random index access.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, +maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceStringBufferWithStringBuilder,Replace `java.lang.StringBuffer` with `java.lang.StringBuilder`,"`StringBuffer` synchronizes every operation, which adds overhead in the common single-threaded case. `StringBuilder` exposes the identical API without the synchronization. This recipe replaces a local `StringBuffer` with a `StringBuilder` when data flow analysis can prove the `StringBuffer` never escapes its method (it is not returned, assigned to a field, or passed as an argument), so no other thread can observe it and the synchronization is redundant. Fields and escaping variables are left untouched.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,,"[{""name"":""org.openrewrite.staticanalysis.table.LegacySynchronizedTypesNotMigrated"",""displayName"":""Legacy synchronized types not migrated"",""instanceName"":""Legacy synchronized types not migrated"",""description"":""Instances of a legacy synchronized type (`Hashtable`, `Vector`, `Stack`, `StringBuffer`) that were found but left unchanged because they could not be proven safe to modernize."",""columns"":[{""name"":""sourcePath"",""type"":""String"",""displayName"":""Source path"",""description"":""The path to the source file containing the unmigrated reference.""},{""name"":""enclosingClass"",""type"":""String"",""displayName"":""Class"",""description"":""The fully qualified name of the class containing the reference.""},{""name"":""unmigratedType"",""type"":""String"",""displayName"":""Unmigrated type"",""description"":""The fully qualified name of the legacy synchronized type that was found but not migrated.""},{""name"":""reason"",""type"":""String"",""displayName"":""Reason"",""description"":""Why the instance was left unchanged.""}]}]" maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceStringBuilderWithString,Replace `StringBuilder#append` with `String`,"Replace `StringBuilder.append()` with String if you are only concatenating a small number of strings and the code is simple and easy to read, as the compiler can optimize simple string concatenation expressions into a single String object, which can be more efficient than using StringBuilder.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceStringConcatenationWithStringValueOf,Replace String concatenation with `String.valueOf()`,"Replace inefficient string concatenation patterns like `"""" + ...` with `String.valueOf(...)`. This improves code readability and may have minor performance benefits. The empty string prefix `"""" +` is an indirect way to convert a value to a `String`, while `String.valueOf()` clearly communicates the conversion intent.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceTextBlockWithString,Replace text block with regular string,Replace text block with a regular multi-line string. Text blocks that fit on a single line without concatenation or escaped newlines gain no readability benefit from the triple-quote syntax and are clearer as plain string literals.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceThreadRunWithThreadStart,Replace calls to `Thread.run()` with `Thread.start()`,`Thread.run()` should not be called directly.,2,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceValidateNotNullHavingSingleArgWithObjectsRequireNonNull,Replace `org.apache.commons.lang3.Validate#notNull` with `Objects#requireNonNull`,Replace `org.apache.commons.lang3.Validate.notNull(Object)` with `Objects.requireNonNull(Object)`.,3,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceValidateNotNullHavingVarargsWithObjectsRequireNonNull,Replace `org.apache.commons.lang3.Validate#notNull` with `Objects#requireNonNull`,"Replace `org.apache.commons.lang3.Validate.notNull(Object, String, Object[])` with `Objects.requireNonNull(Object, String)`.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, +maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceVectorWithArrayList,Replace `java.util.Vector` with `java.util.ArrayList`,"`Vector` synchronizes every operation, which adds overhead in the common single-threaded case. This recipe replaces a local `Vector` with an `ArrayList` when data flow analysis can prove the `Vector` never escapes its method (it is not returned, assigned to a field, or passed as an argument), so no other thread can observe it and the synchronization is redundant. Fields, escaping variables, `Vector`-specific method usages (like `elementAt` or `addElement`), and the `Vector(int, int)` constructor are left untouched.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,,"[{""name"":""org.openrewrite.staticanalysis.table.LegacySynchronizedTypesNotMigrated"",""displayName"":""Legacy synchronized types not migrated"",""instanceName"":""Legacy synchronized types not migrated"",""description"":""Instances of a legacy synchronized type (`Hashtable`, `Vector`, `Stack`, `StringBuffer`) that were found but left unchanged because they could not be proven safe to modernize."",""columns"":[{""name"":""sourcePath"",""type"":""String"",""displayName"":""Source path"",""description"":""The path to the source file containing the unmigrated reference.""},{""name"":""enclosingClass"",""type"":""String"",""displayName"":""Class"",""description"":""The fully qualified name of the class containing the reference.""},{""name"":""unmigratedType"",""type"":""String"",""displayName"":""Unmigrated type"",""description"":""The fully qualified name of the legacy synchronized type that was found but not migrated.""},{""name"":""reason"",""type"":""String"",""displayName"":""Reason"",""description"":""Why the instance was left unchanged.""}]}]" maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.ReplaceWeekYearWithYear,Week Year (YYYY) should not be used for date formatting,"For most dates Week Year (YYYY) and Year (yyyy) yield the same results. However, on the last week of December and the first week of January, Week Year could produce unexpected results. This is a common source of off-by-one-year bugs that typically only manifest around New Year's Eve, making them difficult to catch during development and testing.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.SillyEqualsCheck,Silly equality checks should not be made,Detects `.equals()` calls that compare incompatible types and will always return `false`. Replaces `.equals(null)` with `== null` and array `.equals()` with `Arrays.equals()`. Flags comparisons between unrelated types or between arrays and non-arrays.,1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, maven,org.openrewrite.recipe:rewrite-static-analysis,org.openrewrite.staticanalysis.SimplifyArraysAsList,Simplify `Arrays.asList(..)` with varargs,"Simplifies `Arrays.asList()` method calls that use explicit array creation to use varargs instead. For example, `Arrays.asList(new String[]{""a"", ""b"", ""c""})` becomes `Arrays.asList(""a"", ""b"", ""c"")`. Explicitly constructing an array to pass to a varargs parameter adds visual clutter without changing behavior, since the compiler generates the array automatically.",1,,Static analysis and remediation,,Remediations for issues identified by SAST tools.,, diff --git a/src/main/resources/META-INF/rewrite/static-analysis.yml b/src/main/resources/META-INF/rewrite/static-analysis.yml index ca101eb29..b06a610ef 100644 --- a/src/main/resources/META-INF/rewrite/static-analysis.yml +++ b/src/main/resources/META-INF/rewrite/static-analysis.yml @@ -91,3 +91,19 @@ recipeList: - java.lang.* tags: - RSPEC-S1217 +--- +type: specs.openrewrite.org/v1beta/recipe +name: org.openrewrite.staticanalysis.ModernizeCollections +displayName: Modernize collections +description: >- + Replace the legacy synchronized types `Hashtable`, `Vector`, `Stack`, and `StringBuffer` with their + modern unsynchronized counterparts `HashMap`, `ArrayList`, `Deque`/`ArrayDeque`, and `StringBuilder`. + Each replacement is only applied when data flow analysis can prove the instance is a local variable + that never escapes its method, so the synchronization it provided is redundant. +tags: + - RSPEC-S1149 +recipeList: + - org.openrewrite.staticanalysis.ReplaceHashtableWithHashMap + - org.openrewrite.staticanalysis.ReplaceVectorWithArrayList + - org.openrewrite.staticanalysis.ReplaceStackWithDeque + - org.openrewrite.staticanalysis.ReplaceStringBufferWithStringBuilder diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceHashtableWithHashMapTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceHashtableWithHashMapTest.java new file mode 100644 index 000000000..da5b77b42 --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceHashtableWithHashMapTest.java @@ -0,0 +1,182 @@ +/* + * Copyright 2026 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class ReplaceHashtableWithHashMapTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new ReplaceHashtableWithHashMap()); + } + + @DocumentExample + @Test + void replaceConfinedHashtable() { + rewriteRun( + //language=java + java( + """ + import java.util.Hashtable; + + class Test { + int test() { + Hashtable table = new Hashtable<>(); + table.put("a", 1); + return table.size(); + } + } + """, + """ + import java.util.HashMap; + + class Test { + int test() { + HashMap table = new HashMap<>(); + table.put("a", 1); + return table.size(); + } + } + """ + ) + ); + } + + @Test + void doNotReplaceWhenReturned() { + rewriteRun( + //language=java + java( + """ + import java.util.Hashtable; + + class Test { + Hashtable test() { + Hashtable table = new Hashtable<>(); + table.put("a", 1); + return table; + } + } + """ + ) + ); + } + + @Test + void doNotReplaceWhenAssignedToField() { + rewriteRun( + //language=java + java( + """ + import java.util.Hashtable; + + class Test { + Hashtable field; + + void test() { + Hashtable table = new Hashtable<>(); + this.field = table; + } + } + """ + ) + ); + } + + @Test + void doNotReplaceField() { + rewriteRun( + //language=java + java( + """ + import java.util.Hashtable; + + class Test { + Hashtable field = new Hashtable<>(); + } + """ + ) + ); + } + + @Test + void doNotReplaceWhenEnumerationMethodUsed() { + rewriteRun( + //language=java + java( + """ + import java.util.Enumeration; + import java.util.Hashtable; + + class Test { + void test() { + Hashtable table = new Hashtable<>(); + table.put("a", 1); + Enumeration keys = table.keys(); + } + } + """ + ) + ); + } + + @Test + void doNotReplaceWhenAliasedToDictionaryTypedLocal() { + // Converting `table` would leave `Dictionary<...> d = table;` assigning a HashMap to a Dictionary. + rewriteRun( + //language=java + java( + """ + import java.util.Dictionary; + import java.util.Hashtable; + + class Test { + void test() { + Hashtable table = new Hashtable<>(); + Dictionary d = table; + d.put("a", 1); + } + } + """ + ) + ); + } + + @Test + void doNotReplaceWhenContainsUsed() { + rewriteRun( + //language=java + java( + """ + import java.util.Hashtable; + + class Test { + boolean test() { + Hashtable table = new Hashtable<>(); + table.put("a", 1); + return table.contains(1); + } + } + """ + ) + ); + } +} diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceStringBufferWithStringBuilderTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceStringBufferWithStringBuilderTest.java new file mode 100644 index 000000000..24fad84d4 --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceStringBufferWithStringBuilderTest.java @@ -0,0 +1,236 @@ +/* + * Copyright 2026 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class ReplaceStringBufferWithStringBuilderTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new ReplaceStringBufferWithStringBuilder()); + } + + @DocumentExample + @Test + void replaceConfinedStringBuffer() { + rewriteRun( + //language=java + java( + """ + class Test { + String test() { + StringBuffer sb = new StringBuffer(); + sb.append("a").append("b"); + return sb.toString(); + } + } + """, + """ + class Test { + String test() { + StringBuilder sb = new StringBuilder(); + sb.append("a").append("b"); + return sb.toString(); + } + } + """ + ) + ); + } + + @Test + void replaceWithCapacityAndInitialValue() { + rewriteRun( + //language=java + java( + """ + class Test { + String test() { + StringBuffer sb = new StringBuffer("start"); + sb.append("more"); + return sb.substring(0); + } + } + """, + """ + class Test { + String test() { + StringBuilder sb = new StringBuilder("start"); + sb.append("more"); + return sb.substring(0); + } + } + """ + ) + ); + } + + @Test + void replaceChainedAppendAssignedToLocal() { + // Exercises method-type return re-typing: the inner `append` must return `StringBuilder` + // so the outer `append` and the assignment target resolve without any lingering `StringBuffer`. + rewriteRun( + //language=java + java( + """ + class Test { + int test() { + StringBuffer sb = new StringBuffer(); + CharSequence cs = sb.append("a").append("b"); + return sb.length(); + } + } + """, + """ + class Test { + int test() { + StringBuilder sb = new StringBuilder(); + CharSequence cs = sb.append("a").append("b"); + return sb.length(); + } + } + """ + ) + ); + } + + @Test + void replaceWhenCapturedInLocalLambda() { + rewriteRun( + //language=java + java( + """ + import java.util.List; + + class Test { + String test(List items) { + StringBuffer sb = new StringBuffer(); + items.forEach(x -> sb.append(x)); + return sb.toString(); + } + } + """, + """ + import java.util.List; + + class Test { + String test(List items) { + StringBuilder sb = new StringBuilder(); + items.forEach(x -> sb.append(x)); + return sb.toString(); + } + } + """ + ) + ); + } + + @Test + void doNotReplaceWhenCapturedInLambdaThatEscapesToThread() { + // Data flow tracks the buffer through the capturing lambda: because `r` is handed to a + // `Thread`, the buffer may be mutated concurrently, so the synchronization is not redundant. + rewriteRun( + //language=java + java( + """ + class Test { + void test() { + StringBuffer sb = new StringBuffer(); + Runnable r = () -> sb.append("x"); + new Thread(r).start(); + } + } + """ + ) + ); + } + + @Test + void doNotReplaceWhenReturned() { + rewriteRun( + //language=java + java( + """ + class Test { + StringBuffer test() { + StringBuffer sb = new StringBuffer(); + sb.append("a"); + return sb; + } + } + """ + ) + ); + } + + @Test + void doNotReplaceWhenAssignedToField() { + rewriteRun( + //language=java + java( + """ + class Test { + StringBuffer field; + + void test() { + StringBuffer sb = new StringBuffer(); + this.field = sb; + } + } + """ + ) + ); + } + + @Test + void doNotReplaceWhenPassedAsArgument() { + rewriteRun( + //language=java + java( + """ + class Test { + void consume(StringBuffer other) { + } + + void test() { + StringBuffer sb = new StringBuffer(); + consume(sb); + } + } + """ + ) + ); + } + + @Test + void doNotReplaceField() { + rewriteRun( + //language=java + java( + """ + class Test { + StringBuffer field = new StringBuffer(); + } + """ + ) + ); + } +} diff --git a/src/test/java/org/openrewrite/staticanalysis/ReplaceVectorWithArrayListTest.java b/src/test/java/org/openrewrite/staticanalysis/ReplaceVectorWithArrayListTest.java new file mode 100644 index 000000000..5046370df --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/ReplaceVectorWithArrayListTest.java @@ -0,0 +1,287 @@ +/* + * Copyright 2026 the original author or authors. + *

+ * Licensed under the Moderne Source Available License (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + *

+ * https://docs.moderne.io/licensing/moderne-source-available-license + *

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class ReplaceVectorWithArrayListTest implements RewriteTest { + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new ReplaceVectorWithArrayList()); + } + + @DocumentExample + @Test + void replaceConfinedVector() { + rewriteRun( + //language=java + java( + """ + import java.util.Vector; + + class Test { + int test() { + Vector v = new Vector<>(); + v.add(1); + v.add(2); + return v.size(); + } + } + """, + """ + import java.util.ArrayList; + + class Test { + int test() { + ArrayList v = new ArrayList<>(); + v.add(1); + v.add(2); + return v.size(); + } + } + """ + ) + ); + } + + @Test + void doNotReplaceWhenReturned() { + rewriteRun( + //language=java + java( + """ + import java.util.Vector; + + class Test { + Vector test() { + Vector v = new Vector<>(); + v.add(1); + return v; + } + } + """ + ) + ); + } + + @Test + void doNotReplaceWhenAssignedToField() { + rewriteRun( + //language=java + java( + """ + import java.util.Vector; + + class Test { + Vector field; + + void test() { + Vector v = new Vector<>(); + v.add(1); + this.field = v; + } + } + """ + ) + ); + } + + @Test + void doNotReplaceWhenPassedAsArgument() { + rewriteRun( + //language=java + java( + """ + import java.util.Vector; + + class Test { + void consume(Vector other) { + } + + void test() { + Vector v = new Vector<>(); + v.add(1); + consume(v); + } + } + """ + ) + ); + } + + @Test + void doNotReplaceField() { + rewriteRun( + //language=java + java( + """ + import java.util.Vector; + + class Test { + Vector field = new Vector<>(); + } + """ + ) + ); + } + + @Test + void doNotReplaceWhenVectorSpecificMethodUsed() { + rewriteRun( + //language=java + java( + """ + import java.util.Vector; + + class Test { + void test() { + Vector v = new Vector<>(); + v.addElement(1); + v.elementAt(0); + } + } + """ + ) + ); + } + + @Test + void doNotReplaceCapacityIncrementConstructor() { + rewriteRun( + //language=java + java( + """ + import java.util.Vector; + + class Test { + void test() { + Vector v = new Vector<>(10, 5); + v.add(1); + } + } + """ + ) + ); + } + + @Test + void replaceWhenDeclaredAsListInterface() { + rewriteRun( + //language=java + java( + """ + import java.util.List; + import java.util.Vector; + + class Test { + int test() { + List l = new Vector<>(); + l.add(1); + return l.size(); + } + } + """, + """ + import java.util.ArrayList; + import java.util.List; + + class Test { + int test() { + List l = new ArrayList<>(); + l.add(1); + return l.size(); + } + } + """ + ) + ); + } + + @Test + void doNotReplaceWhenAliasedToVectorTypedLocal() { + // Converting `v` would leave `Vector v2 = v;` assigning an ArrayList to a Vector. + rewriteRun( + //language=java + java( + """ + import java.util.Vector; + + class Test { + void test() { + Vector v = new Vector<>(); + Vector v2 = v; + v2.add(1); + } + } + """ + ) + ); + } + + @Test + void doNotReplaceMultiVariableDeclaration() { + rewriteRun( + //language=java + java( + """ + import java.util.Vector; + + class Test { + void test() { + Vector a = new Vector<>(), b = new Vector<>(); + a.add(1); + b.add(2); + } + } + """ + ) + ); + } + + @Test + void replaceWithInitialCapacity() { + rewriteRun( + //language=java + java( + """ + import java.util.Vector; + + class Test { + void test() { + Vector v = new Vector<>(10); + v.add(1); + } + } + """, + """ + import java.util.ArrayList; + + class Test { + void test() { + ArrayList v = new ArrayList<>(10); + v.add(1); + } + } + """ + ) + ); + } +}