feature: metals error reports #4971
Conversation
A framework for creating error reports in metals.
| swallowUntilSemicolon(scanner.getNextToken(), line + addLine) | ||
| } else { | ||
| line + addLine | ||
| if (token == ITerminalSymbols.TokenNameEOF) line |
There was a problem hiding this comment.
I was looking at errors in metals.log to decide where to create reports and this was throwing an IndexOutOfBounds for scanner.getCurrentTokenSource.
| @@ -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 | |||
There was a problem hiding this comment.
Again. I had an pos on NoPosition error here in metals.log.
mtags/src/main/scala/scala/meta/internal/mtags/CommonMtagsEnrichments.scala
Outdated
Show resolved
Hide resolved
metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala
Outdated
Show resolved
Hide resolved
metals/src/main/scala/scala/meta/internal/metals/MetalsLspService.scala
Outdated
Show resolved
Hide resolved
| swallowUntilSemicolon(scanner.getNextToken(), line + addLine) | ||
| } else { | ||
| line + addLine | ||
| if (token == ITerminalSymbols.TokenNameEOF) line |
The errors are chosen based on what reoccurs in `metals.log`.
metals/src/main/scala/scala/meta/internal/implementation/Supermethods.scala
Outdated
Show resolved
Hide resolved
| @@ -31,7 +32,7 @@ final class AutoImportsProvider( | |||
| params: OffsetParams, | |||
| config: PresentationCompilerConfig, | |||
| buildTargetIdentifier: String, | |||
| ): | |||
| )(using Option[Reports]): | |||
There was a problem hiding this comment.
Can we make it not an Option? Maybe the default being some kind of NOP reporting Reporting.empty ?
There was a problem hiding this comment.
How about:
class EmptyReportContext extends OptReportContext {
override def optUnsanitized(): Option[ReportsProvider] = None
override def optIncognito(): Option[ReportsProvider] = None
override def optBloop(): Option[ReportsProvider] = None
}
I'd rather be explicit, that nothing may happen. It's annoying that adding new types of reports would require more boilerplate code. We can instead pass type of report as an arg, but then opt seems to refer to the optional existence of such a raport type.
There was a problem hiding this comment.
I would prefer:
class EmptyReportContext extends OptReportContext {
override def unsanitized(): ReportsProvider = EmptyReporter
override def incognito(): ReportsProvider = EmptyReporter
override def nloop(): ReportsProvider = EmptyReporter
}but it;s fine if you want to go with your approach
| import scala.meta.io.AbsolutePath | ||
|
|
||
| class ZipReportsProvider( | ||
| buildTargetsInfo: () => List[Map[String, String]], |
There was a problem hiding this comment.
| buildTargetsInfo: () => List[Map[String, String]], | |
| doctor: () => MetalsDoctor |
maybe? This way we clearly see that we depend on it.
There was a problem hiding this comment.
That was a cheat to make the tests simpler... I was thinking also about creating a trait, but there wasn't really a good place to put it.
There was a problem hiding this comment.
Ach ok, let's maybe rename it to doctorTargetsInfo and add a comment?
mtags/src/main/scala/scala/meta/internal/metals/ReportsProvider.scala
Outdated
Show resolved
Hide resolved
mtags/src/main/scala/scala/meta/internal/metals/ReportsProvider.scala
Outdated
Show resolved
Hide resolved
zip reports command: - zips error reports - adds build target info from doctor
A framework for creating error reports in metals.
Types of reports:
For all we replace home and workspace paths for generic placeholders.
part of: #4924