Skip to content

Commit

Permalink
Backport "Add the -Wall option that enables all warnings (Plan B)" to…
Browse files Browse the repository at this point in the history
… LTS (#22107)

Backports #20577 to the 3.3.5.

PR submitted by the release tooling.
[skip ci]
  • Loading branch information
WojciechMazur authored Dec 4, 2024
2 parents 56dbb7e + f372d14 commit d1bf6ee
Show file tree
Hide file tree
Showing 14 changed files with 124 additions and 35 deletions.
65 changes: 39 additions & 26 deletions compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -165,46 +165,50 @@ private sealed trait WarningSettings:

val Whelp: Setting[Boolean] = BooleanSetting("-W", "Print a synopsis of warning options.")
val XfatalWarnings: Setting[Boolean] = BooleanSetting("-Werror", "Fail the compilation if there are any warnings.", aliases = List("-Xfatal-warnings"))
val WvalueDiscard: Setting[Boolean] = BooleanSetting("-Wvalue-discard", "Warn when non-Unit expression results are unused.")
val WNonUnitStatement = BooleanSetting("-Wnonunit-statement", "Warn when block statements are non-Unit expressions.")
val WenumCommentDiscard = BooleanSetting("-Wenum-comment-discard", "Warn when a comment ambiguously assigned to multiple enum cases is discarded.")
val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
val Wall: Setting[Boolean] = BooleanSetting("-Wall", "Enable all warning settings.")
private val WvalueDiscard: Setting[Boolean] = BooleanSetting("-Wvalue-discard", "Warn when non-Unit expression results are unused.")
private val WNonUnitStatement = BooleanSetting("-Wnonunit-statement", "Warn when block statements are non-Unit expressions.")
private val WenumCommentDiscard = BooleanSetting("-Wenum-comment-discard", "Warn when a comment ambiguously assigned to multiple enum cases is discarded.")
private val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting(
name = "-Wunused",
helpArg = "warning",
descr = "Enable or disable specific `unused` warnings",
choices = List(
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 @@ -278,6 +282,16 @@ private sealed trait WarningSettings:
|to prevent the shell from expanding patterns.""".stripMargin,
)

val YcheckInit: Setting[Boolean] = BooleanSetting("-Ysafe-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 checkInit(using Context): Boolean = allOr(YcheckInit)

/** -X "Extended" or "Advanced" settings */
private sealed trait XSettings:
self: SettingGroup =>
Expand Down Expand Up @@ -415,7 +429,6 @@ private sealed trait YSettings:
// Experimental language features
val YnoKindPolymorphism: Setting[Boolean] = BooleanSetting("-Yno-kind-polymorphism", "Disable kind polymorphism.")
val YexplicitNulls: Setting[Boolean] = BooleanSetting("-Yexplicit-nulls", "Make reference types non-nullable. Nullable types can be expressed with unions: e.g. String|Null.")
val YcheckInit: Setting[Boolean] = BooleanSetting("-Ysafe-init", "Ensure safe initialization of objects.")
val YrequireTargetName: Setting[Boolean] = BooleanSetting("-Yrequire-targetName", "Warn if an operator is defined without a @targetName annotation.")
val YrecheckTest: Setting[Boolean] = BooleanSetting("-Yrecheck-test", "Run basic rechecking (internal test only).")
val YccDebug: Setting[Boolean] = BooleanSetting("-Ycc-debug", "Used in conjunction with captureChecking language import, debug info for captured references.")
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 @@ -80,7 +80,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.YcheckInit.value // initialization check
ctx.settings.Whas.checkInit // initialization check

/** The last denotation of this symbol */
private var lastDenot: SymDenotation = _
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 @@ -3923,7 +3923,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
5 changes: 3 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.YcheckInit.value
super.isEnabled && ctx.settings.Whas.checkInit

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

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

units0
Expand Down
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4196,7 +4196,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 Expand Up @@ -4498,7 +4498,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
&& !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
2 changes: 1 addition & 1 deletion tests/pos-with-compiler-cc/dotc/core/Symbols.scala
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ object Symbols {
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.YcheckInit.value // initialization check
ctx.settings.Whas.checkInit // initialization check

/** The last denotation of this symbol */
private var lastDenot: SymDenotation = _
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class Checker extends Phase:
override val runsAfter = Set(Pickler.name)

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

override def runOn(units: List[CompilationUnit])(using Context): List[CompilationUnit] =
val checkCtx = ctx.fresh.setPhase(this.start)
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 d1bf6ee

Please sign in to comment.