scala / bug

Scala 2 bug reports only. Please, no questions — proper bug reports only.
https://scala-lang.org
232 stars 21 forks source link

Quickfixes don't create path dependent types correctly #12943

Open coreywoodfield opened 9 months ago

coreywoodfield commented 9 months ago

Reproduction steps

Scala version: 2.13.12

In Test.scala

trait Trait
object Object {
  object Implementation extends Trait
}
trait Trait2 {
  def foo: Trait
}
class Test {
  object InsideClass extends Trait2 {
    val obj = Object
    override def foo = obj.Implementation
  }
}

Compile with scalac -quickfix:any -Xsource:3 Test.scala and it updates the file to

trait Trait
object Object {
  object Implementation extends Trait
}
trait Trait2 {
  def foo: Trait
}
class Test {
  object InsideClass extends Trait2 {
    val obj = Object
    override def foo: Test.InsideClass.obj.Implementation.type = obj.Implementation
  }
}

which isn't valid, since Test is a class.

Problem

Quickfix uses non-stable identifiers as stable identifiers

SethTisue commented 9 months ago

attn @lrytz

lrytz commented 9 months ago

The InsideClass prefix of the rhs type (InsideClass.obj.Implementation.type) is represented as a ThisType(InsideClassSym), so it uses fullNameString https://github.com/scala/scala/blob/v2.13.12/src/reflect/scala/reflect/internal/Types.scala#L1405.

Changing object InsideClass to class InsideClass, it uses nameString (next line), so the type-toString is InsideClass.this.obj.Implementation.type, which is valid source. I wonder if we should also use nameString for the module class?

Also nicer than blindly emitting the full prefix would be trying to be smart about how much of the prefix is needed. If we have a.b.c, check if c resolves to the right symbol. Or else check b.c, and so on. (The same should apply to args of the type being printed, so probably implemented with a TypeMap).

lrytz commented 9 months ago

The leaner type-toString would be useful for the quickfix used in this ticket

https://github.com/scala/scala/blob/ffc0297072ca8248daac279781d0dea5680c3a86/src/compiler/scala/tools/nsc/typechecker/Namers.scala#L1122

and for the quickfix adding inferred types to implicits

https://github.com/scala/scala/blob/ffc0297072ca8248daac279781d0dea5680c3a86/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala#L212