Add recipes for migrating off a variety of legacy collection types

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`.

src/main/java/org/openrewrite/staticanalysis/ReplaceHashtableWithHashMap.java

@@ -0,0 +1,71 @@
+/*
+ * Copyright 2026 the original author or authors.
+ * <p>
+ * 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
+ * <p>
+ * https://docs.moderne.io/licensing/moderne-source-available-license
+ * <p>
+ * 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<String> 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<String> 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<String> getIncompatibleMethods() {
+        return INCOMPATIBLE_METHODS;
+    }
+
+    @Override
+    Set<String> getIncompatibleSupertypes() {
+        // HashMap does not extend Dictionary, so a value flowing into a Dictionary-typed target cannot be converted.
+        return singleton("java.util.Dictionary");
+    }
+}