Skip to content

Commit

Permalink
Better error messages for common Function/Tuple mistakes
Browse files Browse the repository at this point in the history
Firstly, for `((a, b) => c): (Tuple2[A, B] => C)`, we currently
just offer "missing parameter type." Is something of a rite of
passage to know that you need `{ case (...)}`

This commit stops short DWIM, but does offer a diagnostic to guide
the user towards the supported way of destructuring a `Tuple` in
the sole argument of a `Function1`.

Secondly, another (less common?) way one might try to write a function
to destructure a single tuple argument is:

    (((a, b)) => c)

The parser now matches offers a specific error message for this, and
points out the alternatives.

In both cases, we avoid offering syntactically invalid alternatives,
by detecting names that aren't valid as variable-patterns, and
falling back to generic "paramN" in the error message.

A handly utility function to sequence a list of options is liberated
from the pattern matcher for broader use.
  • Loading branch information
retronym committed Dec 1, 2013
1 parent 073ebbd commit e571c9c
Show file tree
Hide file tree
Showing 10 changed files with 119 additions and 8 deletions.
13 changes: 12 additions & 1 deletion src/compiler/scala/tools/nsc/ast/parser/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -732,16 +732,27 @@ self =>
def removeAsPlaceholder(name: Name) {
placeholderParams = placeholderParams filter (_.name != name)
}
def errorParam = makeParam(nme.ERROR, errorTypeTree setPos o2p(tree.pos.end))
tree match {
case Ident(name) =>
removeAsPlaceholder(name)
makeParam(name.toTermName, TypeTree() setPos o2p(tree.pos.end))
case Typed(Ident(name), tpe) if tpe.isType => // get the ident!
removeAsPlaceholder(name)
makeParam(name.toTermName, tpe)
case build.SyntacticTuple(as) =>
val arity = as.length
val example = analyzer.exampleTuplePattern(as map { case Ident(name) => name; case _ => nme.EMPTY })
val msg =
sm"""|not a legal formal parameter.
|Note: Tuples cannot be directly destructured in method or function parameters.
| Either create a single parameter accepting the Tuple${arity},
| or consider a pattern matching anonymous function: `{ case $example => ... }"""
syntaxError(tree.pos, msg, skipIt = false)
errorParam
case _ =>
syntaxError(tree.pos, "not a legal formal parameter", skipIt = false)
makeParam(nme.ERROR, errorTypeTree setPos o2p(tree.pos.end))
errorParam
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,6 @@ trait MatchOptimization extends MatchTreeMaking with MatchAnalysis {
def defaultBody: Tree
def defaultCase(scrutSym: Symbol = defaultSym, guard: Tree = EmptyTree, body: Tree = defaultBody): CaseDef

private def sequence[T](xs: List[Option[T]]): Option[List[T]] =
if (xs exists (_.isEmpty)) None else Some(xs.flatten)

object GuardAndBodyTreeMakers {
def unapply(tms: List[TreeMaker]): Option[(Tree, Tree)] = {
tms match {
Expand Down
23 changes: 20 additions & 3 deletions src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala
Original file line number Diff line number Diff line change
Expand Up @@ -408,11 +408,28 @@ trait ContextErrors {
setError(tree)
}

def MissingParameterTypeError(fun: Tree, vparam: ValDef, pt: Type) =
def MissingParameterTypeError(fun: Tree, vparam: ValDef, pt: Type, withTupleAddendum: Boolean) = {
def issue(what: String) = {
val addendum: String = fun match {
case Function(params, _) if withTupleAddendum =>
val funArity = params.length
val example = analyzer.exampleTuplePattern(params map (_.name))
(pt baseType FunctionClass(1)) match {
case TypeRef(_, _, arg :: _) if arg.typeSymbol == TupleClass(funArity) && funArity > 1 =>
sm"""|
|Note: The expected type requires a one-argument function accepting a $funArity-Tuple.
| Consider a pattern matching anoynmous function, `{ case $example => ... }`"""
case _ => ""
}
case _ => ""
}
issueNormalTypeError(vparam, what + addendum)
}
if (vparam.mods.isSynthetic) fun match {
case Function(_, Match(_, _)) => MissingParameterTypeAnonMatchError(vparam, pt)
case _ => issueNormalTypeError(vparam, "missing parameter type for expanded function " + fun)
} else issueNormalTypeError(vparam, "missing parameter type")
case _ => issue("missing parameter type for expanded function " + fun)
} else issue("missing parameter type")
}

def MissingParameterTypeAnonMatchError(vparam: Tree, pt: Type) =
issueNormalTypeError(vparam, "missing parameter type for expanded function\n"+
Expand Down
16 changes: 16 additions & 0 deletions src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,22 @@ trait TypeDiagnostics {
case x => x.toString
}

/**
* [a, b, c] => "(a, b, c)"
* [a, B] => "(param1, param2)"
* [a, B, c] => "(param1, ..., param2)"
*/
final def exampleTuplePattern(names: List[Name]): String = {
val arity = names.length
val varPatterNames: Option[List[String]] = sequence(names map {
case name if nme.isVariableName(name) => Some(name.decode)
case _ => None
})
def parenthesize(a: String) = s"($a)"
def genericParams = (Seq("param1") ++ (if (arity > 2) Seq("...") else Nil) ++ Seq(s"param$arity"))
parenthesize(varPatterNames.getOrElse(genericParams).mkString(", "))
}

def alternatives(tree: Tree): List[Type] = tree.tpe match {
case OverloadedType(pre, alternatives) => alternatives map pre.memberType
case _ => Nil
Expand Down
4 changes: 3 additions & 1 deletion src/compiler/scala/tools/nsc/typechecker/Typers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2906,6 +2906,7 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
else if (argpts.lengthCompare(numVparams) != 0)
WrongNumberOfParametersError(fun, argpts)
else {
var issuedMissingParameterTypeError = false
foreach2(fun.vparams, argpts) { (vparam, argpt) =>
if (vparam.tpt.isEmpty) {
vparam.tpt.tpe =
Expand All @@ -2923,7 +2924,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
}
case _ =>
}
MissingParameterTypeError(fun, vparam, pt)
MissingParameterTypeError(fun, vparam, pt, withTupleAddendum = !issuedMissingParameterTypeError)
issuedMissingParameterTypeError = true
ErrorType
}
if (!vparam.tpt.pos.isDefined) vparam.tpt setPos vparam.pos.focus
Expand Down
5 changes: 5 additions & 0 deletions src/reflect/scala/reflect/internal/util/Collections.scala
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,11 @@ trait Collections {
true
}

final def sequence[A](as: List[Option[A]]): Option[List[A]] = {
if (as.exists (_.isEmpty)) None
else Some(as.flatten)
}

final def transposeSafe[A](ass: List[List[A]]): Option[List[List[A]]] = try {
Some(ass.transpose)
} catch {
Expand Down
31 changes: 31 additions & 0 deletions test/files/neg/missing-param-type-tuple.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
missing-param-type-tuple.scala:3: error: missing parameter type
Note: The expected type requires a one-argument function accepting a 2-Tuple.
Consider a pattern matching anoynmous function, `{ case (a, b) => ... }`
val x: ((Int, Int)) => Int = (a, b) => 0
^
missing-param-type-tuple.scala:3: error: missing parameter type
val x: ((Int, Int)) => Int = (a, b) => 0
^
missing-param-type-tuple.scala:5: error: missing parameter type
Note: The expected type requires a one-argument function accepting a 3-Tuple.
Consider a pattern matching anoynmous function, `{ case (param1, ..., param3) => ... }`
val y: ((Int, Int, Int)) => Int = (a, b, !!) => 0
^
missing-param-type-tuple.scala:5: error: missing parameter type
val y: ((Int, Int, Int)) => Int = (a, b, !!) => 0
^
missing-param-type-tuple.scala:5: error: missing parameter type
val y: ((Int, Int, Int)) => Int = (a, b, !!) => 0
^
missing-param-type-tuple.scala:7: error: missing parameter type
Note: The expected type requires a one-argument function accepting a 3-Tuple.
Consider a pattern matching anoynmous function, `{ case (param1, ..., param3) => ... }`
val z: ((Int, Int, Int)) => Int = (a, NotAVariablePatternName, c) => 0
^
missing-param-type-tuple.scala:7: error: missing parameter type
val z: ((Int, Int, Int)) => Int = (a, NotAVariablePatternName, c) => 0
^
missing-param-type-tuple.scala:7: error: missing parameter type
val z: ((Int, Int, Int)) => Int = (a, NotAVariablePatternName, c) => 0
^
8 errors found
8 changes: 8 additions & 0 deletions test/files/neg/missing-param-type-tuple.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class C {

val x: ((Int, Int)) => Int = (a, b) => 0

val y: ((Int, Int, Int)) => Int = (a, b, !!) => 0

val z: ((Int, Int, Int)) => Int = (a, NotAVariablePatternName, c) => 0
}
19 changes: 19 additions & 0 deletions test/files/neg/not-a-legal-formal-parameter-tuple.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
not-a-legal-formal-parameter-tuple.scala:2: error: not a legal formal parameter.
Note: Tuples cannot be directly destructured in method or function parameters.
Either create a single parameter accepting the Tuple2,
or consider a pattern matching anonymous function: `{ case (a, b) => ... }
val x: ((Int, Int) => Int) = (((a, b)) => a)
^
not-a-legal-formal-parameter-tuple.scala:3: error: not a legal formal parameter.
Note: Tuples cannot be directly destructured in method or function parameters.
Either create a single parameter accepting the Tuple2,
or consider a pattern matching anonymous function: `{ case (param1, param2) => ... }
val y: ((Int, Int, Int) => Int) = (((a, !!)) => a)
^
not-a-legal-formal-parameter-tuple.scala:4: error: not a legal formal parameter.
Note: Tuples cannot be directly destructured in method or function parameters.
Either create a single parameter accepting the Tuple3,
or consider a pattern matching anonymous function: `{ case (param1, ..., param3) => ... }
val z: ((Int, Int, Int) => Int) = (((a, NotAPatternVariableName, c)) => a)
^
three errors found
5 changes: 5 additions & 0 deletions test/files/neg/not-a-legal-formal-parameter-tuple.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class C {
val x: ((Int, Int) => Int) = (((a, b)) => a)
val y: ((Int, Int, Int) => Int) = (((a, !!)) => a)
val z: ((Int, Int, Int) => Int) = (((a, NotAPatternVariableName, c)) => a)
}

0 comments on commit e571c9c

Please sign in to comment.