Detect anonymized patvar in given pattern

som-snytt · som-snytt · commit def4dd7a9cfb · 2025-05-07T18:24:48.000-07:00
diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala
@@ -13,13 +13,14 @@ import util.{Property, SourceFile, SourcePosition, SrcPos, Chars}
 import config.{Feature, Config}
 import config.Feature.{sourceVersion, migrateTo3, enabled}
 import config.SourceVersion.*
-import collection.mutable
 import reporting.*
 import printing.Formatting.hl
 import config.Printers
 import parsing.Parsers
+import dotty.tools.dotc.util.chaining.*
 
 import scala.annotation.{unchecked as _, *}, internal.sharable
+import scala.collection.mutable, mutable.ListBuffer
 
 object desugar {
   import untpd.*
@@ -1343,7 +1344,7 @@ object desugar {
       )).withSpan(tree.span)
   end makePolyFunctionType
 
-  /** Invent a name for an anonympus given of type or template `impl`. */
+  /** Invent a name for an anonymous given of type or template `impl`. */
   def inventGivenName(impl: Tree)(using Context): SimpleName =
     val str = impl match
       case impl: Template =>
@@ -2044,33 +2045,35 @@ object desugar {
           makeCaseLambda(CaseDef(gen.pat, EmptyTree, body) :: Nil, matchCheckMode)
       }
 
-      def hasGivenBind(pat: Tree): Boolean = pat.existsSubTree {
-        case pat @ Bind(_, pat1) => pat.mods.is(Given)
+      def hasGivenBind(pat: Tree): Boolean = pat.existsSubTree:
+        case pat @ Bind(_, _) => pat.mods.is(Given)
         case _ => false
-      }
 
       /** Does this pattern define any given bindings */
       def isNestedGivenPattern(pat: Tree): Boolean = pat match
-        case pat @ Bind(_, pat1) => hasGivenBind(pat1)
+        case Bind(_, pat) => hasGivenBind(pat)
         case _ => hasGivenBind(pat)
 
       /** If `pat` is not an Identifier, a Typed(Ident, _), or a Bind, wrap
        *  it in a Bind with a fresh name. Return the transformed pattern, and the identifier
        *  that refers to the bound variable for the pattern. Wildcard Binds are
        *  also replaced by Binds with fresh names.
        */
