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

ShortenFullyQualifiedNames don't shorten FQN Annotations #4222

Open
MBoegers opened this issue Jun 1, 2024 · 6 comments
Open

ShortenFullyQualifiedNames don't shorten FQN Annotations #4222

MBoegers opened this issue Jun 1, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@MBoegers
Copy link
Contributor

MBoegers commented Jun 1, 2024

While working on PR 4217 I discovered that ShortenFullyQualifiedNames does not work as expected for Annotations with Full Qualified Name. As Java Language Spec 9.7.1 states @packages.typename is the FQN form for an Annotation, therefore @java.lang.Overwrite should be transformed to @Overwrite.

What version of OpenRewrite are you using?

I am using the current main branch and unit tests.

How are you running OpenRewrite?

Running ShortenFullyQualifiedNamesTest with reproducing test.

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

See Gist https://gist.github.com/MBoegers/9c805b9caa35144466d9f380ddbb40bd

class A {
    @java.lang.Overwrite
    void foo() {}
}

What did you expect to see?

See expectations in Gist https://gist.github.com/MBoegers/9c805b9caa35144466d9f380ddbb40bd

class A {
    @Overwrite
    void foo() {}
}

What did you see instead?

The recipe makes no changes.

Are you interested in contributing a fix to OpenRewrite?

@MBoegers MBoegers added the bug Something isn't working label Jun 1, 2024
@knutwannheden
Copy link
Contributor

The java.lang types are unfortunately a bit tricky. The reason being that if a type with the same simple name exists in the current package, then that takes precedence over the java.lang type in the absence of any import (that is IIRC). So the visitor in question would have to know if a type by that name exists in the current package.

Is the issue also reproducible for other annotation types?

@MBoegers
Copy link
Contributor Author

MBoegers commented Jun 1, 2024

Yes thought about that edge case as well. If you see the gist the same problem shows up if you use a different annotation like @org.jetbrains.annotations.NotNull. Sorry the gist like was broken 😬

@knutwannheden
Copy link
Contributor

The diff that I am getting for that second test when running it is this:

diff --git a/TypeAnnotationTest.java b/TypeAnnotationTest.java
index 1da8705..aa3a024 100644
--- a/TypeAnnotationTest.java
+++ b/TypeAnnotationTest.java
@@ -1,8 +1,10 @@ 
-import org.jetbrains.annotations.NotNull;
 import java.sql.DatabaseMetaData;
 import java.util.List;
+
 import java.lang.annotation.*;
 
+import org.jetbrains.annotations.NotNull;
+
 class TypeAnnotationTest {
     @NotNull
     String test() {}

So it looks like the test expection's import order is "wrong". When I align it, the test succeeds. So unless I am missing something, the only issue is with java.lang, but I don't see how that can easily be fixed in the short term and if you can live with the slight risk that the code where you apply a JavaTemplate contains a type in the current package with the same name as an unqualified java.lang type in the template, then that should be a valid workaround. I.e. not fully qualify java.lang types in the template.

knutwannheden added a commit that referenced this issue Jun 2, 2024
@MBoegers
Copy link
Contributor Author

MBoegers commented Jun 2, 2024

I'll analyse and give you an answer by tomorrow evening CEST

@MBoegers
Copy link
Contributor Author

MBoegers commented Jun 4, 2024

I can confirm that the recipe works as expected, with the known exception to java.lang.* annotations. @knutwannheden and @timtebeek this is fine for me.

@timtebeek
Copy link
Contributor

So do I understand correctly then that we can close this issue as a known limitation?

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
Status: Backlog
Development

No branches or pull requests

3 participants