scalacenter / scalafix

Refactoring and linting tool for Scala
https://scalacenter.github.io/scalafix/
BSD 3-Clause "New" or "Revised" License
828 stars 186 forks source link

Imports changing targets #82

Closed paulp closed 7 years ago

paulp commented 7 years ago

The import rewriter turns this, which compiles:

package a

import _root_.scalaz._, Scalaz._

package object scalaz {
  Tree
}

into this, which does not.

package a

import _root_.scalaz._
import scalaz.Scalaz._

package object scalaz {
  Tree
}
olafurpg commented 7 years ago

Good gatch! I had actually bumped into a similar issue when testing against circe, and taken special care to preserve _root_ for non-relative imports. However, I forgot to handle expansion of relative imports 😅

I'm running scalafix on Matryoshka now to ensure that the code compiles after organizing imports. There seem to be a few remaining bugs left to iron out.

btw, any opinion on #97?

paulp commented 7 years ago

Yes, matryoshka should make a fine test case, and quasar even a better one.

I would place #97 in the "absolutely must have" category.

olafurpg commented 7 years ago

Actually, the bugs I hit on Matryoshka seems to be integration related. The semantic api is not returning correct symbols for relative imports, causing

import scalaz._, Scalaz._

to rewrite to

import Scalaz._
import scalaz._

I'll need more time to investigate that.

Yes, matryoshka should make a fine test case, and quasar even a better one.

I currently run ; scalafix ; test:compile on the slick repo for every PR. I'll try to add those to the list.

I would place #97 in the "absolutely must have" category.

It's a syntax I've only recently started to use myself, and I like it a lot too!

paulp commented 7 years ago

BTW @olafurpg did you know that _root_ isn't a reserved identifier, and anyone can name a package _root_ if they are so inclined? Good thing we are all very casual about correctness because if "things will be broken" is designed in from day one, it tends to wins.

SI-6217: _root_ should always refer to the root package

olafurpg commented 7 years ago

Keep in mind that this is a welcome environment to learn, teach and contribute. I appreciate you sharing your insights about Scala and scalac internals. I will do my best to address them. However, please leave personal matters off the discussions. In particular, I am referring to "smiling through gritted teeth" (https://github.com/scalacenter/scalafix/issues/83#issuecomment-284914335), my "great attitude" or being "casual about correctness". It's distracting, and reading it frankly hurts.

paulp commented 7 years ago

I think you must have misunderstood something if it hurts, because anything I've said about you has been complimentary. But if you are troubled, I will endeavor not to include editorial flourishes.

heathermiller commented 7 years ago

Paul, this is getting nontechnical. Could you please try to keep to technical comments please?

paulp commented 7 years ago

Is there any way to answer which won't lead to another admonishment? What did I say in my last comment? I have no idea what has imbued this conversation with such importance but I will implement the simple solution.

olafurpg commented 7 years ago

I accept Paul's apology. Let's leave out personal comments and I'm happy :)

dwijnand commented 7 years ago

It saddens me to see how this spiralled out and ended up.

I believe that communication is hard and we humans suck at it. Personally, as a general rule, I'd advise lots of empathy and charitable interpretations towards one another's attempts at communicating.

😞