Warn when a pattern variable shadows a val/var in enclosing scope

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

@@ -284,6 +284,7 @@ private sealed trait WarningSettings:
       ChoiceWithHelp("all", ""),
       ChoiceWithHelp("private-shadow", "Warn if a private field or class parameter shadows a superclass field"),
       ChoiceWithHelp("type-parameter-shadow", "Warn when a type parameter shadows a type already in the scope"),
+      ChoiceWithHelp("pattern-variable-shadow", "Warn when a pattern variable shadows a variable in enclosing scope"),
     ),
     default = Nil
   )
@@ -298,6 +299,8 @@ private sealed trait WarningSettings:
       allOr("private-shadow")
     def typeParameterShadow(using Context) =
       allOr("type-parameter-shadow")
+    def patternVariableShadow(using Context) =
+      allOr("pattern-variable-shadow")
   end WshadowHas
 
   val WsafeInit: Setting[Boolean] = BooleanSetting(WarningSetting, "Wsafe-init", "Ensure safe initialization of objects.")

compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

@@ -241,6 +241,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case InferUnionWarningID // errorNumber: 225
   case TypeParameterShadowsTypeID // errorNumber: 226
   case PrivateShadowsTypeID // errorNumber: 227
+  case PatternVariableShadowsTypeID // errorNumber: 228
 
   def errorNumber = ordinal - 1
 

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -3887,3 +3887,12 @@ final class PrivateShadowsType(shadow: Symbol, shadowed: Symbol)(using Context)
     i"""A private field shadows an inherited field with the same name.
        |This can lead to confusion as the inherited field becomes inaccessible.
        |Consider renaming the private field to avoid the shadowing."""
+
+final class PatternVariableShadowsType(shadow: Symbol, shadowed: Symbol)(using Context)
+    extends NamingMsg(PatternVariableShadowsTypeID):
+  override protected def msg(using Context): String =
+    i"pattern variable ${shadow.name} shadows ${shadowed.showLocated}"
+  override protected def explain(using Context): String =
+    i"""A pattern variable shadows an existing variable in an enclosing scope.
+       |This can lead to subtle bugs as the outer variable becomes inaccessible.
+       |Consider renaming the pattern variable to avoid the shadowing."""

compiler/src/dotty/tools/dotc/transform/CheckShadowing.scala

@@ -3,8 +3,9 @@ package dotty.tools.dotc.transform
 import dotty.tools.dotc.ast.tpd
 import dotty.tools.dotc.transform.MegaPhase.MiniPhase
 import dotty.tools.dotc.report
-import dotty.tools.dotc.reporting.{Message, TypeParameterShadowsType, PrivateShadowsType}
+import dotty.tools.dotc.reporting.{Message, TypeParameterShadowsType, PrivateShadowsType, PatternVariableShadowsType}
 import dotty.tools.dotc.core.Contexts.*
+import dotty.tools.dotc.core.Decorators.*
 import dotty.tools.dotc.core.Flags.*
 import dotty.tools.dotc.util.{Property, SrcPos}
 import dotty.tools.dotc.core.Names.Name
@@ -14,6 +15,7 @@ import dotty.tools.dotc.core.Symbols
 import dotty.tools.dotc.core.Denotations.SingleDenotation
 import dotty.tools.dotc.ast.Trees.Ident
 import dotty.tools.dotc.core.Names.SimpleName
+import dotty.tools.dotc.core.StdNames.nme
 
 class CheckShadowing extends MiniPhase:
   import CheckShadowing.*
@@ -63,8 +65,20 @@ class CheckShadowing extends MiniPhase:
   override def prepareForValDef(tree: tpd.ValDef)(using Context): Context =
     shadowingDataApply(sd =>
       sd.registerPrivateShadows(tree)
+      sd.registerLocal(tree.symbol)
     )
 
+  override def prepareForDefDef(tree: tpd.DefDef)(using Context): Context =
+    shadowingDataApply(sd =>
+      sd.inNewScope()
+      tree.termParamss.flatten.foreach(p => sd.registerLocal(p.symbol))
+    )
+    ctx
+
+  override def prepareForCaseDef(tree: tpd.CaseDef)(using Context): Context =
+    shadowingDataApply(sd => sd.inNewScope())
+    ctx
+
   override def prepareForTypeDef(tree: tpd.TypeDef)(using Context): Context =
     val sym = tree.symbol
     if sym.isAliasType then // if alias, the parent is the current symbol
@@ -98,6 +112,21 @@ class CheckShadowing extends MiniPhase:
       shadowingDataApply(sd => sd.computeTypeParamShadowsFor(tree.symbol)(using ctx))
     tree
 
