openrewrite / rewrite-maven-plugin

OpenRewrite's Maven plugin.
https://openrewrite.github.io/rewrite-maven-plugin/plugin-info.html
Apache License 2.0
127 stars 67 forks source link

Ignore recipes that cannot be parsed #159

Closed okundzich closed 3 years ago

okundzich commented 3 years ago

If yaml files containing recipes are not correctly formatted, have missing options, required fields, descriptions, etc, these recipes should be ignored in general. This is for the use case when someone publishes a bad recipe by mistake, so it doesn't prevent the rest of rewrite from functioning. Since we are looking for recipes on the class path, we can find all sorts of misconfigurations.

Build plugins would need an option/way to describe what is wrong with a specific recipe. This is for the use case when a person is creating a new recipe, and it doesn't activate (it's not found per above), need some way to find out what is wrong with it. For example, discover task with a recipe name will print what is wrong with it (missing description, missing require option, invalid option value etc.) Whatever user experience is most appropriate for these plugins.

okundzich commented 3 years ago

@aegershman

jkschneider commented 3 years ago

@gadams00 I believe the Maven plugin adds logging output for any recipe that doesn't pass validation. @okundzich expressed the intent of this issue here:

This is for the use case when someone publishes a bad recipe by mistake

What we don't want is for some third party (say a library maintainer) that creates a recipe to permanently cause error logging output in the builds of downstream projects, especially when in this example the library comes in transitively to the project. This leaves the end user unable to effectively do anything to deal with the logged error, since they may not own the project that contains the invalid recipe.

The validation added in 7ba9749 is a useful step in the right direction, but more work needs to be done to ensure that the build tool plugins don't throw.

jkschneider commented 3 years ago

Moved to this project from https://github.com/openrewrite/rewrite, because rewrite core has done enough. Now we need to change the default behavior of the build tool plugins to not fill log output with errors when invalid recipes are encountered.

aegershman commented 3 years ago

@okundzich + @jkschneider I believe this is taken care of by #161 (and https://github.com/openrewrite/rewrite-gradle-plugin/pull/47 in gradle)? -- for any activeRecipe, by default, we log the invalid recipe's validation failures to [ERROR], and ignore it by continuing to process.

If a user wanted to fail when encountering any invalid recipe (or downstream subrecipe) in activeRecipe, you can set failOnInvalidActiveRecipes = true.

This is only on activeRecipes, so it doesn't blow up the logs. We don't do this with all "available" recipes, only the ones that a user has provided to activeRecipe.