From 23f18c2bbae22cda01f8b00996bd540197d76d0e Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Thu, 14 Nov 2024 15:37:44 +0100 Subject: [PATCH] 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. --- .../internal/implementation/ImplementationProvider.scala | 2 +- .../meta/internal/metals/InlayHintResolveProvider.scala | 2 +- .../scala/meta/internal/metals/MetalsLspService.scala | 3 ++- .../scala/meta/internal/metals/ReferenceProvider.scala | 2 +- .../meta/internal/metals/SemanticTokensProvider.scala | 2 +- .../main/scala/scala/meta/internal/parsing/Trees.scala | 2 +- .../scala/scala/meta/internal/metals/ReportContext.scala | 9 +++++---- .../scala/scala/meta/internal/pc/CompilerAccess.scala | 2 +- .../scala-2/scala/meta/internal/pc/HoverProvider.scala | 2 +- .../scala-3/scala/meta/internal/pc/HoverProvider.scala | 2 +- .../scala/meta/internal/mtags/JavaToplevelMtags.scala | 5 ++++- .../scala/meta/internal/mtags/ScalaToplevelMtags.scala | 5 ++++- tests/unit/src/test/scala/tests/ReportsSuite.scala | 6 ++++-- 13 files changed, 27 insertions(+), 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 index 7174d333902..5e0a84afec6 100644 --- 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 index 0e88eb0b82d..1b0a89b4353 100644 --- 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 index dc3cbb0f0ac..2b7761c140b 100644 --- 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 index 08438805efc..284d084566e 100644 --- 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 index 6282c3343cf..a58ac0a9f0f 100644 --- 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 index 18b5192ca3f..7169bebe773 100644 --- 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 index 47bd271f56b..6c1e9c66e9a 100644 --- 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 index 8d73f9a9b0f..74031316ae7 100644 --- 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("")} |""".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 index a43d603d04e..9de237c87fb 100644 --- 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 index 94e0b894f2c..624d66fcc74 100644 --- 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 index 2d28bb1f2bc..dda909c06b5 100644 --- 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 index b325cb69d8b..8245528f518 100644 --- 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 index 54a59838cf7..51e0fa4ed07 100644 --- 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("")) + val report2 = + exampleReport("test_error2", Some(URI.create("file://file.scala"))) reportsProvider.incognito.create(report) reportsProvider.incognito.create(report2) val reports = reportsProvider.incognito