From 1b28a05232d7aa4a8e74d60bd9db1e29af0547ea Mon Sep 17 00:00:00 2001 From: Igor Mielientiev Date: Wed, 22 Mar 2017 14:25:32 +0200 Subject: [PATCH 1/4] Fix varargs in methods (Issue: #1625) --- .../dotty/tools/dotc/parsing/Parsers.scala | 26 ++++++++++++++++--- .../dotc/reporting/diagnostic/messages.scala | 12 +++++++++ compiler/test/dotc/tests.scala | 3 +++ .../allParamsAreVarArgs.scala | 3 +++ .../curriedNegExample.scala | 3 +++ .../firstParamIsVarArgs.scala | 3 +++ .../curriedPosExample.scala | 3 +++ .../onlyOneVarArgs.scala | 3 +++ 8 files changed, 53 insertions(+), 3 deletions(-) create mode 100644 tests/neg/varargsInMethodsT1625/allParamsAreVarArgs.scala create mode 100644 tests/neg/varargsInMethodsT1625/curriedNegExample.scala create mode 100644 tests/neg/varargsInMethodsT1625/firstParamIsVarArgs.scala create mode 100644 tests/pos/varargsInMethodsT1625/curriedPosExample.scala create mode 100644 tests/pos/varargsInMethodsT1625/onlyOneVarArgs.scala diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 65c7a290df70..9bbb7cc154cf 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -4,7 +4,7 @@ package parsing import scala.collection.mutable.ListBuffer import scala.collection.immutable.BitSet -import util.{ SourceFile, SourcePosition } +import util.{SourceFile, SourcePosition} import Tokens._ import Scanners._ import MarkupParsers._ @@ -12,7 +12,7 @@ import core._ import Flags._ import Contexts._ import Names._ -import ast.Positioned +import ast.{Positioned, Trees, untpd} import ast.Trees._ import Decorators._ import StdNames._ @@ -20,7 +20,8 @@ import util.Positions._ import Constants._ import ScriptParsers._ import Comments._ -import scala.annotation.{tailrec, switch} + +import scala.annotation.{switch, tailrec} import util.DotClass import rewrite.Rewrites.patch @@ -1921,6 +1922,21 @@ object Parsers { } } + + + private def checkVarArgsRules(vparamss: List[List[untpd.ValDef]]): List[untpd.ValDef] = { + def isVarArgs(tpt: Trees.Tree[Untyped]): Boolean = tpt match { + case PostfixOp(_, op) if op.name == nme.raw.STAR => true + case _ => false + } + + vparamss.flatMap { params => + if (params.nonEmpty) { + params.init.filter(valDef => isVarArgs(valDef.tpt)) + } else List() + } + } + /** DefDef ::= DefSig (`:' Type [`=' Expr] | "=" Expr) * | this ParamClause ParamClauses `=' ConstrExpr * DefDcl ::= DefSig `:' Type @@ -1950,6 +1966,10 @@ object Parsers { val name = ident() val tparams = typeParamClauseOpt(ParamOwner.Def) val vparamss = paramClauses(name) + val listOfErrors = checkVarArgsRules(vparamss) + listOfErrors.foreach { vparam => + syntaxError(VarArgsParamMustComeLast(), vparam.tpt.pos) + } var tpt = fromWithinReturnType(typedOpt()) if (in.isScala2Mode) newLineOptWhenFollowedBy(LBRACE) val rhs = diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 6fa05664688e..ff3ecc39ffe5 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1209,4 +1209,16 @@ object messages { |${parents.mkString(" - ", "\n - ", "")} |""".stripMargin } + + case class VarArgsParamMustComeLast()(implicit ctx: Context) + extends Message(IncorrectRepeatedParameterSyntaxID) { + override def msg: String = "*-parameter must come last" + + override def kind: String = "Syntax" + + override def explanation: String = + hl"""|The varargs field must be the last field in the method signature. + |Attempting to define a field in a method signature after a varargs field is an error. + |""".stripMargin + } } diff --git a/compiler/test/dotc/tests.scala b/compiler/test/dotc/tests.scala index 01db2d9cc893..57df5479eafe 100644 --- a/compiler/test/dotc/tests.scala +++ b/compiler/test/dotc/tests.scala @@ -153,6 +153,7 @@ class tests extends CompilerTest { @Test def pos_anonClassSubtyping = compileFile(posDir, "anonClassSubtyping", twice) @Test def pos_extmethods = compileFile(posDir, "extmethods", twice) @Test def pos_companions = compileFile(posDir, "companions", twice) + @Test def posVarargsInMethodsT1625 = compileFiles(posDir + "varargsInMethodsT1625/") @Test def pos_all = compileFiles(posDir) // twice omitted to make tests run faster @@ -177,6 +178,8 @@ class tests extends CompilerTest { @Test def neg_all = compileFiles(negDir, verbose = true, compileSubDirs = false) @Test def neg_typedIdents() = compileDir(negDir, "typedIdents") + @Test def negVarargsInMethodsT1625 = compileFiles(negDir + "varargsInMethodsT1625/") + val negCustomArgs = negDir + "customArgs/" @Test def neg_typers() = compileFile(negCustomArgs, "typers")(allowDoubleBindings) diff --git a/tests/neg/varargsInMethodsT1625/allParamsAreVarArgs.scala b/tests/neg/varargsInMethodsT1625/allParamsAreVarArgs.scala new file mode 100644 index 000000000000..3e28213a681c --- /dev/null +++ b/tests/neg/varargsInMethodsT1625/allParamsAreVarArgs.scala @@ -0,0 +1,3 @@ +trait T3 { + def foo(x: String*, y: String*, c: String*): Int // error // error: *-parameter must come last +} \ No newline at end of file diff --git a/tests/neg/varargsInMethodsT1625/curriedNegExample.scala b/tests/neg/varargsInMethodsT1625/curriedNegExample.scala new file mode 100644 index 000000000000..2793b6730c5b --- /dev/null +++ b/tests/neg/varargsInMethodsT1625/curriedNegExample.scala @@ -0,0 +1,3 @@ +trait T2 { + def foo(x: String*, y: String*)(z: Boolean*)(u: Int*, l: Int*): Int // error // error: *-parameter must come last +} \ No newline at end of file diff --git a/tests/neg/varargsInMethodsT1625/firstParamIsVarArgs.scala b/tests/neg/varargsInMethodsT1625/firstParamIsVarArgs.scala new file mode 100644 index 000000000000..4a330a5300ef --- /dev/null +++ b/tests/neg/varargsInMethodsT1625/firstParamIsVarArgs.scala @@ -0,0 +1,3 @@ +trait T1 { + def foo(x: String*, y: String): Int // error: *-parameter must come last +} \ No newline at end of file diff --git a/tests/pos/varargsInMethodsT1625/curriedPosExample.scala b/tests/pos/varargsInMethodsT1625/curriedPosExample.scala new file mode 100644 index 000000000000..c614f2dee969 --- /dev/null +++ b/tests/pos/varargsInMethodsT1625/curriedPosExample.scala @@ -0,0 +1,3 @@ +trait T2 { + def foo(x: String, y: String*)(z: Boolean*)(u: Int, l: Int*): Int +} \ No newline at end of file diff --git a/tests/pos/varargsInMethodsT1625/onlyOneVarArgs.scala b/tests/pos/varargsInMethodsT1625/onlyOneVarArgs.scala new file mode 100644 index 000000000000..834960b4a5dc --- /dev/null +++ b/tests/pos/varargsInMethodsT1625/onlyOneVarArgs.scala @@ -0,0 +1,3 @@ +trait T1 { + def foo(x: String*): Int +} \ No newline at end of file From 2924df774960aec80b0a7da59b1a6dcecc7ddcad Mon Sep 17 00:00:00 2001 From: Igor Mielientiev Date: Wed, 22 Mar 2017 14:59:09 +0200 Subject: [PATCH 2/4] Fix minor comments --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 4 ++-- compiler/test/dotc/tests.scala | 4 ++-- .../varargsInMethodsT1625/caseClassConstructorVarArgs.scala | 3 +++ tests/neg/varargsInMethodsT1625/classConstructorVarArgs.scala | 3 +++ 4 files changed, 10 insertions(+), 4 deletions(-) create mode 100644 tests/neg/varargsInMethodsT1625/caseClassConstructorVarArgs.scala create mode 100644 tests/neg/varargsInMethodsT1625/classConstructorVarArgs.scala diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 9bbb7cc154cf..3da2b0bb60b0 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -4,7 +4,7 @@ package parsing import scala.collection.mutable.ListBuffer import scala.collection.immutable.BitSet -import util.{SourceFile, SourcePosition} +import util.{ SourceFile, SourcePosition } import Tokens._ import Scanners._ import MarkupParsers._ @@ -21,7 +21,7 @@ import Constants._ import ScriptParsers._ import Comments._ -import scala.annotation.{switch, tailrec} +import scala.annotation.{tailrec, switch} import util.DotClass import rewrite.Rewrites.patch diff --git a/compiler/test/dotc/tests.scala b/compiler/test/dotc/tests.scala index 57df5479eafe..3ebf7f2c4645 100644 --- a/compiler/test/dotc/tests.scala +++ b/compiler/test/dotc/tests.scala @@ -153,7 +153,7 @@ class tests extends CompilerTest { @Test def pos_anonClassSubtyping = compileFile(posDir, "anonClassSubtyping", twice) @Test def pos_extmethods = compileFile(posDir, "extmethods", twice) @Test def pos_companions = compileFile(posDir, "companions", twice) - @Test def posVarargsInMethodsT1625 = compileFiles(posDir + "varargsInMethodsT1625/") + @Test def posVarargsT1625 = compileFiles(posDir + "varargsInMethodsT1625/") @Test def pos_all = compileFiles(posDir) // twice omitted to make tests run faster @@ -178,7 +178,7 @@ class tests extends CompilerTest { @Test def neg_all = compileFiles(negDir, verbose = true, compileSubDirs = false) @Test def neg_typedIdents() = compileDir(negDir, "typedIdents") - @Test def negVarargsInMethodsT1625 = compileFiles(negDir + "varargsInMethodsT1625/") + @Test def negVarargsT1625 = compileFiles(negDir + "varargsInMethodsT1625/") val negCustomArgs = negDir + "customArgs/" diff --git a/tests/neg/varargsInMethodsT1625/caseClassConstructorVarArgs.scala b/tests/neg/varargsInMethodsT1625/caseClassConstructorVarArgs.scala new file mode 100644 index 000000000000..00706a003cf9 --- /dev/null +++ b/tests/neg/varargsInMethodsT1625/caseClassConstructorVarArgs.scala @@ -0,0 +1,3 @@ +object T5 { + case class Abc(x: String*, c: String*) //error +} \ No newline at end of file diff --git a/tests/neg/varargsInMethodsT1625/classConstructorVarArgs.scala b/tests/neg/varargsInMethodsT1625/classConstructorVarArgs.scala new file mode 100644 index 000000000000..5d64fa8c7f73 --- /dev/null +++ b/tests/neg/varargsInMethodsT1625/classConstructorVarArgs.scala @@ -0,0 +1,3 @@ +class Abc(val x: String*, val c: String*) { + def test = ??? +} \ No newline at end of file From 300dcd2d89a4e41e74fa6c68fb9f530a1ea53a60 Mon Sep 17 00:00:00 2001 From: Igor Mielientiev Date: Wed, 22 Mar 2017 15:00:47 +0200 Subject: [PATCH 3/4] Change varargs parameter message --- .../src/dotty/tools/dotc/reporting/diagnostic/messages.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index ff3ecc39ffe5..57365658e023 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1212,7 +1212,7 @@ object messages { case class VarArgsParamMustComeLast()(implicit ctx: Context) extends Message(IncorrectRepeatedParameterSyntaxID) { - override def msg: String = "*-parameter must come last" + override def msg: String = "varargs parameter must come last" override def kind: String = "Syntax" From 3f2c7b84926e8695c7785ee8f71bd20b24f29596 Mon Sep 17 00:00:00 2001 From: Igor Mielientiev Date: Wed, 22 Mar 2017 15:59:36 +0200 Subject: [PATCH 4/4] Fix failed test. Fix case for constructor. Move `checkVarArgsRules` to `paramClauses` --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 8 ++++---- tests/neg/varargsInMethodsT1625/allParamsAreVarArgs.scala | 2 +- .../caseClassConstructorVarArgs.scala | 2 +- .../varargsInMethodsT1625/classConstructorVarArgs.scala | 2 +- tests/neg/varargsInMethodsT1625/curriedNegExample.scala | 2 +- tests/neg/varargsInMethodsT1625/firstParamIsVarArgs.scala | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 3da2b0bb60b0..b644c94cc0e4 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -1801,6 +1801,10 @@ object Parsers { case _ => syntaxError(AuxConstructorNeedsNonImplicitParameter(), start) } } + val listOfErrors = checkVarArgsRules(result) + listOfErrors.foreach { vparam => + syntaxError(VarArgsParamMustComeLast(), vparam.tpt.pos) + } result } @@ -1966,10 +1970,6 @@ object Parsers { val name = ident() val tparams = typeParamClauseOpt(ParamOwner.Def) val vparamss = paramClauses(name) - val listOfErrors = checkVarArgsRules(vparamss) - listOfErrors.foreach { vparam => - syntaxError(VarArgsParamMustComeLast(), vparam.tpt.pos) - } var tpt = fromWithinReturnType(typedOpt()) if (in.isScala2Mode) newLineOptWhenFollowedBy(LBRACE) val rhs = diff --git a/tests/neg/varargsInMethodsT1625/allParamsAreVarArgs.scala b/tests/neg/varargsInMethodsT1625/allParamsAreVarArgs.scala index 3e28213a681c..aabb1ecea2e9 100644 --- a/tests/neg/varargsInMethodsT1625/allParamsAreVarArgs.scala +++ b/tests/neg/varargsInMethodsT1625/allParamsAreVarArgs.scala @@ -1,3 +1,3 @@ trait T3 { - def foo(x: String*, y: String*, c: String*): Int // error // error: *-parameter must come last + def foo(x: String*, y: String*, c: String*): Int // error // error: varargs parameter must come last } \ No newline at end of file diff --git a/tests/neg/varargsInMethodsT1625/caseClassConstructorVarArgs.scala b/tests/neg/varargsInMethodsT1625/caseClassConstructorVarArgs.scala index 00706a003cf9..8f8a4fcf66e8 100644 --- a/tests/neg/varargsInMethodsT1625/caseClassConstructorVarArgs.scala +++ b/tests/neg/varargsInMethodsT1625/caseClassConstructorVarArgs.scala @@ -1,3 +1,3 @@ object T5 { - case class Abc(x: String*, c: String*) //error + case class Abc(x: String*, c: String*) // error //error: varargs parameter must come last AND found: String* required: String } \ No newline at end of file diff --git a/tests/neg/varargsInMethodsT1625/classConstructorVarArgs.scala b/tests/neg/varargsInMethodsT1625/classConstructorVarArgs.scala index 5d64fa8c7f73..74595cb7dd8e 100644 --- a/tests/neg/varargsInMethodsT1625/classConstructorVarArgs.scala +++ b/tests/neg/varargsInMethodsT1625/classConstructorVarArgs.scala @@ -1,3 +1,3 @@ -class Abc(val x: String*, val c: String*) { +class Abc(val x: String*, val c: String*) { // error: varargs parameter must come last def test = ??? } \ No newline at end of file diff --git a/tests/neg/varargsInMethodsT1625/curriedNegExample.scala b/tests/neg/varargsInMethodsT1625/curriedNegExample.scala index 2793b6730c5b..616ea053946f 100644 --- a/tests/neg/varargsInMethodsT1625/curriedNegExample.scala +++ b/tests/neg/varargsInMethodsT1625/curriedNegExample.scala @@ -1,3 +1,3 @@ trait T2 { - def foo(x: String*, y: String*)(z: Boolean*)(u: Int*, l: Int*): Int // error // error: *-parameter must come last + def foo(x: String*, y: String*)(z: Boolean*)(u: Int*, l: Int*): Int // error // error: varargs parameter must come last } \ No newline at end of file diff --git a/tests/neg/varargsInMethodsT1625/firstParamIsVarArgs.scala b/tests/neg/varargsInMethodsT1625/firstParamIsVarArgs.scala index 4a330a5300ef..eceb7e696a65 100644 --- a/tests/neg/varargsInMethodsT1625/firstParamIsVarArgs.scala +++ b/tests/neg/varargsInMethodsT1625/firstParamIsVarArgs.scala @@ -1,3 +1,3 @@ trait T1 { - def foo(x: String*, y: String): Int // error: *-parameter must come last + def foo(x: String*, y: String): Int // error: varargs parameter must come last } \ No newline at end of file