CFE allows trailing comma and multiple arguments in extension type representation declaration

[lrhn]

The current extension type specification proposal does not allow a trailing comma in the representation type declaration.
Example:

extension type E(int x,) {} // Not allowed

The CFE currently allows this, and it should not.

Further, if I add a second argument, extension type E(int x, int y) {}, the CFE accepts that too, and also an invocation of it:

extension type E(int x, String y) {}
void main() {
 var e = E(1, "b");  // Complains if giving too few arguments.
 print(e.x); // b 
 // print(e.y); // y
 print(e.runtimeType); // String
 int x = e.x; // succeeds.
 print(x.runtimeType); // String, **unsounds**
}

Or make the paramters optional or named:

extension type E({int x = 0}) {}
void main() {
  print(E(x: 42).x); // 42
}

Or I can omit the type entirely: extension type E(x) {} void main() { print(E(42).x); } prints "42".

All this is invalid syntax. While it's nice that the grammar parser allows it, so that we can give better error messages, the syntax should be rejected by a later validation step, preferably still in the parser.

(The analyzer has this validation step. The error given for the trailing comma isn't great,

  error - trailcom.dart:1:23 - Each extension type should have                             
          exactly one representation field. Try combining fields                           
          into a record, or removing extra fields. -                                       
          multiple_representation_fields                        

but for two arguments, or named arguments, an error of

  error - trailcom2.dart:1:18 - Expected a representation field.
          Try providing the representation field for this
          extension type. - expected_representation_field

is reasonable. It has some further down-stream errors from not having a valid representation type declaration, which could be quenched.

The analyzer AST model also has a trailingComma token in the RepresentationDeclaration which should always be null.)

If we ever introduce primary constructors in general, with a syntax compatible with extension type declarations, it's likely that the syntax for extension types will be opened up to allow other singleton parameter lists, and trailing commas (and at that point the analyzer AST could give RepresentationDeclaration an asPrimaryConstructorParameterList method).

So far then the syntax is restricted to '(' <metadata> <type> <identifier> ')'. No more and no less.

[eernstg]

Do you wish to enforce that there cannot be <metadata> on the parameter? I think there was a request for allowing the metadata already now (in spite of the fact that the <representationDeclaration> is currently as minimal as we could make it).

[lrhn]

No, I just forgot about metadata. I never use it :)
Added to the original message now.

[scheglov]

AFAIK we don't have trailingComma in RepresentationDeclaration in the analyzer.
I will add a separate error for trailing comma.
https://dart-review.googlesource.com/c/sdk/+/328340

[lrhn]

There is indeed no trailing comma in the current analyzer AST. I mistook the general commaAfter for being that.

[chloestefantsova]

The issue is addressed by the following: ffd43b2, 0e2ed5a, 75920dd.