diff options
author | Adriaan Moors <adriaan@lightbend.com> | 2017-02-07 17:28:32 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-02-07 17:28:32 -0800 |
commit | 40d01f3a4408747217b249075a09111820b5594b (patch) | |
tree | 2bd4a220f7e8508556b5d61f24b7c434dc46ff83 | |
parent | 40236b0378c597858d666beb4d38977b520df9e7 (diff) | |
parent | b775f4f58098d7f79155b3fe00c0bdb0eabf3d84 (diff) | |
download | scala-40d01f3a4408747217b249075a09111820b5594b.tar.gz scala-40d01f3a4408747217b249075a09111820b5594b.tar.bz2 scala-40d01f3a4408747217b249075a09111820b5594b.zip |
Merge pull request #5585 from som-snytt/issue/10097
SI-10097 Error if no non-implicit case class param
-rw-r--r-- | spec/05-classes-and-objects.md | 13 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/ast/parser/Parsers.scala | 60 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/settings/Warnings.scala | 2 | ||||
-rw-r--r-- | src/reflect/scala/reflect/internal/Printers.scala | 2 | ||||
-rw-r--r-- | src/reflect/scala/reflect/internal/ReificationSupport.scala | 15 | ||||
-rw-r--r-- | test/files/neg/quasiquotes-syntax-error-position.check | 3 | ||||
-rw-r--r-- | test/files/neg/t10097.check | 10 | ||||
-rw-r--r-- | test/files/neg/t10097.flags | 1 | ||||
-rw-r--r-- | test/files/neg/t10097.scala | 4 | ||||
-rw-r--r-- | test/files/neg/t10097b.check | 6 | ||||
-rw-r--r-- | test/files/neg/t10097b.flags | 1 | ||||
-rw-r--r-- | test/files/neg/t10097b.scala | 3 | ||||
-rw-r--r-- | test/files/neg/t8704.check | 11 | ||||
-rw-r--r-- | test/files/neg/t8704.flags | 1 | ||||
-rw-r--r-- | test/files/neg/t8704.scala | 7 | ||||
-rw-r--r-- | test/files/run/patmat-exprs.scala | 4 | ||||
-rw-r--r-- | test/files/run/t10097.check | 3 | ||||
-rw-r--r-- | test/files/run/t10097.flags | 1 | ||||
-rw-r--r-- | test/files/run/t10097.scala | 6 | ||||
-rw-r--r-- | test/files/run/t5907.scala | 2 | ||||
-rw-r--r-- | test/junit/scala/reflect/internal/PrintersTest.scala | 2 |
21 files changed, 122 insertions, 35 deletions
diff --git a/spec/05-classes-and-objects.md b/spec/05-classes-and-objects.md index 75620f57d4..6738c7a5b7 100644 --- a/spec/05-classes-and-objects.md +++ b/spec/05-classes-and-objects.md @@ -709,7 +709,7 @@ Here, value parameter may not form part of the types of any of the parent classes or members of the class template $t$. It is illegal to define two formal value parameters with the same name. - If no formal parameter sections are given, an empty parameter section `()` is assumed. + If a class has no formal parameter section that is not implicit, an empty parameter section `()` is assumed. If a formal parameter declaration $x: T$ is preceded by a `val` or `var` keyword, an accessor (getter) [definition](04-basic-declarations-and-definitions.html#variable-declarations-and-definitions) @@ -842,12 +842,13 @@ TmplDef ::= ‘case’ ‘class’ ClassDef If a class definition is prefixed with `case`, the class is said to be a _case class_. -The formal parameters in the first parameter section of a case class -are called _elements_; they are treated -specially. First, the value of such a parameter can be extracted as a +A case class is required to have a parameter section that is not implicit. +The formal parameters in the first parameter section +are called _elements_ and are treated specially. +First, the value of such a parameter can be extracted as a field of a constructor pattern. Second, a `val` prefix is -implicitly added to such a parameter, unless the parameter carries -already a `val` or `var` modifier. Hence, an accessor +implicitly added to such a parameter, unless the parameter already carries +a `val` or `var` modifier. Hence, an accessor definition for the parameter is [generated](#class-definitions). A case class definition of `$c$[$\mathit{tps}\,$]($\mathit{ps}_1\,$)$\ldots$($\mathit{ps}_n$)` with type diff --git a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala index cf66e0a7dc..c35c0a1019 100644 --- a/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala +++ b/src/compiler/scala/tools/nsc/ast/parser/Parsers.scala @@ -2236,31 +2236,57 @@ self => * }}} */ def paramClauses(owner: Name, contextBounds: List[Tree], ofCaseClass: Boolean): List[List[ValDef]] = { - var implicitmod = 0 - var caseParam = ofCaseClass - def paramClause(): List[ValDef] = { - if (in.token == RPAREN) - return Nil - - if (in.token == IMPLICIT) { - in.nextToken() - implicitmod = Flags.IMPLICIT - } - commaSeparated(param(owner, implicitmod, caseParam )) - } - val vds = new ListBuffer[List[ValDef]] + var implicitSection = -1 + var implicitOffset = -1 + var warnAt = -1 + var caseParam = ofCaseClass + val vds = new ListBuffer[List[ValDef]] val start = in.offset + def paramClause(): List[ValDef] = if (in.token == RPAREN) Nil else { + val implicitmod = + if (in.token == IMPLICIT) { + if (implicitOffset == -1) { implicitOffset = in.offset ; implicitSection = vds.length } + else if (warnAt == -1) warnAt = in.offset + in.nextToken() + Flags.IMPLICIT + } else 0 + commaSeparated(param(owner, implicitmod, caseParam)) + } newLineOptWhenFollowedBy(LPAREN) - if (ofCaseClass && in.token != LPAREN) - syntaxError(in.lastOffset, "case classes without a parameter list are not allowed;\n"+ - "use either case objects or case classes with an explicit `()' as a parameter list.") - while (implicitmod == 0 && in.token == LPAREN) { + while (in.token == LPAREN) { in.nextToken() vds += paramClause() accept(RPAREN) caseParam = false newLineOptWhenFollowedBy(LPAREN) } + if (ofCaseClass) { + if (vds.isEmpty) + syntaxError(start, s"case classes must have a parameter list; try 'case class ${owner.encoded + }()' or 'case object ${owner.encoded}'") + else if (vds.head.nonEmpty && vds.head.head.mods.isImplicit) { + if (settings.isScala213) + syntaxError(start, s"case classes must have a non-implicit parameter list; try 'case class ${ + owner.encoded}()${ vds.map(vs => "(...)").mkString }'") + else { + deprecationWarning(start, s"case classes should have a non-implicit parameter list; adapting to 'case class ${ + owner.encoded}()${ vds.map(vs => "(...)").mkString }'", "2.12.2") + vds.insert(0, List.empty[ValDef]) + vds(1) = vds(1).map(vd => copyValDef(vd)(mods = vd.mods & ~Flags.CASEACCESSOR)) + if (implicitSection != -1) implicitSection += 1 + } + } + } + if (implicitSection != -1 && implicitSection != vds.length - 1) + syntaxError(implicitOffset, "an implicit parameter section must be last") + if (warnAt != -1) + syntaxError(warnAt, "multiple implicit parameter sections are not allowed") + else if (settings.warnExtraImplicit) { + // guard against anomalous class C(private implicit val x: Int)(implicit s: String) + val ttl = vds.count { case ValDef(mods, _, _, _) :: _ => mods.isImplicit ; case _ => false } + if (ttl > 1) + warning(in.offset, s"$ttl parameter sections are effectively implicit") + } val result = vds.toList if (owner == nme.CONSTRUCTOR && (result.isEmpty || (result.head take 1 exists (_.mods.isImplicit)))) { in.token match { diff --git a/src/compiler/scala/tools/nsc/settings/Warnings.scala b/src/compiler/scala/tools/nsc/settings/Warnings.scala index 839e734abc..87534656f9 100644 --- a/src/compiler/scala/tools/nsc/settings/Warnings.scala +++ b/src/compiler/scala/tools/nsc/settings/Warnings.scala @@ -25,6 +25,8 @@ trait Warnings { // currently considered too noisy for general use val warnUnusedImport = BooleanSetting("-Ywarn-unused-import", "Warn when imports are unused.") + val warnExtraImplicit = BooleanSetting("-Ywarn-extra-implicit", "Warn when more than one implicit parameter section is defined.") + // Experimental lint warnings that are turned off, but which could be turned on programmatically. // They are not activated by -Xlint and can't be enabled on the command line because they are not // created using the standard factory methods. diff --git a/src/reflect/scala/reflect/internal/Printers.scala b/src/reflect/scala/reflect/internal/Printers.scala index 9602a2859b..bb352e9d31 100644 --- a/src/reflect/scala/reflect/internal/Printers.scala +++ b/src/reflect/scala/reflect/internal/Printers.scala @@ -775,7 +775,7 @@ trait Printers extends api.Printers { self: SymbolTable => } // constructor's params processing (don't print single empty constructor param list) vparamss match { - case Nil | List(Nil) if (!mods.isCase && !ctorMods.hasFlag(AccessFlags)) => + case Nil | List(Nil) if !mods.isCase && !ctorMods.hasFlag(AccessFlags) => case _ => vparamss foreach printConstrParams } parents diff --git a/src/reflect/scala/reflect/internal/ReificationSupport.scala b/src/reflect/scala/reflect/internal/ReificationSupport.scala index 026438e421..1b2be64654 100644 --- a/src/reflect/scala/reflect/internal/ReificationSupport.scala +++ b/src/reflect/scala/reflect/internal/ReificationSupport.scala @@ -266,7 +266,7 @@ trait ReificationSupport { self: SymbolTable => } // undo gen.mkTemplate - protected object UnMkTemplate { + protected class UnMkTemplate(isCaseClass: Boolean) { def unapply(templ: Template): Option[(List[Tree], ValDef, Modifiers, List[List[ValDef]], List[Tree], List[Tree])] = { val Template(parents, selfType, _) = templ val tbody = treeInfo.untypecheckedTemplBody(templ) @@ -296,8 +296,9 @@ trait ReificationSupport { self: SymbolTable => result(ctorMods, Nil, edefs, body) else { // undo conversion from (implicit ... ) to ()(implicit ... ) when it's the only parameter section + // except that case classes require the explicit leading empty parameter list val vparamssRestoredImplicits = ctorVparamss match { - case Nil :: (tail @ ((head :: _) :: _)) if head.mods.isImplicit => tail + case Nil :: (tail @ ((head :: _) :: _)) if head.mods.isImplicit && !isCaseClass => tail case other => other } // undo flag modifications by merging flag info from constructor args and fieldDefs @@ -314,7 +315,9 @@ trait ReificationSupport { self: SymbolTable => } } } + def asCase = new UnMkTemplate(isCaseClass = true) } + protected object UnMkTemplate extends UnMkTemplate(isCaseClass = false) protected def mkSelfType(tree: Tree) = tree match { case vd: ValDef => @@ -346,9 +349,11 @@ trait ReificationSupport { self: SymbolTable => def unapply(tree: Tree): Option[(Modifiers, TypeName, List[TypeDef], Modifiers, List[List[ValDef]], List[Tree], List[Tree], ValDef, List[Tree])] = tree match { - case ClassDef(mods, name, tparams, UnMkTemplate(parents, selfType, ctorMods, vparamss, earlyDefs, body)) - if !ctorMods.isTrait && !ctorMods.hasFlag(JAVA) => - Some((mods, name, tparams, ctorMods, vparamss, earlyDefs, parents, selfType, body)) + case ClassDef(mods, name, tparams, impl) => + val X = if (mods.isCase) UnMkTemplate.asCase else UnMkTemplate + val X(parents, selfType, ctorMods, vparamss, earlyDefs, body) = impl + if (ctorMods.isTrait || ctorMods.hasFlag(JAVA)) None + else Some((mods, name, tparams, ctorMods, vparamss, earlyDefs, parents, selfType, body)) case _ => None } diff --git a/test/files/neg/quasiquotes-syntax-error-position.check b/test/files/neg/quasiquotes-syntax-error-position.check index 9fd6ce0417..b12a7d13d6 100644 --- a/test/files/neg/quasiquotes-syntax-error-position.check +++ b/test/files/neg/quasiquotes-syntax-error-position.check @@ -16,8 +16,7 @@ quasiquotes-syntax-error-position.scala:9: error: '{' expected but end of quote quasiquotes-syntax-error-position.scala:10: error: ';' expected but '@' found. q"foo@$a" ^ -quasiquotes-syntax-error-position.scala:11: error: case classes without a parameter list are not allowed; -use either case objects or case classes with an explicit `()' as a parameter list. +quasiquotes-syntax-error-position.scala:11: error: case classes must have a parameter list; try 'case class A()' or 'case object A' q"case class A" ^ quasiquotes-syntax-error-position.scala:12: error: identifier expected but ']' found. diff --git a/test/files/neg/t10097.check b/test/files/neg/t10097.check new file mode 100644 index 0000000000..1f70546b57 --- /dev/null +++ b/test/files/neg/t10097.check @@ -0,0 +1,10 @@ +t10097.scala:2: error: case classes must have a non-implicit parameter list; try 'case class C()(...)' +case class C(implicit val c: Int) + ^ +t10097.scala:4: error: case classes must have a non-implicit parameter list; try 'case class D()(...)(...)' +case class D(implicit c: Int)(s: String) + ^ +t10097.scala:4: error: an implicit parameter section must be last +case class D(implicit c: Int)(s: String) + ^ +three errors found diff --git a/test/files/neg/t10097.flags b/test/files/neg/t10097.flags new file mode 100644 index 0000000000..714bbf5125 --- /dev/null +++ b/test/files/neg/t10097.flags @@ -0,0 +1 @@ +-Xsource:2.13 diff --git a/test/files/neg/t10097.scala b/test/files/neg/t10097.scala new file mode 100644 index 0000000000..b2f05e2972 --- /dev/null +++ b/test/files/neg/t10097.scala @@ -0,0 +1,4 @@ + +case class C(implicit val c: Int) + +case class D(implicit c: Int)(s: String) diff --git a/test/files/neg/t10097b.check b/test/files/neg/t10097b.check new file mode 100644 index 0000000000..14535fee34 --- /dev/null +++ b/test/files/neg/t10097b.check @@ -0,0 +1,6 @@ +t10097b.scala:2: warning: case classes should have a non-implicit parameter list; adapting to 'case class C()(...)' +case class C(implicit val c: Int) + ^ +error: No warnings can be incurred under -Xfatal-warnings. +one warning found +one error found diff --git a/test/files/neg/t10097b.flags b/test/files/neg/t10097b.flags new file mode 100644 index 0000000000..c6bfaf1f64 --- /dev/null +++ b/test/files/neg/t10097b.flags @@ -0,0 +1 @@ +-deprecation -Xfatal-warnings diff --git a/test/files/neg/t10097b.scala b/test/files/neg/t10097b.scala new file mode 100644 index 0000000000..f166db6792 --- /dev/null +++ b/test/files/neg/t10097b.scala @@ -0,0 +1,3 @@ + +case class C(implicit val c: Int) + diff --git a/test/files/neg/t8704.check b/test/files/neg/t8704.check new file mode 100644 index 0000000000..b567a8bb17 --- /dev/null +++ b/test/files/neg/t8704.check @@ -0,0 +1,11 @@ +t8704.scala:7: warning: 2 parameter sections are effectively implicit +class D(private implicit val i: Int)(implicit s: String) + ^ +t8704.scala:3: error: an implicit parameter section must be last +class C(i: Int)(implicit j: Int)(implicit k: Int)(n: Int) { + ^ +t8704.scala:3: error: multiple implicit parameter sections are not allowed +class C(i: Int)(implicit j: Int)(implicit k: Int)(n: Int) { + ^ +one warning found +two errors found diff --git a/test/files/neg/t8704.flags b/test/files/neg/t8704.flags new file mode 100644 index 0000000000..f175a06c74 --- /dev/null +++ b/test/files/neg/t8704.flags @@ -0,0 +1 @@ +-Ywarn-extra-implicit diff --git a/test/files/neg/t8704.scala b/test/files/neg/t8704.scala new file mode 100644 index 0000000000..db43bfcaa5 --- /dev/null +++ b/test/files/neg/t8704.scala @@ -0,0 +1,7 @@ + + +class C(i: Int)(implicit j: Int)(implicit k: Int)(n: Int) { + def f = n +} + +class D(private implicit val i: Int)(implicit s: String) diff --git a/test/files/run/patmat-exprs.scala b/test/files/run/patmat-exprs.scala index 7ca5fd3063..d18df9c714 100644 --- a/test/files/run/patmat-exprs.scala +++ b/test/files/run/patmat-exprs.scala @@ -344,13 +344,13 @@ trait Pattern { } - case class Zero[T] (implicit num: NumericOps[T]) extends Leaf[T] { + case class Zero[T]()(implicit num: NumericOps[T]) extends Leaf[T] { def derivative(variable: Var[T]) = Zero[T] def eval(f: Any => Any) = num.zero override def toString = "0" } - case class One[T] (implicit num: NumericOps[T]) extends Leaf[T] { + case class One[T]()(implicit num: NumericOps[T]) extends Leaf[T] { def derivative(variable: Var[T]) = Zero[T] def eval(f: Any => Any) = num.one override def toString = "1" diff --git a/test/files/run/t10097.check b/test/files/run/t10097.check new file mode 100644 index 0000000000..0e8b96061c --- /dev/null +++ b/test/files/run/t10097.check @@ -0,0 +1,3 @@ +t10097.scala:2: warning: case classes should have a non-implicit parameter list; adapting to 'case class C()(...)' +case class C(implicit c: Int) + ^ diff --git a/test/files/run/t10097.flags b/test/files/run/t10097.flags new file mode 100644 index 0000000000..dcc59ebe32 --- /dev/null +++ b/test/files/run/t10097.flags @@ -0,0 +1 @@ +-deprecation diff --git a/test/files/run/t10097.scala b/test/files/run/t10097.scala new file mode 100644 index 0000000000..a16be897cc --- /dev/null +++ b/test/files/run/t10097.scala @@ -0,0 +1,6 @@ + +case class C(implicit c: Int) + +object Test extends App { + assert(C()(42).productArity == 0) +} diff --git a/test/files/run/t5907.scala b/test/files/run/t5907.scala index a005e9fbd3..81fc43e3f5 100644 --- a/test/files/run/t5907.scala +++ b/test/files/run/t5907.scala @@ -86,7 +86,7 @@ object Test extends App { } } -case class C1(implicit x: Int) { +case class C1()(implicit x: Int) { override def toString = s"c1: $x" } case class C2()(y: Int) { diff --git a/test/junit/scala/reflect/internal/PrintersTest.scala b/test/junit/scala/reflect/internal/PrintersTest.scala index 722062ba21..c7cfe0dfbb 100644 --- a/test/junit/scala/reflect/internal/PrintersTest.scala +++ b/test/junit/scala/reflect/internal/PrintersTest.scala @@ -556,7 +556,7 @@ class ClassPrintTest { @Test def testCaseClassWithParams3 = assertPrintedCode(sm""" |{ - | case class X(implicit x: scala.Int, s: scala.Predef.String); + | case class X()(implicit x: scala.Int, s: scala.Predef.String); | () |}""") |