Merge pull request #2432 from dotty-staging/fix-#2426

Fix #2426: Use Scala-2 syntax for annotations of class constructors

Author: smarter

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -294,6 +294,41 @@ object Parsers {
       } finally inFunReturnType = saved
     }
 
+    /** A placeholder for dummy arguments that should be re-parsed as parameters */
+    val ParamNotArg = EmptyTree
+
+    /** A flag indicating we are parsing in the annotations of a primary
+     *  class constructor
+     */
+    private var inClassConstrAnnots = false
+
+    private def fromWithinClassConstr[T](body: => T): T = {
+      val saved = inClassConstrAnnots
+      try {
+        inClassConstrAnnots = true
+        body
+      } finally {
+        inClassConstrAnnots = saved
+        if (lookaheadTokens.nonEmpty) {
+          in.insertTokens(lookaheadTokens.toList)
+          lookaheadTokens.clear()
+        }
+      }
+    }
+
+    /** Lookahead tokens for the case of annotations in class constructors.
+     *  We store tokens in lookahead as long as they can form a valid prefix
+     *  of a class parameter clause.
+     */
+    private var lookaheadTokens = new ListBuffer[TokenData]
+
+    /** Copy current token to end of lookahead */
+    private def saveLookahead() = {
+      val lookahead = new TokenData{}
+      lookahead.copyFrom(in)
+      lookaheadTokens += lookahead
+    }
+
     def migrationWarningOrError(msg: String, offset: Int = in.offset) =
       if (in.isScala2Mode)
         ctx.migrationWarning(msg, source atPos Position(offset))
@@ -1279,20 +1314,79 @@ object Parsers {
       if (in.token == RPAREN) Nil else commaSeparated(exprInParens)
 
     /** ParArgumentExprs ::= `(' [ExprsInParens] `)'
-     *                    |  `(' [ExprsInParens `,'] PostfixExpr `:' `_' `*' ')' \
-     */
-    def parArgumentExprs(): List[Tree] =
-      inParens(if (in.token == RPAREN) Nil else commaSeparated(argumentExpr))
+     *                    |  `(' [ExprsInParens `,'] PostfixExpr `:' `_' `*' ')'
+     *
+     *  Special treatment for arguments of primary class constructor
+     *  annotations. All empty argument lists `(` `)` following the first 
+     *  get represented as `List(ParamNotArg)` instead of `Nil`, indicating that
+     *  the token sequence should be interpreted as an empty parameter clause
+     *  instead. `ParamNotArg` can also be produced when parsing the first
+     *  argument (see `classConstrAnnotExpr`).
+     *
+     *  The method affects `lookaheadTokens` as a side effect.
+     *  If the argument list parses as `List(ParamNotArg)`, `lookaheadTokens`
+     *  contains the tokens that need to be replayed to parse the parameter clause.
+     *  Otherwise, `lookaheadTokens` is empty.
+     */
+    def parArgumentExprs(first: Boolean = false): List[Tree] = {
+      if (inClassConstrAnnots) {
+        assert(lookaheadTokens.isEmpty)
+        saveLookahead()
+        accept(LPAREN)
+        val args =
+          if (in.token == RPAREN)
+            if (first) Nil // first () counts as annotation argument
+            else ParamNotArg :: Nil
+          else {
+            openParens.change(LPAREN, +1)
+            try commaSeparated(argumentExpr)
+            finally openParens.change(LPAREN, -1)
+          }
+        if (args == ParamNotArg :: Nil)
+          in.adjustSepRegions(RPAREN) // simulate `)` without requiring it
+        else {
+          lookaheadTokens.clear()
+          accept(RPAREN)
+        }
+        args
+      }
+      else
+        inParens(if (in.token == RPAREN) Nil else commaSeparated(argumentExpr))
+    }
 
     /** ArgumentExprs ::= ParArgumentExprs
      *                 |  [nl] BlockExpr
      */
     def argumentExprs(): List[Tree] =
       if (in.token == LBRACE) blockExpr() :: Nil else parArgumentExprs()
 
