Skip to content

Commit c308dc1

Browse files
lrytzlrytz
committed
review feedback
1 parent b26acda commit c308dc1

File tree

5 files changed

+88
-67
lines changed

5 files changed

+88
-67
lines changed

‎compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
package dotty.tools.dotc.reporting
22

33
/** Unique IDs identifying the messages */
4-
enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
4+
enum ErrorMessageID extends java.lang.Enum[ErrorMessageID]:
55

66
// IMPORTANT: Add new IDs only at the end and never remove IDs
77
case
@@ -177,4 +177,6 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
177177
CannotExtendFunctionID
178178

179179
def errorNumber = ordinal - 2
180-
}
180+
181+
object ErrorMessageID:
182+
def fromErrorNumber(n: Int) = fromOrdinal(n + 2)

‎compiler/src/dotty/tools/dotc/reporting/WConf.scala‎

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import scala.collection.mutable.ListBuffer
1111
import scala.util.matching.Regex
1212

1313
enum MessageFilter:
14-
def matches(message: Diagnostic): Boolean = this match {
14+
def matches(message: Diagnostic): Boolean = this match
1515
case Any => true
1616
case Deprecated => message.isInstanceOf[Diagnostic.DeprecationWarning]
1717
case Feature => message.isInstanceOf[Diagnostic.FeatureWarning]
@@ -21,7 +21,7 @@ enum MessageFilter:
2121
pattern.findFirstIn(noHighlight).nonEmpty
2222
case MessageID(errorId) => message.msg.errorId == errorId
2323
case None => false
24-
}
24+
2525
case Any, Deprecated, Feature, Unchecked, None
2626
case MessagePattern(pattern: Regex)
2727
case MessageID(errorId: ErrorMessageID)
@@ -40,31 +40,37 @@ object WConf:
4040

4141
private type Conf = (List[MessageFilter], Action)
4242

43-
def parseAction(s: String): Either[List[String], Action] = s match {
43+
def parseAction(s: String): Either[List[String], Action] = s match
4444
case "error" | "e" => Right(Error)
4545
case "warning" | "w" => Right(Warning)
4646
case "verbose" | "v" => Right(Verbose)
4747
case "info" | "i" => Right(Info)
4848
case "silent" | "s" => Right(Silent)
4949
case _ => Left(List(s"unknown action: `$s`"))
50-
}
5150

5251
private def regex(s: String) =
5352
try Right(s.r)
54-
catch { case e: PatternSyntaxException => Left(s"invalid pattern `$s`: ${e.getMessage}") }
53+
catch case e: PatternSyntaxException => Left(s"invalid pattern `$s`: ${e.getMessage}")
5554

5655
@sharable val Splitter = raw"([^=]+)=(.+)".r
5756
@sharable val ErrorId = raw"E?(\d+)".r
5857

58+
def parseFilters(s: String): Either[List[String], List[MessageFilter]] =
59+
// TODO: don't split on escaped \&
60+
val (parseErrors, filters) = s.split('&').view.map(parseFilter).toList.partitionMap(identity)
61+
if parseErrors.nonEmpty then Left(parseErrors)
62+
else if filters.isEmpty then Left(List("no filters or no action defined"))
63+
else Right(filters)
64+
5965
def parseFilter(s: String): Either[String, MessageFilter] = s match
6066
case "any" => Right(Any)
6167
case Splitter(filter, conf) => filter match
6268
case "msg" => regex(conf).map(MessagePattern.apply)
6369
case "id" => conf match
6470
case ErrorId(num) =>
65-
val n = num.toInt + 2
71+
val n = num.toInt
6672
if n < ErrorMessageID.values.length then
67-
Right(MessageID(ErrorMessageID.fromOrdinal(n)))
73+
Right(MessageID(ErrorMessageID.fromErrorNumber(n)))
6874
else
6975
Left(s"unknonw error message id: E$n")
7076
case _ =>
@@ -99,25 +105,22 @@ object WConf:
99105

