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

Add -W enabling warnings #19843

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
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/config/CompilerCommand.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,23 @@ import core.Contexts.*

abstract class CompilerCommand extends CliCommand:
type ConcreteSettings = ScalaSettings
import ScalaSettings.*

private val customMessageFlags: Set[Setting[_]] = Set(ScalaSettings.W)
final def helpMsg(using settings: ConcreteSettings)(using SettingsState, Context): String =
settings.allSettings.find(isHelping) match
case Some(s) => s.description
allSettings.find(isHelping) match
case Some(s) if !customMessageFlags.contains(s) =>
s.description
case _ =>
if (settings.help.value) usageMessage
else if (settings.Vhelp.value) vusageMessage
else if (settings.Whelp.value) wusageMessage
else if (settings.Xhelp.value) xusageMessage
else if (settings.Yhelp.value) yusageMessage
else if (settings.showPlugins.value) ctx.base.pluginDescriptions
else if (settings.XshowPhases.value) phasesMessage
if (help.value) usageMessage
else if (Vhelp.value) vusageMessage
else if (W.value.contains("help")) wusageMessage
else if (Xhelp.value) xusageMessage
else if (Yhelp.value) yusageMessage
else if (showPlugins.value) ctx.base.pluginDescriptions
else if (XshowPhases.value) phasesMessage
else ""

final def isHelpFlag(using settings: ConcreteSettings)(using SettingsState): Boolean =
import settings.*
val flags = Set(help, Vhelp, Whelp, Xhelp, Yhelp, showPlugins, XshowPhases)
val flags = Set(help, Vhelp, Xhelp, Yhelp, showPlugins, XshowPhases)
flags.exists(_.value) || allSettings.exists(isHelping)
106 changes: 70 additions & 36 deletions compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ package config

import scala.language.unsafeNulls
import dotty.tools.dotc.config.PathResolver.Defaults
import dotty.tools.dotc.config.Settings.{Setting, SettingGroup, SettingCategory}
import dotty.tools.dotc.config.Settings.{Setting, SettingGroup, SettingCategory, SettingsState}
import dotty.tools.dotc.config.SourceVersion
import dotty.tools.dotc.core.Contexts.*
import dotty.tools.dotc.rewrites.Rewrites
import dotty.tools.io.{AbstractFile, Directory, JDK9Reflectors, PlainDirectory, NoAbstractFile}
import Setting.ChoiceWithHelp
import ScalaSettingCategories.*

import scala.util.chaining.*
Expand Down Expand Up @@ -156,46 +155,80 @@ private sealed trait VerboseSettings:
private sealed trait WarningSettings:
self: SettingGroup =>

val Whelp: Setting[Boolean] = BooleanSetting(WarningSetting, "W", "Print a synopsis of warning options.")
val W: Setting[List[String]] = MultiChoiceSetting(
WarningSetting,
name = "W",
helpArg = "warning",
descr = "Enable sets of warnings or print a synopsis of warning options.",
choices = List("help", "default", "all"),
choiceHelp = Some(Map(
"help" -> "Print a synopsis of warning options",
"default" -> "Enable default warnings",
"all" -> "Enable all warnings"
)),
default = Nil,
helpOnMissing = true
)

enum WarningGroup(val enabledBy: Set[String]):
case Default extends WarningGroup(Set("default", "all"))
case All extends WarningGroup(Set("all"))

def overrideValueWithW[T](group: WarningGroup, overrideWhen: T, overrideTo: T)(ss: SettingsState, v: T): T =
if W.valueIn(ss).map(_.toString).exists(group.enabledBy.contains) && v == overrideWhen then overrideTo
else v

def overrideWithW(group: WarningGroup)(ss: SettingsState, v: Boolean): Boolean =
overrideValueWithW(group, false, true)(ss, v)

