Skip to content

Commit

Permalink
Add the -Wall option that enables all warnings (Plan B) (#20577)
Browse files Browse the repository at this point in the history
alternative to #20545
  • Loading branch information
Linyxus authored Aug 22, 2024
2 parents a2c53a1 + af0412c commit 8b911ef
Show file tree
Hide file tree
Showing 14 changed files with 124 additions and 35 deletions.
66 changes: 40 additions & 26 deletions compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -158,49 +158,53 @@ private sealed trait WarningSettings:

val Whelp: Setting[Boolean] = BooleanSetting(WarningSetting, "W", "Print a synopsis of warning options.")
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.")
val WNonUnitStatement = BooleanSetting(WarningSetting, "Wnonunit-statement", "Warn when block statements are non-Unit expressions.")
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.")
val WunstableInlineAccessors = BooleanSetting(WarningSetting, "WunstableInlineAccessors", "Warn an inline methods has references to non-stable binary APIs.")
val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
val Wall: Setting[Boolean] = BooleanSetting(WarningSetting, "Wall", "Enable all warning settings.")
private val WvalueDiscard: Setting[Boolean] = BooleanSetting(WarningSetting, "Wvalue-discard", "Warn when non-Unit expression results are unused.")
private val WNonUnitStatement = BooleanSetting(WarningSetting, "Wnonunit-statement", "Warn when block statements are non-Unit expressions.")
private val WenumCommentDiscard = BooleanSetting(WarningSetting, "Wenum-comment-discard", "Warn when a comment ambiguously assigned to multiple enum cases is discarded.")
private val WimplausiblePatterns = BooleanSetting(WarningSetting, "Wimplausible-patterns", "Warn if comparison with a pattern value looks like it might always fail.")
private val WunstableInlineAccessors = BooleanSetting(WarningSetting, "WunstableInlineAccessors", "Warn an inline methods has references to non-stable binary APIs.")
private val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
WarningSetting,
name = "Wunused",
helpArg = "warning",
descr = "Enable or disable specific `unused` warnings",
choices = List(
ChoiceWithHelp("nowarn", ""),
ChoiceWithHelp("all",""),
ChoiceWithHelp("all", ""),
ChoiceWithHelp(
name = "imports",
description = "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."
)
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."
)
),
default = Nil
)
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))
def allOr(s: String)(using Context) = Wall.value || Wunused.value.pipe(us => us.contains("all") || us.contains(s))
def nowarn(using Context) = allOr("nowarn")

// Is any choice set for -Wunused?
def any(using Context): Boolean = Wall.value || Wunused.value.nonEmpty

// overrided by strict-no-implicit-warn
def imports(using Context) =
(allOr("imports") || allOr("linted")) && !(strictNoImplicitWarn)
Expand Down Expand Up @@ -298,6 +302,16 @@ private sealed trait WarningSettings:

val WcheckInit: Setting[Boolean] = BooleanSetting(WarningSetting, "Wsafe-init", "Ensure safe initialization of objects.")

object Whas:
def allOr(s: Setting[Boolean])(using Context): Boolean =
Wall.value || s.value
def valueDiscard(using Context): Boolean = allOr(WvalueDiscard)
def nonUnitStatement(using Context): Boolean = allOr(WNonUnitStatement)
def enumCommentDiscard(using Context): Boolean = allOr(WenumCommentDiscard)
def implausiblePatterns(using Context): Boolean = allOr(WimplausiblePatterns)
def unstableInlineAccessors(using Context): Boolean = allOr(WunstableInlineAccessors)
def checkInit(using Context): Boolean = allOr(WcheckInit)

