Validation error messages can be ambigious

[Bananeweizen]

What is the smallest, simplest way to reproduce the problem?

Enable these rules, all without parameters

org.openrewrite.java.logging.log4j.ParameterizedLogging
org.openrewrite.java.logging.ParameterizedLogging
org.openrewrite.java.logging.slf4j.ParameterizedLogging

What did you see?

[ERROR] Recipe validation error in ParameterizedLogging.methodPattern: is required

This can be very confusing, when looking up the recipe by (short) name. Only 1 of 3 has parameters. Therefore the user may end up adding the parameters to the wrong recipe, reading the wrong documentation etc.

Even though avoiding the full qualified names is generally a nice idea, it can lead to additional trouble.

Are you interested in contributing a fix to OpenRewrite?

Maybe. Depends on which direction a fix would go.

[timtebeek]

Hmm; that's a bit unfortunate indeed, but hopefully rare; In this case it's the shortest one that's missing its options; glad at least that we correctly reject that in the validation.

Indeed not sure what to do specifically here, but fully qualifying the recipe name would only help if there are more such identical class names, which I think is reasonably rare, especially not expecting those to be used together (folks use one or the other).

[timtebeek]

I could see a case though to always fully qualify; especially with a Spring Boot 3.x best practices containing 2556 recipes, it might make sense to always fully qualify the recipe name.

[junseoparkk]

Hi, I’d like to work on this issue.

From the comment, it seems that always showing the fully qualified recipe name in validation error messages might be the preferred direction.
Would it be okay if I proceed with that approach? If so, could you please assign this issue to me?

Thank you.

[timtebeek]

hi @junseoparkk ; appreciate the offer to help! Welcome to get started; we'll assign the issue to you as soon as there is a draft PR. :)

[junseoparkk]

Thanks @timtebeek, I’ll start working on this and will open a draft PR soon!

[Attyuttam]

Hi, I see this issue to be still open. if this is still an issue, I would like to pick it up.

[timtebeek]

Thanks! The validation error is printed in the Maven plugin: https://github.com/openrewrite/rewrite-maven-plugin/blob/63175862aca93b57ad85364505f09dbb58b8cea5/src/main/java/org/openrewrite/maven/AbstractRewriteBaseRunMojo.java#L142-L144

Based on the missing property passed into this method: https://github.com/openrewrite/rewrite/blob/8ce5308a9557dd6ca91253a5c8fb7e6c34a0ef5a/rewrite-core/src/main/java/org/openrewrite/Validated.java#L110-L114

Likely coming from this method where we (now) only pass the simple name: https://github.com/openrewrite/rewrite/blob/cdf1a7aa6f4b2d610677d179eebd1a458ec8823b/rewrite-core/src/main/java/org/openrewrite/Recipe.java#L469-L475

If you change the clazz.getSimpleName() to clazz.getName() you'll likely have solved this issue.

[Vishalup29]

@timtebeek Hi! 👋

This would be my second contribution to OpenRewrite, and I’m excited to learn and help. I’ve prepared a fix based on the guidance shared above (PR raised), and I’m fully open to feedback, corrections, or being pointed in the right direction if I’ve missed anything.

Thank you for the opportunity — looking forward to your review! 🙏😊