Skip to content

Fix varargs in methods #2135

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.

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 4 commits into from
Mar 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 21 additions & 1 deletion compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@ import core._
import Flags._
import Contexts._
import Names._
import ast.Positioned
import ast.{Positioned, Trees, untpd}
import ast.Trees._
import Decorators._
import StdNames._
import util.Positions._
import Constants._
import ScriptParsers._
import Comments._

import scala.annotation.{tailrec, switch}
import util.DotClass
import rewrite.Rewrites.patch
Expand Down Expand Up @@ -1800,6 +1801,10 @@ object Parsers {
case _ => syntaxError(AuxConstructorNeedsNonImplicitParameter(), start)
}
}
val listOfErrors = checkVarArgsRules(result)
listOfErrors.foreach { vparam =>
syntaxError(VarArgsParamMustComeLast(), vparam.tpt.pos)
}
result
}

Expand Down Expand Up @@ -1921,6 +1926,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
Expand Down
12 changes: 12 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1209,4 +1209,16 @@ object messages {
|${parents.mkString(" - ", "\n - ", "")}
|""".stripMargin
}

case class VarArgsParamMustComeLast()(implicit ctx: Context)
extends Message(IncorrectRepeatedParameterSyntaxID) {
override def msg: String = "varargs 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
}
}
3 changes: 3 additions & 0 deletions compiler/test/dotc/tests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 posVarargsT1625 = compileFiles(posDir + "varargsInMethodsT1625/")

@Test def pos_all = compileFiles(posDir) // twice omitted to make tests run faster

Expand All @@ -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 negVarargsT1625 = compileFiles(negDir + "varargsInMethodsT1625/")
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an unnecessary complication, just put your test sources in tests/neg/ and remove this @Test def .... Also use the prefix i1625 for the test sources.

Copy link
Contributor

Choose a reason for hiding this comment

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

this is going away once we merge #2125 anyway, I'll deal with it.


val negCustomArgs = negDir + "customArgs/"

@Test def neg_typers() = compileFile(negCustomArgs, "typers")(allowDoubleBindings)
Expand Down
3 changes: 3 additions & 0 deletions tests/neg/varargsInMethodsT1625/allParamsAreVarArgs.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
trait T3 {
def foo(x: String*, y: String*, c: String*): Int // error // error: varargs parameter must come last
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
object T5 {
case class Abc(x: String*, c: String*) // error //error: varargs parameter must come last AND found: String* required: String
}
3 changes: 3 additions & 0 deletions tests/neg/varargsInMethodsT1625/classConstructorVarArgs.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
class Abc(val x: String*, val c: String*) { // error: varargs parameter must come last
def test = ???
}
3 changes: 3 additions & 0 deletions tests/neg/varargsInMethodsT1625/curriedNegExample.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
trait T2 {
def foo(x: String*, y: String*)(z: Boolean*)(u: Int*, l: Int*): Int // error // error: varargs parameter must come last
}
3 changes: 3 additions & 0 deletions tests/neg/varargsInMethodsT1625/firstParamIsVarArgs.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
trait T1 {
def foo(x: String*, y: String): Int // error: varargs parameter must come last
}
3 changes: 3 additions & 0 deletions tests/pos/varargsInMethodsT1625/curriedPosExample.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
trait T2 {
def foo(x: String, y: String*)(z: Boolean*)(u: Int, l: Int*): Int
}
3 changes: 3 additions & 0 deletions tests/pos/varargsInMethodsT1625/onlyOneVarArgs.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
trait T1 {
def foo(x: String*): Int
}