From e571c9cc3ee5a9e96b899285cdd2df3cdce06898 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Thu, 28 Nov 2013 20:16:17 +0100 Subject: 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. --- .../scala/tools/nsc/ast/parser/Parsers.scala | 13 +++++++++++- .../nsc/transform/patmat/MatchOptimization.scala | 3 --- .../tools/nsc/typechecker/ContextErrors.scala | 23 +++++++++++++++++++--- .../tools/nsc/typechecker/TypeDiagnostics.scala | 16 +++++++++++++++ .../scala/tools/nsc/typechecker/Typers.scala | 4 +++- 5 files changed, 51 insertions(+), 8 deletions(-) (limited to 'src/compiler') 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 -- cgit v1.2.3