From 27bfda877dd57299b5bbe84821e9a64fda323493 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Fri, 30 Sep 2022 14:43:58 +0200 Subject: [PATCH 01/42] Report simple unused import - report unused named import - no given or wildcard import - issue with method reference (e.g. apply, factory, ...) --- compiler/src/dotty/tools/dotc/Compiler.scala | 1 + .../tools/dotc/config/ScalaSettings.scala | 3 +- .../tools/dotc/transform/CheckUnused.scala | 83 +++++++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 compiler/src/dotty/tools/dotc/transform/CheckUnused.scala diff --git a/compiler/src/dotty/tools/dotc/Compiler.scala b/compiler/src/dotty/tools/dotc/Compiler.scala index 46d36c4412c7..8a9cd0ca5b8b 100644 --- a/compiler/src/dotty/tools/dotc/Compiler.scala +++ b/compiler/src/dotty/tools/dotc/Compiler.scala @@ -35,6 +35,7 @@ class Compiler { protected def frontendPhases: List[List[Phase]] = List(new Parser) :: // Compiler frontend: scanner, parser List(new TyperPhase) :: // Compiler frontend: namer, typer + List(new CheckUnused) :: // Check for unused elements List(new YCheckPositions) :: // YCheck positions List(new sbt.ExtractDependencies) :: // Sends information on classes' dependencies to sbt via callbacks List(new semanticdb.ExtractSemanticDB) :: // Extract info into .semanticdb files diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 9e34f8d726b5..a03d569b090e 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -161,12 +161,13 @@ private sealed trait WarningSettings: name = "-Wunused", helpArg = "warning", descr = "Enable or disable specific `unused` warnings", - choices = List("nowarn", "all"), + choices = List("nowarn", "all", "import"), default = Nil ) object WunusedHas: def allOr(s: String)(using Context) = Wunused.value.pipe(us => us.contains("all") || us.contains(s)) def nowarn(using Context) = allOr("nowarn") + def imports(using Context) = allOr("import") val Wconf: Setting[List[String]] = MultiStringSetting( "-Wconf", diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala new file mode 100644 index 000000000000..58fbf0d8284b --- /dev/null +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -0,0 +1,83 @@ +package dotty.tools.dotc.transform + +import dotty.tools.dotc.ast.tpd +import dotty.tools.dotc.ast.tpd.TreeTraverser +import dotty.tools.dotc.core.Contexts._ +import dotty.tools.dotc.core.Decorators.i +import dotty.tools.dotc.core.Phases.Phase +import dotty.tools.dotc.report +import dotty.tools.dotc.reporting.Message +import dotty.tools.dotc.typer.ImportInfo +import dotty.tools.dotc.util.Property +import dotty.tools.dotc.config.ScalaSettings + +class CheckUnused extends Phase { + import CheckUnused.UnusedDatas + + private val _key = Property.Key[UnusedDatas] + + override def phaseName: String = CheckUnused.phaseName + + override def description: String = CheckUnused.description + + override def run(using Context): Unit = + val tree = ctx.compilationUnit.tpdTree + val data = UnusedDatas() + val fresh = ctx.fresh.setProperty(_key, data) + traverser.traverse(tree)(using fresh) + reportUnusedImport(data.notFound) + + private def traverser = new TreeTraverser { + import tpd._ + + + override def traverse(tree: tpd.Tree)(using Context): Unit = tree match + case imp@Import(_, sels) => sels.foreach { s => + if s.isGiven || s.isWildcard then // TODO: handle case + () + else + ctx.property(_key).foreach(_.defOrImported += imp) + traverseChildren(tree) + } + + case ident: Ident => + val id = ident.symbol.id + ctx.property(_key).foreach(_.found += id) + traverseChildren(tree) + case sel: Select => + val id = sel.symbol.id + ctx.property(_key).foreach(_.found += id) + traverseChildren(tree) + case _ => traverseChildren(tree) + + } + + private def reportUnusedImport(imports: Set[tpd.Import])(using Context) = + if ctx.settings.WunusedHas.imports then + imports.foreach { imp => + report.warning(i"unused import", imp.srcPos) + } +} + +object CheckUnused: + val phaseName: String = "check unused" + val description: String = "check for unused elements" + + private class UnusedDatas: + import collection.{mutable, immutable} + import mutable.Set + + val found: Set[Int] = Set() + val defOrImported: Set[tpd.Import] = Set() + + def notFound(using Context): immutable.Set[tpd.Import] = + defOrImported.toSet.filterNot { imp => + imp.selectors.exists{ sel => + val id = imp.expr.tpe.member(sel.name).symbol.id + found(id) + } + } + + end UnusedDatas + + From f2c15517a617cb3e62e33152c6130892badf4e20 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Tue, 4 Oct 2022 16:48:40 +0200 Subject: [PATCH 02/42] Add warnings for unused wildcard imports Emits a warning when none of element of a wildcard ('_') import is used (i.e. a "unused import" warning). --- .../tools/dotc/transform/CheckUnused.scala | 91 +++++++++++++------ 1 file changed, 63 insertions(+), 28 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 58fbf0d8284b..011f6f4935f7 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -10,11 +10,19 @@ import dotty.tools.dotc.reporting.Message import dotty.tools.dotc.typer.ImportInfo import dotty.tools.dotc.util.Property import dotty.tools.dotc.config.ScalaSettings - +import dotty.tools.dotc.ast.untpd.ImportSelector +import dotty.tools.dotc.core.StdNames +import dotty.tools.dotc.ast.untpd +/** + * A compiler phase that checks for unused imports or definitions + * + * Basically, it gathers definition/imports and their usage. If a + * definition/imports does not have any usage, then it is reported. + */ class CheckUnused extends Phase { - import CheckUnused.UnusedDatas + import CheckUnused.UnusedData - private val _key = Property.Key[UnusedDatas] + private val _key = Property.Key[UnusedData] override def phaseName: String = CheckUnused.phaseName @@ -22,40 +30,41 @@ class CheckUnused extends Phase { override def run(using Context): Unit = val tree = ctx.compilationUnit.tpdTree - val data = UnusedDatas() + val data = UnusedData() val fresh = ctx.fresh.setProperty(_key, data) traverser.traverse(tree)(using fresh) reportUnusedImport(data.notFound) + /** + * This traverse is the **main** component of this phase + * + * It traverse the tree the tree and gather the data in the + * corresponding context property + */ private def traverser = new TreeTraverser { import tpd._ - override def traverse(tree: tpd.Tree)(using Context): Unit = tree match case imp@Import(_, sels) => sels.foreach { s => - if s.isGiven || s.isWildcard then // TODO: handle case - () - else - ctx.property(_key).foreach(_.defOrImported += imp) - traverseChildren(tree) + ctx.property(_key).foreach(_.registerImport(imp)) } case ident: Ident => val id = ident.symbol.id - ctx.property(_key).foreach(_.found += id) + ctx.property(_key).foreach(_.registerUsed(id)) traverseChildren(tree) case sel: Select => val id = sel.symbol.id - ctx.property(_key).foreach(_.found += id) + ctx.property(_key).foreach(_.registerUsed(id)) traverseChildren(tree) case _ => traverseChildren(tree) } - private def reportUnusedImport(imports: Set[tpd.Import])(using Context) = + private def reportUnusedImport(sels: Set[ImportSelector])(using Context) = if ctx.settings.WunusedHas.imports then - imports.foreach { imp => - report.warning(i"unused import", imp.srcPos) + sels.foreach { s => + report.warning(i"unused import", s.srcPos) } } @@ -63,21 +72,47 @@ object CheckUnused: val phaseName: String = "check unused" val description: String = "check for unused elements" - private class UnusedDatas: - import collection.{mutable, immutable} - import mutable.Set + /** + * A stateful class gathering the infos on : + * - imports + * - definitions + * - usage + */ + private class UnusedData: + import collection.mutable.{Set => MutSet} + + private val used: MutSet[Int] = MutSet() + private val imported: MutSet[tpd.Import] = MutSet() + + private def isImportExclusion(sel: ImportSelector): Boolean = sel.renamed match + case ident@untpd.Ident(name) => name == StdNames.nme.WILDCARD + case _ => false + + private def isUsedImportSelector(imp: tpd.Import, sel: ImportSelector)(using Context): Boolean = + if sel.isGiven then + true // TODO : handle this case + else if sel.isWildcard then // NOT GIVEN + imp.expr.tpe.allMembers.exists { m => + used(m.symbol.id) + } + else + if isImportExclusion(sel) then + true + else + used(imp.expr.tpe.member(sel.name).symbol.id) + + def notFound(using Context): Set[ImportSelector] = + for { + imp <- imported.toSet + sel <- imp.selectors if !isUsedImportSelector(imp,sel) + } yield sel - val found: Set[Int] = Set() - val defOrImported: Set[tpd.Import] = Set() + /** Register the id of a found (used) symbol */ + def registerUsed(id: Int): Unit = used += id - def notFound(using Context): immutable.Set[tpd.Import] = - defOrImported.toSet.filterNot { imp => - imp.selectors.exists{ sel => - val id = imp.expr.tpe.member(sel.name).symbol.id - found(id) - } - } + /** Register an import */ + def registerImport(imp: tpd.Import): Unit = imported += imp - end UnusedDatas + end UnusedData From d6fa4cb58e2b92311e34d3587e6e572ef71bb0b9 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Sun, 9 Oct 2022 00:15:41 +0200 Subject: [PATCH 03/42] Add tests and fixes for unused warning - Add various tests for unused warnings - Fix warning not emitted when imported object is used another scope --- .../tools/dotc/config/ScalaSettings.scala | 4 +- .../tools/dotc/transform/CheckUnused.scala | 79 ++++++++++++------- .../fatal-warnings/i15503/i15503a.scala | 33 ++++++++ 3 files changed, 87 insertions(+), 29 deletions(-) create mode 100644 tests/neg-custom-args/fatal-warnings/i15503/i15503a.scala diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 502998a7989b..c5bea098c77b 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -162,13 +162,13 @@ private sealed trait WarningSettings: name = "-Wunused", helpArg = "warning", descr = "Enable or disable specific `unused` warnings", - choices = List("nowarn", "all", "import"), + choices = List("nowarn", "all", "imports"), default = Nil ) object WunusedHas: def allOr(s: String)(using Context) = Wunused.value.pipe(us => us.contains("all") || us.contains(s)) def nowarn(using Context) = allOr("nowarn") - def imports(using Context) = allOr("import") + def imports(using Context) = allOr("imports") val Wconf: Setting[List[String]] = MultiStringSetting( "-Wconf", diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 011f6f4935f7..ffdd3e051313 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -13,6 +13,7 @@ import dotty.tools.dotc.config.ScalaSettings import dotty.tools.dotc.ast.untpd.ImportSelector import dotty.tools.dotc.core.StdNames import dotty.tools.dotc.ast.untpd + /** * A compiler phase that checks for unused imports or definitions * @@ -33,7 +34,7 @@ class CheckUnused extends Phase { val data = UnusedData() val fresh = ctx.fresh.setProperty(_key, data) traverser.traverse(tree)(using fresh) - reportUnusedImport(data.notFound) + reportUnusedImport(data.getUnused) /** * This traverse is the **main** component of this phase @@ -48,7 +49,6 @@ class CheckUnused extends Phase { case imp@Import(_, sels) => sels.foreach { s => ctx.property(_key).foreach(_.registerImport(imp)) } - case ident: Ident => val id = ident.symbol.id ctx.property(_key).foreach(_.registerUsed(id)) @@ -57,11 +57,15 @@ class CheckUnused extends Phase { val id = sel.symbol.id ctx.property(_key).foreach(_.registerUsed(id)) traverseChildren(tree) + case tpd.Block(_,_) | tpd.Template(_,_,_,_) => + ctx.property(_key).foreach(_.pushScope()) + traverseChildren(tree) + ctx.property(_key).foreach(_.popScope()) case _ => traverseChildren(tree) } - private def reportUnusedImport(sels: Set[ImportSelector])(using Context) = + private def reportUnusedImport(sels: List[ImportSelector])(using Context) = if ctx.settings.WunusedHas.imports then sels.foreach { s => report.warning(i"unused import", s.srcPos) @@ -78,40 +82,61 @@ object CheckUnused: * - definitions * - usage */ - private class UnusedData: - import collection.mutable.{Set => MutSet} + private class UnusedData: // TODO : handle block nesting + import collection.mutable.{Set => MutSet, Map => MutMap, Stack, ListBuffer} - private val used: MutSet[Int] = MutSet() - private val imported: MutSet[tpd.Import] = MutSet() + private val used = Stack(MutSet[Int]()) + private val impInScope = Stack(MutMap[Int, ListBuffer[ImportSelector]]()) + private val unused = ListBuffer[ImportSelector]() private def isImportExclusion(sel: ImportSelector): Boolean = sel.renamed match case ident@untpd.Ident(name) => name == StdNames.nme.WILDCARD case _ => false - private def isUsedImportSelector(imp: tpd.Import, sel: ImportSelector)(using Context): Boolean = - if sel.isGiven then - true // TODO : handle this case - else if sel.isWildcard then // NOT GIVEN - imp.expr.tpe.allMembers.exists { m => - used(m.symbol.id) - } - else - if isImportExclusion(sel) then - true + /** Register the id of a found (used) symbol */ + def registerUsed(id: Int): Unit = + used.top += id + + /** Register an import */ + def registerImport(imp: tpd.Import)(using Context): Unit = + val tpd.Import(tree, sels) = imp + val map = impInScope.top + val entries = sels.flatMap{ s => + if s.isGiven then + Nil + else if s.isWildcard then // TODO : handle givens + //Nil + tree.tpe.allMembers.map(_.symbol.id -> s) else - used(imp.expr.tpe.member(sel.name).symbol.id) + val id = tree.tpe.member(s.name).symbol.id + List(id -> s) + } + entries.foreach{(id, sel) => + map.get(id) match + case None => map.put(id, ListBuffer(sel)) + case Some(value) => value += sel + } - def notFound(using Context): Set[ImportSelector] = - for { - imp <- imported.toSet - sel <- imp.selectors if !isUsedImportSelector(imp,sel) - } yield sel + /** enter a new scope */ + def pushScope(): Unit = - /** Register the id of a found (used) symbol */ - def registerUsed(id: Int): Unit = used += id + used.push(MutSet()) + impInScope.push(MutMap()) - /** Register an import */ - def registerImport(imp: tpd.Import): Unit = imported += imp + /** leave the current scope */ + def popScope(): Unit = + val popedImp = impInScope.pop() + val notDefined = used.pop().filter{id => + popedImp.remove(id).isEmpty + } + if used.size > 0 then + used.top.addAll(notDefined) + unused.addAll(popedImp.values.flatten) + + /** leave the scope and return unused `ImportSelector`s*/ + def getUnused: List[ImportSelector] = + popScope() + unused.toList end UnusedData diff --git a/tests/neg-custom-args/fatal-warnings/i15503/i15503a.scala b/tests/neg-custom-args/fatal-warnings/i15503/i15503a.scala new file mode 100644 index 000000000000..10b6117460dd --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/i15503/i15503a.scala @@ -0,0 +1,33 @@ +// scalac: -Wunused:imports + + +object FooUnused: + import collection.mutable.Set // error + import collection.mutable.{Map => MutMap} // error + import collection.mutable._ // error + +object FooWildcardUnused: + import collection.mutable._ // error + +object Foo: + import collection.mutable.Set // OK + import collection.mutable.{Map => MutMap} // OK + + val bar = Set() // OK + val baz = MutMap() // OK + +object FooWildcard: + import collection.mutable._ // error + + val bar = Set() // OK + +object FooNestedUnused: + import collection.mutable.Set // error + object Nested: + def hello = 1 + +object FooNested: + import collection.mutable.Set // OK + object Nested: + def hello = Set() + From 9f753341596c7a7e926dca983630957fb27878a5 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Sun, 9 Oct 2022 15:30:05 +0200 Subject: [PATCH 04/42] Add warning unused given imports - Emit warning for unused given imports - Emit warning for wildcard imports when only given instances are used. ``` import SomeGivenImports.given // OK import SomeGivenImports._ // error ``` --- .../tools/dotc/transform/CheckUnused.scala | 41 +++++++++++-------- .../fatal-warnings/i15503/i15503a.scala | 18 +++++++- 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index ffdd3e051313..daefe1f9d16f 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -2,17 +2,18 @@ package dotty.tools.dotc.transform import dotty.tools.dotc.ast.tpd import dotty.tools.dotc.ast.tpd.TreeTraverser +import dotty.tools.dotc.ast.untpd +import dotty.tools.dotc.ast.untpd.ImportSelector +import dotty.tools.dotc.config.ScalaSettings import dotty.tools.dotc.core.Contexts._ import dotty.tools.dotc.core.Decorators.i +import dotty.tools.dotc.core.Flags.Given import dotty.tools.dotc.core.Phases.Phase +import dotty.tools.dotc.core.StdNames import dotty.tools.dotc.report import dotty.tools.dotc.reporting.Message import dotty.tools.dotc.typer.ImportInfo import dotty.tools.dotc.util.Property -import dotty.tools.dotc.config.ScalaSettings -import dotty.tools.dotc.ast.untpd.ImportSelector -import dotty.tools.dotc.core.StdNames -import dotty.tools.dotc.ast.untpd /** * A compiler phase that checks for unused imports or definitions @@ -20,7 +21,7 @@ import dotty.tools.dotc.ast.untpd * Basically, it gathers definition/imports and their usage. If a * definition/imports does not have any usage, then it is reported. */ -class CheckUnused extends Phase { +class CheckUnused extends Phase: import CheckUnused.UnusedData private val _key = Property.Key[UnusedData] @@ -70,7 +71,7 @@ class CheckUnused extends Phase { sels.foreach { s => report.warning(i"unused import", s.srcPos) } -} +end CheckUnused object CheckUnused: val phaseName: String = "check unused" @@ -82,7 +83,7 @@ object CheckUnused: * - definitions * - usage */ - private class UnusedData: // TODO : handle block nesting + private class UnusedData: import collection.mutable.{Set => MutSet, Map => MutMap, Stack, ListBuffer} private val used = Stack(MutSet[Int]()) @@ -102,11 +103,10 @@ object CheckUnused: val tpd.Import(tree, sels) = imp val map = impInScope.top val entries = sels.flatMap{ s => - if s.isGiven then - Nil - else if s.isWildcard then // TODO : handle givens - //Nil - tree.tpe.allMembers.map(_.symbol.id -> s) + if s.isWildcard then + tree.tpe.allMembers + .filter(m => m.symbol.is(Given) == s.isGiven) // given imports + .map(_.symbol.id -> s) else val id = tree.tpe.member(s.name).symbol.id List(id -> s) @@ -119,19 +119,28 @@ object CheckUnused: /** enter a new scope */ def pushScope(): Unit = - used.push(MutSet()) impInScope.push(MutMap()) /** leave the current scope */ def popScope(): Unit = + val usedImp = MutSet[ImportSelector]() val popedImp = impInScope.pop() val notDefined = used.pop().filter{id => - popedImp.remove(id).isEmpty + popedImp.remove(id) match + case None => true + case Some(value) => + usedImp.addAll(value) + false } if used.size > 0 then used.top.addAll(notDefined) - unused.addAll(popedImp.values.flatten) + popedImp.values.flatten.foreach{ sel => + // If **any** of the entities used by the import is used, + // do not add to the `unused` Set + if !usedImp(sel) then + unused += sel + } /** leave the scope and return unused `ImportSelector`s*/ def getUnused: List[ImportSelector] = @@ -139,5 +148,5 @@ object CheckUnused: unused.toList end UnusedData - +end CheckUnused diff --git a/tests/neg-custom-args/fatal-warnings/i15503/i15503a.scala b/tests/neg-custom-args/fatal-warnings/i15503/i15503a.scala index 10b6117460dd..da3aa4a0bfd7 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503/i15503a.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503/i15503a.scala @@ -17,7 +17,7 @@ object Foo: val baz = MutMap() // OK object FooWildcard: - import collection.mutable._ // error + import collection.mutable._ // OK val bar = Set() // OK @@ -31,3 +31,19 @@ object FooNested: object Nested: def hello = Set() +object FooGivenUnused: + import SomeGivenImports.given // error + +object FooGiven: + import SomeGivenImports.given // OK + import SomeGivenImports._ // error + + val foo = summon[Int] + +/** + * Some given values for the test + */ +object SomeGivenImports: + given Int = 0 + given String = "foo" + From e37c2f972374475ad2e66b8b7c0ad43b0e99cf2f Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Mon, 10 Oct 2022 15:03:18 +0200 Subject: [PATCH 05/42] Add tests for inline method --- .../tools/dotc/transform/CheckUnused.scala | 51 ++++++++++++++----- .../fatal-warnings/i15503/i15503a.scala | 38 ++++++++++++++ 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index daefe1f9d16f..ff540a5274aa 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -32,10 +32,12 @@ class CheckUnused extends Phase: override def run(using Context): Unit = val tree = ctx.compilationUnit.tpdTree - val data = UnusedData() - val fresh = ctx.fresh.setProperty(_key, data) - traverser.traverse(tree)(using fresh) - reportUnusedImport(data.getUnused) + val data = UnusedData(ctx) + if data.neededChecks.nonEmpty then + // Execute checks if there exists at lease one config + val fresh = ctx.fresh.setProperty(_key, data) + traverser.traverse(tree)(using fresh) + reportUnusedImport(data.getUnused) /** * This traverse is the **main** component of this phase @@ -77,15 +79,30 @@ object CheckUnused: val phaseName: String = "check unused" val description: String = "check for unused elements" + /** + * Various supported configuration chosen by -Wunused: + */ + private enum UnusedConfig: + case UnusedImports + // TODO : handle other cases like unused local def + /** * A stateful class gathering the infos on : * - imports * - definitions * - usage */ - private class UnusedData: + private class UnusedData(initctx: Context): import collection.mutable.{Set => MutSet, Map => MutMap, Stack, ListBuffer} + val neededChecks = + import UnusedConfig._ + val hasConfig = initctx.settings.WunusedHas + val mut = MutSet[UnusedConfig]() + if hasConfig.imports(using initctx) then + mut += UnusedImports + mut.toSet + private val used = Stack(MutSet[Int]()) private val impInScope = Stack(MutMap[Int, ListBuffer[ImportSelector]]()) private val unused = ListBuffer[ImportSelector]() @@ -108,8 +125,9 @@ object CheckUnused: .filter(m => m.symbol.is(Given) == s.isGiven) // given imports .map(_.symbol.id -> s) else - val id = tree.tpe.member(s.name).symbol.id - List(id -> s) + val id = tree.tpe.member(s.name.toTermName).symbol.id + val typeId = tree.tpe.member(s.name.toTypeName).symbol.id + List(id -> s, typeId -> s) } entries.foreach{(id, sel) => map.get(id) match @@ -125,9 +143,9 @@ object CheckUnused: /** leave the current scope */ def popScope(): Unit = val usedImp = MutSet[ImportSelector]() - val popedImp = impInScope.pop() + val poppedImp = impInScope.pop() val notDefined = used.pop().filter{id => - popedImp.remove(id) match + poppedImp.remove(id) match case None => true case Some(value) => usedImp.addAll(value) @@ -135,17 +153,24 @@ object CheckUnused: } if used.size > 0 then used.top.addAll(notDefined) - popedImp.values.flatten.foreach{ sel => + poppedImp.values.flatten.foreach{ sel => // If **any** of the entities used by the import is used, // do not add to the `unused` Set if !usedImp(sel) then unused += sel } - /** leave the scope and return unused `ImportSelector`s*/ - def getUnused: List[ImportSelector] = + /** + * Leave the scope and return a `List` of unused `ImportSelector`s + * + * The given `List` is sorted by line and then column of the position + */ + def getUnused(using Context): List[ImportSelector] = popScope() - unused.toList + unused.toList.sortBy{ sel => + val pos = sel.srcPos.sourcePos + (pos.line, pos.column) + } end UnusedData end CheckUnused diff --git a/tests/neg-custom-args/fatal-warnings/i15503/i15503a.scala b/tests/neg-custom-args/fatal-warnings/i15503/i15503a.scala index da3aa4a0bfd7..72506bf3d1b3 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503/i15503a.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503/i15503a.scala @@ -40,6 +40,44 @@ object FooGiven: val foo = summon[Int] +/** + * Import used as type name are considered + * as used. + * + * Import here are only used as types, not as + * Term + */ +object FooTypeName: + import collection.mutable.Set // OK + import collection.mutable.Map // OK + import collection.mutable.Seq // OK + import collection.mutable.ArrayBuilder // OK + import collection.mutable.ListBuffer // error + + def checkImplicit[A](using Set[A]) = () + def checkParamType[B](a: Map[B,B]): Seq[B] = ??? + def checkTypeParam[A] = () + + checkTypeParam[ArrayBuilder[Int]] + + +object InlineChecks: + object InlineFoo: + import collection.mutable.Set // OK + import collection.mutable.Map // error + inline def getSet = Set(1) + + object InlinedBar: + import collection.mutable.Set // error + import collection.mutable.Map // error + val a = InlineFoo.getSet + +object MacroChecks: + object StringInterpol: + import collection.mutable.Set // OK + import collection.mutable.Map // OK + println(s"This is a mutableSet : ${Set[Map[Int,Int]]()}") + /** * Some given values for the test */ From 2d6da623fb2a9c8bb1082dde7c90887a1e8c2d8d Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Tue, 11 Oct 2022 16:34:29 +0200 Subject: [PATCH 06/42] Add an `isRunnable` for `CheckUnused` - Add an `isRunnable` methode to the `CheckUnused` phase to avoid unecessary costly checks (suggested by @bishabosha) - Move neg compilation tests out of folder --- .../tools/dotc/transform/CheckUnused.scala | 28 +++++-------------- .../fatal-warnings/{i15503 => }/i15503a.scala | 0 2 files changed, 7 insertions(+), 21 deletions(-) rename tests/neg-custom-args/fatal-warnings/{i15503 => }/i15503a.scala (100%) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index ff540a5274aa..70440e147dff 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -30,14 +30,15 @@ class CheckUnused extends Phase: override def description: String = CheckUnused.description + override def isRunnable(using Context): Boolean = + ctx.settings.WunusedHas.imports + override def run(using Context): Unit = val tree = ctx.compilationUnit.tpdTree val data = UnusedData(ctx) - if data.neededChecks.nonEmpty then - // Execute checks if there exists at lease one config - val fresh = ctx.fresh.setProperty(_key, data) - traverser.traverse(tree)(using fresh) - reportUnusedImport(data.getUnused) + val fresh = ctx.fresh.setProperty(_key, data) + traverser.traverse(tree)(using fresh) + reportUnusedImport(data.getUnused) /** * This traverse is the **main** component of this phase @@ -79,13 +80,6 @@ object CheckUnused: val phaseName: String = "check unused" val description: String = "check for unused elements" - /** - * Various supported configuration chosen by -Wunused: - */ - private enum UnusedConfig: - case UnusedImports - // TODO : handle other cases like unused local def - /** * A stateful class gathering the infos on : * - imports @@ -95,13 +89,6 @@ object CheckUnused: private class UnusedData(initctx: Context): import collection.mutable.{Set => MutSet, Map => MutMap, Stack, ListBuffer} - val neededChecks = - import UnusedConfig._ - val hasConfig = initctx.settings.WunusedHas - val mut = MutSet[UnusedConfig]() - if hasConfig.imports(using initctx) then - mut += UnusedImports - mut.toSet private val used = Stack(MutSet[Int]()) private val impInScope = Stack(MutMap[Int, ListBuffer[ImportSelector]]()) @@ -112,8 +99,7 @@ object CheckUnused: case _ => false /** Register the id of a found (used) symbol */ - def registerUsed(id: Int): Unit = - used.top += id + def registerUsed(id: Int): Unit = used.top += id /** Register an import */ def registerImport(imp: tpd.Import)(using Context): Unit = diff --git a/tests/neg-custom-args/fatal-warnings/i15503/i15503a.scala b/tests/neg-custom-args/fatal-warnings/i15503a.scala similarity index 100% rename from tests/neg-custom-args/fatal-warnings/i15503/i15503a.scala rename to tests/neg-custom-args/fatal-warnings/i15503a.scala From 0801dd60d2a8628a2b0a645c6d4a6006f1ee607a Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Fri, 14 Oct 2022 21:16:23 +0200 Subject: [PATCH 07/42] Ignore exclusion when unused import - Do not emit any warning for import exclusion. - These are hard to interpret. These can be placed on purpose to avoid future usage of a symbol. --- .../dotty/tools/dotc/transform/CheckUnused.scala | 5 ++++- tests/neg-custom-args/fatal-warnings/i15503a.scala | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 70440e147dff..51ba95bcf6b3 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -106,10 +106,13 @@ object CheckUnused: val tpd.Import(tree, sels) = imp val map = impInScope.top val entries = sels.flatMap{ s => - if s.isWildcard then + if isImportExclusion(s) then + Nil // ignore exclusion import + else if s.isWildcard then tree.tpe.allMembers .filter(m => m.symbol.is(Given) == s.isGiven) // given imports .map(_.symbol.id -> s) + else val id = tree.tpe.member(s.name.toTermName).symbol.id val typeId = tree.tpe.member(s.name.toTypeName).symbol.id diff --git a/tests/neg-custom-args/fatal-warnings/i15503a.scala b/tests/neg-custom-args/fatal-warnings/i15503a.scala index 72506bf3d1b3..40eddf6c5e99 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503a.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503a.scala @@ -78,6 +78,20 @@ object MacroChecks: import collection.mutable.Map // OK println(s"This is a mutableSet : ${Set[Map[Int,Int]]()}") + +object InnerMostCheck: + import collection.mutable.* // error + def check = + import collection.mutable.* //OK + val a = Set(1) + +object IgnoreExclusion: + import collection.mutable.{Set => _} // OK + import collection.mutable.{Map => _} // OK + import collection.mutable.{ListBuffer} // error + def check = + val a = Set(1) + val b = Map(1 -> 2) /** * Some given values for the test */ From e1188b63cfd8ed8753e37a9cdeeafdfaae8edc2d Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Sun, 23 Oct 2022 21:25:09 +0200 Subject: [PATCH 08/42] Add "-Wunused:locals" warning support - Add a compiler settings for unused local definition - Add check for unused local variable in CheckUnused phase - Add some "neg-custom-args/fatal-warnings" checks for unused locals definition --- .../tools/dotc/config/ScalaSettings.scala | 3 +- .../tools/dotc/transform/CheckUnused.scala | 110 +++++++++++++++--- .../fatal-warnings/i15503b.scala | 49 ++++++++ 3 files changed, 143 insertions(+), 19 deletions(-) create mode 100644 tests/neg-custom-args/fatal-warnings/i15503b.scala diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index a49ef2dc4f76..b57b9e13ed28 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -163,13 +163,14 @@ private sealed trait WarningSettings: name = "-Wunused", helpArg = "warning", descr = "Enable or disable specific `unused` warnings", - choices = List("nowarn", "all", "imports"), + choices = List("nowarn", "all", "imports", "locals"), default = Nil ) object WunusedHas: def allOr(s: String)(using Context) = Wunused.value.pipe(us => us.contains("all") || us.contains(s)) def nowarn(using Context) = allOr("nowarn") def imports(using Context) = allOr("imports") + def locals(using Context) = allOr("locals") val Wconf: Setting[List[String]] = MultiStringSetting( "-Wconf", diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 51ba95bcf6b3..c99bbb58463a 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -14,6 +14,7 @@ import dotty.tools.dotc.report import dotty.tools.dotc.reporting.Message import dotty.tools.dotc.typer.ImportInfo import dotty.tools.dotc.util.Property +import dotty.tools.dotc.transform.CheckUnused.UnusedData.UnusedResult /** * A compiler phase that checks for unused imports or definitions @@ -31,14 +32,15 @@ class CheckUnused extends Phase: override def description: String = CheckUnused.description override def isRunnable(using Context): Boolean = - ctx.settings.WunusedHas.imports + ctx.settings.WunusedHas.imports || + ctx.settings.WunusedHas.locals override def run(using Context): Unit = val tree = ctx.compilationUnit.tpdTree val data = UnusedData(ctx) val fresh = ctx.fresh.setProperty(_key, data) traverser.traverse(tree)(using fresh) - reportUnusedImport(data.getUnused) + reportUnused(data.getUnused) /** * This traverse is the **main** component of this phase @@ -48,6 +50,7 @@ class CheckUnused extends Phase: */ private def traverser = new TreeTraverser { import tpd._ + import UnusedData.ScopeType override def traverse(tree: tpd.Tree)(using Context): Unit = tree match case imp@Import(_, sels) => sels.foreach { s => @@ -61,19 +64,53 @@ class CheckUnused extends Phase: val id = sel.symbol.id ctx.property(_key).foreach(_.registerUsed(id)) traverseChildren(tree) - case tpd.Block(_,_) | tpd.Template(_,_,_,_) => - ctx.property(_key).foreach(_.pushScope()) + case tpd.Block(_,_) => + var prev: ScopeType = ScopeType.Other + ctx.property(_key).foreach { ud => + ud.pushScope() + prev = ud.currScope + ud.currScope = ScopeType.Local + } + traverseChildren(tree) + ctx.property(_key).foreach { ud => + ud.popScope() + ud.currScope = prev + } + case tpd.Template(_,_,_,_) => + var prev: ScopeType = ScopeType.Other + ctx.property(_key).foreach { ud => + ud.pushScope() + prev = ud.currScope + ud.currScope = ScopeType.Template + } + traverseChildren(tree) + ctx.property(_key).foreach { ud => + ud.popScope() + ud.currScope = prev + } + case t:tpd.ValDef => + ctx.property(_key).foreach(_.registerLocalDef(t)) + traverseChildren(tree) + case t:tpd.DefDef => + ctx.property(_key).foreach(_.registerLocalDef(t)) traverseChildren(tree) - ctx.property(_key).foreach(_.popScope()) case _ => traverseChildren(tree) } - private def reportUnusedImport(sels: List[ImportSelector])(using Context) = + private def reportUnused(res: UnusedData.UnusedResult)(using Context) = + val UnusedData.UnusedResult(imports, locals) = res + /* IMPORTS */ if ctx.settings.WunusedHas.imports then - sels.foreach { s => + imports.foreach { s => report.warning(i"unused import", s.srcPos) } + /* LOCAL VAL OR DEF */ + if ctx.settings.WunusedHas.locals then + locals.foreach { s => + report.warning(i"unused local definition", s.srcPos) + } + end CheckUnused object CheckUnused: @@ -88,18 +125,28 @@ object CheckUnused: */ private class UnusedData(initctx: Context): import collection.mutable.{Set => MutSet, Map => MutMap, Stack, ListBuffer} + import UnusedData.ScopeType + var currScope: ScopeType = ScopeType.Other - private val used = Stack(MutSet[Int]()) + /* IMPORTS */ private val impInScope = Stack(MutMap[Int, ListBuffer[ImportSelector]]()) - private val unused = ListBuffer[ImportSelector]() + private val unusedImport = ListBuffer[ImportSelector]() + private val usedImports = Stack(MutSet[Int]()) + + /* LOCAL DEF OR VAL */ + private val localDefInScope = Stack(MutSet[tpd.ValOrDefDef]()) + private val unusedLocalDef = ListBuffer[tpd.ValOrDefDef]() + private val usedLocal = MutSet[Int]() private def isImportExclusion(sel: ImportSelector): Boolean = sel.renamed match case ident@untpd.Ident(name) => name == StdNames.nme.WILDCARD case _ => false /** Register the id of a found (used) symbol */ - def registerUsed(id: Int): Unit = used.top += id + def registerUsed(id: Int): Unit = + usedImports.top += id + usedLocal += id /** Register an import */ def registerImport(imp: tpd.Import)(using Context): Unit = @@ -124,43 +171,70 @@ object CheckUnused: case Some(value) => value += sel } + def registerLocalDef(valOrDef: tpd.ValOrDefDef)(using Context): Unit = + if currScope == ScopeType.Local then + localDefInScope.top += valOrDef + /** enter a new scope */ def pushScope(): Unit = - used.push(MutSet()) + usedImports.push(MutSet()) impInScope.push(MutMap()) + localDefInScope.push(MutSet()) /** leave the current scope */ - def popScope(): Unit = + def popScope()(using Context): Unit = + popScopeImport() + popScopeLocalDef() + + def popScopeImport(): Unit = val usedImp = MutSet[ImportSelector]() val poppedImp = impInScope.pop() - val notDefined = used.pop().filter{id => + val notDefined = usedImports.pop.filter{id => poppedImp.remove(id) match case None => true case Some(value) => usedImp.addAll(value) false } - if used.size > 0 then - used.top.addAll(notDefined) + if usedImports.size > 0 then + usedImports.top.addAll(notDefined) + poppedImp.values.flatten.foreach{ sel => // If **any** of the entities used by the import is used, // do not add to the `unused` Set if !usedImp(sel) then - unused += sel + unusedImport += sel } + def popScopeLocalDef()(using Context): Unit = + val unused = localDefInScope.pop().filterInPlace(d => !usedLocal(d.symbol.id)) + unusedLocalDef ++= unused + /** * Leave the scope and return a `List` of unused `ImportSelector`s * * The given `List` is sorted by line and then column of the position */ - def getUnused(using Context): List[ImportSelector] = + def getUnused(using Context): UnusedResult = popScope() - unused.toList.sortBy{ sel => + val sortedImp = unusedImport.toList.sortBy{ sel => val pos = sel.srcPos.sourcePos (pos.line, pos.column) } + val sortedLocals = unusedLocalDef.toList.sortBy { sel => + val pos = sel.srcPos.sourcePos + (pos.line, pos.column) + } + UnusedResult(sortedImp, sortedLocals) end UnusedData + + object UnusedData: + enum ScopeType: + case Local + case Template + case Other + + case class UnusedResult(imports: List[ImportSelector], locals: List[tpd.ValOrDefDef]) end CheckUnused diff --git a/tests/neg-custom-args/fatal-warnings/i15503b.scala b/tests/neg-custom-args/fatal-warnings/i15503b.scala new file mode 100644 index 000000000000..ef0a99a4fcfe --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/i15503b.scala @@ -0,0 +1,49 @@ +// scalac: -Wunused:locals + +val a = 1 // OK + +val b = // OK + val e1 = 1 // error + def e2 = 2 // error + 1 + +val c = // OK + val e1 = 1 // OK + def e2 = e1 // OK + e2 + +def d = 1 // OK + +def e = // OK + val e1 = 1 // error + def e2 = 2 // error + 1 + +def f = // OK + val f1 = 1 // OK + def f2 = f1 // OK + f2 + +class Foo { + val b = // OK + val e1 = 1 // error + def e2 = 2 // error + 1 + + val c = // OK + val e1 = 1 // OK + def e2 = e1 // OK + e2 + + def d = 1 // OK + + def e = // OK + val e1 = 1 // error + def e2 = 2 // error + 1 + + def f = // OK + val f1 = 1 // OK + def f2 = f1 // OK + f2 +} From 5447cdbaf857c9956ef36bdb3e045d15e4b067f1 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Mon, 24 Oct 2022 12:10:12 +0200 Subject: [PATCH 09/42] Add "-Wunused:privates" warning option - Add the "privates" option for "-Wunused" - Implement "unused privates member" checks in "CheckUnused" phase - Add a test suit for the previous point --- .../tools/dotc/config/ScalaSettings.scala | 3 +- .../tools/dotc/transform/CheckUnused.scala | 54 ++++++++++++++----- .../fatal-warnings/i15503c.scala | 15 ++++++ 3 files changed, 58 insertions(+), 14 deletions(-) create mode 100644 tests/neg-custom-args/fatal-warnings/i15503c.scala diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index b57b9e13ed28..b38a9baff30d 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -163,7 +163,7 @@ private sealed trait WarningSettings: name = "-Wunused", helpArg = "warning", descr = "Enable or disable specific `unused` warnings", - choices = List("nowarn", "all", "imports", "locals"), + choices = List("nowarn", "all", "imports", "locals", "privates"), default = Nil ) object WunusedHas: @@ -171,6 +171,7 @@ private sealed trait WarningSettings: def nowarn(using Context) = allOr("nowarn") def imports(using Context) = allOr("imports") def locals(using Context) = allOr("locals") + def privates(using Context) = allOr("privates") val Wconf: Setting[List[String]] = MultiStringSetting( "-Wconf", diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index c99bbb58463a..f6340d2eb0a2 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -7,7 +7,7 @@ import dotty.tools.dotc.ast.untpd.ImportSelector import dotty.tools.dotc.config.ScalaSettings import dotty.tools.dotc.core.Contexts._ import dotty.tools.dotc.core.Decorators.i -import dotty.tools.dotc.core.Flags.Given +import dotty.tools.dotc.core.Flags.{Private, Given} import dotty.tools.dotc.core.Phases.Phase import dotty.tools.dotc.core.StdNames import dotty.tools.dotc.report @@ -33,7 +33,8 @@ class CheckUnused extends Phase: override def isRunnable(using Context): Boolean = ctx.settings.WunusedHas.imports || - ctx.settings.WunusedHas.locals + ctx.settings.WunusedHas.locals || + ctx.settings.WunusedHas.privates override def run(using Context): Unit = val tree = ctx.compilationUnit.tpdTree @@ -89,17 +90,17 @@ class CheckUnused extends Phase: ud.currScope = prev } case t:tpd.ValDef => - ctx.property(_key).foreach(_.registerLocalDef(t)) + ctx.property(_key).foreach(_.registerDef(t)) traverseChildren(tree) case t:tpd.DefDef => - ctx.property(_key).foreach(_.registerLocalDef(t)) + ctx.property(_key).foreach(_.registerDef(t)) traverseChildren(tree) case _ => traverseChildren(tree) } private def reportUnused(res: UnusedData.UnusedResult)(using Context) = - val UnusedData.UnusedResult(imports, locals) = res + val UnusedData.UnusedResult(imports, locals, privates) = res /* IMPORTS */ if ctx.settings.WunusedHas.imports then imports.foreach { s => @@ -110,6 +111,11 @@ class CheckUnused extends Phase: locals.foreach { s => report.warning(i"unused local definition", s.srcPos) } + /* PRIVATES VAL OR DEF */ + if ctx.settings.WunusedHas.privates then + privates.foreach { s => + report.warning(i"unused private member", s.srcPos) + } end CheckUnused @@ -134,10 +140,12 @@ object CheckUnused: private val unusedImport = ListBuffer[ImportSelector]() private val usedImports = Stack(MutSet[Int]()) - /* LOCAL DEF OR VAL */ + /* LOCAL DEF OR VAL / Private Def or Val*/ private val localDefInScope = Stack(MutSet[tpd.ValOrDefDef]()) + private val privateDefInScope = Stack(MutSet[tpd.ValOrDefDef]()) private val unusedLocalDef = ListBuffer[tpd.ValOrDefDef]() - private val usedLocal = MutSet[Int]() + private val unusedPrivateDef = ListBuffer[tpd.ValOrDefDef]() + private val usedDef = MutSet[Int]() private def isImportExclusion(sel: ImportSelector): Boolean = sel.renamed match case ident@untpd.Ident(name) => name == StdNames.nme.WILDCARD @@ -146,7 +154,7 @@ object CheckUnused: /** Register the id of a found (used) symbol */ def registerUsed(id: Int): Unit = usedImports.top += id - usedLocal += id + usedDef += id /** Register an import */ def registerImport(imp: tpd.Import)(using Context): Unit = @@ -171,20 +179,26 @@ object CheckUnused: case Some(value) => value += sel } - def registerLocalDef(valOrDef: tpd.ValOrDefDef)(using Context): Unit = + def registerDef(valOrDef: tpd.ValOrDefDef)(using Context): Unit = if currScope == ScopeType.Local then localDefInScope.top += valOrDef + else if currScope == ScopeType.Template && valOrDef.symbol.is(Private) then + privateDefInScope.top += valOrDef /** enter a new scope */ def pushScope(): Unit = + // unused imports : usedImports.push(MutSet()) impInScope.push(MutMap()) + // local and private defs : localDefInScope.push(MutSet()) + privateDefInScope.push(MutSet()) /** leave the current scope */ def popScope()(using Context): Unit = popScopeImport() popScopeLocalDef() + popScopePrivateDef() def popScopeImport(): Unit = val usedImp = MutSet[ImportSelector]() @@ -207,9 +221,15 @@ object CheckUnused: } def popScopeLocalDef()(using Context): Unit = - val unused = localDefInScope.pop().filterInPlace(d => !usedLocal(d.symbol.id)) + val unused = localDefInScope.pop().filterInPlace(d => !usedDef(d.symbol.id)) unusedLocalDef ++= unused + def popScopePrivateDef()(using Context): Unit = + val unused = privateDefInScope.pop().filterInPlace{d => + !usedDef(d.symbol.id) + } + unusedPrivateDef ++= unused + /** * Leave the scope and return a `List` of unused `ImportSelector`s * @@ -221,11 +241,15 @@ object CheckUnused: val pos = sel.srcPos.sourcePos (pos.line, pos.column) } - val sortedLocals = unusedLocalDef.toList.sortBy { sel => + val sortedLocalDefs = unusedLocalDef.toList.sortBy { sel => + val pos = sel.srcPos.sourcePos + (pos.line, pos.column) + } + val sortedPrivateDefs = unusedPrivateDef.toList.sortBy { sel => val pos = sel.srcPos.sourcePos (pos.line, pos.column) } - UnusedResult(sortedImp, sortedLocals) + UnusedResult(sortedImp, sortedLocalDefs, sortedPrivateDefs) end UnusedData @@ -235,6 +259,10 @@ object CheckUnused: case Template case Other - case class UnusedResult(imports: List[ImportSelector], locals: List[tpd.ValOrDefDef]) + case class UnusedResult( + imports: List[ImportSelector], + locals: List[tpd.ValOrDefDef], + privates: List[tpd.ValOrDefDef], + ) end CheckUnused diff --git a/tests/neg-custom-args/fatal-warnings/i15503c.scala b/tests/neg-custom-args/fatal-warnings/i15503c.scala new file mode 100644 index 000000000000..8af0dc32d166 --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/i15503c.scala @@ -0,0 +1,15 @@ +// scalac: -Wunused:privates +class A: + class B: + private[A] val a = 1 // OK + private[B] val b = 1 // OK + private[this] val c = 1 // error + private val d = 1 // error + + private[A] val e = 1 // OK + private[this] val f = e // OK + private val g = f // OK + + val x = 1 // OK + def y = 2 // OK + def z = g // OK \ No newline at end of file From 950e7dee936a3e4b7102564a5eb9cd9ec6f4b37d Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Tue, 25 Oct 2022 11:14:15 +0200 Subject: [PATCH 10/42] Fix CheckUnused phasename --- compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index f6340d2eb0a2..6a2f4b32ea57 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -120,7 +120,7 @@ class CheckUnused extends Phase: end CheckUnused object CheckUnused: - val phaseName: String = "check unused" + val phaseName: String = "checkUnused" val description: String = "check for unused elements" /** From 5ecb0ebf504d643b30815cefed8c934378cbcbf2 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Fri, 28 Oct 2022 17:08:02 +0200 Subject: [PATCH 11/42] fix overlapping warning --- .../tools/dotc/transform/CheckUnused.scala | 72 ++++++++++--------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 6a2f4b32ea57..a65ec16c5e57 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -6,7 +6,7 @@ import dotty.tools.dotc.ast.untpd import dotty.tools.dotc.ast.untpd.ImportSelector import dotty.tools.dotc.config.ScalaSettings import dotty.tools.dotc.core.Contexts._ -import dotty.tools.dotc.core.Decorators.i +import dotty.tools.dotc.core.Decorators.{i,em} import dotty.tools.dotc.core.Flags.{Private, Given} import dotty.tools.dotc.core.Phases.Phase import dotty.tools.dotc.core.StdNames @@ -15,6 +15,8 @@ import dotty.tools.dotc.reporting.Message import dotty.tools.dotc.typer.ImportInfo import dotty.tools.dotc.util.Property import dotty.tools.dotc.transform.CheckUnused.UnusedData.UnusedResult +import dotty.tools.dotc.core.Mode + /** * A compiler phase that checks for unused imports or definitions @@ -100,22 +102,17 @@ class CheckUnused extends Phase: } private def reportUnused(res: UnusedData.UnusedResult)(using Context) = - val UnusedData.UnusedResult(imports, locals, privates) = res - /* IMPORTS */ - if ctx.settings.WunusedHas.imports then - imports.foreach { s => - report.warning(i"unused import", s.srcPos) - } - /* LOCAL VAL OR DEF */ - if ctx.settings.WunusedHas.locals then - locals.foreach { s => - report.warning(i"unused local definition", s.srcPos) - } - /* PRIVATES VAL OR DEF */ - if ctx.settings.WunusedHas.privates then - privates.foreach { s => - report.warning(i"unused private member", s.srcPos) - } + import CheckUnused.WarnTypes + res.foreach { s => + s match + case (t, WarnTypes.Imports) => + println("hey") + report.warning(s"unused import", t) + case (t, WarnTypes.LocalDefs) => + report.warning(s"unused local definition", t.startPos) + case (t, WarnTypes.PrivateMembers) => + report.warning(s"unused private member", t.startPos) + } end CheckUnused @@ -123,6 +120,11 @@ object CheckUnused: val phaseName: String = "checkUnused" val description: String = "check for unused elements" + enum WarnTypes: + case Imports + case LocalDefs + case PrivateMembers + /** * A stateful class gathering the infos on : * - imports @@ -137,7 +139,7 @@ object CheckUnused: /* IMPORTS */ private val impInScope = Stack(MutMap[Int, ListBuffer[ImportSelector]]()) - private val unusedImport = ListBuffer[ImportSelector]() + private val unusedImport = MutSet[ImportSelector]() private val usedImports = Stack(MutSet[Int]()) /* LOCAL DEF OR VAL / Private Def or Val*/ @@ -237,19 +239,25 @@ object CheckUnused: */ def getUnused(using Context): UnusedResult = popScope() - val sortedImp = unusedImport.toList.sortBy{ sel => - val pos = sel.srcPos.sourcePos - (pos.line, pos.column) - } - val sortedLocalDefs = unusedLocalDef.toList.sortBy { sel => - val pos = sel.srcPos.sourcePos - (pos.line, pos.column) - } - val sortedPrivateDefs = unusedPrivateDef.toList.sortBy { sel => - val pos = sel.srcPos.sourcePos + val sortedImp = + if ctx.settings.WunusedHas.imports then + unusedImport.map(d => d.srcPos -> WarnTypes.Imports).toList + else + Nil + val sortedLocalDefs = + if ctx.settings.WunusedHas.locals then + unusedLocalDef.map(d => d.srcPos -> WarnTypes.LocalDefs).toList + else + Nil + val sortedPrivateDefs = + if ctx.settings.WunusedHas.privates then + unusedPrivateDef.map(d => d.srcPos -> WarnTypes.PrivateMembers).toList + else + Nil + List(sortedImp, sortedLocalDefs, sortedPrivateDefs).flatten.sortBy { s => + val pos = s._1.sourcePos (pos.line, pos.column) } - UnusedResult(sortedImp, sortedLocalDefs, sortedPrivateDefs) end UnusedData @@ -259,10 +267,6 @@ object CheckUnused: case Template case Other - case class UnusedResult( - imports: List[ImportSelector], - locals: List[tpd.ValOrDefDef], - privates: List[tpd.ValOrDefDef], - ) + type UnusedResult = List[(dotty.tools.dotc.util.SrcPos, WarnTypes)] end CheckUnused From f993782640d747041d3559dadc0e22fdb9b3a9e0 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Mon, 31 Oct 2022 14:48:27 +0100 Subject: [PATCH 12/42] Fix warning location - Fix the position of unused warning (unused locals, unused privates) to the declaration name. This avoids hidden overlapping warning --- .../src/dotty/tools/dotc/transform/CheckUnused.scala | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index a65ec16c5e57..a5a26beb346d 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -106,12 +106,11 @@ class CheckUnused extends Phase: res.foreach { s => s match case (t, WarnTypes.Imports) => - println("hey") report.warning(s"unused import", t) case (t, WarnTypes.LocalDefs) => - report.warning(s"unused local definition", t.startPos) + report.warning(s"unused local definition", t) case (t, WarnTypes.PrivateMembers) => - report.warning(s"unused private member", t.startPos) + report.warning(s"unused private member", t) } end CheckUnused @@ -246,12 +245,12 @@ object CheckUnused: Nil val sortedLocalDefs = if ctx.settings.WunusedHas.locals then - unusedLocalDef.map(d => d.srcPos -> WarnTypes.LocalDefs).toList + unusedLocalDef.map(d => d.withSpan(d.span.withEnd(d.tpt.startPos.start)) -> WarnTypes.LocalDefs).toList else Nil val sortedPrivateDefs = if ctx.settings.WunusedHas.privates then - unusedPrivateDef.map(d => d.srcPos -> WarnTypes.PrivateMembers).toList + unusedPrivateDef.map(d => d.withSpan(d.span.withEnd(d.tpt.startPos.start)) -> WarnTypes.PrivateMembers).toList else Nil List(sortedImp, sortedLocalDefs, sortedPrivateDefs).flatten.sortBy { s => From b2fd8cbe7d4a64a66009dec403063f59a7eaa723 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Sun, 6 Nov 2022 19:48:30 +0100 Subject: [PATCH 13/42] Add -Wunused:params,explicits,implicits,patvars - Add warnings for unused parmaters (explicit, implicit) - Add warnings for unused pattern variables (in match case) - Add tests suits (fatal-warnings) --- .../tools/dotc/config/ScalaSettings.scala | 8 +- .../tools/dotc/transform/CheckUnused.scala | 111 ++++++++++++++---- .../fatal-warnings/i15503d.scala | 17 +++ .../fatal-warnings/i15503e.scala | 8 ++ .../fatal-warnings/i15503f.scala | 9 ++ .../fatal-warnings/i15503g.scala | 9 ++ 6 files changed, 138 insertions(+), 24 deletions(-) create mode 100644 tests/neg-custom-args/fatal-warnings/i15503d.scala create mode 100644 tests/neg-custom-args/fatal-warnings/i15503e.scala create mode 100644 tests/neg-custom-args/fatal-warnings/i15503f.scala create mode 100644 tests/neg-custom-args/fatal-warnings/i15503g.scala diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 1832ae465ed2..4948b6d01583 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -163,7 +163,7 @@ private sealed trait WarningSettings: name = "-Wunused", helpArg = "warning", descr = "Enable or disable specific `unused` warnings", - choices = List("nowarn", "all", "imports", "locals", "privates"), + choices = List("nowarn", "all", "imports", "locals", "privates", "patvars", "explicits", "implicits", "params"), default = Nil ) object WunusedHas: @@ -171,7 +171,13 @@ private sealed trait WarningSettings: def nowarn(using Context) = allOr("nowarn") def imports(using Context) = allOr("imports") def locals(using Context) = allOr("locals") + /** -Wunused:explicits OR -Wunused:params */ + def explicits(using Context) = allOr("explicits") || allOr("params") + /** -Wunused:implicits OR -Wunused:params */ + def implicits(using Context) = allOr("implicits") || allOr("params") + def params(using Context) = allOr("params") def privates(using Context) = allOr("privates") + def patvars(using Context) = allOr("patvars") val Wconf: Setting[List[String]] = MultiStringSetting( "-Wconf", diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index a5a26beb346d..0ed04e8b1a31 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -5,9 +5,9 @@ import dotty.tools.dotc.ast.tpd.TreeTraverser import dotty.tools.dotc.ast.untpd import dotty.tools.dotc.ast.untpd.ImportSelector import dotty.tools.dotc.config.ScalaSettings -import dotty.tools.dotc.core.Contexts._ -import dotty.tools.dotc.core.Decorators.{i,em} -import dotty.tools.dotc.core.Flags.{Private, Given} +import dotty.tools.dotc.core.Contexts.* +import dotty.tools.dotc.core.Decorators.{em, i} +import dotty.tools.dotc.core.Flags.{Given, GivenVal, Param, Private} import dotty.tools.dotc.core.Phases.Phase import dotty.tools.dotc.core.StdNames import dotty.tools.dotc.report @@ -17,6 +17,8 @@ import dotty.tools.dotc.util.Property import dotty.tools.dotc.transform.CheckUnused.UnusedData.UnusedResult import dotty.tools.dotc.core.Mode +import scala.collection.mutable + /** * A compiler phase that checks for unused imports or definitions @@ -36,11 +38,14 @@ class CheckUnused extends Phase: override def isRunnable(using Context): Boolean = ctx.settings.WunusedHas.imports || ctx.settings.WunusedHas.locals || - ctx.settings.WunusedHas.privates + ctx.settings.WunusedHas.explicits || + ctx.settings.WunusedHas.implicits || + ctx.settings.WunusedHas.privates || + ctx.settings.WunusedHas.patvars override def run(using Context): Unit = val tree = ctx.compilationUnit.tpdTree - val data = UnusedData(ctx) + val data = UnusedData() val fresh = ctx.fresh.setProperty(_key, data) traverser.traverse(tree)(using fresh) reportUnused(data.getUnused) @@ -56,7 +61,7 @@ class CheckUnused extends Phase: import UnusedData.ScopeType override def traverse(tree: tpd.Tree)(using Context): Unit = tree match - case imp@Import(_, sels) => sels.foreach { s => + case imp@Import(_, sels) => sels.foreach { _ => ctx.property(_key).foreach(_.registerImport(imp)) } case ident: Ident => @@ -97,11 +102,14 @@ class CheckUnused extends Phase: case t:tpd.DefDef => ctx.property(_key).foreach(_.registerDef(t)) traverseChildren(tree) + case t: tpd.Bind => + ctx.property(_key).foreach(_.registerPatVar(t)) + traverseChildren(tree) case _ => traverseChildren(tree) } - private def reportUnused(res: UnusedData.UnusedResult)(using Context) = + private def reportUnused(res: UnusedData.UnusedResult)(using Context): Unit = import CheckUnused.WarnTypes res.foreach { s => s match @@ -109,8 +117,14 @@ class CheckUnused extends Phase: report.warning(s"unused import", t) case (t, WarnTypes.LocalDefs) => report.warning(s"unused local definition", t) + case (t, WarnTypes.ExplicitParams) => + report.warning(s"unused explicit parameter", t) + case (t, WarnTypes.ImplicitParams) => + report.warning(s"unused implicit parameter", t) case (t, WarnTypes.PrivateMembers) => report.warning(s"unused private member", t) + case (t, WarnTypes.PatVars) => + report.warning(s"unused pattern variable", t) } end CheckUnused @@ -122,7 +136,10 @@ object CheckUnused: enum WarnTypes: case Imports case LocalDefs + case ExplicitParams + case ImplicitParams case PrivateMembers + case PatVars /** * A stateful class gathering the infos on : @@ -130,26 +147,34 @@ object CheckUnused: * - definitions * - usage */ - private class UnusedData(initctx: Context): - import collection.mutable.{Set => MutSet, Map => MutMap, Stack, ListBuffer} + private class UnusedData: + import collection.mutable.{Set => MutSet, Map => MutMap, Stack => MutStack, ListBuffer} import UnusedData.ScopeType var currScope: ScopeType = ScopeType.Other /* IMPORTS */ - private val impInScope = Stack(MutMap[Int, ListBuffer[ImportSelector]]()) + private val impInScope = MutStack(MutMap[Int, ListBuffer[ImportSelector]]()) private val unusedImport = MutSet[ImportSelector]() - private val usedImports = Stack(MutSet[Int]()) + private val usedImports = MutStack(MutSet[Int]()) + + /* LOCAL DEF OR VAL / Private Def or Val / Pattern variables */ + private val localDefInScope = MutStack(MutSet[tpd.ValOrDefDef]()) + private val privateDefInScope = MutStack(MutSet[tpd.ValOrDefDef]()) + private val explicitParamInScope = MutStack(MutSet[tpd.ValOrDefDef]()) + private val implicitParamInScope = MutStack(MutSet[tpd.ValOrDefDef]()) + private val patVarsInScope = MutStack(MutSet[tpd.Bind]()) - /* LOCAL DEF OR VAL / Private Def or Val*/ - private val localDefInScope = Stack(MutSet[tpd.ValOrDefDef]()) - private val privateDefInScope = Stack(MutSet[tpd.ValOrDefDef]()) private val unusedLocalDef = ListBuffer[tpd.ValOrDefDef]() private val unusedPrivateDef = ListBuffer[tpd.ValOrDefDef]() + private val unusedExplicitParams = ListBuffer[tpd.ValOrDefDef]() + private val unusedImplicitParams = ListBuffer[tpd.ValOrDefDef]() + private val unusedPatVars = ListBuffer[tpd.Bind]() + private val usedDef = MutSet[Int]() private def isImportExclusion(sel: ImportSelector): Boolean = sel.renamed match - case ident@untpd.Ident(name) => name == StdNames.nme.WILDCARD + case untpd.Ident(name) => name == StdNames.nme.WILDCARD case _ => false /** Register the id of a found (used) symbol */ @@ -181,11 +206,19 @@ object CheckUnused: } def registerDef(valOrDef: tpd.ValOrDefDef)(using Context): Unit = - if currScope == ScopeType.Local then + if valOrDef.symbol.is(Param) then + if valOrDef.symbol.is(Given) then + implicitParamInScope.top += valOrDef + else + explicitParamInScope.top += valOrDef + else if currScope == ScopeType.Local then localDefInScope.top += valOrDef else if currScope == ScopeType.Template && valOrDef.symbol.is(Private) then privateDefInScope.top += valOrDef + def registerPatVar(patvar: tpd.Bind)(using Context): Unit = + patVarsInScope.top += patvar + /** enter a new scope */ def pushScope(): Unit = // unused imports : @@ -193,13 +226,19 @@ object CheckUnused: impInScope.push(MutMap()) // local and private defs : localDefInScope.push(MutSet()) + explicitParamInScope.push(MutSet()) + implicitParamInScope.push(MutSet()) privateDefInScope.push(MutSet()) + patVarsInScope.push(MutSet()) /** leave the current scope */ def popScope()(using Context): Unit = popScopeImport() popScopeLocalDef() + popScopeExplicitParam() + popScopeImplicitParam() popScopePrivateDef() + popScopePatVars() def popScopeImport(): Unit = val usedImp = MutSet[ImportSelector]() @@ -211,7 +250,7 @@ object CheckUnused: usedImp.addAll(value) false } - if usedImports.size > 0 then + if usedImports.nonEmpty then usedImports.top.addAll(notDefined) poppedImp.values.flatten.foreach{ sel => @@ -225,12 +264,22 @@ object CheckUnused: val unused = localDefInScope.pop().filterInPlace(d => !usedDef(d.symbol.id)) unusedLocalDef ++= unused + def popScopeExplicitParam()(using Context): Unit = + val unused = explicitParamInScope.pop().filterInPlace(d => !usedDef(d.symbol.id)) + unusedExplicitParams ++= unused + + def popScopeImplicitParam()(using Context): Unit = + val unused = implicitParamInScope.pop().filterInPlace(d => !usedDef(d.symbol.id)) + unusedImplicitParams ++= unused + def popScopePrivateDef()(using Context): Unit = - val unused = privateDefInScope.pop().filterInPlace{d => - !usedDef(d.symbol.id) - } + val unused = privateDefInScope.pop().filterInPlace(d => !usedDef(d.symbol.id)) unusedPrivateDef ++= unused + def popScopePatVars()(using Context): Unit = + val unused = patVarsInScope.pop().filterInPlace(d => !usedDef(d.symbol.id)) + unusedPatVars ++= unused + /** * Leave the scope and return a `List` of unused `ImportSelector`s * @@ -245,15 +294,30 @@ object CheckUnused: Nil val sortedLocalDefs = if ctx.settings.WunusedHas.locals then - unusedLocalDef.map(d => d.withSpan(d.span.withEnd(d.tpt.startPos.start)) -> WarnTypes.LocalDefs).toList + unusedLocalDef.map(d => d.namePos -> WarnTypes.LocalDefs).toList + else + Nil + val sortedExplicitParams = + if ctx.settings.WunusedHas.explicits then + unusedExplicitParams.map(d => d.namePos -> WarnTypes.ExplicitParams).toList + else + Nil + val sortedImplicitParams = + if ctx.settings.WunusedHas.implicits then + unusedImplicitParams.map(d => d.namePos -> WarnTypes.ImplicitParams).toList else Nil val sortedPrivateDefs = if ctx.settings.WunusedHas.privates then - unusedPrivateDef.map(d => d.withSpan(d.span.withEnd(d.tpt.startPos.start)) -> WarnTypes.PrivateMembers).toList + unusedPrivateDef.map(d => d.namePos -> WarnTypes.PrivateMembers).toList + else + Nil + val sortedPatVars = + if ctx.settings.WunusedHas.patvars then + unusedPatVars.map(d => d.namePos -> WarnTypes.PatVars).toList else Nil - List(sortedImp, sortedLocalDefs, sortedPrivateDefs).flatten.sortBy { s => + List(sortedImp, sortedLocalDefs, sortedExplicitParams, sortedImplicitParams, sortedPrivateDefs, sortedPatVars).flatten.sortBy { s => val pos = s._1.sourcePos (pos.line, pos.column) } @@ -264,6 +328,7 @@ object CheckUnused: enum ScopeType: case Local case Template + case Param case Other type UnusedResult = List[(dotty.tools.dotc.util.SrcPos, WarnTypes)] diff --git a/tests/neg-custom-args/fatal-warnings/i15503d.scala b/tests/neg-custom-args/fatal-warnings/i15503d.scala new file mode 100644 index 000000000000..09a7b57bd89f --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/i15503d.scala @@ -0,0 +1,17 @@ +// scalac: -Wunused:patvars + +sealed trait Calc +sealed trait Const extends Calc +case class Sum(a: Calc, b: Calc) extends Calc +case class S(pred: Const) extends Const +case object Z extends Const + +val a = Sum(S(S(Z)),Z) match { + case Sum(a,Z) => Z // error + case Sum(a@S(_),Z) => Z // error + case Sum(a@S(_),Z) => a // OK + case Sum(a@S(b@S(_)), Z) => a // error + case Sum(a@S(b@(S(_))), Z) => Sum(a,b) // OK + case Sum(_,_) => Z // OK + case _ => Z // OK +} diff --git a/tests/neg-custom-args/fatal-warnings/i15503e.scala b/tests/neg-custom-args/fatal-warnings/i15503e.scala new file mode 100644 index 000000000000..ed2f6908bb88 --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/i15503e.scala @@ -0,0 +1,8 @@ +// scalac: -Wunused:explicits + +def f1(a: Int) = a // OK +def f2(a: Int) = 1 // error +def f3(a: Int)(using Int) = a // OK +def f4(a: Int)(using Int) = 1 // error +def f6(a: Int)(using Int) = summon[Int] // error +def f7(a: Int)(using Int) = summon[Int] + a // OK \ No newline at end of file diff --git a/tests/neg-custom-args/fatal-warnings/i15503f.scala b/tests/neg-custom-args/fatal-warnings/i15503f.scala new file mode 100644 index 000000000000..778f90b9fd43 --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/i15503f.scala @@ -0,0 +1,9 @@ +// scalac: -Wunused:implicits + +def f1(a: Int) = a // OK +def f2(a: Int) = 1 // OK +def f3(a: Int)(using Int) = a // error +def f4(a: Int)(using Int) = 1 // error +def f6(a: Int)(using Int) = summon[Int] // OK +def f7(a: Int)(using Int) = summon[Int] + a // OK + diff --git a/tests/neg-custom-args/fatal-warnings/i15503g.scala b/tests/neg-custom-args/fatal-warnings/i15503g.scala new file mode 100644 index 000000000000..dd5c5180267b --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/i15503g.scala @@ -0,0 +1,9 @@ +// scalac: -Wunused:params + +def f1(a: Int) = a // OK +def f2(a: Int) = 1 // error +def f3(a: Int)(using Int) = a // error +def f4(a: Int)(using Int) = 1 // error // error +def f6(a: Int)(using Int) = summon[Int] // error +def f7(a: Int)(using Int) = summon[Int] + a // OK + From 8ac97b35786c62f062c07690e5373195d52c024d Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Wed, 16 Nov 2022 11:04:06 +0100 Subject: [PATCH 14/42] Fix requested changes, add new tests - Fix minor changes - Add more tests - -Wunused:nowarn does not run the CheckUnused phase fix nowarn clean useless val --- .../tools/dotc/config/ScalaSettings.scala | 19 ++-- .../tools/dotc/transform/CheckUnused.scala | 102 ++++++++---------- .../fatal-warnings/i15503h.scala | 20 ++++ .../fatal-warnings/i15503i.scala | 32 ++++++ 4 files changed, 111 insertions(+), 62 deletions(-) create mode 100644 tests/neg-custom-args/fatal-warnings/i15503h.scala create mode 100644 tests/neg-custom-args/fatal-warnings/i15503i.scala diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 4635546705da..201d02a661e7 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -163,21 +163,28 @@ private sealed trait WarningSettings: name = "-Wunused", helpArg = "warning", descr = "Enable or disable specific `unused` warnings", - choices = List("nowarn", "all", "imports", "locals", "privates", "patvars", "explicits", "implicits", "params"), + choices = List("nowarn", "all", "imports", "locals", "privates", "patvars", "explicits", "implicits", "params", "linted"), default = Nil ) object WunusedHas: def allOr(s: String)(using Context) = Wunused.value.pipe(us => us.contains("all") || us.contains(s)) def nowarn(using Context) = allOr("nowarn") - def imports(using Context) = allOr("imports") - def locals(using Context) = allOr("locals") + + def imports(using Context) = + allOr("imports") || allOr("linted") + def locals(using Context) = + allOr("locals") || allOr("linted") /** -Wunused:explicits OR -Wunused:params */ - def explicits(using Context) = allOr("explicits") || allOr("params") + def explicits(using Context) = + allOr("explicits") || allOr("params") /** -Wunused:implicits OR -Wunused:params */ - def implicits(using Context) = allOr("implicits") || allOr("params") + def implicits(using Context) = + allOr("implicits") || allOr("params") || allOr("linted") def params(using Context) = allOr("params") - def privates(using Context) = allOr("privates") + def privates(using Context) = + allOr("privates") || allOr("linted") def patvars(using Context) = allOr("patvars") + def linted(using Context) = allOr("linted") val Wconf: Setting[List[String]] = MultiStringSetting( "-Wconf", diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 0ed04e8b1a31..c9f25538e10a 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -36,12 +36,7 @@ class CheckUnused extends Phase: override def description: String = CheckUnused.description override def isRunnable(using Context): Boolean = - ctx.settings.WunusedHas.imports || - ctx.settings.WunusedHas.locals || - ctx.settings.WunusedHas.explicits || - ctx.settings.WunusedHas.implicits || - ctx.settings.WunusedHas.privates || - ctx.settings.WunusedHas.patvars + ctx.settings.Wunused.value.nonEmpty override def run(using Context): Unit = val tree = ctx.compilationUnit.tpdTree @@ -61,40 +56,17 @@ class CheckUnused extends Phase: import UnusedData.ScopeType override def traverse(tree: tpd.Tree)(using Context): Unit = tree match - case imp@Import(_, sels) => sels.foreach { _ => - ctx.property(_key).foreach(_.registerImport(imp)) - } + case imp:tpd.Import => + ctx.property(_key).foreach(_.registerImport(imp)) case ident: Ident => - val id = ident.symbol.id - ctx.property(_key).foreach(_.registerUsed(id)) + ctx.property(_key).foreach(_.registerUsed(ident.symbol)) traverseChildren(tree) case sel: Select => - val id = sel.symbol.id - ctx.property(_key).foreach(_.registerUsed(id)) - traverseChildren(tree) - case tpd.Block(_,_) => - var prev: ScopeType = ScopeType.Other - ctx.property(_key).foreach { ud => - ud.pushScope() - prev = ud.currScope - ud.currScope = ScopeType.Local - } + ctx.property(_key).foreach(_.registerUsed(sel.symbol)) traverseChildren(tree) + case _: (tpd.Block | tpd.Template) => ctx.property(_key).foreach { ud => - ud.popScope() - ud.currScope = prev - } - case tpd.Template(_,_,_,_) => - var prev: ScopeType = ScopeType.Other - ctx.property(_key).foreach { ud => - ud.pushScope() - prev = ud.currScope - ud.currScope = ScopeType.Template - } - traverseChildren(tree) - ctx.property(_key).foreach { ud => - ud.popScope() - ud.currScope = prev + ud.inNewScope(ScopeType.fromTree(tree))(traverseChildren(tree)) } case t:tpd.ValDef => ctx.property(_key).foreach(_.registerDef(t)) @@ -111,7 +83,7 @@ class CheckUnused extends Phase: private def reportUnused(res: UnusedData.UnusedResult)(using Context): Unit = import CheckUnused.WarnTypes - res.foreach { s => + res.warnings.foreach { s => s match case (t, WarnTypes.Imports) => report.warning(s"unused import", t) @@ -149,14 +121,15 @@ object CheckUnused: */ private class UnusedData: import collection.mutable.{Set => MutSet, Map => MutMap, Stack => MutStack, ListBuffer} + import dotty.tools.dotc.core.Symbols.Symbol import UnusedData.ScopeType - var currScope: ScopeType = ScopeType.Other + var currScopeType: ScopeType = ScopeType.Other /* IMPORTS */ - private val impInScope = MutStack(MutMap[Int, ListBuffer[ImportSelector]]()) + private val impInScope = MutStack(MutMap[Symbol, ListBuffer[ImportSelector]]()) private val unusedImport = MutSet[ImportSelector]() - private val usedImports = MutStack(MutSet[Int]()) + private val usedImports = MutStack(MutSet[Symbol]()) /* LOCAL DEF OR VAL / Private Def or Val / Pattern variables */ private val localDefInScope = MutStack(MutSet[tpd.ValOrDefDef]()) @@ -171,14 +144,26 @@ object CheckUnused: private val unusedImplicitParams = ListBuffer[tpd.ValOrDefDef]() private val unusedPatVars = ListBuffer[tpd.Bind]() - private val usedDef = MutSet[Int]() + private val usedDef = MutSet[Symbol]() + + /** + * Push a new Scope of the given type, executes the given Unit and + * pop it back to the original type. + */ + def inNewScope(newScope: ScopeType)(execInNewScope: => Unit)(using Context): Unit = + val prev = currScopeType + currScopeType = newScope + pushScope() + execInNewScope + popScope() + currScopeType = prev private def isImportExclusion(sel: ImportSelector): Boolean = sel.renamed match case untpd.Ident(name) => name == StdNames.nme.WILDCARD case _ => false /** Register the id of a found (used) symbol */ - def registerUsed(id: Int): Unit = + def registerUsed(id: Symbol): Unit = usedImports.top += id usedDef += id @@ -192,12 +177,11 @@ object CheckUnused: else if s.isWildcard then tree.tpe.allMembers .filter(m => m.symbol.is(Given) == s.isGiven) // given imports - .map(_.symbol.id -> s) - + .map(_.symbol -> s) else - val id = tree.tpe.member(s.name.toTermName).symbol.id - val typeId = tree.tpe.member(s.name.toTypeName).symbol.id - List(id -> s, typeId -> s) + val termSymbol = tree.tpe.member(s.name.toTermName).symbol + val typeSymbol = tree.tpe.member(s.name.toTypeName).symbol + List(termSymbol -> s, typeSymbol -> s) } entries.foreach{(id, sel) => map.get(id) match @@ -211,9 +195,9 @@ object CheckUnused: implicitParamInScope.top += valOrDef else explicitParamInScope.top += valOrDef - else if currScope == ScopeType.Local then + else if currScopeType == ScopeType.Local then localDefInScope.top += valOrDef - else if currScope == ScopeType.Template && valOrDef.symbol.is(Private) then + else if currScopeType == ScopeType.Template && valOrDef.symbol.is(Private) then privateDefInScope.top += valOrDef def registerPatVar(patvar: tpd.Bind)(using Context): Unit = @@ -261,23 +245,23 @@ object CheckUnused: } def popScopeLocalDef()(using Context): Unit = - val unused = localDefInScope.pop().filterInPlace(d => !usedDef(d.symbol.id)) + val unused = localDefInScope.pop().filterInPlace(d => !usedDef(d.symbol)) unusedLocalDef ++= unused def popScopeExplicitParam()(using Context): Unit = - val unused = explicitParamInScope.pop().filterInPlace(d => !usedDef(d.symbol.id)) + val unused = explicitParamInScope.pop().filterInPlace(d => !usedDef(d.symbol)) unusedExplicitParams ++= unused def popScopeImplicitParam()(using Context): Unit = - val unused = implicitParamInScope.pop().filterInPlace(d => !usedDef(d.symbol.id)) + val unused = implicitParamInScope.pop().filterInPlace(d => !usedDef(d.symbol)) unusedImplicitParams ++= unused def popScopePrivateDef()(using Context): Unit = - val unused = privateDefInScope.pop().filterInPlace(d => !usedDef(d.symbol.id)) + val unused = privateDefInScope.pop().filterInPlace(d => !usedDef(d.symbol)) unusedPrivateDef ++= unused def popScopePatVars()(using Context): Unit = - val unused = patVarsInScope.pop().filterInPlace(d => !usedDef(d.symbol.id)) + val unused = patVarsInScope.pop().filterInPlace(d => !usedDef(d.symbol)) unusedPatVars ++= unused /** @@ -317,10 +301,11 @@ object CheckUnused: unusedPatVars.map(d => d.namePos -> WarnTypes.PatVars).toList else Nil - List(sortedImp, sortedLocalDefs, sortedExplicitParams, sortedImplicitParams, sortedPrivateDefs, sortedPatVars).flatten.sortBy { s => + val warnings = List(sortedImp, sortedLocalDefs, sortedExplicitParams, sortedImplicitParams, sortedPrivateDefs, sortedPatVars).flatten.sortBy { s => val pos = s._1.sourcePos (pos.line, pos.column) } + UnusedResult(warnings, Nil) end UnusedData @@ -328,9 +313,14 @@ object CheckUnused: enum ScopeType: case Local case Template - case Param case Other - type UnusedResult = List[(dotty.tools.dotc.util.SrcPos, WarnTypes)] + object ScopeType: + def fromTree(tree: tpd.Tree): ScopeType = tree match + case _:tpd.Template => Template + case _:tpd.Block => Local + case _ => Other + + case class UnusedResult(warnings: List[(dotty.tools.dotc.util.SrcPos, WarnTypes)], usedImports: List[(tpd.Import, untpd.ImportSelector)]) end CheckUnused diff --git a/tests/neg-custom-args/fatal-warnings/i15503h.scala b/tests/neg-custom-args/fatal-warnings/i15503h.scala new file mode 100644 index 000000000000..c0a25f703dcd --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/i15503h.scala @@ -0,0 +1,20 @@ +// scalac: -Wunused:linted + +import collection.mutable.Set // error + +class A { + private val a = 1 // error + val b = 2 // OK + + private def c = 2 // error + def d(using x:Int): Int = 1 // error + def e(x: Int) = 1 // OK + def f = + val x = 1 // error + def f = 2 // error + 3 + + def g(x: Int): Int = x match + case x:1 => 0 // OK + case _ => 1 +} \ No newline at end of file diff --git a/tests/neg-custom-args/fatal-warnings/i15503i.scala b/tests/neg-custom-args/fatal-warnings/i15503i.scala new file mode 100644 index 000000000000..a94cd172b942 --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/i15503i.scala @@ -0,0 +1,32 @@ +// scalac: -Wunused:all + +import collection.mutable.{Map => MutMap} // error +import collection.mutable.Set // error + +class A { + import collection.mutable.{Map => MutMap} // OK + private val a = 1 // error + val b = 2 // OK + val someMap = MutMap() + + private def c1 = 2 // error + private def c2 = 2 // OK + def c3 = c2 + + def d1(using x:Int): Int = 1 // error + def d2(using x:Int): Int = x // OK + + def e1(x: Int) = 1 // error + def e2(x: Int) = x // OK + def f = + val x = 1 // error + def f = 2 // error + val y = 3 // OK + def g = 4 // OK + y + g + + def g(x: Int): Int = x match + case x:1 => 0 // error + case x:2 => x // OK + case _ => 1 // OK +} \ No newline at end of file From 892621bd3cb5e5c2598e652350f0be204d6607e5 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Thu, 17 Nov 2022 15:38:57 +0100 Subject: [PATCH 15/42] Annotation usage, allow unsed Self, add tests - Register annotation usage, so (i.e. @tailrec) is not reported - Allow unsed self name (`self:A =>` members) - Add tests suggested in the PR discussion thread --- .../tools/dotc/transform/CheckUnused.scala | 82 +++++++++++-------- .../fatal-warnings/i15503a.scala | 55 +++++++++++++ .../fatal-warnings/i15503c.scala | 2 + 3 files changed, 105 insertions(+), 34 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index c9f25538e10a..a1e07e0465f6 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -7,7 +7,7 @@ import dotty.tools.dotc.ast.untpd.ImportSelector import dotty.tools.dotc.config.ScalaSettings import dotty.tools.dotc.core.Contexts.* import dotty.tools.dotc.core.Decorators.{em, i} -import dotty.tools.dotc.core.Flags.{Given, GivenVal, Param, Private} +import dotty.tools.dotc.core.Flags.{Given, GivenVal, Param, Private, SelfName} import dotty.tools.dotc.core.Phases.Phase import dotty.tools.dotc.core.StdNames import dotty.tools.dotc.report @@ -55,29 +55,32 @@ class CheckUnused extends Phase: import tpd._ import UnusedData.ScopeType - override def traverse(tree: tpd.Tree)(using Context): Unit = tree match - case imp:tpd.Import => - ctx.property(_key).foreach(_.registerImport(imp)) - case ident: Ident => - ctx.property(_key).foreach(_.registerUsed(ident.symbol)) - traverseChildren(tree) - case sel: Select => - ctx.property(_key).foreach(_.registerUsed(sel.symbol)) - traverseChildren(tree) - case _: (tpd.Block | tpd.Template) => - ctx.property(_key).foreach { ud => - ud.inNewScope(ScopeType.fromTree(tree))(traverseChildren(tree)) - } - case t:tpd.ValDef => - ctx.property(_key).foreach(_.registerDef(t)) - traverseChildren(tree) - case t:tpd.DefDef => - ctx.property(_key).foreach(_.registerDef(t)) - traverseChildren(tree) - case t: tpd.Bind => - ctx.property(_key).foreach(_.registerPatVar(t)) - traverseChildren(tree) - case _ => traverseChildren(tree) + override def traverse(tree: tpd.Tree)(using Context): Unit = + if tree.isDef then // register the annotations for usage + ctx.property(_key).foreach(_.registerUsedAnnotation(tree.symbol)) + tree match + case imp:tpd.Import => + ctx.property(_key).foreach(_.registerImport(imp)) + case ident: Ident => + ctx.property(_key).foreach(_.registerUsed(ident.symbol)) + traverseChildren(tree) + case sel: Select => + ctx.property(_key).foreach(_.registerUsed(sel.symbol)) + traverseChildren(tree) + case _: (tpd.Block | tpd.Template) => + ctx.property(_key).foreach { ud => + ud.inNewScope(ScopeType.fromTree(tree))(traverseChildren(tree)) + } + case t:tpd.ValDef => + ctx.property(_key).foreach(_.registerDef(t)) + traverseChildren(tree) + case t:tpd.DefDef => + ctx.property(_key).foreach(_.registerDef(t)) + traverseChildren(tree) + case t: tpd.Bind => + ctx.property(_key).foreach(_.registerPatVar(t)) + traverseChildren(tree) + case _ => traverseChildren(tree) } @@ -162,10 +165,21 @@ object CheckUnused: case untpd.Ident(name) => name == StdNames.nme.WILDCARD case _ => false - /** Register the id of a found (used) symbol */ - def registerUsed(id: Symbol): Unit = - usedImports.top += id - usedDef += id + + /** Register all annotations of this symbol's denotation */ + def registerUsedAnnotation(sym: Symbol)(using Context): Unit = + val annotSym = sym.denot.annotations.map(_.symbol) + registerUsed(annotSym) + + /** Register a found (used) symbol */ + def registerUsed(sym: Symbol): Unit = + usedImports.top += sym + usedDef += sym + + /** Register a list of found (used) symbols */ + def registerUsed(sym: Seq[Symbol]): Unit = + usedImports.top ++= sym + usedDef ++= sym /** Register an import */ def registerImport(imp: tpd.Import)(using Context): Unit = @@ -183,9 +197,9 @@ object CheckUnused: val typeSymbol = tree.tpe.member(s.name.toTypeName).symbol List(termSymbol -> s, typeSymbol -> s) } - entries.foreach{(id, sel) => - map.get(id) match - case None => map.put(id, ListBuffer(sel)) + entries.foreach{(sym, sel) => + map.get(sym) match + case None => map.put(sym, ListBuffer(sel)) case Some(value) => value += sel } @@ -197,7 +211,7 @@ object CheckUnused: explicitParamInScope.top += valOrDef else if currScopeType == ScopeType.Local then localDefInScope.top += valOrDef - else if currScopeType == ScopeType.Template && valOrDef.symbol.is(Private) then + else if currScopeType == ScopeType.Template && valOrDef.symbol.is(Private, butNot = SelfName) then privateDefInScope.top += valOrDef def registerPatVar(patvar: tpd.Bind)(using Context): Unit = @@ -227,8 +241,8 @@ object CheckUnused: def popScopeImport(): Unit = val usedImp = MutSet[ImportSelector]() val poppedImp = impInScope.pop() - val notDefined = usedImports.pop.filter{id => - poppedImp.remove(id) match + val notDefined = usedImports.pop.filter{sym => + poppedImp.remove(sym) match case None => true case Some(value) => usedImp.addAll(value) diff --git a/tests/neg-custom-args/fatal-warnings/i15503a.scala b/tests/neg-custom-args/fatal-warnings/i15503a.scala index 40eddf6c5e99..9124b6e55bb6 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503a.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503a.scala @@ -99,3 +99,58 @@ object SomeGivenImports: given Int = 0 given String = "foo" +/* BEGIN : Check on packages*/ +package p { + class C +} + +package p { + import p._ // error + package q { + class U { + def f = new C + } + } +} +/* END : Check on packages*/ + +/* BEGIN : tests on meta-language features */ +object TestGivenCoversionScala2: + /* note: scala3 Conversion[U,T] do not require an import */ + import language.implicitConversions // OK + + implicit def doubleToInt(d:Double):Int = d.toInt + + def idInt(i:Int):Int = i + val someInt = idInt(1.0) + +object TestTailrecImport: + import annotation.tailrec // OK + @tailrec + def fac(x:Int, acc:Int = 1): Int = + if x == 0 then acc else fac(x - 1, acc * x) +/* END : tests on meta-language features */ + +/* BEGIN : tests on given import order */ +object GivenImportOrderAtoB: + class X + class Y extends X + object A { implicit val x: X = new X } + object B { implicit val y: Y = new Y } + class C { + import A._ // error + import B._ + def t = implicitly[X] + } + +object GivenImportOrderBtoA: + class X + class Y extends X + object A { implicit val x: X = new X } + object B { implicit val y: Y = new Y } + class C { + import B._ + import A._ // error + def t = implicitly[X] + } +/* END : tests on given import order */ \ No newline at end of file diff --git a/tests/neg-custom-args/fatal-warnings/i15503c.scala b/tests/neg-custom-args/fatal-warnings/i15503c.scala index 8af0dc32d166..efaa7d389c08 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503c.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503c.scala @@ -1,5 +1,7 @@ // scalac: -Wunused:privates +trait C class A: + self: C => // OK class B: private[A] val a = 1 // OK private[B] val b = 1 // OK From 6d8d20d047d736810e249a92ee9e354605918c34 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Thu, 17 Nov 2022 22:14:59 +0100 Subject: [PATCH 16/42] Refactor CheckUnused, fix pkg false negative - Refactor the sets of defs and usage symbols. This results in less objects created and less push-pop scope. - Fix false negative for imports in the same package --- .../tools/dotc/transform/CheckUnused.scala | 196 ++++++++---------- 1 file changed, 83 insertions(+), 113 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index a1e07e0465f6..fd294fcdb7e4 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -17,7 +17,6 @@ import dotty.tools.dotc.util.Property import dotty.tools.dotc.transform.CheckUnused.UnusedData.UnusedResult import dotty.tools.dotc.core.Mode -import scala.collection.mutable /** @@ -56,6 +55,7 @@ class CheckUnused extends Phase: import UnusedData.ScopeType override def traverse(tree: tpd.Tree)(using Context): Unit = + val newCtx = if tree.symbol.exists then ctx.withOwner(tree.symbol) else ctx if tree.isDef then // register the annotations for usage ctx.property(_key).foreach(_.registerUsedAnnotation(tree.symbol)) tree match @@ -63,24 +63,25 @@ class CheckUnused extends Phase: ctx.property(_key).foreach(_.registerImport(imp)) case ident: Ident => ctx.property(_key).foreach(_.registerUsed(ident.symbol)) - traverseChildren(tree) + traverseChildren(tree)(using newCtx) case sel: Select => ctx.property(_key).foreach(_.registerUsed(sel.symbol)) - traverseChildren(tree) + traverseChildren(tree)(using newCtx) case _: (tpd.Block | tpd.Template) => ctx.property(_key).foreach { ud => - ud.inNewScope(ScopeType.fromTree(tree))(traverseChildren(tree)) + ud.inNewScope(ScopeType.fromTree(tree))(traverseChildren(tree)(using newCtx)) } case t:tpd.ValDef => ctx.property(_key).foreach(_.registerDef(t)) - traverseChildren(tree) + traverseChildren(tree)(using newCtx) case t:tpd.DefDef => ctx.property(_key).foreach(_.registerDef(t)) - traverseChildren(tree) + traverseChildren(tree)(using newCtx) case t: tpd.Bind => ctx.property(_key).foreach(_.registerPatVar(t)) - traverseChildren(tree) - case _ => traverseChildren(tree) + traverseChildren(tree)(using newCtx) + case _ => + traverseChildren(tree)(using newCtx) } @@ -123,29 +124,29 @@ object CheckUnused: * - usage */ private class UnusedData: - import collection.mutable.{Set => MutSet, Map => MutMap, Stack => MutStack, ListBuffer} + import collection.mutable.{Set => MutSet, Map => MutMap, Stack => MutStack} import dotty.tools.dotc.core.Symbols.Symbol import UnusedData.ScopeType var currScopeType: ScopeType = ScopeType.Other /* IMPORTS */ - private val impInScope = MutStack(MutMap[Symbol, ListBuffer[ImportSelector]]()) + private val impInScope = MutStack(MutSet[tpd.Import]()) + private val usedInScope = MutStack(MutSet[(Symbol,Boolean)]()) private val unusedImport = MutSet[ImportSelector]() - private val usedImports = MutStack(MutSet[Symbol]()) /* LOCAL DEF OR VAL / Private Def or Val / Pattern variables */ - private val localDefInScope = MutStack(MutSet[tpd.ValOrDefDef]()) - private val privateDefInScope = MutStack(MutSet[tpd.ValOrDefDef]()) - private val explicitParamInScope = MutStack(MutSet[tpd.ValOrDefDef]()) - private val implicitParamInScope = MutStack(MutSet[tpd.ValOrDefDef]()) - private val patVarsInScope = MutStack(MutSet[tpd.Bind]()) - - private val unusedLocalDef = ListBuffer[tpd.ValOrDefDef]() - private val unusedPrivateDef = ListBuffer[tpd.ValOrDefDef]() - private val unusedExplicitParams = ListBuffer[tpd.ValOrDefDef]() - private val unusedImplicitParams = ListBuffer[tpd.ValOrDefDef]() - private val unusedPatVars = ListBuffer[tpd.Bind]() + private val localDefInScope = MutSet[tpd.ValOrDefDef]() + private val privateDefInScope = MutSet[tpd.ValOrDefDef]() + private val explicitParamInScope = MutSet[tpd.ValOrDefDef]() + private val implicitParamInScope = MutSet[tpd.ValOrDefDef]() + private val patVarsInScope = MutSet[tpd.Bind]() + + private val unusedLocalDef = MutSet[tpd.ValOrDefDef]() + private val unusedPrivateDef = MutSet[tpd.ValOrDefDef]() + private val unusedExplicitParams = MutSet[tpd.ValOrDefDef]() + private val unusedImplicitParams = MutSet[tpd.ValOrDefDef]() + private val unusedPatVars = MutSet[tpd.Bind]() private val usedDef = MutSet[Symbol]() @@ -161,122 +162,66 @@ object CheckUnused: popScope() currScopeType = prev - private def isImportExclusion(sel: ImportSelector): Boolean = sel.renamed match - case untpd.Ident(name) => name == StdNames.nme.WILDCARD - case _ => false - - /** Register all annotations of this symbol's denotation */ def registerUsedAnnotation(sym: Symbol)(using Context): Unit = val annotSym = sym.denot.annotations.map(_.symbol) registerUsed(annotSym) /** Register a found (used) symbol */ - def registerUsed(sym: Symbol): Unit = - usedImports.top += sym + def registerUsed(sym: Symbol)(using Context): Unit = + usedInScope.top += sym -> sym.isAccessibleAsIdent usedDef += sym /** Register a list of found (used) symbols */ - def registerUsed(sym: Seq[Symbol]): Unit = - usedImports.top ++= sym - usedDef ++= sym + def registerUsed(syms: Seq[Symbol])(using Context): Unit = + usedInScope.top ++= syms.map(s => s -> s.isAccessibleAsIdent) + usedDef ++= syms /** Register an import */ def registerImport(imp: tpd.Import)(using Context): Unit = - val tpd.Import(tree, sels) = imp - val map = impInScope.top - val entries = sels.flatMap{ s => - if isImportExclusion(s) then - Nil // ignore exclusion import - else if s.isWildcard then - tree.tpe.allMembers - .filter(m => m.symbol.is(Given) == s.isGiven) // given imports - .map(_.symbol -> s) - else - val termSymbol = tree.tpe.member(s.name.toTermName).symbol - val typeSymbol = tree.tpe.member(s.name.toTypeName).symbol - List(termSymbol -> s, typeSymbol -> s) - } - entries.foreach{(sym, sel) => - map.get(sym) match - case None => map.put(sym, ListBuffer(sel)) - case Some(value) => value += sel - } + impInScope.top += imp + unusedImport ++= imp.selectors.filter(s => !isImportExclusion(s)) def registerDef(valOrDef: tpd.ValOrDefDef)(using Context): Unit = if valOrDef.symbol.is(Param) then if valOrDef.symbol.is(Given) then - implicitParamInScope.top += valOrDef + implicitParamInScope += valOrDef else - explicitParamInScope.top += valOrDef + explicitParamInScope += valOrDef else if currScopeType == ScopeType.Local then - localDefInScope.top += valOrDef + localDefInScope += valOrDef else if currScopeType == ScopeType.Template && valOrDef.symbol.is(Private, butNot = SelfName) then - privateDefInScope.top += valOrDef + privateDefInScope += valOrDef def registerPatVar(patvar: tpd.Bind)(using Context): Unit = - patVarsInScope.top += patvar + patVarsInScope += patvar /** enter a new scope */ def pushScope(): Unit = // unused imports : - usedImports.push(MutSet()) - impInScope.push(MutMap()) - // local and private defs : - localDefInScope.push(MutSet()) - explicitParamInScope.push(MutSet()) - implicitParamInScope.push(MutSet()) - privateDefInScope.push(MutSet()) - patVarsInScope.push(MutSet()) + impInScope.push(MutSet()) + usedInScope.push(MutSet()) /** leave the current scope */ def popScope()(using Context): Unit = popScopeImport() - popScopeLocalDef() - popScopeExplicitParam() - popScopeImplicitParam() - popScopePrivateDef() - popScopePatVars() - - def popScopeImport(): Unit = - val usedImp = MutSet[ImportSelector]() - val poppedImp = impInScope.pop() - val notDefined = usedImports.pop.filter{sym => - poppedImp.remove(sym) match - case None => true - case Some(value) => - usedImp.addAll(value) - false - } - if usedImports.nonEmpty then - usedImports.top.addAll(notDefined) - - poppedImp.values.flatten.foreach{ sel => - // If **any** of the entities used by the import is used, - // do not add to the `unused` Set - if !usedImp(sel) then - unusedImport += sel - } - - def popScopeLocalDef()(using Context): Unit = - val unused = localDefInScope.pop().filterInPlace(d => !usedDef(d.symbol)) - unusedLocalDef ++= unused - def popScopeExplicitParam()(using Context): Unit = - val unused = explicitParamInScope.pop().filterInPlace(d => !usedDef(d.symbol)) - unusedExplicitParams ++= unused - - def popScopeImplicitParam()(using Context): Unit = - val unused = implicitParamInScope.pop().filterInPlace(d => !usedDef(d.symbol)) - unusedImplicitParams ++= unused - - def popScopePrivateDef()(using Context): Unit = - val unused = privateDefInScope.pop().filterInPlace(d => !usedDef(d.symbol)) - unusedPrivateDef ++= unused - - def popScopePatVars()(using Context): Unit = - val unused = patVarsInScope.pop().filterInPlace(d => !usedDef(d.symbol)) - unusedPatVars ++= unused + def popScopeImport()(using Context): Unit = + val used = usedInScope.pop().toSet + val imports = impInScope.pop().toSet + val kept = used.filter { t => + val (sym, isAccessible) = t + !imports.exists { imp => + sym.isInImport(imp, isAccessible) match + case None => false + case Some(sel) => + unusedImport -= sel + true + } + } + if usedInScope.nonEmpty then + usedInScope.top ++= kept + usedDef ++= used.map(_._1) /** * Leave the scope and return a `List` of unused `ImportSelector`s @@ -292,27 +237,27 @@ object CheckUnused: Nil val sortedLocalDefs = if ctx.settings.WunusedHas.locals then - unusedLocalDef.map(d => d.namePos -> WarnTypes.LocalDefs).toList + localDefInScope.filter(d => !usedDef(d.symbol)).map(d => d.namePos -> WarnTypes.LocalDefs).toList else Nil val sortedExplicitParams = if ctx.settings.WunusedHas.explicits then - unusedExplicitParams.map(d => d.namePos -> WarnTypes.ExplicitParams).toList + explicitParamInScope.filter(d => !usedDef(d.symbol)).map(d => d.namePos -> WarnTypes.ExplicitParams).toList else Nil val sortedImplicitParams = if ctx.settings.WunusedHas.implicits then - unusedImplicitParams.map(d => d.namePos -> WarnTypes.ImplicitParams).toList + implicitParamInScope.filter(d => !usedDef(d.symbol)).map(d => d.namePos -> WarnTypes.ImplicitParams).toList else Nil val sortedPrivateDefs = if ctx.settings.WunusedHas.privates then - unusedPrivateDef.map(d => d.namePos -> WarnTypes.PrivateMembers).toList + privateDefInScope.filter(d => !usedDef(d.symbol)).map(d => d.namePos -> WarnTypes.PrivateMembers).toList else Nil val sortedPatVars = if ctx.settings.WunusedHas.patvars then - unusedPatVars.map(d => d.namePos -> WarnTypes.PatVars).toList + patVarsInScope.filter(d => !usedDef(d.symbol)).map(d => d.namePos -> WarnTypes.PatVars).toList else Nil val warnings = List(sortedImp, sortedLocalDefs, sortedExplicitParams, sortedImplicitParams, sortedPrivateDefs, sortedPatVars).flatten.sortBy { s => @@ -321,6 +266,31 @@ object CheckUnused: } UnusedResult(warnings, Nil) + //============================ HELPERS ==================================== + + private def isImportExclusion(sel: ImportSelector): Boolean = sel.renamed match + case untpd.Ident(name) => name == StdNames.nme.WILDCARD + case _ => false + extension (sym: Symbol) + def isAccessibleAsIdent(using Context): Boolean = + sym.exists && + ctx.outersIterator.exists{ c => + c.owner == sym.owner + || sym.owner.isClass && c.owner.isClass + && c.owner.thisType.baseClasses.contains(sym.owner) + && c.owner.thisType.member(sym.name).alternatives.contains(sym) + } + def isInImport(imp: tpd.Import, isAccessible: Boolean)(using Context): Option[ImportSelector] = + val tpd.Import(qual, sels) = imp + val sameQualType = + qual.tpe.member(sym.name.toTermName).symbol == sym || + qual.tpe.member(sym.name.toTypeName).symbol == sym + def selector = sels.find(sel => sel.name.toTermName == sym.name || sel.name.toTypeName == sym.name) + def wildcard = sels.find(sel => sel.isWildcard && (sym.is(Given) == sel.isGiven)) + if sameQualType && !isAccessible then + selector.orElse(wildcard) + else + None end UnusedData object UnusedData: From 412412cee9141f0b869d97ce92f79c4a0dd36f55 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Thu, 17 Nov 2022 23:08:43 +0100 Subject: [PATCH 17/42] No -Wunused:imports for language imports --- compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index fd294fcdb7e4..cb8db24cc647 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -35,7 +35,8 @@ class CheckUnused extends Phase: override def description: String = CheckUnused.description override def isRunnable(using Context): Boolean = - ctx.settings.Wunused.value.nonEmpty + ctx.settings.Wunused.value.nonEmpty && + !ctx.isJava override def run(using Context): Unit = val tree = ctx.compilationUnit.tpdTree @@ -179,8 +180,9 @@ object CheckUnused: /** Register an import */ def registerImport(imp: tpd.Import)(using Context): Unit = - impInScope.top += imp - unusedImport ++= imp.selectors.filter(s => !isImportExclusion(s)) + if !tpd.languageImport(imp.expr).nonEmpty then + impInScope.top += imp + unusedImport ++= imp.selectors.filter(s => !isImportExclusion(s)) def registerDef(valOrDef: tpd.ValOrDefDef)(using Context): Unit = if valOrDef.symbol.is(Param) then From f354b9616a62be2aa1caa77e66bfd05742ed0f5c Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Fri, 18 Nov 2022 11:23:37 +0100 Subject: [PATCH 18/42] Clean CheckUnused phase code, add doc+comments --- .../tools/dotc/transform/CheckUnused.scala | 68 +++++++++++++------ 1 file changed, 47 insertions(+), 21 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index cb8db24cc647..72ad07aa1a86 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -51,40 +51,42 @@ class CheckUnused extends Phase: * It traverse the tree the tree and gather the data in the * corresponding context property */ - private def traverser = new TreeTraverser { + private def traverser = new TreeTraverser: import tpd._ import UnusedData.ScopeType + /* Register every imports, definition and usage */ override def traverse(tree: tpd.Tree)(using Context): Unit = + val unusedDataApply = ctx.property(_key).foreach val newCtx = if tree.symbol.exists then ctx.withOwner(tree.symbol) else ctx if tree.isDef then // register the annotations for usage - ctx.property(_key).foreach(_.registerUsedAnnotation(tree.symbol)) + unusedDataApply(_.registerUsedAnnotation(tree.symbol)) tree match case imp:tpd.Import => - ctx.property(_key).foreach(_.registerImport(imp)) + unusedDataApply(_.registerImport(imp)) case ident: Ident => - ctx.property(_key).foreach(_.registerUsed(ident.symbol)) + unusedDataApply(_.registerUsed(ident.symbol)) traverseChildren(tree)(using newCtx) case sel: Select => - ctx.property(_key).foreach(_.registerUsed(sel.symbol)) + unusedDataApply(_.registerUsed(sel.symbol)) traverseChildren(tree)(using newCtx) case _: (tpd.Block | tpd.Template) => - ctx.property(_key).foreach { ud => + unusedDataApply { ud => ud.inNewScope(ScopeType.fromTree(tree))(traverseChildren(tree)(using newCtx)) } case t:tpd.ValDef => - ctx.property(_key).foreach(_.registerDef(t)) + unusedDataApply(_.registerDef(t)) traverseChildren(tree)(using newCtx) case t:tpd.DefDef => - ctx.property(_key).foreach(_.registerDef(t)) + unusedDataApply(_.registerDef(t)) traverseChildren(tree)(using newCtx) case t: tpd.Bind => - ctx.property(_key).foreach(_.registerPatVar(t)) + unusedDataApply(_.registerPatVar(t)) traverseChildren(tree)(using newCtx) case _ => traverseChildren(tree)(using newCtx) - - } + end traverse + end traverser private def reportUnused(res: UnusedData.UnusedResult)(using Context): Unit = import CheckUnused.WarnTypes @@ -129,11 +131,19 @@ object CheckUnused: import dotty.tools.dotc.core.Symbols.Symbol import UnusedData.ScopeType + /** The current scope during the tree traversal */ var currScopeType: ScopeType = ScopeType.Other /* IMPORTS */ private val impInScope = MutStack(MutSet[tpd.Import]()) + /** + * We store the symbol along with their accessibility without import. + * Accessibility to their definition in outer context/scope + * + * See the `isAccessibleAsIdent` extension method below in the file + */ private val usedInScope = MutStack(MutSet[(Symbol,Boolean)]()) + /* unused import collected during traversal */ private val unusedImport = MutSet[ImportSelector]() /* LOCAL DEF OR VAL / Private Def or Val / Pattern variables */ @@ -143,12 +153,14 @@ object CheckUnused: private val implicitParamInScope = MutSet[tpd.ValOrDefDef]() private val patVarsInScope = MutSet[tpd.Bind]() + /* Unused collection collected at the end */ private val unusedLocalDef = MutSet[tpd.ValOrDefDef]() private val unusedPrivateDef = MutSet[tpd.ValOrDefDef]() private val unusedExplicitParams = MutSet[tpd.ValOrDefDef]() private val unusedImplicitParams = MutSet[tpd.ValOrDefDef]() private val unusedPatVars = MutSet[tpd.Bind]() + /** All used symbols */ private val usedDef = MutSet[Symbol]() /** @@ -184,6 +196,7 @@ object CheckUnused: impInScope.top += imp unusedImport ++= imp.selectors.filter(s => !isImportExclusion(s)) + /** Register (or not) some `val` or `def` according to the context, scope and flags */ def registerDef(valOrDef: tpd.ValOrDefDef)(using Context): Unit = if valOrDef.symbol.is(Param) then if valOrDef.symbol.is(Given) then @@ -195,6 +208,7 @@ object CheckUnused: else if currScopeType == ScopeType.Template && valOrDef.symbol.is(Private, butNot = SelfName) then privateDefInScope += valOrDef + /** Register pattern variable */ def registerPatVar(patvar: tpd.Bind)(using Context): Unit = patVarsInScope += patvar @@ -204,15 +218,19 @@ object CheckUnused: impInScope.push(MutSet()) usedInScope.push(MutSet()) - /** leave the current scope */ + /** + * leave the current scope and do : + * + * - If there are imports in this scope check for unused ones + */ def popScope()(using Context): Unit = - popScopeImport() - - def popScopeImport()(using Context): Unit = + // used symbol in this scope val used = usedInScope.pop().toSet + // used imports in this scope val imports = impInScope.pop().toSet val kept = used.filter { t => val (sym, isAccessible) = t + // keep the symbol for outer scope, if it matches **no** import !imports.exists { imp => sym.isInImport(imp, isAccessible) match case None => false @@ -221,9 +239,14 @@ object CheckUnused: true } } + // if there's an outer scope if usedInScope.nonEmpty then + // we keep the symbols not referencing an import in this scope + // as it can be the only reference to an outer import usedInScope.top ++= kept + // register usage in this scope for other warnings at the end of the phase usedDef ++= used.map(_._1) + end popScope /** * Leave the scope and return a `List` of unused `ImportSelector`s @@ -267,13 +290,15 @@ object CheckUnused: (pos.line, pos.column) } UnusedResult(warnings, Nil) - + end getUnused //============================ HELPERS ==================================== private def isImportExclusion(sel: ImportSelector): Boolean = sel.renamed match case untpd.Ident(name) => name == StdNames.nme.WILDCARD case _ => false + extension (sym: Symbol) + /** is accessible without import in current context */ def isAccessibleAsIdent(using Context): Boolean = sym.exists && ctx.outersIterator.exists{ c => @@ -282,17 +307,18 @@ object CheckUnused: && c.owner.thisType.baseClasses.contains(sym.owner) && c.owner.thisType.member(sym.name).alternatives.contains(sym) } + + /** Given an import and accessibility, return an option of selector that match import<->symbol */ def isInImport(imp: tpd.Import, isAccessible: Boolean)(using Context): Option[ImportSelector] = val tpd.Import(qual, sels) = imp - val sameQualType = - qual.tpe.member(sym.name.toTermName).symbol == sym || - qual.tpe.member(sym.name.toTypeName).symbol == sym + val qualHasSymbol = qual.tpe.member(sym.name).symbol == sym def selector = sels.find(sel => sel.name.toTermName == sym.name || sel.name.toTypeName == sym.name) def wildcard = sels.find(sel => sel.isWildcard && (sym.is(Given) == sel.isGiven)) - if sameQualType && !isAccessible then - selector.orElse(wildcard) + if qualHasSymbol && !isAccessible then + selector.orElse(wildcard) // selector with name or wildcard (or given) else None + end UnusedData object UnusedData: From d41c6ddd1ee7f2a2252877c66e7419853253da7f Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Wed, 23 Nov 2022 16:52:48 +0100 Subject: [PATCH 19/42] Remove report unused implicit imports - Named import pointing to an implicit - Given imports `import foo.bar.given` - Add new tests, remove the ones on implicit reporting --- .../tools/dotc/transform/CheckUnused.scala | 10 +++- .../fatal-warnings/i15503a.scala | 58 ++++++++++++++----- 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 72ad07aa1a86..6fa4f2500cf7 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -192,9 +192,12 @@ object CheckUnused: /** Register an import */ def registerImport(imp: tpd.Import)(using Context): Unit = - if !tpd.languageImport(imp.expr).nonEmpty then + val tpd.Import(qual, selectors) = imp + if !tpd.languageImport(qual).nonEmpty then impInScope.top += imp - unusedImport ++= imp.selectors.filter(s => !isImportExclusion(s)) + unusedImport ++= selectors.filter{ s => + !isImportExclusion(s) && !s.isGiven && !isSelectorOnAGiven(qual, s) + } /** Register (or not) some `val` or `def` according to the context, scope and flags */ def registerDef(valOrDef: tpd.ValOrDefDef)(using Context): Unit = @@ -293,6 +296,9 @@ object CheckUnused: end getUnused //============================ HELPERS ==================================== + private def isSelectorOnAGiven(qual: tpd.Tree, sel: ImportSelector)(using Context): Boolean = + qual.tpe.member(sel.name).alternatives.exists(_.symbol.is(Given)) + private def isImportExclusion(sel: ImportSelector): Boolean = sel.renamed match case untpd.Ident(name) => name == StdNames.nme.WILDCARD case _ => false diff --git a/tests/neg-custom-args/fatal-warnings/i15503a.scala b/tests/neg-custom-args/fatal-warnings/i15503a.scala index 9124b6e55bb6..9f4610af0d97 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503a.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503a.scala @@ -31,14 +31,6 @@ object FooNested: object Nested: def hello = Set() -object FooGivenUnused: - import SomeGivenImports.given // error - -object FooGiven: - import SomeGivenImports.given // OK - import SomeGivenImports._ // error - - val foo = summon[Int] /** * Import used as type name are considered @@ -92,12 +84,6 @@ object IgnoreExclusion: def check = val a = Set(1) val b = Map(1 -> 2) -/** - * Some given values for the test - */ -object SomeGivenImports: - given Int = 0 - given String = "foo" /* BEGIN : Check on packages*/ package p { @@ -115,9 +101,10 @@ package p { /* END : Check on packages*/ /* BEGIN : tests on meta-language features */ -object TestGivenCoversionScala2: +object TestLanguageImportAreIgnored: /* note: scala3 Conversion[U,T] do not require an import */ import language.implicitConversions // OK + import language._ // OK implicit def doubleToInt(d:Double):Int = d.toInt @@ -153,4 +140,43 @@ object GivenImportOrderBtoA: import A._ // error def t = implicitly[X] } -/* END : tests on given import order */ \ No newline at end of file +/* END : tests on given import order */ + +/* + * Advanced tests on given imports meta-programming + * + * - Currently also tests that no imported implicits are reported + */ + +package summoninlineconflict: + package lib: + trait A + trait B + trait C + trait X + + given willBeUnused: (A & X) = new A with X {} + given willBeUsed: (A & B) = new A with B {} + given notUsedAtAll: Int = 0 + + package use: + import lib.{A, B, C, willBeUnused, willBeUsed, notUsedAtAll} // OK + import compiletime.summonInline // OK + + transparent inline given conflictInside: C = + summonInline[A] + new {} + + transparent inline given potentialConflict: C = + summonInline[B] + new {} + + val b: B = summon[B] + val c: C = summon[C] + +package unusedgivensimports: + package foo: + given Int = 0 + + package bar: + import foo.given // OK \ No newline at end of file From 95c8a40963aea1893e2f30069f57323a97cb8cbc Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Thu, 24 Nov 2022 15:06:45 +0100 Subject: [PATCH 20/42] Revert to unused implicit check, also synthetics - Check implicit and given imports - Adapt check to some synthetic (@main, package object) - Add new tests to support these features --- .../tools/dotc/transform/CheckUnused.scala | 51 +++++--- .../fatal-warnings/i15503a.scala | 112 ++++++++++-------- .../fatal-warnings/i15503e.scala | 17 ++- 3 files changed, 113 insertions(+), 67 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 6fa4f2500cf7..d390cec316d4 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -7,7 +7,7 @@ import dotty.tools.dotc.ast.untpd.ImportSelector import dotty.tools.dotc.config.ScalaSettings import dotty.tools.dotc.core.Contexts.* import dotty.tools.dotc.core.Decorators.{em, i} -import dotty.tools.dotc.core.Flags.{Given, GivenVal, Param, Private, SelfName} +import dotty.tools.dotc.core.Flags.{Given, Implicit, GivenOrImplicit, Param, Private, SelfName, Synthetic} import dotty.tools.dotc.core.Phases.Phase import dotty.tools.dotc.core.StdNames import dotty.tools.dotc.report @@ -182,27 +182,25 @@ object CheckUnused: /** Register a found (used) symbol */ def registerUsed(sym: Symbol)(using Context): Unit = - usedInScope.top += sym -> sym.isAccessibleAsIdent - usedDef += sym + if !isConstructorOfSynth(sym) then + usedInScope.top += sym -> sym.isAccessibleAsIdent + usedDef += sym /** Register a list of found (used) symbols */ def registerUsed(syms: Seq[Symbol])(using Context): Unit = - usedInScope.top ++= syms.map(s => s -> s.isAccessibleAsIdent) + usedInScope.top ++= syms.filterNot(isConstructorOfSynth).map(s => s -> s.isAccessibleAsIdent) usedDef ++= syms /** Register an import */ def registerImport(imp: tpd.Import)(using Context): Unit = - val tpd.Import(qual, selectors) = imp - if !tpd.languageImport(qual).nonEmpty then + if !tpd.languageImport(imp.expr).nonEmpty then impInScope.top += imp - unusedImport ++= selectors.filter{ s => - !isImportExclusion(s) && !s.isGiven && !isSelectorOnAGiven(qual, s) - } + unusedImport ++= imp.selectors.filter(s => !isImportExclusion(s)) /** Register (or not) some `val` or `def` according to the context, scope and flags */ def registerDef(valOrDef: tpd.ValOrDefDef)(using Context): Unit = - if valOrDef.symbol.is(Param) then - if valOrDef.symbol.is(Given) then + if valOrDef.symbol.is(Param) && !isSyntheticMainParam(valOrDef.symbol) then + if valOrDef.symbol.isOneOf(GivenOrImplicit) then implicitParamInScope += valOrDef else explicitParamInScope += valOrDef @@ -296,8 +294,33 @@ object CheckUnused: end getUnused //============================ HELPERS ==================================== - private def isSelectorOnAGiven(qual: tpd.Tree, sel: ImportSelector)(using Context): Boolean = - qual.tpe.member(sel.name).alternatives.exists(_.symbol.is(Given)) + /** + * Is the the constructor of synthetic package object + * Should be ignored as it is always imported/used in package + * Trigger false negative on used import + * + * Without this check example: + * + * --- WITH PACKAGE : WRONG --- + * {{{ + * package a: + * val x: Int = 0 + * package b: + * import a._ // no warning + * }}} + * --- WITH OBJECT : OK --- + * {{{ + * object a: + * val x: Int = 0 + * object b: + * import a._ // unused warning + * }}} + */ + private def isConstructorOfSynth(sym: Symbol)(using Context): Boolean = + sym.exists && sym.isConstructor && sym.owner.isPackageObject && sym.owner.is(Synthetic) + + private def isSyntheticMainParam(sym: Symbol)(using Context): Boolean = + sym.exists && ctx.platform.isMainMethod(sym.owner) && sym.owner.is(Synthetic) private def isImportExclusion(sel: ImportSelector): Boolean = sel.renamed match case untpd.Ident(name) => name == StdNames.nme.WILDCARD @@ -319,7 +342,7 @@ object CheckUnused: val tpd.Import(qual, sels) = imp val qualHasSymbol = qual.tpe.member(sym.name).symbol == sym def selector = sels.find(sel => sel.name.toTermName == sym.name || sel.name.toTypeName == sym.name) - def wildcard = sels.find(sel => sel.isWildcard && (sym.is(Given) == sel.isGiven)) + def wildcard = sels.find(sel => sel.isWildcard && ((sym.is(Given) == sel.isGiven) || sym.is(Implicit))) if qualHasSymbol && !isAccessible then selector.orElse(wildcard) // selector with name or wildcard (or given) else diff --git a/tests/neg-custom-args/fatal-warnings/i15503a.scala b/tests/neg-custom-args/fatal-warnings/i15503a.scala index 9f4610af0d97..37247fccc863 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503a.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503a.scala @@ -31,6 +31,14 @@ object FooNested: object Nested: def hello = Set() +object FooGivenUnused: + import SomeGivenImports.given // error + +object FooGiven: + import SomeGivenImports.given // OK + import SomeGivenImports._ // error + + val foo = summon[Int] /** * Import used as type name are considered @@ -84,27 +92,43 @@ object IgnoreExclusion: def check = val a = Set(1) val b = Map(1 -> 2) +/** + * Some given values for the test + */ +object SomeGivenImports: + given Int = 0 + given String = "foo" /* BEGIN : Check on packages*/ -package p { - class C -} - -package p { - import p._ // error - package q { - class U { - def f = new C +package testsamepackageimport: + package p { + class C + } + + package p { + import p._ // error + package q { + class U { + def f = new C + } } } -} +// ----------------------- + +package testpackageimport: + package a: + val x: Int = 0 + + package b: + import a._ // error + + /* END : Check on packages*/ /* BEGIN : tests on meta-language features */ -object TestLanguageImportAreIgnored: +object TestGivenCoversionScala2: /* note: scala3 Conversion[U,T] do not require an import */ import language.implicitConversions // OK - import language._ // OK implicit def doubleToInt(d:Double):Int = d.toInt @@ -126,7 +150,7 @@ object GivenImportOrderAtoB: object B { implicit val y: Y = new Y } class C { import A._ // error - import B._ + import B._ // OK def t = implicitly[X] } @@ -136,47 +160,31 @@ object GivenImportOrderBtoA: object A { implicit val x: X = new X } object B { implicit val y: Y = new Y } class C { - import B._ + import B._ // OK import A._ // error def t = implicitly[X] } /* END : tests on given import order */ -/* - * Advanced tests on given imports meta-programming - * - * - Currently also tests that no imported implicits are reported - */ - -package summoninlineconflict: - package lib: - trait A - trait B - trait C - trait X - - given willBeUnused: (A & X) = new A with X {} - given willBeUsed: (A & B) = new A with B {} - given notUsedAtAll: Int = 0 - - package use: - import lib.{A, B, C, willBeUnused, willBeUsed, notUsedAtAll} // OK - import compiletime.summonInline // OK - - transparent inline given conflictInside: C = - summonInline[A] - new {} - - transparent inline given potentialConflict: C = - summonInline[B] - new {} - - val b: B = summon[B] - val c: C = summon[C] - -package unusedgivensimports: - package foo: - given Int = 0 - - package bar: - import foo.given // OK \ No newline at end of file +/* Scala 2 implicits */ +object Scala2ImplicitsGiven: + object A: + implicit val x: Int = 1 + object B: + import A.given // OK + val b = summon[Int] + object C: + import A.given // error + val b = 1 + object D: + import A._ // OK + val b = summon[Int] + object E: + import A._ // error + val b = 1 + object F: + import A.x // OK + val b = summon[Int] + object G: + import A.x // error + val b = 1 \ No newline at end of file diff --git a/tests/neg-custom-args/fatal-warnings/i15503e.scala b/tests/neg-custom-args/fatal-warnings/i15503e.scala index ed2f6908bb88..9ab8d5ba6c2f 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503e.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503e.scala @@ -5,4 +5,19 @@ def f2(a: Int) = 1 // error def f3(a: Int)(using Int) = a // OK def f4(a: Int)(using Int) = 1 // error def f6(a: Int)(using Int) = summon[Int] // error -def f7(a: Int)(using Int) = summon[Int] + a // OK \ No newline at end of file +def f7(a: Int)(using Int) = summon[Int] + a // OK + +package scala2main: + object happyBirthday { + def main(args: Array[String]): Unit = ??? // error + } + +package scala2mainunused: + object happyBirthday { + def main(args: Array[String]): Unit = // OK + val a = args.size + ??? + } + +package scala3main: + @main def hello = ??? // OK \ No newline at end of file From b2f0fa15f0b1215c4916025e1e114a0f0a1fb9d0 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Mon, 5 Dec 2022 16:36:45 +0100 Subject: [PATCH 21/42] Add warn Annotated Types and "new" kw - Annotated types are reported as used - Some times class constructor "this" is used instead of apply It is registered as used - Some new tests --- .../tools/dotc/transform/CheckUnused.scala | 17 ++++++++++++++++- .../fatal-warnings/i15503a.scala | 17 ++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index d390cec316d4..5e164412a817 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -16,6 +16,11 @@ import dotty.tools.dotc.typer.ImportInfo import dotty.tools.dotc.util.Property import dotty.tools.dotc.transform.CheckUnused.UnusedData.UnusedResult import dotty.tools.dotc.core.Mode +import dotty.tools.dotc.core.Types.TypeTraverser +import dotty.tools.dotc.core.Types.Type +import dotty.tools.dotc.core.Types.AnnotatedType +import dotty.tools.dotc.core.Flags.flagsString +import dotty.tools.dotc.core.Flags @@ -83,11 +88,19 @@ class CheckUnused extends Phase: case t: tpd.Bind => unusedDataApply(_.registerPatVar(t)) traverseChildren(tree)(using newCtx) + case t@tpd.TypeTree() => + typeTraverser(unusedDataApply).traverse(t.tpe) + traverseChildren(tree)(using newCtx) case _ => traverseChildren(tree)(using newCtx) end traverse end traverser + private def typeTraverser(dt: (UnusedData => Any) => Unit)(using Context) = new TypeTraverser: + override def traverse(tp: Type): Unit = tp match + case AnnotatedType(_, annot) => dt(_.registerUsed(annot.symbol)) + case _ => traverseChildren(tp) + private def reportUnused(res: UnusedData.UnusedResult)(using Context): Unit = import CheckUnused.WarnTypes res.warnings.foreach { s => @@ -185,6 +198,8 @@ object CheckUnused: if !isConstructorOfSynth(sym) then usedInScope.top += sym -> sym.isAccessibleAsIdent usedDef += sym + if sym.isConstructor && sym.exists then + registerUsed(sym.owner) // constructor are "implicitly" imported with the class /** Register a list of found (used) symbols */ def registerUsed(syms: Seq[Symbol])(using Context): Unit = @@ -343,7 +358,7 @@ object CheckUnused: val qualHasSymbol = qual.tpe.member(sym.name).symbol == sym def selector = sels.find(sel => sel.name.toTermName == sym.name || sel.name.toTypeName == sym.name) def wildcard = sels.find(sel => sel.isWildcard && ((sym.is(Given) == sel.isGiven) || sym.is(Implicit))) - if qualHasSymbol && !isAccessible then + if qualHasSymbol && !isAccessible && sym.exists then selector.orElse(wildcard) // selector with name or wildcard (or given) else None diff --git a/tests/neg-custom-args/fatal-warnings/i15503a.scala b/tests/neg-custom-args/fatal-warnings/i15503a.scala index 37247fccc863..1b6fd3d29e6a 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503a.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503a.scala @@ -187,4 +187,19 @@ object Scala2ImplicitsGiven: val b = summon[Int] object G: import A.x // error - val b = 1 \ No newline at end of file + val b = 1 + +// ------------------------------------- +object TestNewKeyword: + object Foo: + class Aa[T](val x: T) + object Bar: + import Foo.Aa // OK + val v = 1 + val a = new Aa(v) + +// ------------------------------------- +object testAnnotatedType: + import annotation.switch // OK + val a = (??? : @switch) match + case _ => ??? From 82290c8950e909330d686a0932965695a31c6ccb Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Mon, 5 Dec 2022 17:54:29 +0100 Subject: [PATCH 22/42] Fix unused imports in import stmt --- .../src/dotty/tools/dotc/transform/CheckUnused.scala | 1 + tests/neg-custom-args/fatal-warnings/i15503a.scala | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 5e164412a817..851051934dfc 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -69,6 +69,7 @@ class CheckUnused extends Phase: tree match case imp:tpd.Import => unusedDataApply(_.registerImport(imp)) + traverseChildren(tree)(using newCtx) case ident: Ident => unusedDataApply(_.registerUsed(ident.symbol)) traverseChildren(tree)(using newCtx) diff --git a/tests/neg-custom-args/fatal-warnings/i15503a.scala b/tests/neg-custom-args/fatal-warnings/i15503a.scala index 1b6fd3d29e6a..bfb80613bb6b 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503a.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503a.scala @@ -203,3 +203,14 @@ object testAnnotatedType: import annotation.switch // OK val a = (??? : @switch) match case _ => ??? + + +//------------------------------------- +package testImportsInImports: + package a: + package b: + val x = 1 + package c: + import a.b // OK + import b.x // OK + val y = x \ No newline at end of file From 5a755ddaa87cce200fa31f5c6f851a921cf54f3b Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Mon, 5 Dec 2022 21:43:00 +0100 Subject: [PATCH 23/42] Reports more import alternatives - Reports more import alternatives (e.g. overloaded methods) --- compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 851051934dfc..ebe368fcd351 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -356,7 +356,7 @@ object CheckUnused: /** Given an import and accessibility, return an option of selector that match import<->symbol */ def isInImport(imp: tpd.Import, isAccessible: Boolean)(using Context): Option[ImportSelector] = val tpd.Import(qual, sels) = imp - val qualHasSymbol = qual.tpe.member(sym.name).symbol == sym + val qualHasSymbol = qual.tpe.member(sym.name).alternatives.map(_.symbol).contains(sym) def selector = sels.find(sel => sel.name.toTermName == sym.name || sel.name.toTypeName == sym.name) def wildcard = sels.find(sel => sel.isWildcard && ((sym.is(Given) == sel.isGiven) || sym.is(Implicit))) if qualHasSymbol && !isAccessible && sym.exists then From 2ed1f1fe7e84ece9e5a97edf5687b66bac0baf1a Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Mon, 5 Dec 2022 22:23:48 +0100 Subject: [PATCH 24/42] Warn unused Imports methods overload - overloaded where not well resolved and thus not reported - also add a tests --- .../tools/dotc/transform/CheckUnused.scala | 2 +- .../fatal-warnings/i15503a.scala | 25 ++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index ebe368fcd351..a344819d467c 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -76,7 +76,7 @@ class CheckUnused extends Phase: case sel: Select => unusedDataApply(_.registerUsed(sel.symbol)) traverseChildren(tree)(using newCtx) - case _: (tpd.Block | tpd.Template) => + case _: (tpd.Block | tpd.Template | tpd.PackageDef) => unusedDataApply { ud => ud.inNewScope(ScopeType.fromTree(tree))(traverseChildren(tree)(using newCtx)) } diff --git a/tests/neg-custom-args/fatal-warnings/i15503a.scala b/tests/neg-custom-args/fatal-warnings/i15503a.scala index bfb80613bb6b..03ef042116b9 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503a.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503a.scala @@ -213,4 +213,27 @@ package testImportsInImports: package c: import a.b // OK import b.x // OK - val y = x \ No newline at end of file + val y = x + +//------------------------------------- +package testOnOverloadedMethodsImports: + package a: + trait A + trait B + trait C: + def foo(x: A):A = ??? + def foo(x: B):B = ??? + package b: + object D extends a.C + package c: + import b.D.foo // error + package d: + import b.D.foo // OK + def bar = foo((??? : a.A)) + package e: + import b.D.foo // OK + def bar = foo((??? : a.B)) + package f: + import b.D.foo // OK + def bar = foo((??? : a.A)) + def baz = foo((??? : a.B)) \ No newline at end of file From 8a91af199e262c18e7ac382c1192f4fed3fff038 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Mon, 12 Dec 2022 14:35:56 +0100 Subject: [PATCH 25/42] Add an helpful message for `-W` option --- .../dotty/tools/dotc/config/CliCommand.scala | 2 +- .../dotty/tools/dotc/config/ScalaSettings.scala | 17 +++++++++++++++-- .../src/dotty/tools/dotc/config/Settings.scala | 17 +++++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/CliCommand.scala b/compiler/src/dotty/tools/dotc/config/CliCommand.scala index 68c900e405da..914df040fbf7 100644 --- a/compiler/src/dotty/tools/dotc/config/CliCommand.scala +++ b/compiler/src/dotty/tools/dotc/config/CliCommand.scala @@ -60,7 +60,7 @@ trait CliCommand: def defaultValue = s.default match case _: Int | _: String => s.default.toString case _ => "" - val info = List(shortHelp(s), if defaultValue.nonEmpty then s"Default $defaultValue" else "", if s.legalChoices.nonEmpty then s"Choices ${s.legalChoices}" else "") + val info = List(shortHelp(s), if defaultValue.nonEmpty then s"Default $defaultValue" else "", if s.legalChoices.nonEmpty then s"Choices : ${s.legalChoices}" else "") (s.name, info.filter(_.nonEmpty).mkString("\n")) end help diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 201d02a661e7..a3e96bd92039 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -156,14 +156,27 @@ private sealed trait VerboseSettings: */ private sealed trait WarningSettings: self: SettingGroup => + import Setting.ChoiceWithHelp + 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 Wunused: Setting[List[String]] = MultiChoiceSetting( + val Wunused: Setting[List[ChoiceWithHelp[String]]] = MultiChoiceHelpSetting( name = "-Wunused", helpArg = "warning", descr = "Enable or disable specific `unused` warnings", - choices = List("nowarn", "all", "imports", "locals", "privates", "patvars", "explicits", "implicits", "params", "linted"), + choices = List( + ChoiceWithHelp("nowarn", ""), + ChoiceWithHelp("all",""), + ChoiceWithHelp("imports","Warn if an import selector is not referenced."), + ChoiceWithHelp("patvars","Warn if a variable bound in a pattern is unused."), + 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.") + ), default = Nil ) object WunusedHas: diff --git a/compiler/src/dotty/tools/dotc/config/Settings.scala b/compiler/src/dotty/tools/dotc/config/Settings.scala index 277833afbd5d..a7bcc3db5428 100644 --- a/compiler/src/dotty/tools/dotc/config/Settings.scala +++ b/compiler/src/dotty/tools/dotc/config/Settings.scala @@ -11,6 +11,7 @@ import annotation.tailrec import collection.mutable.ArrayBuffer import reflect.ClassTag import scala.util.{Success, Failure} +import dotty.tools.dotc.config.Settings.Setting.ChoiceWithHelp object Settings: @@ -184,6 +185,19 @@ object Settings: 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](choice: T, description: String): + override def equals(x: Any): Boolean = x match + case s:String => s == choice.toString() + case _ => false + override def toString(): String = + s"\n\t- $choice${if description.isBlank() then "" else s"\t: $description"}" + end Setting + class SettingGroup { private val _allSettings = new ArrayBuffer[Setting[?]] @@ -265,6 +279,9 @@ object Settings: def MultiChoiceSetting(name: String, helpArg: String, descr: String, choices: List[String], default: List[String], aliases: List[String] = Nil): Setting[List[String]] = publish(Setting(name, descr, default, helpArg, Some(choices), aliases = aliases)) + def MultiChoiceHelpSetting(name: String, helpArg: String, descr: String, choices: List[ChoiceWithHelp[String]], default: List[ChoiceWithHelp[String]], aliases: List[String] = Nil): Setting[List[ChoiceWithHelp[String]]] = + publish(Setting(name, descr, default, helpArg, Some(choices), aliases = aliases)) + def IntSetting(name: String, descr: String, default: Int, aliases: List[String] = Nil): Setting[Int] = publish(Setting(name, descr, default, aliases = aliases)) From 426d34a39cfd26bb46036a0c1308a821c7b77dc8 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Mon, 12 Dec 2022 14:40:39 +0100 Subject: [PATCH 26/42] Fix typo in help -W --- .../dotty/tools/dotc/config/ScalaSettings.scala | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index a3e96bd92039..14054ac70319 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -168,14 +168,14 @@ private sealed trait WarningSettings: choices = List( ChoiceWithHelp("nowarn", ""), ChoiceWithHelp("all",""), - ChoiceWithHelp("imports","Warn if an import selector is not referenced."), - ChoiceWithHelp("patvars","Warn if a variable bound in a pattern is unused."), - 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("imports","Warn if an import selector is not referenced"), + ChoiceWithHelp("patvars","Warn if a variable bound in a pattern is unused"), + 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") ), default = Nil ) From 5ab55f8a8520358b7b51b9ee06c887f2b8e612d6 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Mon, 12 Dec 2022 14:45:25 +0100 Subject: [PATCH 27/42] Fix isBlank not member of String on CI --- compiler/src/dotty/tools/dotc/config/Settings.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/config/Settings.scala b/compiler/src/dotty/tools/dotc/config/Settings.scala index a7bcc3db5428..4f0133b5cf87 100644 --- a/compiler/src/dotty/tools/dotc/config/Settings.scala +++ b/compiler/src/dotty/tools/dotc/config/Settings.scala @@ -195,7 +195,7 @@ object Settings: case s:String => s == choice.toString() case _ => false override def toString(): String = - s"\n\t- $choice${if description.isBlank() then "" else s"\t: $description"}" + s"\n\t- $choice${if description.isEmpty() then "" else s"\t: $description"}" end Setting class SettingGroup { From 0cb2af0d8ced443f6269615e7c63d03e54a2395c Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Tue, 13 Dec 2022 17:49:34 +0100 Subject: [PATCH 28/42] Add `-Wunused:strict-no-implicit-warn` warning option - Add the new -Wunused:strict-no-implicit-warn flag - Add a test suit i15503j --- .../tools/dotc/config/ScalaSettings.scala | 24 ++++++++++++++---- .../dotty/tools/dotc/config/Settings.scala | 6 ++--- .../tools/dotc/transform/CheckUnused.scala | 18 +++++++++++-- .../fatal-warnings/i15503j.scala | 25 +++++++++++++++++++ 4 files changed, 63 insertions(+), 10 deletions(-) create mode 100644 tests/neg-custom-args/fatal-warnings/i15503j.scala diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 14054ac70319..cdf3b634d2af 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -168,23 +168,33 @@ private sealed trait WarningSettings: choices = List( ChoiceWithHelp("nowarn", ""), ChoiceWithHelp("all",""), - ChoiceWithHelp("imports","Warn if an import selector is not referenced"), + ChoiceWithHelp( + name = "imports", + description = "Warn if an import selector is not referenced.\n" + + "NOTE : overrided by -Wunused:strict-no-implicit-warn"), ChoiceWithHelp("patvars","Warn if a variable bound in a pattern is unused"), 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("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" + ) ), 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 nowarn(using Context) = allOr("nowarn") + // overrided by strict-no-implicit-warn def imports(using Context) = - allOr("imports") || allOr("linted") + (allOr("imports") || allOr("linted")) && !(strictNoImplicitWarn) def locals(using Context) = allOr("locals") || allOr("linted") /** -Wunused:explicits OR -Wunused:params */ @@ -196,8 +206,12 @@ private sealed trait WarningSettings: def params(using Context) = allOr("params") def privates(using Context) = allOr("privates") || allOr("linted") - def patvars(using Context) = allOr("patvars") - def linted(using Context) = allOr("linted") + def patvars(using Context) = + allOr("patvars") + def linted(using Context) = + allOr("linted") + def strictNoImplicitWarn(using Context) = + isChoiceSet("strict-no-implicit-warn") val Wconf: Setting[List[String]] = MultiStringSetting( "-Wconf", diff --git a/compiler/src/dotty/tools/dotc/config/Settings.scala b/compiler/src/dotty/tools/dotc/config/Settings.scala index 4f0133b5cf87..e5eddb953068 100644 --- a/compiler/src/dotty/tools/dotc/config/Settings.scala +++ b/compiler/src/dotty/tools/dotc/config/Settings.scala @@ -190,12 +190,12 @@ object Settings: * * NOTE : `equals` and `toString` have special behaviors */ - case class ChoiceWithHelp[T](choice: T, description: String): + case class ChoiceWithHelp[T](name: T, description: String): override def equals(x: Any): Boolean = x match - case s:String => s == choice.toString() + case s:String => s == name.toString() case _ => false override def toString(): String = - s"\n\t- $choice${if description.isEmpty() then "" else s"\t: $description"}" + s"\n- $name${if description.isEmpty() then "" else s" :\n\t${description.replace("\n","\n\t")}"}" end Setting class SettingGroup { diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index a344819d467c..9c42cc2c3379 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -211,7 +211,9 @@ object CheckUnused: def registerImport(imp: tpd.Import)(using Context): Unit = if !tpd.languageImport(imp.expr).nonEmpty then impInScope.top += imp - unusedImport ++= imp.selectors.filter(s => !isImportExclusion(s)) + unusedImport ++= imp.selectors.filter { s => + !shouldSelectorBeReported(imp, s) && !isImportExclusion(s) + } /** Register (or not) some `val` or `def` according to the context, scope and flags */ def registerDef(valOrDef: tpd.ValOrDefDef)(using Context): Unit = @@ -273,7 +275,7 @@ object CheckUnused: def getUnused(using Context): UnusedResult = popScope() val sortedImp = - if ctx.settings.WunusedHas.imports then + if ctx.settings.WunusedHas.imports || ctx.settings.WunusedHas.strictNoImplicitWarn then unusedImport.map(d => d.srcPos -> WarnTypes.Imports).toList else Nil @@ -342,6 +344,18 @@ object CheckUnused: case untpd.Ident(name) => name == StdNames.nme.WILDCARD case _ => false + /** + * If -Wunused:strict-no-implicit-warn import and this import selector could potentially import implicit. + * return true + */ + private def shouldSelectorBeReported(imp: tpd.Import, sel: ImportSelector)(using Context): Boolean = + if ctx.settings.WunusedHas.strictNoImplicitWarn then + sel.isWildcard || + imp.expr.tpe.member(sel.name.toTermName).alternatives.exists(_.symbol.isOneOf(GivenOrImplicit)) || + imp.expr.tpe.member(sel.name.toTypeName).alternatives.exists(_.symbol.isOneOf(GivenOrImplicit)) + else + false + extension (sym: Symbol) /** is accessible without import in current context */ def isAccessibleAsIdent(using Context): Boolean = diff --git a/tests/neg-custom-args/fatal-warnings/i15503j.scala b/tests/neg-custom-args/fatal-warnings/i15503j.scala new file mode 100644 index 000000000000..8f849da31de6 --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/i15503j.scala @@ -0,0 +1,25 @@ +// scalac: -Wunused:strict-no-implicit-warn + +package unused.test: + package a: + given x: Int = 0 + implicit val y: Int = 1 + val z: Int = 2 + def f: Int = 3 + package b: + import a.given // OK + import a._ // OK + import a.* // OK + import a.x // OK + import a.y // OK + import a.z // error + import a.f // error + package c: + import a.given // OK + import a._ // OK + import a.* // OK + import a.x // OK + import a.y // OK + import a.z // OK + import a.f // OK + def g = f + z + y + x \ No newline at end of file From d1f944166d387db5922d79189e7e9fe02cd57337 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Tue, 13 Dec 2022 18:30:17 +0100 Subject: [PATCH 29/42] Fix test i15503j --- .../fatal-warnings/i15503j.scala | 42 +++++++++++++++++-- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/tests/neg-custom-args/fatal-warnings/i15503j.scala b/tests/neg-custom-args/fatal-warnings/i15503j.scala index 8f849da31de6..51c1fa6fda0c 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503j.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503j.scala @@ -1,6 +1,6 @@ // scalac: -Wunused:strict-no-implicit-warn -package unused.test: +package foo.unused.strict.test: package a: given x: Int = 0 implicit val y: Int = 1 @@ -16,10 +16,44 @@ package unused.test: import a.f // error package c: import a.given // OK - import a._ // OK - import a.* // OK import a.x // OK import a.y // OK import a.z // OK import a.f // OK - def g = f + z + y + x \ No newline at end of file + def g = f + z + y + x + +package foo.implicits.resolution: + class X + class Y extends X + object A { implicit val x: X = new X } + object B { implicit val y: Y = new Y } + class C { + import A._ // OK + import B._ // OK + def t = implicitly[X] + } + +package foo.unused.summon.inlines: + package lib: + trait A + trait B + trait C + trait X + + given willBeUnused: (A & X) = new A with X {} + given willBeUsed: (A & B) = new A with B {} + + package use: + import lib.{A, B, C, willBeUnused, willBeUsed} //OK + import compiletime.summonInline //OK + + transparent inline given conflictInside: C = + summonInline[A] + new {} + + transparent inline given potentialConflict: C = + summonInline[B] + new {} + + val b: B = summon[B] + val c: C = summon[C] \ No newline at end of file From e158a9fa902ad236e5f2016d1e36b12ef83fe134 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Tue, 13 Dec 2022 19:23:03 +0100 Subject: [PATCH 30/42] Add better support for unused renamed import --- .../tools/dotc/transform/CheckUnused.scala | 27 +++++++++---------- .../fatal-warnings/i15503a.scala | 10 ++++++- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 9c42cc2c3379..4089759200ef 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -21,6 +21,7 @@ import dotty.tools.dotc.core.Types.Type import dotty.tools.dotc.core.Types.AnnotatedType import dotty.tools.dotc.core.Flags.flagsString import dotty.tools.dotc.core.Flags +import dotty.tools.dotc.core.Names.Name @@ -71,10 +72,10 @@ class CheckUnused extends Phase: unusedDataApply(_.registerImport(imp)) traverseChildren(tree)(using newCtx) case ident: Ident => - unusedDataApply(_.registerUsed(ident.symbol)) + unusedDataApply(_.registerUsed(ident.symbol, Some(ident.name))) traverseChildren(tree)(using newCtx) case sel: Select => - unusedDataApply(_.registerUsed(sel.symbol)) + unusedDataApply(_.registerUsed(sel.symbol, Some(sel.name))) traverseChildren(tree)(using newCtx) case _: (tpd.Block | tpd.Template | tpd.PackageDef) => unusedDataApply { ud => @@ -99,7 +100,7 @@ class CheckUnused extends Phase: private def typeTraverser(dt: (UnusedData => Any) => Unit)(using Context) = new TypeTraverser: override def traverse(tp: Type): Unit = tp match - case AnnotatedType(_, annot) => dt(_.registerUsed(annot.symbol)) + case AnnotatedType(_, annot) => dt(_.registerUsed(annot.symbol, None)) case _ => traverseChildren(tp) private def reportUnused(res: UnusedData.UnusedResult)(using Context): Unit = @@ -156,7 +157,7 @@ object CheckUnused: * * See the `isAccessibleAsIdent` extension method below in the file */ - private val usedInScope = MutStack(MutSet[(Symbol,Boolean)]()) + private val usedInScope = MutStack(MutSet[(Symbol,Boolean, Option[Name])]()) /* unused import collected during traversal */ private val unusedImport = MutSet[ImportSelector]() @@ -195,17 +196,15 @@ object CheckUnused: registerUsed(annotSym) /** Register a found (used) symbol */ - def registerUsed(sym: Symbol)(using Context): Unit = + def registerUsed(sym: Symbol, name: Option[Name])(using Context): Unit = if !isConstructorOfSynth(sym) then - usedInScope.top += sym -> sym.isAccessibleAsIdent - usedDef += sym + usedInScope.top += ((sym, sym.isAccessibleAsIdent, name)) if sym.isConstructor && sym.exists then - registerUsed(sym.owner) // constructor are "implicitly" imported with the class + registerUsed(sym.owner, None) // constructor are "implicitly" imported with the class /** Register a list of found (used) symbols */ def registerUsed(syms: Seq[Symbol])(using Context): Unit = - usedInScope.top ++= syms.filterNot(isConstructorOfSynth).map(s => s -> s.isAccessibleAsIdent) - usedDef ++= syms + usedInScope.top ++= syms.filterNot(isConstructorOfSynth).map(s => (s, s.isAccessibleAsIdent, None)) /** Register an import */ def registerImport(imp: tpd.Import)(using Context): Unit = @@ -248,10 +247,10 @@ object CheckUnused: // used imports in this scope val imports = impInScope.pop().toSet val kept = used.filter { t => - val (sym, isAccessible) = t + val (sym, isAccessible, optName) = t // keep the symbol for outer scope, if it matches **no** import !imports.exists { imp => - sym.isInImport(imp, isAccessible) match + sym.isInImport(imp, isAccessible, optName) match case None => false case Some(sel) => unusedImport -= sel @@ -368,10 +367,10 @@ object CheckUnused: } /** Given an import and accessibility, return an option of selector that match import<->symbol */ - def isInImport(imp: tpd.Import, isAccessible: Boolean)(using Context): Option[ImportSelector] = + def isInImport(imp: tpd.Import, isAccessible: Boolean, symName: Option[Name])(using Context): Option[ImportSelector] = val tpd.Import(qual, sels) = imp val qualHasSymbol = qual.tpe.member(sym.name).alternatives.map(_.symbol).contains(sym) - def selector = sels.find(sel => sel.name.toTermName == sym.name || sel.name.toTypeName == sym.name) + def selector = sels.find(sel => (sel.name.toTermName == sym.name || sel.name.toTypeName == sym.name) && symName.map(n => n.toTermName == sel.rename).getOrElse(true)) def wildcard = sels.find(sel => sel.isWildcard && ((sym.is(Given) == sel.isGiven) || sym.is(Implicit))) if qualHasSymbol && !isAccessible && sym.exists then selector.orElse(wildcard) // selector with name or wildcard (or given) diff --git a/tests/neg-custom-args/fatal-warnings/i15503a.scala b/tests/neg-custom-args/fatal-warnings/i15503a.scala index 03ef042116b9..13408976ec57 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503a.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503a.scala @@ -236,4 +236,12 @@ package testOnOverloadedMethodsImports: package f: import b.D.foo // OK def bar = foo((??? : a.A)) - def baz = foo((??? : a.B)) \ No newline at end of file + def baz = foo((??? : a.B)) + +//------------------------------------- +package foo.testing.rename.imports: + import collection.mutable.{Set => MutSet1} // OK + import collection.mutable.{Set => MutSet2} // OK + import collection.mutable.{Set => MutSet3} // error + type A[X] = MutSet1[X] + val a = MutSet2(1) \ No newline at end of file From e2b6b61f3f6ceca4e99a31d4772a6d0296de9324 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Wed, 14 Dec 2022 23:12:35 +0100 Subject: [PATCH 31/42] Clean, refine scope, add comments - Tidy the code (space, newline,...) - Refine the scope by making more things private - Add explaination and doc comments (mostly to help reviewer) --- .../tools/dotc/transform/CheckUnused.scala | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 4089759200ef..a9c6c6790af3 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -14,7 +14,6 @@ import dotty.tools.dotc.report import dotty.tools.dotc.reporting.Message import dotty.tools.dotc.typer.ImportInfo import dotty.tools.dotc.util.Property -import dotty.tools.dotc.transform.CheckUnused.UnusedData.UnusedResult import dotty.tools.dotc.core.Mode import dotty.tools.dotc.core.Types.TypeTraverser import dotty.tools.dotc.core.Types.Type @@ -34,6 +33,10 @@ import dotty.tools.dotc.core.Names.Name class CheckUnused extends Phase: import CheckUnused.UnusedData + /** + * The key used to retrieve the "unused entity" analysis metadata, + * from the compilation `Context` + */ private val _key = Property.Key[UnusedData] override def phaseName: String = CheckUnused.phaseName @@ -98,11 +101,13 @@ class CheckUnused extends Phase: end traverse end traverser + /** This is a type traverser which catch some special Types not traversed by the term traverser above */ private def typeTraverser(dt: (UnusedData => Any) => Unit)(using Context) = new TypeTraverser: override def traverse(tp: Type): Unit = tp match case AnnotatedType(_, annot) => dt(_.registerUsed(annot.symbol, None)) case _ => traverseChildren(tp) + /** Do the actual reporting given the result of the anaylsis */ private def reportUnused(res: UnusedData.UnusedResult)(using Context): Unit = import CheckUnused.WarnTypes res.warnings.foreach { s => @@ -127,7 +132,7 @@ object CheckUnused: val phaseName: String = "checkUnused" val description: String = "check for unused elements" - enum WarnTypes: + private enum WarnTypes: case Imports case LocalDefs case ExplicitParams @@ -142,6 +147,7 @@ object CheckUnused: * - usage */ private class UnusedData: + import dotty.tools.dotc.transform.CheckUnused.UnusedData.UnusedResult import collection.mutable.{Set => MutSet, Map => MutMap, Stack => MutStack} import dotty.tools.dotc.core.Symbols.Symbol import UnusedData.ScopeType @@ -195,7 +201,12 @@ object CheckUnused: val annotSym = sym.denot.annotations.map(_.symbol) registerUsed(annotSym) - /** Register a found (used) symbol */ + /** + * Register a found (used) symbol along with its name + * + * The optional name will be used to target the right import + * as the same element can be imported with different renaming + */ def registerUsed(sym: Symbol, name: Option[Name])(using Context): Unit = if !isConstructorOfSynth(sym) then usedInScope.top += ((sym, sym.isAccessibleAsIdent, name)) @@ -336,9 +347,16 @@ object CheckUnused: private def isConstructorOfSynth(sym: Symbol)(using Context): Boolean = sym.exists && sym.isConstructor && sym.owner.isPackageObject && sym.owner.is(Synthetic) + /** + * This is used to avoid reporting the parameters of the synthetic main method + * generated by `@main` + */ private def isSyntheticMainParam(sym: Symbol)(using Context): Boolean = sym.exists && ctx.platform.isMainMethod(sym.owner) && sym.owner.is(Synthetic) + /** + * This is used to ignore exclusion imports (i.e. import `qual`.{`member` => _}) + */ private def isImportExclusion(sel: ImportSelector): Boolean = sel.renamed match case untpd.Ident(name) => name == StdNames.nme.WILDCARD case _ => false @@ -376,21 +394,22 @@ object CheckUnused: selector.orElse(wildcard) // selector with name or wildcard (or given) else None - end UnusedData - object UnusedData: + private object UnusedData: enum ScopeType: case Local case Template case Other object ScopeType: + /** return the scope corresponding to the enclosing scope of the given tree */ def fromTree(tree: tpd.Tree): ScopeType = tree match case _:tpd.Template => Template case _:tpd.Block => Local case _ => Other + /** A container for the results of the used elements analysis */ case class UnusedResult(warnings: List[(dotty.tools.dotc.util.SrcPos, WarnTypes)], usedImports: List[(tpd.Import, untpd.ImportSelector)]) end CheckUnused From b0790d16c4f986ebd99d8776c670ef0f5f220b91 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Fri, 16 Dec 2022 21:46:38 +0100 Subject: [PATCH 32/42] Refactor CheckUnused into a MiniPhase - Add prepare and tranform method from MiniPhase - Keep previous implementation for cases not covered by MiniPhase traversal (Mostly in the type tree) --- .../tools/dotc/transform/CheckUnused.scala | 128 +++++++++++++++--- 1 file changed, 106 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index a9c6c6790af3..b0572bb71b5b 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -21,6 +21,7 @@ import dotty.tools.dotc.core.Types.AnnotatedType import dotty.tools.dotc.core.Flags.flagsString import dotty.tools.dotc.core.Flags import dotty.tools.dotc.core.Names.Name +import dotty.tools.dotc.transform.MegaPhase.MiniPhase @@ -30,7 +31,7 @@ import dotty.tools.dotc.core.Names.Name * Basically, it gathers definition/imports and their usage. If a * definition/imports does not have any usage, then it is reported. */ -class CheckUnused extends Phase: +class CheckUnused extends MiniPhase: import CheckUnused.UnusedData /** @@ -39,6 +40,13 @@ class CheckUnused extends Phase: */ private val _key = Property.Key[UnusedData] + extension (k: Property.Key[UnusedData]) + private def unusedDataApply[U](f: UnusedData => U)(using Context): Context = + ctx.property(_key).foreach(f) + ctx + private def getUnusedData(using Context): Option[UnusedData] = + ctx.property(_key) + override def phaseName: String = CheckUnused.phaseName override def description: String = CheckUnused.description @@ -47,12 +55,84 @@ class CheckUnused extends Phase: ctx.settings.Wunused.value.nonEmpty && !ctx.isJava - override def run(using Context): Unit = + // ========== SETUP ============ + + override def prepareForUnit(tree: tpd.Tree)(using Context): Context = val tree = ctx.compilationUnit.tpdTree val data = UnusedData() val fresh = ctx.fresh.setProperty(_key, data) - traverser.traverse(tree)(using fresh) - reportUnused(data.getUnused) + fresh + + // ========== END + REPORTING ========== + + override def transformUnit(tree: tpd.Tree)(using Context): tpd.Tree = + _key.unusedDataApply(ud => reportUnused(ud.getUnused)) + tree + + // ========== MiniPhase Prepare ========== + override def prepareForOther(tree: tpd.Tree)(using Context): Context = + // A standard tree traverser covers cases not handled by the Mega/MiniPhase + traverser.traverse(tree) + ctx + + override def prepareForIdent(tree: tpd.Ident)(using Context): Context = + _key.unusedDataApply(_.registerUsed(tree.symbol, Some(tree.name))) + + override def prepareForSelect(tree: tpd.Select)(using Context): Context = + _key.unusedDataApply(_.registerUsed(tree.symbol, Some(tree.name))) + + override def prepareForBlock(tree: tpd.Block)(using Context): Context = + pushInBlockTemplatePackageDef(tree) + + override def prepareForTemplate(tree: tpd.Template)(using Context): Context = + pushInBlockTemplatePackageDef(tree) + + override def prepareForPackageDef(tree: tpd.PackageDef)(using Context): Context = + pushInBlockTemplatePackageDef(tree) + + override def prepareForValDef(tree: tpd.ValDef)(using Context): Context = + _key.unusedDataApply(_.registerDef(tree)) + + override def prepareForDefDef(tree: tpd.DefDef)(using Context): Context = + _key.unusedDataApply(_.registerDef(tree)) + + override def prepareForBind(tree: tpd.Bind)(using Context): Context = + _key.unusedDataApply(_.registerPatVar(tree)) + + override def prepareForTypeTree(tree: tpd.TypeTree)(using Context): Context = + typeTraverser(_key.unusedDataApply).traverse(tree.tpe) + ctx + + // ========== MiniPhase Transform ========== + + override def transformBlock(tree: tpd.Block)(using Context): tpd.Tree = + popOutBlockTemplatePackageDef() + tree + + override def transformTemplate(tree: tpd.Template)(using Context): tpd.Tree = + popOutBlockTemplatePackageDef() + tree + + override def transformPackageDef(tree: tpd.PackageDef)(using Context): tpd.Tree = + popOutBlockTemplatePackageDef() + tree + + // ---------- MiniPhase HELPERS ----------- + + private def pushInBlockTemplatePackageDef(tree: tpd.Block | tpd.Template | tpd.PackageDef)(using Context): Context = + _key.unusedDataApply { ud => + ud.pushScope(UnusedData.ScopeType.fromTree(tree)) + } + ctx + + private def popOutBlockTemplatePackageDef()(using Context): Context = + _key.unusedDataApply { ud => + ud.popScope() + } + ctx + + private def newCtx(tree: tpd.Tree)(using Context) = + if tree.symbol.exists then ctx.withOwner(tree.symbol) else ctx /** * This traverse is the **main** component of this phase @@ -66,37 +146,37 @@ class CheckUnused extends Phase: /* Register every imports, definition and usage */ override def traverse(tree: tpd.Tree)(using Context): Unit = - val unusedDataApply = ctx.property(_key).foreach val newCtx = if tree.symbol.exists then ctx.withOwner(tree.symbol) else ctx - if tree.isDef then // register the annotations for usage - unusedDataApply(_.registerUsedAnnotation(tree.symbol)) tree match case imp:tpd.Import => - unusedDataApply(_.registerImport(imp)) + _key.unusedDataApply(_.registerImport(imp)) traverseChildren(tree)(using newCtx) case ident: Ident => - unusedDataApply(_.registerUsed(ident.symbol, Some(ident.name))) + prepareForIdent(ident) traverseChildren(tree)(using newCtx) case sel: Select => - unusedDataApply(_.registerUsed(sel.symbol, Some(sel.name))) + prepareForSelect(sel) traverseChildren(tree)(using newCtx) case _: (tpd.Block | tpd.Template | tpd.PackageDef) => - unusedDataApply { ud => + //! DIFFERS FROM MINIPHASE + _key.unusedDataApply { ud => ud.inNewScope(ScopeType.fromTree(tree))(traverseChildren(tree)(using newCtx)) } case t:tpd.ValDef => - unusedDataApply(_.registerDef(t)) + prepareForValDef(t) traverseChildren(tree)(using newCtx) case t:tpd.DefDef => - unusedDataApply(_.registerDef(t)) + prepareForDefDef(t) traverseChildren(tree)(using newCtx) case t: tpd.Bind => - unusedDataApply(_.registerPatVar(t)) + prepareForBind(t) traverseChildren(tree)(using newCtx) case t@tpd.TypeTree() => - typeTraverser(unusedDataApply).traverse(t.tpe) + //! DIFFERS FROM MINIPHASE + typeTraverser(_key.unusedDataApply).traverse(t.tpe) traverseChildren(tree)(using newCtx) case _ => + //! DIFFERS FROM MINIPHASE traverseChildren(tree)(using newCtx) end traverse end traverser @@ -153,7 +233,7 @@ object CheckUnused: import UnusedData.ScopeType /** The current scope during the tree traversal */ - var currScopeType: ScopeType = ScopeType.Other + var currScopeType: MutStack[ScopeType] = MutStack(ScopeType.Other) /* IMPORTS */ private val impInScope = MutStack(MutSet[tpd.Import]()) @@ -190,11 +270,9 @@ object CheckUnused: */ def inNewScope(newScope: ScopeType)(execInNewScope: => Unit)(using Context): Unit = val prev = currScopeType - currScopeType = newScope - pushScope() + pushScope(newScope) execInNewScope popScope() - currScopeType = prev /** Register all annotations of this symbol's denotation */ def registerUsedAnnotation(sym: Symbol)(using Context): Unit = @@ -227,23 +305,27 @@ object CheckUnused: /** Register (or not) some `val` or `def` according to the context, scope and flags */ def registerDef(valOrDef: tpd.ValOrDefDef)(using Context): Unit = + // register the annotations for usage + registerUsedAnnotation(valOrDef.symbol) if valOrDef.symbol.is(Param) && !isSyntheticMainParam(valOrDef.symbol) then if valOrDef.symbol.isOneOf(GivenOrImplicit) then implicitParamInScope += valOrDef else explicitParamInScope += valOrDef - else if currScopeType == ScopeType.Local then + else if currScopeType.top == ScopeType.Local then localDefInScope += valOrDef - else if currScopeType == ScopeType.Template && valOrDef.symbol.is(Private, butNot = SelfName) then + else if currScopeType.top == ScopeType.Template && valOrDef.symbol.is(Private, butNot = SelfName) then privateDefInScope += valOrDef /** Register pattern variable */ def registerPatVar(patvar: tpd.Bind)(using Context): Unit = + registerUsedAnnotation(patvar.symbol) patVarsInScope += patvar /** enter a new scope */ - def pushScope(): Unit = + def pushScope(newScopeType: ScopeType): Unit = // unused imports : + currScopeType.push(newScopeType) impInScope.push(MutSet()) usedInScope.push(MutSet()) @@ -275,6 +357,8 @@ object CheckUnused: usedInScope.top ++= kept // register usage in this scope for other warnings at the end of the phase usedDef ++= used.map(_._1) + // retrieve previous scope type + currScopeType.pop end popScope /** From 7f04ce3c8deee75d124991c97da1e5cbf65ce912 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Mon, 26 Dec 2022 15:15:37 +0100 Subject: [PATCH 33/42] Add support for @annotation.unused - Does not report defs with @unused - Update the test suit --- .../dotty/tools/dotc/core/Definitions.scala | 1 + .../tools/dotc/transform/CheckUnused.scala | 28 +++++++++++------- .../fatal-warnings/i15503i.scala | 29 ++++++++++++++++++- 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 174244b4a456..5048bb92cc0c 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -991,6 +991,7 @@ class Definitions { @tu lazy val MappedAlternativeAnnot: ClassSymbol = requiredClass("scala.annotation.internal.MappedAlternative") @tu lazy val MigrationAnnot: ClassSymbol = requiredClass("scala.annotation.migration") @tu lazy val NowarnAnnot: ClassSymbol = requiredClass("scala.annotation.nowarn") + @tu lazy val UnusedAnnot: ClassSymbol = requiredClass("scala.annotation.unused") @tu lazy val TransparentTraitAnnot: ClassSymbol = requiredClass("scala.annotation.transparentTrait") @tu lazy val NativeAnnot: ClassSymbol = requiredClass("scala.native") @tu lazy val RepeatedAnnot: ClassSymbol = requiredClass("scala.annotation.internal.Repeated") diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index b0572bb71b5b..ab127df52a78 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -22,6 +22,8 @@ import dotty.tools.dotc.core.Flags.flagsString import dotty.tools.dotc.core.Flags import dotty.tools.dotc.core.Names.Name import dotty.tools.dotc.transform.MegaPhase.MiniPhase +import dotty.tools.dotc.core.Annotations +import dotty.tools.dotc.core.Definitions @@ -307,20 +309,22 @@ object CheckUnused: def registerDef(valOrDef: tpd.ValOrDefDef)(using Context): Unit = // register the annotations for usage registerUsedAnnotation(valOrDef.symbol) - if valOrDef.symbol.is(Param) && !isSyntheticMainParam(valOrDef.symbol) then - if valOrDef.symbol.isOneOf(GivenOrImplicit) then - implicitParamInScope += valOrDef - else - explicitParamInScope += valOrDef - else if currScopeType.top == ScopeType.Local then - localDefInScope += valOrDef - else if currScopeType.top == ScopeType.Template && valOrDef.symbol.is(Private, butNot = SelfName) then - privateDefInScope += valOrDef + if !valOrDef.symbol.isUnusedAnnot then + if valOrDef.symbol.is(Param) && !isSyntheticMainParam(valOrDef.symbol) then + if valOrDef.symbol.isOneOf(GivenOrImplicit) then + implicitParamInScope += valOrDef + else + explicitParamInScope += valOrDef + else if currScopeType.top == ScopeType.Local then + localDefInScope += valOrDef + else if currScopeType.top == ScopeType.Template && valOrDef.symbol.is(Private, butNot = SelfName) then + privateDefInScope += valOrDef /** Register pattern variable */ def registerPatVar(patvar: tpd.Bind)(using Context): Unit = registerUsedAnnotation(patvar.symbol) - patVarsInScope += patvar + if !patvar.symbol.isUnusedAnnot then + patVarsInScope += patvar /** enter a new scope */ def pushScope(newScopeType: ScopeType): Unit = @@ -478,6 +482,10 @@ object CheckUnused: selector.orElse(wildcard) // selector with name or wildcard (or given) else None + + /** Annotated with @unused */ + def isUnusedAnnot(using Context): Boolean = + sym.annotations.exists(a => a.symbol == ctx.definitions.UnusedAnnot) end UnusedData private object UnusedData: diff --git a/tests/neg-custom-args/fatal-warnings/i15503i.scala b/tests/neg-custom-args/fatal-warnings/i15503i.scala index a94cd172b942..c645c0ae164b 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503i.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503i.scala @@ -29,4 +29,31 @@ class A { case x:1 => 0 // error case x:2 => x // OK case _ => 1 // OK -} \ No newline at end of file +} + +/* ---- CHECK scala.annotation.unused ---- */ +package foo.test.scala.annotation: + import annotation.unused // OK + + def a1(a: Int) = a // OK + def a2(a: Int) = 1 // error + def a3(@unused a: Int) = 1 //OK + + def b1 = + def f = 1 // error + 1 + + def b2 = + def f = 1 // OK + f + + def b3 = + @unused def f = 1 // OK + 1 + + object Foo: + private def a = 1 // error + private def b = 2 // OK + @unused private def c = 3 // OK + + def other = b From 779ec7dae5c330c8244c69d8600e07c64f72edec Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Mon, 26 Dec 2022 17:01:26 +0100 Subject: [PATCH 34/42] Add support for trivial body - Params of method with trivial body are not reported as unused - Update the test suit --- .../tools/dotc/transform/CheckUnused.scala | 28 +++++++++++++++++-- .../fatal-warnings/i15503e.scala | 20 +++++++------ .../fatal-warnings/i15503f.scala | 5 +++- .../fatal-warnings/i15503g.scala | 10 +++++-- .../fatal-warnings/i15503h.scala | 2 +- .../fatal-warnings/i15503i.scala | 15 +++++++--- 6 files changed, 62 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index ab127df52a78..44793c007fae 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -24,6 +24,7 @@ import dotty.tools.dotc.core.Names.Name import dotty.tools.dotc.transform.MegaPhase.MiniPhase import dotty.tools.dotc.core.Annotations import dotty.tools.dotc.core.Definitions +import dotty.tools.dotc.core.Types.ConstantType @@ -96,7 +97,11 @@ class CheckUnused extends MiniPhase: _key.unusedDataApply(_.registerDef(tree)) override def prepareForDefDef(tree: tpd.DefDef)(using Context): Context = - _key.unusedDataApply(_.registerDef(tree)) + _key.unusedDataApply{ ud => + import ud.registerTrivial + tree.registerTrivial + ud.registerDef(tree) + } override def prepareForBind(tree: tpd.Bind)(using Context): Context = _key.unusedDataApply(_.registerPatVar(tree)) @@ -266,6 +271,9 @@ object CheckUnused: /** All used symbols */ private val usedDef = MutSet[Symbol]() + /** Trivial definitions, avoid registering params */ + private val trivialDefs = MutSet[Symbol]() + /** * Push a new Scope of the given type, executes the given Unit and * pop it back to the original type. @@ -310,7 +318,7 @@ object CheckUnused: // register the annotations for usage registerUsedAnnotation(valOrDef.symbol) if !valOrDef.symbol.isUnusedAnnot then - if valOrDef.symbol.is(Param) && !isSyntheticMainParam(valOrDef.symbol) then + if valOrDef.symbol.is(Param) && !isSyntheticMainParam(valOrDef.symbol) && !valOrDef.symbol.ownerIsTrivial then if valOrDef.symbol.isOneOf(GivenOrImplicit) then implicitParamInScope += valOrDef else @@ -486,6 +494,22 @@ object CheckUnused: /** Annotated with @unused */ def isUnusedAnnot(using Context): Boolean = sym.annotations.exists(a => a.symbol == ctx.definitions.UnusedAnnot) + + def ownerIsTrivial(using Context): Boolean = + sym.exists && trivialDefs(sym.owner) + + extension (defdef: tpd.DefDef) + // so trivial that it never consumes params + private def isTrivial(using Context): Boolean = + val rhs = defdef.rhs + rhs.symbol == ctx.definitions.Predef_undefined || rhs.tpe =:= ctx.definitions.NothingType || (rhs match { + case _: tpd.Literal => true + case _ => rhs.tpe.isInstanceOf[ConstantType] + }) + def registerTrivial(using Context): Unit = + if defdef.isTrivial then + trivialDefs += defdef.symbol + end UnusedData private object UnusedData: diff --git a/tests/neg-custom-args/fatal-warnings/i15503e.scala b/tests/neg-custom-args/fatal-warnings/i15503e.scala index 9ab8d5ba6c2f..36477c4a1107 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503e.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503e.scala @@ -1,23 +1,27 @@ // scalac: -Wunused:explicits +/* This goes around the "trivial method" detection */ +val default_val = 1 + def f1(a: Int) = a // OK -def f2(a: Int) = 1 // error +def f2(a: Int) = default_val // error def f3(a: Int)(using Int) = a // OK -def f4(a: Int)(using Int) = 1 // error +def f4(a: Int)(using Int) = default_val // error def f6(a: Int)(using Int) = summon[Int] // error def f7(a: Int)(using Int) = summon[Int] + a // OK -package scala2main: +package scala2main.unused.args: object happyBirthday { - def main(args: Array[String]): Unit = ??? // error + def main(args: Array[String]): Unit = println("Hello World") // error } -package scala2mainunused: +package scala2main: object happyBirthday { def main(args: Array[String]): Unit = // OK - val a = args.size - ??? + println(s"Hello World, there are ${args.size} arguments") } package scala3main: - @main def hello = ??? // OK \ No newline at end of file + /* This goes around the "trivial method" detection */ + val default_unit = () + @main def hello = println("Hello World") // OK \ No newline at end of file diff --git a/tests/neg-custom-args/fatal-warnings/i15503f.scala b/tests/neg-custom-args/fatal-warnings/i15503f.scala index 778f90b9fd43..db695da3490b 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503f.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503f.scala @@ -1,9 +1,12 @@ // scalac: -Wunused:implicits +/* This goes around the "trivial method" detection */ +val default_int = 1 + def f1(a: Int) = a // OK def f2(a: Int) = 1 // OK def f3(a: Int)(using Int) = a // error -def f4(a: Int)(using Int) = 1 // error +def f4(a: Int)(using Int) = default_int // error def f6(a: Int)(using Int) = summon[Int] // OK def f7(a: Int)(using Int) = summon[Int] + a // OK diff --git a/tests/neg-custom-args/fatal-warnings/i15503g.scala b/tests/neg-custom-args/fatal-warnings/i15503g.scala index dd5c5180267b..d4daea944184 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503g.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503g.scala @@ -1,9 +1,15 @@ // scalac: -Wunused:params +/* This goes around the "trivial method" detection */ +val default_int = 1 + def f1(a: Int) = a // OK -def f2(a: Int) = 1 // error +def f2(a: Int) = default_int // error def f3(a: Int)(using Int) = a // error -def f4(a: Int)(using Int) = 1 // error // error +def f4(a: Int)(using Int) = default_int // error // error def f6(a: Int)(using Int) = summon[Int] // error def f7(a: Int)(using Int) = summon[Int] + a // OK +/* --- Trivial method check --- */ +def g1(x: Int) = 1 // OK +def g2(x: Int) = ??? // OK \ No newline at end of file diff --git a/tests/neg-custom-args/fatal-warnings/i15503h.scala b/tests/neg-custom-args/fatal-warnings/i15503h.scala index c0a25f703dcd..f8d1d6f2202f 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503h.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503h.scala @@ -7,7 +7,7 @@ class A { val b = 2 // OK private def c = 2 // error - def d(using x:Int): Int = 1 // error + def d(using x:Int): Int = b // error def e(x: Int) = 1 // OK def f = val x = 1 // error diff --git a/tests/neg-custom-args/fatal-warnings/i15503i.scala b/tests/neg-custom-args/fatal-warnings/i15503i.scala index c645c0ae164b..2ff7be5a48a3 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503i.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503i.scala @@ -7,16 +7,20 @@ class A { import collection.mutable.{Map => MutMap} // OK private val a = 1 // error val b = 2 // OK + + /* This goes around the trivial method detection */ + val default_int = 12 + val someMap = MutMap() private def c1 = 2 // error private def c2 = 2 // OK def c3 = c2 - def d1(using x:Int): Int = 1 // error + def d1(using x:Int): Int = default_int // error def d2(using x:Int): Int = x // OK - def e1(x: Int) = 1 // error + def e1(x: Int) = default_int // error def e2(x: Int) = x // OK def f = val x = 1 // error @@ -35,9 +39,12 @@ class A { package foo.test.scala.annotation: import annotation.unused // OK + /* This goes around the trivial method detection */ + val default_int = 12 + def a1(a: Int) = a // OK - def a2(a: Int) = 1 // error - def a3(@unused a: Int) = 1 //OK + def a2(a: Int) = default_int // error + def a3(@unused a: Int) = default_int //OK def b1 = def f = 1 // error From d37d99ebaebe1efa291196d360a116bf3084e405 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Tue, 27 Dec 2022 13:12:28 +0100 Subject: [PATCH 35/42] Fix the import precedence in -Wunused:imports - Wildcard import are reported before named ones - Update the test suit --- .../tools/dotc/transform/CheckUnused.scala | 17 +++++++++++++++-- .../fatal-warnings/i15503a.scala | 8 +++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 44793c007fae..3f4e51aced96 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -351,16 +351,29 @@ object CheckUnused: val used = usedInScope.pop().toSet // used imports in this scope val imports = impInScope.pop().toSet - val kept = used.filter { t => + val kept = used.filterNot { t => val (sym, isAccessible, optName) = t // keep the symbol for outer scope, if it matches **no** import - !imports.exists { imp => + + // This is the first matching wildcard selector + var selWildCard: Option[ImportSelector] = None + + val exists = imports.exists { imp => sym.isInImport(imp, isAccessible, optName) match case None => false + case optSel@Some(sel) if sel.isWildcard => + if selWildCard.isEmpty then selWildCard = optSel + // We keep wildcard symbol for the end as they have the least precedence + false case Some(sel) => unusedImport -= sel true } + if !exists && selWildCard.isDefined then + unusedImport -= selWildCard.get + true // a matching import exists so the symbol won't be kept for outer scope + else + exists } // if there's an outer scope if usedInScope.nonEmpty then diff --git a/tests/neg-custom-args/fatal-warnings/i15503a.scala b/tests/neg-custom-args/fatal-warnings/i15503a.scala index 13408976ec57..28482c7addde 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503a.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503a.scala @@ -244,4 +244,10 @@ package foo.testing.rename.imports: import collection.mutable.{Set => MutSet2} // OK import collection.mutable.{Set => MutSet3} // error type A[X] = MutSet1[X] - val a = MutSet2(1) \ No newline at end of file + val a = MutSet2(1) + +//------------------------------------- +package foo.testing.imports.precedence: + import scala.collection.immutable.{BitSet => _, _} // error + import scala.collection.immutable.BitSet // OK + def t = BitSet.empty \ No newline at end of file From ae24d64b4f40223f21218e5464bac9ae7d5e55f5 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Tue, 27 Dec 2022 16:00:25 +0100 Subject: [PATCH 36/42] Improve -Wunused:locals --- .../tools/dotc/transform/CheckUnused.scala | 83 ++++++++++++++----- .../fatal-warnings/i15503b.scala | 40 +++++++++ 2 files changed, 101 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 3f4e51aced96..759af7eaa0f4 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -61,7 +61,6 @@ class CheckUnused extends MiniPhase: // ========== SETUP ============ override def prepareForUnit(tree: tpd.Tree)(using Context): Context = - val tree = ctx.compilationUnit.tpdTree val data = UnusedData() val fresh = ctx.fresh.setProperty(_key, data) fresh @@ -94,7 +93,11 @@ class CheckUnused extends MiniPhase: pushInBlockTemplatePackageDef(tree) override def prepareForValDef(tree: tpd.ValDef)(using Context): Context = - _key.unusedDataApply(_.registerDef(tree)) + _key.unusedDataApply{ud => + ud.registerDef(tree) + if tree.symbol.is(Flags.Module) then + ud.addIgnoredUsage(tree.symbol) + } override def prepareForDefDef(tree: tpd.DefDef)(using Context): Context = _key.unusedDataApply{ ud => @@ -103,6 +106,12 @@ class CheckUnused extends MiniPhase: ud.registerDef(tree) } + override def prepareForTypeDef(tree: tpd.TypeDef)(using Context): Context = + _key.unusedDataApply{ ud => + ud.registerDef(tree) + ud.addIgnoredUsage(tree.symbol) + } + override def prepareForBind(tree: tpd.Bind)(using Context): Context = _key.unusedDataApply(_.registerPatVar(tree)) @@ -112,6 +121,11 @@ class CheckUnused extends MiniPhase: // ========== MiniPhase Transform ========== + override def transformValDef(tree: tpd.ValDef)(using Context): tpd.Tree = + if tree.symbol.is(Flags.Module) then + _key.unusedDataApply(_.removeIgnoredUsage(tree.symbol)) + tree + override def transformBlock(tree: tpd.Block)(using Context): tpd.Tree = popOutBlockTemplatePackageDef() tree @@ -124,6 +138,10 @@ class CheckUnused extends MiniPhase: popOutBlockTemplatePackageDef() tree + override def transformTypeDef(tree: tpd.TypeDef)(using Context): tpd.Tree = + _key.unusedDataApply(_.removeIgnoredUsage(tree.symbol)) + tree + // ---------- MiniPhase HELPERS ----------- private def pushInBlockTemplatePackageDef(tree: tpd.Block | tpd.Template | tpd.PackageDef)(using Context): Context = @@ -175,6 +193,9 @@ class CheckUnused extends MiniPhase: case t:tpd.DefDef => prepareForDefDef(t) traverseChildren(tree)(using newCtx) + case t:tpd.TypeDef => + prepareForTypeDef(t) + traverseChildren(tree)(using newCtx) case t: tpd.Bind => prepareForBind(t) traverseChildren(tree)(using newCtx) @@ -255,21 +276,23 @@ object CheckUnused: private val unusedImport = MutSet[ImportSelector]() /* LOCAL DEF OR VAL / Private Def or Val / Pattern variables */ - private val localDefInScope = MutSet[tpd.ValOrDefDef]() - private val privateDefInScope = MutSet[tpd.ValOrDefDef]() - private val explicitParamInScope = MutSet[tpd.ValOrDefDef]() - private val implicitParamInScope = MutSet[tpd.ValOrDefDef]() + private val localDefInScope = MutSet[tpd.MemberDef]() + private val privateDefInScope = MutSet[tpd.MemberDef]() + private val explicitParamInScope = MutSet[tpd.MemberDef]() + private val implicitParamInScope = MutSet[tpd.MemberDef]() private val patVarsInScope = MutSet[tpd.Bind]() /* Unused collection collected at the end */ - private val unusedLocalDef = MutSet[tpd.ValOrDefDef]() - private val unusedPrivateDef = MutSet[tpd.ValOrDefDef]() - private val unusedExplicitParams = MutSet[tpd.ValOrDefDef]() - private val unusedImplicitParams = MutSet[tpd.ValOrDefDef]() + private val unusedLocalDef = MutSet[tpd.MemberDef]() + private val unusedPrivateDef = MutSet[tpd.MemberDef]() + private val unusedExplicitParams = MutSet[tpd.MemberDef]() + private val unusedImplicitParams = MutSet[tpd.MemberDef]() private val unusedPatVars = MutSet[tpd.Bind]() /** All used symbols */ private val usedDef = MutSet[Symbol]() + /** Do not register as used */ + private val doNotRegister = MutSet[Symbol]() /** Trivial definitions, avoid registering params */ private val trivialDefs = MutSet[Symbol]() @@ -296,14 +319,30 @@ object CheckUnused: * as the same element can be imported with different renaming */ def registerUsed(sym: Symbol, name: Option[Name])(using Context): Unit = - if !isConstructorOfSynth(sym) then + if !isConstructorOfSynth(sym) && !doNotRegister(sym) then usedInScope.top += ((sym, sym.isAccessibleAsIdent, name)) if sym.isConstructor && sym.exists then registerUsed(sym.owner, None) // constructor are "implicitly" imported with the class /** Register a list of found (used) symbols */ def registerUsed(syms: Seq[Symbol])(using Context): Unit = - usedInScope.top ++= syms.filterNot(isConstructorOfSynth).map(s => (s, s.isAccessibleAsIdent, None)) + usedInScope.top ++= + syms + .filterNot(s => isConstructorOfSynth(s) || doNotRegister(s)) + .map(s => (s, s.isAccessibleAsIdent, None)) + + /** Register a symbol that should be ignored */ + def addIgnoredUsage(sym: Symbol)(using Context): Unit = + doNotRegister += sym + if sym.is(Flags.Module) then + doNotRegister += sym.moduleClass + + /** Remove a symbol that shouldn't be ignored anymore */ + def removeIgnoredUsage(sym: Symbol)(using Context): Unit = + doNotRegister -= sym + if sym.is(Flags.Module) then + doNotRegister -= sym.moduleClass + /** Register an import */ def registerImport(imp: tpd.Import)(using Context): Unit = @@ -314,19 +353,19 @@ object CheckUnused: } /** Register (or not) some `val` or `def` according to the context, scope and flags */ - def registerDef(valOrDef: tpd.ValOrDefDef)(using Context): Unit = + def registerDef(memDef: tpd.MemberDef)(using Context): Unit = // register the annotations for usage - registerUsedAnnotation(valOrDef.symbol) - if !valOrDef.symbol.isUnusedAnnot then - if valOrDef.symbol.is(Param) && !isSyntheticMainParam(valOrDef.symbol) && !valOrDef.symbol.ownerIsTrivial then - if valOrDef.symbol.isOneOf(GivenOrImplicit) then - implicitParamInScope += valOrDef + registerUsedAnnotation(memDef.symbol) + if !memDef.symbol.isUnusedAnnot then + if memDef.symbol.is(Param) && !isSyntheticMainParam(memDef.symbol) && !memDef.symbol.ownerIsTrivial then + if memDef.symbol.isOneOf(GivenOrImplicit) then + implicitParamInScope += memDef else - explicitParamInScope += valOrDef + explicitParamInScope += memDef else if currScopeType.top == ScopeType.Local then - localDefInScope += valOrDef - else if currScopeType.top == ScopeType.Template && valOrDef.symbol.is(Private, butNot = SelfName) then - privateDefInScope += valOrDef + localDefInScope += memDef + else if currScopeType.top == ScopeType.Template && memDef.symbol.is(Private, butNot = SelfName) then + privateDefInScope += memDef /** Register pattern variable */ def registerPatVar(patvar: tpd.Bind)(using Context): Unit = diff --git a/tests/neg-custom-args/fatal-warnings/i15503b.scala b/tests/neg-custom-args/fatal-warnings/i15503b.scala index ef0a99a4fcfe..0587bf781299 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503b.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503b.scala @@ -47,3 +47,43 @@ class Foo { def f2 = f1 // OK f2 } + +// ---- SCALA 2 tests ---- + +package foo.scala2.tests: + class Outer { + class Inner + } + + trait Locals { + def f0 = { + var x = 1 // error + var y = 2 // OK + y = 3 + y + y + } + def f1 = { + val a = new Outer // OK + val b = new Outer // error + new a.Inner + } + def f2 = { + var x = 100 + x + } + } + + object Types { + def l1() = { + object HiObject { def f = this } // error + class Hi { // error + def f1: Hi = new Hi + def f2(x: Hi) = x + } + class DingDongDoobie // error + class Bippy // OK + type Something = Bippy // OK + type OtherThing = String // error + (new Bippy): Something + } + } From 527aa316b5714d24aa358194cea6ea59462e53bc Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Tue, 27 Dec 2022 17:19:03 +0100 Subject: [PATCH 37/42] Fix -Wunused:privates trait accessor --- compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 759af7eaa0f4..e71429d82268 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -356,7 +356,7 @@ object CheckUnused: def registerDef(memDef: tpd.MemberDef)(using Context): Unit = // register the annotations for usage registerUsedAnnotation(memDef.symbol) - if !memDef.symbol.isUnusedAnnot then + if !memDef.symbol.isUnusedAnnot && !memDef.symbol.isAllOf(Flags.AccessorCreationFlags) then if memDef.symbol.is(Param) && !isSyntheticMainParam(memDef.symbol) && !memDef.symbol.ownerIsTrivial then if memDef.symbol.isOneOf(GivenOrImplicit) then implicitParamInScope += memDef From bfa20a1f9452f728bc1876fab4ba3f41ad6bc293 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Tue, 27 Dec 2022 17:53:59 +0100 Subject: [PATCH 38/42] Fix -Wunused:locals,privates with recursive --- .../tools/dotc/transform/CheckUnused.scala | 17 ++++++++++------- .../fatal-warnings/i15503c.scala | 3 +++ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index e71429d82268..1a468dee7894 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -95,8 +95,7 @@ class CheckUnused extends MiniPhase: override def prepareForValDef(tree: tpd.ValDef)(using Context): Context = _key.unusedDataApply{ud => ud.registerDef(tree) - if tree.symbol.is(Flags.Module) then - ud.addIgnoredUsage(tree.symbol) + ud.addIgnoredUsage(tree.symbol) } override def prepareForDefDef(tree: tpd.DefDef)(using Context): Context = @@ -104,6 +103,7 @@ class CheckUnused extends MiniPhase: import ud.registerTrivial tree.registerTrivial ud.registerDef(tree) + ud.addIgnoredUsage(tree.symbol) } override def prepareForTypeDef(tree: tpd.TypeDef)(using Context): Context = @@ -121,11 +121,6 @@ class CheckUnused extends MiniPhase: // ========== MiniPhase Transform ========== - override def transformValDef(tree: tpd.ValDef)(using Context): tpd.Tree = - if tree.symbol.is(Flags.Module) then - _key.unusedDataApply(_.removeIgnoredUsage(tree.symbol)) - tree - override def transformBlock(tree: tpd.Block)(using Context): tpd.Tree = popOutBlockTemplatePackageDef() tree @@ -138,6 +133,14 @@ class CheckUnused extends MiniPhase: popOutBlockTemplatePackageDef() tree + override def transformValDef(tree: tpd.ValDef)(using Context): tpd.Tree = + _key.unusedDataApply(_.removeIgnoredUsage(tree.symbol)) + tree + + override def transformDefDef(tree: tpd.DefDef)(using Context): tpd.Tree = + _key.unusedDataApply(_.removeIgnoredUsage(tree.symbol)) + tree + override def transformTypeDef(tree: tpd.TypeDef)(using Context): tpd.Tree = _key.unusedDataApply(_.removeIgnoredUsage(tree.symbol)) tree diff --git a/tests/neg-custom-args/fatal-warnings/i15503c.scala b/tests/neg-custom-args/fatal-warnings/i15503c.scala index efaa7d389c08..e34587fa5802 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503c.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503c.scala @@ -12,6 +12,9 @@ class A: private[this] val f = e // OK private val g = f // OK + private def fac(x: Int): Int = // error + if x == 0 then 1 else x * fac(x - 1) + val x = 1 // OK def y = 2 // OK def z = g // OK \ No newline at end of file From c70fa68b308887a268346e745240e3232b5dd388 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Thu, 29 Dec 2022 19:33:48 +0100 Subject: [PATCH 39/42] Fixes for -Wunused:params - Also add a (adapted) Scala 2 tests suits --- .../tools/dotc/transform/CheckUnused.scala | 62 +++++++--- .../i15503-scala2/scala2-t11681.scala | 110 ++++++++++++++++++ 2 files changed, 154 insertions(+), 18 deletions(-) create mode 100644 tests/neg-custom-args/fatal-warnings/i15503-scala2/scala2-t11681.scala diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 1a468dee7894..0cab68ab60ee 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -7,7 +7,7 @@ import dotty.tools.dotc.ast.untpd.ImportSelector import dotty.tools.dotc.config.ScalaSettings import dotty.tools.dotc.core.Contexts.* import dotty.tools.dotc.core.Decorators.{em, i} -import dotty.tools.dotc.core.Flags.{Given, Implicit, GivenOrImplicit, Param, Private, SelfName, Synthetic} +import dotty.tools.dotc.core.Flags._ import dotty.tools.dotc.core.Phases.Phase import dotty.tools.dotc.core.StdNames import dotty.tools.dotc.report @@ -25,6 +25,7 @@ import dotty.tools.dotc.transform.MegaPhase.MiniPhase import dotty.tools.dotc.core.Annotations import dotty.tools.dotc.core.Definitions import dotty.tools.dotc.core.Types.ConstantType +import dotty.tools.dotc.core.NameKinds.WildcardParamName @@ -108,8 +109,9 @@ class CheckUnused extends MiniPhase: override def prepareForTypeDef(tree: tpd.TypeDef)(using Context): Context = _key.unusedDataApply{ ud => - ud.registerDef(tree) - ud.addIgnoredUsage(tree.symbol) + if !tree.symbol.is(Param) then // Ignore type parameter (as Scala 2) + ud.registerDef(tree) + ud.addIgnoredUsage(tree.symbol) } override def prepareForBind(tree: tpd.Bind)(using Context): Context = @@ -313,7 +315,7 @@ object CheckUnused: /** Register all annotations of this symbol's denotation */ def registerUsedAnnotation(sym: Symbol)(using Context): Unit = val annotSym = sym.denot.annotations.map(_.symbol) - registerUsed(annotSym) + annotSym.foreach(s => registerUsed(s, None)) /** * Register a found (used) symbol along with its name @@ -327,13 +329,6 @@ object CheckUnused: if sym.isConstructor && sym.exists then registerUsed(sym.owner, None) // constructor are "implicitly" imported with the class - /** Register a list of found (used) symbols */ - def registerUsed(syms: Seq[Symbol])(using Context): Unit = - usedInScope.top ++= - syms - .filterNot(s => isConstructorOfSynth(s) || doNotRegister(s)) - .map(s => (s, s.isAccessibleAsIdent, None)) - /** Register a symbol that should be ignored */ def addIgnoredUsage(sym: Symbol)(using Context): Unit = doNotRegister += sym @@ -359,8 +354,8 @@ object CheckUnused: def registerDef(memDef: tpd.MemberDef)(using Context): Unit = // register the annotations for usage registerUsedAnnotation(memDef.symbol) - if !memDef.symbol.isUnusedAnnot && !memDef.symbol.isAllOf(Flags.AccessorCreationFlags) then - if memDef.symbol.is(Param) && !isSyntheticMainParam(memDef.symbol) && !memDef.symbol.ownerIsTrivial then + if memDef.isValidMemberDef then + if memDef.isValidParam then if memDef.symbol.isOneOf(GivenOrImplicit) then implicitParamInScope += memDef else @@ -526,7 +521,7 @@ object CheckUnused: extension (sym: Symbol) /** is accessible without import in current context */ - def isAccessibleAsIdent(using Context): Boolean = + private def isAccessibleAsIdent(using Context): Boolean = sym.exists && ctx.outersIterator.exists{ c => c.owner == sym.owner @@ -536,7 +531,7 @@ object CheckUnused: } /** Given an import and accessibility, return an option of selector that match import<->symbol */ - def isInImport(imp: tpd.Import, isAccessible: Boolean, symName: Option[Name])(using Context): Option[ImportSelector] = + private def isInImport(imp: tpd.Import, isAccessible: Boolean, symName: Option[Name])(using Context): Option[ImportSelector] = val tpd.Import(qual, sels) = imp val qualHasSymbol = qual.tpe.member(sym.name).alternatives.map(_.symbol).contains(sym) def selector = sels.find(sel => (sel.name.toTermName == sym.name || sel.name.toTypeName == sym.name) && symName.map(n => n.toTermName == sel.rename).getOrElse(true)) @@ -547,17 +542,32 @@ object CheckUnused: None /** Annotated with @unused */ - def isUnusedAnnot(using Context): Boolean = + private def isUnusedAnnot(using Context): Boolean = sym.annotations.exists(a => a.symbol == ctx.definitions.UnusedAnnot) - def ownerIsTrivial(using Context): Boolean = + private def shouldNotReportParamOwner(using Context): Boolean = + if sym.exists then + val owner = sym.owner + trivialDefs(owner) || + owner.is(Flags.Override) || + owner.isPrimaryConstructor || + owner.annotations.exists ( + _.symbol == ctx.definitions.DeprecatedAnnot + ) + else + false + + private def ownerIsTrivial(using Context): Boolean = sym.exists && trivialDefs(sym.owner) extension (defdef: tpd.DefDef) // so trivial that it never consumes params private def isTrivial(using Context): Boolean = val rhs = defdef.rhs - rhs.symbol == ctx.definitions.Predef_undefined || rhs.tpe =:= ctx.definitions.NothingType || (rhs match { + rhs.symbol == ctx.definitions.Predef_undefined || + rhs.tpe =:= ctx.definitions.NothingType || + defdef.symbol.is(Deferred) || + (rhs match { case _: tpd.Literal => true case _ => rhs.tpe.isInstanceOf[ConstantType] }) @@ -565,6 +575,22 @@ object CheckUnused: if defdef.isTrivial then trivialDefs += defdef.symbol + extension (memDef: tpd.MemberDef) + private def isValidMemberDef(using Context): Boolean = + !memDef.symbol.isUnusedAnnot && !memDef.symbol.isAllOf(Flags.AccessorCreationFlags) && !memDef.name.isWildcard + + private def isValidParam(using Context): Boolean = + val sym = memDef.symbol + (sym.is(Param) || sym.isAllOf(PrivateParamAccessor)) && + !isSyntheticMainParam(sym) && + !sym.shouldNotReportParamOwner + + + extension (thisName: Name) + private def isWildcard: Boolean = + thisName == StdNames.nme.WILDCARD || thisName.is(WildcardParamName) + + end UnusedData private object UnusedData: diff --git a/tests/neg-custom-args/fatal-warnings/i15503-scala2/scala2-t11681.scala b/tests/neg-custom-args/fatal-warnings/i15503-scala2/scala2-t11681.scala new file mode 100644 index 000000000000..f04129a19e48 --- /dev/null +++ b/tests/neg-custom-args/fatal-warnings/i15503-scala2/scala2-t11681.scala @@ -0,0 +1,110 @@ +// scalac: -Wunused:params +// + +import Answers._ + +trait InterFace { + /** Call something. */ + def call(a: Int, b: String, c: Double): Int +} + +trait BadAPI extends InterFace { + def f(a: Int, + b: String, // error + c: Double): Int = { + println(c) + a + } + @deprecated("no warn in deprecated API", since="yesterday") + def g(a: Int, + b: String, // OK + c: Double): Int = { + println(c) + a + } + override def call(a: Int, + b: String, // OK + c: Double): Int = { + println(c) + a + } + + def meth(x: Int) = x + + override def equals(other: Any): Boolean = true // OK + + def i(implicit s: String) = answer // error + + /* + def future(x: Int): Int = { + val y = 42 + val x = y // maybe option to warn only if shadowed + x + } + */ +} + +// mustn't alter warnings in super +trait PoorClient extends BadAPI { + override def meth(x: Int) = ??? // OK + override def f(a: Int, b: String, c: Double): Int = a + b.toInt + c.toInt +} + +class Unusing(u: Int) { // error + def f = ??? +} + +class Valuing(val u: Int) // OK + +class Revaluing(u: Int) { def f = u } // OK + +case class CaseyKasem(k: Int) // OK + +case class CaseyAtTheBat(k: Int)(s: String) // error + +trait Ignorance { + def f(readResolve: Int) = answer // error +} + +class Reusing(u: Int) extends Unusing(u) // OK + +// TODO: check +// class Main { +// def main(args: Array[String]): Unit = println("hello, args") // OK +// } + +trait Unimplementation { + def f(u: Int): Int = ??? // OK +} + +trait DumbStuff { + def f(implicit dummy: DummyImplicit) = answer // todo // error + def g(dummy: DummyImplicit) = answer // error +} +trait Proofs { + def f[A, B](implicit ev: A =:= B) = answer // todo // error + def g[A, B](implicit ev: A <:< B) = answer // todo // error + def f2[A, B](ev: A =:= B) = answer // error + def g2[A, B](ev: A <:< B) = answer // error +} + +trait Anonymous { + def f = (i: Int) => answer // error + + def f1 = (_: Int) => answer // OK + + def f2: Int => Int = _ + 1 // OK + + def g = for (i <- List(1)) yield answer // error +} +trait Context[A] +trait Implicits { + def f[A](implicit ctx: Context[A]) = answer // error + def g[A: Context] = answer // error +} +class Bound[A: Context] // error +object Answers { + def answer: Int = 42 +} + +val a$1 = 2 \ No newline at end of file From 7de90b3179aa6ace65b07e141abdf34183510283 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Tue, 3 Jan 2023 17:15:10 +0100 Subject: [PATCH 40/42] Add better support for trivial methods -Wunused - Method having a return type of "SingleType" (as named in Scala 2) are considered trivial (i.e. Nil, object X, object Y, None, ...) - Update test suits --- .../tools/dotc/transform/CheckUnused.scala | 10 +++++-- .../fatal-warnings/i15503e.scala | 29 ++++++++++++++++++- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index 0cab68ab60ee..d5f4a7b5d58a 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -26,6 +26,7 @@ import dotty.tools.dotc.core.Annotations import dotty.tools.dotc.core.Definitions import dotty.tools.dotc.core.Types.ConstantType import dotty.tools.dotc.core.NameKinds.WildcardParamName +import dotty.tools.dotc.core.Types.TermRef @@ -569,7 +570,13 @@ object CheckUnused: defdef.symbol.is(Deferred) || (rhs match { case _: tpd.Literal => true - case _ => rhs.tpe.isInstanceOf[ConstantType] + case _ => rhs.tpe match + case ConstantType(_) => true + case tp: TermRef => + // Detect Scala 2 SingleType + tp.underlying.classSymbol.is(Flags.Module) + case _ => + false }) def registerTrivial(using Context): Unit = if defdef.isTrivial then @@ -590,7 +597,6 @@ object CheckUnused: private def isWildcard: Boolean = thisName == StdNames.nme.WILDCARD || thisName.is(WildcardParamName) - end UnusedData private object UnusedData: diff --git a/tests/neg-custom-args/fatal-warnings/i15503e.scala b/tests/neg-custom-args/fatal-warnings/i15503e.scala index 36477c4a1107..79112942a205 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503e.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503e.scala @@ -24,4 +24,31 @@ package scala2main: package scala3main: /* This goes around the "trivial method" detection */ val default_unit = () - @main def hello = println("Hello World") // OK \ No newline at end of file + @main def hello = println("Hello World") // OK + +package foo.test.lambda.param: + val default_val = 1 + val a = (i: Int) => i // OK + val b = (i: Int) => default_val // error + val c = (_: Int) => default_val // OK + +package foo.test.trivial: + /* A twisted test from Scala 2 */ + class C { + def answer: 42 = 42 + object X + def g0(x: Int) = ??? // OK + def f0(x: Int) = () // OK + def f1(x: Int) = throw new RuntimeException // OK + def f2(x: Int) = 42 // OK + def f3(x: Int): Option[Int] = None // OK + def f4(x: Int) = classOf[Int] // OK + def f5(x: Int) = answer + 27 // OK + def f6(x: Int) = X // OK + def f7(x: Int) = Y // OK + def f8(x: Int): List[C] = Nil // OK + def f9(x: Int): List[Int] = List(1,2,3,4) // error + def foo:Int = 32 // OK + def f77(x: Int) = foo // error + } + object Y From 4a06bc6f7e5a69419a50aeaef15872ebcdd02d60 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Thu, 5 Jan 2023 09:10:17 +0100 Subject: [PATCH 41/42] Remove unused method in CheckUnused --- compiler/src/dotty/tools/dotc/transform/CheckUnused.scala | 3 --- 1 file changed, 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala index d5f4a7b5d58a..5549e5c48d45 100644 --- a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala +++ b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala @@ -558,9 +558,6 @@ object CheckUnused: else false - private def ownerIsTrivial(using Context): Boolean = - sym.exists && trivialDefs(sym.owner) - extension (defdef: tpd.DefDef) // so trivial that it never consumes params private def isTrivial(using Context): Boolean = From 08f807cf3fb9228b844e24ac89778b1338b05644 Mon Sep 17 00:00:00 2001 From: Paul Coral Date: Thu, 5 Jan 2023 13:53:30 +0100 Subject: [PATCH 42/42] Make -Wunused:patvars to unsafe - Suppress -Wunused:patvars - Replace by -Wunused:unsafe-warn-patvars --- .../tools/dotc/config/ScalaSettings.scala | 33 +++++++++++-------- .../fatal-warnings/i15503d.scala | 15 ++++++++- .../fatal-warnings/i15503i.scala | 9 ++--- 3 files changed, 39 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala index 0d1fc34eb405..24fc19a46b2c 100644 --- a/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala +++ b/compiler/src/dotty/tools/dotc/config/ScalaSettings.scala @@ -171,18 +171,24 @@ private sealed trait WarningSettings: name = "imports", description = "Warn if an import selector is not referenced.\n" + "NOTE : overrided by -Wunused:strict-no-implicit-warn"), - ChoiceWithHelp("patvars","Warn if a variable bound in a pattern is unused"), - 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("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 ) @@ -206,7 +212,8 @@ private sealed trait WarningSettings: def privates(using Context) = allOr("privates") || allOr("linted") def patvars(using Context) = - allOr("patvars") + isChoiceSet("unsafe-warn-patvars") // not with "all" + // allOr("patvars") // todo : rename once fixed def linted(using Context) = allOr("linted") def strictNoImplicitWarn(using Context) = diff --git a/tests/neg-custom-args/fatal-warnings/i15503d.scala b/tests/neg-custom-args/fatal-warnings/i15503d.scala index 09a7b57bd89f..6c5973c66a3a 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503d.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503d.scala @@ -1,4 +1,5 @@ -// scalac: -Wunused:patvars +// scalac: -Wunused:unsafe-warn-patvars +// todo : change to :patvars sealed trait Calc sealed trait Const extends Calc @@ -8,10 +9,22 @@ case object Z extends Const val a = Sum(S(S(Z)),Z) match { case Sum(a,Z) => Z // error + // case Sum(a @ _,Z) => Z // todo : this should pass in the future case Sum(a@S(_),Z) => Z // error case Sum(a@S(_),Z) => a // OK case Sum(a@S(b@S(_)), Z) => a // error + case Sum(a@S(b@S(_)), Z) => a // error case Sum(a@S(b@(S(_))), Z) => Sum(a,b) // OK case Sum(_,_) => Z // OK case _ => Z // OK } + +// todo : This should pass in the future +// val b = for { +// case Some(x) <- Option(Option(1)) +// } println(s"$x") + +// todo : This should *NOT* pass in the future +// val c = for { +// case Some(x) <- Option(Option(1)) +// } println(s"hello world") diff --git a/tests/neg-custom-args/fatal-warnings/i15503i.scala b/tests/neg-custom-args/fatal-warnings/i15503i.scala index 2ff7be5a48a3..cecb79b70a34 100644 --- a/tests/neg-custom-args/fatal-warnings/i15503i.scala +++ b/tests/neg-custom-args/fatal-warnings/i15503i.scala @@ -29,10 +29,11 @@ class A { def g = 4 // OK y + g - def g(x: Int): Int = x match - case x:1 => 0 // error - case x:2 => x // OK - case _ => 1 // OK + // todo : uncomment once patvars is fixed + // def g(x: Int): Int = x match + // case x:1 => 0 // ?error + // case x:2 => x // ?OK + // case _ => 1 // ?OK } /* ---- CHECK scala.annotation.unused ---- */