Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quickfixes don't create path dependent types correctly #12943

Open
coreywoodfield opened this issue Feb 6, 2024 · 3 comments
Open

Quickfixes don't create path dependent types correctly #12943

coreywoodfield opened this issue Feb 6, 2024 · 3 comments
Assignees
Milestone

Comments

@coreywoodfield
Copy link

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
Copy link
Member

attn @lrytz

@lrytz lrytz added this to the 2.13.14 milestone Feb 9, 2024
@lrytz
Copy link
Member

lrytz commented Feb 9, 2024

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
Copy link
Member

lrytz commented Feb 9, 2024

@SethTisue SethTisue modified the milestones: 2.13.14, 2.13.15 Mar 13, 2024
@lrytz lrytz self-assigned this Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants