fix: Support auto accessors with TypeScript annotations #15209
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -90,12 +90,14 @@ const TSErrors = ParseErrorEnum`typescript`({ | |
| propertyName: string; | ||
| }) => | ||
| `Property '${propertyName}' cannot have an initializer because it is marked abstract.`, | ||
| AccessorCannotBeOptional: | ||
|
There was a problem hiding this comment. We can (and should) fix the error messages now (i.e. There was a problem hiding this comment. Do error codes refer to the property names? These seem to be only visible from the inside. There was a problem hiding this comment. Yes they are the property names; and they are public. For example, prettier relies on them: https://github.com/prettier/prettier/blob/b50dfd1974d33abb980e9b4df949913932eeb478/src/language-js/parse/babel.js#L183 |
||
| "An 'accessor' property cannot be declared optional.", | ||
| AccessorCannotDeclareThisParameter: | ||
| "'get' and 'set' accessors cannot declare 'this' parameters.", | ||
| AccessorCannotHaveTypeParameters: "An accessor cannot have type parameters.", | ||
| ClassMethodHasDeclare: "Class methods cannot have the 'declare' modifier.", | ||
| ClassMethodHasReadonly: "Class methods cannot have the 'readonly' modifier.", | ||
| ConstInitializerMustBeStringOrNumericLiteralOrLiteralEnumReference: | ||
| "A 'const' initializer in an ambient context must be a string or numeric literal or literal enum reference.", | ||
| ConstructorHasTypeParameters: | ||
| "Type parameters cannot appear on a constructor declaration.", | ||
|
|
@@ -194,11 +196,11 @@ const TSErrors = ParseErrorEnum`typescript`({ | |
| "This syntax is reserved in files with the .mts or .cts extension. Add a trailing comma, as in `<T,>() => ...`.", | ||
| ReservedTypeAssertion: | ||
| "This syntax is reserved in files with the .mts or .cts extension. Use an `as` expression instead.", | ||
| SetAccessorCannotHaveOptionalParameter: | ||
| "A 'set' accessor cannot have an optional parameter.", | ||
| SetAccessorCannotHaveRestParameter: | ||
| "A 'set' accessor cannot have rest parameter.", | ||
| SetAccessorCannotHaveReturnType: | ||
| "A 'set' accessor cannot have a return type annotation.", | ||
| SingleTypeParameterWithoutTrailingComma: ({ | ||
| typeParameterName, | ||
|
|
@@ -835,7 +837,7 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) => | |
| } | ||
| const method: N.TsMethodSignature = nodeAny; | ||
| if (method.kind && this.match(tt.lt)) { | ||
| this.raise(TSErrors.AccessorCannotHaveTypeParameters, { | ||
| at: this.state.curPosition(), | ||
| }); | ||
| } | ||
|
|
@@ -851,7 +853,7 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) => | |
| if (method[paramsKey].length > 0) { | ||
| this.raise(Errors.BadGetterArity, { at: this.state.curPosition() }); | ||
| if (this.isThisParam(method[paramsKey][0])) { | ||
| this.raise(TSErrors.AccessorCannotDeclareThisParameter, { | ||
| at: this.state.curPosition(), | ||
| }); | ||
| } | ||
|
|
@@ -862,26 +864,26 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) => | |
| } else { | ||
| const firstParameter = method[paramsKey][0]; | ||
| if (this.isThisParam(firstParameter)) { | ||
| this.raise(TSErrors.AccessorCannotDeclareThisParameter, { | ||
| at: this.state.curPosition(), | ||
| }); | ||
| } | ||
| if ( | ||
| firstParameter.type === "Identifier" && | ||
| firstParameter.optional | ||
| ) { | ||
| this.raise(TSErrors.SetAccessorCannotHaveOptionalParameter, { | ||
| at: this.state.curPosition(), | ||
| }); | ||
| } | ||
| if (firstParameter.type === "RestElement") { | ||
| this.raise(TSErrors.SetAccessorCannotHaveRestParameter, { | ||
| at: this.state.curPosition(), | ||
| }); | ||
| } | ||
| } | ||
| if (method[returnTypeKey]) { | ||
| this.raise(TSErrors.SetAccessorCannotHaveReturnType, { | ||
| at: method[returnTypeKey], | ||
| }); | ||
| } | ||
|
|
@@ -2794,7 +2796,7 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) => | |
| !isPossiblyLiteralEnum(init) | ||
| ) { | ||
| this.raise( | ||
| TSErrors.ConstInitializerMustBeStringOrNumericLiteralOrLiteralEnumReference, | ||
| { at: init }, | ||
| ); | ||
| } | ||
|
|
@@ -3124,10 +3126,14 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) => | |
| } | ||
|
|
||
| parseClassPropertyAnnotation( | ||
| node: N.ClassProperty | N.ClassPrivateProperty | N.ClassAccessorProperty, | ||
| ): void { | ||
| if (!node.optional) { | ||
| if (this.eat(tt.bang)) { | ||
| node.definite = true; | ||
| } else if (this.eat(tt.question)) { | ||
| node.optional = true; | ||
| } | ||
| } | ||
|
|
||
| const type = this.tsTryParseTypeAnnotation(); | ||
|
|
@@ -3181,6 +3187,16 @@ export default (superClass: ClassWithMixin<typeof Parser, IJSXParserMixin>) => | |
| return super.parseClassPrivateProperty(node); | ||
| } | ||
|
|
||
| parseClassAccessorProperty( | ||
| node: N.ClassAccessorProperty, | ||
| ): N.ClassAccessorProperty { | ||
| this.parseClassPropertyAnnotation(node); | ||
| if (node.optional) { | ||
| this.raise(TSErrors.AccessorCannotBeOptional, { at: node }); | ||
| } | ||
| return super.parseClassAccessorProperty(node); | ||
| } | ||
|
|
||
| pushClassMethod( | ||
| classBody: N.ClassBody, | ||
| method: N.ClassMethod, | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| abstract class Foo { | ||
| declare accessor prop7: number; | ||
JLHwung marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| private accessor #p: any; | ||
|
|
||
| accessor a!; | ||
|
There was a problem hiding this comment. TS will throw |
||
| abstract accessor #s; | ||
| accessor #d?; | ||
| abstract accessor f = 1; | ||
| readonly accessor g; | ||
| } | ||
|
|
||
| declare class C { | ||
| accessor x = 1; | ||
| #y = 1; | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "plugins": [ | ||
| "typescript", | ||
| "decoratorAutoAccessors" | ||
| ] | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,220 @@ | ||
| { | ||
| "type": "File", | ||
| "start":0,"end":239,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":15,"column":1,"index":239}}, | ||
| "errors": [ | ||
| "SyntaxError: An 'accessor' property cannot be declared optional. (7:2)" | ||
|
There was a problem hiding this comment. It seems that other errors are not reported. There was a problem hiding this comment. Yes, I only reported that unrecoverable error. There was a problem hiding this comment. Doesn't the There was a problem hiding this comment. I mean currently the only errors reported are from unrecoverable errors. And the rest are illegal but normal parsing, I don't think it matters, so they haven't been implemented. There was a problem hiding this comment.
Why is There was a problem hiding this comment.
There was a problem hiding this comment.
Although Generally the Babel parser does not report typing errors, such as "the call signature does not match the function interface": they are considered as "runtime errors" because these errors can only be reported from a typing system. But for those errors that can be detected solely from AST, they are most static errors and thus they should be reported by the parser. We have reported similar errors, e.g. Babel throws "Private elements cannot have an accessibility modifier" for class C {
private #Q;
}Therefore I am not convinced the idea that "the rest are illegal but normal parsing". If TS would ever have a spec, they should be early errors and a parser should report them. There was a problem hiding this comment.
My bad, by a better error message, I do mean that we can recover from this error and throw a recoverable error. There was a problem hiding this comment. I personally prefer to make unrecoverable errors recoverable, and normally parsed ts errors are not actively reported. (at least when no user requests these)
I've done this and the only error is from what you said. |
||
| ], | ||
| "program": { | ||
| "type": "Program", | ||
| "start":0,"end":239,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":15,"column":1,"index":239}}, | ||
| "sourceType": "module", | ||
| "interpreter": null, | ||
| "body": [ | ||
| { | ||
| "type": "ClassDeclaration", | ||
| "start":0,"end":190,"loc":{"start":{"line":1,"column":0,"index":0},"end":{"line":10,"column":1,"index":190}}, | ||
| "abstract": true, | ||
| "id": { | ||
| "type": "Identifier", | ||
| "start":15,"end":18,"loc":{"start":{"line":1,"column":15,"index":15},"end":{"line":1,"column":18,"index":18},"identifierName":"Foo"}, | ||
| "name": "Foo" | ||
| }, | ||
| "superClass": null, | ||
| "body": { | ||
| "type": "ClassBody", | ||
| "start":19,"end":190,"loc":{"start":{"line":1,"column":19,"index":19},"end":{"line":10,"column":1,"index":190}}, | ||
| "body": [ | ||
| { | ||
| "type": "ClassAccessorProperty", | ||
| "start":23,"end":54,"loc":{"start":{"line":2,"column":2,"index":23},"end":{"line":2,"column":33,"index":54}}, | ||
| "declare": true, | ||
| "static": false, | ||
| "key": { | ||
| "type": "Identifier", | ||
| "start":40,"end":45,"loc":{"start":{"line":2,"column":19,"index":40},"end":{"line":2,"column":24,"index":45},"identifierName":"prop7"}, | ||
| "name": "prop7" | ||
| }, | ||
| "computed": false, | ||
| "typeAnnotation": { | ||
| "type": "TSTypeAnnotation", | ||
| "start":45,"end":53,"loc":{"start":{"line":2,"column":24,"index":45},"end":{"line":2,"column":32,"index":53}}, | ||
| "typeAnnotation": { | ||
| "type": "TSNumberKeyword", | ||
| "start":47,"end":53,"loc":{"start":{"line":2,"column":26,"index":47},"end":{"line":2,"column":32,"index":53}} | ||
| } | ||
| }, | ||
| "value": null | ||
| }, | ||
| { | ||
| "type": "ClassAccessorProperty", | ||
| "start":57,"end":82,"loc":{"start":{"line":3,"column":2,"index":57},"end":{"line":3,"column":27,"index":82}}, | ||
| "accessibility": "private", | ||
| "static": false, | ||
| "key": { | ||
| "type": "PrivateName", | ||
| "start":74,"end":76,"loc":{"start":{"line":3,"column":19,"index":74},"end":{"line":3,"column":21,"index":76}}, | ||
| "id": { | ||
| "type": "Identifier", | ||
| "start":75,"end":76,"loc":{"start":{"line":3,"column":20,"index":75},"end":{"line":3,"column":21,"index":76},"identifierName":"p"}, | ||
| "name": "p" | ||
| } | ||
| }, | ||
| "computed": false, | ||
| "typeAnnotation": { | ||
| "type": "TSTypeAnnotation", | ||
| "start":76,"end":81,"loc":{"start":{"line":3,"column":21,"index":76},"end":{"line":3,"column":26,"index":81}}, | ||
| "typeAnnotation": { | ||
| "type": "TSAnyKeyword", | ||
| "start":78,"end":81,"loc":{"start":{"line":3,"column":23,"index":78},"end":{"line":3,"column":26,"index":81}} | ||
| } | ||
| }, | ||
| "value": null | ||
| }, | ||
| { | ||
| "type": "ClassAccessorProperty", | ||
| "start":86,"end":98,"loc":{"start":{"line":5,"column":2,"index":86},"end":{"line":5,"column":14,"index":98}}, | ||
| "static": false, | ||
| "key": { | ||
| "type": "Identifier", | ||
| "start":95,"end":96,"loc":{"start":{"line":5,"column":11,"index":95},"end":{"line":5,"column":12,"index":96},"identifierName":"a"}, | ||
| "name": "a" | ||
| }, | ||
| "computed": false, | ||
| "definite": true, | ||
| "value": null | ||
| }, | ||
| { | ||
| "type": "ClassAccessorProperty", | ||
| "start":101,"end":122,"loc":{"start":{"line":6,"column":2,"index":101},"end":{"line":6,"column":23,"index":122}}, | ||
| "abstract": true, | ||
| "static": false, | ||
| "key": { | ||
| "type": "PrivateName", | ||
| "start":119,"end":121,"loc":{"start":{"line":6,"column":20,"index":119},"end":{"line":6,"column":22,"index":121}}, | ||
| "id": { | ||
| "type": "Identifier", | ||
| "start":120,"end":121,"loc":{"start":{"line":6,"column":21,"index":120},"end":{"line":6,"column":22,"index":121},"identifierName":"s"}, | ||
| "name": "s" | ||
| } | ||
| }, | ||
| "computed": false, | ||
| "value": null | ||
| }, | ||
| { | ||
| "type": "ClassAccessorProperty", | ||
| "start":125,"end":138,"loc":{"start":{"line":7,"column":2,"index":125},"end":{"line":7,"column":15,"index":138}}, | ||
| "static": false, | ||
| "key": { | ||
| "type": "PrivateName", | ||
| "start":134,"end":136,"loc":{"start":{"line":7,"column":11,"index":134},"end":{"line":7,"column":13,"index":136}}, | ||
| "id": { | ||
| "type": "Identifier", | ||
| "start":135,"end":136,"loc":{"start":{"line":7,"column":12,"index":135},"end":{"line":7,"column":13,"index":136},"identifierName":"d"}, | ||
| "name": "d" | ||
| } | ||
| }, | ||
| "computed": false, | ||
| "optional": true, | ||
| "value": null | ||
| }, | ||
| { | ||
| "type": "ClassAccessorProperty", | ||
| "start":141,"end":165,"loc":{"start":{"line":8,"column":2,"index":141},"end":{"line":8,"column":26,"index":165}}, | ||
| "abstract": true, | ||
| "static": false, | ||
| "key": { | ||
| "type": "Identifier", | ||
| "start":159,"end":160,"loc":{"start":{"line":8,"column":20,"index":159},"end":{"line":8,"column":21,"index":160},"identifierName":"f"}, | ||
| "name": "f" | ||
| }, | ||
| "computed": false, | ||
| "value": { | ||
| "type": "NumericLiteral", | ||
| "start":163,"end":164,"loc":{"start":{"line":8,"column":24,"index":163},"end":{"line":8,"column":25,"index":164}}, | ||
| "extra": { | ||
| "rawValue": 1, | ||
| "raw": "1" | ||
| }, | ||
| "value": 1 | ||
| } | ||
| }, | ||
| { | ||
| "type": "ClassAccessorProperty", | ||
| "start":168,"end":188,"loc":{"start":{"line":9,"column":2,"index":168},"end":{"line":9,"column":22,"index":188}}, | ||
| "readonly": true, | ||
| "static": false, | ||
| "key": { | ||
| "type": "Identifier", | ||
| "start":186,"end":187,"loc":{"start":{"line":9,"column":20,"index":186},"end":{"line":9,"column":21,"index":187},"identifierName":"g"}, | ||
| "name": "g" | ||
| }, | ||
| "computed": false, | ||
| "value": null | ||
| } | ||
| ] | ||
| } | ||
| }, | ||
| { | ||
| "type": "ClassDeclaration", | ||
| "start":192,"end":239,"loc":{"start":{"line":12,"column":0,"index":192},"end":{"line":15,"column":1,"index":239}}, | ||
| "declare": true, | ||
| "id": { | ||
| "type": "Identifier", | ||
| "start":206,"end":207,"loc":{"start":{"line":12,"column":14,"index":206},"end":{"line":12,"column":15,"index":207},"identifierName":"C"}, | ||
| "name": "C" | ||
| }, | ||
| "superClass": null, | ||
| "body": { | ||
| "type": "ClassBody", | ||
| "start":208,"end":239,"loc":{"start":{"line":12,"column":16,"index":208},"end":{"line":15,"column":1,"index":239}}, | ||
| "body": [ | ||
| { | ||
| "type": "ClassAccessorProperty", | ||
| "start":212,"end":227,"loc":{"start":{"line":13,"column":2,"index":212},"end":{"line":13,"column":17,"index":227}}, | ||
| "static": false, | ||
| "key": { | ||
| "type": "Identifier", | ||
| "start":221,"end":222,"loc":{"start":{"line":13,"column":11,"index":221},"end":{"line":13,"column":12,"index":222},"identifierName":"x"}, | ||
| "name": "x" | ||
| }, | ||
| "computed": false, | ||
| "value": { | ||
| "type": "NumericLiteral", | ||
| "start":225,"end":226,"loc":{"start":{"line":13,"column":15,"index":225},"end":{"line":13,"column":16,"index":226}}, | ||
| "extra": { | ||
| "rawValue": 1, | ||
| "raw": "1" | ||
| }, | ||
| "value": 1 | ||
| } | ||
| }, | ||
| { | ||
| "type": "ClassPrivateProperty", | ||
| "start":230,"end":237,"loc":{"start":{"line":14,"column":2,"index":230},"end":{"line":14,"column":9,"index":237}}, | ||
| "static": false, | ||
| "key": { | ||
| "type": "PrivateName", | ||
| "start":230,"end":232,"loc":{"start":{"line":14,"column":2,"index":230},"end":{"line":14,"column":4,"index":232}}, | ||
| "id": { | ||
| "type": "Identifier", | ||
| "start":231,"end":232,"loc":{"start":{"line":14,"column":3,"index":231},"end":{"line":14,"column":4,"index":232},"identifierName":"y"}, | ||
| "name": "y" | ||
| } | ||
| }, | ||
| "value": { | ||
| "type": "NumericLiteral", | ||
| "start":235,"end":236,"loc":{"start":{"line":14,"column":7,"index":235},"end":{"line":14,"column":8,"index":236}}, | ||
| "extra": { | ||
| "rawValue": 1, | ||
| "raw": "1" | ||
| }, | ||
| "value": 1 | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| ], | ||
| "directives": [] | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| abstract class Foo { | ||
| accessor prop: number = 1; | ||
| static accessor prop2: number = 1; | ||
| accessor #prop3: number = 1; | ||
| accessor [prop4]: number = 1; | ||
| private accessor prop5: number = 1; | ||
| abstract accessor prop6: number; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| { | ||
| "plugins": [ | ||
| "typescript", | ||
| "decoratorAutoAccessors" | ||
| ] | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is kind of annoying, delaying to babel8 means we have to copy and paste a lot of tests, and modify it immediately I'm not sure if it will affect compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do it in a separate PR.
We don't have to defer to Babel 8: The error message improvement can be shipped in patch releases, since it is not part of our API protocol. This approach is also similar to how Node.js treats its error message.