Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: metals error reports #4971

Merged
merged 3 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -23,7 +24,8 @@ class Supermethods(
definitionProvider: DefinitionProvider,
implementationProvider: ImplementationProvider,
)(implicit
ec: ExecutionContext
ec: ExecutionContext,
reports: ReportContext,
) {

def getGoToSuperMethodCommand(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -1867,6 +1874,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(),
tgodzik marked this conversation as resolved.
Show resolved Hide resolved
location.getRange(),
)
)
)
}.asJavaObject
case ServerCommands.ListBuildTargets() =>
Future {
buildTargets.all.toList
Expand Down Expand Up @@ -2734,6 +2758,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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -741,6 +748,7 @@ object ServerCommands {
StopScalaCliServer,
OpenIssue,
OpenFeatureRequest,
ZipReports,
)

val allIds: Set[String] = all.map(_.id).toSet
Expand Down
Original file line number Diff line number Diff line change
@@ -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])
Original file line number Diff line number Diff line change
Expand Up @@ -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] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was looking at errors in metals.log to decide where to create reports and this was throwing an IndexOutOfBounds for scanner.getCurrentTokenSource.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

else {
val addLine = scanner.getCurrentTokenSource.count(_ == '\n')
if (token != ITerminalSymbols.TokenNameSEMICOLON) {
swallowUntilSemicolon(scanner.getNextToken(), line + addLine)
} else {
line + addLine
}
}
}

Expand Down
18 changes: 6 additions & 12 deletions metals/src/main/scala/scala/meta/internal/parsing/Trees.scala
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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]

Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -166,6 +167,8 @@ class MetalsLanguageServer(
serverState.set(ServerState.Initialized(service))
metalsService.underlying = service

new StdReportContext(workspace).cleanUpOldReports()

service.initialize()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again. I had an pos on NoPosition error here in metals.log.

)

def soughtOrOverride(sym: Symbol) =
Expand Down
Loading