Skip to content

Commit f841684

Browse files
som-snyttsom-snytt
committed
Option warnings are conditional
Add test for int setting Clarify option sanity checks Warn if int setting is multiply set Warn on output multiply set Oh scalafix where art thou Tweak per review
1 parent 6062192 commit f841684

File tree

9 files changed

+204
-111
lines changed

9 files changed

+204
-111
lines changed

‎compiler/src/dotty/tools/dotc/config/CliCommand.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ trait CliCommand:
123123
*/
124124
def checkUsage(summary: ArgsSummary, sourcesRequired: Boolean)(using settings: ConcreteSettings)(using SettingsState, Context): Option[List[String]] =
125125
// Print all warnings encountered during arguments parsing
126-
summary.warnings.foreach(report.warning(_))
126+
summary.warnings.foreach(report.configurationWarning(_))
127127

128128
if summary.errors.nonEmpty then
129129
summary.errors foreach (report.error(_))

‎compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ trait CommonScalaSettings:
111111

112112
val javaOutputVersion: Setting[String] = ChoiceSetting("-java-output-version", "version", "Compile code with classes specific to the given version of the Java platform available on the classpath and emit bytecode for this version. Corresponds to -release flag in javac.", ScalaSettings.supportedReleaseVersions, "", aliases = List("-release", "--release"))
113113

114+
val configuration: Setting[Boolean] = BooleanSetting("-configuration", "Warn about conflicting and redundant tool options", aliases = List("--configuration"))
114115
val deprecation: Setting[Boolean] = BooleanSetting("-deprecation", "Emit warning and location for usages of deprecated APIs.", aliases = List("--deprecation"))
115116
val feature: Setting[Boolean] = BooleanSetting("-feature", "Emit warning and location for usages of features that should be imported explicitly.", aliases = List("--feature"))
116117
val explain: Setting[Boolean] = BooleanSetting("-explain", "Explain errors in more detail.", aliases = List("--explain"))

‎compiler/src/dotty/tools/dotc/config/Settings.scala

Lines changed: 76 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,12 @@ object Settings:
3939
this
4040
end SettingsState
4141

42-
case class ArgsSummary(
43-
sstate: SettingsState,
44-
arguments: List[String],
45-
errors: List[String],
46-
warnings: List[String]) {
42+
case class ArgsSummary(sstate: SettingsState, arguments: List[String], errors: List[String], warnings: List[String])
4743

48-
def fail(msg: String): Settings.ArgsSummary =
49-
ArgsSummary(sstate, arguments.tail, errors :+ msg, warnings)
50-
51-
def warn(msg: String): Settings.ArgsSummary =
52-
ArgsSummary(sstate, arguments.tail, errors, warnings :+ msg)
53-
}
44+
extension (summary: ArgsSummary)
45+
def fail(msg: String): ArgsSummary = summary.copy(errors = summary.errors :+ msg)
46+
def warn(msg: String): ArgsSummary = summary.copy(warnings = summary.warnings :+ msg)
47+
def skip(): ArgsSummary = summary.copy(arguments = summary.arguments.drop(1))
5448

