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

Check if a fatal warning issued in typer is silenced, before converting it into an error #18089

Merged
merged 1 commit into from
Oct 12, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions compiler/src/dotty/tools/dotc/reporting/Reporter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
32 changes: 16 additions & 16 deletions tests/neg/refutable-pattern-binding-messages.check
Original file line number Diff line number Diff line change
@@ -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
| ^^^^^^^^^^^
Expand All @@ -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
| ^^^^^^
Expand All @@ -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
| ^
Expand Down
16 changes: 8 additions & 8 deletions tests/neg/warn-value-discard.check
Original file line number Diff line number Diff line change
@@ -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
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -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]
24 changes: 24 additions & 0 deletions tests/pos-special/fatal-warnings/i17735.scala
Original file line number Diff line number Diff line change
@@ -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()
24 changes: 24 additions & 0 deletions tests/pos-special/fatal-warnings/i17735a.scala
Original file line number Diff line number Diff line change
@@ -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()
32 changes: 32 additions & 0 deletions tests/pos-special/fatal-warnings/i17741.scala
Original file line number Diff line number Diff line change
@@ -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()
}
6 changes: 6 additions & 0 deletions tests/pos-special/fatal-warnings/nowarnannot.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
case class F(i: Int)

object Main {
def example() =
List(1, 2, 3).map(F): @annotation.nowarn
}