scala 3 repl does not show if a message is an error or a warning (regression from scala 2)

[bjornregnell]

Compiler version

3.0.0

Minimized code and output

scala> val x = if true then 42
1 |val x = if true then 42
  |                     ^^
  |A pure expression does nothing in statement position; you may be omitting necessary parentheses

scala> val x = val
1 |val x = val
  |        ^^^
  |        expression expected but val found

Expectation

The first message above should include warning and the the second should include error. It is conceptually important to understand if the code has run or not, esp. for learners but also for professionals - I don't know all messages by hart if they are warnings or errors. Now it all looks the same in the Scala 3 REPL.

This is a regression from the Scala 2 REPL, in which you can always tell if a message is an error or a warning.

[som-snytt]

Warning text could be in yellow, error text in red and blinking!

But maybe manage that annoying behavior with -color:no-blink?

[bjornregnell]

I'm copy-pasting things around in various teaching material (also for B&W print) and also, (even) more importantly, some people are color blind so I think warning: and error: is still necessary. Although a colorful life is always nice 🌈
(Sorry if I took potential irony too serious :))

[bjornregnell]

@som-snytt Is this not too hard to fix for a REPL-hacker-noob like me, do you think? I mean, I managed to find out how to prohibit mal-truncation last time with your help ⛑️

[SethTisue]

for reference/comparison, Scala 2:

[dwijnand]

I don't know for sure but I would hazard a guess that this is very noob-friendly :)

[bjornregnell]

Thanks @dwijnand for encouragement. I'll hopefully find a coffee-break slot for looking into this soon.

[bjornregnell]

I have started to browse the code and the REPL has ParseResult and the compiler dotc itself has StoreReporter but I'm uncertain if this should just be a tweak to the printout at the outer rim of the REPL or it should go deep down into the rendering of error messages inside the compiler?

[dwijnand]

I think it's a REPL thing, as the batch compiler already displays it its own way:

$ scalac3 test/files/neg/t8597.1.scala
-- Warning: test/files/neg/t8597.1.scala:5:43 ----------------------------------
5 |  def warn1[T]  = (null: Any) match { case _: T            => } // warn
  |                                           ^^^^
  |                          the type test for T cannot be checked at runtime
1 warning found

[bishabosha]

you can look in compiler/src/dotty/tools/dotc/reporting/MessageRendering.scala for the way it is printed in the standard compiler

[bjornregnell]

Thanks for pointers! Will try to grok it. It seems to me that the REPL should reuse all the reporting stuff from dotc sans the file name + line pos marker, but apparently the REPL just utilizes the vertical bar stuff.

[bjornregnell]

My plan is to try to fix this before or during rendering.formatError which is called from displayErrors and I'm starting to investigate it with some good old println instrumentation but it seems as the error level does not differentiate between warnings and errors; check the level output here:

sbt:scala3> repl
[info] compiling 1 Scala source to /home/bjornr/git/hub/bjornregnell/dotty/compiler/target/scala-3.0.0/classes ...
[info] compiling 1 Scala source to /home/bjornr/git/hub/bjornregnell/dotty/out/bootstrap/scala3-compiler-bootstrapped/scala-3.0.2-RC1-bin-SNAPSHOT-nonbootstrapped/classes ...

scala> val x = if true then 42  // should be a warning but level == 2 not 1
*** DEBUG: start of displayErrors(List(class dotty.tools.dotc.reporting.Diagnostic$Error at rs$line$1:[21..23]: A pure expression does nothing in statement position; you may be omitting necessary parentheses))
*** DEBUG: errs.size = 1
*** DEBUG: errs.head.level = 2
*** DEBUG: rendering.formatError(errs.head) = class dotty.tools.dotc.reporting.Diagnostic at rs$line$1:[21..23]: 1 |val x = if true then 42
  |                     ^^
  |A pure expression does nothing in statement position; you may be omitting necessary parentheses
1 |val x = if true then 42
  |                     ^^
  |A pure expression does nothing in statement position; you may be omitting necessary parentheses

scala> val x: String = 42
*** DEBUG: start of displayErrors(List(class dotty.tools.dotc.reporting.Diagnostic$Error at rs$line$1:[16..18]: Found:    (42 : Int)
Required: String))
*** DEBUG: errs.size = 1
*** DEBUG: errs.head.level = 2
*** DEBUG: rendering.formatError(errs.head) = class dotty.tools.dotc.reporting.Diagnostic at rs$line$1:[16..18]: 1 |val x: String = 42
  |                ^^
  |                Found:    (42 : Int)
  |                Required: String
1 |val x: String = 42
  |                ^^
  |                Found:    (42 : Int)
  |                Required: String

scala> def warn[T] = (null: Any) match { case _: T => }  // should also be a warning
*** DEBUG: start of displayErrors(List(class dotty.tools.dotc.reporting.Diagnostic$Error at rs$line$1:<39..39>: the type test for T cannot be checked at runtime))
*** DEBUG: errs.size = 1
*** DEBUG: errs.head.level = 2
*** DEBUG: rendering.formatError(errs.head) = class dotty.tools.dotc.reporting.Diagnostic at rs$line$1:<39..39>: 1 |def warn[T] = (null: Any) match { case _: T => }
  |                                       ^
  |                          the type test for T cannot be checked at runtime
1 |def warn[T] = (null: Any) match { case _: T => }
  |                                       ^
  |                          the type test for T cannot be checked at runtime

scala>                                                                                                                                                                                   

Above is the output of this tweaked displayErrors during the above sbt repl session:

  private def displayErrors(errs: Seq[Diagnostic])(implicit state: State): State = {
    println(s"*** DEBUG: start of displayErrors($errs)")
    println(s"*** DEBUG: errs.size = ${errs.size}")
    println(s"*** DEBUG: errs.head.level = ${errs.head.level}")
    println(s"*** DEBUG: rendering.formatError(errs.head) = ${rendering.formatError(errs.head)}")
    errs.map(rendering.formatError).map(_.msg).foreach(out.println)
    //println(s"*** DEBUG: end of displayErrors: State = $state")
    state
  }

Seems strange that the level attribute is not at level 1... but .... *Edit: see comments below

[bjornregnell]

Perhaps the Build's fatal warnings propagate? I'm checking currently with all fatal warnings turned off in Build.sbt.