+  override def transformBind(tree: tpd.Bind)(using Context): tpd.Tree =
+    shadowingDataApply(sd =>
+      sd.checkPatternVariableShadow(tree)
+      sd.registerLocal(tree.symbol)
+    )
+    tree
+
+  override def transformDefDef(tree: tpd.DefDef)(using Context): tpd.Tree =
+    shadowingDataApply(sd => sd.outOfScope())
+    tree
+
+  override def transformCaseDef(tree: tpd.CaseDef)(using Context): tpd.Tree =
+    shadowingDataApply(sd => sd.outOfScope())
+    tree
+
   private def isValidTypeParamOwner(owner: Symbol)(using Context): Boolean =
     !owner.isConstructor && !owner.is(Synthetic) && !owner.is(Exported)
 
@@ -149,19 +178,27 @@ object CheckShadowing:
     private val rootImports = MutSet[SingleDenotation]()
     private val explicitsImports = MutStack[MutSet[tpd.Import]]()
     private val renamedImports = MutStack[MutMap[SimpleName, Name]]() // original name -> renamed name
+    private var locals: List[MutSet[Symbol]] = Nil
 
     private val typeParamCandidates = MutMap[Symbol, Seq[tpd.TypeDef]]().withDefaultValue(Seq())
     private val typeParamShadowWarnings = MutSet[ShadowWarning]()
 
     private val privateShadowWarnings = MutSet[ShadowWarning]()
+    private val patternShadowWarnings = MutSet[ShadowWarning]()
 
     def inNewScope()(using Context) =
       explicitsImports.push(MutSet())
       renamedImports.push(MutMap())
+      locals = MutSet() :: locals
 
     def outOfScope()(using Context) =
       explicitsImports.pop()
       renamedImports.pop()
+      locals = locals.tail
+
+    def registerLocal(sym: Symbol)(using Context) =
+      if locals.nonEmpty then
+        locals.head += sym
 
     /** Register the Root imports (at once per compilation unit)*/
     def registerRootImports()(using Context) =
@@ -234,6 +271,42 @@ object CheckShadowing:
       else
         None
 
+    def checkPatternVariableShadow(tree: tpd.Bind)(using Context): Unit =
+      if ctx.settings.WshadowHas.patternVariableShadow && tree.name.isTermName then
+        val sym = tree.symbol
+        lookForShadowedPatternVar(sym).foreach(shadowed =>
+          // skip if we are shadowing a method, which is generally allowed (e.g. x @ Something())
+          if shadowed.exists && shadowed.isTerm && !shadowed.is(Method) then
+             patternShadowWarnings += ShadowWarning(tree.srcPos, PatternVariableShadowsType(sym, shadowed))
+        )
+
+    private def lookForShadowedPatternVar(sym: Symbol)(using Context): Option[Symbol] =
+      lookForImportedShadowedTerm(sym)
+        .orElse(lookForLocalShadowedTerm(sym))
+        .orElse(lookForUnitShadowedTerm(sym))
+
+    private def lookForLocalShadowedTerm(symbol: Symbol)(using Context): Option[Symbol] =
+      // We check locals.tail because locals.head corresponds to the current scope (e.g. the CaseDef or Block)
+      // where the variable is defined. Shadowing is about outer scopes.
+      if locals.nonEmpty then
+        locals.tail.iterator.flatMap(_.find(s => s.name == symbol.name && s != symbol)).nextOption
+      else None
+
+    private def lookForImportedShadowedTerm(symbol: Symbol)(using Context): Option[Symbol] =
+      explicitsImports.flatMap(_.flatMap(imp => symbol.isAnImportedTerm(imp))).headOption
+
+    private def lookForUnitShadowedTerm(symbol: Symbol)(using Context): Option[Symbol] =
+       def loop(ctx: Context): Option[Symbol] =
+         if ctx.scope != null then
+           val s = ctx.scope.lookup(symbol.name)
+           if s.exists && s != symbol then Some(s)
+           else if ctx.outer != null && ctx.outer != ctx then loop(ctx.outer)
+           else None
+         else if ctx.outer != null && ctx.outer != ctx then loop(ctx.outer)
+         else None
+
+       if ctx.outer != null then loop(ctx.outer) else None
+
     /** Get the shadowing analysis's result */
     def getShadowingResult(using Context): List[ShadowWarning] =
       val privateWarnings: List[ShadowWarning] =
@@ -246,7 +319,12 @@ object CheckShadowing:
           typeParamShadowWarnings.toList
         else
           Nil
-      privateWarnings ++ typeParamWarnings
+      val patternWarnings: List[ShadowWarning] =
+        if ctx.settings.WshadowHas.patternVariableShadow then
+          patternShadowWarnings.toList
+        else
+          Nil
+      privateWarnings ++ typeParamWarnings ++ patternWarnings
 
     extension (sym: Symbol)
       /** Looks after any type import symbol in the given import that matches this symbol */
@@ -259,6 +337,21 @@ object CheckShadowing:
           .orElse(typeSelections.map(_.symbol).find(sd => sd.name == sym.name))
           .orElse(simpleSelections.map(_.symbol).find(sd => sd.name == sym.name))
 
