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

Remove differences in diagnostic printing in the REPL #13266

Merged
merged 2 commits into from
Sep 1, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 12 additions & 7 deletions compiler/src/dotty/tools/dotc/reporting/MessageRendering.scala
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,19 @@ trait MessageRendering {
.mkString(EOL)
}

/** The source file path, line and column numbers from the given SourcePosition */
def posFileStr(pos: SourcePosition): String =
val path = pos.source.file.path
if pos.exists then s"$path:${pos.line + 1}:${pos.column}" else path

/** The separator between errors containing the source file and error type
*
* @return separator containing error location and kind
*/
def posStr(pos: SourcePosition, diagnosticLevel: String, message: Message)(using Context): String =
if (pos.source != NoSourcePosition.source) hl(diagnosticLevel)({
val pos1 = pos.nonInlined
val file = if !pos.exists then pos1.source.file.toString else
s"${pos1.source.file.toString}:${pos1.line + 1}:${pos1.column}"
val fileAndPos = posFileStr(pos.nonInlined)
val file = if fileAndPos.isEmpty || fileAndPos.endsWith(" ") then fileAndPos else s"$fileAndPos "
val errId =
if (message.errorId ne ErrorMessageID.NoExplanationID) {
val errorNumber = message.errorId.errorNumber
Expand All @@ -124,7 +128,7 @@ trait MessageRendering {
val kind =
if (message.kind == "") diagnosticLevel
else s"${message.kind} $diagnosticLevel"
val prefix = s"-- ${errId}${kind}: $file "
val prefix = s"-- ${errId}${kind}: $file"

prefix +
("-" * math.max(ctx.settings.pageWidth.value - stripColor(prefix).length, 0))
Expand Down Expand Up @@ -192,12 +196,13 @@ trait MessageRendering {

def diagnosticLevel(dia: Diagnostic): String =
dia match {
case dia: Error => "Error"
case dia: FeatureWarning => "Feature Warning"
case dia: DeprecationWarning => "Deprecation Warning"
case dia: UncheckedWarning => "Unchecked Warning"
case dia: MigrationWarning => "Migration Warning"
case dia: Warning => "Warning"
case dia: Info => "Info"
case _ => dia.level match // Diagnostic isn't sealed (e.g. created in the REPL) so provide a fallback
case interfaces.Diagnostic.ERROR => "Error"
case interfaces.Diagnostic.WARNING => "Warning"
case interfaces.Diagnostic.INFO => "Info"
}
}
14 changes: 0 additions & 14 deletions compiler/src/dotty/tools/repl/Rendering.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,6 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None) {

private val MaxStringElements: Int = 1000 // no need to mkString billions of elements

/** A `MessageRenderer` for the REPL without file positions */
private val messageRenderer = new MessageRendering {
override def posStr(pos: SourcePosition, diagnosticLevel: String, message: Message)(using Context): String =
hl(diagnosticLevel)(s"-- $diagnosticLevel:")
}

private var myClassLoader: ClassLoader = _

private var myReplStringOf: Object => String = _
Expand Down Expand Up @@ -126,14 +120,6 @@ private[repl] class Rendering(parentClassLoader: Option[ClassLoader] = None) {
}
}

/** Formats errors using the `messageRenderer` */
def formatError(dia: Diagnostic)(implicit state: State): Diagnostic =
new Diagnostic(
messageRenderer.messageAndPos(dia)(using state.context),
dia.pos,
dia.level
)

def renderTypeDef(d: Denotation)(using Context): Diagnostic =
infoDiagnostic("// defined " ++ d.symbol.showUser, d)

Expand Down
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/repl/ReplCompiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ class ReplCompiler extends Compiler {
private def runCompilationUnit(unit: CompilationUnit, state: State): Result[(CompilationUnit, State)] = {
val ctx = state.context
ctx.run.compileUnits(unit :: Nil)
ctx.run.printSummary() // this outputs "2 errors found" like normal - but we might decide that's needlessly noisy for the REPL

if (!ctx.reporter.hasErrors) (unit, state).result
else ctx.reporter.removeBufferedMessages(using ctx).errors
Expand Down
24 changes: 18 additions & 6 deletions compiler/src/dotty/tools/repl/ReplDriver.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package dotty.tools.repl

import java.io.{File => JFile, PrintStream}
import java.io.{File => JFile, PrintStream, PrintWriter}
import java.nio.charset.StandardCharsets

import dotty.tools.dotc.ast.Trees._
Expand All @@ -17,9 +17,10 @@ import dotty.tools.dotc.core.NameOps._
import dotty.tools.dotc.core.Names.Name
import dotty.tools.dotc.core.StdNames._
import dotty.tools.dotc.core.Symbols.{Symbol, defn}
import dotty.tools.dotc.interfaces
import dotty.tools.dotc.interactive.Completion
import dotty.tools.dotc.printing.SyntaxHighlighting
import dotty.tools.dotc.reporting.{MessageRendering, StoreReporter}
import dotty.tools.dotc.reporting.{ConsoleReporter, MessageRendering, StoreReporter}
import dotty.tools.dotc.reporting.{Message, Diagnostic}
import dotty.tools.dotc.util.Spans.Span
import dotty.tools.dotc.util.{SourceFile, SourcePosition}
Expand Down Expand Up @@ -261,7 +262,6 @@ class ReplDriver(settings: Array[String],

val warnings = newState.context.reporter
.removeBufferedMessages(using newState.context)
.map(rendering.formatError)

inContext(newState.context) {
val (updatedState, definitions) =
Expand All @@ -278,8 +278,7 @@ class ReplDriver(settings: Array[String],

(definitions ++ warnings)
.sorted
.map(_.msg)
.foreach(out.println)
.foreach(printDiagnostic)

updatedState
}
Expand Down Expand Up @@ -422,7 +421,20 @@ class ReplDriver(settings: Array[String],

/** shows all errors nicely formatted */
private def displayErrors(errs: Seq[Diagnostic])(implicit state: State): State = {
errs.map(rendering.formatError).map(_.msg).foreach(out.println)
errs.foreach(printDiagnostic)
state
}

/** Like ConsoleReporter, but without file paths or real -Xprompt'ing */
private object ReplConsoleReporter extends ConsoleReporter(
reader = null, // this short-circuits the -Xprompt display from waiting for an input
writer = new PrintWriter(out, /* autoFlush = */ true), // write to out, not Console.err
) {
override def posFileStr(pos: SourcePosition) = "" // omit file paths
}

/** Print warnings & errors using ReplConsoleReporter, and info straight to out */
private def printDiagnostic(dia: Diagnostic)(implicit state: State) = dia.level match
case interfaces.Diagnostic.INFO => out.println(dia.msg) // print REPL's special info diagnostics directly to out
case _ => ReplConsoleReporter.doReport(dia)(using state.context)
}
5 changes: 3 additions & 2 deletions compiler/test-resources/repl/1379
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
scala> object Foo { val bar = new Object { def baz = 1 }; bar.baz }
-- Error:
-- [E008] Not Found Error: -----------------------------------------------------
1 | object Foo { val bar = new Object { def baz = 1 }; bar.baz }
| ^^^^^^^
| value baz is not a member of Object
| value baz is not a member of Object
1 error found
53 changes: 39 additions & 14 deletions compiler/test-resources/repl/errmsgs
Original file line number Diff line number Diff line change
@@ -1,85 +1,110 @@
scala> class Inv[T](x: T)
// defined class Inv
scala> val x: List[String] = List(1)
-- Error:
-- [E007] Type Mismatch Error: -------------------------------------------------
1 | val x: List[String] = List(1)
| ^
| Found: (1 : Int)
| Required: String
longer explanation available when compiling with `-explain`
1 error found
scala> val y: List[List[String]] = List(List(1))
-- Error:
-- [E007] Type Mismatch Error: -------------------------------------------------
1 | val y: List[List[String]] = List(List(1))
| ^
| Found: (1 : Int)
| Required: String
longer explanation available when compiling with `-explain`
1 error found
scala> val z: (List[String], List[Int]) = (List(1), List("a"))
-- Error:
-- [E007] Type Mismatch Error: -------------------------------------------------
1 | val z: (List[String], List[Int]) = (List(1), List("a"))
| ^
| Found: (1 : Int)
| Required: String
-- Error:
longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: -------------------------------------------------
1 | val z: (List[String], List[Int]) = (List(1), List("a"))
| ^^^
| Found: ("a" : String)
| Required: Int
longer explanation available when compiling with `-explain`
2 errors found
scala> val a: Inv[String] = new Inv(new Inv(1))
-- Error:
-- [E007] Type Mismatch Error: -------------------------------------------------
1 | val a: Inv[String] = new Inv(new Inv(1))
| ^^^^^^^^^^
| Found: Inv[Int]
| Required: String
longer explanation available when compiling with `-explain`
1 error found
scala> val b: Inv[String] = new Inv(1)
-- Error:
-- [E007] Type Mismatch Error: -------------------------------------------------
1 | val b: Inv[String] = new Inv(1)
| ^
| Found: (1 : Int)
| Required: String
longer explanation available when compiling with `-explain`
1 error found
scala> abstract class C { type T; val x: T; val s: Unit = { type T = String; var y: T = x; locally { def f() = { type T = Int; val z: T = y }; f() } }; }
-- Error:
-- [E007] Type Mismatch Error: -------------------------------------------------
1 | abstract class C { type T; val x: T; val s: Unit = { type T = String; var y: T = x; locally { def f() = { type T = Int; val z: T = y }; f() } }; }
| ^
|Found: (C.this.x : C.this.T)
|Required: T²
|
|where: T is a type in class C
| T² is a type in the initializer of value s which is an alias of String
-- Error:
longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: -------------------------------------------------
1 | abstract class C { type T; val x: T; val s: Unit = { type T = String; var y: T = x; locally { def f() = { type T = Int; val z: T = y }; f() } }; }
| ^
|Found: (y : T)
|Required: T²
|
|where: T is a type in the initializer of value s which is an alias of String
| T² is a type in method f which is an alias of Int
longer explanation available when compiling with `-explain`
2 errors found
scala> class Foo() { def bar: Int = 1 }; val foo = new Foo(); foo.barr
-- Error:
-- [E008] Not Found Error: -----------------------------------------------------
1 | class Foo() { def bar: Int = 1 }; val foo = new Foo(); foo.barr
| ^^^^^^^^
| value barr is not a member of Foo - did you mean foo.bar?
1 error found
scala> val x: List[Int] = "foo" :: List(1)
-- Error:
-- [E007] Type Mismatch Error: -------------------------------------------------
1 | val x: List[Int] = "foo" :: List(1)
| ^^^^^
| Found: ("foo" : String)
| Required: Int
longer explanation available when compiling with `-explain`
1 error found
scala> while ((( foo ))) {}
-- Error:
-- [E006] Not Found Error: -----------------------------------------------------
1 | while ((( foo ))) {}
| ^^^
| Not found: foo
longer explanation available when compiling with `-explain`
1 error found
scala> val a: iDontExist = 1
-- Error:
-- [E006] Not Found Error: -----------------------------------------------------
1 | val a: iDontExist = 1
| ^^^^^^^^^^
| Not found: type iDontExist
longer explanation available when compiling with `-explain`
1 error found
scala> def foo1(x: => Int) = x _
-- Error:
-- [E099] Syntax Error: --------------------------------------------------------
1 | def foo1(x: => Int) = x _
| ^^^
|Only function types can be followed by _ but the current expression has type Int
longer explanation available when compiling with `-explain`
1 error found
scala> def foo2(x: => Int): () => Int = x _
-- Error:
-- [E099] Syntax Error: --------------------------------------------------------
1 | def foo2(x: => Int): () => Int = x _
| ^^^
|Only function types can be followed by _ but the current expression has type Int
longer explanation available when compiling with `-explain`
1 error found
3 changes: 2 additions & 1 deletion compiler/test-resources/repl/errorThenValid
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
scala> val xs = scala.collection.mutable.ListBuffer[Int]
-- Error:
-- [E081] Type Error: ----------------------------------------------------------
1 | val xs = scala.collection.mutable.ListBuffer[Int]
| ^
| Missing parameter type
|
| I could not infer the type of the parameter elems.
1 error found
scala> val xs = scala.collection.mutable.ListBuffer[Int]()
val xs: scala.collection.mutable.ListBuffer[Int] = ListBuffer()
4 changes: 3 additions & 1 deletion compiler/test-resources/repl/i13208.default.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
scala> try 1
-- Warning:
1 warning found
-- [E000] Syntax Warning: ------------------------------------------------------
1 | try 1
| ^^^^^
| A try without catch or finally is equivalent to putting
| its body in a block; no exceptions are handled.
longer explanation available when compiling with `-explain`
val res0: Int = 1
3 changes: 2 additions & 1 deletion compiler/test-resources/repl/i1370
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
scala> object Lives { class Private { def foo1: Any = new Private.C1; def foo2: Any = new Private.C2 }; object Private { class C1 private {}; private class C2 {} } }
-- Error:
-- Error: ----------------------------------------------------------------------
1 | object Lives { class Private { def foo1: Any = new Private.C1; def foo2: Any = new Private.C2 }; object Private { class C1 private {}; private class C2 {} } }
| ^^^^^^^^^^
|constructor C1 cannot be accessed as a member of Lives.Private.C1 from class Private.
1 error found
12 changes: 9 additions & 3 deletions compiler/test-resources/repl/i2063
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
scala> class Foo extends Bar // with one tab
-- Error:
-- [E006] Not Found Error: -----------------------------------------------------
1 | class Foo extends Bar // with one tab
| ^^^
| Not found: type Bar
longer explanation available when compiling with `-explain`
1 error found
scala> class Foo extends Bar // with spaces
-- Error:
-- [E006] Not Found Error: -----------------------------------------------------
1 | class Foo extends Bar // with spaces
| ^^^
| Not found: type Bar
longer explanation available when compiling with `-explain`
1 error found
scala> class Foo extends Bar // with tabs
-- Error:
-- [E006] Not Found Error: -----------------------------------------------------
1 | class Foo extends Bar // with tabs
| ^^^
| Not found: type Bar
longer explanation available when compiling with `-explain`
1 error found
6 changes: 4 additions & 2 deletions compiler/test-resources/repl/i2213
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
scala> def x
-- Error:
-- [E019] Syntax Error: --------------------------------------------------------
1 | def x
| ^
| Missing return type
longer explanation available when compiling with `-explain`
scala> def x: Int
-- Error:
-- [E067] Syntax Error: --------------------------------------------------------
1 | def x: Int
| ^
|Declaration of method x not allowed here: only classes can have declared but undefined members
1 error found
5 changes: 3 additions & 2 deletions compiler/test-resources/repl/i2631
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
scala> class Foo(x : Any) { val foo : Integer = 0; def this() = { this(foo) } }
-- Error:
-- Error: ----------------------------------------------------------------------
1 | class Foo(x : Any) { val foo : Integer = 0; def this() = { this(foo) } }
| ^^^
| foo is not accessible from constructor arguments
| foo is not accessible from constructor arguments
1 error found
3 changes: 2 additions & 1 deletion compiler/test-resources/repl/i4184
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ scala> object bar { class Foo }
scala> implicit def eqFoo: CanEqual[foo.Foo, foo.Foo] = CanEqual.derived
def eqFoo: CanEqual[foo.Foo, foo.Foo]
scala> object Bar { new foo.Foo == new bar.Foo }
-- Error:
-- Error: ----------------------------------------------------------------------
1 | object Bar { new foo.Foo == new bar.Foo }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
| Values of types foo.Foo and bar.Foo cannot be compared with == or !=
1 error found
6 changes: 4 additions & 2 deletions compiler/test-resources/repl/i4217
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
scala> def foo(x: Option[Int]) = x match { case None => }
-- Warning:
1 warning found
-- [E029] Pattern Match Exhaustivity Warning: ----------------------------------
1 | def foo(x: Option[Int]) = x match { case None => }
| ^
| match may not be exhaustive.
|
| It would fail on pattern case: Some(_)
def foo(x: Option[Int]): Unit
longer explanation available when compiling with `-explain`
def foo(x: Option[Int]): Unit
4 changes: 3 additions & 1 deletion compiler/test-resources/repl/i4566
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
scala> object test { type ::[A, B]; def a: Int :: Int = ???; def b: Int = a }
-- Error:
-- [E007] Type Mismatch Error: -------------------------------------------------
1 | object test { type ::[A, B]; def a: Int :: Int = ???; def b: Int = a }
| ^
| Found: Int :: Int
| Required: Int
longer explanation available when compiling with `-explain`
1 error found