TSMappedType - TSTypeParameter.name issue #109
Comments
|
it has to be identifier, i missed this case, we can change that. |
|
// current:
// v string (TSTypeParameter#name)
// v TSTypeReference (TSTypeParameter#constraint)
// vvvvvvvvvvv TSTypeParameter
function a<A extends B>(b: c) {}
// ^^^^ Identifier
// ^ TypeAnnotation (Identifier#typeAnnotation)
// ^ string (Identifier#name)
// IMO, it should be:
// v Identifier (TSTypeParameter#name)
// v TSTypeReference (TSTypeParameter#constraint)
// vvvvvvvvvvv TypeParameter
function a<A extends B>(b: c) {}
// ^^^^ Parameter
// ^ TypeAnnotation (Parameter#typeAnnotation)
// ^ Identifier (Parameter#name)FWIW, I tracked down the history in babylon:
|
|
@JamesHenry besides normal test cases from this project i'm running tests over popular libraries and typescript test cases in https://github.com/armano2/typescript-estree-test (~19k files) but sometimes its hard to spot all potential issues 🦊 i even added d.ts generator with all possible outputs to do some comparison between my changes. TSTypeParameter: compare export interface TSTypeParameter extends BaseNode {
type: 'TSTypeParameter';
+ name: string;
- name: string | Identifier;
default?:
| TSFunctionType
| TSIndexedAccessType
@@ -2237,6 +2166,7 @@ export interface TSTypeParameter extends BaseNode {
| TSObjectKeyword
| TSStringKeyword;
constraint?:
- | Literal
| TSArrayType
| TSConstructorType
| TSFunctionType
|
|
to change this we have to just disable some tests from babel and change this line - name: node.name.text,
+ name: convertChildType(node.name),https://github.com/JamesHenry/typescript-estree/blob/master/src/convert.ts#L2240 |
|
Yep! I won’t get chance to do a PR until tomorrow so if you have time feel free to submit, otherwise I’m very happy to do it |
|
31 tests is failing :( |
Considering the following source:
Prior to v8 we didn't have any test coverage for mapped types.
@armano2 added good test coverage, and at the same time explicit conversion logic (rather than using the
deeplyCopy()fallback which had implicitly been used up until that point) in the PR which would become v8.Because of not having the test coverage in there prior to the code change, we didn't explicitly point out a change which is probably worthy of discussion:
The lowercase
c(TSTypeParameter.name) was previously anIdentifier- now it is just a string value (i.e. it has no node in the AST).I discovered this when trying to update Prettier here to the latest: prettier/prettier#5728
The final failing test case is around prettier being unable to attach the comment
/* commentC */in the way it expects. Naturally this is because it is trying to attached it to theTSTypeParameter.name, which used to be a node, and no longer is.Without this being reinstated to an
Identifiernode, I'm not sure how we could preserve Prettier's behaviour here?We are not ignoring the new mapped type test sources in the ast-alignment-tests, so we must be aligned with the babel AST here, but maybe they both need to change in this case?
Would love to know your thoughts: @uniqueiniquity @armano2 @ikatyang
Full ASTs in each case:
Before v8 (c is an Identifier)
{ "body": [ { "id": { "loc": { "end": { "column": 6, "line": 1 }, "start": { "column": 5, "line": 1 } }, "name": "C", "range": [ 5, 6 ], "type": "Identifier" }, "loc": { "end": { "column": 1, "line": 3 }, "start": { "column": 0, "line": 1 } }, "range": [ 0, 48 ], "type": "TSTypeAliasDeclaration", "typeAnnotation": { "loc": { "end": { "column": 1, "line": 3 }, "start": { "column": 9, "line": 1 } }, "range": [ 9, 48 ], "type": "TSMappedType", "typeAnnotation": { "loc": { "end": { "column": 35, "line": 2 }, "start": { "column": 27, "line": 2 } }, "range": [ 38, 46 ], "type": "TSTypeAnnotation", "typeAnnotation": { "loc": { "end": { "column": 35, "line": 2 }, "start": { "column": 29, "line": 2 } }, "range": [ 40, 46 ], "type": "TSStringKeyword" } }, "typeParameter": { "constraint": { "loc": { "end": { "column": 26, "line": 2 }, "start": { "column": 25, "line": 2 } }, "range": [ 36, 37 ], "type": "TSTypeReference", "typeName": { "loc": { "end": { "column": 26, "line": 2 }, "start": { "column": 25, "line": 2 } }, "name": "C", "range": [ 36, 37 ], "type": "Identifier" } }, "loc": { "end": { "column": 26, "line": 2 }, "start": { "column": 20, "line": 2 } }, "name": { "loc": { "end": { "column": 21, "line": 2 }, "start": { "column": 20, "line": 2 } }, "name": "c", "range": [ 31, 32 ], "type": "Identifier" }, "range": [ 31, 37 ], "type": "TSTypeParameter" } } } ], "loc": { "end": { "column": 1, "line": 3 }, "start": { "column": 0, "line": 1 } }, "range": [ 0, 48 ], "sourceType": "script", "tokens": [ { "loc": { "end": { "column": 4, "line": 1 }, "start": { "column": 0, "line": 1 } }, "range": [ 0, 4 ], "type": "Identifier", "value": "type" }, { "loc": { "end": { "column": 6, "line": 1 }, "start": { "column": 5, "line": 1 } }, "range": [ 5, 6 ], "type": "Identifier", "value": "C" }, { "loc": { "end": { "column": 8, "line": 1 }, "start": { "column": 7, "line": 1 } }, "range": [ 7, 8 ], "type": "Punctuator", "value": "=" }, { "loc": { "end": { "column": 10, "line": 1 }, "start": { "column": 9, "line": 1 } }, "range": [ 9, 10 ], "type": "Punctuator", "value": "{" }, { "loc": { "end": { "column": 5, "line": 2 }, "start": { "column": 4, "line": 2 } }, "range": [ 15, 16 ], "type": "Punctuator", "value": "[" }, { "loc": { "end": { "column": 21, "line": 2 }, "start": { "column": 20, "line": 2 } }, "range": [ 31, 32 ], "type": "Identifier", "value": "c" }, { "loc": { "end": { "column": 24, "line": 2 }, "start": { "column": 22, "line": 2 } }, "range": [ 33, 35 ], "type": "Keyword", "value": "in" }, { "loc": { "end": { "column": 26, "line": 2 }, "start": { "column": 25, "line": 2 } }, "range": [ 36, 37 ], "type": "Identifier", "value": "C" }, { "loc": { "end": { "column": 27, "line": 2 }, "start": { "column": 26, "line": 2 } }, "range": [ 37, 38 ], "type": "Punctuator", "value": "]" }, { "loc": { "end": { "column": 28, "line": 2 }, "start": { "column": 27, "line": 2 } }, "range": [ 38, 39 ], "type": "Punctuator", "value": ":" }, { "loc": { "end": { "column": 35, "line": 2 }, "start": { "column": 29, "line": 2 } }, "range": [ 40, 46 ], "type": "Identifier", "value": "string" }, { "loc": { "end": { "column": 1, "line": 3 }, "start": { "column": 0, "line": 3 } }, "range": [ 47, 48 ], "type": "Punctuator", "value": "}" } ], "type": "Program" }Current (c is a string value)
{ "body": [ { "id": { "loc": { "end": { "column": 6, "line": 1 }, "start": { "column": 5, "line": 1 } }, "name": "C", "range": [ 5, 6 ], "type": "Identifier" }, "loc": { "end": { "column": 1, "line": 3 }, "start": { "column": 0, "line": 1 } }, "range": [ 0, 48 ], "type": "TSTypeAliasDeclaration", "typeAnnotation": { "loc": { "end": { "column": 1, "line": 3 }, "start": { "column": 9, "line": 1 } }, "range": [ 9, 48 ], "type": "TSMappedType", "typeAnnotation": { "loc": { "end": { "column": 35, "line": 2 }, "start": { "column": 29, "line": 2 } }, "range": [ 40, 46 ], "type": "TSStringKeyword" }, "typeParameter": { "constraint": { "loc": { "end": { "column": 26, "line": 2 }, "start": { "column": 25, "line": 2 } }, "range": [ 36, 37 ], "type": "TSTypeReference", "typeName": { "loc": { "end": { "column": 26, "line": 2 }, "start": { "column": 25, "line": 2 } }, "name": "C", "range": [ 36, 37 ], "type": "Identifier" } }, "loc": { "end": { "column": 26, "line": 2 }, "start": { "column": 20, "line": 2 } }, "name": "c", "range": [ 31, 37 ], "type": "TSTypeParameter" } } } ], "loc": { "end": { "column": 1, "line": 3 }, "start": { "column": 0, "line": 1 } }, "range": [ 0, 48 ], "sourceType": "script", "tokens": [ { "loc": { "end": { "column": 4, "line": 1 }, "start": { "column": 0, "line": 1 } }, "range": [ 0, 4 ], "type": "Identifier", "value": "type" }, { "loc": { "end": { "column": 6, "line": 1 }, "start": { "column": 5, "line": 1 } }, "range": [ 5, 6 ], "type": "Identifier", "value": "C" }, { "loc": { "end": { "column": 8, "line": 1 }, "start": { "column": 7, "line": 1 } }, "range": [ 7, 8 ], "type": "Punctuator", "value": "=" }, { "loc": { "end": { "column": 10, "line": 1 }, "start": { "column": 9, "line": 1 } }, "range": [ 9, 10 ], "type": "Punctuator", "value": "{" }, { "loc": { "end": { "column": 5, "line": 2 }, "start": { "column": 4, "line": 2 } }, "range": [ 15, 16 ], "type": "Punctuator", "value": "[" }, { "loc": { "end": { "column": 21, "line": 2 }, "start": { "column": 20, "line": 2 } }, "range": [ 31, 32 ], "type": "Identifier", "value": "c" }, { "loc": { "end": { "column": 24, "line": 2 }, "start": { "column": 22, "line": 2 } }, "range": [ 33, 35 ], "type": "Keyword", "value": "in" }, { "loc": { "end": { "column": 26, "line": 2 }, "start": { "column": 25, "line": 2 } }, "range": [ 36, 37 ], "type": "Identifier", "value": "C" }, { "loc": { "end": { "column": 27, "line": 2 }, "start": { "column": 26, "line": 2 } }, "range": [ 37, 38 ], "type": "Punctuator", "value": "]" }, { "loc": { "end": { "column": 28, "line": 2 }, "start": { "column": 27, "line": 2 } }, "range": [ 38, 39 ], "type": "Punctuator", "value": ":" }, { "loc": { "end": { "column": 35, "line": 2 }, "start": { "column": 29, "line": 2 } }, "range": [ 40, 46 ], "type": "Identifier", "value": "string" }, { "loc": { "end": { "column": 1, "line": 3 }, "start": { "column": 0, "line": 3 } }, "range": [ 47, 48 ], "type": "Punctuator", "value": "}" } ], "type": "Program" }The text was updated successfully, but these errors were encountered: