AddImport: Handle import ambiguity#5439
Conversation
…mberReference.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…mberReference.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
Nice work @amishra-u ! |
|
Personally, I prefer doing this the other way around by using fully qualified names in the template and then apply the ShortenFullyQualifiedTypeReferences visitor to add imports where possible. |
|
@knutwannheden Just to clarify—are you suggesting that we update all existing recipes to use fully qualified names in the template, and then rely on the ShortenFullyQualifiedTypeReferences visitor to handle the imports? Somewhat similar idea #5435 |
|
@sambsnyd @knutwannheden When you have a moment, could you please share your thoughts on this issue? |
|
@amishra-u No, I think it makes sense for AddImport to have this extra safety, as it otherwise ends up braking code. This way we can keep both alternatives in place and let people use either one. |
|
We seem to be having some downstream tests fail which now see FQN not shortened. Could that be cause by the changes here? |
|
@timtebeek fixed it here also fixed all the test in rewrite-testing-frameworks. openrewrite/rewrite-testing-frameworks#731 we may use the same approach for rewrite-apache and rewrite-migrate-java |
What's changed?
Previously, the AddImport logic did not account for import ambiguity, leading to compilation errors when ambiguous imports were added. This update introduces a check for ambiguity before adding an import. If ambiguity is detected, the code now fully qualifies the relevant identifiers instead of adding the import
Checklist