From ad45c3e9b7843f2d6735816d955924e92702805c Mon Sep 17 00:00:00 2001 From: Kacper Korban Date: Wed, 28 Jun 2023 15:05:40 +0200 Subject: [PATCH] Check if a fatal warning issued in typer is silenced, before converting it into an error closes lampepfl#17741 closes lampepfl#17735 --- .../dotty/tools/dotc/reporting/Reporter.scala | 24 +++++++------- .../refutable-pattern-binding-messages.check | 32 +++++++++---------- tests/neg/warn-value-discard.check | 16 +++++----- tests/pos-special/fatal-warnings/i17735.scala | 24 ++++++++++++++ .../pos-special/fatal-warnings/i17735a.scala | 24 ++++++++++++++ tests/pos-special/fatal-warnings/i17741.scala | 32 +++++++++++++++++++ .../fatal-warnings/nowarnannot.scala | 6 ++++ 7 files changed, 123 insertions(+), 35 deletions(-) create mode 100644 tests/pos-special/fatal-warnings/i17735.scala create mode 100644 tests/pos-special/fatal-warnings/i17735a.scala create mode 100644 tests/pos-special/fatal-warnings/i17741.scala create mode 100644 tests/pos-special/fatal-warnings/nowarnannot.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala index f5aadac27296..f9e2cfd3ea3b 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala @@ -149,14 +149,10 @@ abstract class Reporter extends interfaces.ReporterResult { val key = w.enablingOption.name addUnreported(key, 1) case _ => - // conditional warnings that are not enabled are not fatal - val d = dia match - case w: Warning if ctx.settings.XfatalWarnings.value => w.toError - case _ => dia - if !isHidden(d) then // avoid isHidden test for summarized warnings so that message is not forced - markReported(d) - withMode(Mode.Printing)(doReport(d)) - d match { + if !isHidden(dia) then // avoid isHidden test for summarized warnings so that message is not forced + markReported(dia) + withMode(Mode.Printing)(doReport(dia)) + dia match { case _: Warning => _warningCount += 1 case e: Error => errors = e :: errors @@ -169,13 +165,19 @@ abstract class Reporter extends interfaces.ReporterResult { end issueUnconfigured def issueIfNotSuppressed(dia: Diagnostic)(using Context): Unit = + def toErrorIfFatal(dia: Diagnostic) = dia match + case w: Warning if ctx.settings.silentWarnings.value => dia + case w: ConditionalWarning if w.isSummarizedConditional => dia + case w: Warning if ctx.settings.XfatalWarnings.value => w.toError + case _ => dia + def go() = import Action._ dia match - case w: Warning => WConf.parsed.action(w) match + case w: Warning => WConf.parsed.action(dia) match case Error => issueUnconfigured(w.toError) - case Warning => issueUnconfigured(w) - case Verbose => issueUnconfigured(w.setVerbose()) + case Warning => issueUnconfigured(toErrorIfFatal(w)) + case Verbose => issueUnconfigured(toErrorIfFatal(w.setVerbose())) case Info => issueUnconfigured(w.toInfo) case Silent => case _ => issueUnconfigured(dia) diff --git a/tests/neg/refutable-pattern-binding-messages.check b/tests/neg/refutable-pattern-binding-messages.check index 5a9d85fd4447..b1b8866e174f 100644 --- a/tests/neg/refutable-pattern-binding-messages.check +++ b/tests/neg/refutable-pattern-binding-messages.check @@ -1,3 +1,11 @@ +-- Error: tests/neg/refutable-pattern-binding-messages.scala:5:14 ------------------------------------------------------ +5 | val Positive(p) = 5 // error: refutable extractor + | ^^^^^^^^^^^^^^^ + | pattern binding uses refutable extractor `Test.Positive` + | + | If this usage is intentional, this can be communicated by adding `: @unchecked` after the expression, + | which may result in a MatchError at runtime. + | This patch can be rewritten automatically under -rewrite -source 3.2-migration. -- Error: tests/neg/refutable-pattern-binding-messages.scala:6:14 ------------------------------------------------------ 6 | for Positive(i) <- List(1, 2, 3) do () // error: refutable extractor | ^^^^^^^^^^^ @@ -6,6 +14,14 @@ | If this usage is intentional, this can be communicated by adding the `case` keyword before the full pattern, | which will result in a filtering for expression (using `withFilter`). | This patch can be rewritten automatically under -rewrite -source 3.2-migration. +-- Error: tests/neg/refutable-pattern-binding-messages.scala:10:20 ----------------------------------------------------- +10 | val i :: is = List(1, 2, 3) // error: pattern type more specialized + | ^^^^^^^^^^^^^ + | pattern's type ::[Int] is more specialized than the right hand side expression's type List[Int] + | + | If the narrowing is intentional, this can be communicated by adding `: @unchecked` after the expression, + | which may result in a MatchError at runtime. + | This patch can be rewritten automatically under -rewrite -source 3.2-migration. -- Error: tests/neg/refutable-pattern-binding-messages.scala:11:11 ----------------------------------------------------- 11 | for ((x: String) <- xs) do () // error: pattern type more specialized | ^^^^^^ @@ -22,22 +38,6 @@ | If the narrowing is intentional, this can be communicated by adding the `case` keyword before the full pattern, | which will result in a filtering for expression (using `withFilter`). | This patch can be rewritten automatically under -rewrite -source 3.2-migration. --- Error: tests/neg/refutable-pattern-binding-messages.scala:5:14 ------------------------------------------------------ -5 | val Positive(p) = 5 // error: refutable extractor - | ^^^^^^^^^^^^^^^ - | pattern binding uses refutable extractor `Test.Positive` - | - | If this usage is intentional, this can be communicated by adding `: @unchecked` after the expression, - | which may result in a MatchError at runtime. - | This patch can be rewritten automatically under -rewrite -source 3.2-migration. --- Error: tests/neg/refutable-pattern-binding-messages.scala:10:20 ----------------------------------------------------- -10 | val i :: is = List(1, 2, 3) // error: pattern type more specialized - | ^^^^^^^^^^^^^ - | pattern's type ::[Int] is more specialized than the right hand side expression's type List[Int] - | - | If the narrowing is intentional, this can be communicated by adding `: @unchecked` after the expression, - | which may result in a MatchError at runtime. - | This patch can be rewritten automatically under -rewrite -source 3.2-migration. -- Error: tests/neg/refutable-pattern-binding-messages.scala:16:10 ----------------------------------------------------- 16 | val 1 = 2 // error: pattern type does not match | ^ diff --git a/tests/neg/warn-value-discard.check b/tests/neg/warn-value-discard.check index ab6539dd5cd8..ba43c743709f 100644 --- a/tests/neg/warn-value-discard.check +++ b/tests/neg/warn-value-discard.check @@ -1,11 +1,3 @@ --- [E175] Potential Issue Error: tests/neg/warn-value-discard.scala:15:35 ---------------------------------------------- -15 | firstThing().map(_ => secondThing()) // error - | ^^^^^^^^^^^^^ - | discarded non-Unit value of type Either[Failed, Unit] --- [E175] Potential Issue Error: tests/neg/warn-value-discard.scala:18:35 ---------------------------------------------- -18 | firstThing().map(_ => secondThing()) // error - | ^^^^^^^^^^^^^ - | discarded non-Unit value of type Either[Failed, Unit] -- [E175] Potential Issue Error: tests/neg/warn-value-discard.scala:27:36 ---------------------------------------------- 27 | mutable.Set.empty[String].remove("") // error | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -18,3 +10,11 @@ 59 | mutable.Set.empty[String] += "" // error | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | discarded non-Unit value of type scala.collection.mutable.Set[String] +-- [E175] Potential Issue Error: tests/neg/warn-value-discard.scala:15:35 ---------------------------------------------- +15 | firstThing().map(_ => secondThing()) // error + | ^^^^^^^^^^^^^ + | discarded non-Unit value of type Either[Failed, Unit] +-- [E175] Potential Issue Error: tests/neg/warn-value-discard.scala:18:35 ---------------------------------------------- +18 | firstThing().map(_ => secondThing()) // error + | ^^^^^^^^^^^^^ + | discarded non-Unit value of type Either[Failed, Unit] diff --git a/tests/pos-special/fatal-warnings/i17735.scala b/tests/pos-special/fatal-warnings/i17735.scala new file mode 100644 index 000000000000..56050d8fd5fd --- /dev/null +++ b/tests/pos-special/fatal-warnings/i17735.scala @@ -0,0 +1,24 @@ +// scalac: -Wvalue-discard + +import scala.collection.mutable +import scala.annotation.nowarn + +object Foo: + + def f(b: Boolean): String = + val messageBuilder = mutable.StringBuilder() + if b then + // Here @nowarn is effective with or without -Wfatal-warnings + // i.e. no warning without -Wfatal-warnings and no error with -Wfatal-warnings + messageBuilder.append("helloworld").append("\n"): @nowarn("msg=discarded non-Unit value*") + + messageBuilder.result() + + def g(x: String => Unit) = ??? + def h: String = + val messageBuilder = mutable.StringBuilder() + g: s => + // here @nowarn is effective without -Wfatal-warnings (i.e. no warning) + // But with -Wfatal-warnings we get an error + messageBuilder.append("\n").append(s): @nowarn("msg=discarded non-Unit value*") + messageBuilder.result() \ No newline at end of file diff --git a/tests/pos-special/fatal-warnings/i17735a.scala b/tests/pos-special/fatal-warnings/i17735a.scala new file mode 100644 index 000000000000..d089763295e6 --- /dev/null +++ b/tests/pos-special/fatal-warnings/i17735a.scala @@ -0,0 +1,24 @@ +// scalac: -Wvalue-discard -Wconf:msg=non-Unit:s + +import scala.collection.mutable +import scala.annotation.nowarn + +object Test: + + def f(b: Boolean): String = + val messageBuilder = mutable.StringBuilder() + if b then + // Here @nowarn is effective with or without -Wfatal-warnings + // i.e. no warning without -Wfatal-warnings and no error with -Wfatal-warnings + messageBuilder.append("helloworld").append("\n") + + messageBuilder.result() + + def g(x: String => Unit) = ??? + def h: String = + val messageBuilder = mutable.StringBuilder() + g: s => + // here @nowarn is effective without -Wfatal-warnings (i.e. no warning) + // But with -Wfatal-warnings we get an error + messageBuilder.append("\n").append(s) + messageBuilder.result() diff --git a/tests/pos-special/fatal-warnings/i17741.scala b/tests/pos-special/fatal-warnings/i17741.scala new file mode 100644 index 000000000000..07af8b1abd3d --- /dev/null +++ b/tests/pos-special/fatal-warnings/i17741.scala @@ -0,0 +1,32 @@ +// scalac: -Wnonunit-statement + +class Node() +class Elem( + prefix: String, + label: String, + minimizeEmpty: Boolean, + child: Node* +) extends Node +class Text(text: String) extends Node +class NodeBuffer() { + def &+(node: Node): NodeBuffer = + this +} +class NodeSeq() +object NodeSeq { + def seqToNodeSeq(seq: NodeBuffer): Seq[Node] = ??? +} + +object Main { + def example() = { + { + new Elem(null, "foo", false, + { + val $buf: NodeBuffer = new NodeBuffer() + $buf.&+(new Text("bar")) + NodeSeq.seqToNodeSeq($buf) + }* + ) + } + }: @annotation.nowarn() +} \ No newline at end of file diff --git a/tests/pos-special/fatal-warnings/nowarnannot.scala b/tests/pos-special/fatal-warnings/nowarnannot.scala new file mode 100644 index 000000000000..26e9713d0543 --- /dev/null +++ b/tests/pos-special/fatal-warnings/nowarnannot.scala @@ -0,0 +1,6 @@ +case class F(i: Int) + +object Main { + def example() = + List(1, 2, 3).map(F): @annotation.nowarn +}