5549
case class Setting[T: ClassTag] private[Settings] (
5650
name: String,
@@ -73,7 +67,7 @@ object Settings:
7367

7468
def isDefaultIn(state: SettingsState): Boolean = valueIn(state) == default
7569

76-
def isMultivalue: Boolean = implicitly[ClassTag[T]] == ListTag
70+
def isMultivalue: Boolean = summon[ClassTag[T]] == ListTag
7771

7872
def legalChoices: String =
7973
choices match {
@@ -86,91 +80,96 @@ object Settings:
8680
def tryToSet(state: ArgsSummary): ArgsSummary = {
8781
val ArgsSummary(sstate, arg :: args, errors, warnings) = state: @unchecked
8882
def update(value: Any, args: List[String]): ArgsSummary =
89-
var dangers = warnings
9083
val value1 =
91-
if changed && isMultivalue then
92-
val value0 = value.asInstanceOf[List[String]]
93-
val current = valueIn(sstate).asInstanceOf[List[String]]
94-
value0.filter(current.contains).foreach(s => dangers :+= s"Setting $name set to $s redundantly")
95-
current ++ value0
96-
else
97-
if changed then dangers :+= s"Flag $name set repeatedly"
98-
value
84+
if isMultivalue then valueIn(sstate).asInstanceOf[List[String]] ++ value.asInstanceOf[List[String]]
85+
else value
9986
changed = true
100-
ArgsSummary(updateIn(sstate, value1), args, errors, dangers)
101-
end update
87+
ArgsSummary(updateIn(sstate, value1), args, errors, warnings)
88+
89+
def fail(msg: String, args: List[String]) = ArgsSummary(sstate, args, errors :+ msg, warnings)
10290

103-
def fail(msg: String, args: List[String]) =
104-
ArgsSummary(sstate, args, errors :+ msg, warnings)
91+
def missingArg = fail(s"missing argument for option $name", args)
10592

106-
def missingArg =
107-
fail(s"missing argument for option $name", args)
93+
def setBoolean(argValue: String) =
94+
def checkAndSet(v: Boolean) =
95+
val dubious = changed && v != valueIn(sstate).asInstanceOf[Boolean]
96+
val updated = update(v, args)
97+
if dubious then updated.warn(s"Boolean flag $name flipped") else updated
98+
if argValue.equalsIgnoreCase("true") || argValue == "" then checkAndSet(true)
99+
else if argValue.equalsIgnoreCase("false") then checkAndSet(false)
100+
else fail(s"invalid boolean value $argValue", args)
108101

109102
def setString(argValue: String, args: List[String]) =
110103
choices match
111-
case Some(xs) if !xs.contains(argValue) =>
112-
fail(s"$argValue is not a valid choice for $name", args)
113-
case _ =>
114-
update(argValue, args)
104+
case Some(xs) if !xs.contains(argValue) => fail(s"$argValue is not a valid choice for $name", args)
105+
case _ if changed && argValue != valueIn(sstate).asInstanceOf[String] => update(argValue, args).warn(s"Option $name was updated")
106+
case _ => update(argValue, args)
115107

116108
def setInt(argValue: String, args: List[String]) =
117109
try
118110
val x = argValue.toInt
119111
choices match
120-
case Some(r: Range) if x < r.head || r.last < x =>
121-
fail(s"$argValue is out of legal range ${r.head}..${r.last} for $name", args)
122-
case Some(xs) if !xs.contains(x) =>
123-
fail(s"$argValue is not a valid choice for $name", args)
124-
case _ =>
125-
update(x, args)
126-
catch case _: NumberFormatException =>
127-
fail(s"$argValue is not an integer argument for $name", args)
128-
129-
def doSet(argRest: String) = ((implicitly[ClassTag[T]], args): @unchecked) match {
130-
case (BooleanTag, _) =>
131-
update(true, args)
132-
case (OptionTag, _) =>
133-
update(Some(propertyClass.get.getConstructor().newInstance()), args)
134-
case (ListTag, _) =>
135-
if (argRest.isEmpty) missingArg
136-
else
137-
val strings = argRest.split(",").toList
138-
choices match
139-
case Some(valid) => strings.filterNot(valid.contains) match
140-
case Nil => update(strings, args)
112+
case Some(r: Range) if x < r.head || r.last < x => fail(s"$argValue is out of legal range ${r.head}..${r.last} for $name", args)
113+
case Some(xs) if !xs.contains(x) => fail(s"$argValue is not a valid choice for $name", args)
114+
case _ =>
115+
val dubious = changed && x != valueIn(sstate).asInstanceOf[Int]
116+
val updated = update(x, args)
117+
if dubious then updated.warn(s"Option $name was updated") else updated
118+
catch case _: NumberFormatException => fail(s"$argValue is not an integer argument for $name", args)
119+
120+
def doSet(argRest: String): ArgsSummary = (summon[ClassTag[T]]: @unchecked) match {
121+
case BooleanTag => setBoolean(argRest)
122+
case OptionTag => update(Some(propertyClass.get.getConstructor().newInstance()), args)
123+
case ListTag if argRest.isEmpty => missingArg
124+
case ListTag =>
125+
val split = argRest.split(",").toList
126+
def checkRepeated =
127+
var dangers: List[String] = Nil
128+
if changed then
129+
val current = valueIn(sstate).asInstanceOf[List[String]]
130+
split.filter(current.contains).foreach(s => dangers :+= s"Setting $name set to $s redundantly")
131+
dangers.foldLeft(update(split, args))((sum, w) => sum.warn(w))
132+
choices match
133+
case Some(valid) =>
134+
split.filterNot(valid.contains) match
135+
case Nil => checkRepeated
141136
case invalid => fail(s"invalid choice(s) for $name: ${invalid.mkString(",")}", args)
142-
case _ => update(strings, args)
143-
case (StringTag, _) if argRest.nonEmpty || choices.exists(_.contains("")) =>
144-
setString(argRest, args)
145-
case (StringTag, arg2 :: args2) =>
146-
if (arg2 startsWith "-") missingArg
147-
else setString(arg2, args2)
148-
case (OutputTag, arg :: args) =>
149-
val path = Directory(arg)
150-
val isJar = path.extension == "jar"
151-
if (!isJar && !path.isDirectory)
152-
fail(s"'$arg' does not exist or is not a directory or .jar file", args)
153-
else {
154-
val output = if (isJar) JarArchive.create(path) else new PlainDirectory(path)
155-
update(output, args)
156-
}
157-
case (IntTag, args) if argRest.nonEmpty =>
158-
setInt(argRest, args)
159-
case (IntTag, arg2 :: args2) =>
160-
setInt(arg2, args2)
161-
case (VersionTag, _) =>
137+
case _ => checkRepeated
138+
case StringTag if argRest.nonEmpty || choices.exists(_.contains("")) => setString(argRest, args)
139+
case StringTag =>
140+
args match
141+
case arg2 :: _ if arg2.startsWith("-") => missingArg
142+
case arg2 :: args2 => setString(arg2, args2)
143+
case _ => missingArg
144+
case OutputTag =>
145+
args match
146+
case arg :: rest =>
147+
val path = Directory(arg)
148+
val isJar = path.extension == "jar"
149+
if !isJar && !path.isDirectory then fail(s"'$arg' does not exist or is not a directory or .jar file", rest)
150+
else
151+
val output = if isJar then JarArchive.create(path) else PlainDirectory(path)
152+
if changed && output != valueIn(sstate).asInstanceOf[AbstractFile] then update(output, rest).warn(s"Option $name was updated")
153+
else update(output, rest)
154+
case _ => missingArg
155+
case IntTag if argRest.nonEmpty => setInt(argRest, args)
156+
case IntTag =>
157+
args match
158+
case arg :: rest => setInt(arg, rest)
159+
case _ => missingArg
160+
case VersionTag =>
162161
ScalaVersion.parse(argRest) match {
163162
case Success(v) => update(v, args)
164-
case Failure(ex) => fail(ex.getMessage, args)
163+
case Failure(x) => fail(x.getMessage, args)
165164
}
166-
case (_, Nil) =>
167-
missingArg
165+
case _ => missingArg
168166
}
169167

170168
def matches(argName: String) = (name :: aliases).exists(_ == argName)
171169

170+
// begin
172171
if (prefix != "" && arg.startsWith(prefix))
173-
doSet(arg drop prefix.length)
172+
doSet(arg.drop(prefix.length))
174173
else if (prefix == "" && matches(arg.takeWhile(_ != ':')))
175174
doSet(arg.dropWhile(_ != ':').drop(1))
176175
else
@@ -237,7 +236,7 @@ object Settings:
237236
if state1 ne state then state1
238237
else loop(settings1)
239238
case Nil =>
240-
state.warn(s"bad option '$x' was ignored")
239+
state.warn(s"bad option '$x' was ignored").skip()
241240
processArguments(loop(allSettings.toList), processAll, skipped)
242241
case arg :: args =>
243242
if processAll then processArguments(stateWithArgs(args), processAll, skipped :+ arg)

‎compiler/src/dotty/tools/dotc/report.scala

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ object report:
2323
private def issueWarning(warning: Warning)(using Context): Unit =
2424
ctx.reporter.report(warning)
2525

26+
def configurationWarning(msg: Message, pos: SrcPos = NoSourcePosition)(using Context): Unit =
27+
issueWarning(ConfigurationWarning(msg, pos.sourcePos))
28+
2629
def deprecationWarning(msg: Message, pos: SrcPos = NoSourcePosition)(using Context): Unit =
2730
issueWarning(new DeprecationWarning(msg, pos.sourcePos))
2831

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ object Diagnostic:
7777
def enablingOption(using Context): Setting[Boolean] = ctx.settings.deprecation
7878
}
7979

80+
class ConfigurationWarning(msg: Message, pos: SourcePosition) extends ConditionalWarning(msg, pos):
81+
def enablingOption(using Context): Setting[Boolean] = ctx.settings.configuration
82+
8083
class MigrationWarning(
8184
msg: Message,
8285
pos: SourcePosition

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -205,14 +205,13 @@ abstract class Reporter extends interfaces.ReporterResult {
205205
incompleteHandler(dia, ctx)
206206

207207
/** Summary of warnings and errors */
208-
def summary: String = {
209-
val b = new mutable.ListBuffer[String]
208+
def summary: String =
209+
val b = mutable.ListBuffer.empty[String]
210210
if (warningCount > 0)
211211
b += countString(warningCount, "warning") + " found"
212212
if (errorCount > 0)
213213
b += countString(errorCount, "error") + " found"
214214
b.mkString("\n")
215-
}
216215

217216
def summarizeUnreportedWarnings()(using Context): Unit =
218217
for (settingName, count) <- unreportedWarnings do
@@ -221,10 +220,10 @@ abstract class Reporter extends interfaces.ReporterResult {
221220
report(Warning(msg, NoSourcePosition))
222221

223222
/** Print the summary of warnings and errors */
224-
def printSummary()(using Context): Unit = {
225-
val s = summary
226-
if (s != "") report(new Info(s, NoSourcePosition))
227-
}
223+
def printSummary()(using Context): Unit =
224+
summary match
225+
case "" =>
226+
case s => report(Info(s, NoSourcePosition))
228227

229228
/** Returns a string meaning "n elements". */
230229
protected def countString(n: Int, elements: String): String = n match {

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import scala.util.matching.Regex
1414
enum MessageFilter:
1515
def matches(message: Diagnostic): Boolean = this match
1616
case Any => true
17+
case Configuration => message.isInstanceOf[Diagnostic.ConfigurationWarning]
1718
case Deprecated => message.isInstanceOf[Diagnostic.DeprecationWarning]
1819
case Feature => message.isInstanceOf[Diagnostic.FeatureWarning]
1920
case Unchecked => message.isInstanceOf[Diagnostic.UncheckedWarning]
@@ -23,7 +24,7 @@ enum MessageFilter:
2324
case MessageID(errorId) => message.msg.errorId == errorId
2425
case None => false
2526

26-
case Any, Deprecated, Feature, Unchecked, None
27+
case Any, Configuration, Deprecated, Feature, Unchecked, None
2728
case MessagePattern(pattern: Regex)
2829
case MessageID(errorId: ErrorMessageID)
2930

@@ -80,10 +81,11 @@ object WConf:
8081
catch case _: IllegalArgumentException => Left(s"unknown error message name: $conf")
8182

8283
case "cat" => conf match
83-
case "deprecation" => Right(Deprecated)
84-
case "feature" => Right(Feature)
85-
case "unchecked" => Right(Unchecked)
86-
case _ => Left(s"unknown category: $conf")
84+
case "configuration" => Right(Configuration)
85+
case "deprecation" => Right(Deprecated)
86+
case "feature" => Right(Feature)
87+
case "unchecked" => Right(Unchecked)
88+
case _ => Left(s"unknown category: $conf")
8789
case _ => Left(s"unknown filter: $filter")
8890
case _ => Left(s"unknown filter: $s")
8991

0 commit comments

Comments
 (0)