Support additional Refaster features seen in error-prone-support

[timtebeek]

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.

• [x] com.google.errorprone.refaster.Refaster as seen in EqualityRules.java used ~326 times

  • 21

• [x] @UseImportPolicy as seen in MockitoRules.java used ~235 times

  • 53

• [x] @DoNotCall as seen in TestNGToAssertJRules.java used 3 times
• [ ] Generic type parameters are currently not supported as seen in ImmutableSortedMapRules.java used ~496 times
• [ ] @AlsoNegation as seen in BigDecimalRules.java used ~19 times
• [ ] @Placeholder as seen in LongStreamRules.java used ~15 times
• [ ] @Repeated as seen in WebClientRules.java used ~9 times
• [ ] @Matches & @NotMatches as seen in ReactorRules.java used ~1 + 2 times
• [ ] "If statements are currently not supported" used ~8 times
• [ ] "Lambdas are currently not supported" used ~2 times
• [ ] "Method references are currently not supported" used ~1 times
• [ ] Support of exclude templates that contain more than one statement, as the template matching would fail

[Stephan202]

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

[timtebeek]

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]

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]

@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]

@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.

• https://github.com/openrewrite/rewrite-templating/pull/53

[timtebeek]

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]

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

[timtebeek]

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

• https://github.com/openrewrite/rewrite-templating/pull/72

  [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]

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]

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]

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]

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