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

ChangeType is not addressing import conflicts #4452

Closed
desantisernesto opened this issue Aug 27, 2024 · 2 comments · Fixed by #4458
Closed

ChangeType is not addressing import conflicts #4452

desantisernesto opened this issue Aug 27, 2024 · 2 comments · Fixed by #4458
Assignees
Labels
bug Something isn't working

Comments

@desantisernesto
Copy link

desantisernesto commented Aug 27, 2024

What version of OpenRewrite are you using?

I am using

  • OpenRewrite v8.33.8
  • Gradle plugin v8.4
  • rewrite-java v8.33.8

How are you running OpenRewrite?

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

public class ChangeTypeTest implements RewriteTest {

    @Override
    public void defaults(RecipeSpec spec) {
        spec.recipe(
                new ChangeType("javax.annotation.Nonnull", "org.checkerframework.checker.nullness.qual.NonNull", null));
    }

    @Test
    void conflictingImports() {

        // language=java
        rewriteRun(
                // dissabling validation to avoid the 'Identifier type is missing or malformed' error. I couldn't add
                // the lombok dependency to the test.
                spec -> spec.typeValidationOptions(TypeValidation.none()),
                java(
                        """
                        import lombok.NonNull;
                        import javax.annotation.Nonnull;
                        import org.immutables.value.Value;

                        @Value.Immutable
                        @Value.Style(passAnnotations = Nonnull.class)
                        public interface ConflictingImports {
                                private void lombokMethod(@NonNull final String lombokNonNull){}
                        }
                        """,
                        """
                        import lombok.NonNull;
                        import org.immutables.value.Value;

                        @Value.Immutable
                        @Value.Style(passAnnotations = org.checkerframework.checker.nullness.qual.NonNull.class)
                        public interface ConflictingImports {
                                private void lombokMethod(@NonNull final String lombokNonNull){}
                        }
                        """));
    }
}

What did you expect to see?

If where is already a class imported with the same name in the original code (lombok.NonNull in the example), the recipe should fully-qualified the new Type included (org.checkerframework.checker.nullness.qual.NonNull).

What did you see instead?

I'm seeing two imports for the same name type, from different packages.

import lombok.NonNull;
import org.checkerframework.checker.nullness.qual.NonNull;

What is the full stack trace of any errors you encountered?

Unexpected result in "ConflictingImports.java":

diff --git a/ConflictingImports.java b/ConflictingImports.java
index 4172665..31c1781 100644
--- a/ConflictingImports.java
+++ b/ConflictingImports.java
@@ -1,8 +1,9 @@ 
 import lombok.NonNull;
+import org.checkerframework.checker.nullness.qual.NonNull;
 import org.immutables.value.Value;
 
 @Value.Immutable
-@Value.Style(passAnnotations = org.checkerframework.checker.nullness.qual.NonNull.class)
+@Value.Style(passAnnotations = NonNull.class)
 public interface ConflictingImports {
         private void lombokMethod(@NonNull final String lombokNonNull){}
 }
\ No newline at end of file

expected:

  import lombok.NonNull;
  import org.immutables.value.Value;
  
  @Value.Immutable
  @Value.Style(passAnnotations = org.checkerframework.checker.nullness.qual.NonNull.class)
  public interface ConflictingImports {
          private void lombokMethod(@NonNull final String lombokNonNull){}
  }

but was:

  import lombok.NonNull;
  import org.checkerframework.checker.nullness.qual.NonNull;
  import org.immutables.value.Value;
  
  @Value.Immutable
  @Value.Style(passAnnotations = NonNull.class)
  public interface ConflictingImports {
          private void lombokMethod(@NonNull final String lombokNonNull){}
  }

Extra information, slack thread about this issue:

https://rewriteoss.slack.com/archives/C01A843MWG5/p1723733741135269?thread_ts=1723658700.585459&cid=C01A843MWG5

Are you interested in contributing a fix to OpenRewrite?

Not at this moment. Maybe in the future.

@timtebeek
Copy link
Contributor

Thanks for the report! We're working on something similar in

Lets see if we can resolve both of those in that same PR.

@Laurens-W
Copy link
Contributor

Thanks for the report! We're working on something similar in

Lets see if we can resolve both of those in that same PR.

This issue seems separate enough from the ambiguous static imports that it warrants a separate PR, I've added the example provided in a draft PR as seen above :)

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.

3 participants