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 additional Refaster features seen in error-prone-support #47

Open
3 of 12 tasks
timtebeek opened this issue Dec 17, 2023 · 12 comments
Open
3 of 12 tasks

Support additional Refaster features seen in error-prone-support #47

timtebeek opened this issue Dec 17, 2023 · 12 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@timtebeek
Copy link
Contributor

timtebeek commented Dec 17, 2023

What problem are you trying to solve?

Expand the set of Refaster rules in Error Prone Support that we cover with OpenRewrite recipes.

Describe the solution you'd like

Support the following cases not currently covered.

@timtebeek timtebeek added the enhancement New feature or request label Dec 17, 2023
@timtebeek timtebeek moved this to Backlog in OpenRewrite Dec 17, 2023
@timtebeek timtebeek added the help wanted Extra attention is needed label Dec 17, 2023
@Stephan202
Copy link
Contributor

Hey @timtebeek! The @DoNotCall annotation has no impact for Refaster; it's there only to appease the DoNotCall check :)

@timtebeek
Copy link
Contributor Author

Hey @timtebeek! The @DoNotCall annotation has no impact for Refaster; it's there only to appease the DoNotCall check :)

Thanks for that context! Right now we skip any rules where that annotation is present, but it seems like we can tolerate the presence of that annotation and just generate the recipes in those cases. Good to know!

@timtebeek
Copy link
Contributor Author

Even if we ignore the presence of @UseImportOlicy annotation it seems we still have some work to do before recipe correctly compile, as we don't yet add resolve and imports explicitly. Internally we had relied on after templates always using fully qualified refs, but we can't expect that going forward.

@knutwannheden
Copy link
Contributor

@timtebeek I think we can. I thought the template code is already being generated with all fully qualified references (regardless if the input uses qualified references or not). That was in any case the idea.

@timtebeek
Copy link
Contributor Author

timtebeek commented Dec 23, 2023

@timtebeek I think we can. I thought the template code is already being generated with all fully qualified references (regardless if the input uses qualified references or not). That was in any case the idea.

Exploring the addition of new static imports now in this PR; based on what we do versus need it looks like we have some work to do still.

@timtebeek
Copy link
Contributor Author

Some quick stats on why various refaster rules were excluded, such that we know what to cover next:

[INFO] 791x Generics are currently not supported
[INFO] 40x com.google.errorprone.refaster.Refaster is currently not supported
[INFO] 19x @AlsoNegation is currently not supported
[INFO] 15x @Placeholder is currently not supported
[INFO] 8x If statements are currently not supported
[INFO] 1x @Matches is currently not supported

@knutwannheden
Copy link
Contributor

Basic generics support is available. Not yet for gwneric type variables, however.

@timtebeek
Copy link
Contributor Author

timtebeek commented Feb 2, 2024

With Knut's support for generics and a very minor new exclusion, these are the new stats with regards to what's supported/skipped:

[INFO] @Matches is currently not supported=1
[INFO] @Repeated is currently not supported=1
[INFO] Method references are currently not supported=1
[INFO] Lambdas are currently not supported=2
[INFO] If statements are currently not supported=8
[INFO] @AlsoNegation is currently not supported=11
[INFO] @Placeholder is currently not supported=15
[INFO] com.google.errorprone.refaster.Refaster is currently not supported=75
[INFO] Generic type parameters are currently not supported=495

All together we now generate 442 before templates; for 298 after templates.

@timtebeek
Copy link
Contributor Author

timtebeek commented Mar 1, 2024

As discussed yesterday with @rickie we might want to detect any use of OnlineDocumentation, and if present add a link to that URL in the documentation that we generate, both to give credit and additional context to what a recipe would effectively do.
https://github.com/PicnicSupermarket/error-prone-support/blob/8f64489fa0dded9255d20c227d0810af1cf52493/refaster-support/src/main/java/tech/picnic/errorprone/refaster/annotation/OnlineDocumentation.java#L44-L48

We already parse any JavaDoc that's present (even if it's often missing); we might want to expand (or generate) the Javadocs present in Error Prone Support to improve how those are presented to our users.

We might also want to create an aggregate yaml recipe across all Refaster recipes generated, to make it easier to apply all recipes at once.

@rickie
Copy link

rickie commented Mar 7, 2024

About the @OnlineDocumentation, it has default values that are used by Picnic. However, others can use the annotation to link to their own website if they have one. So supporting the annotation will not be a "Picnic-specific" solution.

@timtebeek
Copy link
Contributor Author

We can also already generate templates for any rules that use @AlsoNegation, without generating that negation just yet. Adding that negated recipe can then be left as a next step.

@timtebeek
Copy link
Contributor Author

With #78 we now also have some initial links to the docs for the wrapping outer classes. Possible future improvements are

  • Also link to the docs for nested inner classes, when the outer class is annotated
  • Use the OnlineDocumentation annotation assigned or default value, rather than assume the URLs are stable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
Status: Backlog
Development

No branches or pull requests

4 participants