100106
def fromSettings(settings: List[String]): Either[List[String], WConf] =
101107
if (settings.isEmpty) Right(WConf(Nil))
102-
else {
103-
val parsedConfs: List[Either[List[String], (List[MessageFilter], Action)]] = settings.map(conf => {
104-
val parts = conf.split("[&:]") // TODO: don't split on escaped \&
105-
val (ms, fs) = parts.view.init.map(parseFilter).toList.partitionMap(identity)
106-
if (ms.nonEmpty) Left(ms)
107-
else if (fs.isEmpty) Left(List("no filters or no action defined"))
108-
else parseAction(parts.last).map((fs, _))
109-
})
110-
val (ms, fs) = parsedConfs.partitionMap(identity)
111-
if (ms.nonEmpty) Left(ms.flatten)
112-
else Right(WConf(fs))
113-
}
114-
115-
case class Suppression(annotPos: SourcePosition, filters: List[MessageFilter], start: Int, end: Int, verbose: Boolean):
108+
else
109+
val parsedConfs: List[Either[List[String], (List[MessageFilter], Action)]] = settings.map(conf =>
110+
val filtersAndAction = conf.split(':')
111+
if filtersAndAction.length != 2 then Left(List("exactly one `:` expected (<filter>&...&<filter>:<action>)"))
112+
else
113+
parseFilters(filtersAndAction(0)).flatMap(filters =>
114+
parseAction(filtersAndAction(1)).map((filters, _))))
115+
val (parseErrorss, configs) = parsedConfs.partitionMap(identity)
116+
if (parseErrorss.nonEmpty) Left(parseErrorss.flatten)
117+
else Right(WConf(configs))
118+
119+
class Suppression(val annotPos: SourcePosition, filters: List[MessageFilter], val start: Int, end: Int, val verbose: Boolean):
116120
private[this] var _used = false
117121
def used: Boolean = _used
118-
def markUsed(): Unit = { _used = true }
122+
def markUsed(): Unit = _used = true
119123

120-
def matches(dia: Diagnostic): Boolean = {
124+
def matches(dia: Diagnostic): Boolean =
121125
val pos = dia.pos
122-
pos.exists && start <= pos.start && pos.end <= end && (verbose || filters.forall(_.matches(dia)))
123-
}
126+
pos.exists && start <= pos.start && pos.end <= end && filters.forall(_.matches(dia))

‎compiler/src/dotty/tools/dotc/typer/Typer.scala‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2091,27 +2091,30 @@ class Typer extends Namer
20912091
val annot = Annotations.Annotation(tree)
20922092
def argPos = annot.argument(0).getOrElse(tree).sourcePos
20932093
var verbose = false
2094+
var erroneous = false
20942095
val filters =
20952096
if annot.arguments.isEmpty then List(MessageFilter.Any)
20962097
else annot.argumentConstantString(0) match
20972098
case None => annot.argument(0) match
2098-
case Some(t: Select) if t.name.is(DefaultGetterName) => List(MessageFilter.Any)
2099+
case Some(t: Select) if t.name.is(DefaultGetterName) =>
2100+
// default argument used for `@nowarn` and `@nowarn()`
2101+
List(MessageFilter.Any)
20992102
case _ =>
21002103
report.warning(s"filter needs to be a compile-time constant string", argPos)
2101-
Nil
2102-
case Some("") => Nil
2104+
List(MessageFilter.None)
2105+
case Some("") =>
2106+
List(MessageFilter.Any)
21032107
case Some("verbose") | Some("v") =>
21042108
verbose = true
2105-
Nil
2109+
List(MessageFilter.Any)
21062110
case Some(s) =>
2107-
val (ms, fs) = s.split('&').map(WConf.parseFilter).toList.partitionMap(identity)
2108-
if ms.nonEmpty then
2109-
report.warning(s"Invalid message filter\n${ms.mkString("\n")}", argPos)
2110-
List(MessageFilter.None)
2111-
else
2112-
fs
2111+
WConf.parseFilters(s).fold(parseErrors =>
2112+
report.warning (s"Invalid message filter\n${parseErrors.mkString ("\n")}", argPos)
2113+
List(MessageFilter.None),
2114+
identity)
21132115
val range = mdef.sourcePos
21142116
val sup = Suppression(tree.sourcePos, filters, range.start, range.end, verbose)
2117+
// invalid suppressions, don't report as unused
21152118
if filters == List(MessageFilter.None) then sup.markUsed()
21162119
ctx.run.suppressions.addSuppression(sup)
21172120

‎tests/neg-custom-args/nowarn/nowarn.check‎

