`OrganizeImports` rule can break compilation

satorg commented 1 month ago

Actual Behavior

A real example. Consider a project with the following two dependencies:

Also consider a scala file with the following imports section:

import io.circe._
import fs2._

After applying the OrganizeImports rule the section will be simply re-arranged:

import fs2._
import io.circe._

Which breaks the compilation, because there's io sub-package inside fs2 and therefore the compiler will be assuming that we're trying to import fs2.io.circe._ at the second line.

Expected Behavior

I can think of two strategies of how it could be mitigated:

Do not push import io.whatever below import fs2._ if the latter already contains io. E.g., the following imports sections:
```
import gg.hh.ii._
import io.circe._
import aa.bb.cc._
import fs2._
```
could be re-arranged this way in such a case:
```
import aa.bb.cc._
import io.circe._
import fs2._
import gg.hh.ii._
```
Prefix conflicting imports with _root_.

I think, these two approaches can be made configurable.

bjaglin commented 3 weeks ago

Thanks @satorg for the clear report and the suggested solutions! This is an annoying scenario that does not look trivial to handle. I am surprised it hasn't been reported before.

Conflict detection

The naive approach to detect potential conflicts would be for each non-_root_, non-relative import to check whether its first segment (io in the example) might be in scope because it's imported either through (a) a regular import or (b) a sub-package of a wildcard import. (a) is trivial as it's just a reference check, but (b) is not as we need to rely on information not captured in SemanticDB. It can be done with scalameta's GlobalSymbolTable as used in ExplicitResultTypes, but this works only on Scala 2, so some custom classpath-inspection code probably needs to be written. In any case, we should really avoid calling the compiler as it would bring even more complexity.

Strategies

I would add another potential strategy (3), which might be a good tradeoff between the complexity of (1) and the user-unfriendliness/verbosity of (2): keep these potential conflicts in a separate group, that remains unshuffled and comes before all others. This would be similar to how relative imports are treated today. In your second example, that would mean

import io.circe._
import aa.bb.cc._
import fs2._
import gg.hh.ii._

I am happy to help and review PRs bringing either strategy (or several, behind a configuration flag).

satorg commented 3 weeks ago

@bjaglin , thank you for the clarification.

Just for a record: as you might know, IntelliJ IDEA has its own "Organize Imports" tool. And looks like the most recent IDEA version together with the most recent Scala plugin is able to solve this kind of conundrum. However, I found out that it struggles with another one, which scalafix seems to be able to get around.

For those who might be interested in tracking both, I filed a corresponding issue in their tracker too: SCL-22769

scalacenter / scalafix