From 5727187a647dbce044ddb0e577c07f5710585997 Mon Sep 17 00:00:00 2001 From: Yichen Xu Date: Sun, 16 Jun 2024 16:46:00 +0100 Subject: [PATCH 1/4] Add `-Wall` --- .../tools/dotc/config/ScalaSettings.scala | 68 +++++++++++-------- .../src/dotty/tools/dotc/core/Symbols.scala | 2 +- .../dotc/inlines/PrepareInlineable.scala | 2 +- .../dotty/tools/dotc/parsing/Parsers.scala | 2 +- .../tools/dotc/transform/CheckUnused.scala | 2 +- .../tools/dotc/transform/init/Checker.scala | 4 +- .../src/dotty/tools/dotc/typer/Linter.scala | 4 +- .../src/dotty/tools/dotc/typer/Typer.scala | 2 +- 8 files changed, 50 insertions(+), 36 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index c64521ec74e1..bcfc651aeb92 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -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 = Wunused.value.nonEmpty + // overrided by strict-no-implicit-warn def imports(using Context) = (allOr("imports") || allOr("linted")) && !(strictNoImplicitWarn) @@ -296,7 +300,17 @@ private sealed trait WarningSettings: def typeParameterShadow(using Context) = allOr("type-parameter-shadow") - val WcheckInit: Setting[Boolean] = BooleanSetting(WarningSetting, "Wsafe-init", "Ensure safe initialization of objects.") + private 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: diff --git a/compiler/src/dotty/tools/dotc/core/Symbols.scala b/compiler/src/dotty/tools/dotc/core/Symbols.scala index da0ecac47b7d..b8830f2520c5 100644 --- a/compiler/src/dotty/tools/dotc/core/Symbols.scala +++ b/compiler/src/dotty/tools/dotc/core/Symbols.scala @@ -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 */ diff --git a/compiler/src/dotty/tools/dotc/inlines/PrepareInlineable.scala b/compiler/src/dotty/tools/dotc/inlines/PrepareInlineable.scala index 1acc6a1c8317..bb950fbe43cd 100644 --- a/compiler/src/dotty/tools/dotc/inlines/PrepareInlineable.scala +++ b/compiler/src/dotty/tools/dotc/inlines/PrepareInlineable.scala @@ -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) } diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index e28ba5fd669e..3a987a9358e4 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -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. diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index d8389ff964a4..10a4721ad1d0 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -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 ============ diff --git a/compiler/src/dotty/tools/dotc/transform/init/Checker.scala b/compiler/src/dotty/tools/dotc/transform/init/Checker.scala index 9e78bd5474a3..4d5c467cf4fe 100644 --- a/compiler/src/dotty/tools/dotc/transform/init/Checker.scala +++ b/compiler/src/dotty/tools/dotc/transform/init/Checker.scala @@ -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 @@ -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 diff --git a/compiler/src/dotty/tools/dotc/typer/Linter.scala b/compiler/src/dotty/tools/dotc/typer/Linter.scala index c0ba581b3732..4c02bf80df63 100644 --- a/compiler/src/dotty/tools/dotc/typer/Linter.scala +++ b/compiler/src/dotty/tools/dotc/typer/Linter.scala @@ -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(()))) => @@ -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 diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 1c779ac050fa..d5e7e693111c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -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) From d9e5fd4440e91af32672064e9dcfaa822add501a Mon Sep 17 00:00:00 2001 From: Yichen Xu Date: Mon, 10 Jun 2024 18:25:07 +0100 Subject: [PATCH 2/4] Add test cases for `-Wall` --- tests/warn/i18559a.check | 12 ++++++++++++ tests/warn/i18559a.scala | 15 +++++++++++++++ tests/warn/i18559b.check | 12 ++++++++++++ tests/warn/i18559b.scala | 9 +++++++++ tests/warn/i18559c.check | 4 ++++ tests/warn/i18559c.scala | 15 +++++++++++++++ 6 files changed, 67 insertions(+) create mode 100644 tests/warn/i18559a.check create mode 100644 tests/warn/i18559a.scala create mode 100644 tests/warn/i18559b.check create mode 100644 tests/warn/i18559b.scala create mode 100644 tests/warn/i18559c.check create mode 100644 tests/warn/i18559c.scala diff --git a/tests/warn/i18559a.check b/tests/warn/i18559a.check new file mode 100644 index 000000000000..9652a4d97ac8 --- /dev/null +++ b/tests/warn/i18559a.check @@ -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 diff --git a/tests/warn/i18559a.scala b/tests/warn/i18559a.scala new file mode 100644 index 000000000000..24f1cae449b8 --- /dev/null +++ b/tests/warn/i18559a.scala @@ -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" diff --git a/tests/warn/i18559b.check b/tests/warn/i18559b.check new file mode 100644 index 000000000000..710df8234a9a --- /dev/null +++ b/tests/warn/i18559b.check @@ -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 ] + | ^^^^^^^^^ diff --git a/tests/warn/i18559b.scala b/tests/warn/i18559b.scala new file mode 100644 index 000000000000..dac6e8c57c83 --- /dev/null +++ b/tests/warn/i18559b.scala @@ -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 diff --git a/tests/warn/i18559c.check b/tests/warn/i18559c.check new file mode 100644 index 000000000000..7fd42a48db0c --- /dev/null +++ b/tests/warn/i18559c.check @@ -0,0 +1,4 @@ +-- [E198] Unused Symbol Warning: tests/warn/i18559c.scala:8:8 ---------------------------------------------------------- +8 | val x = 1 // warn + | ^ + | unused local definition diff --git a/tests/warn/i18559c.scala b/tests/warn/i18559c.scala new file mode 100644 index 000000000000..3ca0c8893a66 --- /dev/null +++ b/tests/warn/i18559c.scala @@ -0,0 +1,15 @@ +//> using options -Wall -Wunused:locals +// This test checks that -Wall leaves -Wunused:... untouched if it is already set +object FooImportUnused: + import collection.mutable.Set // not warn + +object FooUnusedLocal: + def test(): Unit = + val x = 1 // warn + +object FooGivenUnused: + import SomeGivenImports.given // not warn + +object SomeGivenImports: + given Int = 0 + given String = "foo" From 81f2c8e4d267f29f9d5aff72144759a2810ac288 Mon Sep 17 00:00:00 2001 From: Yichen Xu Date: Mon, 1 Jul 2024 23:27:40 +0200 Subject: [PATCH 3/4] Make `WcheckInit` public This is needed to make ScalaSettingsTests.scala compile --- compiler/src/dotty/tools/dotc/config/ScalaSettings.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index bcfc651aeb92..011b31aba50a 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -300,7 +300,7 @@ private sealed trait WarningSettings: def typeParameterShadow(using Context) = allOr("type-parameter-shadow") - private val WcheckInit: Setting[Boolean] = BooleanSetting(WarningSetting, "Wsafe-init", "Ensure safe initialization of objects.") + val WcheckInit: Setting[Boolean] = BooleanSetting(WarningSetting, "Wsafe-init", "Ensure safe initialization of objects.") object Whas: def allOr(s: Setting[Boolean])(using Context): Boolean = From af0412ced6ae8eca55a682ef3f8d42229cbd8782 Mon Sep 17 00:00:00 2001 From: Yichen Xu Date: Tue, 2 Jul 2024 02:47:05 +0200 Subject: [PATCH 4/4] Let `-Wall` override `-Wunused` --- compiler/src/dotty/tools/dotc/config/ScalaSettings.scala | 2 +- tests/warn/i18559c.check | 8 ++++++++ tests/warn/i18559c.scala | 6 +++--- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 011b31aba50a..d775a4239d1b 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -203,7 +203,7 @@ private sealed trait WarningSettings: def nowarn(using Context) = allOr("nowarn") // Is any choice set for -Wunused? - def any(using Context): Boolean = Wunused.value.nonEmpty + def any(using Context): Boolean = Wall.value || Wunused.value.nonEmpty // overrided by strict-no-implicit-warn def imports(using Context) = diff --git a/tests/warn/i18559c.check b/tests/warn/i18559c.check index 7fd42a48db0c..1c1bd86bf15f 100644 --- a/tests/warn/i18559c.check +++ b/tests/warn/i18559c.check @@ -1,4 +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 diff --git a/tests/warn/i18559c.scala b/tests/warn/i18559c.scala index 3ca0c8893a66..34576cd831b0 100644 --- a/tests/warn/i18559c.scala +++ b/tests/warn/i18559c.scala @@ -1,14 +1,14 @@ //> using options -Wall -Wunused:locals -// This test checks that -Wall leaves -Wunused:... untouched if it is already set +// This test checks that -Wall overrides -Wunused:... if it is already set object FooImportUnused: - import collection.mutable.Set // not warn + import collection.mutable.Set // warn object FooUnusedLocal: def test(): Unit = val x = 1 // warn object FooGivenUnused: - import SomeGivenImports.given // not warn + import SomeGivenImports.given // warn object SomeGivenImports: given Int = 0