Fixes annotation, sort of

[eed3si9n]

Fixes #41

Problem

1. class constructor should accept annotation, but it doesn't.
2. Apparently Scala 3 does allow zero, one, or multiple parameter list
   to an annotation, and differentiated between the class parameter by
   its lack of type ascription.

Solution

1. Add optional annotation for constructor. At this point, we can at
   least parse the code with annotation on the ctor.
2. Unfortunatley, however, tree-sitter doesn't make enough lookahead to
   distinguish postfix expr or ascription expression (x: Int),
   for now the class parameters will end up being parsed as arguments to
   the annotation.

[smarter]

annotation should only accept a single parameter list

In Scala 3 this was relaxed to allow zero, one or more parameter lists based on whether the parameter list has type ascriptions (so class Bla @foo (x: Int) is parsed as having a zero-parameter annotation @foo: https://github.com/lampepfl/dotty/blob/main/tests/pos/i2426.scala, https://github.com/lampepfl/dotty/blob/722acb91ca9aca4cc74ab830be85cc0d933ac8b6/compiler/src/dotty/tools/dotc/parsing/Parsers.scala#L2579-L2585
Sorry about that 😅

[eed3si9n]

In Scala 3 this was relaxed to allow zero, one or more parameter lists based on whether the parameter list has type ascriptions

oo interesting. Thanks for the hint. Considering:

Annotation        ::=  ‘@’ SimpleType1 {ParArgumentExprs}
ParArgumentExprs  ::=  ‘(’ [ExprsInParens] ‘)’
                    |  ‘(’ ‘using’ ExprsInParens ‘)’
                    |  ‘(’ [ExprsInParens ‘,’] PostfixExpr ‘*’ ‘)’
ExprsInParens     ::=  ExprInParens {‘,’ ExprInParens}
ExprInParens      ::=  PostfixExpr ‘:’ Type
                    |  Expr

I guess we should think of AnnotationArgument ::= '(' [PostfixExpr]* ')'

[ckipp01]

Just to make sure we're on the same page

Unfortunatley, however, tree-sitter doesn't make enough lookahead to
distinguish postfix expr or ascription expression (x: Int),
for now the class parameters will end up being parsed as arguments to
the annotation.

I sort of actually see this as a regression in the class parameter parsing this. Do you feel like this is an improvement enough in handling annotations to move forward, or no? I don't have strong feelings on it, and honestly I never even knew you could have multiple parameter groups in annotations (mainly because I've just never used it).

[eed3si9n]

@ckipp01

I sort of actually see this as a regression in the class parameter parsing this. Do you feel like this is an improvement enough in handling annotations to move forward, or no? I don't have strong feelings on it, and honestly I never even knew you could have multiple parameter groups in annotations (mainly because I've just never used it).

The status quo on master is that a legal constructor with an attribute fails to parse, and its methods would leak out:

$ cat examples/C.scala
// TODO: The last argument should become class_parameters
class A @Inject()(x: Int, y: Int) {
  def x = 1
}

$ tree-sitter parse examples/C.scala
(compilation_unit [0, 0] - [5, 0]
  (comment [0, 0] - [0, 57])
  (class_definition [1, 0] - [1, 7]
    name: (identifier [1, 6] - [1, 7]))
  (function_definition [1, 8] - [2, 11]
    (annotation [1, 8] - [1, 33]
      name: (type_identifier [1, 9] - [1, 15])
      arguments: (arguments [1, 15] - [1, 17])
      arguments: (arguments [1, 17] - [1, 33]
        (ascription_expression [1, 18] - [1, 24]
          (identifier [1, 18] - [1, 19])
          (type_identifier [1, 21] - [1, 24]))
        (ascription_expression [1, 26] - [1, 32]
          (identifier [1, 26] - [1, 27])
          (type_identifier [1, 29] - [1, 32]))))
    (ERROR [1, 34] - [2, 2])
    name: (identifier [2, 6] - [2, 7])
    body: (integer_literal [2, 10] - [2, 11]))
  (ERROR [3, 0] - [5, 0]))
examples/C.scala        0 ms    (ERROR [1, 34] - [2, 2])

With this PR, it does the same ascription_expression thing, and it no longer fails to parse. I think this is a slight improvement because at least it will parse as a class.

$ tree-sitter parse examples/C.scala
(compilation_unit [0, 0] - [5, 0]
  (comment [0, 0] - [0, 57])
  (class_definition [1, 0] - [3, 1]
    name: (identifier [1, 6] - [1, 7])
    (annotation [1, 8] - [1, 33]
      name: (type_identifier [1, 9] - [1, 15])
      arguments: (arguments [1, 15] - [1, 17])
      arguments: (arguments [1, 17] - [1, 33]
        (ascription_expression [1, 18] - [1, 24]
          (identifier [1, 18] - [1, 19])
          (type_identifier [1, 21] - [1, 24]))
        (ascription_expression [1, 26] - [1, 32]
          (identifier [1, 26] - [1, 27])
          (type_identifier [1, 29] - [1, 32]))))
    body: (template_body [1, 34] - [3, 1]
      (function_definition [2, 2] - [2, 11]
        name: (identifier [2, 6] - [2, 7])
        body: (integer_literal [2, 10] - [2, 11])))))

[ckipp01]

With this PR, it does the same ascription_expression thing, and it fails to parse. I think this is a slight improvement because at least it will parse as a class.

Ah, agree. Thanks for clarifying.

[ckipp01]

🚀

[smarter]

I think behaving like scala 2 for now would give you the most amount of code parsed correctly considering no one knows the new scala 3 behavior exists

[eed3si9n]

I think behaving like scala 2 for now would give you the most amount of code parsed correctly considering no one knows the new scala 3 behavior exists

Yea. I thought about throwing Scala 3 under the bus as well, but it's a fairly common thing to have parameter-less attribute, so we'd still have issues misinterpreting attributes unless we make a special case for constructor attributes only?

[eed3si9n]

Assuming that constructor attribute in general is fairly uncommon, I think this is ok for now.

[smarter]

I think people usually annotate constructors with java annotations where you always have exactly one parameter list.