openrewrite / rewrite

Automated mass refactoring of source code.
https://docs.openrewrite.org
Apache License 2.0
2.15k stars 320 forks source link

`RemoveUnusedImports` removes used imports and adds duplicated ones #3275

Closed pzygielo closed 1 year ago

pzygielo commented 1 year ago

What version of OpenRewrite are you using?

I am using

How are you running OpenRewrite?

I am using the Maven plugin, and my project is a single module project

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

https://github.com/pzrep/openrewrite-unused-import-record

What did you expect to see?

I expected to see no changes.

What did you see instead?

https://github.com/pzrep/openrewrite-unused-import-record/actions/runs/5100022041/jobs/9168054354

Depends on number of imports done with * I guess:

  1. star-import is replaced with explicit imports, including duplicated [Record] one:

    diff --git a/src/main/java/pzrep/p2/Client1.java b/src/main/java/pzrep/p2/Client1.java
    index 2533533..edfecd5 100644
    --- a/src/main/java/pzrep/p2/Client1.java
    +++ b/src/main/java/pzrep/p2/Client1.java
    @@ -1,7 +1,10 @@
    package pzrep.p2;
    
    import pzrep.p1.Record;
    -import pzrep.p1.*;
    +import pzrep.p1.A;
    +import pzrep.p1.B0;
    +import pzrep.p1.B1;
    +import pzrep.p1.Record;
    
    class Client1 {
     void f(Record r) {

    This is unfortunate but allows to compile modified class.

  2. required import gets removed

    
    diff --git a/src/main/java/pzrep/p2/Client2.java b/src/main/java/pzrep/p2/Client2.java
    index 2bb52f5..50aea66 100644
    --- a/src/main/java/pzrep/p2/Client2.java
    +++ b/src/main/java/pzrep/p2/Client2.java
    @@ -1,6 +1,5 @@
    package pzrep.p2;

-import pzrep.p1.Record; import pzrep.p1.*;

class Client2 {


This makes modified source to be uncompilable. (That's the original issue I observed with https://www.jooq.org/javadoc/latest/org.jooq/org/jooq/Record.html)
pzygielo commented 1 year ago

I see that duplicated Record import (p. 1 above) is no longer inserted (looks that something in maven plugin 5.0.0->5.2.1 helps :smile:)

joanvr commented 1 year ago

this was very kindly fixed by @tclayton-newr in #3397; you can already try this in our snapshot versions, or with the next release in a week or so

pzygielo commented 1 year ago

Still having my Record removed in

which seems to use rewrite 8.1.10.

pzygielo commented 1 year ago

Still having my Record removed in

pzygielo commented 1 year ago

Still having my Record removed in

pzygielo commented 1 year ago

Could this be reopened (or cloned to new one if needed) by maintainers, please?

pzygielo commented 1 year ago

Is #3527 included https://github.com/openrewrite/rewrite/releases/tag/v8.5.0 (and then in rewrite-maven-plugin:5.5.2)?

pzygielo commented 1 year ago

If it is included in plugin 5.5.2 - then this issue is still not solved, but was closed as completed by GH with keyword.

I prepared reproducer that is linked in opening post. If someone decides to claim that this issue is fixed - may I ask to validate such statement with it?

Or please reopen and close as not planned to not give me the false hope! :grinning: