Skip to content

Commit

Permalink
Merge pull request #10521 from lrytz/filteringQuickfix
Browse files Browse the repository at this point in the history
Message comparison in FilteringReporter strips trailing [quickfixable]
  • Loading branch information
lrytz committed Aug 30, 2023
2 parents 6cbff91 + 361a97c commit b079015
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 66 deletions.
18 changes: 8 additions & 10 deletions src/compiler/scala/tools/nsc/Reporting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -310,16 +310,14 @@ trait Reporting extends internal.Reporting { self: ast.Positions with Compilatio

def featureWarning(pos: Position, featureName: String, featureDesc: String, featureTrait: Symbol, construct: => String = "", required: Boolean, site: Symbol): Unit = {
val req = if (required) "needs to" else "should"
val fqname = "scala.language." + featureName
val explain = (
if (reportedFeature contains featureTrait) "" else
s"""
|----
|This can be achieved by adding the import clause 'import $fqname'
|or by setting the compiler option -language:$featureName.
|See the Scaladoc for value $fqname for a discussion
|why the feature $req be explicitly enabled.""".stripMargin
)
val fqname = s"scala.language.$featureName"
val explain =
if (reportedFeature contains featureTrait) ""
else sm"""|
|This can be achieved by adding the import clause 'import $fqname'
|or by setting the compiler option -language:$featureName.
|See the Scaladoc for value $fqname for a discussion
|why the feature $req be explicitly enabled."""
reportedFeature += featureTrait

val msg = s"$featureDesc $req be enabled\nby making the implicit value $fqname visible.$explain".replace("#", construct)
Expand Down
8 changes: 1 addition & 7 deletions src/compiler/scala/tools/nsc/reporters/PrintReporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,13 @@ trait PrintReporter extends internal.Reporter {

/** Format a message and emit it. */
protected def display(pos: Position, msg: String, severity: Severity): Unit = {
val text = formatMessage(pos, s"${clabel(severity)}${postProcess(msg)}", shortname)
val text = formatMessage(pos, s"${clabel(severity)}${msg}", shortname)
severity match {
case internal.Reporter.INFO => echoMessage(text)
case _ => printMessage(text)
}
}

/** Postprocess a message string for reporting.
*
* The default implementation uses `Reporter.explanation` to include the explanatory addendum.
*/
protected def postProcess(msg: String): String = Reporter.explanation(msg)

def displayPrompt(): Unit = {
writer.println()
writer.print("a)bort, s)tack, r)esume: ")
Expand Down
33 changes: 6 additions & 27 deletions src/compiler/scala/tools/nsc/reporters/Reporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -55,32 +55,6 @@ object Reporter {
val loader = new ClassLoader(getClass.getClassLoader) with ScalaClassLoader
loader.create[FilteringReporter](settings.reporter.value, settings.errorFn)(settings)
}

/** Take the message with its explanation, if it has one, but stripping the separator line.
*/
def explanation(msg: String): String =
if (msg == null) {
msg
} else {
val marker = msg.indexOf("\n----\n")
if (marker > 0) msg.substring(0, marker + 1) + msg.substring(marker + 6) else msg
}

/** Drop any explanation from the message, including the newline between the message and separator line.
*/
def stripExplanation(msg: String): String =
if (msg == null) {
msg
} else {
val marker = msg.indexOf("\n----\n")
if (marker > 0) msg.substring(0, marker) else msg
}

/** Split the message into main message and explanation, as iterators of the text. */
def splitExplanation(msg: String): (Iterator[String], Iterator[String]) = {
val (err, exp) = msg.linesIterator.span(!_.startsWith("----"))
(err, exp.drop(1))
}
}

/** The reporter used in a Global instance.
Expand Down Expand Up @@ -150,12 +124,17 @@ abstract class FilteringReporter extends Reporter {
}
if (show) {
positions(fpos) = severity
messages(fpos) ::= Reporter.stripExplanation(msg) // ignore explanatory suffix for suppressing duplicates
messages(fpos) ::= stripQuickfixable(msg)
}
show
}
}

private def stripQuickfixable(msg: String): String = {
val i = msg.indexOf(" [quickfixable]")
if (i > 0) msg.substring(0, i) else msg
}

override def reset(): Unit = {
super.reset()
positions.clear()
Expand Down
21 changes: 3 additions & 18 deletions src/repl-frontend/scala/tools/nsc/interpreter/shell/Reporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import java.io.PrintWriter
import scala.reflect.internal
import scala.reflect.internal.util.{CodeAction, NoSourceFile, Position, StringOps}
import scala.tools.nsc.interpreter.{Naming, ReplReporter, ReplRequest}
import scala.tools.nsc.reporters.{FilteringReporter, Reporter}
import scala.tools.nsc.reporters.FilteringReporter
import scala.tools.nsc.{ConsoleWriter, NewLinePrintWriter, Settings}

object ReplReporterImpl {
Expand Down Expand Up @@ -154,30 +154,15 @@ class ReplReporterImpl(val config: ShellConfig, val settings: Settings = new Set
if (colorOk) severityColor(severity) + clabel(severity) + RESET
else clabel(severity)

printMessage(pos, prefix + msg)
printMessageAt(pos, prefix + msg)
}

private var boringExplanations = Set.empty[String]

// indent errors, error message uses the caret to point at the line already on the screen instead of repeating it
// TODO: can we splice the error into the code the user typed when multiple lines were entered?
// (should also comment out the error to keep multi-line copy/pastable)
// TODO: multiple errors are not very intuitive (should the second error for same line repeat the line?)
// TODO: the console could be empty due to external changes (also, :reset? -- see unfortunate example in jvm/interpreter (plusOne))
def printMessage(posIn: Position, msg0: String): Unit = {
val msg = {
val main = Reporter.stripExplanation(msg0)
if (main eq msg0) main
else {
val (_, explanation) = Reporter.splitExplanation(msg0)
val suffix = explanation.mkString("\n")
if (boringExplanations(suffix)) main
else {
boringExplanations += suffix
s"$main\n$suffix"
}
}
}
def printMessageAt(posIn: Position, msg: String): Unit = {
if ((posIn eq null) || (posIn.source eq NoSourceFile)) printMessage(msg)
else if (posIn.source.file.name == "<console>" && posIn.line == 1) {
// If there's only one line of input, and it's already printed on the console (as indicated by the position's source file name),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@ class PositionFilterTest {
def `filters split messages`(): Unit = {
val filter = createFilter
val msg = "This is an important warning."
val longMessage = s"""$msg
|----
|Here is some fine print.
|Be sure to read it carefully.""".stripMargin
val longMessage = s"$msg [quickfixable]"
filter.warning(pos, longMessage)
filter.warning(pos, msg)
assertEquals(1, store.infos.size)
Expand Down

0 comments on commit b079015

Please sign in to comment.