From 2d3d0345305da347213878f7a8f4226a0dd93b90 Mon Sep 17 00:00:00 2001 From: e5LA <208197507+e5LA@users.noreply.github.com> Date: Mon, 21 Jul 2025 21:44:41 +0200 Subject: [PATCH 1/4] feat: add FluentSetterRecipe --- .../staticanalysis/FluentSetterRecipe.java | 224 ++++++++ .../FluentSetterRecipeTest.java | 538 ++++++++++++++++++ 2 files changed, 762 insertions(+) create mode 100644 src/main/java/org/openrewrite/staticanalysis/FluentSetterRecipe.java create mode 100644 src/test/java/org/openrewrite/staticanalysis/FluentSetterRecipeTest.java diff --git a/src/main/java/org/openrewrite/staticanalysis/FluentSetterRecipe.java b/src/main/java/org/openrewrite/staticanalysis/FluentSetterRecipe.java new file mode 100644 index 000000000..471fafd68 --- /dev/null +++ b/src/main/java/org/openrewrite/staticanalysis/FluentSetterRecipe.java @@ -0,0 +1,224 @@ +/* + * Copyright 2025 the original author or authors. + *

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

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

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import lombok.EqualsAndHashCode; +import lombok.Value; +import org.jspecify.annotations.Nullable; +import org.openrewrite.ExecutionContext; +import org.openrewrite.Option; +import org.openrewrite.Recipe; +import org.openrewrite.internal.ListUtils; +import org.openrewrite.java.JavaIsoVisitor; +import org.openrewrite.java.tree.J; +import org.openrewrite.java.tree.JavaType; +import org.openrewrite.java.tree.Space; +import org.openrewrite.java.tree.Statement; +import org.openrewrite.marker.Markers; + +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.regex.Pattern; + +import static java.util.Collections.emptyList; +import static org.openrewrite.Tree.randomId; + +@EqualsAndHashCode(callSuper = false) +@Value +public class FluentSetterRecipe extends Recipe { + + @Option(displayName = "Include all void methods", description = + "Whether to convert all void methods to return `this`, not just setters. " + + "When false, only methods matching setter patterns will be converted.", required = false) + @Nullable + Boolean includeAllVoidMethods; + + @Option(displayName = "Method name pattern", description = + "A regular expression pattern to match method names. " + + "Only methods matching this pattern will be converted. " + + "Defaults to setter pattern when includeAllVoidMethods is false.", example = "set.*", required = false) + @Nullable + String methodNamePattern; + + @Option(displayName = "Exclude method patterns", description = + "A regular expression pattern for method names to exclude from conversion. " + + "Methods matching this pattern will not be converted.", example = "main|run", required = false) + @Nullable + String excludeMethodPattern; + + @Override + public String getDisplayName() { + return "Convert setters to return `this` for fluent interfaces"; + } + + @Override + public String getDescription() { + return "Converts void setter methods (and optionally other void methods) to return `this` " + + "to enable method chaining and fluent interfaces."; + } + + @Override + public JavaIsoVisitor getVisitor() { + return new JavaIsoVisitor() { + + @Override + public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, + ExecutionContext ctx) { + method = super.visitMethodDeclaration(method, ctx); + if (!shouldConvertMethod(method)) { + return method; + } + + J.ClassDeclaration containingClass = getCursor().firstEnclosing(J.ClassDeclaration.class); + if (containingClass == null || containingClass.getType() == null) { + return method; + } + + JavaType.FullyQualified classType = containingClass.getType(); + String className = classType.getClassName(); + if (className.contains(".")) { + className = className.substring(className.lastIndexOf('.') + 1); + } + + Space returnTypeSpace = method.getReturnTypeExpression() == null ? Space.EMPTY + : method.getReturnTypeExpression().getPrefix(); + Markers returnTypeMarkers = method.getReturnTypeExpression() == null ? Markers.EMPTY + : method.getReturnTypeExpression().getMarkers(); + + J.Identifier returnTypeIdentifier = new J.Identifier( + randomId(), + returnTypeSpace, + returnTypeMarkers, + emptyList(), + className, + classType, + null + ); + + J.MethodDeclaration updatedMethod = method + .withReturnTypeExpression(returnTypeIdentifier); + + if (updatedMethod.getBody() != null) { + Space indentation; + List statements = updatedMethod.getBody().getStatements(); + if (!statements.isEmpty()) { + indentation = statements.get(statements.size() - 1).getPrefix(); + } else { + indentation = updatedMethod.getBody().getPrefix(); + } + + J.Return returnThis = new J.Return( + randomId(), + Space.format("\n" + indentation.getIndent()), + Markers.EMPTY, + new J.Identifier( + randomId(), + Space.SINGLE_SPACE, + Markers.EMPTY, + emptyList(), + "this", + classType, + null + ) + ); + + updatedMethod = updatedMethod.withBody( + updatedMethod.getBody().withStatements( + ListUtils.concat(updatedMethod.getBody().getStatements(), returnThis) + ) + ); + } + + return updatedMethod; + } + + + private boolean shouldConvertMethod(J.MethodDeclaration method) { + if (method.getReturnTypeExpression() == null + || method.getReturnTypeExpression().getType() != JavaType.Primitive.Void) { + return false; + } + + if (method.hasModifier(J.Modifier.Type.Static)) { + return false; + } + + if (method.hasModifier(J.Modifier.Type.Abstract) || method.getBody() == null) { + return false; + } + + if (method.isConstructor()) { + return false; + } + + if (method.getBody() != null && hasReturnStatement(method.getBody())) { + return false; + } + + String methodName = method.getSimpleName(); + + if (excludeMethodPattern != null && !excludeMethodPattern.trim().isEmpty()) { + Pattern excludePattern = Pattern.compile(excludeMethodPattern); + if (excludePattern.matcher(methodName).matches()) { + return false; + } + } + + if (methodNamePattern != null && !methodNamePattern.trim().isEmpty()) { + Pattern namePattern = Pattern.compile(methodNamePattern); + return namePattern.matcher(methodName).matches(); + } + + if (includeAllVoidMethods != null && includeAllVoidMethods) { + return true; + } + + // Default behavior: only setter methods + return isSetterMethod(method); + } + + private boolean isSetterMethod(J.MethodDeclaration method) { + String methodName = method.getSimpleName(); + if (!methodName.startsWith("set") || methodName.length() <= 3) { + return false; + } + + // Must have exactly one parameter + if (method.getParameters().size() != 1) { + return false; + } + + // The character after "set" should be uppercase (setName, not setup) + char charAfterSet = methodName.charAt(3); + return Character.isUpperCase(charAfterSet); + } + + private boolean hasReturnStatement(J.Block body) { + AtomicBoolean hasReturn = new AtomicBoolean(false); + + new JavaIsoVisitor() { + @Override + public J.Return visitReturn(J.Return returnStmt, AtomicBoolean found) { + found.set(true); + return returnStmt; + } + }.visit(body, hasReturn); + + return hasReturn.get(); + } + }; + } +} diff --git a/src/test/java/org/openrewrite/staticanalysis/FluentSetterRecipeTest.java b/src/test/java/org/openrewrite/staticanalysis/FluentSetterRecipeTest.java new file mode 100644 index 000000000..e24c2ef1c --- /dev/null +++ b/src/test/java/org/openrewrite/staticanalysis/FluentSetterRecipeTest.java @@ -0,0 +1,538 @@ +/* + * Copyright 2025 the original author or authors. + *

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

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

+ * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.openrewrite.staticanalysis; + +import org.junit.jupiter.api.Test; +import org.openrewrite.test.RecipeSpec; +import org.openrewrite.test.RewriteTest; + +import static org.openrewrite.java.Assertions.java; + +class FluentSetterRecipeTest implements RewriteTest { + + @Override + public void defaults(RecipeSpec spec) { + spec.recipe(new FluentSetterRecipe(false, null, null)); + } + + @Test + void convertSimpleSetter() { + rewriteRun( + java( + """ + public class Person { + private String name; + + public void setName(String name) { + this.name = name; + } + } + """, + """ + public class Person { + private String name; + + public Person setName(String name) { + this.name = name; + return this; + } + } + """ + ) + ); + } + + @Test + void convertSetterWithExtraIdent() { + rewriteRun( + java( + """ + public class Person { + private String name; + + public void setName(String name) { + this.name = name; + } + } + """, + """ + public class Person { + private String name; + + public Person setName(String name) { + this.name = name; + return this; + } + } + """ + ) + ); + } + + @Test + void convertMultipleSetters() { + rewriteRun( + java( + """ + public class Person { + private String name; + private int age; + private String email; + + public void setName(String name) { + System.out.println(name); + this.name = name; + } + + public void setAge(int age) { + this.age = age; + } + + public void setEmail(String email) { + this.email = email; + } + } + """, + """ + public class Person { + private String name; + private int age; + private String email; + + public Person setName(String name) { + System.out.println(name); + this.name = name; + return this; + } + + public Person setAge(int age) { + this.age = age; + return this; + } + + public Person setEmail(String email) { + this.email = email; + return this; + } + } + """ + ) + ); + } + + @Test + void skipNonSetterVoidMethods() { + rewriteRun( + java( + """ + public class Example { + private String name; + + public void setName(String name) { + this.name = name; + } + + public void doSomething() { + System.out.println("doing something"); + } + + public void run() { + // some logic + } + } + """, + """ + public class Example { + private String name; + + public Example setName(String name) { + this.name = name; + return this; + } + + public void doSomething() { + System.out.println("doing something"); + } + + public void run() { + // some logic + } + } + """ + ) + ); + } + + @Test + void skipStaticMethods() { + rewriteRun( + java( + """ + public class Example { + private static String globalName; + private String name; + + public static void setGlobalName(String name) { + globalName = name; + } + + public void setName(String name) { + this.name = name; + } + } + """, + """ + public class Example { + private static String globalName; + private String name; + + public static void setGlobalName(String name) { + globalName = name; + } + + public Example setName(String name) { + this.name = name; + return this; + } + } + """ + ) + ); + } + + @Test + void skipMethodsWithExistingReturnStatement() { + rewriteRun( + java( + """ + public class Example { + private String name; + + public void setName(String name) { + if (name == null) { + return; + } + this.name = name; + } + } + """ + ) + ); + } + + @Test + void skipNonVoidMethods() { + rewriteRun( + java( + """ + public class Example { + private String name; + + public String setName(String name) { + this.name = name; + return "success"; + } + + public String getName() { + return name; + } + } + """ + ) + ); + } + + @Test + void skipAbstractMethods() { + rewriteRun( + java( + """ + public abstract class Example { + private String name; + + public abstract void setName(String name); + } + """ + ) + ); + } + + @Test + void skipInvalidSetterPatterns() { + rewriteRun( + java( + """ + public class Example { + private String name; + + // Not a setter - doesn't start with 'set' + public void updateName(String name) { + this.name = name; + } + + // Not a setter - 'set' but lowercase next char + public void setup() { + // setup logic + } + + // Not a setter - too many parameters + public void setNameAndAge(String name, int age) { + this.name = name; + } + } + """ + ) + ); + } + + @Test + void includeAllVoidMethods() { + rewriteRun( + spec -> spec.recipe(new FluentSetterRecipe(true, null, null)), + java( + """ + public class Example { + private String name; + + public void setName(String name) { + this.name = name; + } + + public void doSomething() { + System.out.println("doing something"); + } + + public void process(String data) { + System.out.println("processing something"); + } + } + """, + """ + public class Example { + private String name; + + public Example setName(String name) { + this.name = name; + return this; + } + + public Example doSomething() { + System.out.println("doing something"); + return this; + } + + public Example process(String data) { + System.out.println("processing something"); + return this; + } + } + """ + ) + ); + } + + @Test + void customMethodNamePattern() { + rewriteRun( + spec -> spec.recipe(new FluentSetterRecipe(false, "add.*|remove.*", null)), + java( + """ + public class Example { + private String name; + + public void setName(String name) { + this.name = name; + } + + public void addItem(String item) { + System.out.println("adding item"); + } + + public void removeItem(String item) { + System.out.println("removing item"); + } + + public void doSomething() { + } + } + """, + """ + public class Example { + private String name; + + public void setName(String name) { + this.name = name; + } + + public Example addItem(String item) { + System.out.println("adding item"); + return this; + } + + public Example removeItem(String item) { + System.out.println("removing item"); + return this; + } + + public void doSomething() { + } + } + """ + ) + ); + } + + @Test + void excludeMethodPattern() { + rewriteRun( + spec -> spec.recipe(new FluentSetterRecipe(true, null, "main|run|execute")), + java( + """ + public class Example { + private String name; + + public void setName(String name) { + this.name = name; + } + + public void doSomething() { + System.out.println("doing something"); + } + + // should be excluded + public void run() { + } + + // should be excluded + public void execute() { + } + + // should be excluded (also static) + public static void main(String[] args) { + } + } + """, + """ + public class Example { + private String name; + + public Example setName(String name) { + this.name = name; + return this; + } + + public Example doSomething() { + System.out.println("doing something"); + return this; + } + + // should be excluded + public void run() { + } + + // should be excluded + public void execute() { + } + + // should be excluded (also static) + public static void main(String[] args) { + } + } + """ + ) + ); + } + + @Test + void worksWithInnerClasses() { + rewriteRun( + java( + """ + public class Outer { + private String outerName; + + public void setOuterName(String name) { + this.outerName = name; + } + + public static class Inner { + private String innerName; + + public void setInnerName(String name) { + this.innerName = name; + } + } + } + """, + """ + public class Outer { + private String outerName; + + public Outer setOuterName(String name) { + this.outerName = name; + return this; + } + + public static class Inner { + private String innerName; + + public Inner setInnerName(String name) { + this.innerName = name; + return this; + } + } + } + """ + ) + ); + } + + @Test + void preservesMethodModifiers() { + rewriteRun( + java( + """ + public class Example { + private String name; + + protected void setName(String name) { + this.name = name; + } + + private void setPrivateName(String name) { + this.name = name; + } + } + """, + """ + public class Example { + private String name; + + protected Example setName(String name) { + this.name = name; + return this; + } + + private Example setPrivateName(String name) { + this.name = name; + return this; + } + } + """ + ) + ); + } +} From d8673d1edfa6d9d04188004af5e8b7d6024ca600 Mon Sep 17 00:00:00 2001 From: e5LA <208197507+e5LA@users.noreply.github.com> Date: Tue, 22 Jul 2025 07:44:36 +0200 Subject: [PATCH 2/4] refactor: fix formatting --- .../staticanalysis/FluentSetterRecipe.java | 64 ++++--------------- .../FluentSetterRecipeTest.java | 2 + 2 files changed, 15 insertions(+), 51 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/FluentSetterRecipe.java b/src/main/java/org/openrewrite/staticanalysis/FluentSetterRecipe.java index 471fafd68..ba6decd0b 100644 --- a/src/main/java/org/openrewrite/staticanalysis/FluentSetterRecipe.java +++ b/src/main/java/org/openrewrite/staticanalysis/FluentSetterRecipe.java @@ -40,22 +40,15 @@ @Value public class FluentSetterRecipe extends Recipe { - @Option(displayName = "Include all void methods", description = - "Whether to convert all void methods to return `this`, not just setters. " - + "When false, only methods matching setter patterns will be converted.", required = false) + @Option(displayName = "Include all void methods", description = "Whether to convert all void methods to return `this`, not just setters. When false, only methods matching setter patterns will be converted.", required = false) @Nullable Boolean includeAllVoidMethods; - @Option(displayName = "Method name pattern", description = - "A regular expression pattern to match method names. " - + "Only methods matching this pattern will be converted. " - + "Defaults to setter pattern when includeAllVoidMethods is false.", example = "set.*", required = false) + @Option(displayName = "Method name pattern", description = "A regular expression pattern to match method names. Only methods matching this pattern will be converted. Defaults to setter pattern when includeAllVoidMethods is false.", example = "set.*", required = false) @Nullable String methodNamePattern; - @Option(displayName = "Exclude method patterns", description = - "A regular expression pattern for method names to exclude from conversion. " - + "Methods matching this pattern will not be converted.", example = "main|run", required = false) + @Option(displayName = "Exclude method patterns", description = "A regular expression pattern for method names to exclude from conversion. Methods matching this pattern will not be converted.", example = "main|run", required = false) @Nullable String excludeMethodPattern; @@ -66,8 +59,7 @@ public String getDisplayName() { @Override public String getDescription() { - return "Converts void setter methods (and optionally other void methods) to return `this` " - + "to enable method chaining and fluent interfaces."; + return "Converts void setter methods (and optionally other void methods) to return `this` to enable method chaining and fluent interfaces."; } @Override @@ -75,8 +67,7 @@ public JavaIsoVisitor getVisitor() { return new JavaIsoVisitor() { @Override - public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, - ExecutionContext ctx) { + public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, ExecutionContext ctx) { method = super.visitMethodDeclaration(method, ctx); if (!shouldConvertMethod(method)) { return method; @@ -93,23 +84,12 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, className = className.substring(className.lastIndexOf('.') + 1); } - Space returnTypeSpace = method.getReturnTypeExpression() == null ? Space.EMPTY - : method.getReturnTypeExpression().getPrefix(); - Markers returnTypeMarkers = method.getReturnTypeExpression() == null ? Markers.EMPTY - : method.getReturnTypeExpression().getMarkers(); + Space returnTypeSpace = method.getReturnTypeExpression() == null ? Space.EMPTY : method.getReturnTypeExpression().getPrefix(); + Markers returnTypeMarkers = method.getReturnTypeExpression() == null ? Markers.EMPTY : method.getReturnTypeExpression().getMarkers(); - J.Identifier returnTypeIdentifier = new J.Identifier( - randomId(), - returnTypeSpace, - returnTypeMarkers, - emptyList(), - className, - classType, - null - ); + J.Identifier returnTypeIdentifier = new J.Identifier(randomId(), returnTypeSpace, returnTypeMarkers, emptyList(), className, classType, null); - J.MethodDeclaration updatedMethod = method - .withReturnTypeExpression(returnTypeIdentifier); + J.MethodDeclaration updatedMethod = method.withReturnTypeExpression(returnTypeIdentifier); if (updatedMethod.getBody() != null) { Space indentation; @@ -120,26 +100,9 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, indentation = updatedMethod.getBody().getPrefix(); } - J.Return returnThis = new J.Return( - randomId(), - Space.format("\n" + indentation.getIndent()), - Markers.EMPTY, - new J.Identifier( - randomId(), - Space.SINGLE_SPACE, - Markers.EMPTY, - emptyList(), - "this", - classType, - null - ) - ); - - updatedMethod = updatedMethod.withBody( - updatedMethod.getBody().withStatements( - ListUtils.concat(updatedMethod.getBody().getStatements(), returnThis) - ) - ); + J.Return returnThis = new J.Return(randomId(), Space.format("\n" + indentation.getIndent()), Markers.EMPTY, new J.Identifier(randomId(), Space.SINGLE_SPACE, Markers.EMPTY, emptyList(), "this", classType, null)); + + updatedMethod = updatedMethod.withBody(updatedMethod.getBody().withStatements(ListUtils.concat(updatedMethod.getBody().getStatements(), returnThis))); } return updatedMethod; @@ -147,8 +110,7 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, private boolean shouldConvertMethod(J.MethodDeclaration method) { - if (method.getReturnTypeExpression() == null - || method.getReturnTypeExpression().getType() != JavaType.Primitive.Void) { + if (method.getReturnTypeExpression() == null || method.getReturnTypeExpression().getType() != JavaType.Primitive.Void) { return false; } diff --git a/src/test/java/org/openrewrite/staticanalysis/FluentSetterRecipeTest.java b/src/test/java/org/openrewrite/staticanalysis/FluentSetterRecipeTest.java index e24c2ef1c..95a71b0cd 100644 --- a/src/test/java/org/openrewrite/staticanalysis/FluentSetterRecipeTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/FluentSetterRecipeTest.java @@ -16,6 +16,7 @@ package org.openrewrite.staticanalysis; import org.junit.jupiter.api.Test; +import org.openrewrite.DocumentExample; import org.openrewrite.test.RecipeSpec; import org.openrewrite.test.RewriteTest; @@ -28,6 +29,7 @@ public void defaults(RecipeSpec spec) { spec.recipe(new FluentSetterRecipe(false, null, null)); } + @DocumentExample @Test void convertSimpleSetter() { rewriteRun( From 363366cfb26728c1a8c4c3bfe2c930428ab15952 Mon Sep 17 00:00:00 2001 From: e5LA <208197507+e5LA@users.noreply.github.com> Date: Sun, 3 Aug 2025 17:04:01 +0200 Subject: [PATCH 3/4] refactor: renaming class, removing Recipe suffix --- .../{FluentSetterRecipe.java => FluentSetter.java} | 2 +- ...uentSetterRecipeTest.java => FluentSetterTest.java} | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) rename src/main/java/org/openrewrite/staticanalysis/{FluentSetterRecipe.java => FluentSetter.java} (99%) rename src/test/java/org/openrewrite/staticanalysis/{FluentSetterRecipeTest.java => FluentSetterTest.java} (97%) diff --git a/src/main/java/org/openrewrite/staticanalysis/FluentSetterRecipe.java b/src/main/java/org/openrewrite/staticanalysis/FluentSetter.java similarity index 99% rename from src/main/java/org/openrewrite/staticanalysis/FluentSetterRecipe.java rename to src/main/java/org/openrewrite/staticanalysis/FluentSetter.java index ba6decd0b..678f4f6c4 100644 --- a/src/main/java/org/openrewrite/staticanalysis/FluentSetterRecipe.java +++ b/src/main/java/org/openrewrite/staticanalysis/FluentSetter.java @@ -38,7 +38,7 @@ @EqualsAndHashCode(callSuper = false) @Value -public class FluentSetterRecipe extends Recipe { +public class FluentSetter extends Recipe { @Option(displayName = "Include all void methods", description = "Whether to convert all void methods to return `this`, not just setters. When false, only methods matching setter patterns will be converted.", required = false) @Nullable diff --git a/src/test/java/org/openrewrite/staticanalysis/FluentSetterRecipeTest.java b/src/test/java/org/openrewrite/staticanalysis/FluentSetterTest.java similarity index 97% rename from src/test/java/org/openrewrite/staticanalysis/FluentSetterRecipeTest.java rename to src/test/java/org/openrewrite/staticanalysis/FluentSetterTest.java index 95a71b0cd..eb9999079 100644 --- a/src/test/java/org/openrewrite/staticanalysis/FluentSetterRecipeTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/FluentSetterTest.java @@ -22,11 +22,11 @@ import static org.openrewrite.java.Assertions.java; -class FluentSetterRecipeTest implements RewriteTest { +class FluentSetterTest implements RewriteTest { @Override public void defaults(RecipeSpec spec) { - spec.recipe(new FluentSetterRecipe(false, null, null)); + spec.recipe(new FluentSetter(false, null, null)); } @DocumentExample @@ -303,7 +303,7 @@ public void setNameAndAge(String name, int age) { @Test void includeAllVoidMethods() { rewriteRun( - spec -> spec.recipe(new FluentSetterRecipe(true, null, null)), + spec -> spec.recipe(new FluentSetter(true, null, null)), java( """ public class Example { @@ -349,7 +349,7 @@ public Example process(String data) { @Test void customMethodNamePattern() { rewriteRun( - spec -> spec.recipe(new FluentSetterRecipe(false, "add.*|remove.*", null)), + spec -> spec.recipe(new FluentSetter(false, "add.*|remove.*", null)), java( """ public class Example { @@ -400,7 +400,7 @@ public void doSomething() { @Test void excludeMethodPattern() { rewriteRun( - spec -> spec.recipe(new FluentSetterRecipe(true, null, "main|run|execute")), + spec -> spec.recipe(new FluentSetter(true, null, "main|run|execute")), java( """ public class Example { From 829139a439c4289e31919e6392dcdf768e09379e Mon Sep 17 00:00:00 2001 From: e5LA <208197507+e5LA@users.noreply.github.com> Date: Mon, 1 Sep 2025 20:30:22 +0200 Subject: [PATCH 4/4] Apply final class safety check when no method pattern specified --- .../staticanalysis/FluentSetter.java | 28 +- .../staticanalysis/FluentSetterTest.java | 263 +++++++++++++++--- 2 files changed, 255 insertions(+), 36 deletions(-) diff --git a/src/main/java/org/openrewrite/staticanalysis/FluentSetter.java b/src/main/java/org/openrewrite/staticanalysis/FluentSetter.java index 678f4f6c4..856eecaaa 100644 --- a/src/main/java/org/openrewrite/staticanalysis/FluentSetter.java +++ b/src/main/java/org/openrewrite/staticanalysis/FluentSetter.java @@ -59,7 +59,8 @@ public String getDisplayName() { @Override public String getDescription() { - return "Converts void setter methods (and optionally other void methods) to return `this` to enable method chaining and fluent interfaces."; + return "Converts void setter methods (and optionally other void methods) to return `this` to enable method chaining and fluent interfaces. " + + "For safety, only converts methods in final classes by default to avoid breaking inheritance. Use methodNamePattern to opt-in for non-final classes."; } @Override @@ -78,6 +79,7 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex return method; } + // Extract simple class name from fully qualified name or inner class name JavaType.FullyQualified classType = containingClass.getType(); String className = classType.getClassName(); if (className.contains(".")) { @@ -108,7 +110,6 @@ public J.MethodDeclaration visitMethodDeclaration(J.MethodDeclaration method, Ex return updatedMethod; } - private boolean shouldConvertMethod(J.MethodDeclaration method) { if (method.getReturnTypeExpression() == null || method.getReturnTypeExpression().getType() != JavaType.Primitive.Void) { return false; @@ -126,7 +127,7 @@ private boolean shouldConvertMethod(J.MethodDeclaration method) { return false; } - if (method.getBody() != null && hasReturnStatement(method.getBody())) { + if (method.getBody() != null && (hasReturnStatement(method.getBody()) || onlyThrowsException(method.getBody()))) { return false; } @@ -144,11 +145,15 @@ private boolean shouldConvertMethod(J.MethodDeclaration method) { return namePattern.matcher(methodName).matches(); } + J.ClassDeclaration containingClass = getCursor().firstEnclosing(J.ClassDeclaration.class); + if (containingClass == null || !containingClass.hasModifier(J.Modifier.Type.Final)) { + return false; + } + if (includeAllVoidMethods != null && includeAllVoidMethods) { return true; } - // Default behavior: only setter methods return isSetterMethod(method); } @@ -181,6 +186,21 @@ public J.Return visitReturn(J.Return returnStmt, AtomicBoolean found) { return hasReturn.get(); } + + private boolean onlyThrowsException(J.Block body) { + List statements = body.getStatements(); + if (statements.isEmpty()) { + return false; + } + + for (Statement statement : statements) { + if (!(statement instanceof J.Throw)) { + return false; + } + } + + return true; + } }; } } diff --git a/src/test/java/org/openrewrite/staticanalysis/FluentSetterTest.java b/src/test/java/org/openrewrite/staticanalysis/FluentSetterTest.java index eb9999079..677baae43 100644 --- a/src/test/java/org/openrewrite/staticanalysis/FluentSetterTest.java +++ b/src/test/java/org/openrewrite/staticanalysis/FluentSetterTest.java @@ -31,11 +31,11 @@ public void defaults(RecipeSpec spec) { @DocumentExample @Test - void convertSimpleSetter() { + void convertSimpleSetterInFinalClass() { rewriteRun( java( """ - public class Person { + public final class Person { private String name; public void setName(String name) { @@ -44,7 +44,7 @@ public void setName(String name) { } """, """ - public class Person { + public final class Person { private String name; public Person setName(String name) { @@ -58,20 +58,37 @@ public Person setName(String name) { } @Test - void convertSetterWithExtraIdent() { + void skipSetterInNonFinalClass() { rewriteRun( java( """ public class Person { private String name; + public void setName(String name) { + this.name = name; + } + } + """ + ) + ); + } + + @Test + void convertSetterWithExtraIdent() { + rewriteRun( + java( + """ + public final class Person { + private String name; + public void setName(String name) { this.name = name; } } """, """ - public class Person { + public final class Person { private String name; public Person setName(String name) { @@ -85,11 +102,11 @@ public Person setName(String name) { } @Test - void convertMultipleSetters() { + void convertMultipleSettersInFinalClass() { rewriteRun( java( """ - public class Person { + public final class Person { private String name; private int age; private String email; @@ -109,7 +126,7 @@ public void setEmail(String email) { } """, """ - public class Person { + public final class Person { private String name; private int age; private String email; @@ -136,11 +153,11 @@ public Person setEmail(String email) { } @Test - void skipNonSetterVoidMethods() { + void skipNonSetterVoidMethodsInFinalClass() { rewriteRun( java( """ - public class Example { + public final class Example { private String name; public void setName(String name) { @@ -157,7 +174,7 @@ public void run() { } """, """ - public class Example { + public final class Example { private String name; public Example setName(String name) { @@ -183,7 +200,7 @@ void skipStaticMethods() { rewriteRun( java( """ - public class Example { + public final class Example { private static String globalName; private String name; @@ -197,7 +214,7 @@ public void setName(String name) { } """, """ - public class Example { + public final class Example { private static String globalName; private String name; @@ -220,7 +237,7 @@ void skipMethodsWithExistingReturnStatement() { rewriteRun( java( """ - public class Example { + public final class Example { private String name; public void setName(String name) { @@ -240,7 +257,7 @@ void skipNonVoidMethods() { rewriteRun( java( """ - public class Example { + public final class Example { private String name; public String setName(String name) { @@ -277,20 +294,17 @@ void skipInvalidSetterPatterns() { rewriteRun( java( """ - public class Example { + public final class Example { private String name; - // Not a setter - doesn't start with 'set' public void updateName(String name) { this.name = name; } - // Not a setter - 'set' but lowercase next char public void setup() { // setup logic } - // Not a setter - too many parameters public void setNameAndAge(String name, int age) { this.name = name; } @@ -301,19 +315,19 @@ public void setNameAndAge(String name, int age) { } @Test - void includeAllVoidMethods() { + void includeAllVoidMethodsInFinalClass() { rewriteRun( spec -> spec.recipe(new FluentSetter(true, null, null)), java( """ - public class Example { + public final class Example { private String name; public void setName(String name) { this.name = name; } - public void doSomething() { + void doSomething() { System.out.println("doing something"); } @@ -323,7 +337,7 @@ public void process(String data) { } """, """ - public class Example { + public final class Example { private String name; public Example setName(String name) { @@ -331,7 +345,7 @@ public Example setName(String name) { return this; } - public Example doSomething() { + Example doSomething() { System.out.println("doing something"); return this; } @@ -403,7 +417,7 @@ void excludeMethodPattern() { spec -> spec.recipe(new FluentSetter(true, null, "main|run|execute")), java( """ - public class Example { + public final class Example { private String name; public void setName(String name) { @@ -428,7 +442,7 @@ public static void main(String[] args) { } """, """ - public class Example { + public final class Example { private String name; public Example setName(String name) { @@ -463,14 +477,14 @@ void worksWithInnerClasses() { rewriteRun( java( """ - public class Outer { + public final class Outer { private String outerName; public void setOuterName(String name) { this.outerName = name; } - public static class Inner { + public static final class Inner { private String innerName; public void setInnerName(String name) { @@ -480,7 +494,7 @@ public void setInnerName(String name) { } """, """ - public class Outer { + public final class Outer { private String outerName; public Outer setOuterName(String name) { @@ -488,7 +502,7 @@ public Outer setOuterName(String name) { return this; } - public static class Inner { + public static final class Inner { private String innerName; public Inner setInnerName(String name) { @@ -507,7 +521,7 @@ void preservesMethodModifiers() { rewriteRun( java( """ - public class Example { + public final class Example { private String name; protected void setName(String name) { @@ -520,7 +534,7 @@ private void setPrivateName(String name) { } """, """ - public class Example { + public final class Example { private String name; protected Example setName(String name) { @@ -537,4 +551,189 @@ private Example setPrivateName(String name) { ) ); } + + @Test + void withPatternConvertsNonFinalClasses() { + rewriteRun( + spec -> spec.recipe(new FluentSetter(false, "set.*", null)), + java( + """ + public class Person { + private String name; + + public void setName(String name) { + this.name = name; + } + } + """, + """ + public class Person { + private String name; + + public Person setName(String name) { + this.name = name; + return this; + } + } + """ + ) + ); + } + + @Test + void excludePatternWorksWithFinalClasses() { + rewriteRun( + spec -> spec.recipe(new FluentSetter(false, null, "setName")), + java( + """ + public final class Example { + private String name; + private int age; + + public void setName(String name) { + this.name = name; + } + + public void setAge(int age) { + this.age = age; + } + } + """, + """ + public final class Example { + private String name; + private int age; + + public void setName(String name) { + this.name = name; + } + + public Example setAge(int age) { + this.age = age; + return this; + } + } + """ + ) + ); + } + + @Test + void includeAllVoidMethodsInFinalClassOnly() { + rewriteRun( + spec -> spec.recipe(new FluentSetter(true, null, null)), + java( + """ + public final class FinalExample { + private String name; + + public void setName(String name) { + this.name = name; + } + + public void doSomething() { + System.out.println("doing something"); + } + } + + public class NonFinalExample { + private String name; + + public void setName(String name) { + this.name = name; + } + + public void doSomething() { + System.out.println("doing something"); + } + } + """, + """ + public final class FinalExample { + private String name; + + public FinalExample setName(String name) { + this.name = name; + return this; + } + + public FinalExample doSomething() { + System.out.println("doing something"); + return this; + } + } + + public class NonFinalExample { + private String name; + + public void setName(String name) { + this.name = name; + } + + public void doSomething() { + System.out.println("doing something"); + } + } + """ + ) + ); + } + + @Test + void combinedPatternAndExcludePattern() { + rewriteRun( + spec -> spec.recipe(new FluentSetter(false, "set.*", "setName")), + java( + """ + public class Example { + private String name; + private int age; + + public void setName(String name) { + this.name = name; + } + + public void setAge(int age) { + this.age = age; + } + } + """, + """ + public class Example { + private String name; + private int age; + + public void setName(String name) { + this.name = name; + } + + public Example setAge(int age) { + this.age = age; + return this; + } + } + """ + ) + ); + } + + @Test + void skipMethodsThatOnlyThrowExceptions() { + rewriteRun( + java( + """ + public final class Example { + + public void setRawOffset(final int offsetMillis) { + throw new UnsupportedOperationException(); + } + + public void setImmutableValue(String value) { + throw new IllegalStateException("Cannot modify immutable value"); + } + } + """ + ) + ); + } }