Explicit result breaks when using `this.type`.

[ckipp01]

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]

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]

Thanks for reporting!

I guess there is something fishy in

scalafix/scalafix-rules/src/main/scala-2/scalafix/internal/rule/CompilerTypePrinter.scala

Lines 299 to 389 in 449a06a

	// Clean up the type before passing it to `g.shortType()`.
	private def preProcess(tpe: Type, gsym: Symbol): Type = {
	def loop(tpe: Type): Type = {
	tpe match {
	case TypeRef(pre, sym, args)
	if gsymbolReplacements.contains(semanticdbSymbol(sym)) =>
	loop(TypeRef(pre, gsymbolReplacements(semanticdbSymbol(sym)), args))
	case tp @ ThisType(sym)
	if tp.toString() == s"${gsym.owner.nameString}.this.type" =>
	new PrettyType("this.type")
	case ConstantType(Constant(c: Symbol)) if c.hasFlag(gf.JAVA_ENUM) =>
	// Manually widen Java enums to obtain `x: FileVisitResult`
	// instead of `x: FileVisitResult.Continue.type`
	TypeRef(ThisType(tpe.typeSymbol.owner), tpe.typeSymbol, Nil)
	case t: PolyType => loop(t.resultType)
	case t: MethodType => loop(t.resultType)
	case RefinedType(parents, _) =>
	// Remove redundant `Product with Serializable`, if possible.
	val productRootClass = definitions.ProductClass.seq
	.toSet[Symbol] + definitions.ProductRootClass
	val serializableClass =
	CompilerCompat.serializableClass(g).toSet[Symbol]
	val parentSymbols = parents.map(_.typeSymbol).toSet
	val strippedParents =
	if (
	parentSymbols
	.intersect(productRootClass)
	.nonEmpty && parentSymbols
	.intersect(serializableClass)
	.nonEmpty
	) {
	parents.filterNot(isPossibleSyntheticParent)
	} else parents
	val newParents =
	if (strippedParents.nonEmpty) strippedParents
	else parents
	RefinedType(newParents.map(loop), EmptyScope)
	case NullaryMethodType(tpe) =>
	NullaryMethodType(loop(tpe))
	case TypeRef(pre, sym, args) =>
	val tpeString = tpe.toString()
	// NOTE(olafur): special case where `Type.toString()` produces
	// unparseable syntax such as `Path#foo.type`. In these cases, we
	// either try to simplify the type into the parents of the singleton
	// type. When we can't safely simplify the type, we give up and
	// don't annotate the type. See "WidenSingleType.scala" for test
	// cases that stress this code path.
	val isSimplifyToParents =
	tpe.toString().endsWith(".type") &&
	pre.prefixString.endsWith("#")
	if (isSimplifyToParents) {
	val hasMeaningfulParent = sym.info.parents.exists { parent =>
	!isPossibleSyntheticParent(parent)
	}
	val hasMeaningfulDeclaration =
	sym.info.decls.exists(decl =>
	!decl.isOverridingSymbol && !decl.isConstructor
	)
	if (hasMeaningfulParent && !hasMeaningfulDeclaration) {
	loop(RefinedType(sym.info.parents, EmptyScope))
	} else {
	throw new NotImplementedError(
	s"don't know how to produce parseable type for '$tpeString'"
	)
	}
	} else {
	TypeRef(loop(pre), sym, args.map(loop))
	}
	case ExistentialType(head :: Nil, underlying) =>
	head.info match {
	case b @ TypeBounds(RefinedType(parents, _), hi)
	if parents.length > 1 =>
	// Remove the lower bound large `Type[_ >: A with B with C <: D
	// with Serializable]` so that it becomes only `Type[_ <: D]`.
	// Large lower bounds `_ >: A with B with C ...` happen in
	// situations like `val x = List(A, B, C)`.
	head.setInfo(TypeBounds(definitions.NothingTpe, loop(hi)))
	case _ =>
	}
	tpe
	case SingleType(ThisType(osym), sym)
	if sym.isKindaTheSameAs(sym) &&
	gsymbolReplacements.contains(semanticdbSymbol(sym)) =>
	loop(
	TypeRef(NoPrefix, gsymbolReplacements(semanticdbSymbol(sym)), Nil)
	)
	case tpe => tpe
	}
	}
	loop(tpe)
	}

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

Probably related to #1534?

[lefou]

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: joan38/mill-scalafix#174