val XfatalWarnings: Setting[Boolean] = BooleanSetting(WarningSetting, "Werror", "Fail the compilation if there are any warnings.", aliases = List("-Xfatal-warnings"))
val WvalueDiscard: Setting[Boolean] = BooleanSetting(WarningSetting, "Wvalue-discard", "Warn when non-Unit expression results are unused.")
.mapValue(overrideWithW(WarningGroup.Default))
val WNonUnitStatement = BooleanSetting(WarningSetting, "Wnonunit-statement", "Warn when block statements are non-Unit expressions.")
.mapValue(overrideWithW(WarningGroup.Default))
val WenumCommentDiscard = BooleanSetting(WarningSetting, "Wenum-comment-discard", "Warn when a comment ambiguously assigned to multiple enum cases is discarded.")
val WimplausiblePatterns = BooleanSetting(WarningSetting, "Wimplausible-patterns", "Warn if comparison with a pattern value looks like it might always fail.")
.mapValue(overrideWithW(WarningGroup.Default))
val WunstableInlineAccessors = BooleanSetting(WarningSetting, "WunstableInlineAccessors", "Warn an inline methods has references to non-stable binary APIs.")
val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
.mapValue(overrideWithW(WarningGroup.Default))
val Wunused: Setting[List[String]] = MultiChoiceSetting(
WarningSetting,
name = "Wunused",
helpArg = "warning",
descr = "Enable or disable specific `unused` warnings",
choices = List(
ChoiceWithHelp("nowarn", ""),
ChoiceWithHelp("all",""),
ChoiceWithHelp(
name = "imports",
description = "Warn if an import selector is not referenced.\n" +
"nowarn",
"all",
"imports",
"privates",
"locals",
"explicits",
"implicits",
"params",
"linted",
"strict-no-implicit-warn",
"unsafe-warn-patvars"
),
choiceHelp = Some(Map(
"nowarn" -> "",
"all" -> "",
"imports" -> ("Warn if an import selector is not referenced.\n" +
"NOTE : overrided by -Wunused:strict-no-implicit-warn"),
ChoiceWithHelp("privates","Warn if a private member is unused"),
ChoiceWithHelp("locals","Warn if a local definition is unused"),
ChoiceWithHelp("explicits","Warn if an explicit parameter is unused"),
ChoiceWithHelp("implicits","Warn if an implicit parameter is unused"),
ChoiceWithHelp("params","Enable -Wunused:explicits,implicits"),
ChoiceWithHelp("linted","Enable -Wunused:imports,privates,locals,implicits"),
ChoiceWithHelp(
name = "strict-no-implicit-warn",
description = "Same as -Wunused:import, only for imports of explicit named members.\n" +
"NOTE : This overrides -Wunused:imports and NOT set by -Wunused:all"
),
// ChoiceWithHelp("patvars","Warn if a variable bound in a pattern is unused"),
ChoiceWithHelp(
name = "unsafe-warn-patvars",
description = "(UNSAFE) Warn if a variable bound in a pattern is unused.\n" +
"This warning can generate false positive, as warning cannot be\n" +
"suppressed yet."
)
),
"privates" -> "Warn if a private member is unused",
"locals" -> "Warn if a local definition is unused",
"explicits" -> "Warn if an explicit parameter is unused",
"implicits" -> "Warn if an implicit parameter is unused",
"params" -> "Enable -Wunused:explicits,implicits",
"linted" -> "Enable -Wunused:imports,privates,locals,implicits",
"strict-no-implicit-warn" -> ("Same as -Wunused:import, only for imports of explicit named members.\n" +
"NOTE : This overrides -Wunused:imports and NOT set by -Wunused:all"),
"unsafe-warn-patvars" -> ("(UNSAFE) Warn if a variable bound in a pattern is unused.\n" +
"This warning can generate false positive, as warning cannot be\n" +
"suppressed yet.")
)),
default = Nil
)
).mapValue(overrideValueWithW(WarningGroup.Default, Nil, List("linted")))

