search

openrewrite / rewrite-docs

Stores the markdown documents used to generate docs.openrewrite.org
https://docs.openrewrite.org
Apache License 2.0
39 stars 42 forks source link

Issue with Precondition documentation generation #250

Open SimonVerhoeven opened 8 months ago

SimonVerhoeven commented 8 months ago

For recipes such as https://docs.openrewrite.org/recipes/java/spring/boot3/enablevirtualthreads which contain a precondition (in this case org.openrewrite.java.search.HasJavaVersion the recipe list shows Precondition bellwether which links to https://github.com/openrewrite/rewrite-docs/blob/master/reference/recipes/config/declarativerecipe$preconditionbellwether.md rather than the expected HasJavaVersion recipe + link.

timtebeek commented 8 months ago

Thanks for pointing this out! You've found a neat implementation detail around how we handle preconditions in declarative recipes; the message you saw originates here. Since the class is package private the best I could do for now is recognize such recipes by their message, and then skip them.

timtebeek commented 8 months ago

Not fixed yes as per: https://openrewrite.github.io/rewrite-recipe-markdown-generator/reference/recipes/java/spring/boot3/enablevirtualthreads.html

timtebeek commented 6 months ago

So the issue here is that the bellwether is shown on the Yaml tab: image

That's an implementation detail; not something for folks to implement. Might mean we need to move this to https://github.com/openrewrite/rewrite-recipe-markdown-generator

There it shows up in the yaml tab as

type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.spring.boot3.EnableVirtualThreads
displayName: Enable Virtual Threads on Java 21
description: Set `spring.threads.virtual.enabled` to `true` in `application.properties` or `application.yml`.
recipeList:
  - org.openrewrite.config.DeclarativeRecipe$PreconditionBellwether
  - org.openrewrite.java.spring.AddSpringProperty
timtebeek commented 6 months ago

Compare that to how the recipe is actually implemented: https://github.com/openrewrite/rewrite-spring/blob/b76e03437143632ec9d42ffc0a5e7af2b88f11c9/src/main/resources/META-INF/rewrite/spring-boot-32.yml#L63-L73

type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.spring.boot3.EnableVirtualThreads
displayName: Enable Virtual Threads on Java 21
description: Set `spring.threads.virtual.enabled` to `true` in `application.properties` or `application.yml`.
preconditions:
  - org.openrewrite.java.search.HasJavaVersion:
      version: 21.X
recipeList:
  - org.openrewrite.java.spring.AddSpringProperty:
      property: spring.threads.virtual.enabled
      value: true
timtebeek commented 6 months ago

As discussed on the above PR: it would be better to show the configured precondition & recipe arguments in the bellwether. So right now what we show is:

Current

No bellwether (good), no precondition (missed opportunity, it's in the bellwether), no arguments to AddSpringProperty, (leading to a broken yaml recipe)

type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.spring.boot3.EnableVirtualThreads
displayName: Enable Virtual Threads on Java 21
description: Set `spring.threads.virtual.enabled` to `true` in `application.properties` or `application.yml`.
recipeList:
  - org.openrewrite.java.spring.AddSpringProperty

Ideal

As per the source

type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.spring.boot3.EnableVirtualThreads
displayName: Enable Virtual Threads on Java 21
description: Set `spring.threads.virtual.enabled` to `true` in `application.properties` or `application.yml`.
preconditions:
  - org.openrewrite.java.search.HasJavaVersion:
      version: 21.X
recipeList:
  - org.openrewrite.java.spring.AddSpringProperty:
      property: spring.threads.virtual.enabled
      value: true
mike-solomon commented 6 months ago

As mentioned in this issue, I believe that changes will need to be made in rewrite itself (specifically RecipeDescriptor) to allow for Preconditions to be made available to the docs.