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

OrganizeImports rule can break compilation #2006

Open
satorg opened this issue Jun 6, 2024 · 2 comments
Open

OrganizeImports rule can break compilation #2006

satorg opened this issue Jun 6, 2024 · 2 comments

Comments

@satorg
Copy link

satorg commented Jun 6, 2024

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:

  1. 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._
  2. Prefix conflicting imports with _root_.

I think, these two approaches can be made configurable.

@bjaglin
Copy link
Collaborator

bjaglin commented Jun 26, 2024

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).

@bjaglin bjaglin added the bug label Jun 26, 2024
@satorg
Copy link
Author

satorg commented Jun 26, 2024

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants