Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support return Refaster.anyOf(...) #21

Closed
timtebeek opened this issue Aug 15, 2023 · 4 comments
Closed

Support return Refaster.anyOf(...) #21

timtebeek opened this issue Aug 15, 2023 · 4 comments
Assignees
Labels
enhancement New feature or request parser-java

Comments

@timtebeek
Copy link
Contributor

timtebeek commented Aug 15, 2023

What problem are you trying to solve?

Right now we skip all refaster rules using any form of Refaster.anyOf(...), whereas we could potentially cover return Refaster.anyOf(...) with less effort as compared to supporting Refaster.anyOf(...) anywhere in template for matches.
This will allow us to cover additional rules defined in PicnicSupermarket/error-prone-support, as discovered in #5 (comment).

Describe the solution you'd like

We already support multiple @BeforeTemplate annotated methods. We should expand support for multiple before statements to cover return Refaster.anyOf as seen here.

@BeforeTemplate
boolean before(Map<K, V> map) {
  return Refaster.anyOf(map.keySet(), map.values(), map.entrySet()).isEmpty();
}

Refaster.anyOf(...) used anywhere else but in the return, repeatedly, or with preceding statements is left out of scope for now.

Have you considered any alternatives or workarounds?

Not yet.

Additional context

@timtebeek
Copy link
Contributor Author

Had a brief look at adding this one over the weekend; the easiest case to tackle is likely something like this:

public class RefasterAnyOf {
    @BeforeTemplate
    boolean before(String s) {
        return Refaster.anyOf(s.length() < 1, s.length() == 0);
    }

    @AfterTemplate
    boolean after(String s) {
        return s.isEmpty();
    }
}

And then ideally turn that into something similar to the multiple before statements we already know:

JavaTemplate before = Semantics.expression(this, "before", (String s) -> s.length() < 1).build();
JavaTemplate before2 = Semantics.expression(this, "before2", (String s) -> s.length() == 0).build();
JavaTemplate after = Semantics.expression(this, "after", (String s) -> s.isEmpty()).build();

Implementation wise there's the slight challenge of the List<JCTree.JCMethodDecl> beforeTemplates being passed around and used four different times for imports, template naming, lambda bodies and preconditions. The challenge is then where to detect Refaster.anyOf and how to adapt the code to use it's arguments.

@knutwannheden
Copy link
Contributor

The arguments to Refaster.anyOf() should also be considered as separate when detecting the used types and methods to build the UsedType and UsedMethod preconditions.

@timtebeek
Copy link
Contributor Author

Fixed in #76

@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenRewrite Mar 7, 2024
@knutwannheden
Copy link
Contributor

It also works when the anyOf() is not directly the return expression. What doesn't work ia if a template contains multiple such calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request parser-java
Projects
Archived in project
Development

No branches or pull requests

2 participants