/** -X "Extended" or "Advanced" settings */
private sealed trait XSettings:
self: SettingGroup =>
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/Symbols.scala
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ object Symbols extends SymUtils {
ctx.settings.YretainTrees.value ||
denot.owner.isTerm || // no risk of leaking memory after a run for these
denot.isOneOf(InlineOrProxy) || // need to keep inline info
ctx.settings.WcheckInit.value || // initialization check
ctx.settings.Whas.checkInit || // initialization check
ctx.settings.YcheckInitGlobal.value

/** The last denotation of this symbol */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ object PrepareInlineable {
postTransform(super.transform(preTransform(tree)))

protected def checkUnstableAccessor(accessedTree: Tree, accessor: Symbol)(using Context): Unit =
if ctx.settings.WunstableInlineAccessors.value then
if ctx.settings.Whas.unstableInlineAccessors then
val accessorTree = accessorDef(accessor, accessedTree.symbol)
report.warning(reporting.UnstableInlineAccessor(accessedTree.symbol, accessorTree), accessedTree)
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4117,7 +4117,7 @@ object Parsers {
if (in.token == COMMA) {
in.nextToken()
val ids = commaSeparated(() => termIdent())
if ctx.settings.WenumCommentDiscard.value then
if ctx.settings.Whas.enumCommentDiscard then
in.getDocComment(start).foreach: comm =>
warning(
em"""Ambiguous Scaladoc comment on multiple cases is ignored.
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class CheckUnused private (phaseMode: CheckUnused.PhaseMode, suffix: String, _ke

override def isRunnable(using Context): Boolean =
super.isRunnable &&
ctx.settings.Wunused.value.nonEmpty &&
ctx.settings.WunusedHas.any &&
!ctx.isJava

// ========== SETUP ============
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/init/Checker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class Checker extends Phase:
override val runsAfter = Set(Pickler.name)

override def isEnabled(using Context): Boolean =
super.isEnabled && (ctx.settings.WcheckInit.value || ctx.settings.YcheckInitGlobal.value)
super.isEnabled && (ctx.settings.Whas.checkInit || ctx.settings.YcheckInitGlobal.value)

def traverse(traverser: InitTreeTraverser)(using Context): Boolean = monitor(phaseName):
val unit = ctx.compilationUnit
Expand All @@ -50,7 +50,7 @@ class Checker extends Phase:
cancellable {
val classes = traverser.getClasses()

if ctx.settings.WcheckInit.value then
if ctx.settings.Whas.checkInit then
Semantic.checkClasses(classes)(using checkCtx)

if ctx.settings.YcheckInitGlobal.value then
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Linter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ object Linter:
&& !isJavaApplication(t) // Java methods are inherently side-effecting
// && !treeInfo.hasExplicitUnit(t) // suppressed by explicit expr: Unit // TODO Should explicit `: Unit` be added as warning suppression?

if ctx.settings.WNonUnitStatement.value && !ctx.isAfterTyper && checkInterestingShapes(t) then
if ctx.settings.Whas.nonUnitStatement && !ctx.isAfterTyper && checkInterestingShapes(t) then
val where = t match
case Block(_, res) => res
case If(_, thenpart, Literal(Constant(()))) =>
Expand Down Expand Up @@ -119,7 +119,7 @@ object Linter:
// still compute `canEqual(A & B, B & A) = true`.
canEqual(a, b.tp1) || canEqual(a, b.tp2)

if ctx.settings.WimplausiblePatterns.value && !canEqual(pat.tpe, selType) then
if ctx.settings.Whas.implausiblePatterns && !canEqual(pat.tpe, selType) then
report.warning(ImplausiblePatternWarning(pat, selType), pat.srcPos)
end warnOnImplausiblePattern

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4525,7 +4525,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
// so will take the code path that decides on inlining
val tree1 = adapt(tree, WildcardType, locked)
checkStatementPurity(tree1)(tree, ctx.owner, isUnitExpr = true)
if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.WvalueDiscard.value && !isThisTypeResult(tree)) {
if (!ctx.isAfterTyper && !tree.isInstanceOf[Inlined] && ctx.settings.Whas.valueDiscard && !isThisTypeResult(tree)) {
report.warning(ValueDiscarding(tree.tpe), tree.srcPos)
}
return tpd.Block(tree1 :: Nil, unitLiteral)
Expand Down
12 changes: 12 additions & 0 deletions tests/warn/i18559a.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- [E198] Unused Symbol Warning: tests/warn/i18559a.scala:4:28 ---------------------------------------------------------
4 | import collection.mutable.Set // warn
| ^^^
| unused import
-- [E198] Unused Symbol Warning: tests/warn/i18559a.scala:8:8 ----------------------------------------------------------
8 | val x = 1 // warn
| ^
| unused local definition
-- [E198] Unused Symbol Warning: tests/warn/i18559a.scala:11:26 --------------------------------------------------------
11 | import SomeGivenImports.given // warn
| ^^^^^
| unused import
15 changes: 15 additions & 0 deletions tests/warn/i18559a.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//> using options -Wall
// This test checks that -Wall turns on -Wunused:all if -Wunused is not set
object FooImportUnused:
import collection.mutable.Set // warn

object FooUnusedLocal:
def test(): Unit =
val x = 1 // warn

object FooGivenUnused:
import SomeGivenImports.given // warn

object SomeGivenImports:
given Int = 0
given String = "foo"
12 changes: 12 additions & 0 deletions tests/warn/i18559b.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- Warning: tests/warn/i18559b.scala:8:6 -------------------------------------------------------------------------------
8 | val localFile: String = s"${url.##}.tmp" // warn
| ^
| Access non-initialized value localFile. Calling trace:
| ├── class RemoteFile(url: String) extends AbstractFile: [ i18559b.scala:7 ]
| │ ^
| ├── abstract class AbstractFile: [ i18559b.scala:3 ]
| │ ^
| ├── val extension: String = name.substring(4) [ i18559b.scala:5 ]
| │ ^^^^
| └── def name: String = localFile [ i18559b.scala:9 ]
| ^^^^^^^^^
9 changes: 9 additions & 0 deletions tests/warn/i18559b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//> using options -Wall
// This test checks that -Wall turns on -Wsafe-init
abstract class AbstractFile:
def name: String
val extension: String = name.substring(4)

class RemoteFile(url: String) extends AbstractFile:
val localFile: String = s"${url.##}.tmp" // warn
def name: String = localFile
12 changes: 12 additions & 0 deletions tests/warn/i18559c.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- [E198] Unused Symbol Warning: tests/warn/i18559c.scala:4:28 ---------------------------------------------------------
4 | import collection.mutable.Set // warn
| ^^^
| unused import
-- [E198] Unused Symbol Warning: tests/warn/i18559c.scala:8:8 ----------------------------------------------------------
8 | val x = 1 // warn
| ^
| unused local definition
-- [E198] Unused Symbol Warning: tests/warn/i18559c.scala:11:26 --------------------------------------------------------
11 | import SomeGivenImports.given // warn
| ^^^^^
| unused import
15 changes: 15 additions & 0 deletions tests/warn/i18559c.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//> using options -Wall -Wunused:locals
// This test checks that -Wall overrides -Wunused:... if it is already set
object FooImportUnused:
import collection.mutable.Set // warn

object FooUnusedLocal:
def test(): Unit =
val x = 1 // warn

object FooGivenUnused:
import SomeGivenImports.given // warn

object SomeGivenImports:
given Int = 0
given String = "foo"

0 comments on commit 8b911ef

Please sign in to comment.