Fix CheckUnused missed some used symbols

- Register every symbol and related (companion, module, etc.) - Fix scala#16682 - Update test suits
little-inferno · Jan 25, 2023 · 2fa30bf · 2fa30bf
1 parent abffc80
commit 2fa30bf
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 13 deletions.
diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
@@ -102,7 +102,9 @@ class CheckUnused extends MiniPhase:
 
   override def prepareForValDef(tree: tpd.ValDef)(using Context): Context =
     _key.unusedDataApply{ud =>
-      ud.registerDef(tree)
+      // do not register the ValDef generated for `object`
+      if !tree.symbol.is(Module) then 
+        ud.registerDef(tree)
       ud.addIgnoredUsage(tree.symbol)
     }
 
@@ -341,15 +343,11 @@ object CheckUnused:
 
     /** 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
+      doNotRegister ++= sym.everySymbol
 
     /** 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
+      doNotRegister --= sym.everySymbol
 
 
     /** Register an import */
@@ -447,27 +445,37 @@ object CheckUnused:
           Nil
       val sortedLocalDefs =
         if ctx.settings.WunusedHas.locals then
-          localDefInScope.filter(d => !usedDef(d.symbol)).map(d => d.namePos -> WarnTypes.LocalDefs).toList
+          localDefInScope
+            .filterNot(d => d.symbol.usedDefContains)
+            .map(d => d.namePos -> WarnTypes.LocalDefs).toList
         else
           Nil
       val sortedExplicitParams =
         if ctx.settings.WunusedHas.explicits then
-          explicitParamInScope.filter(d => !usedDef(d.symbol)).map(d => d.namePos -> WarnTypes.ExplicitParams).toList
+          explicitParamInScope
+            .filterNot(d => d.symbol.usedDefContains)
+            .map(d => d.namePos -> WarnTypes.ExplicitParams).toList
         else
           Nil
       val sortedImplicitParams =
         if ctx.settings.WunusedHas.implicits then
-          implicitParamInScope.filter(d => !usedDef(d.symbol)).map(d => d.namePos -> WarnTypes.ImplicitParams).toList
+          implicitParamInScope
+            .filterNot(d => d.symbol.usedDefContains)
+            .map(d => d.namePos -> WarnTypes.ImplicitParams).toList
         else
           Nil
       val sortedPrivateDefs =
         if ctx.settings.WunusedHas.privates then
-          privateDefInScope.filter(d => !usedDef(d.symbol)).map(d => d.namePos -> WarnTypes.PrivateMembers).toList
+          privateDefInScope
+            .filterNot(d => d.symbol.usedDefContains)
+            .map(d => d.namePos -> WarnTypes.PrivateMembers).toList
         else
           Nil
       val sortedPatVars =
         if ctx.settings.WunusedHas.patvars then
-          patVarsInScope.filter(d => !usedDef(d.symbol)).map(d => d.namePos -> WarnTypes.PatVars).toList
+          patVarsInScope
+            .filterNot(d => d.symbol.usedDefContains)
+            .map(d => d.namePos -> WarnTypes.PatVars).toList
         else
           Nil
       val warnings = List(sortedImp, sortedLocalDefs, sortedExplicitParams, sortedImplicitParams, sortedPrivateDefs, sortedPatVars).flatten.sortBy { s =>
@@ -567,6 +575,14 @@ object CheckUnused:
         else
           false
 
+      private def usedDefContains(using Context): Boolean = 
+        sym.everySymbol.exists(usedDef.apply)
+
+      private def everySymbol(using Context): List[Symbol] = 
+        List(sym, sym.companionClass, sym.companionModule, sym.moduleClass).filter(_.exists)
+
+    end extension
+
     extension (defdef: tpd.DefDef)
       // so trivial that it never consumes params
       private def isTrivial(using Context): Boolean =

diff --git a/tests/neg-custom-args/fatal-warnings/i15503b.scala b/tests/neg-custom-args/fatal-warnings/i15503b.scala
@@ -87,3 +87,16 @@ package foo.scala2.tests:
       (new Bippy): Something
     }
   }
+
+package test.foo.twisted.i16682:
+  def myPackage = 
+    object IntExtractor: // OK
+      def unapply(s: String): Option[Int] = s.toIntOption
+
+    def isInt(s: String) = s match { // OK
+      case IntExtractor(i) => println(s"Number $i")
+      case _ => println("NaN")
+    }
+    isInt
+
+  def f = myPackage("42")
diff --git a/tests/neg-custom-args/fatal-warnings/i15503c.scala b/tests/neg-custom-args/fatal-warnings/i15503c.scala
@@ -24,4 +24,17 @@ package foo.test.contructors:
   class B private (val x: Int) // OK
   class C private (private val x: Int) // error
   class D private (private val x: Int): // OK
-    def y = x
+    def y = x
+
+
+package test.foo.i16682:
+  object myPackage:
+    private object IntExtractor: // OK
+        def unapply(s: String): Option[Int] = s.toIntOption
+
+    def isInt(s: String) = s match {
+        case IntExtractor(i) => println(s"Number $i")
+        case _ => println("NaN")
+    }
+
+  def f = myPackage.isInt("42")