TSMappedType - TSTypeParameter.name issue

[JamesHenry]

Considering the following source:

type C = {
    [/* commentC */ c in C]: string
}

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 an Identifier - 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 the TSTypeParameter.name, which used to be a node, and no longer is.

Without this being reinstated to an Identifier node, 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"
}

[armano2]

it has to be identifier, i missed this case, we can change that.

[ikatyang]

Identifier looks more reasonable to me, but it does not look like a TSTypeParameter-only issue but for all Identifier/TypeParameter:

// 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:

• TSTypeParameter was introduced in TypeScript parser babel/babylon#523, which followed the existing format from Flow.
• (Flow)TypeParameter was introduced in Support defaults in Flow's type parameter declarations babel/babylon#25, which was modified from Identifier
  • Identifier { name: string, typeAnnotation?: TypeAnnotation }

[armano2]

@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.

estree-13.5.2...estree-5.3

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

[armano2]

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

[JamesHenry]

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

[armano2]

31 tests is failing :(