`RemoveUnusedImports` creating ambiguous imports when there's explicit imports accompanying wildcards

[protocol7]

What version of OpenRewrite are you using?

org.openrewrite:rewrite-core:8.56.1

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

When using RemoveUnusedImports and GoogleJavaFormat together with overlapping wildcard imports, you may end up with ambiguous imports, making the code no longer compile.

Here's a test case:

  @Test
  public void wildcardImports() {
    JavaParser.Builder<?, ?> parser =
        JavaParser.fromJavaVersion()
            .styles(ClasspathLoader.loadStyle("org.openrewrite.java.GoogleJavaFormat"));

    rewriteRun(
        spec ->
            spec.recipe(new RemoveUnusedImports()).parser(parser).expectedCyclesThatMakeChanges(2),
        java(
            """
            package com.helloworld;

            import java.sql.*;
            import java.sql.Date;
            import java.util.*;

            public final class Main {
              public static void main(final String... args) {
                final Date date = new Date(123);
                final java.util.Date date2 = new java.util.Date(123);
              }
            }
            """,
            """
            package com.helloworld;

            import java.sql.Date;

            public final class Main {
              public static void main(final String... args) {
                final Date date = new Date(123);
                final java.util.Date date2 = new java.util.Date(123);
              }
            }
            """));
  }

Resulting in:

org.opentest4j.AssertionFailedError: [Unexpected result in "com/helloworld/Main.java":
diff --git a/com/helloworld/Main.java b/com/helloworld/Main.java
index e2bf8f5..4374a31 100644
--- a/com/helloworld/Main.java
+++ b/com/helloworld/Main.java
@@ -1,6 +1,7 @@
 package com.helloworld;

 import java.sql.Date;
+import java.util.Date;

 public final class Main {
]
expected:
  "package com.helloworld;

  import java.sql.Date;

  public final class Main {
    public static void main(final String... args) {
      final Date date = new Date(123);
      final java.util.Date date2 = new java.util.Date(123);
    }
  }"
 but was:
  "package com.helloworld;

  import java.sql.Date;
  import java.util.Date;

  public final class Main {
    public static void main(final String... args) {
      final Date date = new Date(123);
      final java.util.Date date2 = new java.util.Date(123);
    }
  }"
	at org.openrewrite.test.RewriteTest.assertContentEquals(RewriteTest.java:622)
	at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:511)
	at org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:130)

[protocol7]

This may seem like a very rare corner case, but we've had multiple examples of this happening in our code base, in particular when using protobuf generated classes, which tend to generate many imports and overlapping class names.

[timtebeek]

Good to have a runnable report, thanks! Can't say I've used that GoogleJavaFormat myself, but the implementation is telling:

rewrite/rewrite-java/src/main/resources/META-INF/rewrite/google-java-format.yml

Lines 21 to 27 in 43eab96

	- org.openrewrite.java.style.ImportLayoutStyle:
	classCountToUseStarImport: 999
	nameCountToUseStarImport: 999
	layout:
	- import static all other imports
	- <blank line>
	- import all other imports

Still strange that you end up with a import java.util.Date import when that's only used fully qualified. Did you try to troubleshoot already?

[protocol7]

Updated the test case slightly, the List isn't needed, that came from my trying to reduce down one of our real cases.

Yes, I assume that there is some interaction between the recipe cleaning up imports, and the style resolving wildcard imports. I have not dug any further.

[timtebeek]

Thanks! Quick note here that the ClasspathLoader is likely internal, but this achieves the same:

JavaParser.Builder<?, ?> parser = JavaParser.fromJavaVersion().styles(singletonList(
  new NamedStyles(
    Tree.randomId(), "test", "test", "test", emptySet(),
    List.of(new ImportLayoutStyle(
      999,
      999,
      List.of(new ImportLayoutStyle.Block.ImportPackage(true, "all other imports", false)),
      List.of()))
  )
));

But it also seems to replicate without any explicit style:

    @Issue("https://github.com/openrewrite/rewrite/issues/5703")
    @Test
    public void wildcardImports() {
        rewriteRun(
          spec -> spec.recipe(new RemoveUnusedImports()).expectedCyclesThatMakeChanges(2),
          java(
            """
              package com.helloworld;
              
              import java.sql.*;
              import java.sql.Date;
              import java.util.*;
              
              public final class Main {
                public static void main(final String... args) {
                  final Date date = new Date(123);
                  final java.util.Date date2 = new java.util.Date(123);
                }
              }
              """,
            """
              package com.helloworld;
              
              import java.sql.Date;
              
              public final class Main {
                public static void main(final String... args) {
                  final Date date = new Date(123);
                  final java.util.Date date2 = new java.util.Date(123);
                }
              }
              """
          )
        );
    }

[protocol7]

But it also seems to replicate without any explicit style

Nice find. I tried to do that earlier but must have screwed up somehow as it no longer failed.

I found another case in our code base where instead this happened with a class in the same package. Here's the updated test cases:

  @Test
  public void wildcardImports() {
    rewriteRun(
        spec ->
            spec.recipe(new RemoveUnusedImports()).expectedCyclesThatMakeChanges(2),
        java(
            """
            package com.helloworld;

            import java.sql.*;
            import java.sql.Date;
            import java.util.*;

            public final class Main {
              final Date date = new Date(123);
              final java.util.Date date2 = new java.util.Date(123);
            }
            """,
            """
            package com.helloworld;

            import java.sql.Date;

            public final class Main {
              final Date date = new Date(123);
              final java.util.Date date2 = new java.util.Date(123);
            }
            """));
  }

  @Test
  public void wildcardImports2() {
    rewriteRun(
        spec ->
            spec.recipe(new RemoveUnusedImports()),
        java(
            """
            package com.helloworld;

            public class Date {}
            """),
        java(
            """
            package com.helloworld;

            import java.util.*;

            public final class Main {
              final Date date = new Date();
              final java.util.Date date2 = new java.util.Date(123);
            }
            """,
            """
            package com.helloworld;

            public final class Main {
              final Date date = new Date();
              final java.util.Date date2 = new java.util.Date(123);
            }
            """));
  }