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

Call ShortenFullyQualifiedTypeReferences after visit #18

Merged
merged 5 commits into from
Aug 14, 2023
Merged

Conversation

timtebeek
Copy link
Contributor

What's changed?

We now call doAfterVisit(new ShortenFullyQualifiedTypeReferences().getVisitor()) from within the if statement.

What's your motivation?

I think this is what we discussed, such that templates can use fully qualified references for convenience, yet insert shortened references in practice.

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

Did you indeed intend to call the visitor like this, or in a more targeted way?

Anyone you would like to review specifically?

@knutwannheden

Have you considered any alternatives or workarounds?

Not sure if there's any other ways we might want to explore.

Any additional context

This ought to help PRs such as this on

@timtebeek timtebeek self-assigned this Aug 13, 2023
@timtebeek timtebeek added the enhancement New feature or request label Aug 13, 2023
Copy link
Contributor

@knutwannheden knutwannheden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding the ShortenFullyQualifiedTypeReferences recipe on its own is not very interesting. We should IMHO change the template generation code so that it only produces fully qualified references to types and static members. But this will be more work, as I think I remember that the Refaster processor currently lets the other annotation processor generate the template code. Possibly we should for more flexibility instead inline it into the Refaster processor.

@timtebeek
Copy link
Contributor Author

As discussed over on openrewrite/rewrite-migrate-java#272 (comment)

@timtebeek timtebeek merged commit c15a636 into main Aug 14, 2023
@timtebeek timtebeek deleted the shorten_fqn branch August 14, 2023 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants