Skip to content

#1589: Improve error message for WrongNumberOfArgs #1623

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 25, 2016
Merged

#1589: Improve error message for WrongNumberOfArgs #1623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
63f1a3c
Improve error message for WrongNumberOfArgs
ShaneDelmore Oct 25, 2016
a2245dd
Update IllegalVariableInPatternAlternative error message
ShaneDelmore Oct 25, 2016
aaae563
Improve error message formatting
ShaneDelmore Oct 25, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/ast/Desugar.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) =>
Expand Down
75 changes: 74 additions & 1 deletion src/dotty/tools/dotc/reporting/diagnostic/messages.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@ 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}
import config.Settings.Setting
import interfaces.Diagnostic.{ERROR, WARNING, INFO}
import dotty.tools.dotc.ast.tpd
import printing.Highlighting._
import printing.Formatting

Expand Down Expand Up @@ -605,4 +606,76 @@ 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(22) {
val kind = "Syntax"
val expectedCount = expectedArgs.length
val actualCount = actual.length
val msgPrefix = if (actualCount > expectedCount) "Too many" else "Not enough"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would wrap this in NoColor so that it's not highlighted in the msg.


//TODO add def simpleParamName to TypeParamInfo
val expectedArgString = fntpe.widen.typeParams.map(_.paramName.unexpandedName.show).mkString("[", ", ", "]")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was supposed to have a //TODO right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it now.


val actualArgString = actual.map(_.show).mkString("[", ", ", "]")

val prettyName = fntpe.termSymbol match {
case NoSymbol => fntpe.show
case symbol => symbol.showFullName
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to make another PR to add def prettyName (or something like it) to a few types. I will leave it as a follow up unless you prefer it as part of this PR.

With these changes

object Test {
  var bad = List[Int, String]
  case class Bar[D, E](baz: D, qux: E)
  var bad3: Bar[Int] = null
}

results in

-- [E019] Syntax Error: test.scala -------------------------------------------------------------------------------------
16 |  var bad = List[Int, String]
   |            ^^^^^^^^^^^^^^^^^
   |            Too many type arguments for scala.collection.immutable.List.apply[A]
   |            expected: [A]
   |            actual:   [Int, String]

Explanation
===========
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: (Int, String) = (1, "one")
val list: List[(Int, String)] = List(tuple2)

-- [E019] Syntax Error: test.scala -------------------------------------------------------------------------------------
19 |  var bad3: Bar[Int] = null
   |            ^^^^^^^^
   |            Not enough type arguments for Test.Bar[D, E]
   |            expected: [D, E]
   |            actual:   [Int]

Explanation
===========
You have not supplied enough type parameters
If you specify one type parameter then you need to specify every type parameter.


val msg =
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:
|
|${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
}
}

case class IllegalVariableInPatternAlternative()(implicit ctx: Context)
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:
|
|$varInAlternative
|could be implemented by moving each alternative into a separate case:
|
|$fixedVarInAlternative""".stripMargin
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example message for too many type args

-- [E018] Syntax Error: test.scala -------------------------------------------------------------------------------------
16 |  var bad = List[Int, String]
   |            ^^^^^^^^^^^^^^^^^
   |Too many type arguments for ([scala$collection$immutable$List$apply$$A] => (xs: A*)scala.collection.immutable.List[A])(List.apply)
   |expected: [A]
   |actual:   [Int, String]

Explanation
===========
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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example message for not enough type args

-- [E018] Syntax Error: test.scala -------------------------------------------------------------------------------------
17 |  var bad2 = Map[(Int, String)]
   |             ^^^^^^^^^^^^^^^^^^
   |Not enough type arguments for ([scala$collection$generic$GenMapFactory$apply$$A, scala$collection$generic$GenMapFactory$apply$$B] =>
   |  (elems: (A, B)*)scala.collection.immutable.Map[A, B]
   |)(Map.apply)
   |expected: [A, B]
   |actual:   [Tuple2[Int, String]]

Explanation
===========
You have not supplied enough type parameters
  If you specify one type parameter then you need to specify every type parameter.

}
4 changes: 2 additions & 2 deletions src/dotty/tools/dotc/typer/ErrorReporting.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {

Expand Down
6 changes: 3 additions & 3 deletions src/dotty/tools/dotc/typer/TypeAssigner.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/typer/Typer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) = {
Expand Down