scalacenter / scalafix

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

Explicit result breaks when using `this.type`. #1631

Open ckipp01 opened 2 years ago

ckipp01 commented 2 years ago

I encountered this when writing a Mill plugin. When creating an ExternalModule you'll have code that looks like this:

import mill._
import mill.define.Command
import mill.define.ExternalModule
import mill.eval.Evaluator
import mill.main.EvaluatorScopt

object Example extends ExternalModule {

  def foo(ev: Evaluator): Command[Unit] = T.command {}

  implicit def millScoptEvaluatorReads[T]: EvaluatorScopt[T] =
    new mill.main.EvaluatorScopt[T]()

  lazy val millDiscover = mill.define.Discover[this.type]

}

If you run scalafix against this wanting ExplicitResultTypes you'll end up with broken code since the final lazy vall millDiscover will turn into:

  lazy val millDiscover: Discover[Example] = mill.define.Discover[this.type]

Which will error with:

not found: type Example

I'd expect running this rule not to break my code.

You can reproduce this with the Scala file up above placed in a foo/src/Example.scala and the following files:

// build.sc
import mill._
import scalalib._
import mill.scalalib.api.ZincWorkerUtil
import $ivy.`com.goyeau::mill-scalafix::0.2.8`
import com.goyeau.mill.scalafix.ScalafixModule

object foo extends ScalaModule with ScalafixModule {

  def scalaVersion = "2.13.8"

  def scalafixScalaBinaryVersion =
    ZincWorkerUtil.scalaBinaryVersion(scalaVersion())

  override def compileIvyDeps = super.compileIvyDeps() ++ Agg(
    ivy"com.lihaoyi::mill-scalalib:0.10.5"
  )
}
// .scalafix.conf
rules = [
  ExplicitResultTypes
]

Then you can either run them ~via Mill or~ Metals.

ckipp01 commented 2 years ago

Actually hold on. Running this will mill foo.fix doesn't actually apply the patch, but running it with Metals does. I wonder why there is a difference there. This might be something funky going on with Metals.

bjaglin commented 1 year ago

Thanks for reporting!

I guess there is something fishy in https://github.com/scalacenter/scalafix/blob/449a06a03d2595a1ec6f44201353881b3edc4aee/scalafix-rules/src/main/scala-2/scalafix/internal/rule/CompilerTypePrinter.scala#L299-L389

I wonder why there is a difference there. This might be something funky going on with Metals.

Probably related to https://github.com/scalacenter/scalafix/issues/1534?

lefou commented 8 months ago

Actually hold on. Running this will mill foo.fix doesn't actually apply the patch, but running it with Metals does. I wonder why there is a difference there. This might be something funky going on with Metals.

This might be an issue with mill-scalafix: https://github.com/joan38/mill-scalafix/issues/174