openrewrite / rewrite-templating

Automated templating using code snippets.
Apache License 2.0
16 stars 7 forks source link

missing SHORTEN_NAMES for parameterless recipes #66

Closed Bananeweizen closed 9 months ago

Bananeweizen commented 10 months ago

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

        @BeforeTemplate
        String before() {
            return "foo";
        }

        @AfterTemplate
        String after() {
            return SomeClass.SOME_CONSTANT;
        }

What did you expect to see?

The generated recipe should contain SHORTEN_NAMES. It's meant to be generated with every recipe, according to https://github.com/openrewrite/rewrite-templating/blob/0e4a1440df56537041bbc86294dc77f22758c288/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java#L272

What did you see instead?

No additional recipe references. Therefore running the recipe leads to a FQN for SomeClass.SOME_CONSTANT. (Workaround: Add the ShortenFullyQualifiedTypeReferences to the recipe list explicitly).

Root cause is https://github.com/openrewrite/rewrite-templating/blob/0e4a1440df56537041bbc86294dc77f22758c288/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java#L278 not adding the embedOptions collection (it does this correctly in https://github.com/openrewrite/rewrite-templating/blob/main/src/main/java/org/openrewrite/java/template/processor/RefasterTemplateProcessor.java#L284).

Are you interested in contributing a fix to OpenRewrite?

Yes. I would combine the 2 code blocks to remove the code duplication, since such code duplications will lead to similar failures again.

timtebeek commented 10 months ago

Thanks for the in depth look and offer to help @Bananeweizen ; do you want me to assign this issue to you then?

Bananeweizen commented 10 months ago

Yes, I already started debugging this, but might take a bit more of my spare time to get used to the environment.

timtebeek commented 10 months ago

Sure! Appreciate you exploring a type of recipe we hadn't tested before. Looking over the example once more it seems like a reimplementation of Replace String literal with constant, but with compile time checking; neat!