object WunusedHas:
def isChoiceSet(s: String)(using Context) = Wunused.value.pipe(us => us.contains(s))
def allOr(s: String)(using Context) = Wunused.value.pipe(us => us.contains("all") || us.contains(s))
Expand Down Expand Up @@ -271,18 +304,19 @@ private sealed trait WarningSettings:
|to prevent the shell from expanding patterns.""".stripMargin,
)

val Wshadow: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
val Wshadow: Setting[List[String]] = MultiChoiceSetting(
WarningSetting,
name = "Wshadow",
helpArg = "warning",
descr = "Enable or disable specific `shadow` warnings",
choices = List(
ChoiceWithHelp("all", ""),
ChoiceWithHelp("private-shadow", "Warn if a private field or class parameter shadows a superclass field"),
ChoiceWithHelp("type-parameter-shadow", "Warn when a type parameter shadows a type already in the scope"),
),
choices = List("all", "private-shadow", "type-parameter-shadow"),
choiceHelp = Some(Map(
"all" -> "",
"private-shadow" -> "Warn if a private field or class parameter shadows a superclass field",
"type-parameter-shadow" -> "Warn when a type parameter shadows a type already in the scope"
)),
default = Nil
)
).mapValue(overrideValueWithW(WarningGroup.Default, Nil, List("all")))

object WshadowHas:
def allOr(s: String)(using Context) =
Expand Down
51 changes: 26 additions & 25 deletions compiler/src/dotty/tools/dotc/config/Settings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import collection.mutable.ArrayBuffer
import collection.mutable
import reflect.ClassTag
import scala.util.{Success, Failure}
import dotty.tools.dotc.config.Settings.Setting.ChoiceWithHelp

object Settings:

Expand Down Expand Up @@ -82,19 +81,25 @@ object Settings:
propertyClass: Option[Class[?]] = None,
deprecationMsg: Option[String] = None,
// kept only for -Ykind-projector option compatibility
legacyArgs: Boolean = false)(private[Settings] val idx: Int) {

legacyArgs: Boolean = false,
mapValue: (SettingsState, T) => T = (a, b: T) => b,
helpOnMissing: Boolean = false,
choiceHelp: Option[Map[Any, String]] = None)(private[Settings] val idx: Int) {

private val ct = summon[ClassTag[T]]
validateSettingString(prefix.getOrElse(name))
aliases.foreach(validateSettingString)
assert(name.startsWith(s"-${category.prefixLetter}"), s"Setting $name does not start with category -$category")
assert(legacyArgs || !choices.exists(_.contains("")), s"Empty string is not supported as a choice for setting $name")
// Without the following assertion, it would be easy to mistakenly try to pass a file to a setting that ignores invalid args.
// Example: -opt Main.scala would be interpreted as -opt:Main.scala, and the source file would be ignored.
assert(!(summon[ClassTag[T]] == ListTag && ignoreInvalidArgs), s"Ignoring invalid args is not supported for multivalue settings: $name")
assert(!(ct == ListTag && ignoreInvalidArgs), s"Ignoring invalid args is not supported for multivalue settings: $name")
assert(!helpOnMissing || ct == StringTag || ct == ListTag, s"helpOnMissing is only supported for String or List settings: $name")
assert(choices.isDefined || choiceHelp.isEmpty, s"choiceHelp is only supported for settings with choices: $name")

val allFullNames: List[String] = s"$name" :: s"-$name" :: aliases

def valueIn(state: SettingsState): T = state.value(idx).asInstanceOf[T]
def valueIn(state: SettingsState): T = mapValue(state, state.value(idx).asInstanceOf[T])

def updateIn(state: SettingsState, x: Any): SettingsState = x match
case _: T => state.update(idx, x)
Expand All @@ -105,15 +110,22 @@ object Settings:
def isMultivalue: Boolean = summon[ClassTag[T]] == ListTag

def acceptsNoArg: Boolean = summon[ClassTag[T]] == BooleanTag || summon[ClassTag[T]] == OptionTag || choices.exists(_.contains(""))

s"\n- $name${if description.isEmpty() then "" else s" :\n\t${description.replace("\n","\n\t")}"}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to be an unused string value

def legalChoices: String =
def toString[T](choice: T) =
choiceHelp match
case Some(choiceMap) => s"\n- $choice${if choiceMap.isDefinedAt(choice) then s":\n\t${choiceMap(choice).replace("\n","\n\t")}" else ""}"
case None => choice.toString

choices match {
case Some(xs) if xs.isEmpty => ""
case Some(r: Range) => s"${r.head}..${r.last}"
case Some(xs) => xs.mkString(", ")
case Some(xs) => xs.map(toString).mkString(", ")
case None => ""
}

def mapValue(f: (SettingsState, T) => T): Setting[T] = copy(mapValue = f)(idx)

def tryToSet(state: ArgsSummary): ArgsSummary = {
val ArgsSummary(sstate, arg :: args, errors, warnings) = state: @unchecked
def update(value: Any, args: List[String]): ArgsSummary =
Expand All @@ -138,7 +150,11 @@ object Settings:

def missingArg =
val msg = s"missing argument for option $name"
if ignoreInvalidArgs then warn(msg + ", the tag was ignored", args) else fail(msg, args)
if helpOnMissing then
val helpValue = if ct == StringTag then "help" else List("help")
update(helpValue, args)
else if ignoreInvalidArgs then warn(msg + ", the tag was ignored", args)
else fail(msg, args)

def invalidChoices(invalid: List[String]) =
val msg = s"invalid choice(s) for $name: ${invalid.mkString(",")}"
Expand Down Expand Up @@ -245,18 +261,6 @@ object Settings:
def value(using Context): T = setting.valueIn(ctx.settingsState)
def update(x: T)(using Context): SettingsState = setting.updateIn(ctx.settingsState, x)
def isDefault(using Context): Boolean = setting.isDefaultIn(ctx.settingsState)

/**
* A choice with help description.
*
* NOTE : `equals` and `toString` have special behaviors
*/
case class ChoiceWithHelp[T](name: T, description: String):
override def equals(x: Any): Boolean = x match
case s:String => s == name.toString()
case _ => false
override def toString(): String =
s"\n- $name${if description.isEmpty() then "" else s" :\n\t${description.replace("\n","\n\t")}"}"
end Setting

class SettingGroup {
Expand Down Expand Up @@ -342,11 +346,8 @@ object Settings:
def ChoiceSetting(category: SettingCategory, name: String, helpArg: String, descr: String, choices: List[String], default: String, aliases: List[String] = Nil, legacyArgs: Boolean = false): Setting[String] =
publish(Setting(category, prependName(name), descr, default, helpArg, Some(choices), aliases = aliases, legacyArgs = legacyArgs))

def MultiChoiceSetting(category: SettingCategory, name: String, helpArg: String, descr: String, choices: List[String], default: List[String], aliases: List[String] = Nil): Setting[List[String]] =
publish(Setting(category, prependName(name), descr, default, helpArg, Some(choices), aliases = aliases))

def MultiChoiceHelpSetting(category: SettingCategory, name: String, helpArg: String, descr: String, choices: List[ChoiceWithHelp[String]], default: List[ChoiceWithHelp[String]], aliases: List[String] = Nil): Setting[List[ChoiceWithHelp[String]]] =
publish(Setting(category, prependName(name), descr, default, helpArg, Some(choices), aliases = aliases))
def MultiChoiceSetting(category: SettingCategory, name: String, helpArg: String, descr: String, choices: List[String], default: List[String], aliases: List[String] = Nil, helpOnMissing: Boolean = false, choiceHelp: Option[Map[Any, String]] = None): Setting[List[String]] =
publish(Setting(category, prependName(name), descr, default, helpArg, Some(choices), aliases = aliases, helpOnMissing = helpOnMissing, choiceHelp = choiceHelp))

def IntSetting(category: SettingCategory, name: String, descr: String, default: Int, aliases: List[String] = Nil): Setting[Int] =
publish(Setting(category, prependName(name), descr, default, aliases = aliases))
Expand Down
14 changes: 14 additions & 0 deletions compiler/test/dotty/tools/dotc/config/ScalaSettingsTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,20 @@ class ScalaSettingsTests:
val sut = reporting.WConf.fromSettings(conf).getOrElse(???)
assertEquals(Action.Silent, sut.action(depr))

@Test def `Treat parameterless -W as help`: Unit =
val sets = config.ScalaSettings
val args = List("-W")
val sumy = ArgsSummary(sets.defaultState, args, errors = Nil, warnings = Nil)
val proc = sets.processArguments(sumy, processAll = true, skipped = Nil)
assertTrue(sets.W.valueIn(sumy.sstate).contains("help"))

@Test def `Enable default warns with -W default`: Unit =
val sets = config.ScalaSettings
val args = List("-W", "default")
val sumy = ArgsSummary(sets.defaultState, args, errors = Nil, warnings = Nil)
val proc = sets.processArguments(sumy, processAll = true, skipped = Nil)
assertTrue(sets.Wunused.valueIn(sumy.sstate).contains("linted"))
assertTrue(sets.WNonUnitStatement.valueIn(sumy.sstate))
assertTrue(sets.WvalueDiscard.valueIn(sumy.sstate))

end ScalaSettingsTests
56 changes: 56 additions & 0 deletions tests/warn/i18559.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
-- [E176] Potential Issue Warning: tests/warn/i18559.scala:63:4 --------------------------------------------------------
63 | improved // warn
| ^^^^^^^^
| unused value of type (improved : => scala.concurrent.Future[Int])
-- [E175] Potential Issue Warning: tests/warn/i18559.scala:51:35 -------------------------------------------------------
51 | firstThing().map(_ => secondThing()) // warn
| ^^^^^^^^^^^^^
| discarded non-Unit value of type Either[Failed, Unit]
-- [E175] Potential Issue Warning: tests/warn/i18559.scala:54:35 -------------------------------------------------------
54 | firstThing().map(_ => secondThing()) // warn
| ^^^^^^^^^^^^^
| discarded non-Unit value of type Either[Failed, Unit]
-- Warning: tests/warn/i18559.scala:4:6 --------------------------------------------------------------------------------
4 | var e3 = 2 // warn
| ^^
| unused local definition
-- Warning: tests/warn/i18559.scala:8:28 -------------------------------------------------------------------------------
8 | import collection.mutable.Set // warn
| ^^^
| unused import
-- Warning: tests/warn/i18559.scala:9:33 -------------------------------------------------------------------------------
9 | import collection.mutable.{Map => MutMap} // warn
| ^^^^^^^^^^^^^
| unused import
-- Warning: tests/warn/i18559.scala:10:28 ------------------------------------------------------------------------------
10 | import collection.mutable._ // warn
| ^
| unused import
-- Warning: tests/warn/i18559.scala:13:28 ------------------------------------------------------------------------------
13 | import collection.mutable._ // warn
| ^
| unused import
-- [E030] Match case Unreachable Warning: tests/warn/i18559.scala:31:10 ------------------------------------------------
31 | case Sum(a@S(_),Z) => Z // warn
| ^^^^^^^^^^^^^
| Unreachable case
-- [E030] Match case Unreachable Warning: tests/warn/i18559.scala:32:10 ------------------------------------------------
32 | case Sum(a@S(_),Z) => a // warn unreachable
| ^^^^^^^^^^^^^
| Unreachable case
-- [E030] Match case Unreachable Warning: tests/warn/i18559.scala:33:10 ------------------------------------------------
33 | case Sum(a@S(b@S(_)), Z) => a // warn
| ^^^^^^^^^^^^^^^^^^^
| Unreachable case
-- [E030] Match case Unreachable Warning: tests/warn/i18559.scala:34:10 ------------------------------------------------
34 | case Sum(a@S(b@S(_)), Z) => a // warn
| ^^^^^^^^^^^^^^^^^^^
| Unreachable case
-- [E030] Match case Unreachable Warning: tests/warn/i18559.scala:35:10 ------------------------------------------------
35 | case Sum(a@S(b@(S(_))), Z) => Sum(a,b) // warn unreachable
| ^^^^^^^^^^^^^^^^^^^^^
| Unreachable case
-- [E121] Pattern Match Warning: tests/warn/i18559.scala:37:7 ----------------------------------------------------------
37 | case _ => Z // warn unreachable
| ^
| Unreachable case except for null (if this is intentional, consider writing case null => instead).
Loading
Loading