RewriteTest / YamlResourceLoader Enhancements

[nmck257]

What's changed?

• YamlResourceLoader now uses jackson to try to instantiate imperative recipes with zero args if basic reflection fails. And, reverted change to RecipeSpec which made it scanRuntimeClasspath unnecessarily for yaml recipes. Fixes RewriteTest validation fails on a DeclarativeRecipe calling an imperative recipe with no arguments #3414
• adding RewriteTest/RecipeSpec recipeFromResources option to scan all rewrite yamls on classpath (ignoring classes. Fixes RewriteTest + YamlResourceLoader - recipe validation should be recursive, or perhaps disabled? #3096

Anything in particular you'd like reviewers to focus on?

• In ClasspathScanningLoader, the constructor overloads were starting to get diverse in functionality, so I opted for a factory method for this new scenario. Open to changing it
• In ClasspathScanningLoader, there's this comment, which I'm not sure is accurate today; I think the new case RecipeLifecycleTest::declarativeRecipeChainFromResourcesIncludesImperativeRecipesInDescriptors disproves it

  rewrite/rewrite-core/src/main/java/org/openrewrite/config/ClasspathScanningLoader.java

  Lines 102 to 106 in 1834d4e

  	/**
  	* This must be called _after_ scanClasses or the descriptors of declarative recipes will be missing any
  	* non-declarative recipes they depend on that would be discovered by scanClasses
  	*/
  	private void scanYaml(ClassGraph classGraph, Properties properties, Collection<? extends ResourceLoader> dependencyResourceLoaders, @Nullable ClassLoader classLoader) {

• I considered removing the reflective attempt to instantiate the recipe and only using jackson, but that led to a regression with recipes like this, because jackson (for some reason) picked the 1-arg constructor (passing a null value) instead of the annotated zero-arg constructor (which would have set the required field by default) https://github.com/openrewrite/rewrite-migrate-java/blame/main/src/main/java/org/openrewrite/java/migrate/UseJavaUtilBase64.java

Anyone you would like to review specifically?

@sambsnyd

Have you considered any alternatives or workarounds?

• Scanning the full runtime classpath when you're just looking for Yaml files is slow and doesn't seem beneficial
• You can also explicitly list the yamls you care about when defining your RecipeSpec, but with recursive validation, if you're testing a recipe which chains to many recipes across many files, then that becomes painful to maintain

Checklist

• I've added unit tests to cover both positive and negative cases
• I've added the license header to any new files through ./gradlew licenseFormat
• I've used the IntelliJ IDEA auto-formatter on affected files
• I've updated the documentation (if applicable)

[nmck257]

looking at the failing test, it was actually failing for the "wrong" reason before: "recipe 'org.openrewrite.text.ChangeText' does not exist."

now, it's passing, because the single toText field on ChangeText isn't actually annotated as non-nullable so validation doesn't consider it to be required.

I'm thinking we fix it by adjusting NullUtils::findNonNullFields to include fields annotated with Option and not explicitly setting required=false ? Will draft that commit now and add to the PR

edit done