Merge pull request #10447 from dotty-staging/fix-10430

Fix #10430: relativize path in tasty

Author: nicolasstucki

compiler/src/dotty/tools/dotc/core/tasty/PositionPickler.scala

@@ -8,20 +8,21 @@ import dotty.tools.tasty.TastyBuffer
 import TastyBuffer._
 
 import ast._
-import ast.Trees._
-import ast.Trees.WithLazyField
+import Trees.WithLazyField
 import util.{SourceFile, NoSource}
 import core._
-import Contexts._, Symbols._, Annotations._, Decorators._
+import Annotations._, Decorators._
 import collection.mutable
 import util.Spans._
 
 class PositionPickler(
     pickler: TastyPickler,
     addrOfTree: PositionPickler.TreeToAddr,
-    treeAnnots: untpd.MemberDef => List[tpd.Tree]) {
+    treeAnnots: untpd.MemberDef => List[tpd.Tree],
+    relativePathReference: String){
 
   import ast.tpd._
+
   val buf: TastyBuffer = new TastyBuffer(5000)
   pickler.newSection(PositionsSection, buf)
 
@@ -69,19 +70,8 @@ class PositionPickler(
 
     def pickleSource(source: SourceFile): Unit = {
       buf.writeInt(SOURCE)
-      val pathName = source.path
-      val pickledPath =
-        val originalPath = java.nio.file.Paths.get(pathName.toString).normalize()
-        if originalPath.isAbsolute then
-          val path = originalPath.toAbsolutePath().normalize()
-          val cwd = java.nio.file.Paths.get("").toAbsolutePath().normalize()
-          try cwd.relativize(path)
-          catch case _: IllegalArgumentException =>
-            warnings += "Could not relativize path for pickling: " + originalPath
-            originalPath
-        else
-          originalPath
-      buf.writeInt(pickler.nameBuffer.nameIndex(pickledPath.toString.toTermName).index)
+      val relativePath = SourceFile.relativePath(source, relativePathReference)
+      buf.writeInt(pickler.nameBuffer.nameIndex(relativePath.toTermName).index)
     }
 
     /** True if x's position shouldn't be reconstructed automatically from its initial span

compiler/src/dotty/tools/dotc/core/tasty/TastyPrinter.scala

@@ -152,6 +152,16 @@ class TastyPrinter(bytes: Array[Byte]) {
         sb.append(treeStr("%10d".format(addr.index)))
         sb.append(s": ${offsetToInt(pos.start)} .. ${pos.end}\n")
       }
+
+      val sources = posUnpickler.sourcePaths
+      sb.append(s"\n source paths:\n")
+      val sortedPath = sources.toSeq.sortBy(_._1.index)
+      for ((addr, path) <- sortedPath) {
+        sb.append(treeStr("%10d: ".format(addr.index)))
+        sb.append(path)
+        sb.append("\n")
+      }
+
       sb.result
     }
   }

compiler/src/dotty/tools/dotc/quoted/PickledQuotes.scala

@@ -167,7 +167,8 @@ object PickledQuotes {
     treePkl.compactify()
     if tree.span.exists then
       val positionWarnings = new mutable.ListBuffer[String]()
-      new PositionPickler(pickler, treePkl.buf.addrOfTree, treePkl.treeAnnots)
+      val reference = ctx.settings.sourceroot.value
+      new PositionPickler(pickler, treePkl.buf.addrOfTree, treePkl.treeAnnots, reference)
         .picklePositions(ctx.compilationUnit.source, tree :: Nil, positionWarnings)
       positionWarnings.foreach(report.warning(_))
 

compiler/src/dotty/tools/dotc/semanticdb/ExtractSemanticDB.scala

@@ -5,26 +5,23 @@ package semanticdb
 import core._
 import Phases._
 import ast.tpd._
+import ast.untpd.given
 import ast.Trees.mods
 import Contexts._
 import Symbols._
 import Flags._
 import Names.Name
 import StdNames.nme
+import NameOps._
 import util.Spans.Span
 import util.{SourceFile, SourcePosition}
-import scala.jdk.CollectionConverters._
-import collection.mutable
-import java.nio.file.Paths
-
-import dotty.tools.dotc.transform.SymUtils._
-
-import PartialFunction.condOpt
-
-import ast.untpd.given
-import NameOps._
+import transform.SymUtils._
 
+import scala.jdk.CollectionConverters._
+import scala.collection.mutable
 import scala.annotation.{ threadUnsafe => tu, tailrec }
+import scala.PartialFunction.condOpt
+
 
 /** Extract symbol references and uses to semanticdb files.
  *  See https://scalameta.org/docs/semanticdb/specification.html#symbol-1
@@ -590,37 +587,29 @@ object ExtractSemanticDB:
   import java.nio.file.Path
   import scala.collection.JavaConverters._
   import java.nio.file.Files
+  import java.nio.file.Paths
 
   val name: String = "extractSemanticDB"
 
   def write(source: SourceFile, occurrences: List[SymbolOccurrence], symbolInfos: List[SymbolInformation])(using Context): Unit =
     def absolutePath(path: Path): Path = path.toAbsolutePath.normalize
-    def commonPrefix[T](z: T)(i1: Iterable[T], i2: Iterable[T])(app: (T, T) => T): T =
-      (i1 lazyZip i2).takeWhile(p => p(0) == p(1)).map(_(0)).foldLeft(z)(app)
-    val sourcePath = absolutePath(source.file.jpath)
-    val sourceRoot =
-      // Here if `sourceRoot` and `sourcePath` do not share a common prefix then `relPath` will not be normalised,
-      // containing ../.. etc, which is problematic when appending to `/META-INF/semanticdb/` and will not be accepted
-      // by Files.createDirectories on JDK 11.
-      val sourceRoot0 = absolutePath(Paths.get(ctx.settings.sourceroot.value))
-      commonPrefix(sourcePath.getRoot)(sourcePath.asScala, sourceRoot0.asScala)(_ resolve _)
     val semanticdbTarget =
       val semanticdbTargetSetting = ctx.settings.semanticdbTarget.value
       absolutePath(
         if semanticdbTargetSetting.isEmpty then ctx.settings.outputDir.value.jpath
         else Paths.get(semanticdbTargetSetting)
       )
-    val relPath = sourceRoot.relativize(sourcePath)
+    val relPath = SourceFile.relativePath(source, ctx.settings.sourceroot.value)
     val outpath = semanticdbTarget
       .resolve("META-INF")
       .resolve("semanticdb")
       .resolve(relPath)
-      .resolveSibling(sourcePath.getFileName().toString() + ".semanticdb")
+      .resolveSibling(source.name + ".semanticdb")
     Files.createDirectories(outpath.getParent())
     val doc: TextDocument = TextDocument(
       schema = Schema.SEMANTICDB4,
       language = Language.SCALA,
-      uri = Tools.mkURIstring(relPath),
+      uri = Tools.mkURIstring(Paths.get(relPath)),
       text = "",
       md5 = internal.MD5.compute(String(source.content)),
       symbols = symbolInfos,

compiler/src/dotty/tools/dotc/transform/Pickler.scala

@@ -72,7 +72,8 @@ class Pickler extends Phase {
         Future {
           treePkl.compactify()
           if tree.span.exists then
-            new PositionPickler(pickler, treePkl.buf.addrOfTree, treePkl.treeAnnots)
+            val reference = ctx.settings.sourceroot.value
+            new PositionPickler(pickler, treePkl.buf.addrOfTree, treePkl.treeAnnots, reference)
               .picklePositions(unit.source, tree :: Nil, positionWarnings)
 
           if !ctx.settings.YdropComments.value then

compiler/src/dotty/tools/dotc/transform/PostTyper.scala

@@ -335,21 +335,8 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase
                && ctx.compilationUnit.source.exists
                && sym != defn.SourceFileAnnot
             then
-              def sourcerootPath =
-                java.nio.file.Paths.get(ctx.settings.sourceroot.value)
-                .toAbsolutePath
-                .normalize
-              val file = ctx.compilationUnit.source.file
-              val jpath = file.jpath
-              val relativePath =
-                if jpath eq null then file.path // repl and other custom tests use abstract files with no path
-                else if jpath.isAbsolute then
-                  val cunitPath = jpath.normalize
-                  // On Windows we can only relativize paths if root component matches
-                  // (see implementation of sun.nio.fs.WindowsPath#relativize)
-                  try sourcerootPath.relativize(cunitPath).toString
-                  catch case _: IllegalArgumentException => cunitPath.toString
-                else jpath.normalize.toString
+              val reference = ctx.settings.sourceroot.value
+              val relativePath = util.SourceFile.relativePath(ctx.compilationUnit.source, reference)
               sym.addAnnotation(Annotation.makeSourceFile(relativePath))
           else (tree.rhs, sym.info) match
             case (rhs: LambdaTypeTree, bounds: TypeBounds) =>

compiler/src/dotty/tools/dotc/util/SourceFile.scala

@@ -209,6 +209,34 @@ object SourceFile {
     val src = new SourceFile(new VirtualFile(name, content.getBytes(StandardCharsets.UTF_8)), scala.io.Codec.UTF8)
     src._maybeInComplete = maybeIncomplete
     src
+
+  /** Returns the relative path of `source` within the `reference` path
+   *
+   *  It returns the absolute path of `source` if it is not contained in `reference`.
+   */
+  def relativePath(source: SourceFile, reference: String): String = {
+    val file = source.file
+    val jpath = file.jpath
+    if jpath eq null then
+      file.path // repl and other custom tests use abstract files with no path
+    else
+      val sourcePath = jpath.toAbsolutePath.normalize
+      val refPath = java.nio.file.Paths.get(reference).toAbsolutePath.normalize
+
+      if sourcePath.startsWith(refPath) then
+        // On Windows we can only relativize paths if root component matches
+        // (see implementation of sun.nio.fs.WindowsPath#relativize)
+        //
+        //     try refPath.relativize(sourcePath).toString
+        //     catch case _: IllegalArgumentException => sourcePath.toString
+        //
+        // As we already check that the prefix matches, the special handling for
+        // Windows is not needed.
+
+        refPath.relativize(sourcePath).toString
+      else
+        sourcePath.toString
+  }
 }
 
 @sharable object NoSource extends SourceFile(NoAbstractFile, Array[Char]()) {

language-server/test/dotty/tools/languageserver/util/server/TestFile.scala

@@ -9,6 +9,6 @@ class TestFile(val file: String) extends AnyVal {
 }
 
 object TestFile {
-  lazy val testDir: Path = Paths.get("../out/ide-tests").toAbsolutePath
+  lazy val testDir: Path = Paths.get("../out/ide-tests").toAbsolutePath.normalize
   lazy val sourceDir: Path = testDir.resolve("src")
 }

project/scripts/cmdTests

@@ -33,10 +33,24 @@ clear_out "$OUT"
 "$SBT" ";scalac -d $OUT/out.jar $SOURCE; scalac -decompile -color:never $OUT/out.jar" > "$tmp"
 grep -qe "def main(args: scala.Array\[scala.Predef.String\]): scala.Unit =" "$tmp"
 
+
 echo "testing that paths SourceFile annotations are relativized"
 clear_out "$OUT"
-"$SBT" ";scalac -d $OUT/out.jar $(pwd)/$SOURCE; scalac -decompile -color:never $OUT/out.jar" > "$tmp"
-grep -qe "SourceFile(\"$SOURCE\")" "$tmp"
+"$SBT" "scalac -d $OUT/out.jar -sourceroot tests/pos $(pwd)/tests/pos/i10430/lib.scala $(pwd)/tests/pos/i10430/app.scala"
+"$SBT" "scalac -decompile -print-tasty -color:never $OUT/out.jar" > "$tmp"
+cat "$tmp" # for debugging
+grep -q ": i10430/lib.scala" "$tmp"
+grep -q ": i10430/app.scala" "$tmp"
+grep -q "[i10430/lib.scala]" "$tmp"
+grep -q "[i10430/app.scala]" "$tmp"
+if grep -q "tests/pos/i10430/lib.scala" "$tmp"; then
+  echo "incorrect source file path in tasty"
+  exit 1
+fi
+if grep -q "tests/pos/i10430/app.scala" "$tmp"; then
+  echo "incorrect source file path in tasty"
+  exit 1
+fi
 
 echo "testing sbt scalac with suspension"
 clear_out "$OUT"

tests/pos/i10430/app.scala

@@ -0,0 +1,4 @@
+object App {
+  val y = lib.double(5)
+  println(y)
+}