Skip to content
This repository has been archived by the owner on Jul 14, 2023. It is now read-only.

OrganizeImports produces non-sensical imports #137

Open
sideeffffect opened this issue Nov 26, 2020 · 5 comments
Open

OrganizeImports produces non-sensical imports #137

sideeffffect opened this issue Nov 26, 2020 · 5 comments
Labels
bug Something isn't working

Comments

@sideeffffect
Copy link
Contributor

OrganizeImports produced non-sensical imports, like

// test-tests/shared/src/test/scala/zio/test/environment/RandomSpec.scala

import scala.util.Random.random.Random
// test/shared/src/main/scala/zio/test/package.scala

import zio.test.PlatformSpecific.TestEnvironment.collection.immutable.SortedSet
...
import scala.collection.immutable.SortedSet.duration.Duration

Discovered in PR zio/zio#4449

Full commit with all the changes OrganizeImports made is here: zio/zio@495623e

These non-sensical imports had to be manually fixed by this commit zio/zio@689c205

The original snapshot of the codebase is here https://github.com/zio/zio/tree/1816c4232ee16f8e50ef4291b7810ebdea4e8529 + you have to adjust the OrganizeImports settings to

OrganizeImports {
  expandRelative = true
  groupedImports = Merge
  # IntelliJ IDEA's order so that they don't fight each other
  groups = [
    "java."
    "*"
    "scala."
  ]
}

RemoveUnused {
  imports = false // handled by OrganizeImports
}

It should be 100% reproducible, just run fix from sbt

The version of OrganizeImports used is 0.4.4 https://github.com/zio/zio/blob/1816c4232ee16f8e50ef4291b7810ebdea4e8529/project/BuildHelper.scala#L236

Are there any other details that would help you debug this?

@liancheng liancheng added the bug Something isn't working label Nov 26, 2020
@liancheng
Copy link
Owner

liancheng commented Nov 27, 2020

@sideeffffect, thanks for reporting! I was trying to reproduce this issue, but running either sbt fix or sbt test/scalafixAll hits GC issues and sbt kept crashing or being super slow (my current test/scalafixAll run has been running for over 6 min and hasn't finished yet, and sbt printed tons of warnings about GC issues...). I checked ZIO CircleCI settings, but I didn't find anything related to JVM memory settings. Does ZIO has certain JVM/GC requirements to build?

@liancheng
Copy link
Owner

Another thing I tried was invoking the "organize imports" code action (which is powered by OrganizeImports) in Vim with Metals 0.9.7 just on those two problematic files, and I got the correct results for both of them. Metals 0.9.7 uses OrganizeImports 0.4.2, could you please help to confirm whether 0.4.2 works for you? If there's still an issue, I suspect it's due to certain unexpected interactions between different Scalafix rules.

BTW, my previous sbt test/scalafixAll run finally ended with an OOM error :(

@sideeffffect
Copy link
Contributor Author

Maybe running with more memory (-Xmx4092M) could help. Full build on my computer takes cca 10 mins.

@sideeffffect
Copy link
Contributor Author

I've tried to run it again on the same snapshot with both 0.4.2 and 0.4.4, and fix finished with different outcomes. OrganizeImports always produced nonsensical imports in different two files. It indeed seems like some weird interaction between Scalafix rules.

It's not a huge deal, it's easy to fix manually, because it's only in few files.
But it still would be great to get to the bottom of the problem. Perhaps you could use ZIO as a stress test for OrganizeImports, but you'll have to give sbt a lot of memory (as I've shown above).

@liancheng
Copy link
Owner

@sideeffffect, thanks for the follow-up! Using ZIO as a stress test is a great idea. I did want to add a few automated tests to check out certain repos and run OrganizeImports against them for correctness test and maybe also perf tests.

Unfortunately, I don't have enough cycles recently, but will try to find some time to root cause this issue later.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants