rewrite-static-analysis icon indicating copy to clipboard operation
rewrite-static-analysis copied to clipboard

Add FluentSetterRecipe

Open e5LA opened this issue 5 months ago • 4 comments

What's changed?

Added new FluentSetterRecipe to convert void setter methods (and optionally other void methods) into fluent-style methods that return this.

What's your motivation?

  • Closes https://github.com/openrewrite/rewrite-static-analysis/issues/623

Anything in particular you'd like reviewers to focus on?

  • Indentation and placement of the injected return this; statement.
  • Return types use the simple name (e.g. Inner instead of Outer.Inner)

Checklist

  • [x] I've added unit tests to cover both positive and negative cases
  • [x] I've read and applied the recipe conventions and best practices
  • [x] I've used the IntelliJ IDEA auto-formatter on affected files

e5LA avatar Jul 21 '25 19:07 e5LA

Thanks for the review @greg-at-moderne - that's good point, I will have a look.

e5LA avatar Jul 23 '25 06:07 e5LA

Sorry for the delay, I didn't have much time lately. While working on detecting whether a method overrides a superclass method, I realized there's a case we need to consider:

public class Parent {
    public void setName(String name) {
        this.name = name;
    }
}

public class Child extends Parent {
    private String name;

    @Override
    public void setName(String name) {
        this.name = name;
    }
}

In this situation, I see two options:

  1. Either transform both Parent#setName and Child#setName to return this (+changing the return type) - that means we're explicitly transforming overridden methods.

  2. Or skip transforming Child#setName because it’s an override - that seems straightforward. However, the tricky part is with Parent#setName - it's not itself an override, but it is overridden. I wonder - can we reliably detect that it’s being overridden in a subclass?

e5LA avatar Jul 30 '25 09:07 e5LA

Thanks both! Wondering a bit indeed how we can make this safe; we don't immediately have a way to know if a method might be overridden, unless we use a ScanningRecipe, and even then we might miss cases where it's a library. Perhaps the safe thing to do would be to limit this to either the provided method pattern (opt in), or final classes when left out.

We can optionally and separately expand FinalClass to make it a scanning recipe that adds that modifier to any classes not extended, that way the combination of these two recipes would achieve the safe results we're after.

timtebeek avatar Aug 24 '25 13:08 timtebeek

Thanks for the feedback @timtebeek, that's good suggestion.

I've updated the recipe, now by default it converts methods only in final classes. When the pattern is provided, it converts methods in any class.

Your idea about expanding FinalClass recipe also sounds good. I might take a look at it too.

e5LA avatar Sep 01 '25 18:09 e5LA