From 63f1a3c5d0149d173eb8f65405b5983591752229 Mon Sep 17 00:00:00 2001 From: Shane Delmore Date: Mon, 24 Oct 2016 17:34:37 -0700 Subject: [PATCH 1/3] Improve error message for WrongNumberOfArgs --- .../dotc/reporting/diagnostic/messages.scala | 35 +++++++++++++++++++ .../tools/dotc/typer/ErrorReporting.scala | 4 +-- src/dotty/tools/dotc/typer/TypeAssigner.scala | 6 ++-- src/dotty/tools/dotc/typer/Typer.scala | 2 +- 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index f76512bc72e1..57d96224a311 100644 --- a/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -10,6 +10,7 @@ import util.{SourceFile, NoSource} import util.{SourcePosition, NoSourcePosition} import config.Settings.Setting import interfaces.Diagnostic.{ERROR, WARNING, INFO} +import dotty.tools.dotc.ast.tpd import printing.Highlighting._ import printing.Formatting @@ -605,4 +606,38 @@ object messages { |${"func(bool => // do something...)"} |""".stripMargin } + case class WrongNumberOfArgs(fntpe: Type, argKind: String, expectedArgs: List[TypeParamInfo], actual: List[untpd.Tree])(implicit ctx: Context) + extends Message(18) { + val kind = "Syntax" + val expectedCount = expectedArgs.length + val actualCount = actual.length + val msgPrefix = if (actualCount > expectedCount) "Too many" else "Not enough" + + val expectedArgString = (fntpe.widen match { + case pt: MethodOrPoly => pt //ensure we return a type that will have useful typeParms + case _ => fntpe + }).typeParams.map(_.paramName.show.split("\\$").last).mkString("[", ", ", "]") + + val actualArgString = actual.map(_.show.split("\\.").last).mkString("[", ", ", "]") + + val msg = + hl"""|$msgPrefix ${argKind} arguments for $fntpe + |expected: $expectedArgString + |actual: $actualArgString""".stripMargin + + val explanation = { + if (actualCount > expectedCount) + hl"""|You have supplied too many type parameters + | + |For example List takes a single type parameter (List[A]) + | If you need to hold more types in a list then you need to combine them + | into another data type that can contain the number of types you need, + | In this example one solution would be to use a Tuple: + | val tuple2: Tuple2[Int, String = (1, "one) + | List[(Int, String)] = List(tuple2)""".stripMargin + else + hl"""|You have not supplied enough type parameters + | If you specify one type parameter then you need to specify every type parameter.""".stripMargin + } + } } diff --git a/src/dotty/tools/dotc/typer/ErrorReporting.scala b/src/dotty/tools/dotc/typer/ErrorReporting.scala index 1d22dc6465ed..a18c83ff8cd3 100644 --- a/src/dotty/tools/dotc/typer/ErrorReporting.scala +++ b/src/dotty/tools/dotc/typer/ErrorReporting.scala @@ -49,8 +49,8 @@ object ErrorReporting { errorMsg(ex.show, ctx) } - def wrongNumberOfArgs(fntpe: Type, kind: String, expected: Int, pos: Position)(implicit ctx: Context) = - errorType(em"wrong number of ${kind}arguments for $fntpe, expected: $expected", pos) + def wrongNumberOfArgs(fntpe: Type, kind: String, expectedArgs: List[TypeParamInfo], actual: List[untpd.Tree], pos: Position)(implicit ctx: Context) = + errorType(WrongNumberOfArgs(fntpe, kind, expectedArgs, actual)(ctx), pos) class Errors(implicit ctx: Context) { diff --git a/src/dotty/tools/dotc/typer/TypeAssigner.scala b/src/dotty/tools/dotc/typer/TypeAssigner.scala index 861847b11d79..b1fed308cfd0 100644 --- a/src/dotty/tools/dotc/typer/TypeAssigner.scala +++ b/src/dotty/tools/dotc/typer/TypeAssigner.scala @@ -314,7 +314,7 @@ trait TypeAssigner { val ownType = fn.tpe.widen match { case fntpe @ MethodType(_, ptypes) => if (sameLength(ptypes, args) || ctx.phase.prev.relaxedTyping) fntpe.instantiate(args.tpes) - else wrongNumberOfArgs(fn.tpe, "", ptypes.length, tree.pos) + else wrongNumberOfArgs(fn.tpe, "", fntpe.typeParams, args, tree.pos) case t => errorType(i"${err.exprStr(fn)} does not take parameters", tree.pos) } @@ -369,7 +369,7 @@ trait TypeAssigner { else { val argTypes = args.tpes if (sameLength(argTypes, paramNames) || ctx.phase.prev.relaxedTyping) pt.instantiate(argTypes) - else wrongNumberOfArgs(fn.tpe, "type ", pt.paramNames.length, tree.pos) + else wrongNumberOfArgs(fn.tpe, "type", pt.typeParams, args, tree.pos) } case _ => errorType(i"${err.exprStr(fn)} does not take type parameters", tree.pos) @@ -451,7 +451,7 @@ trait TypeAssigner { val ownType = if (hasNamedArg(args)) (tycon.tpe /: args)(refineNamed) else if (sameLength(tparams, args)) tycon.tpe.appliedTo(args.tpes) - else wrongNumberOfArgs(tycon.tpe, "type ", tparams.length, tree.pos) + else wrongNumberOfArgs(tycon.tpe, "type", tparams, args, tree.pos) tree.withType(ownType) } diff --git a/src/dotty/tools/dotc/typer/Typer.scala b/src/dotty/tools/dotc/typer/Typer.scala index 6fb0dd7c7e6a..56c04c063716 100644 --- a/src/dotty/tools/dotc/typer/Typer.scala +++ b/src/dotty/tools/dotc/typer/Typer.scala @@ -1035,7 +1035,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit if (hasNamedArg(args)) typedNamedArgs(args) else { if (args.length != tparams.length) { - wrongNumberOfArgs(tpt1.tpe, "type ", tparams.length, tree.pos) + wrongNumberOfArgs(tpt1.tpe, "type", tparams, args, tree.pos) args = args.take(tparams.length) } def typedArg(arg: untpd.Tree, tparam: TypeParamInfo) = { From a2245dde469e29b0a93479a73961511e269b7440 Mon Sep 17 00:00:00 2001 From: Shane Delmore Date: Mon, 24 Oct 2016 18:20:29 -0700 Subject: [PATCH 2/3] Update IllegalVariableInPatternAlternative error message --- src/dotty/tools/dotc/ast/Desugar.scala | 2 +- .../dotc/reporting/diagnostic/messages.scala | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/dotty/tools/dotc/ast/Desugar.scala b/src/dotty/tools/dotc/ast/Desugar.scala index 8b8e0b3186a2..639dac930b6f 100644 --- a/src/dotty/tools/dotc/ast/Desugar.scala +++ b/src/dotty/tools/dotc/ast/Desugar.scala @@ -1051,7 +1051,7 @@ object desugar { elems foreach collect case Alternative(trees) => for (tree <- trees; (vble, _) <- getVariables(tree)) - ctx.error("illegal variable in pattern alternative", vble.pos) + ctx.error(IllegalVariableInPatternAlternative(), vble.pos) case Annotated(arg, _) => collect(arg) case InterpolatedString(_, segments) => diff --git a/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 57d96224a311..7e897f0815ac 100644 --- a/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -640,4 +640,30 @@ object messages { | If you specify one type parameter then you need to specify every type parameter.""".stripMargin } } + + case class IllegalVariableInPatternAlternative()(implicit ctx: Context) + extends Message(19) { + val kind = "Syntax" + + val msg = hl"""|Variables are not allowed in alternative patterns""" + + val explanation = { + hl"""|Variables are not allowed within alternate pattern matches. + |You can workaround this issue by adding additional cases for each alternative. + |For example, the illegal function: + | def g(pair: (Int,Int)): Int = pair match { + | case (1, n) | (n, 1) => n + | case _ => 0 + | } + | + | could be implemented by moving each alternative into a separate case: + | def g(pair: (Int,Int)): Int = pair match { + | case (1, n) => n + | case (n, 1) => n + | case _ => 0 + | } + | + |""".stripMargin + } + } } From aaae563d6de83e5f60ef763d384d1b4f76f9c1dd Mon Sep 17 00:00:00 2001 From: Shane Delmore Date: Tue, 25 Oct 2016 13:35:46 -0700 Subject: [PATCH 3/3] Improve error message formatting --- .../dotc/reporting/diagnostic/messages.scala | 66 +++++++++++-------- 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 7e897f0815ac..2913f4f4c49c 100644 --- a/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -4,7 +4,7 @@ package reporting package diagnostic import dotc.core._ -import Contexts.Context, Decorators._, Symbols._, Names._, Types._ +import Contexts.Context, Decorators._, Symbols._, Names._, NameOps._, Types._ import ast.untpd.{Modifiers, ModuleDef} import util.{SourceFile, NoSource} import util.{SourcePosition, NoSourcePosition} @@ -607,63 +607,75 @@ object messages { |""".stripMargin } case class WrongNumberOfArgs(fntpe: Type, argKind: String, expectedArgs: List[TypeParamInfo], actual: List[untpd.Tree])(implicit ctx: Context) - extends Message(18) { + extends Message(22) { val kind = "Syntax" val expectedCount = expectedArgs.length val actualCount = actual.length val msgPrefix = if (actualCount > expectedCount) "Too many" else "Not enough" - val expectedArgString = (fntpe.widen match { - case pt: MethodOrPoly => pt //ensure we return a type that will have useful typeParms - case _ => fntpe - }).typeParams.map(_.paramName.show.split("\\$").last).mkString("[", ", ", "]") + //TODO add def simpleParamName to TypeParamInfo + val expectedArgString = fntpe.widen.typeParams.map(_.paramName.unexpandedName.show).mkString("[", ", ", "]") - val actualArgString = actual.map(_.show.split("\\.").last).mkString("[", ", ", "]") + val actualArgString = actual.map(_.show).mkString("[", ", ", "]") + + val prettyName = fntpe.termSymbol match { + case NoSymbol => fntpe.show + case symbol => symbol.showFullName + } val msg = - hl"""|$msgPrefix ${argKind} arguments for $fntpe + hl"""|${NoColor(msgPrefix)} ${argKind} arguments for $prettyName$expectedArgString |expected: $expectedArgString |actual: $actualArgString""".stripMargin val explanation = { + val tooManyTypeParams = + """|val tuple2: (Int, String) = (1, "one") + |val list: List[(Int, String)] = List(tuple2)""".stripMargin + if (actualCount > expectedCount) hl"""|You have supplied too many type parameters | |For example List takes a single type parameter (List[A]) - | If you need to hold more types in a list then you need to combine them - | into another data type that can contain the number of types you need, - | In this example one solution would be to use a Tuple: - | val tuple2: Tuple2[Int, String = (1, "one) - | List[(Int, String)] = List(tuple2)""".stripMargin + |If you need to hold more types in a list then you need to combine them + |into another data type that can contain the number of types you need, + |In this example one solution would be to use a Tuple: + | + |${tooManyTypeParams}""".stripMargin else hl"""|You have not supplied enough type parameters - | If you specify one type parameter then you need to specify every type parameter.""".stripMargin + |If you specify one type parameter then you need to specify every type parameter.""".stripMargin } } case class IllegalVariableInPatternAlternative()(implicit ctx: Context) - extends Message(19) { + extends Message(23) { val kind = "Syntax" val msg = hl"""|Variables are not allowed in alternative patterns""" val explanation = { + val varInAlternative = + """|def g(pair: (Int,Int)): Int = pair match { + | case (1, n) | (n, 1) => n + | case _ => 0 + |}""".stripMargin + + val fixedVarInAlternative = + """|def g(pair: (Int,Int)): Int = pair match { + | case (1, n) => n + | case (n, 1) => n + | case _ => 0 + |}""".stripMargin + hl"""|Variables are not allowed within alternate pattern matches. |You can workaround this issue by adding additional cases for each alternative. |For example, the illegal function: - | def g(pair: (Int,Int)): Int = pair match { - | case (1, n) | (n, 1) => n - | case _ => 0 - | } - | - | could be implemented by moving each alternative into a separate case: - | def g(pair: (Int,Int)): Int = pair match { - | case (1, n) => n - | case (n, 1) => n - | case _ => 0 - | } | - |""".stripMargin + |$varInAlternative + |could be implemented by moving each alternative into a separate case: + | + |$fixedVarInAlternative""".stripMargin } } }