Skip to content

Commit

Permalink
SI-6675 -Xlint arity enforcement for extractors
Browse files Browse the repository at this point in the history
Extractor Patterns changed in 2.10.0 to implement
the letter of the spec, which allows a single binding
to capture an entire TupleN. But this can hide arity
mismatches, especially if the case body uses the
bound value as an `Any`.

This change warns when this happens under -Xlint.
  • Loading branch information
retronym committed Jan 15, 2013
1 parent 9ea0a20 commit 692372c
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 10 deletions.
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/matching/Patterns.scala
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ trait Patterns extends ast.TreeDSL {
case _ => toPats(args)
}

def resTypes = analyzer.unapplyTypeList(unfn.symbol, unfn.tpe, args.length)
def resTypes = analyzer.unapplyTypeList(unfn.pos, unfn.symbol, unfn.tpe, args.length)
def resTypesString = resTypes match {
case Nil => "Boolean"
case xs => xs.mkString(", ")
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/transform/UnCurry.scala
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ abstract class UnCurry extends InfoTransform
val fn1 = withInPattern(false)(transform(fn))
val args1 = transformTrees(fn.symbol.name match {
case nme.unapply => args
case nme.unapplySeq => transformArgs(tree.pos, fn.symbol, args, analyzer.unapplyTypeList(fn.symbol, fn.tpe, args.length))
case nme.unapplySeq => transformArgs(tree.pos, fn.symbol, args, analyzer.unapplyTypeList(fn.pos, fn.symbol, fn.tpe, args.length))
case _ => sys.error("internal error: UnApply node has wrong symbol")
})
treeCopy.UnApply(tree, fn1, args1)
Expand Down
15 changes: 11 additions & 4 deletions src/compiler/scala/tools/nsc/typechecker/Infer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ trait Infer extends Checkable {
* n > 1 and unapply’s result type is Option[(T1, ..., Tn)], for some types T1, ..., Tn.
* the argument patterns p1, ..., pn are typed in turn with expected types T1, ..., Tn
*/
def extractorFormalTypes(resTp: Type, nbSubPats: Int, unappSym: Symbol): (List[Type], List[Type]) = {
def extractorFormalTypes(pos: Position, resTp: Type, nbSubPats: Int, unappSym: Symbol): (List[Type], List[Type]) = {
val isUnapplySeq = unappSym.name == nme.unapplySeq
val booleanExtractor = resTp.typeSymbolDirect == BooleanClass

Expand All @@ -87,11 +87,18 @@ trait Infer extends Checkable {
if (nbSubPats == 0 && booleanExtractor && !isUnapplySeq) Nil
else resTp.baseType(OptionClass).typeArgs match {
case optionTArg :: Nil =>
if (nbSubPats == 1)
def productArgs = getProductArgs(optionTArg)
if (nbSubPats == 1) {
if (isUnapplySeq) List(seqToRepeatedChecked(optionTArg))
else List(optionTArg)
else {
val productArity = productArgs.size
if (productArity > 1 && settings.lint.value)
global.currentUnit.warning(pos, s"extractor pattern binds a single value to a Product${productArity} of type ${optionTArg}")
List(optionTArg)
}
}
// TODO: update spec to reflect we allow any ProductN, not just TupleN
else getProductArgs(optionTArg) match {
else productArgs match {
case Nil if isUnapplySeq => List(seqToRepeatedChecked(optionTArg))
case tps if isUnapplySeq => tps.init :+ seqToRepeatedChecked(tps.last)
case tps => tps
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/scala/tools/nsc/typechecker/Typers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3465,7 +3465,7 @@ trait Typers extends Modes with Adaptations with Tags {
val resTp = fun1.tpe.finalResultType.normalize
val nbSubPats = args.length

val (formals, formalsExpanded) = extractorFormalTypes(resTp, nbSubPats, fun1.symbol)
val (formals, formalsExpanded) = extractorFormalTypes(fun0.pos, resTp, nbSubPats, fun1.symbol)
if (formals == null) duplErrorTree(WrongNumberOfArgsError(tree, fun))
else {
val args1 = typedArgs(args, mode, formals, formalsExpanded)
Expand Down Expand Up @@ -5311,7 +5311,7 @@ trait Typers extends Modes with Adaptations with Tags {

def typedUnApply(tree: UnApply) = {
val fun1 = typed(tree.fun)
val tpes = formalTypes(unapplyTypeList(tree.fun.symbol, fun1.tpe, tree.args.length), tree.args.length)
val tpes = formalTypes(unapplyTypeList(tree.fun.pos, tree.fun.symbol, fun1.tpe, tree.args.length), tree.args.length)
val args1 = map2(tree.args, tpes)(typedPattern)
treeCopy.UnApply(tree, fun1, args1) setType pt
}
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/scala/tools/nsc/typechecker/Unapplies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ trait Unapplies extends ast.TreeDSL
/** returns type list for return type of the extraction
* @see extractorFormalTypes
*/
def unapplyTypeList(ufn: Symbol, ufntpe: Type, nbSubPats: Int) = {
def unapplyTypeList(pos: Position, ufn: Symbol, ufntpe: Type, nbSubPats: Int) = {
assert(ufn.isMethod, ufn)
//Console.println("utl "+ufntpe+" "+ufntpe.typeSymbol)
ufn.name match {
case nme.unapply | nme.unapplySeq =>
val (formals, _) = extractorFormalTypes(unapplyUnwrap(ufntpe), nbSubPats, ufn)
val (formals, _) = extractorFormalTypes(pos, unapplyUnwrap(ufntpe), nbSubPats, ufn)
if (formals == null) throw new TypeError(s"$ufn of type $ufntpe cannot extract $nbSubPats sub-patterns")
else formals
case _ => throw new TypeError(ufn+" is not an unapply or unapplySeq")
Expand Down
4 changes: 4 additions & 0 deletions test/files/neg/t6675.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
t6675.scala:10: error: extractor pattern binds a single value to a Product3 of type (Int, Int, Int)
"" match { case X(b) => b } // should warn under -Xlint. Not an error because of SI-6111
^
one error found
1 change: 1 addition & 0 deletions test/files/neg/t6675.flags
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
-Xlint -Xfatal-warnings
13 changes: 13 additions & 0 deletions test/files/neg/t6675.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
object X {
def unapply(s: String): Option[(Int,Int,Int)] = Some((1,2,3))
}

object Y {
def unapplySeq(s: String): Option[Seq[(Int,Int,Int)]] = Some(Seq((1,2,3)))
}

object Test {
"" match { case X(b) => b } // should warn under -Xlint. Not an error because of SI-6111

"" match { case Y(b) => b } // no warning
}

0 comments on commit 692372c

Please sign in to comment.