From d062ebf1fa00fdbc97b20e442dba7d8f2215d58f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Tue, 14 Oct 2025 15:06:28 +0200 Subject: [PATCH 01/12] basic recipe to find thread local variables that are never mutated --- .../FindImmutableThreadLocalVariables.java | 204 ++++++++++++ ...FindImmutableThreadLocalVariablesTest.java | 295 ++++++++++++++++++ 2 files changed, 499 insertions(+) create mode 100644 src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java create mode 100644 src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java b/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java new file mode 100644 index 0000000000..0ce9fa690d --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java @@ -0,0 +1,204 @@ +/* + * Copyright 2024 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.java.migrate.search; + +import lombok.EqualsAndHashCode; +import lombok.Value; +import org.openrewrite.ExecutionContext; +import org.openrewrite.ScanningRecipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.TypeUtils; +import org.openrewrite.marker.SearchResult; + +import java.util.HashSet; +import java.util.Set; + +@EqualsAndHashCode(callSuper = false) +public class FindImmutableThreadLocalVariables extends ScanningRecipe { + + private static final MethodMatcher THREAD_LOCAL_SET = new MethodMatcher("java.lang.ThreadLocal set(..)"); + private static final MethodMatcher THREAD_LOCAL_REMOVE = new MethodMatcher("java.lang.ThreadLocal remove()"); + private static final MethodMatcher INHERITABLE_THREAD_LOCAL_SET = new MethodMatcher("java.lang.InheritableThreadLocal set(..)"); + private static final MethodMatcher INHERITABLE_THREAD_LOCAL_REMOVE = new MethodMatcher("java.lang.InheritableThreadLocal remove()"); + + @Override + public String getDisplayName() { + return "Find immutable ThreadLocal variables"; + } + + @Override + public String getDescription() { + return "Find `ThreadLocal` variables that are never mutated and could be candidates for migration to `ScopedValue` in Java 25+. " + + "This recipe identifies `ThreadLocal` variables that are only initialized but never reassigned or modified through `set()` or `remove()` methods."; + } + + @Value + static class ThreadLocalAccumulator { + Set allThreadLocals = new HashSet<>(); + Set mutatedThreadLocals = new HashSet<>(); + + public boolean isImmutable(ThreadLocalVariable var) { + return allThreadLocals.contains(var) && !mutatedThreadLocals.contains(var); + } + } + + @Value + static class ThreadLocalVariable { + String name; + String className; + + static ThreadLocalVariable fromVariable(J.VariableDeclarations.NamedVariable variable, J.ClassDeclaration classDecl) { + String className = classDecl != null && classDecl.getType() != null ? + classDecl.getType().getFullyQualifiedName() : ""; + return new ThreadLocalVariable(variable.getName().getSimpleName(), className); + } + } + + @Override + public ThreadLocalAccumulator getInitialValue(ExecutionContext ctx) { + return new ThreadLocalAccumulator(); + } + + @Override + public TreeVisitor getScanner(ThreadLocalAccumulator acc) { + return new JavaIsoVisitor() { + @Override + public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext ctx) { + variable = super.visitVariable(variable, ctx); + + if (isThreadLocalType(variable.getType())) { + J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + ThreadLocalVariable tlVar = ThreadLocalVariable.fromVariable(variable, classDecl); + acc.allThreadLocals.add(tlVar); + + // Check if this is a local variable - we don't mark local variables + J.Block enclosingBlock = getCursor().firstEnclosing(J.Block.class); + J.MethodDeclaration enclosingMethod = getCursor().firstEnclosing(J.MethodDeclaration.class); + if (enclosingMethod != null && enclosingBlock != null && enclosingBlock != enclosingMethod.getBody()) { + // This is a local variable inside a method + acc.mutatedThreadLocals.add(tlVar); + } + } + return variable; + } + + @Override + public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ctx) { + assignment = super.visitAssignment(assignment, ctx); + + // Check if we're reassigning a ThreadLocal variable + if (assignment.getVariable() instanceof J.Identifier) { + J.Identifier id = (J.Identifier) assignment.getVariable(); + if (isThreadLocalType(id.getType())) { + J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + String className = classDecl != null && classDecl.getType() != null ? + classDecl.getType().getFullyQualifiedName() : ""; + acc.mutatedThreadLocals.add(new ThreadLocalVariable(id.getSimpleName(), className)); + } + } + return assignment; + } + + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + method = super.visitMethodInvocation(method, ctx); + + // Check for ThreadLocal.set() or ThreadLocal.remove() calls + if (THREAD_LOCAL_SET.matches(method) || THREAD_LOCAL_REMOVE.matches(method) || + INHERITABLE_THREAD_LOCAL_SET.matches(method) || INHERITABLE_THREAD_LOCAL_REMOVE.matches(method)) { + + J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + String className = classDecl != null && classDecl.getType() != null ? + classDecl.getType().getFullyQualifiedName() : ""; + + // Get the ThreadLocal instance being mutated + String varName = null; + if (method.getSelect() instanceof J.Identifier) { + varName = ((J.Identifier) method.getSelect()).getSimpleName(); + } else if (method.getSelect() instanceof J.FieldAccess) { + varName = ((J.FieldAccess) method.getSelect()).getSimpleName(); + } + + if (varName != null) { + acc.mutatedThreadLocals.add(new ThreadLocalVariable(varName, className)); + } + } + return method; + } + + @Override + public J.Unary visitUnary(J.Unary unary, ExecutionContext ctx) { + unary = super.visitUnary(unary, ctx); + + // Check for operations that would mutate the ThreadLocal reference (unlikely but thorough) + if (unary.getExpression() instanceof J.Identifier) { + J.Identifier id = (J.Identifier) unary.getExpression(); + if (isThreadLocalType(id.getType())) { + switch (unary.getOperator()) { + case PreIncrement: + case PreDecrement: + case PostIncrement: + case PostDecrement: + J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + String className = classDecl != null && classDecl.getType() != null ? + classDecl.getType().getFullyQualifiedName() : ""; + acc.mutatedThreadLocals.add(new ThreadLocalVariable(id.getSimpleName(), className)); + break; + default: + break; + } + } + } + return unary; + } + }; + } + + @Override + public TreeVisitor getVisitor(ThreadLocalAccumulator acc) { + return new JavaIsoVisitor() { + @Override + public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) { + multiVariable = super.visitVariableDeclarations(multiVariable, ctx); + + // Check if any of the variables in this declaration are immutable ThreadLocals + J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + for (J.VariableDeclarations.NamedVariable variable : multiVariable.getVariables()) { + if (isThreadLocalType(variable.getType())) { + ThreadLocalVariable tlVar = ThreadLocalVariable.fromVariable(variable, classDecl); + if (acc.isImmutable(tlVar)) { + return SearchResult.found( + multiVariable, + "ThreadLocal candidate for ScopedValue migration - never mutated after initialization" + ); + } + } + } + + return multiVariable; + } + }; + } + + private static boolean isThreadLocalType(JavaType type) { + return TypeUtils.isOfClassType(type, "java.lang.ThreadLocal") || + TypeUtils.isOfClassType(type, "java.lang.InheritableThreadLocal"); + } +} \ No newline at end of file diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java b/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java new file mode 100644 index 0000000000..b2191c1ec4 --- /dev/null +++ b/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java @@ -0,0 +1,295 @@ +/* + * Copyright 2024 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.java.migrate.search; + +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 FindImmutableThreadLocalVariablesTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new FindImmutableThreadLocalVariables()); + } + + @DocumentExample + @Test + void identifySimpleImmutableThreadLocal() { + rewriteRun( + java( + """ + class Example { + private static final ThreadLocal TL = new ThreadLocal<>(); + + public String getValue() { + return TL.get(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final ThreadLocal TL = new ThreadLocal<>(); + + public String getValue() { + return TL.get(); + } + } + """ + ) + ); + } + + @Test + void identifyThreadLocalWithInitialValue() { + rewriteRun( + java( + """ + class Example { + private static final ThreadLocal COUNTER = ThreadLocal.withInitial(() -> 0); + + public int getCount() { + return COUNTER.get(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final ThreadLocal COUNTER = ThreadLocal.withInitial(() -> 0); + + public int getCount() { + return COUNTER.get(); + } + } + """ + ) + ); + } + + @Test + void doNotMarkThreadLocalWithSetCall() { + rewriteRun( + java( + """ + class Example { + private static final ThreadLocal TL = new ThreadLocal<>(); + + public void setValue(String value) { + TL.set(value); + } + + public String getValue() { + return TL.get(); + } + } + """ + ) + ); + } + + @Test + void doNotMarkThreadLocalWithRemoveCall() { + rewriteRun( + java( + """ + class Example { + private static final ThreadLocal TL = new ThreadLocal<>(); + + public void cleanup() { + TL.remove(); + } + + public String getValue() { + return TL.get(); + } + } + """ + ) + ); + } + + @Test + void doNotMarkReassignedThreadLocal() { + rewriteRun( + java( + """ + class Example { + private static ThreadLocal tl = new ThreadLocal<>(); + + public void reset() { + tl = new ThreadLocal<>(); + } + + public String getValue() { + return tl.get(); + } + } + """ + ) + ); + } + + @Test + void handleMultipleThreadLocalsWithMixedPatterns() { + rewriteRun( + java( + """ + class Example { + private static final ThreadLocal IMMUTABLE_TL = new ThreadLocal<>(); + private static final ThreadLocal MUTABLE_TL = new ThreadLocal<>(); + private static final ThreadLocal ANOTHER_IMMUTABLE = ThreadLocal.withInitial(() -> false); + + public void updateMutable(int value) { + MUTABLE_TL.set(value); + } + + public String getImmutable() { + return IMMUTABLE_TL.get(); + } + + public Boolean getAnother() { + return ANOTHER_IMMUTABLE.get(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final ThreadLocal IMMUTABLE_TL = new ThreadLocal<>(); + private static final ThreadLocal MUTABLE_TL = new ThreadLocal<>(); + /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final ThreadLocal ANOTHER_IMMUTABLE = ThreadLocal.withInitial(() -> false); + + public void updateMutable(int value) { + MUTABLE_TL.set(value); + } + + public String getImmutable() { + return IMMUTABLE_TL.get(); + } + + public Boolean getAnother() { + return ANOTHER_IMMUTABLE.get(); + } + } + """ + ) + ); + } + + @Test + void identifyInstanceThreadLocal() { + rewriteRun( + java( + """ + class Example { + private final ThreadLocal instanceTL = new ThreadLocal<>(); + + public String getValue() { + return instanceTL.get(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private final ThreadLocal instanceTL = new ThreadLocal<>(); + + public String getValue() { + return instanceTL.get(); + } + } + """ + ) + ); + } + + @Test + void handleThreadLocalWithComplexInitialization() { + rewriteRun( + java( + """ + import java.text.SimpleDateFormat; + + class Example { + private static final ThreadLocal DATE_FORMAT = + ThreadLocal.withInitial(() -> new SimpleDateFormat("yyyy-MM-dd")); + + public String formatDate(java.util.Date date) { + return DATE_FORMAT.get().format(date); + } + } + """, + """ + import java.text.SimpleDateFormat; + + class Example { + /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final ThreadLocal DATE_FORMAT = + ThreadLocal.withInitial(() -> new SimpleDateFormat("yyyy-MM-dd")); + + public String formatDate(java.util.Date date) { + return DATE_FORMAT.get().format(date); + } + } + """ + ) + ); + } + + @Test + void doNotMarkLocalVariableThreadLocal() { + // Local ThreadLocals are unusual but should not be marked as they have different lifecycle + rewriteRun( + java( + """ + class Example { + public void method() { + ThreadLocal localTL = new ThreadLocal<>(); + localTL.set("value"); + System.out.println(localTL.get()); + } + } + """ + ) + ); + } + + @Test + void handleInheritableThreadLocal() { + rewriteRun( + java( + """ + class Example { + private static final InheritableThreadLocal ITL = new InheritableThreadLocal<>(); + + public String getValue() { + return ITL.get(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final InheritableThreadLocal ITL = new InheritableThreadLocal<>(); + + public String getValue() { + return ITL.get(); + } + } + """ + ) + ); + } +} \ No newline at end of file From 9120a12c76aefd40166cf443cbbdf832bf0a9d7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Tue, 14 Oct 2025 15:10:54 +0200 Subject: [PATCH 02/12] Acknowledge that we only check file internal ATM --- .../ConvertImmutableClassToRecord.java | 288 ++++++ .../ConvertImmutableClassToRecordTest.java | 821 ++++++++++++++++++ 2 files changed, 1109 insertions(+) create mode 100644 src/main/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecord.java create mode 100644 src/test/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecordTest.java diff --git a/src/main/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecord.java b/src/main/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecord.java new file mode 100644 index 0000000000..fefe222a22 --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecord.java @@ -0,0 +1,288 @@ +/* + * Copyright 2024 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.java.migrate; + +import lombok.EqualsAndHashCode; +import lombok.Value; +import org.jspecify.annotations.Nullable; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Option; +import org.openrewrite.Recipe; +import org.openrewrite.TreeVisitor; +import org.openrewrite.java.ChangeMethodName; +import org.openrewrite.java.JavaTemplate; +import org.openrewrite.java.JavaVisitor; +import org.openrewrite.java.tree.J; + +import java.util.*; +import java.util.stream.Collectors; + +@Value +@EqualsAndHashCode(callSuper = false) +public class ConvertImmutableClassToRecord extends Recipe { + + @Option(displayName = "Package pattern", + description = "A glob pattern to match packages where classes should be converted to records. " + + "If not specified, the recipe will be applied to all packages.", + example = "com.example.**", + required = false) + @Nullable + String packagePattern; + + @Override + public String getDisplayName() { + return "Convert immutable class to record"; + } + + @Override + public String getDescription() { + //language=Markdown + return "Converts immutable classes to Java records. This is a composite recipe that:\n" + + " 1. Identifies classes that meet record conversion criteria\n" + + " 2. Converts eligible classes to records\n" + + " 3. Updates all getter method calls to use record accessor syntax\n" + + "\n" + + " A class is eligible for conversion if it:\n" + + " - Has only private fields\n" + + " - Has corresponding getter methods for all fields\n" + + " - Has no setter methods\n" + + " - Does not extend another class\n" + + " - Is effectively immutable"; + } + + @Override + public TreeVisitor getVisitor() { + return new ConvertImmutableClassToRecordVisitor(); + } + + private class ConvertImmutableClassToRecordVisitor extends JavaVisitor { + + @Override + public J visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { + if (!isEligibleForRecordConversion(classDecl) || !matchesPackagePattern(classDecl)) { + return classDecl; + } + + // Get all getter methods that will need to be updated + List getterMethods = getGetterMethods(classDecl); + String classTypeFqn = getFullyQualifiedName(classDecl); + + // Schedule method name changes for all getters after this transformation + for (J.MethodDeclaration getter : getterMethods) { + String fieldName = getFieldNameFromGetter(getter); + if (fieldName != null) { + String oldMethodPattern = classTypeFqn + " " + getter.getSimpleName() + "()"; + doAfterVisit(new ChangeMethodName(oldMethodPattern, fieldName, true, null).getVisitor()); + } + } + + // Transform the class to a record + return transformToRecord(classDecl); + } + + private J.ClassDeclaration transformToRecord(J.ClassDeclaration classDecl) { + List fields = getPrivateFields(classDecl); + List utilityMethods = extractUtilityMethods(classDecl); + + // Create record components string + StringBuilder recordComponents = new StringBuilder(); + for (int i = 0; i < fields.size(); i++) { + J.VariableDeclarations field = fields.get(i); + for (J.VariableDeclarations.NamedVariable var : field.getVariables()) { + if (recordComponents.length() > 0) { + recordComponents.append(", "); + } + + // Include field annotations + StringBuilder fieldComponent = new StringBuilder(); + if (!field.getLeadingAnnotations().isEmpty()) { + for (J.Annotation annotation : field.getLeadingAnnotations()) { + fieldComponent.append("@").append(annotation.getAnnotationType().toString()).append(" "); + } + } + fieldComponent.append(field.getTypeExpression()).append(" ").append(var.getSimpleName()); + recordComponents.append(fieldComponent); + } + } + + // Build record template + StringBuilder recordTemplate = new StringBuilder(); + + // Add class-level annotations and modifiers + if (!classDecl.getLeadingAnnotations().isEmpty() || !classDecl.getModifiers().isEmpty()) { + for (J.Annotation annotation : classDecl.getLeadingAnnotations()) { + recordTemplate.append("@").append(annotation.getAnnotationType().toString()).append(" "); + } + for (J.Modifier modifier : classDecl.getModifiers()) { + if (modifier.getType() != J.Modifier.Type.Final) { // Records are implicitly final + recordTemplate.append(modifier.getType().toString().toLowerCase()).append(" "); + } + } + } + + recordTemplate.append("record ").append(classDecl.getSimpleName()).append("(") + .append(recordComponents).append(")"); + + if (!classDecl.getImplements().isEmpty()) { + recordTemplate.append(" implements "); + for (int i = 0; i < classDecl.getImplements().size(); i++) { + if (i > 0) recordTemplate.append(", "); + recordTemplate.append("#{any()}"); + } + } + + recordTemplate.append(" { "); + for (int i = 0; i < utilityMethods.size(); i++) { + recordTemplate.append("#{any()} "); + } + recordTemplate.append("}"); + + // Prepare template parameters + List templateParams = new ArrayList<>(); + if (!classDecl.getImplements().isEmpty()) { + for (J j : classDecl.getImplements()) { + templateParams.add(j); + } + } + for (J.MethodDeclaration method : utilityMethods) { + templateParams.add(method); + } + + JavaTemplate template = JavaTemplate.builder(recordTemplate.toString()).build(); + return template.apply(updateCursor(classDecl), classDecl.getCoordinates().replace(), templateParams.toArray()); + } + + private boolean isEligibleForRecordConversion(J.ClassDeclaration classDecl) { + // Skip if class extends another class (records can't extend classes) + if (classDecl.getExtends() != null) { + return false; + } + + // Skip if class is not a regular class + if (classDecl.getKind() != J.ClassDeclaration.Kind.Type.Class) { + return false; + } + + List fields = getPrivateFields(classDecl); + if (fields.isEmpty()) { + return false; + } + + // Check if all fields have corresponding getters + Set fieldNames = fields.stream() + .flatMap(field -> field.getVariables().stream()) + .map(J.VariableDeclarations.NamedVariable::getSimpleName) + .collect(Collectors.toSet()); + + Set getterFields = getGetterMethods(classDecl).stream() + .map(this::getFieldNameFromGetter) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); + + return fieldNames.equals(getterFields) && !hasSetterMethods(classDecl); + } + + private List extractUtilityMethods(J.ClassDeclaration classDecl) { + return classDecl.getBody().getStatements().stream() + .filter(J.MethodDeclaration.class::isInstance) + .map(J.MethodDeclaration.class::cast) + .filter(method -> !isConstructor(method) && !isGetterMethod(method) && !isSetterMethod(method)) + .collect(Collectors.toList()); + } + + private List getPrivateFields(J.ClassDeclaration classDecl) { + return classDecl.getBody().getStatements().stream() + .filter(J.VariableDeclarations.class::isInstance) + .map(J.VariableDeclarations.class::cast) + .filter(field -> field.hasModifier(J.Modifier.Type.Private)) + .collect(Collectors.toList()); + } + + private List getGetterMethods(J.ClassDeclaration classDecl) { + return classDecl.getBody().getStatements().stream() + .filter(J.MethodDeclaration.class::isInstance) + .map(J.MethodDeclaration.class::cast) + .filter(this::isGetterMethod) + .collect(Collectors.toList()); + } + + private boolean hasSetterMethods(J.ClassDeclaration classDecl) { + return classDecl.getBody().getStatements().stream() + .filter(J.MethodDeclaration.class::isInstance) + .map(J.MethodDeclaration.class::cast) + .anyMatch(this::isSetterMethod); + } + + private boolean isConstructor(J.MethodDeclaration method) { + return method.isConstructor(); + } + + private boolean isGetterMethod(J.MethodDeclaration method) { + String methodName = method.getSimpleName(); + return (methodName.startsWith("get") || methodName.startsWith("is")) + && method.getParameters().isEmpty() + && method.getReturnTypeExpression() != null; + } + + private boolean isSetterMethod(J.MethodDeclaration method) { + String methodName = method.getSimpleName(); + return methodName.startsWith("set") + && method.getParameters().size() == 1; + } + + @Nullable + private String getFieldNameFromGetter(J.MethodDeclaration getter) { + String methodName = getter.getSimpleName(); + if (methodName.startsWith("get") && methodName.length() > 3) { + return Character.toLowerCase(methodName.charAt(3)) + methodName.substring(4); + } else if (methodName.startsWith("is") && methodName.length() > 2) { + return Character.toLowerCase(methodName.charAt(2)) + methodName.substring(3); + } + return null; + } + + private boolean matchesPackagePattern(J.ClassDeclaration classDecl) { + if (packagePattern == null || packagePattern.trim().isEmpty()) { + return true; // No pattern specified, match all + } + + String packageName = getPackageName(classDecl); + if (packageName == null) { + return packagePattern.equals("*"); // Default package + } + + // Simple glob pattern matching + String pattern = packagePattern.replace("**", ".*").replace("*", "[^.]*"); + return packageName.matches(pattern); + } + + private String getPackageName(J.ClassDeclaration classDecl) { + J.CompilationUnit cu = getCursor().firstEnclosing(J.CompilationUnit.class); + if (cu != null && cu.getPackageDeclaration() != null) { + return cu.getPackageDeclaration().getExpression().toString(); + } + return null; + } + + private String getFullyQualifiedName(J.ClassDeclaration classDecl) { + String packageName = getPackageName(classDecl); + if (packageName != null) { + return packageName + "." + classDecl.getSimpleName(); + } + return classDecl.getSimpleName(); + } + } +} diff --git a/src/test/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecordTest.java b/src/test/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecordTest.java new file mode 100644 index 0000000000..169bc5bc4e --- /dev/null +++ b/src/test/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecordTest.java @@ -0,0 +1,821 @@ +/* + * Copyright 2024 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.java.migrate; + +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 ConvertImmutableClassToRecordTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new ConvertImmutableClassToRecord(null)); + } + + @DocumentExample + @Test + void simpleImmutableClass() { + rewriteRun( + //language=java + java( + """ + class Person { + private final String name; + private final int age; + + public Person(String name, int age) { + this.name = name; + this.age = age; + } + + public String getName() { + return name; + } + + public int getAge() { + return age; + } + } + """, + """ + record Person(String name, int age) { + } + """ + ) + ); + } + + @Test + void completeClassToRecordTransformationWithMethodCallUpdates() { + rewriteRun( + //language=java + java( + """ + class Person { + private final String name; + private final int age; + + public Person(String name, int age) { + this.name = name; + this.age = age; + } + + public String getName() { + return name; + } + + public int getAge() { + return age; + } + + public String getDisplayName() { + return name + " (" + age + ")"; + } + } + """, + """ + record Person(String name, int age) { + public String getDisplayName() { + return name + " (" + age + ")"; + } + } + """ + ), + //language=java + java( + """ + class PersonService { + public void processPerson(Person person) { + String name = person.getName(); + int age = person.getAge(); + String display = person.getDisplayName(); + System.out.println("Processing: " + display); + } + } + """, + """ + class PersonService { + public void processPerson(Person person) { + String name = person.name(); + int age = person.age(); + String display = person.getDisplayName(); + System.out.println("Processing: " + display); + } + } + """ + ) + ); + } + + @Test + void packagePatternRestriction() { + rewriteRun( + spec -> spec.recipe(new ConvertImmutableClassToRecord("com.example.model.**")), + //language=java + java( + """ + package com.example.model; + + class Person { + private final String name; + + public Person(String name) { + this.name = name; + } + + public String getName() { + return name; + } + } + """, + """ + package com.example.model; + + record Person(String name) { + } + """ + ), + //language=java + java( + """ + package com.example.service; + + class User { + private final String username; + + public User(String username) { + this.username = username; + } + + public String getUsername() { + return username; + } + } + """ + // Should not be transformed due to package pattern restriction + ) + ); + } + + @Test + void multipleClassesTransformation() { + rewriteRun( + //language=java + java( + """ + class Address { + private final String street; + private final String city; + + public Address(String street, String city) { + this.street = street; + this.city = city; + } + + public String getStreet() { + return street; + } + + public String getCity() { + return city; + } + } + + class Person { + private final String name; + private final Address address; + + public Person(String name, Address address) { + this.name = name; + this.address = address; + } + + public String getName() { + return name; + } + + public Address getAddress() { + return address; + } + } + """, + """ + record Address(String street, String city) { + } + + record Person(String name, Address address) { + } + """ + ), + //language=java + java( + """ + class AddressService { + public void printFullAddress(Person person) { + Address addr = person.getAddress(); + String street = addr.getStreet(); + String city = addr.getCity(); + System.out.println(street + ", " + city); + } + } + """, + """ + class AddressService { + public void printFullAddress(Person person) { + Address addr = person.address(); + String street = addr.street(); + String city = addr.city(); + System.out.println(street + ", " + city); + } + } + """ + ) + ); + } + + @Test + void complexBusinessLogicPreservation() { + rewriteRun( + //language=java + java( + """ + import java.util.Objects; + + class Product { + private final String id; + private final String name; + private final double price; + + public Product(String id, String name, double price) { + this.id = id; + this.name = name; + this.price = price; + } + + public String getId() { + return id; + } + + public String getName() { + return name; + } + + public double getPrice() { + return price; + } + + public String getFormattedPrice() { + return String.format("$%.2f", price); + } + + public boolean isExpensive() { + return price > 100.0; + } + + public Product withDiscount(double discountPercent) { + double newPrice = price * (1 - discountPercent / 100); + return new Product(id, name, newPrice); + } + } + """, + """ + import java.util.Objects; + + record Product(String id, String name, double price) { + public String getFormattedPrice() { + return String.format("$%.2f", price); + } + + public boolean isExpensive() { + return price > 100.0; + } + + public Product withDiscount(double discountPercent) { + double newPrice = price * (1 - discountPercent / 100); + return new Product(id, name, newPrice); + } + } + """ + ) + ); + } + + @Test + void nestedClassesHandling() { + rewriteRun( + //language=java + java( + """ + class OuterClass { + private String value; + + static class InnerData { + private final String data; + private final int count; + + public InnerData(String data, int count) { + this.data = data; + this.count = count; + } + + public String getData() { + return data; + } + + public int getCount() { + return count; + } + } + + public void process(InnerData inner) { + String data = inner.getData(); + int count = inner.getCount(); + System.out.println(data + ": " + count); + } + } + """, + """ + class OuterClass { + private String value; + + static record InnerData(String data, int count) { + } + + public void process(InnerData inner) { + String data = inner.data(); + int count = inner.count(); + System.out.println(data + ": " + count); + } + } + """ + ) + ); + } + + @Test + void genericTypesSupport() { + rewriteRun( + //language=java + java( + """ + import java.util.List; + import java.util.Optional; + + class Container { + private final T value; + private final List items; + + public Container(T value, List items) { + this.value = value; + this.items = items; + } + + public T getValue() { + return value; + } + + public List getItems() { + return items; + } + + public Optional findFirst() { + return items.isEmpty() ? Optional.empty() : Optional.of(items.get(0)); + } + } + """, + """ + import java.util.List; + import java.util.Optional; + + record Container(T value, List items) { + public Optional findFirst() { + return items.isEmpty() ? Optional.empty() : Optional.of(items.get(0)); + } + } + """ + ) + ); + } + + @Test + void integrationWithStreamOperations() { + rewriteRun( + //language=java + java( + """ + import java.util.List; + import java.util.stream.Collectors; + + class Employee { + private final String name; + private final String department; + private final double salary; + + public Employee(String name, String department, double salary) { + this.name = name; + this.department = department; + this.salary = salary; + } + + public String getName() { + return name; + } + + public String getDepartment() { + return department; + } + + public double getSalary() { + return salary; + } + } + + class EmployeeService { + public List getNamesByDepartment(List employees, String dept) { + return employees.stream() + .filter(emp -> emp.getDepartment().equals(dept)) + .map(Employee::getName) + .collect(Collectors.toList()); + } + + public double getTotalSalary(List employees) { + return employees.stream() + .mapToDouble(Employee::getSalary) + .sum(); + } + } + """, + """ + import java.util.List; + import java.util.stream.Collectors; + + record Employee(String name, String department, double salary) { + } + + class EmployeeService { + public List getNamesByDepartment(List employees, String dept) { + return employees.stream() + .filter(emp -> emp.department().equals(dept)) + .map(Employee::name) + .collect(Collectors.toList()); + } + + public double getTotalSalary(List employees) { + return employees.stream() + .mapToDouble(Employee::salary) + .sum(); + } + } + """ + ) + ); + } + + @Test + void immutableClassWithAnnotations() { + rewriteRun( + //language=java + java( + """ + import javax.validation.constraints.NotNull; + import javax.validation.constraints.Min; + + @Entity + public class Person { + @NotNull + private final String name; + + @Min(0) + private final int age; + + public Person(String name, int age) { + this.name = name; + this.age = age; + } + + public String getName() { + return name; + } + + public int getAge() { + return age; + } + } + """, + """ + import javax.validation.constraints.NotNull; + import javax.validation.constraints.Min; + + @Entity + public record Person(@NotNull String name, @Min(0) int age) { + } + """ + ) + ); + } + + @Test + void booleanGetterWithIsPrefix() { + rewriteRun( + //language=java + java( + """ + class User { + private final String name; + private final boolean active; + + public User(String name, boolean active) { + this.name = name; + this.active = active; + } + + public String getName() { + return name; + } + + public boolean isActive() { + return active; + } + } + """, + """ + record User(String name, boolean active) { + } + """ + ) + ); + } + + @Test + void classWithCollectionFields() { + rewriteRun( + //language=java + java( + """ + import java.util.List; + import java.util.Set; + + class Student { + private final String name; + private final List courses; + private final Set skills; + + public Student(String name, List courses, Set skills) { + this.name = name; + this.courses = courses; + this.skills = skills; + } + + public String getName() { + return name; + } + + public List getCourses() { + return courses; + } + + public Set getSkills() { + return skills; + } + } + """, + """ + import java.util.List; + import java.util.Set; + + record Student(String name, List courses, Set skills) { + } + """ + ) + ); + } + + @Test + void classWithInterfaceImplementation() { + rewriteRun( + //language=java + java( + """ + interface Identifiable { + String getId(); + } + + class Entity implements Identifiable { + private final String id; + private final String name; + + public Entity(String id, String name) { + this.id = id; + this.name = name; + } + + public String getId() { + return id; + } + + public String getName() { + return name; + } + } + """, + """ + interface Identifiable { + String getId(); + } + + record Entity(String id, String name) implements Identifiable { + } + """ + ) + ); + } + + @Test + void shouldNotConvertClassWithSetterMethods() { + rewriteRun( + //language=java + java( + """ + class MutablePerson { + private String name; + private int age; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public int getAge() { + return age; + } + + public void setAge(int age) { + this.age = age; + } + } + """ + ) + ); + } + + @Test + void shouldNotConvertClassThatExtendsAnotherClass() { + rewriteRun( + //language=java + java( + """ + class BaseEntity { + private final String id; + + public BaseEntity(String id) { + this.id = id; + } + + public String getId() { + return id; + } + } + + class Person extends BaseEntity { + private final String name; + + public Person(String id, String name) { + super(id); + this.name = name; + } + + public String getName() { + return name; + } + } + """ + ) + ); + } + + @Test + void shouldNotConvertClassWithoutGetters() { + rewriteRun( + //language=java + java( + """ + class DataHolder { + private final String data; + + public DataHolder(String data) { + this.data = data; + } + + public void processData() { + System.out.println(data); + } + } + """ + ) + ); + } + + @Test + void shouldNotConvertClassWithMissingGetters() { + rewriteRun( + //language=java + java( + """ + class PartialPerson { + private final String name; + private final int age; + + public PartialPerson(String name, int age) { + this.name = name; + this.age = age; + } + + public String getName() { + return name; + } + + // Missing getAge() method + } + """ + ) + ); + } + + @Test + void shouldNotConvertEnum() { + rewriteRun( + //language=java + java( + """ + enum Status { + ACTIVE, + INACTIVE; + + public boolean isActive() { + return this == ACTIVE; + } + } + """ + ) + ); + } + + @Test + void shouldNotConvertInterface() { + rewriteRun( + //language=java + java( + """ + interface Repository { + String getName(); + int getVersion(); + } + """ + ) + ); + } + + @Test + void shouldNotConvertAbstractClass() { + rewriteRun( + //language=java + java( + """ + abstract class AbstractEntity { + private final String id; + + public AbstractEntity(String id) { + this.id = id; + } + + public String getId() { + return id; + } + + public abstract void process(); + } + """ + ) + ); + } +} From aae284988457e8beb70e6d93d15c4ade8d2efa34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Tue, 14 Oct 2025 15:15:58 +0200 Subject: [PATCH 03/12] Acknowledge that we only check file internal ATM --- .../FindImmutableThreadLocalVariables.java | 18 +++++-- ...FindImmutableThreadLocalVariablesTest.java | 52 +++++++++++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java b/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java index 0ce9fa690d..1f84c563f7 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java +++ b/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java @@ -46,7 +46,9 @@ public String getDisplayName() { @Override public String getDescription() { return "Find `ThreadLocal` variables that are never mutated and could be candidates for migration to `ScopedValue` in Java 25+. " + - "This recipe identifies `ThreadLocal` variables that are only initialized but never reassigned or modified through `set()` or `remove()` methods."; + "This recipe identifies `ThreadLocal` variables that are only initialized but never reassigned or modified through `set()` or `remove()` methods. " + + "Note: This recipe only analyzes mutations within the same source file. ThreadLocal fields accessible from other classes " + + "(public, protected, or package-private) may be mutated elsewhere in the codebase."; } @Value @@ -184,10 +186,16 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m if (isThreadLocalType(variable.getType())) { ThreadLocalVariable tlVar = ThreadLocalVariable.fromVariable(variable, classDecl); if (acc.isImmutable(tlVar)) { - return SearchResult.found( - multiVariable, - "ThreadLocal candidate for ScopedValue migration - never mutated after initialization" - ); + // Only mark private fields as candidates, since public/protected/package-private + // fields could be mutated from other classes + boolean isPrivate = multiVariable.getModifiers().stream() + .anyMatch(mod -> mod.getType() == J.Modifier.Type.Private); + + String message = isPrivate + ? "ThreadLocal candidate for ScopedValue migration - never mutated after initialization" + : "ThreadLocal candidate for ScopedValue migration - never mutated in this file (but may be mutated elsewhere due to non-private access)"; + + return SearchResult.found(multiVariable, message); } } } diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java b/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java index b2191c1ec4..6ec5696858 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java @@ -267,6 +267,58 @@ public void method() { ); } + @Test + void warnAboutPackagePrivateThreadLocal() { + rewriteRun( + java( + """ + class Example { + static final ThreadLocal PACKAGE_TL = new ThreadLocal<>(); + + public String getValue() { + return PACKAGE_TL.get(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal candidate for ScopedValue migration - never mutated in this file (but may be mutated elsewhere due to non-private access))~~>*/static final ThreadLocal PACKAGE_TL = new ThreadLocal<>(); + + public String getValue() { + return PACKAGE_TL.get(); + } + } + """ + ) + ); + } + + @Test + void warnAboutProtectedThreadLocal() { + rewriteRun( + java( + """ + class Example { + protected static final ThreadLocal PROTECTED_TL = new ThreadLocal<>(); + + public String getValue() { + return PROTECTED_TL.get(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal candidate for ScopedValue migration - never mutated in this file (but may be mutated elsewhere due to non-private access))~~>*/protected static final ThreadLocal PROTECTED_TL = new ThreadLocal<>(); + + public String getValue() { + return PROTECTED_TL.get(); + } + } + """ + ) + ); + } + @Test void handleInheritableThreadLocal() { rewriteRun( From 92368e889ed77c0e2c45f1ab09039ccbb4772b48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Wed, 15 Oct 2025 10:33:57 +0200 Subject: [PATCH 04/12] enable scanning capabilities to detect ThreadLocals that are mutated in outher files --- .../FindImmutableThreadLocalVariables.java | 310 +++++++++---- ...ableThreadLocalVariablesCrossFileTest.java | 419 ++++++++++++++++++ ...FindImmutableThreadLocalVariablesTest.java | 4 +- 3 files changed, 652 insertions(+), 81 deletions(-) create mode 100644 src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesCrossFileTest.java diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java b/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java index 1f84c563f7..71389ebb15 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java +++ b/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java @@ -19,19 +19,21 @@ import lombok.Value; import org.openrewrite.ExecutionContext; import org.openrewrite.ScanningRecipe; +import org.openrewrite.SourceFile; import org.openrewrite.TreeVisitor; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; import org.openrewrite.java.tree.TypeUtils; import org.openrewrite.marker.SearchResult; -import java.util.HashSet; -import java.util.Set; +import java.nio.file.Path; +import java.util.*; @EqualsAndHashCode(callSuper = false) -public class FindImmutableThreadLocalVariables extends ScanningRecipe { +public class FindImmutableThreadLocalVariables extends ScanningRecipe { private static final MethodMatcher THREAD_LOCAL_SET = new MethodMatcher("java.lang.ThreadLocal set(..)"); private static final MethodMatcher THREAD_LOCAL_REMOVE = new MethodMatcher("java.lang.ThreadLocal remove()"); @@ -47,55 +49,153 @@ public String getDisplayName() { public String getDescription() { return "Find `ThreadLocal` variables that are never mutated and could be candidates for migration to `ScopedValue` in Java 25+. " + "This recipe identifies `ThreadLocal` variables that are only initialized but never reassigned or modified through `set()` or `remove()` methods. " + - "Note: This recipe only analyzes mutations within the same source file. ThreadLocal fields accessible from other classes " + - "(public, protected, or package-private) may be mutated elsewhere in the codebase."; + "The recipe analyzes mutations across all source files in the project to provide accurate results."; } @Value - static class ThreadLocalAccumulator { - Set allThreadLocals = new HashSet<>(); - Set mutatedThreadLocals = new HashSet<>(); + static class ThreadLocalUsageAccumulator { + // Map of ThreadLocal declarations: fullyQualifiedName -> declaration info + Map declarations = new HashMap<>(); + // Map of ThreadLocal mutations: fullyQualifiedName -> list of mutation locations + Map> mutations = new HashMap<>(); + // Map of ThreadLocal accesses: fullyQualifiedName -> list of access locations + Map> accesses = new HashMap<>(); - public boolean isImmutable(ThreadLocalVariable var) { - return allThreadLocals.contains(var) && !mutatedThreadLocals.contains(var); + void addDeclaration(String fullyQualifiedName, ThreadLocalDeclaration declaration) { + declarations.put(fullyQualifiedName, declaration); + } + + void addMutation(String fullyQualifiedName, MutationLocation mutation) { + mutations.computeIfAbsent(fullyQualifiedName, k -> new ArrayList<>()).add(mutation); + } + + void addAccess(String fullyQualifiedName, AccessLocation access) { + accesses.computeIfAbsent(fullyQualifiedName, k -> new ArrayList<>()).add(access); + } + + boolean hasExternalMutations(String fullyQualifiedName) { + List mutationList = mutations.get(fullyQualifiedName); + if (mutationList == null || mutationList.isEmpty()) { + return false; + } + + ThreadLocalDeclaration declaration = declarations.get(fullyQualifiedName); + if (declaration == null) { + return true; // Conservative: if we can't find the declaration, assume it can be mutated + } + + // Check if any mutation is from a different file than the declaration + for (MutationLocation mutation : mutationList) { + if (!mutation.getSourcePath().equals(declaration.getSourcePath())) { + return true; + } + } + return false; + } + + boolean hasMutations(String fullyQualifiedName) { + return mutations.containsKey(fullyQualifiedName) && !mutations.get(fullyQualifiedName).isEmpty(); } } @Value - static class ThreadLocalVariable { - String name; + static class ThreadLocalDeclaration { + String fullyQualifiedName; // e.g., "com.example.MyClass.MY_THREAD_LOCAL" String className; + String fieldName; + boolean isPrivate; + boolean isStatic; + boolean isFinal; + Path sourcePath; + } - static ThreadLocalVariable fromVariable(J.VariableDeclarations.NamedVariable variable, J.ClassDeclaration classDecl) { - String className = classDecl != null && classDecl.getType() != null ? - classDecl.getType().getFullyQualifiedName() : ""; - return new ThreadLocalVariable(variable.getName().getSimpleName(), className); + @Value + static class MutationLocation { + Path sourcePath; + String className; + String methodName; + MutationType type; + + enum MutationType { + REASSIGNMENT, + SET_CALL, + REMOVE_CALL, + OTHER + } + } + + @Value + static class AccessLocation { + Path sourcePath; + String className; + String methodName; + AccessType type; + + enum AccessType { + GET_CALL, + FIELD_READ, + OTHER } } @Override - public ThreadLocalAccumulator getInitialValue(ExecutionContext ctx) { - return new ThreadLocalAccumulator(); + public ThreadLocalUsageAccumulator getInitialValue(ExecutionContext ctx) { + return new ThreadLocalUsageAccumulator(); } @Override - public TreeVisitor getScanner(ThreadLocalAccumulator acc) { + public TreeVisitor getScanner(ThreadLocalUsageAccumulator acc) { return new JavaIsoVisitor() { + @Override + public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { + // Store the current compilation unit's path for tracking locations + getCursor().putMessage("currentSourcePath", cu.getSourcePath()); + return super.visitCompilationUnit(cu, ctx); + } + @Override public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext ctx) { variable = super.visitVariable(variable, ctx); if (isThreadLocalType(variable.getType())) { + // Check if this is a field (not a local variable) + // A field is declared directly in a class, not inside a method + J.MethodDeclaration enclosingMethod = getCursor().firstEnclosing(J.MethodDeclaration.class); J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); - ThreadLocalVariable tlVar = ThreadLocalVariable.fromVariable(variable, classDecl); - acc.allThreadLocals.add(tlVar); - // Check if this is a local variable - we don't mark local variables - J.Block enclosingBlock = getCursor().firstEnclosing(J.Block.class); - J.MethodDeclaration enclosingMethod = getCursor().firstEnclosing(J.MethodDeclaration.class); - if (enclosingMethod != null && enclosingBlock != null && enclosingBlock != enclosingMethod.getBody()) { - // This is a local variable inside a method - acc.mutatedThreadLocals.add(tlVar); + // It's a field if there's no enclosing method, or if the variable is declared + // directly in the class body (not inside a method body) + boolean isField = classDecl != null && (enclosingMethod == null || + getCursor().getPathAsStream() + .filter(J.Block.class::isInstance) + .map(J.Block.class::cast) + .noneMatch(block -> block == enclosingMethod.getBody())); + + if (isField) { + J.VariableDeclarations variableDecls = getCursor().firstEnclosing(J.VariableDeclarations.class); + + if (classDecl != null && variableDecls != null) { + String className = classDecl.getType() != null ? + classDecl.getType().getFullyQualifiedName() : "UnknownClass"; + String fieldName = variable.getName().getSimpleName(); + String fullyQualifiedName = className + "." + fieldName; + + boolean isPrivate = variableDecls.getModifiers().stream() + .anyMatch(mod -> mod.getType() == J.Modifier.Type.Private); + boolean isStatic = variableDecls.getModifiers().stream() + .anyMatch(mod -> mod.getType() == J.Modifier.Type.Static); + boolean isFinal = variableDecls.getModifiers().stream() + .anyMatch(mod -> mod.getType() == J.Modifier.Type.Final); + + Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); + + ThreadLocalDeclaration declaration = new ThreadLocalDeclaration( + fullyQualifiedName, className, fieldName, + isPrivate, isStatic, isFinal, sourcePath + ); + + acc.addDeclaration(fullyQualifiedName, declaration); + } } } return variable; @@ -105,14 +205,22 @@ public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ctx) { assignment = super.visitAssignment(assignment, ctx); - // Check if we're reassigning a ThreadLocal variable - if (assignment.getVariable() instanceof J.Identifier) { - J.Identifier id = (J.Identifier) assignment.getVariable(); - if (isThreadLocalType(id.getType())) { + // Check if we're reassigning a ThreadLocal field + if (isThreadLocalFieldAccess(assignment.getVariable())) { + String fullyQualifiedName = getFieldFullyQualifiedName(assignment.getVariable()); + if (fullyQualifiedName != null) { + Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + J.MethodDeclaration methodDecl = getCursor().firstEnclosing(J.MethodDeclaration.class); + String className = classDecl != null && classDecl.getType() != null ? - classDecl.getType().getFullyQualifiedName() : ""; - acc.mutatedThreadLocals.add(new ThreadLocalVariable(id.getSimpleName(), className)); + classDecl.getType().getFullyQualifiedName() : "UnknownClass"; + String methodName = methodDecl != null ? methodDecl.getSimpleName() : "UnknownMethod"; + + MutationLocation mutation = new MutationLocation( + sourcePath, className, methodName, MutationLocation.MutationType.REASSIGNMENT + ); + acc.addMutation(fullyQualifiedName, mutation); } } return assignment; @@ -126,55 +234,80 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu if (THREAD_LOCAL_SET.matches(method) || THREAD_LOCAL_REMOVE.matches(method) || INHERITABLE_THREAD_LOCAL_SET.matches(method) || INHERITABLE_THREAD_LOCAL_REMOVE.matches(method)) { - J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); - String className = classDecl != null && classDecl.getType() != null ? - classDecl.getType().getFullyQualifiedName() : ""; - - // Get the ThreadLocal instance being mutated - String varName = null; - if (method.getSelect() instanceof J.Identifier) { - varName = ((J.Identifier) method.getSelect()).getSimpleName(); - } else if (method.getSelect() instanceof J.FieldAccess) { - varName = ((J.FieldAccess) method.getSelect()).getSimpleName(); + String fullyQualifiedName = getFieldFullyQualifiedName(method.getSelect()); + if (fullyQualifiedName != null) { + Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); + J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + J.MethodDeclaration methodDecl = getCursor().firstEnclosing(J.MethodDeclaration.class); + + String className = classDecl != null && classDecl.getType() != null ? + classDecl.getType().getFullyQualifiedName() : "UnknownClass"; + String methodName = methodDecl != null ? methodDecl.getSimpleName() : "UnknownMethod"; + + MutationLocation.MutationType mutationType = + (THREAD_LOCAL_SET.matches(method) || INHERITABLE_THREAD_LOCAL_SET.matches(method)) ? + MutationLocation.MutationType.SET_CALL : MutationLocation.MutationType.REMOVE_CALL; + + MutationLocation mutation = new MutationLocation( + sourcePath, className, methodName, mutationType + ); + acc.addMutation(fullyQualifiedName, mutation); } + } else if (method.getSimpleName().equals("get") && isThreadLocalType(method.getType())) { + // Track get() calls for completeness + String fullyQualifiedName = getFieldFullyQualifiedName(method.getSelect()); + if (fullyQualifiedName != null) { + Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); + J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + J.MethodDeclaration methodDecl = getCursor().firstEnclosing(J.MethodDeclaration.class); - if (varName != null) { - acc.mutatedThreadLocals.add(new ThreadLocalVariable(varName, className)); + String className = classDecl != null && classDecl.getType() != null ? + classDecl.getType().getFullyQualifiedName() : "UnknownClass"; + String methodName = methodDecl != null ? methodDecl.getSimpleName() : "UnknownMethod"; + + AccessLocation access = new AccessLocation( + sourcePath, className, methodName, AccessLocation.AccessType.GET_CALL + ); + acc.addAccess(fullyQualifiedName, access); } } return method; } - @Override - public J.Unary visitUnary(J.Unary unary, ExecutionContext ctx) { - unary = super.visitUnary(unary, ctx); - - // Check for operations that would mutate the ThreadLocal reference (unlikely but thorough) - if (unary.getExpression() instanceof J.Identifier) { - J.Identifier id = (J.Identifier) unary.getExpression(); - if (isThreadLocalType(id.getType())) { - switch (unary.getOperator()) { - case PreIncrement: - case PreDecrement: - case PostIncrement: - case PostDecrement: - J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); - String className = classDecl != null && classDecl.getType() != null ? - classDecl.getType().getFullyQualifiedName() : ""; - acc.mutatedThreadLocals.add(new ThreadLocalVariable(id.getSimpleName(), className)); - break; - default: - break; - } + private boolean isThreadLocalFieldAccess(Expression expression) { + if (expression instanceof J.Identifier) { + J.Identifier id = (J.Identifier) expression; + return isThreadLocalType(id.getType()); + } else if (expression instanceof J.FieldAccess) { + J.FieldAccess fieldAccess = (J.FieldAccess) expression; + return isThreadLocalType(fieldAccess.getType()); + } + return false; + } + + private String getFieldFullyQualifiedName(Expression expression) { + if (expression instanceof J.Identifier) { + J.Identifier id = (J.Identifier) expression; + JavaType.Variable varType = id.getFieldType(); + if (varType != null && varType.getOwner() instanceof JavaType.FullyQualified) { + JavaType.FullyQualified owner = (JavaType.FullyQualified) varType.getOwner(); + return owner.getFullyQualifiedName() + "." + varType.getName(); + } + } else if (expression instanceof J.FieldAccess) { + J.FieldAccess fieldAccess = (J.FieldAccess) expression; + JavaType.Variable varType = fieldAccess.getName().getFieldType(); + if (varType != null && varType.getOwner() instanceof JavaType.FullyQualified) { + JavaType.FullyQualified owner = (JavaType.FullyQualified) varType.getOwner(); + return owner.getFullyQualifiedName() + "." + varType.getName(); } } - return unary; + return null; } }; } @Override - public TreeVisitor getVisitor(ThreadLocalAccumulator acc) { + public TreeVisitor getVisitor(ThreadLocalUsageAccumulator acc) { return new JavaIsoVisitor() { @Override public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) { @@ -182,20 +315,39 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m // Check if any of the variables in this declaration are immutable ThreadLocals J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + for (J.VariableDeclarations.NamedVariable variable : multiVariable.getVariables()) { - if (isThreadLocalType(variable.getType())) { - ThreadLocalVariable tlVar = ThreadLocalVariable.fromVariable(variable, classDecl); - if (acc.isImmutable(tlVar)) { - // Only mark private fields as candidates, since public/protected/package-private - // fields could be mutated from other classes - boolean isPrivate = multiVariable.getModifiers().stream() - .anyMatch(mod -> mod.getType() == J.Modifier.Type.Private); + if (isThreadLocalType(variable.getType()) && classDecl != null) { + String className = classDecl.getType() != null ? + classDecl.getType().getFullyQualifiedName() : "UnknownClass"; + String fieldName = variable.getName().getSimpleName(); + String fullyQualifiedName = className + "." + fieldName; + + ThreadLocalDeclaration declaration = acc.declarations.get(fullyQualifiedName); + if (declaration != null) { + boolean hasMutations = acc.hasMutations(fullyQualifiedName); + boolean hasExternalMutations = acc.hasExternalMutations(fullyQualifiedName); - String message = isPrivate - ? "ThreadLocal candidate for ScopedValue migration - never mutated after initialization" - : "ThreadLocal candidate for ScopedValue migration - never mutated in this file (but may be mutated elsewhere due to non-private access)"; + if (!hasMutations) { + // No mutations at all + String message = declaration.isPrivate() ? + "ThreadLocal candidate for ScopedValue migration - never mutated after initialization" : + "ThreadLocal candidate for ScopedValue migration - never mutated in project (but accessible from outside due to non-private access)"; + return SearchResult.found(multiVariable, message); + } else if (!hasExternalMutations && declaration.isPrivate()) { + // Has mutations, but only in the declaring file and it's private + // This is still safe since no external access is possible + // However, we should check if mutations are only in the initializer + List mutations = acc.mutations.get(fullyQualifiedName); + boolean onlyLocalMutations = mutations.stream() + .allMatch(m -> m.getSourcePath().equals(declaration.getSourcePath())); - return SearchResult.found(multiVariable, message); + if (onlyLocalMutations) { + // Check if it's only mutated in the same class + // For now, we'll be conservative and not mark it + continue; + } + } } } } diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesCrossFileTest.java b/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesCrossFileTest.java new file mode 100644 index 0000000000..61dec9d617 --- /dev/null +++ b/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesCrossFileTest.java @@ -0,0 +1,419 @@ +/* + * Copyright 2024 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.java.migrate.search; + +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 FindImmutableThreadLocalVariablesCrossFileTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new FindImmutableThreadLocalVariables()); + } + + @DocumentExample + @Test + void detectMutationFromAnotherClassInSamePackage() { + rewriteRun( + // First class with package-private ThreadLocal + java( + """ + package com.example; + + class ThreadLocalHolder { + static final ThreadLocal SHARED_TL = new ThreadLocal<>(); + + public String getValue() { + return SHARED_TL.get(); + } + } + """ + ), + // Second class that mutates the ThreadLocal + java( + """ + package com.example; + + class ThreadLocalMutator { + public void mutate() { + ThreadLocalHolder.SHARED_TL.set("mutated"); + } + + public void cleanup() { + ThreadLocalHolder.SHARED_TL.remove(); + } + } + """ + ) + ); + // The ThreadLocal should NOT be marked as immutable because it's mutated in ThreadLocalMutator + } + + @Test + void detectNoMutationAcrossMultipleClasses() { + rewriteRun( + java( + """ + package com.example; + + public class ReadOnlyHolder { + public static final ThreadLocal COUNTER = ThreadLocal.withInitial(() -> 0); + + public int getCount() { + return COUNTER.get(); + } + } + """, + """ + package com.example; + + public class ReadOnlyHolder { + /*~~(ThreadLocal candidate for ScopedValue migration - never mutated in project (but accessible from outside due to non-private access))~~>*/public static final ThreadLocal COUNTER = ThreadLocal.withInitial(() -> 0); + + public int getCount() { + return COUNTER.get(); + } + } + """ + ), + java( + """ + package com.example; + + class Reader1 { + public void readValue() { + Integer value = ReadOnlyHolder.COUNTER.get(); + System.out.println(value); + } + } + """ + ), + java( + """ + package com.example; + + class Reader2 { + public int calculate() { + return ReadOnlyHolder.COUNTER.get() + 10; + } + } + """ + ) + ); + // The ThreadLocal should be marked with a warning since it's public but never mutated + } + + @Test + void detectMutationThroughInheritance() { + rewriteRun( + java( + """ + package com.example; + + public class BaseClass { + protected static final ThreadLocal PROTECTED_TL = new ThreadLocal<>(); + + public String getValue() { + return PROTECTED_TL.get(); + } + } + """ + ), + java( + """ + package com.example.sub; + + import com.example.BaseClass; + + public class SubClass extends BaseClass { + public void modifyThreadLocal() { + PROTECTED_TL.set("modified by subclass"); + } + } + """ + ) + ); + // The ThreadLocal should NOT be marked as immutable because it's mutated in SubClass + } + + @Test + void privateThreadLocalNotAccessibleFromOtherClass() { + rewriteRun( + java( + """ + package com.example; + + public class PrivateHolder { + private static final ThreadLocal PRIVATE_TL = new ThreadLocal<>(); + + public String getValue() { + return PRIVATE_TL.get(); + } + + public static class NestedClass { + public void tryToAccess() { + // Can access private field from nested class + String value = PRIVATE_TL.get(); + } + } + } + """, + """ + package com.example; + + public class PrivateHolder { + /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final ThreadLocal PRIVATE_TL = new ThreadLocal<>(); + + public String getValue() { + return PRIVATE_TL.get(); + } + + public static class NestedClass { + public void tryToAccess() { + // Can access private field from nested class + String value = PRIVATE_TL.get(); + } + } + } + """ + ), + java( + """ + package com.example; + + class ExternalClass { + public void cannotAccess() { + // Cannot access private ThreadLocal from PrivateHolder + // This class cannot mutate PRIVATE_TL + } + } + """ + ) + ); + // Private ThreadLocal should be marked as immutable since it can't be mutated externally + } + + @Test + void detectMutationInNestedClass() { + rewriteRun( + java( + """ + package com.example; + + public class OuterClass { + private static final ThreadLocal TL = new ThreadLocal<>(); + + public String getValue() { + return TL.get(); + } + + public static class InnerClass { + public void mutate() { + TL.set("mutated by inner class"); + } + } + } + """ + ) + ); + // ThreadLocal should NOT be marked as immutable because it's mutated in InnerClass + } + + @Test + void detectMutationThroughStaticImport() { + rewriteRun( + java( + """ + package com.example; + + public class ThreadLocalProvider { + public static final ThreadLocal STATIC_TL = new ThreadLocal<>(); + } + """ + ), + java( + """ + package com.example.user; + + import static com.example.ThreadLocalProvider.STATIC_TL; + + public class StaticImportUser { + public void mutate() { + STATIC_TL.set("mutated through static import"); + } + } + """ + ) + ); + // ThreadLocal should NOT be marked as immutable due to mutation through static import + } + + @Test + void multipleThreadLocalsWithMixedAccessPatterns() { + rewriteRun( + java( + """ + package com.example; + + public class MultipleThreadLocals { + private static final ThreadLocal PRIVATE_IMMUTABLE = new ThreadLocal<>(); + static final ThreadLocal PACKAGE_MUTATED = new ThreadLocal<>(); + public static final ThreadLocal PUBLIC_READ_ONLY = new ThreadLocal<>(); + protected static final ThreadLocal PROTECTED_MUTATED = new ThreadLocal<>(); + + public void readAll() { + String p1 = PRIVATE_IMMUTABLE.get(); + String p2 = PACKAGE_MUTATED.get(); + String p3 = PUBLIC_READ_ONLY.get(); + String p4 = PROTECTED_MUTATED.get(); + } + } + """, + """ + package com.example; + + public class MultipleThreadLocals { + /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final ThreadLocal PRIVATE_IMMUTABLE = new ThreadLocal<>(); + static final ThreadLocal PACKAGE_MUTATED = new ThreadLocal<>(); + /*~~(ThreadLocal candidate for ScopedValue migration - never mutated in project (but accessible from outside due to non-private access))~~>*/public static final ThreadLocal PUBLIC_READ_ONLY = new ThreadLocal<>(); + protected static final ThreadLocal PROTECTED_MUTATED = new ThreadLocal<>(); + + public void readAll() { + String p1 = PRIVATE_IMMUTABLE.get(); + String p2 = PACKAGE_MUTATED.get(); + String p3 = PUBLIC_READ_ONLY.get(); + String p4 = PROTECTED_MUTATED.get(); + } + } + """ + ), + java( + """ + package com.example; + + class Mutator { + public void mutate() { + MultipleThreadLocals.PACKAGE_MUTATED.set("mutated"); + MultipleThreadLocals.PROTECTED_MUTATED.remove(); + } + + public void readOnly() { + String value = MultipleThreadLocals.PUBLIC_READ_ONLY.get(); + } + } + """ + ) + ); + } + + @Test + void detectMutationThroughMethodReference() { + rewriteRun( + java( + """ + package com.example; + + import java.util.function.Consumer; + + public class MethodReferenceExample { + public static final ThreadLocal TL = new ThreadLocal<>(); + + public static void setValue(String value) { + TL.set(value); + } + + public void useMethodReference() { + Consumer setter = MethodReferenceExample::setValue; + setter.accept("value"); + } + } + """ + ) + ); + // ThreadLocal is mutated through setValue method, should NOT be marked as immutable + } + + @Test + void detectIndirectMutationThroughPublicSetter() { + rewriteRun( + java( + """ + package com.example; + + public class IndirectMutation { + private static final ThreadLocal PRIVATE_TL = new ThreadLocal<>(); + + public static void setThreadLocalValue(String value) { + PRIVATE_TL.set(value); + } + + public static String getThreadLocalValue() { + return PRIVATE_TL.get(); + } + } + """ + ), + java( + """ + package com.example; + + class ExternalSetter { + public void mutateIndirectly() { + IndirectMutation.setThreadLocalValue("mutated"); + } + } + """ + ) + ); + // ThreadLocal should NOT be marked as immutable because setThreadLocalValue mutates it + } + + @Test + void instanceThreadLocalWithCrossFileAccess() { + rewriteRun( + java( + """ + package com.example; + + public class InstanceThreadLocalHolder { + public final ThreadLocal instanceTL = new ThreadLocal<>(); + + public String getValue() { + return instanceTL.get(); + } + } + """ + ), + java( + """ + package com.example; + + class InstanceMutator { + public void mutate(InstanceThreadLocalHolder holder) { + holder.instanceTL.set("mutated"); + } + } + """ + ) + ); + // Instance ThreadLocal should NOT be marked as immutable due to external mutation + } +} \ No newline at end of file diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java b/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java index 6ec5696858..fc08a334b3 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java @@ -282,7 +282,7 @@ public String getValue() { """, """ class Example { - /*~~(ThreadLocal candidate for ScopedValue migration - never mutated in this file (but may be mutated elsewhere due to non-private access))~~>*/static final ThreadLocal PACKAGE_TL = new ThreadLocal<>(); + /*~~(ThreadLocal candidate for ScopedValue migration - never mutated in project (but accessible from outside due to non-private access))~~>*/static final ThreadLocal PACKAGE_TL = new ThreadLocal<>(); public String getValue() { return PACKAGE_TL.get(); @@ -308,7 +308,7 @@ public String getValue() { """, """ class Example { - /*~~(ThreadLocal candidate for ScopedValue migration - never mutated in this file (but may be mutated elsewhere due to non-private access))~~>*/protected static final ThreadLocal PROTECTED_TL = new ThreadLocal<>(); + /*~~(ThreadLocal candidate for ScopedValue migration - never mutated in project (but accessible from outside due to non-private access))~~>*/protected static final ThreadLocal PROTECTED_TL = new ThreadLocal<>(); public String getValue() { return PROTECTED_TL.get(); From 347870f1dd45f0d311b8ef181f58c245bf1c1252 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Wed, 15 Oct 2025 11:01:03 +0200 Subject: [PATCH 05/12] add preconditions --- .../ConvertImmutableClassToRecord.java | 7 +- .../FindImmutableThreadLocalVariables.java | 382 +++++++++--------- ...ableThreadLocalVariablesCrossFileTest.java | 2 +- ...FindImmutableThreadLocalVariablesTest.java | 2 +- 4 files changed, 187 insertions(+), 206 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecord.java b/src/main/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecord.java index fefe222a22..aa7c2496ed 100644 --- a/src/main/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecord.java +++ b/src/main/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecord.java @@ -139,7 +139,9 @@ private J.ClassDeclaration transformToRecord(J.ClassDeclaration classDecl) { if (!classDecl.getImplements().isEmpty()) { recordTemplate.append(" implements "); for (int i = 0; i < classDecl.getImplements().size(); i++) { - if (i > 0) recordTemplate.append(", "); + if (i > 0) { + recordTemplate.append( ", " ); + } recordTemplate.append("#{any()}"); } } @@ -243,8 +245,7 @@ private boolean isSetterMethod(J.MethodDeclaration method) { && method.getParameters().size() == 1; } - @Nullable - private String getFieldNameFromGetter(J.MethodDeclaration getter) { + private @Nullable String getFieldNameFromGetter(J.MethodDeclaration getter) { String methodName = getter.getSimpleName(); if (methodName.startsWith("get") && methodName.length() > 3) { return Character.toLowerCase(methodName.charAt(3)) + methodName.substring(4); diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java b/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java index 71389ebb15..2adbfcfed5 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java +++ b/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java @@ -23,6 +23,8 @@ import org.openrewrite.TreeVisitor; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.search.UsesJavaVersion; +import org.openrewrite.java.search.UsesType; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; import org.openrewrite.java.tree.JavaType; @@ -30,15 +32,22 @@ import org.openrewrite.marker.SearchResult; import java.nio.file.Path; -import java.util.*; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.openrewrite.Preconditions.*; @EqualsAndHashCode(callSuper = false) public class FindImmutableThreadLocalVariables extends ScanningRecipe { - private static final MethodMatcher THREAD_LOCAL_SET = new MethodMatcher("java.lang.ThreadLocal set(..)"); - private static final MethodMatcher THREAD_LOCAL_REMOVE = new MethodMatcher("java.lang.ThreadLocal remove()"); - private static final MethodMatcher INHERITABLE_THREAD_LOCAL_SET = new MethodMatcher("java.lang.InheritableThreadLocal set(..)"); - private static final MethodMatcher INHERITABLE_THREAD_LOCAL_REMOVE = new MethodMatcher("java.lang.InheritableThreadLocal remove()"); + private static final String THREAD_LOCAL_FQN = "java.lang.ThreadLocal"; + private static final String INHERITED_THREAD_LOCAL_FQN = "java.lang.InheritableThreadLocal"; + private static final MethodMatcher THREAD_LOCAL_SET = new MethodMatcher(THREAD_LOCAL_FQN + " set(..)"); + private static final MethodMatcher THREAD_LOCAL_REMOVE = new MethodMatcher(THREAD_LOCAL_FQN + " remove()"); + private static final MethodMatcher INHERITABLE_THREAD_LOCAL_SET = new MethodMatcher(INHERITED_THREAD_LOCAL_FQN + " set(..)"); + private static final MethodMatcher INHERITABLE_THREAD_LOCAL_REMOVE = new MethodMatcher(INHERITED_THREAD_LOCAL_FQN + " remove()"); @Override public String getDisplayName() { @@ -47,9 +56,7 @@ public String getDisplayName() { @Override public String getDescription() { - return "Find `ThreadLocal` variables that are never mutated and could be candidates for migration to `ScopedValue` in Java 25+. " + - "This recipe identifies `ThreadLocal` variables that are only initialized but never reassigned or modified through `set()` or `remove()` methods. " + - "The recipe analyzes mutations across all source files in the project to provide accurate results."; + return "Find `ThreadLocal` variables that are never mutated and could be candidates for migration to `ScopedValue` in Java 25+. " + "This recipe identifies `ThreadLocal` variables that are only initialized but never reassigned or modified through `set()` or `remove()` methods. " + "The recipe analyzes mutations across all source files in the project to provide accurate results."; } @Value @@ -117,10 +124,7 @@ static class MutationLocation { MutationType type; enum MutationType { - REASSIGNMENT, - SET_CALL, - REMOVE_CALL, - OTHER + REASSIGNMENT, SET_CALL, REMOVE_CALL, OTHER } } @@ -132,9 +136,7 @@ static class AccessLocation { AccessType type; enum AccessType { - GET_CALL, - FIELD_READ, - OTHER + GET_CALL, FIELD_READ, OTHER } } @@ -145,220 +147,198 @@ public ThreadLocalUsageAccumulator getInitialValue(ExecutionContext ctx) { @Override public TreeVisitor getScanner(ThreadLocalUsageAccumulator acc) { - return new JavaIsoVisitor() { - @Override - public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { - // Store the current compilation unit's path for tracking locations - getCursor().putMessage("currentSourcePath", cu.getSourcePath()); - return super.visitCompilationUnit(cu, ctx); - } + return check( + and(new UsesJavaVersion<>(25), + or(new UsesType<>(THREAD_LOCAL_FQN, true), new UsesType<>(INHERITED_THREAD_LOCAL_FQN, true))), + new JavaIsoVisitor() { + @Override + public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { + // Store the current compilation unit's path for tracking locations + getCursor().putMessage("currentSourcePath", cu.getSourcePath()); + return super.visitCompilationUnit(cu, ctx); + } + + @Override + public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext ctx) { + variable = super.visitVariable(variable, ctx); + + if (isThreadLocalType(variable.getType())) { + // Check if this is a field (not a local variable) + // A field is declared directly in a class, not inside a method + J.MethodDeclaration enclosingMethod = getCursor().firstEnclosing(J.MethodDeclaration.class); + J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + + // It's a field if there's no enclosing method, or if the variable is declared + // directly in the class body (not inside a method body) + boolean isField = classDecl != null && (enclosingMethod == null || getCursor().getPathAsStream().filter(J.Block.class::isInstance).map(J.Block.class::cast).noneMatch(block -> block == enclosingMethod.getBody())); - @Override - public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext ctx) { - variable = super.visitVariable(variable, ctx); - - if (isThreadLocalType(variable.getType())) { - // Check if this is a field (not a local variable) - // A field is declared directly in a class, not inside a method - J.MethodDeclaration enclosingMethod = getCursor().firstEnclosing(J.MethodDeclaration.class); - J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); - - // It's a field if there's no enclosing method, or if the variable is declared - // directly in the class body (not inside a method body) - boolean isField = classDecl != null && (enclosingMethod == null || - getCursor().getPathAsStream() - .filter(J.Block.class::isInstance) - .map(J.Block.class::cast) - .noneMatch(block -> block == enclosingMethod.getBody())); - - if (isField) { - J.VariableDeclarations variableDecls = getCursor().firstEnclosing(J.VariableDeclarations.class); - - if (classDecl != null && variableDecls != null) { - String className = classDecl.getType() != null ? - classDecl.getType().getFullyQualifiedName() : "UnknownClass"; - String fieldName = variable.getName().getSimpleName(); - String fullyQualifiedName = className + "." + fieldName; - - boolean isPrivate = variableDecls.getModifiers().stream() - .anyMatch(mod -> mod.getType() == J.Modifier.Type.Private); - boolean isStatic = variableDecls.getModifiers().stream() - .anyMatch(mod -> mod.getType() == J.Modifier.Type.Static); - boolean isFinal = variableDecls.getModifiers().stream() - .anyMatch(mod -> mod.getType() == J.Modifier.Type.Final); - - Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); - - ThreadLocalDeclaration declaration = new ThreadLocalDeclaration( - fullyQualifiedName, className, fieldName, - isPrivate, isStatic, isFinal, sourcePath - ); - - acc.addDeclaration(fullyQualifiedName, declaration); + if (isField) { + J.VariableDeclarations variableDecls = getCursor().firstEnclosing(J.VariableDeclarations.class); + + if (classDecl != null && variableDecls != null) { + String className = classDecl.getType() != null ? classDecl.getType().getFullyQualifiedName() : "UnknownClass"; + String fieldName = variable.getName().getSimpleName(); + String fullyQualifiedName = className + "." + fieldName; + + boolean isPrivate = variableDecls.getModifiers().stream().anyMatch(mod -> mod.getType() == J.Modifier.Type.Private); + boolean isStatic = variableDecls.getModifiers().stream().anyMatch(mod -> mod.getType() == J.Modifier.Type.Static); + boolean isFinal = variableDecls.getModifiers().stream().anyMatch(mod -> mod.getType() == J.Modifier.Type.Final); + + Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); + + ThreadLocalDeclaration declaration = new ThreadLocalDeclaration(fullyQualifiedName, className, fieldName, isPrivate, isStatic, isFinal, sourcePath); + + acc.addDeclaration(fullyQualifiedName, declaration); + } + } } + return variable; } - } - return variable; - } - @Override - public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ctx) { - assignment = super.visitAssignment(assignment, ctx); + @Override + public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ctx) { + assignment = super.visitAssignment(assignment, ctx); - // Check if we're reassigning a ThreadLocal field - if (isThreadLocalFieldAccess(assignment.getVariable())) { - String fullyQualifiedName = getFieldFullyQualifiedName(assignment.getVariable()); - if (fullyQualifiedName != null) { - Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); - J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); - J.MethodDeclaration methodDecl = getCursor().firstEnclosing(J.MethodDeclaration.class); + // Check if we're reassigning a ThreadLocal field + if (isThreadLocalFieldAccess(assignment.getVariable())) { + String fullyQualifiedName = getFieldFullyQualifiedName(assignment.getVariable()); + if (fullyQualifiedName != null) { + Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); + J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + J.MethodDeclaration methodDecl = getCursor().firstEnclosing(J.MethodDeclaration.class); - String className = classDecl != null && classDecl.getType() != null ? - classDecl.getType().getFullyQualifiedName() : "UnknownClass"; - String methodName = methodDecl != null ? methodDecl.getSimpleName() : "UnknownMethod"; + String className = classDecl != null && classDecl.getType() != null ? classDecl.getType().getFullyQualifiedName() : "UnknownClass"; + String methodName = methodDecl != null ? methodDecl.getSimpleName() : "UnknownMethod"; - MutationLocation mutation = new MutationLocation( - sourcePath, className, methodName, MutationLocation.MutationType.REASSIGNMENT - ); - acc.addMutation(fullyQualifiedName, mutation); + MutationLocation mutation = new MutationLocation(sourcePath, className, methodName, MutationLocation.MutationType.REASSIGNMENT); + acc.addMutation(fullyQualifiedName, mutation); + } + } + return assignment; } - } - return assignment; - } - @Override - public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { - method = super.visitMethodInvocation(method, ctx); + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + method = super.visitMethodInvocation(method, ctx); - // Check for ThreadLocal.set() or ThreadLocal.remove() calls - if (THREAD_LOCAL_SET.matches(method) || THREAD_LOCAL_REMOVE.matches(method) || - INHERITABLE_THREAD_LOCAL_SET.matches(method) || INHERITABLE_THREAD_LOCAL_REMOVE.matches(method)) { + // Check for ThreadLocal.set() or ThreadLocal.remove() calls + if (THREAD_LOCAL_SET.matches(method) || THREAD_LOCAL_REMOVE.matches(method) || INHERITABLE_THREAD_LOCAL_SET.matches(method) || INHERITABLE_THREAD_LOCAL_REMOVE.matches(method)) { - String fullyQualifiedName = getFieldFullyQualifiedName(method.getSelect()); - if (fullyQualifiedName != null) { - Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); - J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); - J.MethodDeclaration methodDecl = getCursor().firstEnclosing(J.MethodDeclaration.class); + String fullyQualifiedName = getFieldFullyQualifiedName(method.getSelect()); + if (fullyQualifiedName != null) { + Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); + J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + J.MethodDeclaration methodDecl = getCursor().firstEnclosing(J.MethodDeclaration.class); - String className = classDecl != null && classDecl.getType() != null ? - classDecl.getType().getFullyQualifiedName() : "UnknownClass"; - String methodName = methodDecl != null ? methodDecl.getSimpleName() : "UnknownMethod"; + String className = classDecl != null && classDecl.getType() != null ? classDecl.getType().getFullyQualifiedName() : "UnknownClass"; + String methodName = methodDecl != null ? methodDecl.getSimpleName() : "UnknownMethod"; - MutationLocation.MutationType mutationType = - (THREAD_LOCAL_SET.matches(method) || INHERITABLE_THREAD_LOCAL_SET.matches(method)) ? - MutationLocation.MutationType.SET_CALL : MutationLocation.MutationType.REMOVE_CALL; + MutationLocation.MutationType mutationType = (THREAD_LOCAL_SET.matches(method) || INHERITABLE_THREAD_LOCAL_SET.matches(method)) ? MutationLocation.MutationType.SET_CALL : MutationLocation.MutationType.REMOVE_CALL; - MutationLocation mutation = new MutationLocation( - sourcePath, className, methodName, mutationType - ); - acc.addMutation(fullyQualifiedName, mutation); + MutationLocation mutation = new MutationLocation(sourcePath, className, methodName, mutationType); + acc.addMutation(fullyQualifiedName, mutation); + } + } else if (method.getSimpleName().equals("get") && isThreadLocalType(method.getType())) { + // Track get() calls for completeness + String fullyQualifiedName = getFieldFullyQualifiedName(method.getSelect()); + if (fullyQualifiedName != null) { + Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); + J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + J.MethodDeclaration methodDecl = getCursor().firstEnclosing(J.MethodDeclaration.class); + + String className = classDecl != null && classDecl.getType() != null ? classDecl.getType().getFullyQualifiedName() : "UnknownClass"; + String methodName = methodDecl != null ? methodDecl.getSimpleName() : "UnknownMethod"; + + AccessLocation access = new AccessLocation(sourcePath, className, methodName, AccessLocation.AccessType.GET_CALL); + acc.addAccess(fullyQualifiedName, access); + } + } + return method; } - } else if (method.getSimpleName().equals("get") && isThreadLocalType(method.getType())) { - // Track get() calls for completeness - String fullyQualifiedName = getFieldFullyQualifiedName(method.getSelect()); - if (fullyQualifiedName != null) { - Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); - J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); - J.MethodDeclaration methodDecl = getCursor().firstEnclosing(J.MethodDeclaration.class); - - String className = classDecl != null && classDecl.getType() != null ? - classDecl.getType().getFullyQualifiedName() : "UnknownClass"; - String methodName = methodDecl != null ? methodDecl.getSimpleName() : "UnknownMethod"; - AccessLocation access = new AccessLocation( - sourcePath, className, methodName, AccessLocation.AccessType.GET_CALL - ); - acc.addAccess(fullyQualifiedName, access); + private boolean isThreadLocalFieldAccess(Expression expression) { + if (expression instanceof J.Identifier) { + J.Identifier id = (J.Identifier) expression; + return isThreadLocalType(id.getType()); + } else if (expression instanceof J.FieldAccess) { + J.FieldAccess fieldAccess = (J.FieldAccess) expression; + return isThreadLocalType(fieldAccess.getType()); + } + return false; } - } - return method; - } - private boolean isThreadLocalFieldAccess(Expression expression) { - if (expression instanceof J.Identifier) { - J.Identifier id = (J.Identifier) expression; - return isThreadLocalType(id.getType()); - } else if (expression instanceof J.FieldAccess) { - J.FieldAccess fieldAccess = (J.FieldAccess) expression; - return isThreadLocalType(fieldAccess.getType()); - } - return false; - } - - private String getFieldFullyQualifiedName(Expression expression) { - if (expression instanceof J.Identifier) { - J.Identifier id = (J.Identifier) expression; - JavaType.Variable varType = id.getFieldType(); - if (varType != null && varType.getOwner() instanceof JavaType.FullyQualified) { - JavaType.FullyQualified owner = (JavaType.FullyQualified) varType.getOwner(); - return owner.getFullyQualifiedName() + "." + varType.getName(); - } - } else if (expression instanceof J.FieldAccess) { - J.FieldAccess fieldAccess = (J.FieldAccess) expression; - JavaType.Variable varType = fieldAccess.getName().getFieldType(); - if (varType != null && varType.getOwner() instanceof JavaType.FullyQualified) { - JavaType.FullyQualified owner = (JavaType.FullyQualified) varType.getOwner(); - return owner.getFullyQualifiedName() + "." + varType.getName(); + private String getFieldFullyQualifiedName(Expression expression) { + if (expression instanceof J.Identifier) { + J.Identifier id = (J.Identifier) expression; + JavaType.Variable varType = id.getFieldType(); + if (varType != null && varType.getOwner() instanceof JavaType.FullyQualified) { + JavaType.FullyQualified owner = (JavaType.FullyQualified) varType.getOwner(); + return owner.getFullyQualifiedName() + "." + varType.getName(); + } + } else if (expression instanceof J.FieldAccess) { + J.FieldAccess fieldAccess = (J.FieldAccess) expression; + JavaType.Variable varType = fieldAccess.getName().getFieldType(); + if (varType != null && varType.getOwner() instanceof JavaType.FullyQualified) { + JavaType.FullyQualified owner = (JavaType.FullyQualified) varType.getOwner(); + return owner.getFullyQualifiedName() + "." + varType.getName(); + } + } + return null; } - } - return null; - } - }; + }); } @Override public TreeVisitor getVisitor(ThreadLocalUsageAccumulator acc) { - return new JavaIsoVisitor() { - @Override - public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) { - multiVariable = super.visitVariableDeclarations(multiVariable, ctx); - - // Check if any of the variables in this declaration are immutable ThreadLocals - J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); - - for (J.VariableDeclarations.NamedVariable variable : multiVariable.getVariables()) { - if (isThreadLocalType(variable.getType()) && classDecl != null) { - String className = classDecl.getType() != null ? - classDecl.getType().getFullyQualifiedName() : "UnknownClass"; - String fieldName = variable.getName().getSimpleName(); - String fullyQualifiedName = className + "." + fieldName; - - ThreadLocalDeclaration declaration = acc.declarations.get(fullyQualifiedName); - if (declaration != null) { - boolean hasMutations = acc.hasMutations(fullyQualifiedName); - boolean hasExternalMutations = acc.hasExternalMutations(fullyQualifiedName); - - if (!hasMutations) { - // No mutations at all - String message = declaration.isPrivate() ? - "ThreadLocal candidate for ScopedValue migration - never mutated after initialization" : - "ThreadLocal candidate for ScopedValue migration - never mutated in project (but accessible from outside due to non-private access)"; - return SearchResult.found(multiVariable, message); - } else if (!hasExternalMutations && declaration.isPrivate()) { - // Has mutations, but only in the declaring file and it's private - // This is still safe since no external access is possible - // However, we should check if mutations are only in the initializer - List mutations = acc.mutations.get(fullyQualifiedName); - boolean onlyLocalMutations = mutations.stream() - .allMatch(m -> m.getSourcePath().equals(declaration.getSourcePath())); - - if (onlyLocalMutations) { - // Check if it's only mutated in the same class - // For now, we'll be conservative and not mark it - continue; + return check(!acc.declarations.isEmpty(), check( + and(new UsesJavaVersion<>(25), + or(new UsesType<>(THREAD_LOCAL_FQN, true), new UsesType<>(INHERITED_THREAD_LOCAL_FQN, true))), + new JavaIsoVisitor() { + @Override + public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) { + multiVariable = super.visitVariableDeclarations(multiVariable, ctx); + + // Check if any of the variables in this declaration are immutable ThreadLocals + J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + + for (J.VariableDeclarations.NamedVariable variable : multiVariable.getVariables()) { + if (isThreadLocalType(variable.getType()) && classDecl != null) { + String className = classDecl.getType() != null ? classDecl.getType().getFullyQualifiedName() : "UnknownClass"; + String fieldName = variable.getName().getSimpleName(); + String fullyQualifiedName = className + "." + fieldName; + + ThreadLocalDeclaration declaration = acc.declarations.get(fullyQualifiedName); + if (declaration != null) { + boolean hasMutations = acc.hasMutations(fullyQualifiedName); + boolean hasExternalMutations = acc.hasExternalMutations(fullyQualifiedName); + + if (!hasMutations) { + // No mutations at all + String message = declaration.isPrivate() ? "ThreadLocal candidate for ScopedValue migration - never mutated after initialization" : "ThreadLocal candidate for ScopedValue migration - never mutated in project (but accessible from outside due to non-private access)"; + return SearchResult.found(multiVariable, message); + } else if (!hasExternalMutations && declaration.isPrivate()) { + // Has mutations, but only in the declaring file and it's private + // This is still safe since no external access is possible + // However, we should check if mutations are only in the initializer + List mutations = acc.mutations.get(fullyQualifiedName); + boolean onlyLocalMutations = mutations.stream().allMatch(m -> m.getSourcePath().equals(declaration.getSourcePath())); + + if (onlyLocalMutations) { + // Check if it's only mutated in the same class + // For now, we'll be conservative and not mark it + continue; + } + } } } } - } - } - return multiVariable; - } - }; + return multiVariable; + } + })); } private static boolean isThreadLocalType(JavaType type) { - return TypeUtils.isOfClassType(type, "java.lang.ThreadLocal") || - TypeUtils.isOfClassType(type, "java.lang.InheritableThreadLocal"); + return TypeUtils.isOfClassType(type, THREAD_LOCAL_FQN) || TypeUtils.isOfClassType(type, INHERITED_THREAD_LOCAL_FQN); } -} \ No newline at end of file +} diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesCrossFileTest.java b/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesCrossFileTest.java index 61dec9d617..52f7465778 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesCrossFileTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesCrossFileTest.java @@ -416,4 +416,4 @@ public void mutate(InstanceThreadLocalHolder holder) { ); // Instance ThreadLocal should NOT be marked as immutable due to external mutation } -} \ No newline at end of file +} diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java b/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java index fc08a334b3..821ce41a6e 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java @@ -344,4 +344,4 @@ public String getValue() { ) ); } -} \ No newline at end of file +} From a3bd2a9ac1f53906bbe8c2fb2799489836e14eb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Thu, 16 Oct 2025 23:45:22 +0200 Subject: [PATCH 06/12] replace recipe with 3 independent recipes --- .../search/AbstractFindThreadLocals.java | 356 ++++++++++++++++++ .../FindImmutableThreadLocalVariables.java | 344 ----------------- .../search/FindNeverMutatedThreadLocals.java | 50 +++ .../FindThreadLocalsMutatableFromOutside.java | 62 +++ ...hreadLocalsMutatedOnlyInDefiningScope.java | 60 +++ .../java/migrate/search/ThreadLocalTable.java | 61 +++ ...everMutatedThreadLocalsCrossFileTest.java} | 12 +- ... => FindNeverMutatedThreadLocalsTest.java} | 25 +- ...dThreadLocalsMutatableFromOutsideTest.java | 254 +++++++++++++ ...dLocalsMutatedOnlyInDefiningScopeTest.java | 254 +++++++++++++ 10 files changed, 1117 insertions(+), 361 deletions(-) create mode 100644 src/main/java/org/openrewrite/java/migrate/search/AbstractFindThreadLocals.java delete mode 100644 src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java create mode 100644 src/main/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocals.java create mode 100644 src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatableFromOutside.java create mode 100644 src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScope.java create mode 100644 src/main/java/org/openrewrite/java/migrate/search/ThreadLocalTable.java rename src/test/java/org/openrewrite/java/migrate/search/{FindImmutableThreadLocalVariablesCrossFileTest.java => FindNeverMutatedThreadLocalsCrossFileTest.java} (92%) rename src/test/java/org/openrewrite/java/migrate/search/{FindImmutableThreadLocalVariablesTest.java => FindNeverMutatedThreadLocalsTest.java} (82%) create mode 100644 src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatableFromOutsideTest.java create mode 100644 src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java diff --git a/src/main/java/org/openrewrite/java/migrate/search/AbstractFindThreadLocals.java b/src/main/java/org/openrewrite/java/migrate/search/AbstractFindThreadLocals.java new file mode 100644 index 0000000000..440d666050 --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/search/AbstractFindThreadLocals.java @@ -0,0 +1,356 @@ +/* + * Copyright 2024 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.java.migrate.search; + +import lombok.Value; +import org.openrewrite.ExecutionContext; +import org.openrewrite.ScanningRecipe; +import org.openrewrite.SourceFile; +import org.openrewrite.TreeVisitor; +import org.openrewrite.internal.lang.Nullable; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.search.UsesType; +import org.openrewrite.java.tree.Expression; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.TypeUtils; +import org.openrewrite.marker.SearchResult; + +import java.nio.file.Path; +import java.util.*; + +import static org.openrewrite.Preconditions.*; + +public abstract class AbstractFindThreadLocals extends ScanningRecipe { + + protected static final String THREAD_LOCAL_FQN = "java.lang.ThreadLocal"; + protected static final String INHERITED_THREAD_LOCAL_FQN = "java.lang.InheritableThreadLocal"; + + private static final MethodMatcher THREAD_LOCAL_SET = new MethodMatcher(THREAD_LOCAL_FQN + " set(..)"); + private static final MethodMatcher THREAD_LOCAL_REMOVE = new MethodMatcher(THREAD_LOCAL_FQN + " remove()"); + private static final MethodMatcher INHERITABLE_THREAD_LOCAL_SET = new MethodMatcher(INHERITED_THREAD_LOCAL_FQN + " set(..)"); + private static final MethodMatcher INHERITABLE_THREAD_LOCAL_REMOVE = new MethodMatcher(INHERITED_THREAD_LOCAL_FQN + " remove()"); + + @Nullable + protected transient ThreadLocalTable dataTable; + + @Value + public static class ThreadLocalAccumulator { + Map threadLocals = new HashMap<>(); + + public void recordDeclaration(String fqn, Path sourcePath, boolean isPrivate, boolean isStatic, boolean isFinal) { + threadLocals.computeIfAbsent(fqn, k -> new ThreadLocalInfo()) + .setDeclaration(sourcePath, isPrivate, isStatic, isFinal); + } + + public void recordMutation(String fqn, Path sourcePath, boolean isInitContext) { + ThreadLocalInfo info = threadLocals.computeIfAbsent(fqn, k -> new ThreadLocalInfo()); + if (isInitContext) { + info.addInitMutation(sourcePath); + } else { + info.addRegularMutation(sourcePath); + } + } + + public ThreadLocalInfo getInfo(String fqn) { + return threadLocals.get(fqn); + } + + public boolean hasDeclarations() { + return threadLocals.values().stream().anyMatch(ThreadLocalInfo::isDeclared); + } + } + + public static class ThreadLocalInfo { + private Path declarationPath; + private boolean isPrivate; + private boolean isStatic; + private boolean isFinal; + private boolean declared; + private final Set initMutationPaths = new HashSet<>(); + private final Set regularMutationPaths = new HashSet<>(); + + void setDeclaration(Path path, boolean priv, boolean stat, boolean fin) { + this.declarationPath = path; + this.isPrivate = priv; + this.isStatic = stat; + this.isFinal = fin; + this.declared = true; + } + + void addInitMutation(Path path) { + initMutationPaths.add(path); + } + + void addRegularMutation(Path path) { + regularMutationPaths.add(path); + } + + public boolean isPrivate() { + return isPrivate; + } + + public boolean isStatic() { + return isStatic; + } + + public boolean isFinal() { + return isFinal; + } + + public boolean isDeclared() { + return declared; + } + + public boolean hasAnyMutation() { + return !initMutationPaths.isEmpty() || !regularMutationPaths.isEmpty(); + } + + public boolean hasOnlyInitMutations() { + return !initMutationPaths.isEmpty() && regularMutationPaths.isEmpty(); + } + + public boolean hasExternalMutations() { + if (!declared || declarationPath == null) { + return true; // Conservative + } + + // Check if any mutation is from a different file + return initMutationPaths.stream().anyMatch(p -> !p.equals(declarationPath)) || + regularMutationPaths.stream().anyMatch(p -> !p.equals(declarationPath)); + } + + public boolean isOnlyLocallyMutated() { + if (!declared || declarationPath == null) { + return false; + } + + // All mutations must be from the same file as declaration + return initMutationPaths.stream().allMatch(p -> p.equals(declarationPath)) && + regularMutationPaths.stream().allMatch(p -> p.equals(declarationPath)); + } + } + + @Override + public ThreadLocalAccumulator getInitialValue(ExecutionContext ctx) { + return new ThreadLocalAccumulator(); + } + + @Override + public TreeVisitor getScanner(ThreadLocalAccumulator acc) { + return check( + or(new UsesType<>(THREAD_LOCAL_FQN, true), + new UsesType<>(INHERITED_THREAD_LOCAL_FQN, true)), + new JavaIsoVisitor() { + @Override + public J.VariableDeclarations.NamedVariable visitVariable( + J.VariableDeclarations.NamedVariable variable, ExecutionContext ctx) { + variable = super.visitVariable(variable, ctx); + + if (isThreadLocalType(variable.getType())) { + // Only track fields, not local variables + J.MethodDeclaration enclosingMethod = getCursor().firstEnclosing(J.MethodDeclaration.class); + J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + + if (classDecl != null && enclosingMethod == null) { + // It's a field + J.VariableDeclarations variableDecls = getCursor().firstEnclosing(J.VariableDeclarations.class); + if (variableDecls != null) { + String className = classDecl.getType() != null ? + classDecl.getType().getFullyQualifiedName() : "UnknownClass"; + String fqn = className + "." + variable.getName().getSimpleName(); + + boolean isPrivate = hasModifier(variableDecls.getModifiers(), J.Modifier.Type.Private); + boolean isStatic = hasModifier(variableDecls.getModifiers(), J.Modifier.Type.Static); + boolean isFinal = hasModifier(variableDecls.getModifiers(), J.Modifier.Type.Final); + Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); + + acc.recordDeclaration(fqn, sourcePath, isPrivate, isStatic, isFinal); + } + } + } + return variable; + } + + @Override + public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { + method = super.visitMethodInvocation(method, ctx); + + // Check for ThreadLocal.set() or ThreadLocal.remove() calls + if (THREAD_LOCAL_SET.matches(method) || THREAD_LOCAL_REMOVE.matches(method) || + INHERITABLE_THREAD_LOCAL_SET.matches(method) || INHERITABLE_THREAD_LOCAL_REMOVE.matches(method)) { + + String fqn = getFieldFullyQualifiedName(method.getSelect()); + if (fqn != null) { + Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); + boolean isInitContext = isInInitializationContext(); + acc.recordMutation(fqn, sourcePath, isInitContext); + } + } + return method; + } + + @Override + public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ctx) { + assignment = super.visitAssignment(assignment, ctx); + + // Check if we're reassigning a ThreadLocal field + if (isThreadLocalFieldAccess(assignment.getVariable())) { + String fqn = getFieldFullyQualifiedName(assignment.getVariable()); + if (fqn != null) { + Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); + boolean isInitContext = isInInitializationContext(); + acc.recordMutation(fqn, sourcePath, isInitContext); + } + } + return assignment; + } + + private boolean isInInitializationContext() { + J.MethodDeclaration methodDecl = getCursor().firstEnclosing(J.MethodDeclaration.class); + + if (methodDecl == null) { + // Check if we're in a static initializer block + return getCursor().getPathAsStream() + .filter(J.Block.class::isInstance) + .map(J.Block.class::cast) + .anyMatch(J.Block::isStatic); + } + + // Check if it's a constructor + return methodDecl.isConstructor(); + } + + private boolean isThreadLocalFieldAccess(Expression expression) { + if (expression instanceof J.Identifier) { + return isThreadLocalType(((J.Identifier) expression).getType()); + } else if (expression instanceof J.FieldAccess) { + return isThreadLocalType(((J.FieldAccess) expression).getType()); + } + return false; + } + + private String getFieldFullyQualifiedName(Expression expression) { + JavaType.Variable varType = null; + + if (expression instanceof J.Identifier) { + varType = ((J.Identifier) expression).getFieldType(); + } else if (expression instanceof J.FieldAccess) { + varType = ((J.FieldAccess) expression).getName().getFieldType(); + } + + if (varType != null && varType.getOwner() instanceof JavaType.FullyQualified) { + JavaType.FullyQualified owner = (JavaType.FullyQualified) varType.getOwner(); + return owner.getFullyQualifiedName() + "." + varType.getName(); + } + return null; + } + + private boolean hasModifier(List modifiers, J.Modifier.Type type) { + return modifiers.stream().anyMatch(m -> m.getType() == type); + } + }); + } + + @Override + public TreeVisitor getVisitor(ThreadLocalAccumulator acc) { + return check(acc.hasDeclarations(), + new JavaIsoVisitor() { + @Override + public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { + if (dataTable == null) { + dataTable = new ThreadLocalTable(AbstractFindThreadLocals.this); + } + return super.visitCompilationUnit(cu, ctx); + } + + @Override + public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) { + multiVariable = super.visitVariableDeclarations(multiVariable, ctx); + + J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + + for (J.VariableDeclarations.NamedVariable variable : multiVariable.getVariables()) { + if (isThreadLocalType(variable.getType()) && classDecl != null) { + String className = classDecl.getType() != null ? + classDecl.getType().getFullyQualifiedName() : "UnknownClass"; + String fieldName = variable.getName().getSimpleName(); + String fqn = className + "." + fieldName; + + ThreadLocalInfo info = acc.getInfo(fqn); + if (info != null && shouldMarkThreadLocal(info)) { + String message = getMessage(info); + String mutationType = getMutationType(info); + + // Add to data table + if (dataTable != null) { + dataTable.insertRow(ctx, new ThreadLocalTable.Row( + getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath().toString(), + className, + fieldName, + getAccessModifier(multiVariable.getModifiers()), + getModifiers(multiVariable.getModifiers()), + mutationType, + message + )); + } + + return SearchResult.found(multiVariable, message); + } + } + } + + return multiVariable; + } + + private String getAccessModifier(List modifiers) { + if (hasModifier(modifiers, J.Modifier.Type.Private)) { + return "private"; + } else if (hasModifier(modifiers, J.Modifier.Type.Protected)) { + return "protected"; + } else if (hasModifier(modifiers, J.Modifier.Type.Public)) { + return "public"; + } + return "package-private"; + } + + private String getModifiers(List modifiers) { + List mods = new ArrayList<>(); + if (hasModifier(modifiers, J.Modifier.Type.Static)) { + mods.add("static"); + } + if (hasModifier(modifiers, J.Modifier.Type.Final)) { + mods.add("final"); + } + return String.join(" ", mods); + } + + private boolean hasModifier(List modifiers, J.Modifier.Type type) { + return modifiers.stream().anyMatch(m -> m.getType() == type); + } + }); + } + + protected abstract boolean shouldMarkThreadLocal(ThreadLocalInfo info); + protected abstract String getMessage(ThreadLocalInfo info); + protected abstract String getMutationType(ThreadLocalInfo info); + + protected static boolean isThreadLocalType(JavaType type) { + return TypeUtils.isOfClassType(type, THREAD_LOCAL_FQN) || + TypeUtils.isOfClassType(type, INHERITED_THREAD_LOCAL_FQN); + } +} \ No newline at end of file diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java b/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java deleted file mode 100644 index 2adbfcfed5..0000000000 --- a/src/main/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariables.java +++ /dev/null @@ -1,344 +0,0 @@ -/* - * Copyright 2024 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.java.migrate.search; - -import lombok.EqualsAndHashCode; -import lombok.Value; -import org.openrewrite.ExecutionContext; -import org.openrewrite.ScanningRecipe; -import org.openrewrite.SourceFile; -import org.openrewrite.TreeVisitor; -import org.openrewrite.java.JavaIsoVisitor; -import org.openrewrite.java.MethodMatcher; -import org.openrewrite.java.search.UsesJavaVersion; -import org.openrewrite.java.search.UsesType; -import org.openrewrite.java.tree.Expression; -import org.openrewrite.java.tree.J; -import org.openrewrite.java.tree.JavaType; -import org.openrewrite.java.tree.TypeUtils; -import org.openrewrite.marker.SearchResult; - -import java.nio.file.Path; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import static org.openrewrite.Preconditions.*; - -@EqualsAndHashCode(callSuper = false) -public class FindImmutableThreadLocalVariables extends ScanningRecipe { - - private static final String THREAD_LOCAL_FQN = "java.lang.ThreadLocal"; - private static final String INHERITED_THREAD_LOCAL_FQN = "java.lang.InheritableThreadLocal"; - private static final MethodMatcher THREAD_LOCAL_SET = new MethodMatcher(THREAD_LOCAL_FQN + " set(..)"); - private static final MethodMatcher THREAD_LOCAL_REMOVE = new MethodMatcher(THREAD_LOCAL_FQN + " remove()"); - private static final MethodMatcher INHERITABLE_THREAD_LOCAL_SET = new MethodMatcher(INHERITED_THREAD_LOCAL_FQN + " set(..)"); - private static final MethodMatcher INHERITABLE_THREAD_LOCAL_REMOVE = new MethodMatcher(INHERITED_THREAD_LOCAL_FQN + " remove()"); - - @Override - public String getDisplayName() { - return "Find immutable ThreadLocal variables"; - } - - @Override - public String getDescription() { - return "Find `ThreadLocal` variables that are never mutated and could be candidates for migration to `ScopedValue` in Java 25+. " + "This recipe identifies `ThreadLocal` variables that are only initialized but never reassigned or modified through `set()` or `remove()` methods. " + "The recipe analyzes mutations across all source files in the project to provide accurate results."; - } - - @Value - static class ThreadLocalUsageAccumulator { - // Map of ThreadLocal declarations: fullyQualifiedName -> declaration info - Map declarations = new HashMap<>(); - // Map of ThreadLocal mutations: fullyQualifiedName -> list of mutation locations - Map> mutations = new HashMap<>(); - // Map of ThreadLocal accesses: fullyQualifiedName -> list of access locations - Map> accesses = new HashMap<>(); - - void addDeclaration(String fullyQualifiedName, ThreadLocalDeclaration declaration) { - declarations.put(fullyQualifiedName, declaration); - } - - void addMutation(String fullyQualifiedName, MutationLocation mutation) { - mutations.computeIfAbsent(fullyQualifiedName, k -> new ArrayList<>()).add(mutation); - } - - void addAccess(String fullyQualifiedName, AccessLocation access) { - accesses.computeIfAbsent(fullyQualifiedName, k -> new ArrayList<>()).add(access); - } - - boolean hasExternalMutations(String fullyQualifiedName) { - List mutationList = mutations.get(fullyQualifiedName); - if (mutationList == null || mutationList.isEmpty()) { - return false; - } - - ThreadLocalDeclaration declaration = declarations.get(fullyQualifiedName); - if (declaration == null) { - return true; // Conservative: if we can't find the declaration, assume it can be mutated - } - - // Check if any mutation is from a different file than the declaration - for (MutationLocation mutation : mutationList) { - if (!mutation.getSourcePath().equals(declaration.getSourcePath())) { - return true; - } - } - return false; - } - - boolean hasMutations(String fullyQualifiedName) { - return mutations.containsKey(fullyQualifiedName) && !mutations.get(fullyQualifiedName).isEmpty(); - } - } - - @Value - static class ThreadLocalDeclaration { - String fullyQualifiedName; // e.g., "com.example.MyClass.MY_THREAD_LOCAL" - String className; - String fieldName; - boolean isPrivate; - boolean isStatic; - boolean isFinal; - Path sourcePath; - } - - @Value - static class MutationLocation { - Path sourcePath; - String className; - String methodName; - MutationType type; - - enum MutationType { - REASSIGNMENT, SET_CALL, REMOVE_CALL, OTHER - } - } - - @Value - static class AccessLocation { - Path sourcePath; - String className; - String methodName; - AccessType type; - - enum AccessType { - GET_CALL, FIELD_READ, OTHER - } - } - - @Override - public ThreadLocalUsageAccumulator getInitialValue(ExecutionContext ctx) { - return new ThreadLocalUsageAccumulator(); - } - - @Override - public TreeVisitor getScanner(ThreadLocalUsageAccumulator acc) { - return check( - and(new UsesJavaVersion<>(25), - or(new UsesType<>(THREAD_LOCAL_FQN, true), new UsesType<>(INHERITED_THREAD_LOCAL_FQN, true))), - new JavaIsoVisitor() { - @Override - public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { - // Store the current compilation unit's path for tracking locations - getCursor().putMessage("currentSourcePath", cu.getSourcePath()); - return super.visitCompilationUnit(cu, ctx); - } - - @Override - public J.VariableDeclarations.NamedVariable visitVariable(J.VariableDeclarations.NamedVariable variable, ExecutionContext ctx) { - variable = super.visitVariable(variable, ctx); - - if (isThreadLocalType(variable.getType())) { - // Check if this is a field (not a local variable) - // A field is declared directly in a class, not inside a method - J.MethodDeclaration enclosingMethod = getCursor().firstEnclosing(J.MethodDeclaration.class); - J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); - - // It's a field if there's no enclosing method, or if the variable is declared - // directly in the class body (not inside a method body) - boolean isField = classDecl != null && (enclosingMethod == null || getCursor().getPathAsStream().filter(J.Block.class::isInstance).map(J.Block.class::cast).noneMatch(block -> block == enclosingMethod.getBody())); - - if (isField) { - J.VariableDeclarations variableDecls = getCursor().firstEnclosing(J.VariableDeclarations.class); - - if (classDecl != null && variableDecls != null) { - String className = classDecl.getType() != null ? classDecl.getType().getFullyQualifiedName() : "UnknownClass"; - String fieldName = variable.getName().getSimpleName(); - String fullyQualifiedName = className + "." + fieldName; - - boolean isPrivate = variableDecls.getModifiers().stream().anyMatch(mod -> mod.getType() == J.Modifier.Type.Private); - boolean isStatic = variableDecls.getModifiers().stream().anyMatch(mod -> mod.getType() == J.Modifier.Type.Static); - boolean isFinal = variableDecls.getModifiers().stream().anyMatch(mod -> mod.getType() == J.Modifier.Type.Final); - - Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); - - ThreadLocalDeclaration declaration = new ThreadLocalDeclaration(fullyQualifiedName, className, fieldName, isPrivate, isStatic, isFinal, sourcePath); - - acc.addDeclaration(fullyQualifiedName, declaration); - } - } - } - return variable; - } - - @Override - public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ctx) { - assignment = super.visitAssignment(assignment, ctx); - - // Check if we're reassigning a ThreadLocal field - if (isThreadLocalFieldAccess(assignment.getVariable())) { - String fullyQualifiedName = getFieldFullyQualifiedName(assignment.getVariable()); - if (fullyQualifiedName != null) { - Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); - J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); - J.MethodDeclaration methodDecl = getCursor().firstEnclosing(J.MethodDeclaration.class); - - String className = classDecl != null && classDecl.getType() != null ? classDecl.getType().getFullyQualifiedName() : "UnknownClass"; - String methodName = methodDecl != null ? methodDecl.getSimpleName() : "UnknownMethod"; - - MutationLocation mutation = new MutationLocation(sourcePath, className, methodName, MutationLocation.MutationType.REASSIGNMENT); - acc.addMutation(fullyQualifiedName, mutation); - } - } - return assignment; - } - - @Override - public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { - method = super.visitMethodInvocation(method, ctx); - - // Check for ThreadLocal.set() or ThreadLocal.remove() calls - if (THREAD_LOCAL_SET.matches(method) || THREAD_LOCAL_REMOVE.matches(method) || INHERITABLE_THREAD_LOCAL_SET.matches(method) || INHERITABLE_THREAD_LOCAL_REMOVE.matches(method)) { - - String fullyQualifiedName = getFieldFullyQualifiedName(method.getSelect()); - if (fullyQualifiedName != null) { - Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); - J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); - J.MethodDeclaration methodDecl = getCursor().firstEnclosing(J.MethodDeclaration.class); - - String className = classDecl != null && classDecl.getType() != null ? classDecl.getType().getFullyQualifiedName() : "UnknownClass"; - String methodName = methodDecl != null ? methodDecl.getSimpleName() : "UnknownMethod"; - - MutationLocation.MutationType mutationType = (THREAD_LOCAL_SET.matches(method) || INHERITABLE_THREAD_LOCAL_SET.matches(method)) ? MutationLocation.MutationType.SET_CALL : MutationLocation.MutationType.REMOVE_CALL; - - MutationLocation mutation = new MutationLocation(sourcePath, className, methodName, mutationType); - acc.addMutation(fullyQualifiedName, mutation); - } - } else if (method.getSimpleName().equals("get") && isThreadLocalType(method.getType())) { - // Track get() calls for completeness - String fullyQualifiedName = getFieldFullyQualifiedName(method.getSelect()); - if (fullyQualifiedName != null) { - Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); - J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); - J.MethodDeclaration methodDecl = getCursor().firstEnclosing(J.MethodDeclaration.class); - - String className = classDecl != null && classDecl.getType() != null ? classDecl.getType().getFullyQualifiedName() : "UnknownClass"; - String methodName = methodDecl != null ? methodDecl.getSimpleName() : "UnknownMethod"; - - AccessLocation access = new AccessLocation(sourcePath, className, methodName, AccessLocation.AccessType.GET_CALL); - acc.addAccess(fullyQualifiedName, access); - } - } - return method; - } - - private boolean isThreadLocalFieldAccess(Expression expression) { - if (expression instanceof J.Identifier) { - J.Identifier id = (J.Identifier) expression; - return isThreadLocalType(id.getType()); - } else if (expression instanceof J.FieldAccess) { - J.FieldAccess fieldAccess = (J.FieldAccess) expression; - return isThreadLocalType(fieldAccess.getType()); - } - return false; - } - - private String getFieldFullyQualifiedName(Expression expression) { - if (expression instanceof J.Identifier) { - J.Identifier id = (J.Identifier) expression; - JavaType.Variable varType = id.getFieldType(); - if (varType != null && varType.getOwner() instanceof JavaType.FullyQualified) { - JavaType.FullyQualified owner = (JavaType.FullyQualified) varType.getOwner(); - return owner.getFullyQualifiedName() + "." + varType.getName(); - } - } else if (expression instanceof J.FieldAccess) { - J.FieldAccess fieldAccess = (J.FieldAccess) expression; - JavaType.Variable varType = fieldAccess.getName().getFieldType(); - if (varType != null && varType.getOwner() instanceof JavaType.FullyQualified) { - JavaType.FullyQualified owner = (JavaType.FullyQualified) varType.getOwner(); - return owner.getFullyQualifiedName() + "." + varType.getName(); - } - } - return null; - } - }); - } - - @Override - public TreeVisitor getVisitor(ThreadLocalUsageAccumulator acc) { - return check(!acc.declarations.isEmpty(), check( - and(new UsesJavaVersion<>(25), - or(new UsesType<>(THREAD_LOCAL_FQN, true), new UsesType<>(INHERITED_THREAD_LOCAL_FQN, true))), - new JavaIsoVisitor() { - @Override - public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) { - multiVariable = super.visitVariableDeclarations(multiVariable, ctx); - - // Check if any of the variables in this declaration are immutable ThreadLocals - J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); - - for (J.VariableDeclarations.NamedVariable variable : multiVariable.getVariables()) { - if (isThreadLocalType(variable.getType()) && classDecl != null) { - String className = classDecl.getType() != null ? classDecl.getType().getFullyQualifiedName() : "UnknownClass"; - String fieldName = variable.getName().getSimpleName(); - String fullyQualifiedName = className + "." + fieldName; - - ThreadLocalDeclaration declaration = acc.declarations.get(fullyQualifiedName); - if (declaration != null) { - boolean hasMutations = acc.hasMutations(fullyQualifiedName); - boolean hasExternalMutations = acc.hasExternalMutations(fullyQualifiedName); - - if (!hasMutations) { - // No mutations at all - String message = declaration.isPrivate() ? "ThreadLocal candidate for ScopedValue migration - never mutated after initialization" : "ThreadLocal candidate for ScopedValue migration - never mutated in project (but accessible from outside due to non-private access)"; - return SearchResult.found(multiVariable, message); - } else if (!hasExternalMutations && declaration.isPrivate()) { - // Has mutations, but only in the declaring file and it's private - // This is still safe since no external access is possible - // However, we should check if mutations are only in the initializer - List mutations = acc.mutations.get(fullyQualifiedName); - boolean onlyLocalMutations = mutations.stream().allMatch(m -> m.getSourcePath().equals(declaration.getSourcePath())); - - if (onlyLocalMutations) { - // Check if it's only mutated in the same class - // For now, we'll be conservative and not mark it - continue; - } - } - } - } - } - - return multiVariable; - } - })); - } - - private static boolean isThreadLocalType(JavaType type) { - return TypeUtils.isOfClassType(type, THREAD_LOCAL_FQN) || TypeUtils.isOfClassType(type, INHERITED_THREAD_LOCAL_FQN); - } -} diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocals.java b/src/main/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocals.java new file mode 100644 index 0000000000..414e4caa84 --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocals.java @@ -0,0 +1,50 @@ +/* + * Copyright 2024 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.java.migrate.search; + +import lombok.EqualsAndHashCode; + +@EqualsAndHashCode(callSuper = false) +public class FindNeverMutatedThreadLocals extends AbstractFindThreadLocals { + + @Override + public String getDisplayName() { + return "Find ThreadLocal variables that are never mutated"; + } + + @Override + public String getDescription() { + return "Find `ThreadLocal` variables that are never mutated after initialization. " + + "These are prime candidates for migration to `ScopedValue` in Java 25+ as they are effectively immutable. " + + "The recipe identifies `ThreadLocal` variables that are only initialized but never reassigned or modified through `set()` or `remove()` methods."; + } + + @Override + protected boolean shouldMarkThreadLocal(ThreadLocalInfo info) { + // Mark ThreadLocals that have no mutations at all + return !info.hasAnyMutation(); + } + + @Override + protected String getMessage(ThreadLocalInfo info) { + return "ThreadLocal is never mutated and could be replaced with ScopedValue"; + } + + @Override + protected String getMutationType(ThreadLocalInfo info) { + return "Never mutated"; + } +} \ No newline at end of file diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatableFromOutside.java b/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatableFromOutside.java new file mode 100644 index 0000000000..b91cb97784 --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatableFromOutside.java @@ -0,0 +1,62 @@ +/* + * Copyright 2024 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.java.migrate.search; + +import lombok.EqualsAndHashCode; + +@EqualsAndHashCode(callSuper = false) +public class FindThreadLocalsMutatableFromOutside extends AbstractFindThreadLocals { + + @Override + public String getDisplayName() { + return "Find ThreadLocal variables mutatable from outside their defining class"; + } + + @Override + public String getDescription() { + return "Find `ThreadLocal` variables that can be mutated from outside their defining class. " + + "These ThreadLocals have the highest risk as they can be modified by any code with access to them. " + + "This includes non-private ThreadLocals or those mutated from other classes in the codebase."; + } + + @Override + protected boolean shouldMarkThreadLocal(ThreadLocalInfo info) { + // Mark ThreadLocals that are either: + // 1. Actually mutated from outside their defining class + // 2. Non-private (and thus potentially mutable from outside) + return info.hasExternalMutations() || !info.isPrivate(); + } + + @Override + protected String getMessage(ThreadLocalInfo info) { + if (info.hasExternalMutations()) { + return "ThreadLocal is mutated from outside its defining class"; + } else { + // Non-private but not currently mutated externally + String access = info.isStatic() ? "static " : ""; + return "ThreadLocal is " + access + "non-private and can potentially be mutated from outside"; + } + } + + @Override + protected String getMutationType(ThreadLocalInfo info) { + if (info.hasExternalMutations()) { + return "Mutated externally"; + } else { + return "Potentially mutable"; + } + } +} \ No newline at end of file diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScope.java b/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScope.java new file mode 100644 index 0000000000..9934d6993e --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScope.java @@ -0,0 +1,60 @@ +/* + * Copyright 2024 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.java.migrate.search; + +import lombok.EqualsAndHashCode; + +@EqualsAndHashCode(callSuper = false) +public class FindThreadLocalsMutatedOnlyInDefiningScope extends AbstractFindThreadLocals { + + @Override + public String getDisplayName() { + return "Find ThreadLocal variables mutated only in their defining scope"; + } + + @Override + public String getDescription() { + return "Find `ThreadLocal` variables that are only mutated within their defining class or initialization context (constructor/static initializer). " + + "These may be candidates for refactoring as they have limited mutation scope. " + + "The recipe identifies `ThreadLocal` variables that are only modified during initialization or within their declaring class."; + } + + @Override + protected boolean shouldMarkThreadLocal(ThreadLocalInfo info) { + // Mark private ThreadLocals that ARE mutated, but only locally + return info.hasAnyMutation() && + info.isPrivate() && + info.isOnlyLocallyMutated(); + } + + @Override + protected String getMessage(ThreadLocalInfo info) { + if (info.hasOnlyInitMutations()) { + return "ThreadLocal is only mutated during initialization (constructor/static initializer)"; + } else { + return "ThreadLocal is only mutated within its defining class"; + } + } + + @Override + protected String getMutationType(ThreadLocalInfo info) { + if (info.hasOnlyInitMutations()) { + return "Mutated only in initialization"; + } else { + return "Mutated in defining class"; + } + } +} \ No newline at end of file diff --git a/src/main/java/org/openrewrite/java/migrate/search/ThreadLocalTable.java b/src/main/java/org/openrewrite/java/migrate/search/ThreadLocalTable.java new file mode 100644 index 0000000000..f968f84b5a --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/search/ThreadLocalTable.java @@ -0,0 +1,61 @@ +/* + * Copyright 2024 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.java.migrate.search; + +import lombok.Value; +import org.openrewrite.Column; +import org.openrewrite.DataTable; +import org.openrewrite.Recipe; + +public class ThreadLocalTable extends DataTable { + + public ThreadLocalTable(Recipe recipe) { + super(recipe, + "ThreadLocal usage", + "ThreadLocal variables and their mutation patterns."); + } + + @Value + public static class Row { + @Column(displayName = "Source file", + description = "The source file containing the ThreadLocal declaration.") + String sourceFile; + + @Column(displayName = "Class name", + description = "The fully qualified class name where the ThreadLocal is declared.") + String className; + + @Column(displayName = "Field name", + description = "The name of the ThreadLocal field.") + String fieldName; + + @Column(displayName = "Access modifier", + description = "The access modifier of the ThreadLocal field (private, protected, public, package-private).") + String accessModifier; + + @Column(displayName = "Field modifiers", + description = "Additional modifiers like static, final.") + String modifiers; + + @Column(displayName = "Mutation type", + description = "Type of mutation detected (Never mutated, Mutated only in initialization, Mutated in defining class, Mutated externally, Potentially mutable).") + String mutationType; + + @Column(displayName = "Message", + description = "Detailed message about the ThreadLocal's usage pattern.") + String message; + } +} \ No newline at end of file diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesCrossFileTest.java b/src/test/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocalsCrossFileTest.java similarity index 92% rename from src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesCrossFileTest.java rename to src/test/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocalsCrossFileTest.java index 52f7465778..1984076117 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesCrossFileTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocalsCrossFileTest.java @@ -22,11 +22,11 @@ import static org.openrewrite.java.Assertions.java; -class FindImmutableThreadLocalVariablesCrossFileTest implements RewriteTest { +class FindNeverMutatedThreadLocalsCrossFileTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { - spec.recipe(new FindImmutableThreadLocalVariables()); + spec.recipe(new FindNeverMutatedThreadLocals()); } @DocumentExample @@ -86,7 +86,7 @@ public int getCount() { package com.example; public class ReadOnlyHolder { - /*~~(ThreadLocal candidate for ScopedValue migration - never mutated in project (but accessible from outside due to non-private access))~~>*/public static final ThreadLocal COUNTER = ThreadLocal.withInitial(() -> 0); + /*~~(ThreadLocal is never mutated and could be replaced with ScopedValue)~~>*/public static final ThreadLocal COUNTER = ThreadLocal.withInitial(() -> 0); public int getCount() { return COUNTER.get(); @@ -180,7 +180,7 @@ public void tryToAccess() { package com.example; public class PrivateHolder { - /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final ThreadLocal PRIVATE_TL = new ThreadLocal<>(); + /*~~(ThreadLocal is never mutated and could be replaced with ScopedValue)~~>*/private static final ThreadLocal PRIVATE_TL = new ThreadLocal<>(); public String getValue() { return PRIVATE_TL.get(); @@ -291,9 +291,9 @@ public void readAll() { package com.example; public class MultipleThreadLocals { - /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final ThreadLocal PRIVATE_IMMUTABLE = new ThreadLocal<>(); + /*~~(ThreadLocal is never mutated and could be replaced with ScopedValue)~~>*/private static final ThreadLocal PRIVATE_IMMUTABLE = new ThreadLocal<>(); static final ThreadLocal PACKAGE_MUTATED = new ThreadLocal<>(); - /*~~(ThreadLocal candidate for ScopedValue migration - never mutated in project (but accessible from outside due to non-private access))~~>*/public static final ThreadLocal PUBLIC_READ_ONLY = new ThreadLocal<>(); + /*~~(ThreadLocal is never mutated and could be replaced with ScopedValue)~~>*/public static final ThreadLocal PUBLIC_READ_ONLY = new ThreadLocal<>(); protected static final ThreadLocal PROTECTED_MUTATED = new ThreadLocal<>(); public void readAll() { diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java b/src/test/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocalsTest.java similarity index 82% rename from src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java rename to src/test/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocalsTest.java index 821ce41a6e..a33f49f7bb 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/FindImmutableThreadLocalVariablesTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocalsTest.java @@ -22,11 +22,11 @@ import static org.openrewrite.java.Assertions.java; -class FindImmutableThreadLocalVariablesTest implements RewriteTest { +class FindNeverMutatedThreadLocalsTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { - spec.recipe(new FindImmutableThreadLocalVariables()); + spec.recipe(new FindNeverMutatedThreadLocals()); } @DocumentExample @@ -45,7 +45,7 @@ public String getValue() { """, """ class Example { - /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final ThreadLocal TL = new ThreadLocal<>(); + /*~~(ThreadLocal is never mutated and could be replaced with ScopedValue)~~>*/private static final ThreadLocal TL = new ThreadLocal<>(); public String getValue() { return TL.get(); @@ -71,7 +71,7 @@ public int getCount() { """, """ class Example { - /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final ThreadLocal COUNTER = ThreadLocal.withInitial(() -> 0); + /*~~(ThreadLocal is never mutated and could be replaced with ScopedValue)~~>*/private static final ThreadLocal COUNTER = ThreadLocal.withInitial(() -> 0); public int getCount() { return COUNTER.get(); @@ -170,9 +170,9 @@ public Boolean getAnother() { """, """ class Example { - /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final ThreadLocal IMMUTABLE_TL = new ThreadLocal<>(); + /*~~(ThreadLocal is never mutated and could be replaced with ScopedValue)~~>*/private static final ThreadLocal IMMUTABLE_TL = new ThreadLocal<>(); private static final ThreadLocal MUTABLE_TL = new ThreadLocal<>(); - /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final ThreadLocal ANOTHER_IMMUTABLE = ThreadLocal.withInitial(() -> false); + /*~~(ThreadLocal is never mutated and could be replaced with ScopedValue)~~>*/private static final ThreadLocal ANOTHER_IMMUTABLE = ThreadLocal.withInitial(() -> false); public void updateMutable(int value) { MUTABLE_TL.set(value); @@ -206,7 +206,7 @@ public String getValue() { """, """ class Example { - /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private final ThreadLocal instanceTL = new ThreadLocal<>(); + /*~~(ThreadLocal is never mutated and could be replaced with ScopedValue)~~>*/private final ThreadLocal instanceTL = new ThreadLocal<>(); public String getValue() { return instanceTL.get(); @@ -237,7 +237,7 @@ public String formatDate(java.util.Date date) { import java.text.SimpleDateFormat; class Example { - /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final ThreadLocal DATE_FORMAT = + /*~~(ThreadLocal is never mutated and could be replaced with ScopedValue)~~>*/private static final ThreadLocal DATE_FORMAT = ThreadLocal.withInitial(() -> new SimpleDateFormat("yyyy-MM-dd")); public String formatDate(java.util.Date date) { @@ -269,6 +269,9 @@ public void method() { @Test void warnAboutPackagePrivateThreadLocal() { + // Package-private ThreadLocals without mutations are still flagged by FindNeverMutatedThreadLocals + // but should NOT be flagged by FindThreadLocalsMutatableFromOutside if they have no mutations + // For this test, we'll test that it's flagged as never mutated rewriteRun( java( """ @@ -282,7 +285,7 @@ public String getValue() { """, """ class Example { - /*~~(ThreadLocal candidate for ScopedValue migration - never mutated in project (but accessible from outside due to non-private access))~~>*/static final ThreadLocal PACKAGE_TL = new ThreadLocal<>(); + /*~~(ThreadLocal is never mutated and could be replaced with ScopedValue)~~>*/static final ThreadLocal PACKAGE_TL = new ThreadLocal<>(); public String getValue() { return PACKAGE_TL.get(); @@ -308,7 +311,7 @@ public String getValue() { """, """ class Example { - /*~~(ThreadLocal candidate for ScopedValue migration - never mutated in project (but accessible from outside due to non-private access))~~>*/protected static final ThreadLocal PROTECTED_TL = new ThreadLocal<>(); + /*~~(ThreadLocal is never mutated and could be replaced with ScopedValue)~~>*/protected static final ThreadLocal PROTECTED_TL = new ThreadLocal<>(); public String getValue() { return PROTECTED_TL.get(); @@ -334,7 +337,7 @@ public String getValue() { """, """ class Example { - /*~~(ThreadLocal candidate for ScopedValue migration - never mutated after initialization)~~>*/private static final InheritableThreadLocal ITL = new InheritableThreadLocal<>(); + /*~~(ThreadLocal is never mutated and could be replaced with ScopedValue)~~>*/private static final InheritableThreadLocal ITL = new InheritableThreadLocal<>(); public String getValue() { return ITL.get(); diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatableFromOutsideTest.java b/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatableFromOutsideTest.java new file mode 100644 index 0000000000..5f02aaa4cc --- /dev/null +++ b/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatableFromOutsideTest.java @@ -0,0 +1,254 @@ +/* + * Copyright 2024 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.java.migrate.search; + +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 FindThreadLocalsMutatableFromOutsideTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new FindThreadLocalsMutatableFromOutside()); + } + + @DocumentExample + @Test + void identifyNonPrivateThreadLocal() { + rewriteRun( + java( + """ + class Example { + public static final ThreadLocal PUBLIC_TL = new ThreadLocal<>(); + + public String getValue() { + return PUBLIC_TL.get(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal is static non-private and can potentially be mutated from outside)~~>*/public static final ThreadLocal PUBLIC_TL = new ThreadLocal<>(); + + public String getValue() { + return PUBLIC_TL.get(); + } + } + """ + ) + ); + } + + @Test + void identifyPackagePrivateThreadLocal() { + rewriteRun( + java( + """ + class Example { + static final ThreadLocal PACKAGE_TL = new ThreadLocal<>(); + + public String getValue() { + return PACKAGE_TL.get(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal is static non-private and can potentially be mutated from outside)~~>*/static final ThreadLocal PACKAGE_TL = new ThreadLocal<>(); + + public String getValue() { + return PACKAGE_TL.get(); + } + } + """ + ) + ); + } + + @Test + void identifyProtectedThreadLocal() { + rewriteRun( + java( + """ + class Example { + protected static final ThreadLocal PROTECTED_TL = new ThreadLocal<>(); + + public String getValue() { + return PROTECTED_TL.get(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal is static non-private and can potentially be mutated from outside)~~>*/protected static final ThreadLocal PROTECTED_TL = new ThreadLocal<>(); + + public String getValue() { + return PROTECTED_TL.get(); + } + } + """ + ) + ); + } + + @Test + void doNotMarkPrivateThreadLocal() { + rewriteRun( + java( + """ + class Example { + private static final ThreadLocal PRIVATE_TL = new ThreadLocal<>(); + + public String getValue() { + return PRIVATE_TL.get(); + } + } + """ + ) + ); + } + + @Test + void identifyThreadLocalMutatedFromOutside() { + rewriteRun( + java( + """ + package com.example; + + public class ThreadLocalHolder { + public static final ThreadLocal SHARED_TL = new ThreadLocal<>(); + + public String getValue() { + return SHARED_TL.get(); + } + } + """, + """ + package com.example; + + public class ThreadLocalHolder { + /*~~(ThreadLocal is mutated from outside its defining class)~~>*/public static final ThreadLocal SHARED_TL = new ThreadLocal<>(); + + public String getValue() { + return SHARED_TL.get(); + } + } + """ + ), + java( + """ + package com.example; + + class ThreadLocalMutator { + public void mutate() { + ThreadLocalHolder.SHARED_TL.set("mutated"); + } + } + """ + ) + ); + } + + @Test + void identifyInstanceThreadLocalNonPrivate() { + rewriteRun( + java( + """ + class Example { + public final ThreadLocal instanceTL = new ThreadLocal<>(); + + public String getValue() { + return instanceTL.get(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal is non-private and can potentially be mutated from outside)~~>*/public final ThreadLocal instanceTL = new ThreadLocal<>(); + + public String getValue() { + return instanceTL.get(); + } + } + """ + ) + ); + } + + @Test + void doNotMarkPrivateInstanceThreadLocal() { + rewriteRun( + java( + """ + class Example { + private final ThreadLocal instanceTL = new ThreadLocal<>(); + + public String getValue() { + return instanceTL.get(); + } + } + """ + ) + ); + } + + @Test + void identifyProtectedMutatedFromSubclass() { + rewriteRun( + java( + """ + package com.example; + + public class BaseClass { + protected static final ThreadLocal PROTECTED_TL = new ThreadLocal<>(); + + public String getValue() { + return PROTECTED_TL.get(); + } + } + """, + """ + package com.example; + + public class BaseClass { + /*~~(ThreadLocal is mutated from outside its defining class)~~>*/protected static final ThreadLocal PROTECTED_TL = new ThreadLocal<>(); + + public String getValue() { + return PROTECTED_TL.get(); + } + } + """ + ), + java( + """ + package com.example.sub; + + import com.example.BaseClass; + + public class SubClass extends BaseClass { + public void modifyThreadLocal() { + PROTECTED_TL.set("modified by subclass"); + } + } + """ + ) + ); + } +} \ No newline at end of file diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java b/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java new file mode 100644 index 0000000000..095737c460 --- /dev/null +++ b/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java @@ -0,0 +1,254 @@ +/* + * Copyright 2024 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.java.migrate.search; + +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 FindThreadLocalsMutatedOnlyInDefiningScopeTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new FindThreadLocalsMutatedOnlyInDefiningScope()); + } + + @DocumentExample + @Test + void identifyThreadLocalMutatedOnlyInConstructor() { + rewriteRun( + java( + """ + class Example { + private final ThreadLocal TL = new ThreadLocal<>(); + + public Example() { + TL.set("initial value"); + } + + public String getValue() { + return TL.get(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal is only mutated during initialization (constructor/static initializer))~~>*/private final ThreadLocal TL = new ThreadLocal<>(); + + public Example() { + TL.set("initial value"); + } + + public String getValue() { + return TL.get(); + } + } + """ + ) + ); + } + + @Test + void identifyThreadLocalMutatedOnlyInStaticInitializer() { + rewriteRun( + java( + """ + class Example { + private static final ThreadLocal TL = new ThreadLocal<>(); + + static { + TL.set("static initial value"); + } + + public String getValue() { + return TL.get(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal is only mutated during initialization (constructor/static initializer))~~>*/private static final ThreadLocal TL = new ThreadLocal<>(); + + static { + TL.set("static initial value"); + } + + public String getValue() { + return TL.get(); + } + } + """ + ) + ); + } + + @Test + void identifyThreadLocalMutatedInDefiningClass() { + rewriteRun( + java( + """ + class Example { + private static final ThreadLocal TL = new ThreadLocal<>(); + + public void setValue(String value) { + TL.set(value); + } + + public String getValue() { + return TL.get(); + } + + public void cleanup() { + TL.remove(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal is only mutated within its defining class)~~>*/private static final ThreadLocal TL = new ThreadLocal<>(); + + public void setValue(String value) { + TL.set(value); + } + + public String getValue() { + return TL.get(); + } + + public void cleanup() { + TL.remove(); + } + } + """ + ) + ); + } + + @Test + void doNotMarkThreadLocalNeverMutated() { + // This should not be marked by this recipe - it's for FindNeverMutatedThreadLocals + rewriteRun( + java( + """ + class Example { + private static final ThreadLocal TL = new ThreadLocal<>(); + + public String getValue() { + return TL.get(); + } + } + """ + ) + ); + } + + @Test + void doNotMarkNonPrivateThreadLocal() { + // Non-private ThreadLocals shouldn't be marked by this recipe + rewriteRun( + java( + """ + class Example { + static final ThreadLocal TL = new ThreadLocal<>(); + + public void setValue(String value) { + TL.set(value); + } + } + """ + ) + ); + } + + @Test + void identifyInstanceThreadLocalMutatedInConstructor() { + rewriteRun( + java( + """ + class Example { + private final ThreadLocal counter = new ThreadLocal<>(); + + public Example(int initial) { + counter.set(initial); + } + + public Integer getCount() { + return counter.get(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal is only mutated during initialization (constructor/static initializer))~~>*/private final ThreadLocal counter = new ThreadLocal<>(); + + public Example(int initial) { + counter.set(initial); + } + + public Integer getCount() { + return counter.get(); + } + } + """ + ) + ); + } + + @Test + void identifyThreadLocalWithMixedInitAndClassMutations() { + rewriteRun( + java( + """ + class Example { + private static final ThreadLocal TL = new ThreadLocal<>(); + + static { + TL.set("initial"); + } + + public void update(String value) { + TL.set(value); + } + + public String getValue() { + return TL.get(); + } + } + """, + """ + class Example { + /*~~(ThreadLocal is only mutated within its defining class)~~>*/private static final ThreadLocal TL = new ThreadLocal<>(); + + static { + TL.set("initial"); + } + + public void update(String value) { + TL.set(value); + } + + public String getValue() { + return TL.get(); + } + } + """ + ) + ); + } +} \ No newline at end of file From 01f2e9fc1a216a19bf1c56b9ebe2e6e33f8845da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Fri, 17 Oct 2025 00:09:32 +0200 Subject: [PATCH 07/12] First round of review --- .../search/AbstractFindThreadLocals.java | 147 ++++++++++-------- .../search/FindNeverMutatedThreadLocals.java | 11 ++ ...> FindThreadLocalsMutableFromOutside.java} | 15 +- ...hreadLocalsMutatedOnlyInDefiningScope.java | 41 +++-- ...ndThreadLocalsMutableFromOutsideTest.java} | 4 +- 5 files changed, 137 insertions(+), 81 deletions(-) rename src/main/java/org/openrewrite/java/migrate/search/{FindThreadLocalsMutatableFromOutside.java => FindThreadLocalsMutableFromOutside.java} (83%) rename src/test/java/org/openrewrite/java/migrate/search/{FindThreadLocalsMutatableFromOutsideTest.java => FindThreadLocalsMutableFromOutsideTest.java} (98%) diff --git a/src/main/java/org/openrewrite/java/migrate/search/AbstractFindThreadLocals.java b/src/main/java/org/openrewrite/java/migrate/search/AbstractFindThreadLocals.java index 440d666050..e35acf7675 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/AbstractFindThreadLocals.java +++ b/src/main/java/org/openrewrite/java/migrate/search/AbstractFindThreadLocals.java @@ -16,11 +16,11 @@ package org.openrewrite.java.migrate.search; import lombok.Value; +import org.jspecify.annotations.Nullable; import org.openrewrite.ExecutionContext; import org.openrewrite.ScanningRecipe; import org.openrewrite.SourceFile; import org.openrewrite.TreeVisitor; -import org.openrewrite.internal.lang.Nullable; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.MethodMatcher; import org.openrewrite.java.search.UsesType; @@ -66,7 +66,7 @@ public void recordMutation(String fqn, Path sourcePath, boolean isInitContext) { } } - public ThreadLocalInfo getInfo(String fqn) { + public @Nullable ThreadLocalInfo getInfo(String fqn) { return threadLocals.get(fqn); } @@ -76,7 +76,7 @@ public boolean hasDeclarations() { } public static class ThreadLocalInfo { - private Path declarationPath; + private @Nullable Path declarationPath; private boolean isPrivate; private boolean isStatic; private boolean isFinal; @@ -161,28 +161,40 @@ public J.VariableDeclarations.NamedVariable visitVariable( J.VariableDeclarations.NamedVariable variable, ExecutionContext ctx) { variable = super.visitVariable(variable, ctx); - if (isThreadLocalType(variable.getType())) { - // Only track fields, not local variables - J.MethodDeclaration enclosingMethod = getCursor().firstEnclosing(J.MethodDeclaration.class); - J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); - - if (classDecl != null && enclosingMethod == null) { - // It's a field - J.VariableDeclarations variableDecls = getCursor().firstEnclosing(J.VariableDeclarations.class); - if (variableDecls != null) { - String className = classDecl.getType() != null ? - classDecl.getType().getFullyQualifiedName() : "UnknownClass"; - String fqn = className + "." + variable.getName().getSimpleName(); - - boolean isPrivate = hasModifier(variableDecls.getModifiers(), J.Modifier.Type.Private); - boolean isStatic = hasModifier(variableDecls.getModifiers(), J.Modifier.Type.Static); - boolean isFinal = hasModifier(variableDecls.getModifiers(), J.Modifier.Type.Final); - Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); - - acc.recordDeclaration(fqn, sourcePath, isPrivate, isStatic, isFinal); - } - } + // Early return for non-ThreadLocal types + if (!isThreadLocalType(variable.getType())) { + return variable; + } + + // Early return for local variables (not fields) + J.MethodDeclaration enclosingMethod = getCursor().firstEnclosing(J.MethodDeclaration.class); + if (enclosingMethod != null) { + return variable; + } + + // Early return if not in a class + J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + if (classDecl == null) { + return variable; } + + // Early return if we can't find the variable declarations + J.VariableDeclarations variableDecls = getCursor().firstEnclosing(J.VariableDeclarations.class); + if (variableDecls == null) { + return variable; + } + + // Process ThreadLocal field declaration + JavaType.@Nullable FullyQualified classType = classDecl.getType(); + String className = classType != null ? classType.getFullyQualifiedName() : "UnknownClass"; + String fqn = className + "." + variable.getName().getSimpleName(); + + boolean isPrivate = variableDecls.hasModifier(J.Modifier.Type.Private); + boolean isStatic = variableDecls.hasModifier(J.Modifier.Type.Static); + boolean isFinal = variableDecls.hasModifier(J.Modifier.Type.Final); + Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); + + acc.recordDeclaration(fqn, sourcePath, isPrivate, isStatic, isFinal); return variable; } @@ -190,17 +202,20 @@ public J.VariableDeclarations.NamedVariable visitVariable( public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) { method = super.visitMethodInvocation(method, ctx); - // Check for ThreadLocal.set() or ThreadLocal.remove() calls - if (THREAD_LOCAL_SET.matches(method) || THREAD_LOCAL_REMOVE.matches(method) || - INHERITABLE_THREAD_LOCAL_SET.matches(method) || INHERITABLE_THREAD_LOCAL_REMOVE.matches(method)) { + // Early return if not a ThreadLocal mutation method + if (!THREAD_LOCAL_SET.matches(method) && !THREAD_LOCAL_REMOVE.matches(method) && + !INHERITABLE_THREAD_LOCAL_SET.matches(method) && !INHERITABLE_THREAD_LOCAL_REMOVE.matches(method)) { + return method; + } - String fqn = getFieldFullyQualifiedName(method.getSelect()); - if (fqn != null) { - Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); - boolean isInitContext = isInInitializationContext(); - acc.recordMutation(fqn, sourcePath, isInitContext); - } + @Nullable String fqn = getFieldFullyQualifiedName(method.getSelect()); + if (fqn == null) { + return method; } + + Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); + boolean isInitContext = isInInitializationContext(); + acc.recordMutation(fqn, sourcePath, isInitContext); return method; } @@ -208,15 +223,19 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ctx) { assignment = super.visitAssignment(assignment, ctx); - // Check if we're reassigning a ThreadLocal field - if (isThreadLocalFieldAccess(assignment.getVariable())) { - String fqn = getFieldFullyQualifiedName(assignment.getVariable()); - if (fqn != null) { - Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); - boolean isInitContext = isInInitializationContext(); - acc.recordMutation(fqn, sourcePath, isInitContext); - } + // Early return if not a ThreadLocal field access + if (!isThreadLocalFieldAccess(assignment.getVariable())) { + return assignment; } + + @Nullable String fqn = getFieldFullyQualifiedName(assignment.getVariable()); + if (fqn == null) { + return assignment; + } + + Path sourcePath = getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath(); + boolean isInitContext = isInInitializationContext(); + acc.recordMutation(fqn, sourcePath, isInitContext); return assignment; } @@ -244,25 +263,30 @@ private boolean isThreadLocalFieldAccess(Expression expression) { return false; } - private String getFieldFullyQualifiedName(Expression expression) { - JavaType.Variable varType = null; + private @Nullable String getFieldFullyQualifiedName(@Nullable Expression expression) { + if (expression == null) { + return null; + } + JavaType.@Nullable Variable varType = null; if (expression instanceof J.Identifier) { varType = ((J.Identifier) expression).getFieldType(); } else if (expression instanceof J.FieldAccess) { varType = ((J.FieldAccess) expression).getName().getFieldType(); } - if (varType != null && varType.getOwner() instanceof JavaType.FullyQualified) { - JavaType.FullyQualified owner = (JavaType.FullyQualified) varType.getOwner(); - return owner.getFullyQualifiedName() + "." + varType.getName(); + if (varType == null) { + return null; + } + + @Nullable JavaType owner = varType.getOwner(); + if (!(owner instanceof JavaType.FullyQualified)) { + return null; } - return null; - } - private boolean hasModifier(List modifiers, J.Modifier.Type type) { - return modifiers.stream().anyMatch(m -> m.getType() == type); + return ((JavaType.FullyQualified) owner).getFullyQualifiedName() + "." + varType.getName(); } + }); } @@ -302,8 +326,8 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath().toString(), className, fieldName, - getAccessModifier(multiVariable.getModifiers()), - getModifiers(multiVariable.getModifiers()), + getAccessModifier(multiVariable), + getModifiers(multiVariable), mutationType, message )); @@ -317,31 +341,28 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m return multiVariable; } - private String getAccessModifier(List modifiers) { - if (hasModifier(modifiers, J.Modifier.Type.Private)) { + private String getAccessModifier(J.VariableDeclarations variableDecls) { + if (variableDecls.hasModifier(J.Modifier.Type.Private)) { return "private"; - } else if (hasModifier(modifiers, J.Modifier.Type.Protected)) { + } else if (variableDecls.hasModifier(J.Modifier.Type.Protected)) { return "protected"; - } else if (hasModifier(modifiers, J.Modifier.Type.Public)) { + } else if (variableDecls.hasModifier(J.Modifier.Type.Public)) { return "public"; } return "package-private"; } - private String getModifiers(List modifiers) { + private String getModifiers(J.VariableDeclarations variableDecls) { List mods = new ArrayList<>(); - if (hasModifier(modifiers, J.Modifier.Type.Static)) { + if (variableDecls.hasModifier(J.Modifier.Type.Static)) { mods.add("static"); } - if (hasModifier(modifiers, J.Modifier.Type.Final)) { + if (variableDecls.hasModifier(J.Modifier.Type.Final)) { mods.add("final"); } return String.join(" ", mods); } - private boolean hasModifier(List modifiers, J.Modifier.Type type) { - return modifiers.stream().anyMatch(m -> m.getType() == type); - } }); } @@ -349,7 +370,7 @@ private boolean hasModifier(List modifiers, J.Modifier.Type type) { protected abstract String getMessage(ThreadLocalInfo info); protected abstract String getMutationType(ThreadLocalInfo info); - protected static boolean isThreadLocalType(JavaType type) { + protected static boolean isThreadLocalType(@Nullable JavaType type) { return TypeUtils.isOfClassType(type, THREAD_LOCAL_FQN) || TypeUtils.isOfClassType(type, INHERITED_THREAD_LOCAL_FQN); } diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocals.java b/src/main/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocals.java index 414e4caa84..6fe60b1ff9 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocals.java +++ b/src/main/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocals.java @@ -16,7 +16,13 @@ package org.openrewrite.java.migrate.search; import lombok.EqualsAndHashCode; +import lombok.Value; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +@Value @EqualsAndHashCode(callSuper = false) public class FindNeverMutatedThreadLocals extends AbstractFindThreadLocals { @@ -32,6 +38,11 @@ public String getDescription() { "The recipe identifies `ThreadLocal` variables that are only initialized but never reassigned or modified through `set()` or `remove()` methods."; } + @Override + public Set getTags() { + return new HashSet<>(Arrays.asList("java25", "threadlocal", "scopedvalue", "migration")); + } + @Override protected boolean shouldMarkThreadLocal(ThreadLocalInfo info) { // Mark ThreadLocals that have no mutations at all diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatableFromOutside.java b/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutside.java similarity index 83% rename from src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatableFromOutside.java rename to src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutside.java index b91cb97784..a3a60141cd 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatableFromOutside.java +++ b/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutside.java @@ -16,13 +16,19 @@ package org.openrewrite.java.migrate.search; import lombok.EqualsAndHashCode; +import lombok.Value; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +@Value @EqualsAndHashCode(callSuper = false) -public class FindThreadLocalsMutatableFromOutside extends AbstractFindThreadLocals { +public class FindThreadLocalsMutableFromOutside extends AbstractFindThreadLocals { @Override public String getDisplayName() { - return "Find ThreadLocal variables mutatable from outside their defining class"; + return "Find ThreadLocal variables mutable from outside their defining class"; } @Override @@ -32,6 +38,11 @@ public String getDescription() { "This includes non-private ThreadLocals or those mutated from other classes in the codebase."; } + @Override + public Set getTags() { + return new HashSet<>(Arrays.asList("java25", "threadlocal", "scopedvalue", "migration", "security")); + } + @Override protected boolean shouldMarkThreadLocal(ThreadLocalInfo info) { // Mark ThreadLocals that are either: diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScope.java b/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScope.java index 9934d6993e..6f27a5b034 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScope.java +++ b/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScope.java @@ -16,7 +16,13 @@ package org.openrewrite.java.migrate.search; import lombok.EqualsAndHashCode; +import lombok.Value; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +@Value @EqualsAndHashCode(callSuper = false) public class FindThreadLocalsMutatedOnlyInDefiningScope extends AbstractFindThreadLocals { @@ -32,29 +38,36 @@ public String getDescription() { "The recipe identifies `ThreadLocal` variables that are only modified during initialization or within their declaring class."; } + @Override + public Set getTags() { + return new HashSet<>(Arrays.asList("java25", "threadlocal", "scopedvalue", "migration")); + } + @Override protected boolean shouldMarkThreadLocal(ThreadLocalInfo info) { - // Mark private ThreadLocals that ARE mutated, but only locally - return info.hasAnyMutation() && - info.isPrivate() && - info.isOnlyLocallyMutated(); + // Early return for ThreadLocals without mutations + if (!info.hasAnyMutation()) { + return false; + } + // Early return for non-private ThreadLocals + if (!info.isPrivate()) { + return false; + } + // Mark if only locally mutated + return info.isOnlyLocallyMutated(); } @Override protected String getMessage(ThreadLocalInfo info) { - if (info.hasOnlyInitMutations()) { - return "ThreadLocal is only mutated during initialization (constructor/static initializer)"; - } else { - return "ThreadLocal is only mutated within its defining class"; - } + return info.hasOnlyInitMutations() + ? "ThreadLocal is only mutated during initialization (constructor/static initializer)" + : "ThreadLocal is only mutated within its defining class"; } @Override protected String getMutationType(ThreadLocalInfo info) { - if (info.hasOnlyInitMutations()) { - return "Mutated only in initialization"; - } else { - return "Mutated in defining class"; - } + return info.hasOnlyInitMutations() + ? "Mutated only in initialization" + : "Mutated in defining class"; } } \ No newline at end of file diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatableFromOutsideTest.java b/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutsideTest.java similarity index 98% rename from src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatableFromOutsideTest.java rename to src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutsideTest.java index 5f02aaa4cc..19fe6e4200 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatableFromOutsideTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutsideTest.java @@ -22,11 +22,11 @@ import static org.openrewrite.java.Assertions.java; -class FindThreadLocalsMutatableFromOutsideTest implements RewriteTest { +class FindThreadLocalsMutableFromOutsideTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { - spec.recipe(new FindThreadLocalsMutatableFromOutside()); + spec.recipe(new FindThreadLocalsMutableFromOutside()); } @DocumentExample From 7563cb6abe18393eb4eb552196a67db46e79af9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Fri, 17 Oct 2025 00:10:35 +0200 Subject: [PATCH 08/12] apply best practices --- .../java/migrate/search/AbstractFindThreadLocals.java | 2 +- .../java/migrate/search/FindNeverMutatedThreadLocals.java | 2 +- .../java/migrate/search/FindThreadLocalsMutableFromOutside.java | 2 +- .../search/FindThreadLocalsMutatedOnlyInDefiningScope.java | 2 +- .../org/openrewrite/java/migrate/search/ThreadLocalTable.java | 2 +- .../migrate/search/FindThreadLocalsMutableFromOutsideTest.java | 2 +- .../search/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/openrewrite/java/migrate/search/AbstractFindThreadLocals.java b/src/main/java/org/openrewrite/java/migrate/search/AbstractFindThreadLocals.java index e35acf7675..050df7dd2a 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/AbstractFindThreadLocals.java +++ b/src/main/java/org/openrewrite/java/migrate/search/AbstractFindThreadLocals.java @@ -374,4 +374,4 @@ protected static boolean isThreadLocalType(@Nullable JavaType type) { return TypeUtils.isOfClassType(type, THREAD_LOCAL_FQN) || TypeUtils.isOfClassType(type, INHERITED_THREAD_LOCAL_FQN); } -} \ No newline at end of file +} diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocals.java b/src/main/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocals.java index 6fe60b1ff9..eccdc18d69 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocals.java +++ b/src/main/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocals.java @@ -58,4 +58,4 @@ protected String getMessage(ThreadLocalInfo info) { protected String getMutationType(ThreadLocalInfo info) { return "Never mutated"; } -} \ No newline at end of file +} diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutside.java b/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutside.java index a3a60141cd..372bdd9bac 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutside.java +++ b/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutside.java @@ -70,4 +70,4 @@ protected String getMutationType(ThreadLocalInfo info) { return "Potentially mutable"; } } -} \ No newline at end of file +} diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScope.java b/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScope.java index 6f27a5b034..a25b15e6f8 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScope.java +++ b/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScope.java @@ -70,4 +70,4 @@ protected String getMutationType(ThreadLocalInfo info) { ? "Mutated only in initialization" : "Mutated in defining class"; } -} \ No newline at end of file +} diff --git a/src/main/java/org/openrewrite/java/migrate/search/ThreadLocalTable.java b/src/main/java/org/openrewrite/java/migrate/search/ThreadLocalTable.java index f968f84b5a..13051ccaf5 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/ThreadLocalTable.java +++ b/src/main/java/org/openrewrite/java/migrate/search/ThreadLocalTable.java @@ -58,4 +58,4 @@ public static class Row { description = "Detailed message about the ThreadLocal's usage pattern.") String message; } -} \ No newline at end of file +} diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutsideTest.java b/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutsideTest.java index 19fe6e4200..3ef34db3f1 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutsideTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutsideTest.java @@ -251,4 +251,4 @@ public void modifyThreadLocal() { ) ); } -} \ No newline at end of file +} diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java b/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java index 095737c460..ce7693ccb3 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java @@ -251,4 +251,4 @@ public String getValue() { ) ); } -} \ No newline at end of file +} From 88d6365b1b06a6dd4318eb8bd1223c443e707b70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Fri, 17 Oct 2025 00:15:25 +0200 Subject: [PATCH 09/12] remove old artifacts --- .../ConvertImmutableClassToRecord.java | 289 ------ .../ConvertImmutableClassToRecordTest.java | 821 ------------------ 2 files changed, 1110 deletions(-) delete mode 100644 src/main/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecord.java delete mode 100644 src/test/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecordTest.java diff --git a/src/main/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecord.java b/src/main/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecord.java deleted file mode 100644 index aa7c2496ed..0000000000 --- a/src/main/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecord.java +++ /dev/null @@ -1,289 +0,0 @@ -/* - * Copyright 2024 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.java.migrate; - -import lombok.EqualsAndHashCode; -import lombok.Value; -import org.jspecify.annotations.Nullable; -import org.openrewrite.ExecutionContext; -import org.openrewrite.Option; -import org.openrewrite.Recipe; -import org.openrewrite.TreeVisitor; -import org.openrewrite.java.ChangeMethodName; -import org.openrewrite.java.JavaTemplate; -import org.openrewrite.java.JavaVisitor; -import org.openrewrite.java.tree.J; - -import java.util.*; -import java.util.stream.Collectors; - -@Value -@EqualsAndHashCode(callSuper = false) -public class ConvertImmutableClassToRecord extends Recipe { - - @Option(displayName = "Package pattern", - description = "A glob pattern to match packages where classes should be converted to records. " + - "If not specified, the recipe will be applied to all packages.", - example = "com.example.**", - required = false) - @Nullable - String packagePattern; - - @Override - public String getDisplayName() { - return "Convert immutable class to record"; - } - - @Override - public String getDescription() { - //language=Markdown - return "Converts immutable classes to Java records. This is a composite recipe that:\n" + - " 1. Identifies classes that meet record conversion criteria\n" + - " 2. Converts eligible classes to records\n" + - " 3. Updates all getter method calls to use record accessor syntax\n" + - "\n" + - " A class is eligible for conversion if it:\n" + - " - Has only private fields\n" + - " - Has corresponding getter methods for all fields\n" + - " - Has no setter methods\n" + - " - Does not extend another class\n" + - " - Is effectively immutable"; - } - - @Override - public TreeVisitor getVisitor() { - return new ConvertImmutableClassToRecordVisitor(); - } - - private class ConvertImmutableClassToRecordVisitor extends JavaVisitor { - - @Override - public J visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext ctx) { - if (!isEligibleForRecordConversion(classDecl) || !matchesPackagePattern(classDecl)) { - return classDecl; - } - - // Get all getter methods that will need to be updated - List getterMethods = getGetterMethods(classDecl); - String classTypeFqn = getFullyQualifiedName(classDecl); - - // Schedule method name changes for all getters after this transformation - for (J.MethodDeclaration getter : getterMethods) { - String fieldName = getFieldNameFromGetter(getter); - if (fieldName != null) { - String oldMethodPattern = classTypeFqn + " " + getter.getSimpleName() + "()"; - doAfterVisit(new ChangeMethodName(oldMethodPattern, fieldName, true, null).getVisitor()); - } - } - - // Transform the class to a record - return transformToRecord(classDecl); - } - - private J.ClassDeclaration transformToRecord(J.ClassDeclaration classDecl) { - List fields = getPrivateFields(classDecl); - List utilityMethods = extractUtilityMethods(classDecl); - - // Create record components string - StringBuilder recordComponents = new StringBuilder(); - for (int i = 0; i < fields.size(); i++) { - J.VariableDeclarations field = fields.get(i); - for (J.VariableDeclarations.NamedVariable var : field.getVariables()) { - if (recordComponents.length() > 0) { - recordComponents.append(", "); - } - - // Include field annotations - StringBuilder fieldComponent = new StringBuilder(); - if (!field.getLeadingAnnotations().isEmpty()) { - for (J.Annotation annotation : field.getLeadingAnnotations()) { - fieldComponent.append("@").append(annotation.getAnnotationType().toString()).append(" "); - } - } - fieldComponent.append(field.getTypeExpression()).append(" ").append(var.getSimpleName()); - recordComponents.append(fieldComponent); - } - } - - // Build record template - StringBuilder recordTemplate = new StringBuilder(); - - // Add class-level annotations and modifiers - if (!classDecl.getLeadingAnnotations().isEmpty() || !classDecl.getModifiers().isEmpty()) { - for (J.Annotation annotation : classDecl.getLeadingAnnotations()) { - recordTemplate.append("@").append(annotation.getAnnotationType().toString()).append(" "); - } - for (J.Modifier modifier : classDecl.getModifiers()) { - if (modifier.getType() != J.Modifier.Type.Final) { // Records are implicitly final - recordTemplate.append(modifier.getType().toString().toLowerCase()).append(" "); - } - } - } - - recordTemplate.append("record ").append(classDecl.getSimpleName()).append("(") - .append(recordComponents).append(")"); - - if (!classDecl.getImplements().isEmpty()) { - recordTemplate.append(" implements "); - for (int i = 0; i < classDecl.getImplements().size(); i++) { - if (i > 0) { - recordTemplate.append( ", " ); - } - recordTemplate.append("#{any()}"); - } - } - - recordTemplate.append(" { "); - for (int i = 0; i < utilityMethods.size(); i++) { - recordTemplate.append("#{any()} "); - } - recordTemplate.append("}"); - - // Prepare template parameters - List templateParams = new ArrayList<>(); - if (!classDecl.getImplements().isEmpty()) { - for (J j : classDecl.getImplements()) { - templateParams.add(j); - } - } - for (J.MethodDeclaration method : utilityMethods) { - templateParams.add(method); - } - - JavaTemplate template = JavaTemplate.builder(recordTemplate.toString()).build(); - return template.apply(updateCursor(classDecl), classDecl.getCoordinates().replace(), templateParams.toArray()); - } - - private boolean isEligibleForRecordConversion(J.ClassDeclaration classDecl) { - // Skip if class extends another class (records can't extend classes) - if (classDecl.getExtends() != null) { - return false; - } - - // Skip if class is not a regular class - if (classDecl.getKind() != J.ClassDeclaration.Kind.Type.Class) { - return false; - } - - List fields = getPrivateFields(classDecl); - if (fields.isEmpty()) { - return false; - } - - // Check if all fields have corresponding getters - Set fieldNames = fields.stream() - .flatMap(field -> field.getVariables().stream()) - .map(J.VariableDeclarations.NamedVariable::getSimpleName) - .collect(Collectors.toSet()); - - Set getterFields = getGetterMethods(classDecl).stream() - .map(this::getFieldNameFromGetter) - .filter(Objects::nonNull) - .collect(Collectors.toSet()); - - return fieldNames.equals(getterFields) && !hasSetterMethods(classDecl); - } - - private List extractUtilityMethods(J.ClassDeclaration classDecl) { - return classDecl.getBody().getStatements().stream() - .filter(J.MethodDeclaration.class::isInstance) - .map(J.MethodDeclaration.class::cast) - .filter(method -> !isConstructor(method) && !isGetterMethod(method) && !isSetterMethod(method)) - .collect(Collectors.toList()); - } - - private List getPrivateFields(J.ClassDeclaration classDecl) { - return classDecl.getBody().getStatements().stream() - .filter(J.VariableDeclarations.class::isInstance) - .map(J.VariableDeclarations.class::cast) - .filter(field -> field.hasModifier(J.Modifier.Type.Private)) - .collect(Collectors.toList()); - } - - private List getGetterMethods(J.ClassDeclaration classDecl) { - return classDecl.getBody().getStatements().stream() - .filter(J.MethodDeclaration.class::isInstance) - .map(J.MethodDeclaration.class::cast) - .filter(this::isGetterMethod) - .collect(Collectors.toList()); - } - - private boolean hasSetterMethods(J.ClassDeclaration classDecl) { - return classDecl.getBody().getStatements().stream() - .filter(J.MethodDeclaration.class::isInstance) - .map(J.MethodDeclaration.class::cast) - .anyMatch(this::isSetterMethod); - } - - private boolean isConstructor(J.MethodDeclaration method) { - return method.isConstructor(); - } - - private boolean isGetterMethod(J.MethodDeclaration method) { - String methodName = method.getSimpleName(); - return (methodName.startsWith("get") || methodName.startsWith("is")) - && method.getParameters().isEmpty() - && method.getReturnTypeExpression() != null; - } - - private boolean isSetterMethod(J.MethodDeclaration method) { - String methodName = method.getSimpleName(); - return methodName.startsWith("set") - && method.getParameters().size() == 1; - } - - private @Nullable String getFieldNameFromGetter(J.MethodDeclaration getter) { - String methodName = getter.getSimpleName(); - if (methodName.startsWith("get") && methodName.length() > 3) { - return Character.toLowerCase(methodName.charAt(3)) + methodName.substring(4); - } else if (methodName.startsWith("is") && methodName.length() > 2) { - return Character.toLowerCase(methodName.charAt(2)) + methodName.substring(3); - } - return null; - } - - private boolean matchesPackagePattern(J.ClassDeclaration classDecl) { - if (packagePattern == null || packagePattern.trim().isEmpty()) { - return true; // No pattern specified, match all - } - - String packageName = getPackageName(classDecl); - if (packageName == null) { - return packagePattern.equals("*"); // Default package - } - - // Simple glob pattern matching - String pattern = packagePattern.replace("**", ".*").replace("*", "[^.]*"); - return packageName.matches(pattern); - } - - private String getPackageName(J.ClassDeclaration classDecl) { - J.CompilationUnit cu = getCursor().firstEnclosing(J.CompilationUnit.class); - if (cu != null && cu.getPackageDeclaration() != null) { - return cu.getPackageDeclaration().getExpression().toString(); - } - return null; - } - - private String getFullyQualifiedName(J.ClassDeclaration classDecl) { - String packageName = getPackageName(classDecl); - if (packageName != null) { - return packageName + "." + classDecl.getSimpleName(); - } - return classDecl.getSimpleName(); - } - } -} diff --git a/src/test/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecordTest.java b/src/test/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecordTest.java deleted file mode 100644 index 169bc5bc4e..0000000000 --- a/src/test/java/org/openrewrite/java/migrate/ConvertImmutableClassToRecordTest.java +++ /dev/null @@ -1,821 +0,0 @@ -/* - * Copyright 2024 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.java.migrate; - -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 ConvertImmutableClassToRecordTest implements RewriteTest { - - @Override - public void defaults(RecipeSpec spec) { - spec.recipe(new ConvertImmutableClassToRecord(null)); - } - - @DocumentExample - @Test - void simpleImmutableClass() { - rewriteRun( - //language=java - java( - """ - class Person { - private final String name; - private final int age; - - public Person(String name, int age) { - this.name = name; - this.age = age; - } - - public String getName() { - return name; - } - - public int getAge() { - return age; - } - } - """, - """ - record Person(String name, int age) { - } - """ - ) - ); - } - - @Test - void completeClassToRecordTransformationWithMethodCallUpdates() { - rewriteRun( - //language=java - java( - """ - class Person { - private final String name; - private final int age; - - public Person(String name, int age) { - this.name = name; - this.age = age; - } - - public String getName() { - return name; - } - - public int getAge() { - return age; - } - - public String getDisplayName() { - return name + " (" + age + ")"; - } - } - """, - """ - record Person(String name, int age) { - public String getDisplayName() { - return name + " (" + age + ")"; - } - } - """ - ), - //language=java - java( - """ - class PersonService { - public void processPerson(Person person) { - String name = person.getName(); - int age = person.getAge(); - String display = person.getDisplayName(); - System.out.println("Processing: " + display); - } - } - """, - """ - class PersonService { - public void processPerson(Person person) { - String name = person.name(); - int age = person.age(); - String display = person.getDisplayName(); - System.out.println("Processing: " + display); - } - } - """ - ) - ); - } - - @Test - void packagePatternRestriction() { - rewriteRun( - spec -> spec.recipe(new ConvertImmutableClassToRecord("com.example.model.**")), - //language=java - java( - """ - package com.example.model; - - class Person { - private final String name; - - public Person(String name) { - this.name = name; - } - - public String getName() { - return name; - } - } - """, - """ - package com.example.model; - - record Person(String name) { - } - """ - ), - //language=java - java( - """ - package com.example.service; - - class User { - private final String username; - - public User(String username) { - this.username = username; - } - - public String getUsername() { - return username; - } - } - """ - // Should not be transformed due to package pattern restriction - ) - ); - } - - @Test - void multipleClassesTransformation() { - rewriteRun( - //language=java - java( - """ - class Address { - private final String street; - private final String city; - - public Address(String street, String city) { - this.street = street; - this.city = city; - } - - public String getStreet() { - return street; - } - - public String getCity() { - return city; - } - } - - class Person { - private final String name; - private final Address address; - - public Person(String name, Address address) { - this.name = name; - this.address = address; - } - - public String getName() { - return name; - } - - public Address getAddress() { - return address; - } - } - """, - """ - record Address(String street, String city) { - } - - record Person(String name, Address address) { - } - """ - ), - //language=java - java( - """ - class AddressService { - public void printFullAddress(Person person) { - Address addr = person.getAddress(); - String street = addr.getStreet(); - String city = addr.getCity(); - System.out.println(street + ", " + city); - } - } - """, - """ - class AddressService { - public void printFullAddress(Person person) { - Address addr = person.address(); - String street = addr.street(); - String city = addr.city(); - System.out.println(street + ", " + city); - } - } - """ - ) - ); - } - - @Test - void complexBusinessLogicPreservation() { - rewriteRun( - //language=java - java( - """ - import java.util.Objects; - - class Product { - private final String id; - private final String name; - private final double price; - - public Product(String id, String name, double price) { - this.id = id; - this.name = name; - this.price = price; - } - - public String getId() { - return id; - } - - public String getName() { - return name; - } - - public double getPrice() { - return price; - } - - public String getFormattedPrice() { - return String.format("$%.2f", price); - } - - public boolean isExpensive() { - return price > 100.0; - } - - public Product withDiscount(double discountPercent) { - double newPrice = price * (1 - discountPercent / 100); - return new Product(id, name, newPrice); - } - } - """, - """ - import java.util.Objects; - - record Product(String id, String name, double price) { - public String getFormattedPrice() { - return String.format("$%.2f", price); - } - - public boolean isExpensive() { - return price > 100.0; - } - - public Product withDiscount(double discountPercent) { - double newPrice = price * (1 - discountPercent / 100); - return new Product(id, name, newPrice); - } - } - """ - ) - ); - } - - @Test - void nestedClassesHandling() { - rewriteRun( - //language=java - java( - """ - class OuterClass { - private String value; - - static class InnerData { - private final String data; - private final int count; - - public InnerData(String data, int count) { - this.data = data; - this.count = count; - } - - public String getData() { - return data; - } - - public int getCount() { - return count; - } - } - - public void process(InnerData inner) { - String data = inner.getData(); - int count = inner.getCount(); - System.out.println(data + ": " + count); - } - } - """, - """ - class OuterClass { - private String value; - - static record InnerData(String data, int count) { - } - - public void process(InnerData inner) { - String data = inner.data(); - int count = inner.count(); - System.out.println(data + ": " + count); - } - } - """ - ) - ); - } - - @Test - void genericTypesSupport() { - rewriteRun( - //language=java - java( - """ - import java.util.List; - import java.util.Optional; - - class Container { - private final T value; - private final List items; - - public Container(T value, List items) { - this.value = value; - this.items = items; - } - - public T getValue() { - return value; - } - - public List getItems() { - return items; - } - - public Optional findFirst() { - return items.isEmpty() ? Optional.empty() : Optional.of(items.get(0)); - } - } - """, - """ - import java.util.List; - import java.util.Optional; - - record Container(T value, List items) { - public Optional findFirst() { - return items.isEmpty() ? Optional.empty() : Optional.of(items.get(0)); - } - } - """ - ) - ); - } - - @Test - void integrationWithStreamOperations() { - rewriteRun( - //language=java - java( - """ - import java.util.List; - import java.util.stream.Collectors; - - class Employee { - private final String name; - private final String department; - private final double salary; - - public Employee(String name, String department, double salary) { - this.name = name; - this.department = department; - this.salary = salary; - } - - public String getName() { - return name; - } - - public String getDepartment() { - return department; - } - - public double getSalary() { - return salary; - } - } - - class EmployeeService { - public List getNamesByDepartment(List employees, String dept) { - return employees.stream() - .filter(emp -> emp.getDepartment().equals(dept)) - .map(Employee::getName) - .collect(Collectors.toList()); - } - - public double getTotalSalary(List employees) { - return employees.stream() - .mapToDouble(Employee::getSalary) - .sum(); - } - } - """, - """ - import java.util.List; - import java.util.stream.Collectors; - - record Employee(String name, String department, double salary) { - } - - class EmployeeService { - public List getNamesByDepartment(List employees, String dept) { - return employees.stream() - .filter(emp -> emp.department().equals(dept)) - .map(Employee::name) - .collect(Collectors.toList()); - } - - public double getTotalSalary(List employees) { - return employees.stream() - .mapToDouble(Employee::salary) - .sum(); - } - } - """ - ) - ); - } - - @Test - void immutableClassWithAnnotations() { - rewriteRun( - //language=java - java( - """ - import javax.validation.constraints.NotNull; - import javax.validation.constraints.Min; - - @Entity - public class Person { - @NotNull - private final String name; - - @Min(0) - private final int age; - - public Person(String name, int age) { - this.name = name; - this.age = age; - } - - public String getName() { - return name; - } - - public int getAge() { - return age; - } - } - """, - """ - import javax.validation.constraints.NotNull; - import javax.validation.constraints.Min; - - @Entity - public record Person(@NotNull String name, @Min(0) int age) { - } - """ - ) - ); - } - - @Test - void booleanGetterWithIsPrefix() { - rewriteRun( - //language=java - java( - """ - class User { - private final String name; - private final boolean active; - - public User(String name, boolean active) { - this.name = name; - this.active = active; - } - - public String getName() { - return name; - } - - public boolean isActive() { - return active; - } - } - """, - """ - record User(String name, boolean active) { - } - """ - ) - ); - } - - @Test - void classWithCollectionFields() { - rewriteRun( - //language=java - java( - """ - import java.util.List; - import java.util.Set; - - class Student { - private final String name; - private final List courses; - private final Set skills; - - public Student(String name, List courses, Set skills) { - this.name = name; - this.courses = courses; - this.skills = skills; - } - - public String getName() { - return name; - } - - public List getCourses() { - return courses; - } - - public Set getSkills() { - return skills; - } - } - """, - """ - import java.util.List; - import java.util.Set; - - record Student(String name, List courses, Set skills) { - } - """ - ) - ); - } - - @Test - void classWithInterfaceImplementation() { - rewriteRun( - //language=java - java( - """ - interface Identifiable { - String getId(); - } - - class Entity implements Identifiable { - private final String id; - private final String name; - - public Entity(String id, String name) { - this.id = id; - this.name = name; - } - - public String getId() { - return id; - } - - public String getName() { - return name; - } - } - """, - """ - interface Identifiable { - String getId(); - } - - record Entity(String id, String name) implements Identifiable { - } - """ - ) - ); - } - - @Test - void shouldNotConvertClassWithSetterMethods() { - rewriteRun( - //language=java - java( - """ - class MutablePerson { - private String name; - private int age; - - public String getName() { - return name; - } - - public void setName(String name) { - this.name = name; - } - - public int getAge() { - return age; - } - - public void setAge(int age) { - this.age = age; - } - } - """ - ) - ); - } - - @Test - void shouldNotConvertClassThatExtendsAnotherClass() { - rewriteRun( - //language=java - java( - """ - class BaseEntity { - private final String id; - - public BaseEntity(String id) { - this.id = id; - } - - public String getId() { - return id; - } - } - - class Person extends BaseEntity { - private final String name; - - public Person(String id, String name) { - super(id); - this.name = name; - } - - public String getName() { - return name; - } - } - """ - ) - ); - } - - @Test - void shouldNotConvertClassWithoutGetters() { - rewriteRun( - //language=java - java( - """ - class DataHolder { - private final String data; - - public DataHolder(String data) { - this.data = data; - } - - public void processData() { - System.out.println(data); - } - } - """ - ) - ); - } - - @Test - void shouldNotConvertClassWithMissingGetters() { - rewriteRun( - //language=java - java( - """ - class PartialPerson { - private final String name; - private final int age; - - public PartialPerson(String name, int age) { - this.name = name; - this.age = age; - } - - public String getName() { - return name; - } - - // Missing getAge() method - } - """ - ) - ); - } - - @Test - void shouldNotConvertEnum() { - rewriteRun( - //language=java - java( - """ - enum Status { - ACTIVE, - INACTIVE; - - public boolean isActive() { - return this == ACTIVE; - } - } - """ - ) - ); - } - - @Test - void shouldNotConvertInterface() { - rewriteRun( - //language=java - java( - """ - interface Repository { - String getName(); - int getVersion(); - } - """ - ) - ); - } - - @Test - void shouldNotConvertAbstractClass() { - rewriteRun( - //language=java - java( - """ - abstract class AbstractEntity { - private final String id; - - public AbstractEntity(String id) { - this.id = id; - } - - public String getId() { - return id; - } - - public abstract void process(); - } - """ - ) - ); - } -} From 9d2a7982a3c1c8be4e0b4636aa2ae19a16b46b86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Fri, 17 Oct 2025 09:46:33 +0200 Subject: [PATCH 10/12] apply code review --- .../AbstractFindThreadLocals.java | 133 +++++++++++------- .../FindNeverMutatedThreadLocals.java | 4 +- .../FindThreadLocalsMutableFromOutside.java | 15 +- ...hreadLocalsMutatedOnlyInDefiningScope.java | 24 ++-- .../search/threadlocal/package-info.java | 21 +++ ...NeverMutatedThreadLocalsCrossFileTest.java | 2 +- .../FindNeverMutatedThreadLocalsTest.java | 2 +- ...indThreadLocalsMutableFromOutsideTest.java | 2 +- ...dLocalsMutatedOnlyInDefiningScopeTest.java | 2 +- 9 files changed, 130 insertions(+), 75 deletions(-) rename src/main/java/org/openrewrite/java/migrate/search/{ => threadlocal}/AbstractFindThreadLocals.java (78%) rename src/main/java/org/openrewrite/java/migrate/search/{ => threadlocal}/FindNeverMutatedThreadLocals.java (95%) rename src/main/java/org/openrewrite/java/migrate/search/{ => threadlocal}/FindThreadLocalsMutableFromOutside.java (86%) rename src/main/java/org/openrewrite/java/migrate/search/{ => threadlocal}/FindThreadLocalsMutatedOnlyInDefiningScope.java (78%) create mode 100644 src/main/java/org/openrewrite/java/migrate/search/threadlocal/package-info.java rename src/test/java/org/openrewrite/java/migrate/search/{ => threadlocal}/FindNeverMutatedThreadLocalsCrossFileTest.java (99%) rename src/test/java/org/openrewrite/java/migrate/search/{ => threadlocal}/FindNeverMutatedThreadLocalsTest.java (99%) rename src/test/java/org/openrewrite/java/migrate/search/{ => threadlocal}/FindThreadLocalsMutableFromOutsideTest.java (99%) rename src/test/java/org/openrewrite/java/migrate/search/{ => threadlocal}/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java (99%) diff --git a/src/main/java/org/openrewrite/java/migrate/search/AbstractFindThreadLocals.java b/src/main/java/org/openrewrite/java/migrate/search/threadlocal/AbstractFindThreadLocals.java similarity index 78% rename from src/main/java/org/openrewrite/java/migrate/search/AbstractFindThreadLocals.java rename to src/main/java/org/openrewrite/java/migrate/search/threadlocal/AbstractFindThreadLocals.java index 050df7dd2a..1b32173630 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/AbstractFindThreadLocals.java +++ b/src/main/java/org/openrewrite/java/migrate/search/threadlocal/AbstractFindThreadLocals.java @@ -13,8 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.openrewrite.java.migrate.search; +package org.openrewrite.java.migrate.search.threadlocal; +import lombok.Getter; import lombok.Value; import org.jspecify.annotations.Nullable; import org.openrewrite.ExecutionContext; @@ -23,6 +24,7 @@ import org.openrewrite.TreeVisitor; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.MethodMatcher; +import org.openrewrite.java.migrate.search.ThreadLocalTable; import org.openrewrite.java.search.UsesType; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; @@ -33,7 +35,9 @@ import java.nio.file.Path; import java.util.*; -import static org.openrewrite.Preconditions.*; +import static org.openrewrite.Preconditions.check; +import static org.openrewrite.Preconditions.or; + public abstract class AbstractFindThreadLocals extends ScanningRecipe { @@ -45,8 +49,7 @@ public abstract class AbstractFindThreadLocals extends ScanningRecipe initMutationPaths = new HashSet<>(); private final Set regularMutationPaths = new HashSet<>(); @@ -92,38 +99,37 @@ void setDeclaration(Path path, boolean priv, boolean stat, boolean fin) { this.declared = true; } + /** + * Records a mutation from an initialization context (constructor/static initializer). + */ void addInitMutation(Path path) { initMutationPaths.add(path); } + /** + * Records a regular (non-initialization) mutation. + */ void addRegularMutation(Path path) { regularMutationPaths.add(path); } - public boolean isPrivate() { - return isPrivate; - } - - public boolean isStatic() { - return isStatic; - } - - public boolean isFinal() { - return isFinal; - } - - public boolean isDeclared() { - return declared; - } - - public boolean hasAnyMutation() { - return !initMutationPaths.isEmpty() || !regularMutationPaths.isEmpty(); + /** + * Checks if there are no mutations (both init and regular). + */ + public boolean hasNoMutation() { + return initMutationPaths.isEmpty() && regularMutationPaths.isEmpty(); } + /** + * Checks if there are only mutations from initialization contexts (constructors/static initializers). + */ public boolean hasOnlyInitMutations() { return !initMutationPaths.isEmpty() && regularMutationPaths.isEmpty(); } + /** + * Checks if there are any mutations (both init and regular) from files other than the declaration file. + */ public boolean hasExternalMutations() { if (!declared || declarationPath == null) { return true; // Conservative @@ -134,6 +140,9 @@ public boolean hasExternalMutations() { regularMutationPaths.stream().anyMatch(p -> !p.equals(declarationPath)); } + /** + * Checks if all mutations (both init and regular) are from the same file as the declaration. + */ public boolean isOnlyLocallyMutated() { if (!declared || declarationPath == null) { return false; @@ -208,7 +217,7 @@ public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, Execu return method; } - @Nullable String fqn = getFieldFullyQualifiedName(method.getSelect()); + String fqn = getFieldFullyQualifiedName(method.getSelect()); if (fqn == null) { return method; } @@ -228,7 +237,7 @@ public J.Assignment visitAssignment(J.Assignment assignment, ExecutionContext ct return assignment; } - @Nullable String fqn = getFieldFullyQualifiedName(assignment.getVariable()); + String fqn = getFieldFullyQualifiedName(assignment.getVariable()); if (fqn == null) { return assignment; } @@ -256,9 +265,10 @@ private boolean isInInitializationContext() { private boolean isThreadLocalFieldAccess(Expression expression) { if (expression instanceof J.Identifier) { - return isThreadLocalType(((J.Identifier) expression).getType()); - } else if (expression instanceof J.FieldAccess) { - return isThreadLocalType(((J.FieldAccess) expression).getType()); + return isThreadLocalType(expression.getType()); + } + if (expression instanceof J.FieldAccess) { + return isThreadLocalType(expression.getType()); } return false; } @@ -279,7 +289,7 @@ private boolean isThreadLocalFieldAccess(Expression expression) { return null; } - @Nullable JavaType owner = varType.getOwner(); + JavaType owner = varType.getOwner(); if (!(owner instanceof JavaType.FullyQualified)) { return null; } @@ -294,22 +304,19 @@ private boolean isThreadLocalFieldAccess(Expression expression) { public TreeVisitor getVisitor(ThreadLocalAccumulator acc) { return check(acc.hasDeclarations(), new JavaIsoVisitor() { - @Override - public J.CompilationUnit visitCompilationUnit(J.CompilationUnit cu, ExecutionContext ctx) { - if (dataTable == null) { - dataTable = new ThreadLocalTable(AbstractFindThreadLocals.this); - } - return super.visitCompilationUnit(cu, ctx); - } + @Override public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations multiVariable, ExecutionContext ctx) { multiVariable = super.visitVariableDeclarations(multiVariable, ctx); J.ClassDeclaration classDecl = getCursor().firstEnclosing(J.ClassDeclaration.class); + if(classDecl == null) { + return multiVariable; + } for (J.VariableDeclarations.NamedVariable variable : multiVariable.getVariables()) { - if (isThreadLocalType(variable.getType()) && classDecl != null) { + if (isThreadLocalType(variable.getType())) { String className = classDecl.getType() != null ? classDecl.getType().getFullyQualifiedName() : "UnknownClass"; String fieldName = variable.getName().getSimpleName(); @@ -320,18 +327,15 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m String message = getMessage(info); String mutationType = getMutationType(info); - // Add to data table - if (dataTable != null) { - dataTable.insertRow(ctx, new ThreadLocalTable.Row( - getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath().toString(), - className, - fieldName, - getAccessModifier(multiVariable), - getModifiers(multiVariable), - mutationType, - message - )); - } + dataTable.insertRow(ctx, new ThreadLocalTable.Row( + getCursor().firstEnclosingOrThrow(SourceFile.class).getSourcePath().toString(), + className, + fieldName, + getAccessModifier(multiVariable), + getFieldModifiers(multiVariable), + mutationType, + message + )); return SearchResult.found(multiVariable, message); } @@ -344,15 +348,17 @@ public J.VariableDeclarations visitVariableDeclarations(J.VariableDeclarations m private String getAccessModifier(J.VariableDeclarations variableDecls) { if (variableDecls.hasModifier(J.Modifier.Type.Private)) { return "private"; - } else if (variableDecls.hasModifier(J.Modifier.Type.Protected)) { + } + if (variableDecls.hasModifier(J.Modifier.Type.Protected)) { return "protected"; - } else if (variableDecls.hasModifier(J.Modifier.Type.Public)) { + } + if (variableDecls.hasModifier(J.Modifier.Type.Public)) { return "public"; } return "package-private"; } - private String getModifiers(J.VariableDeclarations variableDecls) { + private String getFieldModifiers(J.VariableDeclarations variableDecls) { List mods = new ArrayList<>(); if (variableDecls.hasModifier(J.Modifier.Type.Static)) { mods.add("static"); @@ -366,8 +372,33 @@ private String getModifiers(J.VariableDeclarations variableDecls) { }); } + /** + * Determines whether a ThreadLocal should be marked based on its usage info. + * Implementations should define the criteria for marking. + * It is used to decide if a ThreadLocal variable should be highlighted in the results. + * If an expected ThreadLocal instance is missing from the results, consider adjusting this method. + * + * @param info The ThreadLocalInfo containing usage details. + * @return true if the ThreadLocal should be marked, false otherwise. + */ protected abstract boolean shouldMarkThreadLocal(ThreadLocalInfo info); + /** + * Generates a descriptive message about the ThreadLocal's usage pattern. + * Implementations should provide context-specific messages. + * It is used to receive the Markers message and the Data Tables detailed message. + * + * @param info The ThreadLocalInfo containing usage details. + * @return A string message describing the ThreadLocal's usage. + */ protected abstract String getMessage(ThreadLocalInfo info); + /** + * Determines the mutation type of the ThreadLocal based on its usage info. + * Implementations should define the mutation categories. + * It is used to populate the Data Tables human-readable mutation type column. + * + * @param info The ThreadLocalInfo containing usage details. + * @return A string representing the mutation type. + */ protected abstract String getMutationType(ThreadLocalInfo info); protected static boolean isThreadLocalType(@Nullable JavaType type) { diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocals.java b/src/main/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocals.java similarity index 95% rename from src/main/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocals.java rename to src/main/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocals.java index eccdc18d69..728040176b 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocals.java +++ b/src/main/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocals.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.openrewrite.java.migrate.search; +package org.openrewrite.java.migrate.search.threadlocal; import lombok.EqualsAndHashCode; import lombok.Value; @@ -46,7 +46,7 @@ public Set getTags() { @Override protected boolean shouldMarkThreadLocal(ThreadLocalInfo info) { // Mark ThreadLocals that have no mutations at all - return !info.hasAnyMutation(); + return info.hasNoMutation(); } @Override diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutside.java b/src/main/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutableFromOutside.java similarity index 86% rename from src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutside.java rename to src/main/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutableFromOutside.java index 372bdd9bac..b6c0909640 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutside.java +++ b/src/main/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutableFromOutside.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.openrewrite.java.migrate.search; +package org.openrewrite.java.migrate.search.threadlocal; import lombok.EqualsAndHashCode; import lombok.Value; @@ -33,6 +33,7 @@ public String getDisplayName() { @Override public String getDescription() { + //language=markdown return "Find `ThreadLocal` variables that can be mutated from outside their defining class. " + "These ThreadLocals have the highest risk as they can be modified by any code with access to them. " + "This includes non-private ThreadLocals or those mutated from other classes in the codebase."; @@ -55,19 +56,19 @@ protected boolean shouldMarkThreadLocal(ThreadLocalInfo info) { protected String getMessage(ThreadLocalInfo info) { if (info.hasExternalMutations()) { return "ThreadLocal is mutated from outside its defining class"; - } else { - // Non-private but not currently mutated externally - String access = info.isStatic() ? "static " : ""; - return "ThreadLocal is " + access + "non-private and can potentially be mutated from outside"; } + + // Non-private but not currently mutated externally + String access = info.isStatic() ? "static " : ""; + return "ThreadLocal is " + access + "non-private and can potentially be mutated from outside"; } @Override protected String getMutationType(ThreadLocalInfo info) { if (info.hasExternalMutations()) { return "Mutated externally"; - } else { - return "Potentially mutable"; } + + return "Potentially mutable"; } } diff --git a/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScope.java b/src/main/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutatedOnlyInDefiningScope.java similarity index 78% rename from src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScope.java rename to src/main/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutatedOnlyInDefiningScope.java index a25b15e6f8..844b5c88da 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScope.java +++ b/src/main/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutatedOnlyInDefiningScope.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.openrewrite.java.migrate.search; +package org.openrewrite.java.migrate.search.threadlocal; import lombok.EqualsAndHashCode; import lombok.Value; @@ -33,6 +33,7 @@ public String getDisplayName() { @Override public String getDescription() { + //language=markdown return "Find `ThreadLocal` variables that are only mutated within their defining class or initialization context (constructor/static initializer). " + "These may be candidates for refactoring as they have limited mutation scope. " + "The recipe identifies `ThreadLocal` variables that are only modified during initialization or within their declaring class."; @@ -45,29 +46,30 @@ public Set getTags() { @Override protected boolean shouldMarkThreadLocal(ThreadLocalInfo info) { - // Early return for ThreadLocals without mutations - if (!info.hasAnyMutation()) { + if (info.hasNoMutation()) { return false; } - // Early return for non-private ThreadLocals if (!info.isPrivate()) { return false; } - // Mark if only locally mutated return info.isOnlyLocallyMutated(); } @Override protected String getMessage(ThreadLocalInfo info) { - return info.hasOnlyInitMutations() - ? "ThreadLocal is only mutated during initialization (constructor/static initializer)" - : "ThreadLocal is only mutated within its defining class"; + if (info.hasOnlyInitMutations()) { + return "ThreadLocal is only mutated during initialization (constructor/static initializer)"; + } + + return "ThreadLocal is only mutated within its defining class"; } @Override protected String getMutationType(ThreadLocalInfo info) { - return info.hasOnlyInitMutations() - ? "Mutated only in initialization" - : "Mutated in defining class"; + if (info.hasOnlyInitMutations()) { + return "Mutated only in initialization"; + } + + return "Mutated in defining class"; } } diff --git a/src/main/java/org/openrewrite/java/migrate/search/threadlocal/package-info.java b/src/main/java/org/openrewrite/java/migrate/search/threadlocal/package-info.java new file mode 100644 index 0000000000..34bcf91372 --- /dev/null +++ b/src/main/java/org/openrewrite/java/migrate/search/threadlocal/package-info.java @@ -0,0 +1,21 @@ +/* + * Copyright 2024 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. + */ +@NullMarked +@NonNullFields +package org.openrewrite.java.migrate.search.threadlocal; + +import org.jspecify.annotations.NullMarked; +import org.openrewrite.internal.lang.NonNullFields; diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocalsCrossFileTest.java b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocalsCrossFileTest.java similarity index 99% rename from src/test/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocalsCrossFileTest.java rename to src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocalsCrossFileTest.java index 1984076117..d19906a99d 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocalsCrossFileTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocalsCrossFileTest.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.openrewrite.java.migrate.search; +package org.openrewrite.java.migrate.search.threadlocal; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocalsTest.java b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocalsTest.java similarity index 99% rename from src/test/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocalsTest.java rename to src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocalsTest.java index a33f49f7bb..03f2add0a1 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/FindNeverMutatedThreadLocalsTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocalsTest.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.openrewrite.java.migrate.search; +package org.openrewrite.java.migrate.search.threadlocal; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutsideTest.java b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutableFromOutsideTest.java similarity index 99% rename from src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutsideTest.java rename to src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutableFromOutsideTest.java index 3ef34db3f1..56308de926 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutableFromOutsideTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutableFromOutsideTest.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.openrewrite.java.migrate.search; +package org.openrewrite.java.migrate.search.threadlocal; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; diff --git a/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java similarity index 99% rename from src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java rename to src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java index ce7693ccb3..d496183db8 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.openrewrite.java.migrate.search; +package org.openrewrite.java.migrate.search.threadlocal; import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; From e6b2aff677571ff15c9f6bb7d1f9f7e74553ee7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Thu, 15 Jan 2026 10:05:21 +0100 Subject: [PATCH 11/12] Move ThreadLocalTable to table package and add data table tests - Moved ThreadLocalTable from search/ to table/ package to align with repository conventions - Added data table verification tests to all four ThreadLocal recipe test files - Updated import in AbstractFindThreadLocals to use new package location --- .../threadlocal/AbstractFindThreadLocals.java | 2 +- .../{search => table}/ThreadLocalTable.java | 2 +- ...NeverMutatedThreadLocalsCrossFileTest.java | 39 +++++++++++++++++++ .../FindNeverMutatedThreadLocalsTest.java | 28 +++++++++++++ ...indThreadLocalsMutableFromOutsideTest.java | 27 +++++++++++++ ...dLocalsMutatedOnlyInDefiningScopeTest.java | 35 +++++++++++++++++ 6 files changed, 131 insertions(+), 2 deletions(-) rename src/main/java/org/openrewrite/java/migrate/{search => table}/ThreadLocalTable.java (98%) diff --git a/src/main/java/org/openrewrite/java/migrate/search/threadlocal/AbstractFindThreadLocals.java b/src/main/java/org/openrewrite/java/migrate/search/threadlocal/AbstractFindThreadLocals.java index 1b32173630..0bbbfdeda2 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/threadlocal/AbstractFindThreadLocals.java +++ b/src/main/java/org/openrewrite/java/migrate/search/threadlocal/AbstractFindThreadLocals.java @@ -24,7 +24,7 @@ import org.openrewrite.TreeVisitor; import org.openrewrite.java.JavaIsoVisitor; import org.openrewrite.java.MethodMatcher; -import org.openrewrite.java.migrate.search.ThreadLocalTable; +import org.openrewrite.java.migrate.table.ThreadLocalTable; import org.openrewrite.java.search.UsesType; import org.openrewrite.java.tree.Expression; import org.openrewrite.java.tree.J; diff --git a/src/main/java/org/openrewrite/java/migrate/search/ThreadLocalTable.java b/src/main/java/org/openrewrite/java/migrate/table/ThreadLocalTable.java similarity index 98% rename from src/main/java/org/openrewrite/java/migrate/search/ThreadLocalTable.java rename to src/main/java/org/openrewrite/java/migrate/table/ThreadLocalTable.java index 13051ccaf5..9b69be14e4 100644 --- a/src/main/java/org/openrewrite/java/migrate/search/ThreadLocalTable.java +++ b/src/main/java/org/openrewrite/java/migrate/table/ThreadLocalTable.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.openrewrite.java.migrate.search; +package org.openrewrite.java.migrate.table; import lombok.Value; import org.openrewrite.Column; diff --git a/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocalsCrossFileTest.java b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocalsCrossFileTest.java index d19906a99d..1ab12ea666 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocalsCrossFileTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocalsCrossFileTest.java @@ -17,9 +17,11 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; +import org.openrewrite.java.migrate.table.ThreadLocalTable; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.java.Assertions.java; class FindNeverMutatedThreadLocalsCrossFileTest implements RewriteTest { @@ -416,4 +418,41 @@ public void mutate(InstanceThreadLocalHolder holder) { ); // Instance ThreadLocal should NOT be marked as immutable due to external mutation } + + @Test + void verifyDataTableOutputCrossFile() { + rewriteRun( + spec -> spec.dataTable(ThreadLocalTable.Row.class, rows -> { + assertThat(rows).hasSize(1); + assertThat(rows.get(0).getClassName()).isEqualTo("com.example.Holder"); + assertThat(rows.get(0).getFieldName()).isEqualTo("TL"); + assertThat(rows.get(0).getMutationType()).isEqualTo("Never mutated"); + }), + java( + """ + package com.example; + + class Holder { + private static final ThreadLocal TL = new ThreadLocal<>(); + } + """, + """ + package com.example; + + class Holder { + /*~~(ThreadLocal is never mutated and could be replaced with ScopedValue)~~>*/private static final ThreadLocal TL = new ThreadLocal<>(); + } + """ + ), + java( + """ + package com.example; + + class Reader { + // Only reads, no mutation + } + """ + ) + ); + } } diff --git a/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocalsTest.java b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocalsTest.java index 03f2add0a1..4c8fa83527 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocalsTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindNeverMutatedThreadLocalsTest.java @@ -17,9 +17,11 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; +import org.openrewrite.java.migrate.table.ThreadLocalTable; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.java.Assertions.java; class FindNeverMutatedThreadLocalsTest implements RewriteTest { @@ -347,4 +349,30 @@ public String getValue() { ) ); } + + @Test + void verifyDataTableOutput() { + rewriteRun( + spec -> spec.dataTable(ThreadLocalTable.Row.class, rows -> { + assertThat(rows).hasSize(1); + assertThat(rows.get(0).getClassName()).isEqualTo("Example"); + assertThat(rows.get(0).getFieldName()).isEqualTo("TL"); + assertThat(rows.get(0).getAccessModifier()).isEqualTo("private"); + assertThat(rows.get(0).getModifiers()).isEqualTo("static final"); + assertThat(rows.get(0).getMutationType()).isEqualTo("Never mutated"); + }), + java( + """ + class Example { + private static final ThreadLocal TL = new ThreadLocal<>(); + } + """, + """ + class Example { + /*~~(ThreadLocal is never mutated and could be replaced with ScopedValue)~~>*/private static final ThreadLocal TL = new ThreadLocal<>(); + } + """ + ) + ); + } } diff --git a/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutableFromOutsideTest.java b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutableFromOutsideTest.java index 56308de926..cc6feb8140 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutableFromOutsideTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutableFromOutsideTest.java @@ -17,9 +17,11 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; +import org.openrewrite.java.migrate.table.ThreadLocalTable; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.java.Assertions.java; class FindThreadLocalsMutableFromOutsideTest implements RewriteTest { @@ -251,4 +253,29 @@ public void modifyThreadLocal() { ) ); } + + @Test + void verifyDataTableOutput() { + rewriteRun( + spec -> spec.dataTable(ThreadLocalTable.Row.class, rows -> { + assertThat(rows).hasSize(1); + assertThat(rows.get(0).getClassName()).isEqualTo("Example"); + assertThat(rows.get(0).getFieldName()).isEqualTo("PUBLIC_TL"); + assertThat(rows.get(0).getAccessModifier()).isEqualTo("public"); + assertThat(rows.get(0).getMutationType()).isEqualTo("Potentially mutable"); + }), + java( + """ + class Example { + public static final ThreadLocal PUBLIC_TL = new ThreadLocal<>(); + } + """, + """ + class Example { + /*~~(ThreadLocal is static non-private and can potentially be mutated from outside)~~>*/public static final ThreadLocal PUBLIC_TL = new ThreadLocal<>(); + } + """ + ) + ); + } } diff --git a/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java index d496183db8..9eb36aee0e 100644 --- a/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java +++ b/src/test/java/org/openrewrite/java/migrate/search/threadlocal/FindThreadLocalsMutatedOnlyInDefiningScopeTest.java @@ -17,9 +17,11 @@ import org.junit.jupiter.api.Test; import org.openrewrite.DocumentExample; +import org.openrewrite.java.migrate.table.ThreadLocalTable; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; +import static org.assertj.core.api.Assertions.assertThat; import static org.openrewrite.java.Assertions.java; class FindThreadLocalsMutatedOnlyInDefiningScopeTest implements RewriteTest { @@ -251,4 +253,37 @@ public String getValue() { ) ); } + + @Test + void verifyDataTableOutput() { + rewriteRun( + spec -> spec.dataTable(ThreadLocalTable.Row.class, rows -> { + assertThat(rows).hasSize(1); + assertThat(rows.get(0).getClassName()).isEqualTo("Example"); + assertThat(rows.get(0).getFieldName()).isEqualTo("TL"); + assertThat(rows.get(0).getAccessModifier()).isEqualTo("private"); + assertThat(rows.get(0).getMutationType()).isEqualTo("Mutated only in initialization"); + }), + java( + """ + class Example { + private static final ThreadLocal TL = new ThreadLocal<>(); + + static { + TL.set("initial"); + } + } + """, + """ + class Example { + /*~~(ThreadLocal is only mutated during initialization (constructor/static initializer))~~>*/private static final ThreadLocal TL = new ThreadLocal<>(); + + static { + TL.set("initial"); + } + } + """ + ) + ); + } } From 21e10490e4778cfd70bde77c15dea2f7b288e5ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Merlin=20B=C3=B6gershausen?= Date: Thu, 15 Jan 2026 10:08:46 +0100 Subject: [PATCH 12/12] Add ThreadLocal recipes to recipe catalog --- src/main/resources/META-INF/rewrite/recipes.csv | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/resources/META-INF/rewrite/recipes.csv b/src/main/resources/META-INF/rewrite/recipes.csv index 8ec636813e..91be24dab5 100644 --- a/src/main/resources/META-INF/rewrite/recipes.csv +++ b/src/main/resources/META-INF/rewrite/recipes.csv @@ -397,6 +397,9 @@ maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.s maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.search.FindInternalJavaxApis,Find uses of internal javax APIs,The libraries that define these APIs will have to be migrated before any of the repositories that use them.,1,,Search,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,"[{""name"":""methodPattern"",""type"":""String"",""displayName"":""Method pattern"",""description"":""Optionally limit the search to declarations that match the provided method pattern."",""example"":""java.util.List add(..)""}]","[{""name"":""org.openrewrite.java.table.MethodCalls"",""displayName"":""Method calls"",""description"":""The text of matching method invocations."",""columns"":[{""name"":""sourceFile"",""type"":""String"",""displayName"":""Source file"",""description"":""The source file that the method call occurred in.""},{""name"":""method"",""type"":""String"",""displayName"":""Method call"",""description"":""The text of the method call.""},{""name"":""className"",""type"":""String"",""displayName"":""Class name"",""description"":""The class name of the method call.""},{""name"":""methodName"",""type"":""String"",""displayName"":""Method name"",""description"":""The method name of the method call.""},{""name"":""argumentTypes"",""type"":""String"",""displayName"":""Argument types"",""description"":""The argument types of the method call.""}]}]" maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.search.FindJavaVersion,Find Java versions in use,Finds Java versions in use.,1,,Search,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,,"[{""name"":""org.openrewrite.java.migrate.table.JavaVersionTable"",""displayName"":""Java version table"",""description"":""Records versions of Java in use"",""columns"":[{""name"":""sourceVersion"",""type"":""String"",""displayName"":""Source compatibility"",""description"":""The version of Java used to compile the source code""},{""name"":""targetVersion"",""type"":""String"",""displayName"":""Target compatibility"",""description"":""The version of Java the bytecode is compiled to run on""}]}]" maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.search.PlanJavaMigration,Plan a Java version migration,Study the set of Java versions and associated tools in use across many repositories.,1,,Search,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,,"[{""name"":""org.openrewrite.java.migrate.table.JavaVersionMigrationPlan"",""displayName"":""Java version migration plan"",""description"":""A per-repository view of the current state of Java versions and associated build tools"",""columns"":[{""name"":""hasJava"",""type"":""boolean"",""displayName"":""Has Java"",""description"":""Whether this is a Java repository at all.""},{""name"":""sourceCompatibility"",""type"":""String"",""displayName"":""Source compatibility"",""description"":""The source compatibility of the source file.""},{""name"":""majorVersionSourceCompatibility"",""type"":""Integer"",""displayName"":""Major version source compatibility"",""description"":""The major version.""},{""name"":""targetCompatibility"",""type"":""String"",""displayName"":""Target compatibility"",""description"":""The target compatibility or `--release` version of the source file.""},{""name"":""gradleVersion"",""type"":""String"",""displayName"":""Gradle version"",""description"":""The version of Gradle in use, if any.""},{""name"":""hasGradleBuild"",""type"":""Boolean"",""displayName"":""Has Gradle build"",""description"":""Whether a build.gradle file exists in the repository.""},{""name"":""mavenVersion"",""type"":""String"",""displayName"":""Maven version"",""description"":""The version of Maven in use, if any.""},{""name"":""hasMavenPom"",""type"":""Boolean"",""displayName"":""Has Maven pom"",""description"":""Whether a pom.xml file exists in the repository.""}]}]" +maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.search.threadlocal.FindNeverMutatedThreadLocals,Find ThreadLocal variables that are never mutated,Find `ThreadLocal` variables that are never mutated after initialization. These are prime candidates for migration to `ScopedValue` in Java 25+ as they are effectively immutable. The recipe identifies `ThreadLocal` variables that are only initialized but never reassigned or modified through `set()` or `remove()` methods.,1,Threadlocal,Search,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,,"[{""name"":""org.openrewrite.java.migrate.table.ThreadLocalTable"",""displayName"":""ThreadLocal usage"",""description"":""ThreadLocal variables and their mutation patterns."",""columns"":[{""name"":""sourceFile"",""type"":""String"",""displayName"":""Source file"",""description"":""The source file containing the ThreadLocal declaration.""},{""name"":""className"",""type"":""String"",""displayName"":""Class name"",""description"":""The fully qualified class name where the ThreadLocal is declared.""},{""name"":""fieldName"",""type"":""String"",""displayName"":""Field name"",""description"":""The name of the ThreadLocal field.""},{""name"":""accessModifier"",""type"":""String"",""displayName"":""Access modifier"",""description"":""The access modifier of the ThreadLocal field (private, protected, public, package-private).""},{""name"":""modifiers"",""type"":""String"",""displayName"":""Field modifiers"",""description"":""Additional modifiers like static, final.""},{""name"":""mutationType"",""type"":""String"",""displayName"":""Mutation type"",""description"":""Type of mutation detected (Never mutated, Mutated only in initialization, Mutated in defining class, Mutated externally, Potentially mutable).""},{""name"":""message"",""type"":""String"",""displayName"":""Message"",""description"":""Detailed message about the ThreadLocal's usage pattern.""}]}]" +maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.search.threadlocal.FindThreadLocalsMutableFromOutside,Find ThreadLocal variables mutable from outside their defining class,Find `ThreadLocal` variables that can be mutated from outside their defining class. These ThreadLocals have the highest risk as they can be modified by any code with access to them. This includes non-private ThreadLocals or those mutated from other classes in the codebase.,1,Threadlocal,Search,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,,"[{""name"":""org.openrewrite.java.migrate.table.ThreadLocalTable"",""displayName"":""ThreadLocal usage"",""description"":""ThreadLocal variables and their mutation patterns."",""columns"":[{""name"":""sourceFile"",""type"":""String"",""displayName"":""Source file"",""description"":""The source file containing the ThreadLocal declaration.""},{""name"":""className"",""type"":""String"",""displayName"":""Class name"",""description"":""The fully qualified class name where the ThreadLocal is declared.""},{""name"":""fieldName"",""type"":""String"",""displayName"":""Field name"",""description"":""The name of the ThreadLocal field.""},{""name"":""accessModifier"",""type"":""String"",""displayName"":""Access modifier"",""description"":""The access modifier of the ThreadLocal field (private, protected, public, package-private).""},{""name"":""modifiers"",""type"":""String"",""displayName"":""Field modifiers"",""description"":""Additional modifiers like static, final.""},{""name"":""mutationType"",""type"":""String"",""displayName"":""Mutation type"",""description"":""Type of mutation detected (Never mutated, Mutated only in initialization, Mutated in defining class, Mutated externally, Potentially mutable).""},{""name"":""message"",""type"":""String"",""displayName"":""Message"",""description"":""Detailed message about the ThreadLocal's usage pattern.""}]}]" +maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.search.threadlocal.FindThreadLocalsMutatedOnlyInDefiningScope,Find ThreadLocal variables mutated only in their defining scope,Find `ThreadLocal` variables that are only mutated within their defining class or initialization context (constructor/static initializer). These may be candidates for refactoring as they have limited mutation scope. The recipe identifies `ThreadLocal` variables that are only modified during initialization or within their declaring class.,1,Threadlocal,Search,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,,"[{""name"":""org.openrewrite.java.migrate.table.ThreadLocalTable"",""displayName"":""ThreadLocal usage"",""description"":""ThreadLocal variables and their mutation patterns."",""columns"":[{""name"":""sourceFile"",""type"":""String"",""displayName"":""Source file"",""description"":""The source file containing the ThreadLocal declaration.""},{""name"":""className"",""type"":""String"",""displayName"":""Class name"",""description"":""The fully qualified class name where the ThreadLocal is declared.""},{""name"":""fieldName"",""type"":""String"",""displayName"":""Field name"",""description"":""The name of the ThreadLocal field.""},{""name"":""accessModifier"",""type"":""String"",""displayName"":""Access modifier"",""description"":""The access modifier of the ThreadLocal field (private, protected, public, package-private).""},{""name"":""modifiers"",""type"":""String"",""displayName"":""Field modifiers"",""description"":""Additional modifiers like static, final.""},{""name"":""mutationType"",""type"":""String"",""displayName"":""Mutation type"",""description"":""Type of mutation detected (Never mutated, Mutated only in initialization, Mutated in defining class, Mutated externally, Potentially mutable).""},{""name"":""message"",""type"":""String"",""displayName"":""Message"",""description"":""Detailed message about the ThreadLocal's usage pattern.""}]}]" maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.sql.MigrateDriverManagerSetLogStream,Use `DriverManager#setLogWriter(java.io.PrintWriter)`,Use `DriverManager#setLogWriter(java.io.PrintWriter)` instead of the deprecated `DriverManager#setLogStream(java.io.PrintStream)` in Java 1.2 or higher.,1,,`java.sql` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.sql.JavaSqlAPIs,Use modernized `java.sql` APIs,"Certain Java sql APIs have become deprecated and their usages changed, necessitating usage changes.",3,,`java.sql` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,, maven,org.openrewrite.recipe:rewrite-migrate-java,org.openrewrite.java.migrate.util.IteratorNext,Replace `iterator().next()` with `getFirst()`,Replace `SequencedCollection.iterator().next()` with `getFirst()`.,1,,`java.util` APIs,Modernize,Java,,,Modernize your code to best use the project's current JDK version. Take advantage of newly available APIs and reduce the dependency of your code on third party dependencies where there is equivalent functionality in the Java standard library.,Basic building blocks for transforming Java code.,,