-      def makeIdPat(pat: Tree): (Tree, Ident) = pat match {
-        case bind @ Bind(name, pat1) =>
-          if name == nme.WILDCARD then
-            val name = UniqueName.fresh()
-            (cpy.Bind(pat)(name, pat1).withMods(bind.mods), Ident(name))
-          else (pat, Ident(name))
+      def makeIdPat(pat: Tree): (Tree, Ident) = pat match
+        case bind @ Bind(nme.WILDCARD, pat) =>
+          val name = UniqueName.fresh()
+          cpy.Bind(pat)(name, pat)
+            .withMods(bind.mods)
+            .withSpan(bind.span)
+            .withAttachment(PatternVar, ())
+          ->
+          Ident(name)
+        case Bind(name, _) => (pat, Ident(name))
         case id: Ident if isVarPattern(id) && id.name != nme.WILDCARD => (id, id)
         case Typed(id: Ident, _) if isVarPattern(id) && id.name != nme.WILDCARD => (pat, id)
         case _ =>
           val name = UniqueName.fresh()
           (Bind(name, pat), Ident(name))
-      }
 
       /** Make a pattern filter:
        *    rhs.withFilter { case pat => true case _ => false }
@@ -2173,36 +2176,44 @@ object desugar {
         case (gen: GenFrom) :: (rest @ (GenFrom(_, _, _) :: _)) =>
           val cont = makeFor(mapName, flatMapName, rest, body)
           Apply(rhsSelect(gen, flatMapName), makeLambda(gen, cont))
-        case (gen: GenFrom) :: rest
-        if sourceVersion.enablesBetterFors
-          && rest.dropWhile(_.isInstanceOf[GenAlias]).headOption.forall(e => e.isInstanceOf[GenFrom]) // possible aliases followed by a generator or end of for
-          && !rest.takeWhile(_.isInstanceOf[GenAlias]).exists(a => isNestedGivenPattern(a.asInstanceOf[GenAlias].pat)) =>
-          val cont = makeFor(mapName, flatMapName, rest, body)
-          val selectName =
-            if rest.exists(_.isInstanceOf[GenFrom]) then flatMapName
-            else mapName
-          val aply = Apply(rhsSelect(gen, selectName), makeLambda(gen, cont))
-          markTrailingMap(aply, gen, selectName)
-          aply
-        case (gen: GenFrom) :: (rest @ GenAlias(_, _) :: _) =>
-          val (valeqs, rest1) = rest.span(_.isInstanceOf[GenAlias])
-          val pats = valeqs map { case GenAlias(pat, _) => pat }
-          val rhss = valeqs map { case GenAlias(_, rhs) => rhs }
-          val (defpat0, id0) = makeIdPat(gen.pat)
-          val (defpats, ids) = (pats map makeIdPat).unzip
-          val pdefs = valeqs.lazyZip(defpats).lazyZip(rhss).map { (valeq, defpat, rhs) =>
-            val mods = defpat match
-              case defTree: DefTree => defTree.mods
-              case _ => Modifiers()
-            makePatDef(valeq, mods, defpat, rhs)
-          }
-          val rhs1 = makeFor(nme.map, nme.flatMap, GenFrom(defpat0, gen.expr, gen.checkMode) :: Nil, Block(pdefs, makeTuple(id0 :: ids).withAttachment(ForArtifact, ())))
-          val allpats = gen.pat :: pats
-          val vfrom1 = GenFrom(makeTuple(allpats), rhs1, GenCheckMode.Ignore)
-          makeFor(mapName, flatMapName, vfrom1 :: rest1, body)
+        case (gen: GenFrom) :: (tail @ GenAlias(_, _) :: _) =>
+          val (valeqs, suffix) = tail.span(_.isInstanceOf[GenAlias])
+          // possible aliases followed by a generator or end of for, when betterFors.
+          // exclude value definitions with a given pattern (given T = x)
+          val better = sourceVersion.enablesBetterFors
+            && suffix.headOption.forall(_.isInstanceOf[GenFrom])
+            && !valeqs.exists(a => isNestedGivenPattern(a.asInstanceOf[GenAlias].pat))
+          if better then
+            val cont = makeFor(mapName, flatMapName, enums = tail, body)
+            val selectName =
+              if suffix.exists(_.isInstanceOf[GenFrom]) then flatMapName
+              else mapName
+            Apply(rhsSelect(gen, selectName), makeLambda(gen, cont))
+              .tap(markTrailingMap(_, gen, selectName))
+          else
+            val pats = valeqs.map { case GenAlias(pat, _) => pat }
+            val rhss = valeqs.map { case GenAlias(_, rhs) => rhs }
+            val (defpat0, id0) = makeIdPat(gen.pat)
+            val (defpats, ids) = pats.map(makeIdPat).unzip
+            val pdefs = valeqs.lazyZip(defpats).lazyZip(rhss).map { (valeq, defpat, rhs) =>
+              val mods = defpat match
+                case defTree: DefTree => defTree.mods
+                case _ => Modifiers()
+              makePatDef(valeq, mods, defpat, rhs)
+            }
+            val rhs1 =
+              val enums = GenFrom(defpat0, gen.expr, gen.checkMode) :: Nil
+              val body = Block(pdefs, makeTuple(id0 :: ids).withAttachment(ForArtifact, ()))
+              makeFor(nme.map, nme.flatMap, enums, body)
+            val allpats = gen.pat :: pats
+            val vfrom1 = GenFrom(makeTuple(allpats), rhs1, GenCheckMode.Ignore)
+            makeFor(mapName, flatMapName, enums = vfrom1 :: suffix, body)
+          end if
         case (gen: GenFrom) :: test :: rest =>
-          val filtered = Apply(rhsSelect(gen, nme.withFilter), makeLambda(gen, test))
-          val genFrom = GenFrom(gen.pat, filtered, if sourceVersion.enablesBetterFors then GenCheckMode.Filtered else GenCheckMode.Ignore)
+          val genFrom =
+            val filtered = Apply(rhsSelect(gen, nme.withFilter), makeLambda(gen, test))
+            val mode = if sourceVersion.enablesBetterFors then GenCheckMode.Filtered else GenCheckMode.Ignore
+            GenFrom(gen.pat, filtered, mode)
           makeFor(mapName, flatMapName, genFrom :: rest, body)
         case GenAlias(_, _) :: _ if sourceVersion.enablesBetterFors =>
           val (valeqs, rest) = enums.span(_.isInstanceOf[GenAlias])
@@ -2396,7 +2407,7 @@ object desugar {
    *  without duplicates
    */
   private def getVariables(tree: Tree, shouldAddGiven: Context ?=> Bind => Boolean)(using Context): List[VarInfo] = {
-    val buf = mutable.ListBuffer[VarInfo]()
+    val buf = ListBuffer.empty[VarInfo]
     def seenName(name: Name) = buf exists (_._1.name == name)
     def add(named: NameTree, t: Tree): Unit =
       if (!seenName(named.name) && named.name.isTermName) buf += ((named, t))
diff --git a/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala b/compiler/src/dotty/tools/dotc/transform/CheckUnused.scala
@@ -444,10 +444,15 @@ object CheckUnused:
         then
           imps.put(imp, ())
       case tree: Bind =>
-        if !tree.name.isInstanceOf[DerivedName] && !tree.name.is(WildcardParamName) then
+        def addBindAt(pos: SrcPos): Unit =
           if tree.hasAttachment(NoWarn) then
             nowarn.addOne(tree.symbol)
-          pats.addOne((tree.symbol, tree.namePos))
+          pats.addOne((tree.symbol, pos))
+        if !tree.name.isInstanceOf[DerivedName] && !tree.name.is(WildcardParamName) then
+          addBindAt(tree.namePos)
+        else if tree.hasAttachment(PatternVar) then
+          addBindAt(tree.srcPos.sourcePos)
+          addRef(tree.symbol)
       case tree: NamedDefTree =>
         if tree.hasAttachment(PatternVar) then
           if !tree.name.isInstanceOf[DerivedName] then
@@ -611,6 +616,7 @@ object CheckUnused:
         if pos.span.isSynthetic then pos else pos.sourcePos.withSpan(pos.span.toSynthetic)
       // patvars in for comprehensions share the pos of where the name was introduced
       val byPos = infos.pats.groupMap(uniformPos(_, _))((sym, pos) => sym)
+      //println(infos.pats.mkString("PATVARS\n", "\n", "\n----\n"))
       for (pos, syms) <- byPos if pos.span.exists && !syms.exists(_.hasAnnotation(defn.UnusedAnnot)) do
         if !syms.exists(infos.refs(_)) then
           if !syms.exists(v => !v.isLocal && !v.is(Private) || infos.nowarn(v)) then
diff --git a/tests/pos/i23119.scala b/tests/pos/i23119.scala
@@ -0,0 +1,22 @@
+//> using options -Wunused:patvars -Werror
+def make: IndexedSeq[FalsePositive] =
+  for {
+    i <- 1 to 2
+    given Int = i
+    fp = FalsePositive()
+  } yield fp
+
+/* compare to
+def alt: IndexedSeq[FalsePositive] =
+  given String = "hi"
+  for {
+    i <- 1 to 2
+    k @ (j :: Nil) = List(i) // pattern in one var
+    //j: Int = i // simple assign because irrefutable
+    fp = FalsePositive(using j)
+  } yield fp
+*/
+
+class FalsePositive(using Int):
+  def usage(): Unit =
+    println(summon[Int])