+      /** Looks after any term import symbol in the given import that matches this symbol */
+      private def isAnImportedTerm(imp: tpd.Import)(using Context): Option[Symbol] =
+        val tpd.Import(qual, sels) = imp
+        val name = sym.name
+        val explicit = sels.find(_.rename == name).map { sel =>
+           qual.tpe.member(sel.name).symbol
+        }
+        explicit.orElse {
+          if sels.exists(_.name == nme.WILDCARD) then
+             val member = qual.tpe.member(name)
+             if member.exists then Some(member.symbol) else None
+          else None
+        }
+
   end ShadowingData
 
+
 end CheckShadowing

compiler/src/dotty/tools/dotc/typer/Typer.scala

@@ -2898,6 +2898,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
     assignType(cpy.TypeBoundsTree(tree)(lo2, hi2, alias1), lo2, hi2, alias1)
   end typedTypeBoundsTree
 
+
   def typedBind(tree: untpd.Bind, pt: Type)(using Context): Tree = {
     if !isFullyDefined(pt, ForceDegree.all) then
       return errorTree(tree, em"expected type of $tree is not fully defined")

tests/warn/i10749.check

@@ -0,0 +1,36 @@
+-- [E228] Naming Warning: tests/warn/i10749.scala:9:16 -----------------------------------------------------------------
+9 |      case Some(x) => x  // warn: pattern variable shadows outer x
+  |                ^
+  |                pattern variable x shadows value x
+  |
+  | longer explanation available when compiling with `-explain`
+-- [E228] Naming Warning: tests/warn/i10749.scala:14:9 -----------------------------------------------------------------
+14 |    case y => y  // warn: pattern variable shadows parameter y
+   |         ^
+   |         pattern variable y shadows parameter y
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E228] Naming Warning: tests/warn/i10749.scala:33:21 ----------------------------------------------------------------
+33 |      case Some(Some(outer)) => outer  // warn: shadows outer val
+   |                     ^^^^^
+   |                     pattern variable outer shadows value outer
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E228] Naming Warning: tests/warn/i10749.scala:41:17 ----------------------------------------------------------------
+41 |      case (Some(a),  // warn
+   |                 ^
+   |                 pattern variable a shadows value a
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E228] Naming Warning: tests/warn/i10749.scala:42:17 ----------------------------------------------------------------
+42 |            Some(b)) => a + b  // warn
+   |                 ^
+   |                 pattern variable b shadows value b
+   |
+   | longer explanation available when compiling with `-explain`
+-- [E228] Naming Warning: tests/warn/i10749.scala:49:11 ----------------------------------------------------------------
+49 |      Some(x) <- List(Some(1))  // warn: shadows x
+   |           ^
+   |           pattern variable x shadows value x
+   |
+   | longer explanation available when compiling with `-explain`

tests/warn/i10749.scala

@@ -0,0 +1,57 @@
+//> using options -Wshadow:pattern-variable-shadow
+
+object Test:
+
+  // Test 1: Pattern shadows local val
+  def test1 =
+    val x = 1
+    (Some(2): Option[Int]) match
+      case Some(x) => x  // warn: pattern variable shadows outer x
+      case None => 0
+
+  // Test 2: Pattern shadows parameter
+  def test2(y: Int) = y match
+    case y => y  // warn: pattern variable shadows parameter y
+
+  // Test 3: Wildcard is OK
+  def test3 =
+    val z = 1
+    (Some(2): Option[Int]) match
+      case Some(_) => 0  // ok: wildcard doesn't create binding
+      case None => z
+
+  // Test 4: No shadowing
+  def test4 =
+    (Some(2): Option[Int]) match
+      case Some(fresh) => fresh  // ok: no shadowing
+      case None => 0
+
+  // Test 5: Nested match shadows outer pattern variable (not just outer val)
+  def test5 =
+    val outer = 1
+    (Some(Some(2)): Option[Option[Int]]) match
+      case Some(Some(outer)) => outer  // warn: shadows outer val
+      case _ => 0
+
+  // Test 6: Multiple patterns in a tuple
+  def test6 =
+    val a = 1
+    val b = 2
+    ((Some(1): Option[Int]), (Some(2): Option[Int])) match
+      case (Some(a),  // warn
+            Some(b)) => a + b  // warn
+      case _ => 0
+
+  // Test 7: Pattern in for-comprehension
+  def test7 =
+    val x = 1
+    for
+      Some(x) <- List(Some(1))  // warn: shadows x
+    yield x
+
+  // Test 8: Stable identifier - should NOT warn (different semantics)
+  def test8 =
+    val y = 1
+    (Some(1): Option[Int]) match
+      case Some(`y`) => 1  // ok: backticks mean stable identifier match, not new binding
+      case _ => 0