Loosey-goosey implication in selector
If the shadowed element appears anywhere in the selector expression,
don't warn about the shadowing pattern variable.
This means that there might be no type relationship between the two
symbols, and that elements of tuples might be swapped, i.e.,
there is no ordering relationship between arguments in the selector
and in the pattern.
For example, `(x, y) match { case (y, x) => }`.
For example, `t.toString match { case t => }`.
This commit is contained in:
Som Snytt
parent
43c386c1dc
commit
e534c4475c
|
|
@ -2046,28 +2046,48 @@ abstract class RefChecks extends Transform {
|
|||
if (isMultiline && settings.warnByNameImplicit) checkImplicitlyAdaptedBlockResult(expr)
|
||||
|
||||
tree
|
||||
case Match(selector0, cases) =>
|
||||
val selector = selector0 match {
|
||||
case Typed(expr, _) => expr
|
||||
case _ => selector0
|
||||
case Match(selector, cases) =>
|
||||
// only warn if it could be put in backticks in a pattern
|
||||
def isWarnable(sym: Symbol): Boolean =
|
||||
sym != null && sym.exists &&
|
||||
!sym.hasPackageFlag && sym.isStable && !isByName(sym) &&
|
||||
!sym.hasAttachment[PatVarDefAttachment.type] // val (_, v) = with one var is shadowed in desugaring
|
||||
//!toCheck.isSynthetic // self-type symbols are synthetic: value self (<synthetic>), do warn
|
||||
|
||||
class CheckSelector extends InternalTraverser {
|
||||
var selectorSymbols: List[Symbol] = null
|
||||
override def traverse(t: Tree): Unit = {
|
||||
val include = t match {
|
||||
case _: This => true // !t.symbol.isStable
|
||||
case _: SymTree => isWarnable(t.symbol)
|
||||
case _ => false
|
||||
}
|
||||
if (include) selectorSymbols ::= t.symbol
|
||||
t.traverse(this)
|
||||
}
|
||||
// true if the shadowed toCheck appears in the selector expression
|
||||
def implicatesSelector(toCheck: Symbol): Boolean = {
|
||||
if (selectorSymbols == null) {
|
||||
selectorSymbols = Nil
|
||||
apply(selector)
|
||||
}
|
||||
selectorSymbols.exists(sym => sym.eq(toCheck) || sym.accessedOrSelf.eq(toCheck.accessedOrSelf) ||
|
||||
toCheck.isThisSym && toCheck.owner == sym) // self match { case self: S => }, selector C.this is class symbol
|
||||
}
|
||||
}
|
||||
def notNull(s: Symbol) = if (s != null) s else NoSymbol
|
||||
val selectorSymbol = notNull(selector.symbol)
|
||||
val checkSelector = new CheckSelector
|
||||
// true to warn about shadowed when selSym is the scrutinee
|
||||
def checkShadowed(shadowed: Symbol)(selSym: Symbol): Boolean = {
|
||||
def checkShadowed(shadowed: Symbol): Boolean = {
|
||||
def checkShadowedSymbol(toCheck: Symbol): Boolean =
|
||||
//!toCheck.isSynthetic && // self-type symbols are synthetic: value self (<synthetic>)
|
||||
!toCheck.hasPackageFlag && toCheck.isStable && !isByName(toCheck) &&
|
||||
!toCheck.hasAttachment[PatVarDefAttachment.type] && // val (_, v) = with one var
|
||||
(selSym.eq(NoSymbol) || // following checks if selector symbol exists
|
||||
toCheck.ne(selSym) && toCheck.accessedOrSelf.ne(selSym.accessedOrSelf) &&
|
||||
!(toCheck.isThisSym && toCheck.owner == selSym)) // self match { case self: S => }, selector C.this is class symbol
|
||||
if (shadowed.isOverloaded) shadowed.alternatives.exists(checkShadowedSymbol) else checkShadowedSymbol(shadowed)
|
||||
isWarnable(toCheck) && !checkSelector.implicatesSelector(toCheck)
|
||||
|
||||
if (shadowed.isOverloaded) shadowed.alternatives.exists(checkShadowedSymbol)
|
||||
else checkShadowedSymbol(shadowed)
|
||||
}
|
||||
// warn if any checkable pattern var shadows, in the context of the given selector
|
||||
// warn if any checkable pattern var shadows, in the context of the selector,
|
||||
// or for `tree match case Apply(fun, args) =>` check whether names in args equal names of fun.params
|
||||
def check(p: Tree, selSym: Symbol): Unit = {
|
||||
val traverser = new Traverser {
|
||||
def checkPattern(p: Tree): Unit = {
|
||||
val traverser = new InternalTraverser {
|
||||
// names absolved of shadowing because it is a "current" parameter (of a case class, etc)
|
||||
var absolved: List[Name] = Nil
|
||||
override def traverse(t: Tree): Unit = t match {
|
||||
|
|
@ -2079,7 +2099,7 @@ abstract class RefChecks extends Transform {
|
|||
try traverse(arg)
|
||||
finally absolved = absolved.tail
|
||||
}
|
||||
case _ => super.traverse(t)
|
||||
case _ => t.traverse(this)
|
||||
}
|
||||
case bind @ Bind(name, _) =>
|
||||
def richLocation(sym: Symbol): String = sym.ownsString match {
|
||||
|
|
@ -2088,35 +2108,17 @@ abstract class RefChecks extends Transform {
|
|||
}
|
||||
for (shade <- bind.getAndRemoveAttachment[PatShadowAttachment]) {
|
||||
val shadowed = shade.shadowed
|
||||
if (!absolved.contains(name) && !bind.symbol.hasTransOwner(shadowed.accessedOrSelf) && checkShadowed(shadowed)(selSym))
|
||||
if (!absolved.contains(name) && !bind.symbol.hasTransOwner(shadowed.accessedOrSelf) && checkShadowed(shadowed))
|
||||
refchecksWarning(bind.pos, s"Name $name is already introduced in an enclosing scope as ${richLocation(shadowed)}. Did you intend to match it using backquoted `$name`?", WarningCategory.OtherShadowing)
|
||||
|
||||
}
|
||||
case _ => super.traverse(t)
|
||||
case _ => t.traverse(this)
|
||||
}
|
||||
}
|
||||
traverser.traverse(p)
|
||||
traverser(p)
|
||||
}
|
||||
// check the patterns for unfriendly shadowing, patvars bearing PatShadowAttachment
|
||||
def checkShadowingPatternVars(): Unit =
|
||||
selector match {
|
||||
// or for `(x, y) match case p(x, y) =>` check the corresponding args
|
||||
case treeInfo.Applied(Select(core, nme.apply), _, tupleArgs :: Nil) if isTupleSymbol(core.symbol.companion) =>
|
||||
for (cdef <- cases)
|
||||
cdef.pat match {
|
||||
case Apply(_, args) if sameLength(args, tupleArgs) =>
|
||||
foreach2(args, tupleArgs)((arg, sel) => check(arg, notNull(sel.symbol)))
|
||||
case UnApply(_, args) if sameLength(args, tupleArgs) =>
|
||||
foreach2(args, tupleArgs)((arg, sel) => check(arg, notNull(sel.symbol)))
|
||||
case p => check(p, selectorSymbol)
|
||||
}
|
||||
// or for `tp.dealiased match case tp =>` check `tp` if it looks like a derived value, but not `this.x` or `x.asInstanceOf`
|
||||
case treeInfo.Applied(Select(core, _), _, _) if core.symbol != null && !core.symbol.isThisSym && selector.tpe <:< core.tpe.widen =>
|
||||
for (cdef <- cases) check(cdef.pat, core.symbol)
|
||||
case _ =>
|
||||
for (cdef <- cases) check(cdef.pat, selectorSymbol)
|
||||
}
|
||||
if (settings.warnPatternShadow) checkShadowingPatternVars()
|
||||
if (settings.warnPatternShadow) for (cdef <- cases) checkPattern(cdef.pat)
|
||||
tree
|
||||
case _ => tree
|
||||
}
|
||||
|
|
|
|||
|
|
@ -4,27 +4,18 @@ t11850.scala:9: warning: Name x is already introduced in an enclosing scope as v
|
|||
t11850.scala:28: warning: Name x is already introduced in an enclosing scope as value x in class CT. Did you intend to match it using backquoted `x`?
|
||||
case x => 1 // warn
|
||||
^
|
||||
t11850.scala:118: warning: Name x is already introduced in an enclosing scope as value x at line 117. Did you intend to match it using backquoted `x`?
|
||||
case Y(x, y) => x+y // only allow 1-1
|
||||
^
|
||||
t11850.scala:118: warning: Name y is already introduced in an enclosing scope as value y at line 117. Did you intend to match it using backquoted `y`?
|
||||
case Y(x, y) => x+y // only allow 1-1
|
||||
^
|
||||
t11850.scala:125: warning: Name self is already introduced in an enclosing scope as value self in class Selfie. Did you intend to match it using backquoted `self`?
|
||||
t11850.scala:135: warning: Name self is already introduced in an enclosing scope as value self in class Selfie. Did you intend to match it using backquoted `self`?
|
||||
case (x, self) => x
|
||||
^
|
||||
t11850.scala:170: warning: Name _1 is already introduced in an enclosing scope as value _1 in class weird but true. Did you intend to match it using backquoted `_1`?
|
||||
case (_1, 27) => 3 // briefly did not warn as param name
|
||||
^
|
||||
t11850.scala:187: warning: Name x is already introduced in an enclosing scope as value x at line 186. Did you intend to match it using backquoted `x`?
|
||||
t11850.scala:202: warning: Name x is already introduced in an enclosing scope as value x at line 201. Did you intend to match it using backquoted `x`?
|
||||
case x => x.toString
|
||||
^
|
||||
t11850.scala:203: warning: Name c is already introduced in an enclosing scope as value c in class pattern matches occasionally appear in pattern-matching anonymous functions. Did you intend to match it using backquoted `c`?
|
||||
t11850.scala:224: warning: Name c is already introduced in an enclosing scope as value c in class pattern matches occasionally appear in pattern-matching anonymous functions. Did you intend to match it using backquoted `c`?
|
||||
def f = c.collect { case c if c.flag => c.toString }
|
||||
^
|
||||
t11850.scala:212: warning: Name x is already introduced in an enclosing scope as object x in object is it worth qualifying what kind of term. Did you intend to match it using backquoted `x`?
|
||||
t11850.scala:233: warning: Name x is already introduced in an enclosing scope as object x in object is it worth qualifying what kind of term. Did you intend to match it using backquoted `x`?
|
||||
case x => 1
|
||||
^
|
||||
error: No warnings can be incurred under -Werror.
|
||||
9 warnings
|
||||
6 warnings
|
||||
1 error
|
||||
|
|
|
|||
|
|
@ -104,20 +104,30 @@ object Y { def unapply(p: (Int, Int, Int)): Option[(Int, Int)] = Option(p).map {
|
|||
class Tupling {
|
||||
def f(x: Int, y: Int): Int = (x, y) match {
|
||||
case (42, 27) => 5
|
||||
case (x, y) => x+y // correspond to tuple arg
|
||||
case (x, y) => x+y // correspond to tuple arg (or anywhere in selector)
|
||||
}
|
||||
def g(x: Some[Int], y: Some[Int]): Int = (x, y) match {
|
||||
case (Some(42), Some(27)) => 5
|
||||
case (Some(x), Some(y)) => x+y // correspond to tuple arg but not top level pattern
|
||||
case (Some(x), Some(y)) => x+y // correspond to tuple arg but not top level pattern (or anywhere in selector)
|
||||
}
|
||||
def e(x: Int, y: Int): Int = (x, y) match {
|
||||
case X(x, y) => x+y // extractor args correspond to tuple args
|
||||
case X(x, y) => x+y // extractor args correspond to tuple args (or anywhere in selector)
|
||||
case _ => -1
|
||||
}
|
||||
def err(x: Int, y: Int, z: Int): Int = (x, y, z) match {
|
||||
case Y(x, y) => x+y // only allow 1-1
|
||||
case Y(x, y) => x+y // only allow 1-1 (or anywhere in selector)
|
||||
case _ => -1
|
||||
}
|
||||
def swap(x: Int, y: Int): Int = (x, y) match {
|
||||
case X(y, x) => x+y // anywhere in selector
|
||||
case _ => -1
|
||||
}
|
||||
def add1(x: Int): Int = x + 1 match {
|
||||
case x => x+42 // anywhere in selector
|
||||
}
|
||||
def add2(x: Int): Int = 1 + x match {
|
||||
case x => x+42 // anywhere in selector
|
||||
}
|
||||
}
|
||||
class Selfie { self =>
|
||||
def f(x: Int, y: Selfie): Int = (x, y) match {
|
||||
|
|
@ -183,10 +193,21 @@ class `derived thing is refinement` {
|
|||
}
|
||||
}
|
||||
class `kosher selector` {
|
||||
def f(x: Any) = x.toString match {
|
||||
case x => x
|
||||
}
|
||||
}
|
||||
class `unkosher selector` {
|
||||
def f(x: Any) = 42 match {
|
||||
case x => x.toString
|
||||
}
|
||||
}
|
||||
class `also unkosher selector` {
|
||||
// selector is a value derived from x but it is an unrelated type; x does not "refine" Thing
|
||||
def f(x: Thing) = x.toString match {
|
||||
case x => x
|
||||
}
|
||||
}
|
||||
class `lukas asked whats that null check for` {
|
||||
import annotation._
|
||||
def isOperatorPart(c: Char): Boolean = (c: @unchecked) match {
|
||||
|
|
|
|||
Loading…
Reference in New Issue