Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

missing SHORTEN_NAMES for parameterless recipes #66

Closed
Bananeweizen opened this issue Jan 22, 2024 · 3 comments · Fixed by #68
Closed

missing SHORTEN_NAMES for parameterless recipes #66

Bananeweizen opened this issue Jan 22, 2024 · 3 comments · Fixed by #68
Assignees
Labels
bug Something isn't working

Comments

@Bananeweizen
Copy link
Contributor

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

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

recipe.append(" return embed(").append(after).append(".apply(getCursor(), elem.getCoordinates().replace()), getCursor(), ctx);\n");
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.

@Bananeweizen Bananeweizen added the bug Something isn't working label Jan 22, 2024
@timtebeek timtebeek moved this to Backlog in OpenRewrite Jan 22, 2024
@timtebeek
Copy link
Contributor

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

@Bananeweizen
Copy link
Contributor Author

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

@timtebeek
Copy link
Contributor

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!

Bananeweizen added a commit to Bananeweizen/rewrite-templating that referenced this issue Jan 27, 2024
Deduplicate the logic for creating the recipe code to avoid an
inconcistency in how the embedOptions are used.

Fixes openrewrite#66.
knutwannheden pushed a commit that referenced this issue Jan 27, 2024
Deduplicate the logic for creating the recipe code to avoid an
inconcistency in how the embedOptions are used.

Fixes #66.
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenRewrite Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants