improvements: Explicitely use URI instead of string

Otherwise, if a wrong string is used we will get an exception, which has already happened in the wild.
scalameta · Nov 14, 2024 · 23f18c2 · 23f18c2
1 parent 2df881d
commit 23f18c2
Show file tree

Hide file tree

Showing 13 changed files with 27 additions and 17 deletions.
diff --git a/metals/src/main/scala/scala/meta/internal/implementation/ImplementationProvider.scala b/metals/src/main/scala/scala/meta/internal/implementation/ImplementationProvider.scala
@@ -181,7 +181,7 @@ final class ImplementationProvider(
                   |$dealisedSymbol
                   |""".stripMargin,
               s"missing def: $dealisedSymbol",
-              Some(source.toURI.toString()),
+              Some(source.toURI),
             )
           )
         }

diff --git a/metals/src/main/scala/scala/meta/internal/metals/InlayHintResolveProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/InlayHintResolveProvider.scala
@@ -121,7 +121,7 @@ final class InlayHintResolveProvider(
           |""".stripMargin,
       s"failed to resolve inlayHint in $path",
       id = Some(s"$path::${pos.getLine()}:${pos.getCharacter()}"),
-      path = Some(path.toString()),
+      path = Some(path.toURI),
       error = Some(error),
     )
   }

diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala
@@ -17,6 +17,7 @@ import scala.concurrent.TimeoutException
 import scala.concurrent.duration._
 import scala.util.Failure
 import scala.util.Success
+import scala.util.Try
 import scala.util.control.NonFatal
 
 import scala.meta.internal.bsp.BspSession
@@ -163,7 +164,7 @@ abstract class MetalsLspService(
     folder.toNIO,
     _.flatMap { uri =>
       for {
-        filePath <- uri.toAbsolutePathSafe
+        filePath <- Try(AbsolutePath(Paths.get(uri))).toOption
         buildTargetId <- buildTargets.inverseSources(filePath)
         name <- buildTargets.info(buildTargetId).map(_.getDisplayName())
       } yield name

diff --git a/metals/src/main/scala/scala/meta/internal/metals/ReferenceProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/ReferenceProvider.scala
@@ -248,7 +248,7 @@ final class ReferenceProvider(
                       }
                       .mkString("\n"),
                     s"Could not find any locations for ${result.occurrence}, printing index state",
-                    Some(source.toString()),
+                    Some(source.toURI),
                     Some(
                       source.toString() + ":" + result.occurrence.getOrElse("")
                     ),

diff --git a/metals/src/main/scala/scala/meta/internal/metals/SemanticTokensProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/SemanticTokensProvider.scala
@@ -99,7 +99,7 @@ object SemanticTokensProvider {
               "semantic-tokens-provider",
               params.text(),
               s"Could not find semantic tokens for: ${params.uri()}",
-              Some(params.uri().toString()),
+              Some(params.uri()),
               error = Some(t),
             )
           )

diff --git a/metals/src/main/scala/scala/meta/internal/parsing/Trees.scala b/metals/src/main/scala/scala/meta/internal/parsing/Trees.scala
@@ -188,7 +188,7 @@ final class Trees(
             s"stackoverflow_${path.filename}",
             text,
             s"Stack overflow in ${path.filename}",
-            path = Some(path.toURI.toString()),
+            path = Some(path.toURI),
           )
         )
         val message =

diff --git a/mtags-shared/src/main/scala/scala/meta/internal/metals/ReportContext.scala b/mtags-shared/src/main/scala/scala/meta/internal/metals/ReportContext.scala
@@ -1,5 +1,6 @@
 package scala.meta.internal.metals
 
+import java.net.URI
 import java.nio.file.Files
 import java.nio.file.Path
 import java.nio.file.Paths
@@ -39,7 +40,7 @@ trait Reporter {
 
 class StdReportContext(
     workspace: Path,
-    resolveBuildTarget: Option[String] => Option[String],
+    resolveBuildTarget: Option[URI] => Option[String],
     level: ReportLevel = ReportLevel.Info
 ) extends ReportContext {
   val reportsDir: Path = workspace.resolve(StdReportContext.reportsDir)
@@ -85,7 +86,7 @@ class StdReportContext(
 class StdReporter(
     workspace: Path,
     pathToReports: Path,
-    resolveBuildTarget: Option[String] => Option[String],
+    resolveBuildTarget: Option[URI] => Option[String],
     level: ReportLevel,
     override val name: String
 ) extends Reporter {
@@ -221,7 +222,7 @@ case class Report(
     name: String,
     text: String,
     shortSummary: String,
-    path: Option[String] = None,
+    path: Option[URI] = None,
     id: Option[String] = None,
     error: Option[Throwable] = None
 ) {
@@ -267,7 +268,7 @@ object Report {
       name: String,
       text: String,
       error: Throwable,
-      path: Option[String]
+      path: Option[URI]
   ): Report =
     Report(
       name,

diff --git a/mtags-shared/src/main/scala/scala/meta/internal/pc/CompilerAccess.scala b/mtags-shared/src/main/scala/scala/meta/internal/pc/CompilerAccess.scala
@@ -216,7 +216,7 @@ abstract class CompilerAccess[Reporter, Compiler](
             |${params.map(_.printed()).getOrElse("<NONE>")}
             |""".stripMargin,
         error,
-        path = params.map(_.uri().toString)
+        path = params.map(_.uri())
       )
     val pathToReport =
       rc.unsanitized.create(report)

diff --git a/mtags/src/main/scala-2/scala/meta/internal/pc/HoverProvider.scala b/mtags/src/main/scala-2/scala/meta/internal/pc/HoverProvider.scala
@@ -53,7 +53,7 @@ class HoverProvider(
       val hasErroneousType =
         if (tree.tpe != null) tree.tpe.isErroneous
         else "type null"
-      val fileName = params.uri().toString()
+      val fileName = params.uri()
       val posId =
         if (tree.pos.isDefined) tree.pos.start
         else pos.start

diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/HoverProvider.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/HoverProvider.scala
@@ -88,7 +88,7 @@ object HoverProvider:
               |""".stripMargin,
           s"empty hover in $uri",
           id = Some(s"$uri::$posId"),
-          path = Some(uri.toString),
+          path = Some(uri),
         )
       end report
       reportContext.unsanitized.create(report, ifVerbose = true)

diff --git a/mtags/src/main/scala/scala/meta/internal/mtags/JavaToplevelMtags.scala b/mtags/src/main/scala/scala/meta/internal/mtags/JavaToplevelMtags.scala
@@ -1,6 +1,9 @@
 package scala.meta.internal.mtags
 
+import java.nio.file.Paths
+
 import scala.annotation.tailrec
+import scala.util.Try
 import scala.util.control.NonFatal
 
 import scala.meta.dialects
@@ -49,7 +52,7 @@ class JavaToplevelMtags(
                 |```
                 |""".stripMargin,
             s"Java indexer failed with and exception.",
-            path = Some(input.path),
+            path = Try(Paths.get(input.path).toUri()).toOption,
             id = Some(input.path),
             error = Some(e)
           )

diff --git a/mtags/src/main/scala/scala/meta/internal/mtags/ScalaToplevelMtags.scala b/mtags/src/main/scala/scala/meta/internal/mtags/ScalaToplevelMtags.scala
@@ -1,6 +1,9 @@
 package scala.meta.internal.mtags
 
+import java.nio.file.Paths
+
 import scala.annotation.tailrec
+import scala.util.Try
 
 import scala.meta.Dialect
 import scala.meta.inputs.Input
@@ -1139,7 +1142,7 @@ class ScalaToplevelMtags(
         failMessage(expected),
         s"expected $expected; obtained $currentToken",
         id = Some(s"""${input.path}:${newPosition}"""),
-        path = Some(input.path)
+        path = Try(Paths.get(input.path).toUri()).toOption
       )
     )
   }

diff --git a/tests/unit/src/test/scala/tests/ReportsSuite.scala b/tests/unit/src/test/scala/tests/ReportsSuite.scala
@@ -1,5 +1,6 @@
 package tests
 
+import java.net.URI
 import java.nio.charset.StandardCharsets
 import java.nio.file.Files
 import java.nio.file.Paths
@@ -36,7 +37,7 @@ class ReportsSuite extends BaseSuite {
         |${workspaceStr}/WrongFile.scala
         |""".stripMargin
 
-  def exampleReport(name: String, path: Option[String] = None): Report =
+  def exampleReport(name: String, path: Option[URI] = None): Report =
     Report(name, exampleText(), "Test error report.", path)
 
   override def afterEach(context: AfterEach): Unit = {
@@ -70,7 +71,8 @@ class ReportsSuite extends BaseSuite {
 
   test("get-name-summary-and-buildTarget") {
     val report = exampleReport("test_error")
-    val report2 = exampleReport("test_error2", Some("<path>"))
+    val report2 =
+      exampleReport("test_error2", Some(URI.create("file://file.scala")))
     reportsProvider.incognito.create(report)
     reportsProvider.incognito.create(report2)
     val reports = reportsProvider.incognito