Assign response from withTypeParameters in SpacesVisitor

[timtebeek]

I have a local recipe that still has some false positives, but already was able to find these two cases where the response to an @With annotated method call went unchecked.

• Inspired by Adding a missing return, without which a supposed optimization does nothing #5160

[knutwannheden]

How did you find these cases? I would quite like to annotate all with-methods as @Contract(pure=true) and we could indeed do that using @With(onMethod=...). I have also contemplated using the Lombok SPI to register our own handler for@With (or possibly a custom annotation) to make it aware of padding using JLeftPadded etc. That could then automatically declare the methods as pure.

[timtebeek]

This was for now just a silly local recipe with some false positives, that I had thought to add to rewrite-rewrite & PR checks eventually:

import org.openrewrite.ExecutionContext;
import org.openrewrite.Preconditions;
import org.openrewrite.Recipe;
import org.openrewrite.TreeVisitor;
import org.openrewrite.java.search.UsesMethod;
import org.openrewrite.java.tree.J;
import org.openrewrite.marker.SearchResult;

public class UnusedWither extends Recipe {

    private static final MethodMatcher WITH_MATCHER = new MethodMatcher("*..*#with*(..)", true);

    @Override
    public String getDisplayName() {
        return "Find unused withers";
    }

    @Override
    public String getDescription() {
        return "Find wither methods called, but their return value is not used.";
    }

    @Override
    public TreeVisitor<?, ExecutionContext> getVisitor() {
        JavaIsoVisitor<ExecutionContext> visitor = new JavaIsoVisitor<ExecutionContext>() {
            @Override
            public J.MethodInvocation visitMethodInvocation(J.MethodInvocation method, ExecutionContext ctx) {
                if (WITH_MATCHER.matches(method) &&
                        getCursor().getParentTreeCursor().getValue() instanceof J.Block) {
                    return SearchResult.found(method);
                }
                return super.visitMethodInvocation(method, ctx);
            }
        };
        return Preconditions.check(new UsesMethod<>(WITH_MATCHER), visitor);
    }
}

You'll notice that it picks up any with named method, including things like AssertJ assertion methods like withMessageContaining, as well as with-ers that return this instead of a new copy. Still it was helpful to identify these cases, and the one in rewrite-python, as it's an easy mistake to make:

• Assign response from pattern.withChildren rewrite-python#139

[knutwannheden]

I would quite like to have the methods declared as pure, as then IDEA would show a warning when calling these methods as if they had a side effect.

[timtebeek]

Agree that'd be nice, but that would likely mean either create our own meta annotation for With methods, which then adds that contract, or add that contract to each usage of the Lombok With annotation. Both seem slightly out of scope for this small fix; can we get this in already?

[knutwannheden]

Definitely, yes. I didn't mean to stop the PR from getting merged.

[sambsnyd]

Good catch Tim!

[timtebeek]

Thanks! There's one more similar case in

• Assign response from pattern.withChildren rewrite-python#139