-    val argumentExpr = () => exprInParens() match {
-      case a @ Assign(Ident(id), rhs) => cpy.NamedArg(a)(id, rhs)
-      case e => e
+    val argumentExpr = () => {
+      val arg =
+        if (inClassConstrAnnots && lookaheadTokens.nonEmpty) classConstrAnnotExpr()
+        else exprInParens()
+      arg match {
+        case arg @ Assign(Ident(id), rhs) => cpy.NamedArg(arg)(id, rhs)
+        case arg => arg
+      }
+    }
+
+    /** Handle first argument of an argument list to an annotation of
+     *  a primary class constructor. If the current token either cannot
+     *  start an expression or is an identifier and is followed by `:`,
+     *  stop parsing the rest of the expression and return `EmptyTree`,
+     *  indicating that we should re-parse the expression as a parameter clause.
+     *  Otherwise parse as normal.
+     */
+    def classConstrAnnotExpr() = {
+      if (in.token == IDENTIFIER) {
+        saveLookahead()
+        postfixExpr() match {
+          case Ident(_) if in.token == COLON => ParamNotArg
+          case t => expr1Rest(t, Location.InParens)
+        }
+      }
+      else if (isExprIntro) exprInParens()
+      else ParamNotArg
     }
 
     /** ArgumentExprss ::= {ArgumentExprs}
@@ -1304,9 +1398,17 @@ object Parsers {
     }
 
     /** ParArgumentExprss ::= {ParArgumentExprs}
+     *
+     *  Special treatment for arguments of primary class constructor
+     *  annotations. If an argument list returns `List(ParamNotArg)`
+     *  ignore it, and return prefix parsed before that list instead.
      */
     def parArgumentExprss(fn: Tree): Tree =
-      if (in.token == LPAREN) parArgumentExprss(Apply(fn, parArgumentExprs()))
+      if (in.token == LPAREN) {
+        val args = parArgumentExprs(first = !fn.isInstanceOf[Trees.Apply[_]])
+        if (inClassConstrAnnots && args == ParamNotArg :: Nil) fn
+        else parArgumentExprss(Apply(fn, args))
+      }
       else fn
 
     /** BlockExpr ::= `{' (CaseClauses | Block) `}'
@@ -2093,21 +2195,15 @@ object Parsers {
      */
     def classConstr(owner: Name, isCaseClass: Boolean = false): DefDef = atPos(in.lastOffset) {
       val tparams = typeParamClauseOpt(ParamOwner.Class)
-      val cmods = constrModsOpt(owner)
+      val cmods = fromWithinClassConstr(constrModsOpt(owner))
       val vparamss = paramClauses(owner, isCaseClass)
       makeConstructor(tparams, vparamss).withMods(cmods)
     }
 
-    /** ConstrMods        ::=  AccessModifier
-     *                      |  Annotation {Annotation} (AccessModifier | `this')
+    /** ConstrMods        ::=  {Annotation} [AccessModifier]
      */
-    def constrModsOpt(owner: Name): Modifiers = {
-      val mods = modifiers(accessModifierTokens, annotsAsMods())
-      if (mods.hasAnnotations && !mods.hasFlags)
-        if (in.token == THIS) in.nextToken()
-        else syntaxError(AnnotatedPrimaryConstructorRequiresModifierOrThis(owner), mods.annotations.last.pos)
-      mods
-    }
+    def constrModsOpt(owner: Name): Modifiers =
+      modifiers(accessModifierTokens, annotsAsMods())
 
     /** ObjectDef       ::= id TemplateOpt
      */

compiler/src/dotty/tools/dotc/parsing/Scanners.scala

@@ -65,7 +65,6 @@ object Scanners {
       */
     var errOffset: Offset = NoOffset
 
-
     /** Generate an error at the given offset */
     def error(msg: String, off: Offset = offset) = {
       ctx.error(msg, source atPos Position(off))
@@ -217,11 +216,42 @@ object Scanners {
 
     private class TokenData0 extends TokenData
 
-    /** we need one token lookahead and one token history
+    /** The scanner itself needs one token lookahead and one token history
      */
     val next : TokenData = new TokenData0
     private val prev : TokenData = new TokenData0
 
+    /** The parser can also add more lookahead tokens via `insertTokens`.
+     *  Tokens beyond `next` are stored in `following`.
+     */
+    private var following: List[TokenData] = Nil
+
+    /** Push a copy of token data `td` to `following` */
+    private def pushCopy(td: TokenData) = {
+      val copy = new TokenData0
+      copy.copyFrom(td)
+      following = copy :: following
+    }
+
+    /** If following is empty, invalidate token data `td` by setting
+     *  `td.token` to `EMPTY`. Otherwise pop head of `following` into `td`.
+     */
+    private def popCopy(td: TokenData) =
+      if (following.isEmpty) td.token = EMPTY
+      else {
+        td.copyFrom(following.head)
+        following = following.tail
+      }
+
+    /** Insert tokens `tds` in front of current token */
+    def insertTokens(tds: List[TokenData]) = {
+      if (next.token != EMPTY) pushCopy(next)
+      pushCopy(this)
+      following = tds ++ following
+      popCopy(this)
+      if (following.nonEmpty) popCopy(next)
+    }
+
     /** a stack of tokens which indicates whether line-ends can be statement separators
      *  also used for keeping track of nesting levels.
      *  We keep track of the closing symbol of a region. This can be
@@ -310,7 +340,7 @@ object Scanners {
         if (token == ERROR) adjustSepRegions(STRINGLIT)
       } else {
         this copyFrom next
-        next.token = EMPTY
+        popCopy(next)
       }
 
       /** Insert NEWLINE or NEWLINES if

compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java

@@ -50,7 +50,7 @@ public enum ErrorMessageID {
     ExpectedTokenButFoundID,
     MixedLeftAndRightAssociativeOpsID,
     CantInstantiateAbstractClassOrTraitID,
-    AnnotatedPrimaryConstructorRequiresModifierOrThisID,
+    DUMMY_AVAILABLE_1,
     OverloadedOrRecursiveMethodNeedsResultTypeID,
     RecursiveValueNeedsResultTypeID,
     CyclicReferenceInvolvingID,

compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala

@@ -1138,21 +1138,6 @@ object messages {
            |""".stripMargin
   }
 
-  case class AnnotatedPrimaryConstructorRequiresModifierOrThis(cls: Name)(implicit ctx: Context)
-  extends Message(AnnotatedPrimaryConstructorRequiresModifierOrThisID) {
-    val kind = "Syntax"
-    val msg = hl"""${"private"}, ${"protected"}, or ${"this"} expected for annotated primary constructor"""
-    val explanation =
-      hl"""|When using annotations with a primary constructor of a class,
-           |the annotation must be followed by an access modifier
-           |(${"private"} or ${"protected"}) or ${"this"}.
-           |
-           |For example:
-           |  ${"class Sample @deprecated this(param: Parameter) { ..."}
-           |                           ^^^^
-           |""".stripMargin
-  }
-
   case class OverloadedOrRecursiveMethodNeedsResultType(tree: Names.TermName)(implicit ctx: Context)
   extends Message(OverloadedOrRecursiveMethodNeedsResultTypeID) {
     val kind = "Syntax"

compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala

@@ -198,21 +198,6 @@ class ErrorMessagesTests extends ErrorMessagesTest {
       assertTrue("expected trait", isTrait)
     }
 
-  @Test def constructorModifier =
-    checkMessagesAfter("frontend") {
-      """
-        |class AnotherClass @deprecated ()
-      """.stripMargin
-    }
-    .expect { (ictx, messages) =>
-      implicit val ctx: Context = ictx
-      val defn = ictx.definitions
-
-      assertMessageCount(1, messages)
-      val AnnotatedPrimaryConstructorRequiresModifierOrThis(cls) :: Nil = messages
-      assertEquals("AnotherClass", cls.show)
-    }
-
   @Test def overloadedMethodNeedsReturnType =
     checkMessagesAfter("frontend") {
       """

docs/docs/internals/syntax.md

@@ -335,8 +335,7 @@ TmplDef           ::=  ([‘case’ | `enum'] ‘class’ | trait’) ClassDef
                     |  `enum' EnumDef
 ClassDef          ::=  id ClassConstr TemplateOpt                               ClassDef(mods, name, tparams, templ)
 ClassConstr       ::=  [ClsTypeParamClause] [ConstrMods] ClsParamClauses         with DefDef(_, <init>, Nil, vparamss, EmptyTree, EmptyTree) as first stat
-ConstrMods        ::=  AccessModifier
-                    |  Annotation {Annotation} (AccessModifier | ‘this’)
+ConstrMods        ::=  {Annotation} [AccessModifier]
 ObjectDef         ::=  id TemplateOpt                                           ModuleDef(mods, name, template)  // no constructor
 EnumDef           ::=  id ClassConstr [`extends' [ConstrApps]]                  EnumDef(mods, name, tparams, template)
                        [nl] ‘{’ EnumCaseStat {semi EnumCaseStat} ‘}’

tests/pos/i2426.scala

@@ -0,0 +1,24 @@
+class Foo @deprecated("foo", "2.11.0") (x: Int)
+
+class Bar @deprecated(x: Int)
+
+class Baz1 @deprecated(implicit c: C)
+class Baz2 @deprecated()(implicit c: C)
+class Baz3 @deprecated()()(implicit c: C)
+
+object Test {
+  implicit val c: C = obj
+  new Baz1
+  new Baz2
+  new Baz3()
+}
+
+class D(implicit x: C)
+
+class C
+object obj extends C
+
+class ann(x: C)(y: C, s: String) extends scala.annotation.Annotation
+
+class Bam @ann(obj)(obj, "h")(n: String)
+