Lines changed: 40 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,15 @@
55
| its body in a block; no exceptions are handled.
66

77
longer explanation available when compiling with `-explain`
8-
-- [E000] Syntax Warning: tests/neg-custom-args/nowarn/nowarn.scala:21:26 ----------------------------------------------
9-
21 |@nowarn("id=1") def t4d = try 1 // error and warning (unused nowarn, wrong id)
8+
-- [E000] Syntax Warning: tests/neg-custom-args/nowarn/nowarn.scala:24:26 ----------------------------------------------
9+
24 |@nowarn("id=1") def t4d = try 1 // error and warning (unused nowarn, wrong id)
1010
| ^^^^^
1111
| A try without catch or finally is equivalent to putting
1212
| its body in a block; no exceptions are handled.
1313

1414
longer explanation available when compiling with `-explain`
15-
-- [E000] Syntax Warning: tests/neg-custom-args/nowarn/nowarn.scala:23:28 ----------------------------------------------
16-
23 |@nowarn("verbose") def t5 = try 1 // warning with details
15+
-- [E000] Syntax Warning: tests/neg-custom-args/nowarn/nowarn.scala:26:28 ----------------------------------------------
16+
26 |@nowarn("verbose") def t5 = try 1 // warning with details
1717
| ^^^^^
1818
| A try without catch or finally is equivalent to putting
1919
| its body in a block; no exceptions are handled.
@@ -33,55 +33,65 @@ longer explanation available when compiling with `-explain`
3333
| ^^^^^^
3434
| Invalid message filter
3535
| unknown filter: wat?
36-
-- Deprecation Warning: tests/neg-custom-args/nowarn/nowarn.scala:27:10 ------------------------------------------------
37-
27 |def t6a = f // warning (refchecks, deprecation)
36+
-- [E129] Potential Issue Warning: tests/neg-custom-args/nowarn/nowarn.scala:16:12 -------------------------------------
37+
16 |def t2a = { 1; 2 } // warning (invalid nowarn doesn't silence)
38+
| ^
39+
| A pure expression does nothing in statement position; you may be omitting necessary parentheses
40+
41+
longer explanation available when compiling with `-explain`
42+
-- Warning: tests/neg-custom-args/nowarn/nowarn.scala:15:8 -------------------------------------------------------------
43+
15 |@nowarn(t1a.toString) // warning (typer, argument not a compile-time constant)
44+
| ^^^^^^^^^^^^
45+
| filter needs to be a compile-time constant string
46+
-- Deprecation Warning: tests/neg-custom-args/nowarn/nowarn.scala:30:10 ------------------------------------------------
47+
30 |def t6a = f // warning (refchecks, deprecation)
3848
| ^
3949
| method f is deprecated
40-
-- Deprecation Warning: tests/neg-custom-args/nowarn/nowarn.scala:30:30 ------------------------------------------------
41-
30 |@nowarn("msg=fish") def t6d = f // error (unused nowarn), warning (deprecation)
50+
-- Deprecation Warning: tests/neg-custom-args/nowarn/nowarn.scala:33:30 ------------------------------------------------
51+
33 |@nowarn("msg=fish") def t6d = f // error (unused nowarn), warning (deprecation)
4252
| ^
4353
| method f is deprecated
44-
-- Deprecation Warning: tests/neg-custom-args/nowarn/nowarn.scala:37:10 ------------------------------------------------
45-
37 |def t7c = f: // warning (deprecation)
54+
-- Deprecation Warning: tests/neg-custom-args/nowarn/nowarn.scala:40:10 ------------------------------------------------
55+
40 |def t7c = f: // warning (deprecation)
4656
| ^
4757
| method f is deprecated
48-
-- Unchecked Warning: tests/neg-custom-args/nowarn/nowarn.scala:43:7 ---------------------------------------------------
49-
43 | case _: List[Int] => 0 // warning (patmat, unchecked)
58+
-- Unchecked Warning: tests/neg-custom-args/nowarn/nowarn.scala:46:7 ---------------------------------------------------
59+
46 | case _: List[Int] => 0 // warning (patmat, unchecked)
5060
| ^
5161
| the type test for List[Int] cannot be checked at runtime
52-
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:21:1 ---------------------------------------------------------------
53-
21 |@nowarn("id=1") def t4d = try 1 // error and warning (unused nowarn, wrong id)
62+
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:24:1 ---------------------------------------------------------------
63+
24 |@nowarn("id=1") def t4d = try 1 // error and warning (unused nowarn, wrong id)
5464
|^^^^^^^^^^^^^^^
5565
|@nowarn annotation does not suppress any warnings
56-
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:30:1 ---------------------------------------------------------------
57-
30 |@nowarn("msg=fish") def t6d = f // error (unused nowarn), warning (deprecation)
66+
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:33:1 ---------------------------------------------------------------
67+
33 |@nowarn("msg=fish") def t6d = f // error (unused nowarn), warning (deprecation)
5868
|^^^^^^^^^^^^^^^^^^^
5969
|@nowarn annotation does not suppress any warnings
60-
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:38:3 ---------------------------------------------------------------
61-
38 | @nowarn("msg=fish") // error (unused nowarn)
70+
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:41:3 ---------------------------------------------------------------
71+
41 | @nowarn("msg=fish") // error (unused nowarn)
6272
| ^^^^^^^^^^^^^^^^^^^
6373
| @nowarn annotation does not suppress any warnings
64-
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:50:0 ---------------------------------------------------------------
65-
50 |@nowarn def t9a = { 1: @nowarn; 2 } // error (outer @nowarn is unused)
74+
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:53:0 ---------------------------------------------------------------
75+
53 |@nowarn def t9a = { 1: @nowarn; 2 } // error (outer @nowarn is unused)
6676
|^^^^^^^
6777
|@nowarn annotation does not suppress any warnings
68-
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:51:27 --------------------------------------------------------------
69-
51 |@nowarn def t9b = { 1: Int @nowarn; 2 } // error (inner @nowarn is unused, it covers the type, not the expression)
78+
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:54:27 --------------------------------------------------------------
79+
54 |@nowarn def t9b = { 1: Int @nowarn; 2 } // error (inner @nowarn is unused, it covers the type, not the expression)
7080
| ^^^^^^^
7181
| @nowarn annotation does not suppress any warnings
72-
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:56:0 ---------------------------------------------------------------
73-
56 |@nowarn @ann(f) def t10b = 0 // error (unused nowarn)
82+
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:59:0 ---------------------------------------------------------------
83+
59 |@nowarn @ann(f) def t10b = 0 // error (unused nowarn)
7484
|^^^^^^^
7585
|@nowarn annotation does not suppress any warnings
76-
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:57:8 ---------------------------------------------------------------
77-
57 |@ann(f: @nowarn) def t10c = 0 // error (unused nowarn), should be silent
86+
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:60:8 ---------------------------------------------------------------
87+
60 |@ann(f: @nowarn) def t10c = 0 // error (unused nowarn), should be silent
7888
| ^^^^^^^
7989
| @nowarn annotation does not suppress any warnings
80-
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:60:0 ---------------------------------------------------------------
81-
60 |@nowarn class I1a { // error (unused nowarn)
90+
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:63:0 ---------------------------------------------------------------
91+
63 |@nowarn class I1a { // error (unused nowarn)
8292
|^^^^^^^
8393
|@nowarn annotation does not suppress any warnings
84-
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:65:0 ---------------------------------------------------------------
85-
65 |@nowarn class I1b { // error (unused nowarn)
94+
-- Error: tests/neg-custom-args/nowarn/nowarn.scala:68:0 ---------------------------------------------------------------
95+
68 |@nowarn class I1b { // error (unused nowarn)
8696
|^^^^^^^
8797
|@nowarn annotation does not suppress any warnings

‎tests/neg-custom-args/nowarn/nowarn.scala‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ def t1a = try 1 // warning (parser)
1212
@nowarn("wat?") // warning (typer, invalid filter)
1313
def t2 = { 1; 2 } // warning (the invalid nowarn doesn't silence anything)
1414

15+
@nowarn(t1a.toString) // warning (typer, argument not a compile-time constant)
16+
def t2a = { 1; 2 } // warning (invalid nowarn doesn't silence)
17+
1518
@nowarn("id=E129") def t3a = { 1; 2 }
1619
@nowarn("name=PureExpressionInStatementPosition") def t3b = { 1; 2 }
1720

0 commit comments

Comments
 (0)