From 46c9495c7bc32ccee4f3b9fce694eba831b2beb0 Mon Sep 17 00:00:00 2001 From: Vitor Vieira Date: Thu, 7 Dec 2017 20:57:15 +0100 Subject: [PATCH 1/2] Fix #3540: Add compilation failure when implicit class has two non-implicit arguments in primary constructor. --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 9 +++++++-- .../reporting/diagnostic/ErrorMessageID.java | 1 + .../dotc/reporting/diagnostic/messages.scala | 17 ++++++++++++++++- .../dotc/reporting/ErrorMessagesTests.scala | 15 +++++++++++++++ tests/neg/i2464.scala | 4 ++++ tests/neg/i3540.scala | 9 +++++++++ tests/pos/i3540.scala | 5 +++++ 7 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 tests/neg/i2464.scala create mode 100644 tests/neg/i3540.scala create mode 100644 tests/pos/i3540.scala diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 621a98a50a17..e9d02525546e 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -285,10 +285,11 @@ object desugar { val isCaseClass = mods.is(Case) && !mods.is(Module) val isCaseObject = mods.is(Case) && mods.is(Module) + val isImplicit = mods.is(Implicit) val isEnum = mods.hasMod[Mod.Enum] && !mods.is(Module) val isEnumCase = isLegalEnumCase(cdef) val isValueClass = parents.nonEmpty && isAnyVal(parents.head) - // This is not watertight, but `extends AnyVal` will be replaced by `inline` later. + // This is not watertight, but `extends AnyVal` will be replaced by `inline` later. val originalTparams = constr1.tparams @@ -505,7 +506,7 @@ object desugar { // synthetic implicit C[Ts](p11: T11, ..., p1N: T1N) ... (pM1: TM1, ..., pMN: TMN): C[Ts] = // new C[Ts](p11, ..., p1N) ... (pM1, ..., pMN) = val implicitWrappers = - if (!mods.is(Implicit)) + if (!isImplicit) Nil else if (ctx.owner is Package) { ctx.error(TopLevelImplicitClass(cdef), cdef.pos) @@ -515,6 +516,10 @@ object desugar { ctx.error(ImplicitCaseClass(cdef), cdef.pos) Nil } + else if (arity != 1) { + ctx.error(ImplicitClassPrimaryConstructorArity(), cdef.pos) + Nil + } else // implicit wrapper is typechecked in same scope as constructor, so // we can reuse the constructor parameters; no derived params are needed. diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java index 0f84c8e3bf8c..1b7486840bbb 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java @@ -20,6 +20,7 @@ public enum ErrorMessageID { EarlyDefinitionsNotSupportedID, TopLevelImplicitClassID, ImplicitCaseClassID, + ImplicitClassPrimaryConstructorArityID, ObjectMayNotHaveSelfTypeID, TupleTooLongID, RepeatedModifierID, diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index b894b8dc312d..8b855d6c5a1b 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -20,7 +20,6 @@ import printing.Formatting import ErrorMessageID._ import Denotations.SingleDenotation import dotty.tools.dotc.ast.Trees -import dotty.tools.dotc.ast.untpd.Modifiers import dotty.tools.dotc.config.ScalaVersion import dotty.tools.dotc.core.Flags.{FlagSet, Mutable} import dotty.tools.dotc.core.SymDenotations.SymDenotation @@ -453,6 +452,22 @@ object messages { |""" + implicitClassRestrictionsText } + case class ImplicitClassPrimaryConstructorArity()(implicit ctx: Context) + extends Message(ImplicitClassPrimaryConstructorArityID){ + val kind = "Syntax" + val msg = hl"Implicit classes must accept exactly one primary constructor parameter" + val explanation = { + val example = "implicit class RichDate(date: java.util.Date)" + hl"""Implicit classes may only take one non-implicit argument in their constructor. For example: + | + | $example + | + |While it’s possible to create an implicit class with more than one non-implicit argument, + |such classes aren’t used during implicit lookup. + |""" + implicitClassRestrictionsText + } + } + case class ObjectMayNotHaveSelfType(mdef: untpd.ModuleDef)(implicit ctx: Context) extends Message(ObjectMayNotHaveSelfTypeID) { val kind = "Syntax" diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala index e0611366e31e..69f985e03f61 100644 --- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala +++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala @@ -801,6 +801,21 @@ class ErrorMessagesTests extends ErrorMessagesTest { assertEquals(err, ExpectedClassOrObjectDef()) } + @Test def implicitClassPrimaryConstructorArity = + checkMessagesAfter("frontend") { + """ + |object Test { + | implicit class Foo(i: Int, s: String) + |} + """.stripMargin + } + .expect { (itcx, messages) => + implicit val ctx: Context = itcx + assertMessageCount(1, messages) + val err :: Nil = messages + assertEquals(err, ImplicitClassPrimaryConstructorArity()) + } + @Test def anonymousFunctionMissingParamType = checkMessagesAfter("refchecks") { """ diff --git a/tests/neg/i2464.scala b/tests/neg/i2464.scala new file mode 100644 index 000000000000..8ee7a16d7771 --- /dev/null +++ b/tests/neg/i2464.scala @@ -0,0 +1,4 @@ +object Foo { + object Bar + implicit class Bar // error: Implicit classes must accept exactly one primary constructor parameter +} diff --git a/tests/neg/i3540.scala b/tests/neg/i3540.scala new file mode 100644 index 000000000000..c1755aac45af --- /dev/null +++ b/tests/neg/i3540.scala @@ -0,0 +1,9 @@ +object Test { + implicit class Foo(i: Int, s: String) // error: Implicit classes must accept exactly one primary constructor parameter + implicit class Foo0 // error: Implicit classes must accept exactly one primary constructor parameter + implicit class Foo1() // error: Implicit classes must accept exactly one primary constructor parameter + implicit class Foo2()(x: Int) // error: Implicit classes must accept exactly one primary constructor parameter + implicit case class Bar0 // error: A case class must have at least one parameter list + implicit case class Bar1() // error: A case class may not be defined as implicit + implicit case class Bar2()(x: Int) // error: A case class may not be defined as implicit +} diff --git a/tests/pos/i3540.scala b/tests/pos/i3540.scala new file mode 100644 index 000000000000..c03ba66dbdf6 --- /dev/null +++ b/tests/pos/i3540.scala @@ -0,0 +1,5 @@ +object Test { + implicit class Foo(x: Int)(implicit y: Int) + implicit class Foo0(x: Int)(y: Int)(implicit z: Int) // OK but not used during implicit lookup + implicit class Foo1(x: Int)(y: Int) // OK but not used during implicit lookup +} From d96c86e06b37a6748ab7ece7e719546f29fe4781 Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Mon, 11 Dec 2017 22:52:55 +0100 Subject: [PATCH 2/2] Polishing --- .../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 8b855d6c5a1b..f279dafdb407 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -455,7 +455,7 @@ object messages { case class ImplicitClassPrimaryConstructorArity()(implicit ctx: Context) extends Message(ImplicitClassPrimaryConstructorArityID){ val kind = "Syntax" - val msg = hl"Implicit classes must accept exactly one primary constructor parameter" + val msg = "Implicit classes must accept exactly one primary constructor parameter" val explanation = { val example = "implicit class RichDate(date: java.util.Date)" hl"""Implicit classes may only take one non-implicit argument in their constructor. For example: