diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2013-11-28 20:16:17 +0100 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2013-12-01 23:23:01 +0100 |
commit | e571c9cc3ee5a9e96b899285cdd2df3cdce06898 (patch) | |
tree | 7118c1dc5cf23dac0495d3c3e334be621de98916 /src | |
parent | 073ebbd20ce9775260b83a78ecf9ed6a3e6d3d9e (diff) | |
download | scala-e571c9cc3ee5a9e96b899285cdd2df3cdce06898.tar.gz scala-e571c9cc3ee5a9e96b899285cdd2df3cdce06898.tar.bz2 scala-e571c9cc3ee5a9e96b899285cdd2df3cdce06898.zip |
Better error messages for common Function/Tuple mistakes
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.
Diffstat (limited to 'src')
6 files changed, 56 insertions, 8 deletions
diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala index 0429e295b4..91db1bb92a 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala @@ -732,6 +732,7 @@ 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) @@ -739,9 +740,19 @@ self => 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 } } diff --git a/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala b/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala index 45698c0c76..8ff7824159 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/MatchOptimization.scala @@ -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 { diff --git a/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala b/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala index 7ecc2be9be..2472f63993 100644 --- a/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala +++ b/src/compiler/scala/tools/nsc/typechecker/ContextErrors.scala @@ -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"+ diff --git a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala index 695a1e2e24..b801b644fb 100644 --- a/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala +++ b/src/compiler/scala/tools/nsc/typechecker/TypeDiagnostics.scala @@ -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 diff --git a/src/compiler/scala/tools/nsc/typechecker/Typers.scala b/src/compiler/scala/tools/nsc/typechecker/Typers.scala index 6d799b0098..4973346eff 100644 --- a/src/compiler/scala/tools/nsc/typechecker/Typers.scala +++ b/src/compiler/scala/tools/nsc/typechecker/Typers.scala @@ -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 = @@ -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 diff --git a/src/reflect/scala/reflect/internal/util/Collections.scala b/src/reflect/scala/reflect/internal/util/Collections.scala index 738baddc08..7cc2952c96 100644 --- a/src/reflect/scala/reflect/internal/util/Collections.scala +++ b/src/reflect/scala/reflect/internal/util/Collections.scala @@ -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 { |