Fix undercompilation upon ctor change

[eed3si9n]

Fixes #19910
Fixes sbt/zinc#1334

Problem

Scala 3 compiler registers special zincMangledName for constructors for the purpose of incremental compilation. Currently the zincMangledName contains the package name, which does not match the use site tracking,
thereby causing undercompilation during incremental compilation after a ctor change, like adding a parameter.

There is an existing scripted test, but coincidentally the test class does NOT include packages, so the test ends up passing.

Solution

1. This PR reproduces the issue by adding package name to the test.
2. This also fixes the problem by changing the zincMangedName to sym.owner.name ++ ";init;".

Note about zincMangedName

zincMangedName in general is a good idea, which adds the class name into otherwise common name <init>.
By making the name more unique it prevents overcompilation when one of the ctors changes in a file.

[smarter]

Currently the zincMangledName contains the package name, which does not match the use site tracking

Hmm, but shouldn't the use-site tracking also go through zincMangledName? If it isn't right now, then can we avoid this discrepancy? sbt/zinc#288 also takes the package name into account so it seems like it should be possible in theory.

[eed3si9n]

Yes, you're right. As far as I can tell, the undercompilation bug does not exist in Scala 2.x, and adding a new parameter correctly fails to compile using the https://github.com/sake92/metals-mill-scala3-class-params-repro example on Scala 2.12.19:

[info] compiling 1 Scala source to /private/tmp/metals-mill-scala3-class-params-repro/scalaHelloWorld/target/scala-2.12/classes ...
[error] /private/tmp/metals-mill-scala3-class-params-repro/scalaHelloWorld/src/main/scala/example/Hello.scala:5:17: not enough arguments for constructor MyService: (str: String, x: Int)example.MyService.
[error] Unspecified value parameter x.
[error]   val service = new MyService("fssdfs")
[error]                 ^
[error] one error found
[error] (hello / Compile / compileIncremental) Compilation failed

Also Scala 3 addUsedName(...) uses zincMangledName so in theory they should match up:

scala3/compiler/src/dotty/tools/dotc/sbt/ExtractDependencies.scala

Lines 338 to 339 in a9bb881

	def addUsedName(sym: Symbol, includeSealedChildren: Boolean = false)(using Context): Unit =
	addUsedRawName(sym.zincMangledName, includeSealedChildren)

so something about Scala 3 bridge is causing this.

[bishabosha]

I did some extra testing and it seems the actual problem is the . separator in the fullName, if I replace it with e.g. :: then it's fine, so I'll suggest to do that to keep the fine-grained-ness

[eed3si9n]

@bishabosha Feel free to take whatever from this PR and send a new one, or add commits to your liking on this PR.

[bishabosha]

@sjrd I added a new name kind, kept it private to sbt package to avoid unintentional leaking to TASTy. The other option would be creating a new SimpleName by replacing . in the mangledString but I think that would unnecessarily add extra entries to the name cache

[sjrd]

If the problem is the . in string form, consider changing how you serialize the Names as Strings for the purposes of giving them to zinc. This would avoid the extra NameKind.

[smarter]

It might be useful to use ; as a separator like in sbt/zinc#288 so that builds involving both scala2 and scala3 projects have incremental compilation working correctly? (Which makes me wonder if sbt/zinc#288 is missing a change in the java api extractor)

[bishabosha]

oh that's a good point seeing as dependency tracking is across modules

[bishabosha]

@smarter @sjrd now I avoid the new name kind, and we use ; in the mangled string. In the ideal case we would avoid creating new termName entries, but that can be improved later.

The main problem is that ExtractDependencies uses the name as a key in the cache, which is more efficient than converting names to string, so we would maybe need a new structure that can accommodate strings and names

[smarter]

Thanks everyone!

[eed3si9n]

Thanks @bishabosha for getting this across the finish line!