Static import is removed and leaves ambiguous reference behind

[Laurens-W]

What's your motivation?

RemoveUnusedImports recipe should not remove static import when this leaves an ambiguous reference behind

Anyone you would like to review specifically?

@timtebeek

Have you considered any alternatives or workarounds?

Any additional context

static types were not being added to the typesInUse object on a CompilationUnit

Checklist

• I've added unit tests to cover both positive and negative cases
• I've read and applied the recipe conventions and best practices
• I've used the IntelliJ IDEA auto-formatter on affected files

[github-actions]

Some suggestions could not be made:

• rewrite-test/src/test/java/org/openrewrite/test/internal/RewriteTestTest.java
  • lines 107-107

[timtebeek]

Capturing the failure here:

expected:

  package org.test;	
  	
  import static org.a.ABC.*;	
  import static org.b.DEF.*;	
  import static org.a.ABC.ALL;	
  	
  public class Test {	
    private String abc = ALL;	
    private String a = A;	
    private String b = B;	
    private String c = C;	
    private String d = D;	
    private String e = E;	
    private String f = F;	
  }

but was:

  package org.test;	
  	
  import static org.a.ABC.*;	
  import static org.b.DEF.*;	
  	
  public class Test {	
    private String abc = ALL;	
    private String a = A;	
    private String b = B;	
    private String c = C;	
    private String d = D;	
    private String e = E;	
    private String f = F;	
  }

[github-actions]

Some suggestions could not be made:

• rewrite-test/src/test/java/org/openrewrite/test/internal/RewriteTestTest.java
  • lines 107-107

[timtebeek]

This indeed seems to match the case reported, thanks! Lets look at what we can do to fix the issue.

[timtebeek]

Perhaps good to briefly evaluate if we can fold in a fix for this issue as well

• ChangeType is not addressing import conflicts #4452

[timtebeek]

I've reverted the earlier change to TypesInUse, as in reality it's not that type that's used, but the variables that were already part of TypesInUse. Using just the imports, the variables, and the owner of those variables I was able to do a change local to RemoveUnusedImports to detect ambiguous member references from wildcards, and not make changes in those cases. At the same time I covered an additional case where we can clear out one explicit import due to wildcards imports while keeping another explicit import.

This should ensure there's minimal impact from the change, as we don't want to incorrectly assign types as used, and risk those being left over as imports elsewhere when in reality not used.

Thanks for getting this started!