From 4cd89177d491df27490bad2af8581e842441d898 Mon Sep 17 00:00:00 2001 From: Katarzyna Marek Date: Wed, 15 Feb 2023 17:19:50 +0100 Subject: [PATCH] reports for chosen errors The errors are chosen based on what reoccurs in `metals.log`. --- .../internal/metals/MetalsEnrichments.scala | 36 ------------------ .../internal/metals/MetalsLspService.scala | 30 +++++++++++++-- .../parsing/JavaFoldingRangeExtractor.scala | 14 ++++--- .../scala/meta/internal/pc/PcCollector.scala | 2 +- .../pc/ScalaPresentationCompiler.scala | 1 + .../internal/pc/AutoImportsProvider.scala | 4 +- .../internal/pc/CompilerSearchVisitor.scala | 10 +++++ .../pc/ScalaPresentationCompiler.scala | 4 ++ .../pc/completions/CompletionProvider.scala | 3 ++ .../internal/pc/completions/Completions.scala | 8 +++- .../completions/InterpolatorCompletions.scala | 11 +++++- .../internal/metals/ReportsProvider.scala | 14 ++++--- .../mtags/CommonMtagsEnrichments.scala | 37 +++++++++++++++++++ 13 files changed, 120 insertions(+), 54 deletions(-) rename {metals => mtags}/src/main/scala/scala/meta/internal/metals/ReportsProvider.scala (87%) diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala index 7a2ec2455c9..bde39b5b563 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala @@ -27,7 +27,6 @@ import scala.concurrent.Promise import scala.concurrent.duration.FiniteDuration import scala.util.Properties import scala.util.Try -import scala.util.control.NonFatal import scala.{meta => m} import scala.meta.Template @@ -488,9 +487,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 +509,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( @@ -1086,11 +1057,4 @@ object MetalsEnrichments } } - implicit class XtensionAny[T](v: T) { - def withExec[R](toExec: T => R): T = { - toExec(v) - v - } - } - } 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 68b42871166..3cb2ee1ba0e 100644 --- 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.Reports import scala.meta.internal.metals.ammonite.Ammonite import scala.meta.internal.metals.callHierarchy.CallHierarchyProvider import scala.meta.internal.metals.clients.language.ConfiguredLanguageClient @@ -1170,9 +1171,25 @@ class MetalsLspService( None } - uriOpt match { - case Some(uri) => { - val path = uri.toAbsolutePath + val pathOpt = uriOpt.flatMap { uri => + try { + Some(uri.toAbsolutePath) + } catch { + case NonFatal(e) => + reports.incognito.createReport( + "did-focus-absolute-path", + s"""|metals/didFocusTextDocument + |Uri: $uri + |Error message: ${e.getMessage()} + |Error: ${e} + |""".stripMargin, + ) + None + } + } + + pathOpt match { + case Some(path) => { focusedDocument = Some(path) buildTargets .inverseSources(path) @@ -2736,6 +2753,13 @@ 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} + |Error message: ${e.getMessage()} + |Error: ${e.underlying} + |""".stripMargin, + ) 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/parsing/JavaFoldingRangeExtractor.scala b/metals/src/main/scala/scala/meta/internal/parsing/JavaFoldingRangeExtractor.scala index ec1f7d77a58..9cee3a6a1ae 100644 --- 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/mtags/src/main/scala-2/scala/meta/internal/pc/PcCollector.scala b/mtags/src/main/scala-2/scala/meta/internal/pc/PcCollector.scala index 5d3d9826122..0b803878e86 100644 --- 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) = diff --git a/mtags/src/main/scala-2/scala/meta/internal/pc/ScalaPresentationCompiler.scala b/mtags/src/main/scala-2/scala/meta/internal/pc/ScalaPresentationCompiler.scala index d1ac210aa13..6b17739857f 100644 --- a/mtags/src/main/scala-2/scala/meta/internal/pc/ScalaPresentationCompiler.scala +++ b/mtags/src/main/scala-2/scala/meta/internal/pc/ScalaPresentationCompiler.scala @@ -20,6 +20,7 @@ import scala.tools.nsc.reporters.StoreReporter import scala.meta.internal.jdk.CollectionConverters._ import scala.meta.internal.metals.EmptyCancelToken +import scala.meta.internal.metals.Reports import scala.meta.internal.mtags.BuildInfo import scala.meta.internal.mtags.MtagsEnrichments._ import scala.meta.pc.AutoImportsResult diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/AutoImportsProvider.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/AutoImportsProvider.scala index 6a0ac25774b..bbe20a8e30e 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/AutoImportsProvider.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/AutoImportsProvider.scala @@ -6,6 +6,7 @@ import java.{util as ju} import scala.collection.mutable import scala.jdk.CollectionConverters.* +import scala.meta.internal.metals.Reports import scala.meta.internal.mtags.MtagsEnrichments.* import scala.meta.internal.pc.AutoImports.* import scala.meta.internal.pc.completions.CompletionPos @@ -31,6 +32,7 @@ final class AutoImportsProvider( params: OffsetParams, config: PresentationCompilerConfig, buildTargetIdentifier: String, + reports: Option[Reports], ): def autoImports(isExtension: Boolean): List[AutoImportsResult] = @@ -66,7 +68,7 @@ final class AutoImportsProvider( def isExactMatch(sym: Symbol, query: String): Boolean = sym.name.show == query - val visitor = new CompilerSearchVisitor(name, visit) + val visitor = new CompilerSearchVisitor(name, visit, reports) if isExtension then search.searchMethods(name, buildTargetIdentifier, visitor) else search.search(name, buildTargetIdentifier, visitor) diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/CompilerSearchVisitor.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/CompilerSearchVisitor.scala index cbc6408f629..47435a9b2c1 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/CompilerSearchVisitor.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/CompilerSearchVisitor.scala @@ -5,6 +5,7 @@ import java.util.logging.Logger import scala.util.control.NonFatal +import scala.meta.internal.metals.Reports import scala.meta.pc.* import dotty.tools.dotc.core.Contexts.* @@ -16,6 +17,7 @@ import dotty.tools.dotc.core.Symbols.* class CompilerSearchVisitor( query: String, visitSymbol: Symbol => Boolean, + reports: Option[Reports], )(using ctx: Context) extends SymbolSearchVisitor: @@ -25,6 +27,14 @@ class CompilerSearchVisitor( sym != NoSymbol && sym.isPublic catch case NonFatal(e) => + reports.foreach( + _.incognito.createReport( + "is_public", + s"""|Symbol: $sym + |Error message: ${e.getMessage()} + |""".stripMargin, + ) + ) logger.log(Level.SEVERE, e.getMessage(), e) false diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/ScalaPresentationCompiler.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/ScalaPresentationCompiler.scala index e44c2f9fb23..492b5d29969 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/ScalaPresentationCompiler.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/ScalaPresentationCompiler.scala @@ -15,6 +15,7 @@ import scala.concurrent.ExecutionContext import scala.concurrent.ExecutionContextExecutor import scala.meta.internal.metals.EmptyCancelToken +import scala.meta.internal.metals.Reports import scala.meta.internal.mtags.BuildInfo import scala.meta.internal.mtags.MtagsEnrichments.* import scala.meta.internal.pc.AutoImports.* @@ -57,6 +58,7 @@ case class ScalaPresentationCompiler( private val forbiddenOptions = Set("-print-lines", "-print-tasty") private val forbiddenDoubleOptions = Set("-release") + private val optReports = workspace.map(Reports(_)) val compilerAccess: CompilerAccess[StoreReporter, MetalsDriver] = Scala3CompilerAccess( @@ -115,6 +117,7 @@ case class ScalaPresentationCompiler( config, buildTargetIdentifier, workspace, + optReports, ).completions() } @@ -205,6 +208,7 @@ case class ScalaPresentationCompiler( params, config, buildTargetIdentifier, + optReports, ) .autoImports(isExtension) .asJava diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionProvider.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionProvider.scala index 9da0c20e332..f48b75fa34f 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionProvider.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/CompletionProvider.scala @@ -6,6 +6,7 @@ import java.nio.file.Path import scala.annotation.tailrec import scala.collection.JavaConverters.* +import scala.meta.internal.metals.Reports import scala.meta.internal.mtags.MtagsEnrichments.* import scala.meta.internal.pc.AutoImports.AutoImportEdits import scala.meta.internal.pc.AutoImports.AutoImportsGenerator @@ -38,6 +39,7 @@ class CompletionProvider( config: PresentationCompilerConfig, buildTargetIdentifier: String, workspace: Option[Path], + reports: Option[Reports], ): def completions(): CompletionList = val uri = params.uri @@ -84,6 +86,7 @@ class CompletionProvider( workspace, autoImportsGen, driver.settings, + reports, ).completions() val items = completions.zipWithIndex.map { case (item, idx) => diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/Completions.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/Completions.scala index b268612ca0a..7ce8c14933d 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/Completions.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/Completions.scala @@ -7,6 +7,7 @@ import java.nio.file.Paths import scala.collection.mutable import scala.meta.internal.metals.Fuzzy +import scala.meta.internal.metals.Reports import scala.meta.internal.mtags.BuildInfo import scala.meta.internal.mtags.MtagsEnrichments.* import scala.meta.internal.pc.AutoImports.AutoImportsGenerator @@ -51,6 +52,7 @@ class Completions( workspace: Option[Path], autoImports: AutoImportsGenerator, options: List[String], + reports: Option[Reports], ): implicit val context: Context = ctx @@ -469,6 +471,7 @@ class Completions( search, config, buildTargetIdentifier, + reports, ) .filterInteresting(enrich = false) ._1 @@ -579,7 +582,9 @@ class Completions( sym, sym.decodedName, CompletionValue.Workspace(_, _, _, sym), - ).forall(visit), + ).forall(visit) + , + reports, ) Some(search.search(query, buildTargetIdentifier, visitor)) case CompletionKind.Members if query.nonEmpty => @@ -595,6 +600,7 @@ class Completions( CompletionValue.Extension(_, _, _), ).forall(visit) else false, + reports, ) Some(search.searchMethods(query, buildTargetIdentifier, visitor)) case CompletionKind.Members => // query.isEmpry diff --git a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/InterpolatorCompletions.scala b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/InterpolatorCompletions.scala index 1d4c23fe2bf..8a7f9c4ff8d 100644 --- a/mtags/src/main/scala-3/scala/meta/internal/pc/completions/InterpolatorCompletions.scala +++ b/mtags/src/main/scala-3/scala/meta/internal/pc/completions/InterpolatorCompletions.scala @@ -3,6 +3,7 @@ package scala.meta.internal.pc.completions import scala.collection.mutable.ListBuffer import scala.meta.internal.metals.Fuzzy +import scala.meta.internal.metals.Reports import scala.meta.internal.mtags.MtagsEnrichments.* import scala.meta.internal.mtags.MtagsEnrichments.given import scala.meta.internal.pc.AutoImports.AutoImport @@ -41,6 +42,7 @@ object InterpolatorCompletions: search: SymbolSearch, config: PresentationCompilerConfig, buildTargetIdentifier: String, + reports: Option[Reports], )(using Context) = InterpolationSplice(pos.span.point, text.toCharArray(), text) match case Some(interpolator) => @@ -58,6 +60,7 @@ object InterpolatorCompletions: search, config, buildTargetIdentifier, + reports, ) case None => InterpolatorCompletions.contributeMember( @@ -70,6 +73,7 @@ object InterpolatorCompletions: snippetsEnabled, search, buildTargetIdentifier, + reports, ) end match end contribute @@ -122,6 +126,7 @@ object InterpolatorCompletions: areSnippetsSupported: Boolean, search: SymbolSearch, buildTargetIdentifier: String, + reports: Option[Reports], )(using Context): List[CompletionValue] = def newText( name: String, @@ -152,6 +157,7 @@ object InterpolatorCompletions: buffer.append(sym) true else false, + reports, ) search.searchMethods(completionPos.query, buildTargetIdentifier, visitor) buffer.toList @@ -241,6 +247,7 @@ object InterpolatorCompletions: search: SymbolSearch, config: PresentationCompilerConfig, buildTargetIdentifier: String, + reports: Option[Reports], )(using ctx: Context): List[CompletionValue] = val litStartPos = lit.span.start val litEndPos = lit.span.end - Cursor.value.length() @@ -289,7 +296,9 @@ object InterpolatorCompletions: case IndexedContext.Result.InScope => false case _ => if sym.is(Flags.Module) then workspaceSymbols += sym - true, + true + , + reports, ) if interpolator.name.nonEmpty then search.search(interpolator.name, buildTargetIdentifier, visitor) diff --git a/metals/src/main/scala/scala/meta/internal/metals/ReportsProvider.scala b/mtags/src/main/scala/scala/meta/internal/metals/ReportsProvider.scala similarity index 87% rename from metals/src/main/scala/scala/meta/internal/metals/ReportsProvider.scala rename to mtags/src/main/scala/scala/meta/internal/metals/ReportsProvider.scala index 7d48a5b021c..a309bd093d8 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/ReportsProvider.scala +++ b/mtags/src/main/scala/scala/meta/internal/metals/ReportsProvider.scala @@ -6,21 +6,21 @@ import java.nio.file.Path import java.util.zip.ZipEntry import java.util.zip.ZipOutputStream -import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.mtags.MtagsEnrichments._ import scala.meta.io.AbsolutePath import scala.meta.io.RelativePath class Reports(workspace: AbsolutePath) { private lazy val reportsDir = - workspace.resolve(Directories.reports).withExec { d => + workspace.resolve(Reports.reportsDir).withExec { d => Files.createDirectories(d.toNIO) } val unsanitized = - new ReportsProvider(workspace, Directories.reports.resolve("metals-full")) + new ReportsProvider(workspace, Reports.reportsDir.resolve("metals-full")) val incognito = - new ReportsProvider(workspace, Directories.reports.resolve("metals")) + new ReportsProvider(workspace, Reports.reportsDir.resolve("metals")) val bloop = - new ReportsProvider(workspace, Directories.reports.resolve("bloop")) + new ReportsProvider(workspace, Reports.reportsDir.resolve("bloop")) def all: List[ReportsProvider] = List(unsanitized, incognito, bloop) def allToZip: List[ReportsProvider] = List(incognito, bloop) @@ -105,6 +105,10 @@ object Reports { val WORKSPACE_STR = "" val HOME_STR = "" val ZIP_FILE_NAME = "reports.zip" + + def reportsDir: RelativePath = + RelativePath(".metals").resolve(".reports") + def apply(path: Path) = new Reports(AbsolutePath(path)) } case class Report(file: File, timestamp: Long) { diff --git a/mtags/src/main/scala/scala/meta/internal/mtags/CommonMtagsEnrichments.scala b/mtags/src/main/scala/scala/meta/internal/mtags/CommonMtagsEnrichments.scala index 6623bc280d1..409ccc17cb0 100644 --- a/mtags/src/main/scala/scala/meta/internal/mtags/CommonMtagsEnrichments.scala +++ b/mtags/src/main/scala/scala/meta/internal/mtags/CommonMtagsEnrichments.scala @@ -5,6 +5,8 @@ import java.nio.charset.StandardCharsets import java.nio.file.Files import java.nio.file.Path import java.nio.file.Paths +import java.nio.file.StandardCopyOption +import java.nio.file.StandardOpenOption import java.util import java.util.logging.Level import java.util.logging.Logger @@ -582,6 +584,34 @@ trait CommonMtagsEnrichments { def startWith(other: AbsolutePath): Boolean = path.toNIO.startsWith(other.toNIO) + + def createDirectories(): AbsolutePath = + AbsolutePath(Files.createDirectories(path.dealias.toNIO)) + + 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) + } + } } implicit class XtensionJavaPriorityQueue[A](q: util.PriorityQueue[A]) { @@ -678,4 +708,11 @@ trait CommonMtagsEnrichments { } val EXTENSION: Int = s.SymbolInformation.Property.values.map(_.value).max << 1 + + implicit class XtensionAny[T](v: T) { + def withExec[R](toExec: T => R): T = { + toExec(v) + v + } + } }