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 the -Wall option that enables all warnings (Plan B) #20577

Merged
merged 4 commits into from
Aug 22, 2024
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
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 @@ -4083,7 +4083,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 @@ -4478,7 +4478,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"
Loading