8000 Bug: incorrect AST produced for nested namespaces · Issue #4966 · typescript-eslint/typescript-eslint · GitHub
[go: up one dir, main page]
Skip to content

Bug: incorrect AST produced for nested namespaces #4966

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
bradzacher opened this issue May 12, 2022 · 3 comments
Closed

Bug: incorrect AST produced for nested namespaces #4966

bradzacher opened this issue May 12, 2022 · 3 comments
Labels
accepting prs Go ahead, send a pull request that resolves this issue AST PRs and Issues about the AST structure breaking change This change will require a new major version to be released bug Something isn't working
Milestone
6.0.0

Comments

@bradzacher
Copy link
Member

This was forgotten when we worked around #2573

For namespace foo { namespace bar {} } we produce the following AST:
TSModuleDeclaration[id = foo] > TSModuleBlock > TSModuleDeclaration[id = bar] > TSModuleBlock

For namespace foo.bar {} produces the following AST:
TSModuleDeclaration[id = foo] > TSModuleDeclaration[id = bar] > TSModuleBlock

The AST we produce is driven by the AST TS produces - they emit the nested module declaration, so our code does as well.
Ultimately however is pretty incorrect and does not correctly represent the code as written.

Instead we should emit an EntityName, thus allowing TSQualifiedName in the .id:
TSModuleDeclaration[id = foo.bar] > TSModuleBlock
@bradzacher bradzacher added bug Something isn't working breaking change This change will require a new major version to be released AST PRs and Issues about the AST structure accepting prs Go ahead, send a pull request that resolves this issue labels May 12, 2022
@bradzacher bradzacher added this to the 6.0.0 milestone May 12, 2022
@armano2
Copy link
Collaborator
armano2 commented May 14, 2022

i agree with this change, but this is not a bug 😸

@bradzacher
Copy link
Member Author

It is a bug because we never intended this to be the AST. Our types were wrong for the node because we didn't know TS emitted this weird AST!

@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Dec 24, 2022
Merged
3 tasks
@bradzacher bradzacher mentioned this issue Feb 8, 2023
Closed
4 tasks
@JoshuaKGoldberg
Copy link
Member

This has been merged into the v6 branch and will be available in that major version.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
accepting prs < 46DE tool-tip id="tooltip-beca225b-f62c-4434-afd2-501d9617a020" for="label-9ab5fd" popover="manual" data-direction="s" data-type="description" data-view-component="true" class="sr-only position-absolute">Go ahead, send a pull request that resolves this issue AST PRs and Issues about the AST structure breaking change This change will require a new major version to be released bug Something isn't working
Projects
None yet
Milestone
6.0.0
Development

No branches or pull requests
3 participants
@armano2 @JoshuaKGoldberg @bradzacher
0