feature: metals error reports (#4971)

* feature: reports A framework for creating error reports in metals. * reports for chosen errors The errors are chosen based on what reoccurs in `metals.log`. * feature: zip command zip reports command: - zips error reports - adds build target info from doctor
scalameta · Mar 17, 2023 · cd6c659 · cd6c659
1 parent 565ac31
commit cd6c659
Show file tree

Hide file tree

Showing 23 changed files with 489 additions and 64 deletions.
diff --git a/metals/src/main/scala/scala/meta/internal/implementation/Supermethods.scala b/metals/src/main/scala/scala/meta/internal/implementation/Supermethods.scala
@@ -7,6 +7,7 @@ import scala.meta.internal.implementation.Supermethods.formatMethodSymbolForQuic
 import scala.meta.internal.metals.ClientCommands
 import scala.meta.internal.metals.DefinitionProvider
 import scala.meta.internal.metals.MetalsEnrichments._
+import scala.meta.internal.metals.ReportContext
 import scala.meta.internal.metals.clients.language.MetalsLanguageClient
 import scala.meta.internal.metals.clients.language.MetalsQuickPickItem
 import scala.meta.internal.metals.clients.language.MetalsQuickPickParams
@@ -23,7 +24,8 @@ class Supermethods(
     definitionProvider: DefinitionProvider,
     implementationProvider: ImplementationProvider,
 )(implicit
-    ec: ExecutionContext
+    ec: ExecutionContext,
+    reports: ReportContext,
 ) {
 
   def getGoToSuperMethodCommand(

diff --git a/metals/src/main/scala/scala/meta/internal/metals/Directories.scala b/metals/src/main/scala/scala/meta/internal/metals/Directories.scala
@@ -8,8 +8,6 @@ object Directories {
     RelativePath(".metals").resolve("readonly")
   def tmp: RelativePath =
     RelativePath(".metals").resolve(".tmp")
-  def reports: RelativePath =
-    RelativePath(".metals").resolve(".reports")
   def dependencies: RelativePath =
     readonly.resolve(dependenciesName)
   def log: RelativePath =

diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala
@@ -488,9 +488,6 @@ object MetalsEnrichments
       else Files.move(path.toNIO, newPath.toNIO)
     }
 
-    def createDirectories(): AbsolutePath =
-      AbsolutePath(Files.createDirectories(path.dealias.toNIO))
-
     def createAndGetDirectories(): Seq[AbsolutePath] = {
       def createDirectoriesRec(
           absolutePath: AbsolutePath,
@@ -513,31 +510,6 @@ object MetalsEnrichments
       path.listRecursive.toList.reverse.foreach(_.delete())
     }
 
-    def writeText(text: String): Unit = {
-      path.parent.createDirectories()
-      val tmp = Files.createTempFile("metals", path.filename)
-      // Write contents first to a temporary file and then try to
-      // atomically move the file to the destination. The atomic move
-      // reduces the risk that another tool will concurrently read the
-      // file contents during a half-complete file write.
-      Files.write(
-        tmp,
-        text.getBytes(StandardCharsets.UTF_8),
-        StandardOpenOption.TRUNCATE_EXISTING,
-      )
-      try {
-        Files.move(
-          tmp,
-          path.toNIO,
-          StandardCopyOption.REPLACE_EXISTING,
-          StandardCopyOption.ATOMIC_MOVE,
-        )
-      } catch {
-        case NonFatal(_) =>
-          Files.move(tmp, path.toNIO, StandardCopyOption.REPLACE_EXISTING)
-      }
-    }
-
     def appendText(text: String): Unit = {
       path.parent.createDirectories()
       Files.write(
@@ -601,7 +573,21 @@ object MetalsEnrichments
         case _ => None
       }
 
-    def toAbsolutePathSafe: Option[AbsolutePath] = Try(toAbsolutePath).toOption
+    def toAbsolutePathSafe(implicit
+        reports: ReportContext
+    ): Option[AbsolutePath] =
+      try {
+        Some(toAbsolutePath)
+      } catch {
+        case NonFatal(e) =>
+          reports.incognito.createReport(
+            "absolute-path",
+            s"""|Uri: $value
+                |""".stripMargin,
+            e,
+          )
+          None
+      }
 
     def toAbsolutePath: AbsolutePath = toAbsolutePath(followSymlink = true)
 

diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala
@@ -43,6 +43,7 @@ import scala.meta.internal.metals.BuildInfo
 import scala.meta.internal.metals.Messages.AmmoniteJvmParametersChange
 import scala.meta.internal.metals.Messages.IncompatibleBloopVersion
 import scala.meta.internal.metals.MetalsEnrichments._
+import scala.meta.internal.metals.StdReportContext
 import scala.meta.internal.metals.ammonite.Ammonite
 import scala.meta.internal.metals.callHierarchy.CallHierarchyProvider
 import scala.meta.internal.metals.clients.language.ConfiguredLanguageClient
@@ -189,6 +190,11 @@ class MetalsLspService(
 
   val tables: Tables = register(new Tables(workspace, time))
 
+  implicit val reports: StdReportContext = new StdReportContext(workspace)
+
+  val zipReportsProvider: ZipReportsProvider =
+    new ZipReportsProvider(doctor.getTargetsInfoForReports, reports)
+
   private val buildTools: BuildTools = new BuildTools(
     workspace,
     bspGlobalDirectories,
@@ -288,7 +294,7 @@ class MetalsLspService(
   )
 
   private val timerProvider: TimerProvider = new TimerProvider(time)
-  private val trees = new Trees(buffers, scalaVersionSelector, workspace)
+  private val trees = new Trees(buffers, scalaVersionSelector)
 
   private val documentSymbolProvider = new DocumentSymbolProvider(
     trees,
@@ -1168,9 +1174,10 @@ class MetalsLspService(
         None
     }
 
-    uriOpt match {
-      case Some(uri) => {
-        val path = uri.toAbsolutePath
+    val pathOpt = uriOpt.flatMap(_.toAbsolutePathSafe)
+
+    pathOpt match {
+      case Some(path) => {
         focusedDocument = Some(path)
         buildTargets
           .inverseSources(path)
@@ -1868,6 +1875,23 @@ class MetalsLspService(
           doctor.onVisibilityDidChange(true)
           doctor.executeRunDoctor()
         }.asJavaObject
+      case ServerCommands.ZipReports() =>
+        Future {
+          val zip = zipReportsProvider.zip()
+          val pos = new l.Position(0, 0)
+          val location = new Location(
+            zip.toURI.toString(),
+            new l.Range(pos, pos),
+          )
+          languageClient.metalsExecuteClientCommand(
+            ClientCommands.GotoLocation.toExecuteCommandParams(
+              ClientCommands.WindowLocation(
+                location.getUri(),
+                location.getRange(),
+              )
+            )
+          )
+        }.asJavaObject
       case ServerCommands.ListBuildTargets() =>
         Future {
           buildTargets.all.toList
@@ -2735,6 +2759,11 @@ class MetalsLspService(
         case e: IndexingExceptions.PathIndexingException =>
           scribe.error(s"issues while parsing: ${e.path}", e.underlying)
         case e: IndexingExceptions.InvalidSymbolException =>
+          reports.incognito.createReport(
+            "invalid-symbol",
+            s"""Symbol: ${e.symbol}""".stripMargin,
+            e,
+          )
           scribe.error(s"searching for `${e.symbol}` failed", e.underlying)
         case _: NoSuchFileException =>
         // only comes for badly configured jar with `/Users` path added.

diff --git a/metals/src/main/scala/scala/meta/internal/metals/ServerCommands.scala b/metals/src/main/scala/scala/meta/internal/metals/ServerCommands.scala
@@ -192,6 +192,13 @@ object ServerCommands {
        |""".stripMargin,
   )
 
+  val ZipReports = new Command(
+    "zip-reports",
+    "Create a zip with error reports",
+    """|Creates a zip from incognito and bloop reports with additional information about build targets.
+       |""".stripMargin,
+  )
+
   val RunScalafix = new ParametrizedCommand[TextDocumentPositionParams](
     "scalafix-run",
     "Run all Scalafix Rules",
@@ -741,6 +748,7 @@ object ServerCommands {
       StopScalaCliServer,
       OpenIssue,
       OpenFeatureRequest,
+      ZipReports,
     )
 
   val allIds: Set[String] = all.map(_.id).toSet

diff --git a/metals/src/main/scala/scala/meta/internal/metals/ZipReportsProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/ZipReportsProvider.scala
@@ -0,0 +1,73 @@
+package scala.meta.internal.metals
+
+import java.nio.file.Files
+import java.util.zip.ZipEntry
+import java.util.zip.ZipOutputStream
+
+import scala.meta.internal.mtags.MtagsEnrichments._
+import scala.meta.io.AbsolutePath
+
+class ZipReportsProvider(
+    doctorTargetsInfo: () => List[
+      Map[String, String]
+    ], // we pass the function instead of a whole doctor for the simplicity of testing
+    reportContext: StdReportContext,
+) {
+
+  def zip(): AbsolutePath = {
+    val buildTargersFile = storeBuildTargetsInfo()
+    zipReports(List(buildTargersFile))
+    crateReportReadme()
+  }
+
+  private def crateReportReadme(): AbsolutePath = {
+    val path = reportContext.reportsDir.resolve("READ_ME.md")
+    if (Files.notExists(path.toNIO)) {
+      path.writeText(
+        s"""|Please attach `${StdReportContext.ZIP_FILE_NAME}` to your GitHub issue.
+            |Reports zip URI: ${reportContext.reportsDir.resolve(StdReportContext.ZIP_FILE_NAME).toURI(false)}
+            |""".stripMargin
+      )
+    }
+    path
+  }
+
+  private def storeBuildTargetsInfo(): FileToZip = {
+    val text = doctorTargetsInfo().zipWithIndex
+      .map { case (info, ind) =>
+        s"""|#### $ind
+            |${info.toList.map { case (key, value) => s"$key: $value" }.mkString("\n")}
+            |""".stripMargin
+      }
+      .mkString("\n")
+    FileToZip("build-targets-info.md", text.getBytes())
+  }
+
+  private def zipReports(additionalToZip: List[FileToZip]): AbsolutePath = {
+    val path = reportContext.reportsDir.resolve(StdReportContext.ZIP_FILE_NAME)
+    val zipOut = new ZipOutputStream(Files.newOutputStream(path.toNIO))
+
+    for {
+      reportsProvider <- reportContext.allToZip
+      report <- reportsProvider.getReports()
+    } {
+      val zipEntry = new ZipEntry(report.name)
+      zipOut.putNextEntry(zipEntry)
+      zipOut.write(Files.readAllBytes(report.toPath))
+    }
+
+    for {
+      toZip <- additionalToZip
+    } {
+      val zipEntry = new ZipEntry(toZip.name)
+      zipOut.putNextEntry(zipEntry)
+      zipOut.write(toZip.text)
+    }
+
+    zipOut.close()
+
+    path
+  }
+}
+
+case class FileToZip(name: String, text: Array[Byte])
diff --git a/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/Doctor.scala
@@ -475,6 +475,12 @@ final class Doctor(
       }
   }
 
+  def getTargetsInfoForReports(): List[Map[String, String]] =
+    allTargetIds()
+      .flatMap(extractTargetInfo(_))
+      .map(_.toMap(exclude = List("gotoCommand", "buildTarget")))
+      .toList
+
   private def extractTargetInfo(
       targetId: BuildTargetIdentifier
   ): List[DoctorTargetInfo] = {

diff --git a/metals/src/main/scala/scala/meta/internal/metals/doctor/DoctorResults.scala b/metals/src/main/scala/scala/meta/internal/metals/doctor/DoctorResults.scala
@@ -75,6 +75,20 @@ final case class DoctorTargetInfo(
       "recommendation" -> recommenedFix,
     )
 
+  def toMap(exclude: List[String] = List()): Map[String, String] =
+    Map(
+      "buildTarget" -> name,
+      "gotoCommand" -> gotoCommand,
+      "compilationStatus" -> compilationStatus.explanation,
+      "targetType" -> targetType,
+      "diagnostics" -> diagnosticsStatus.explanation,
+      "interactive" -> interactiveStatus.explanation,
+      "semanticdb" -> indexesStatus.explanation,
+      "debugging" -> debuggingStatus.explanation,
+      "java" -> javaStatus.explanation,
+      "recommendation" -> recommenedFix,
+    ) -- exclude
+
 }
 
 /**

diff --git a/metals/src/main/scala/scala/meta/internal/parsing/JavaFoldingRangeExtractor.scala b/metals/src/main/scala/scala/meta/internal/parsing/JavaFoldingRangeExtractor.scala
@@ -46,14 +46,16 @@ final class JavaFoldingRangeExtractor(
   def extract(): List[FoldingRange] = {
     val scanner = ToolFactory.createScanner(true, true, false, true)
     scanner.setSource(text.toCharArray())
-
     @tailrec
     def swallowUntilSemicolon(token: Int, line: Int): Int = {
-      val addLine = scanner.getCurrentTokenSource.count(_ == '\n')
-      if (token != ITerminalSymbols.TokenNameSEMICOLON) {
-        swallowUntilSemicolon(scanner.getNextToken(), line + addLine)
-      } else {
-        line + addLine
+      if (token == ITerminalSymbols.TokenNameEOF) line
+      else {
+        val addLine = scanner.getCurrentTokenSource.count(_ == '\n')
+        if (token != ITerminalSymbols.TokenNameSEMICOLON) {
+          swallowUntilSemicolon(scanner.getNextToken(), line + addLine)
+        } else {
+          line + addLine
+        }
       }
     }
 

diff --git a/metals/src/main/scala/scala/meta/internal/parsing/Trees.scala b/metals/src/main/scala/scala/meta/internal/parsing/Trees.scala
@@ -1,15 +1,13 @@
 package scala.meta.internal.parsing
 
-import java.nio.file.Files
-
 import scala.collection.concurrent.TrieMap
 import scala.reflect.ClassTag
 
 import scala.meta._
 import scala.meta.inputs.Position
 import scala.meta.internal.metals.Buffers
-import scala.meta.internal.metals.Directories
 import scala.meta.internal.metals.MetalsEnrichments._
+import scala.meta.internal.metals.ReportContext
 import scala.meta.internal.metals.ScalaVersionSelector
 import scala.meta.io.AbsolutePath
 import scala.meta.parsers.Parse
@@ -30,8 +28,7 @@ import org.eclipse.{lsp4j => l}
 final class Trees(
     buffers: Buffers,
     scalaVersionSelector: ScalaVersionSelector,
-    workspace: AbsolutePath,
-) {
+)(implicit reports: ReportContext) {
 
   private val trees = TrieMap.empty[AbsolutePath, Tree]
 
@@ -141,13 +138,10 @@ final class Trees(
     } catch {
       // if the parsers breaks we should not throw the exception further
       case _: StackOverflowError =>
-        val reportsDir = workspace.resolve(Directories.reports)
-        val newPathCopy =
-          reportsDir.resolve(
-            "stackoverflow_" + System.currentTimeMillis() + "_" + path.filename
-          )
-        Files.createDirectories(reportsDir.toNIO)
-        newPathCopy.writeText(text)
+        val newPathCopy = reports.unsanitized.createReport(
+          s"stackoverflow_${path.filename}",
+          text,
+        )
         val message =
           s"Could not parse $path, saved the current snapshot to ${newPathCopy}"
         scribe.warn(message)

diff --git a/metals/src/main/scala/scala/meta/metals/MetalsLanguageServer.scala b/metals/src/main/scala/scala/meta/metals/MetalsLanguageServer.scala
@@ -16,6 +16,7 @@ import scala.meta.internal.metals.MetalsEnrichments._
 import scala.meta.internal.metals.MetalsLspService
 import scala.meta.internal.metals.MetalsServerInputs
 import scala.meta.internal.metals.MutableCancelable
+import scala.meta.internal.metals.StdReportContext
 import scala.meta.internal.metals.ThreadPools
 import scala.meta.internal.metals.clients.language.MetalsLanguageClient
 import scala.meta.internal.metals.clients.language.NoopLanguageClient
@@ -166,6 +167,8 @@ class MetalsLanguageServer(
           serverState.set(ServerState.Initialized(service))
           metalsService.underlying = service
 
+          new StdReportContext(workspace).cleanUpOldReports()
+
           service.initialize()
       }
     }

diff --git a/mtags/src/main/scala-2/scala/meta/internal/pc/PcCollector.scala b/mtags/src/main/scala-2/scala/meta/internal/pc/PcCollector.scala
@@ -228,7 +228,7 @@ abstract class PcCollector[T](
         def isForComprehensionOwner(named: NameTree) =
           soughtNames(named.name) &&
             named.symbol.owner.isAnonymousFunction && owners.exists(
-              _.pos.point == named.symbol.owner.pos.point
+              pos.isDefined && _.pos.point == named.symbol.owner.pos.point
             )
 
         def soughtOrOverride(sym: Symbol) =