From 01b878ecdb2c9b842eb185d4c2bdebb1940ad50d Mon Sep 17 00:00:00 2001 From: DonIsaac <22823424+DonIsaac@users.noreply.github.com> Date: Tue, 8 Oct 2024 08:39:30 +0000 Subject: [PATCH] feat(parser)!: use `BindingIdentifier` for `namespace` declaration names (#6003) Use `BindingIdentifier` instead of `IdentifierName` so that AST visitors can access the bound symbol id for the namespace's name. This makes namespace consistent with other named declarations, such as `Class`, `Function`, and `TSInterfaceDeclaration`. We should consider modifying `TSModuleDeclarationName::StringLiteral(...)` to also store a `symbol_id`, since one gets bound in semantic for it as well. --- crates/oxc_ast/src/ast/ts.rs | 2 +- .../oxc_ast/src/generated/assert_layouts.rs | 24 ++++---- crates/oxc_ast/src/generated/ast_builder.rs | 12 ++-- crates/oxc_ast/src/generated/visit.rs | 2 +- crates/oxc_ast/src/generated/visit_mut.rs | 2 +- crates/oxc_parser/src/ts/statement.rs | 2 +- crates/oxc_semantic/src/binder.rs | 10 +++- .../src/typescript/namespace.rs | 3 +- crates/oxc_traverse/src/generated/walk.rs | 2 +- .../coverage/snapshots/parser_typescript.snap | 59 ++++++++++++++----- 10 files changed, 77 insertions(+), 41 deletions(-) diff --git a/crates/oxc_ast/src/ast/ts.rs b/crates/oxc_ast/src/ast/ts.rs index ae3327211f08a..bc2d87054a807 100644 --- a/crates/oxc_ast/src/ast/ts.rs +++ b/crates/oxc_ast/src/ast/ts.rs @@ -1391,7 +1391,7 @@ pub enum TSModuleDeclarationKind { #[cfg_attr(feature = "serialize", derive(Serialize, Tsify))] #[serde(untagged)] pub enum TSModuleDeclarationName<'a> { - Identifier(IdentifierName<'a>) = 0, + Identifier(BindingIdentifier<'a>) = 0, StringLiteral(StringLiteral<'a>) = 1, } diff --git a/crates/oxc_ast/src/generated/assert_layouts.rs b/crates/oxc_ast/src/generated/assert_layouts.rs index f5a4a6d88b0ec..520a82908ae44 100644 --- a/crates/oxc_ast/src/generated/assert_layouts.rs +++ b/crates/oxc_ast/src/generated/assert_layouts.rs @@ -1091,19 +1091,19 @@ const _: () = { assert!(size_of::() == 16usize); assert!(align_of::() == 8usize); - assert!(size_of::() == 64usize); + assert!(size_of::() == 72usize); assert!(align_of::() == 8usize); assert!(offset_of!(TSModuleDeclaration, span) == 0usize); assert!(offset_of!(TSModuleDeclaration, id) == 8usize); - assert!(offset_of!(TSModuleDeclaration, body) == 40usize); - assert!(offset_of!(TSModuleDeclaration, kind) == 56usize); - assert!(offset_of!(TSModuleDeclaration, declare) == 57usize); - assert!(offset_of!(TSModuleDeclaration, scope_id) == 60usize); + assert!(offset_of!(TSModuleDeclaration, body) == 48usize); + assert!(offset_of!(TSModuleDeclaration, kind) == 64usize); + assert!(offset_of!(TSModuleDeclaration, declare) == 65usize); + assert!(offset_of!(TSModuleDeclaration, scope_id) == 68usize); assert!(size_of::() == 1usize); assert!(align_of::() == 1usize); - assert!(size_of::() == 32usize); + assert!(size_of::() == 40usize); assert!(align_of::() == 8usize); assert!(size_of::() == 16usize); @@ -2628,19 +2628,19 @@ const _: () = { assert!(size_of::() == 12usize); assert!(align_of::() == 4usize); - assert!(size_of::() == 44usize); + assert!(size_of::() == 48usize); assert!(align_of::() == 4usize); assert!(offset_of!(TSModuleDeclaration, span) == 0usize); assert!(offset_of!(TSModuleDeclaration, id) == 8usize); - assert!(offset_of!(TSModuleDeclaration, body) == 28usize); - assert!(offset_of!(TSModuleDeclaration, kind) == 36usize); - assert!(offset_of!(TSModuleDeclaration, declare) == 37usize); - assert!(offset_of!(TSModuleDeclaration, scope_id) == 40usize); + assert!(offset_of!(TSModuleDeclaration, body) == 32usize); + assert!(offset_of!(TSModuleDeclaration, kind) == 40usize); + assert!(offset_of!(TSModuleDeclaration, declare) == 41usize); + assert!(offset_of!(TSModuleDeclaration, scope_id) == 44usize); assert!(size_of::() == 1usize); assert!(align_of::() == 1usize); - assert!(size_of::() == 20usize); + assert!(size_of::() == 24usize); assert!(align_of::() == 4usize); assert!(size_of::() == 8usize); diff --git a/crates/oxc_ast/src/generated/ast_builder.rs b/crates/oxc_ast/src/generated/ast_builder.rs index 41f53072c2cc6..e0fe52d16d83b 100644 --- a/crates/oxc_ast/src/generated/ast_builder.rs +++ b/crates/oxc_ast/src/generated/ast_builder.rs @@ -11423,9 +11423,9 @@ impl<'a> AstBuilder<'a> { /// /// ## Parameters /// - span: The [`Span`] covering this node - /// - name + /// - name: The identifier name being bound. #[inline] - pub fn ts_module_declaration_name_identifier_name( + pub fn ts_module_declaration_name_binding_identifier( self, span: Span, name: A, @@ -11433,17 +11433,17 @@ impl<'a> AstBuilder<'a> { where A: IntoIn<'a, Atom<'a>>, { - TSModuleDeclarationName::Identifier(self.identifier_name(span, name)) + TSModuleDeclarationName::Identifier(self.binding_identifier(span, name)) } - /// Convert a [`IdentifierName`] into a [`TSModuleDeclarationName::Identifier`] + /// Convert a [`BindingIdentifier`] into a [`TSModuleDeclarationName::Identifier`] #[inline] - pub fn ts_module_declaration_name_from_identifier_name( + pub fn ts_module_declaration_name_from_binding_identifier( self, inner: T, ) -> TSModuleDeclarationName<'a> where - T: IntoIn<'a, IdentifierName<'a>>, + T: IntoIn<'a, BindingIdentifier<'a>>, { TSModuleDeclarationName::Identifier(inner.into_in(self.allocator)) } diff --git a/crates/oxc_ast/src/generated/visit.rs b/crates/oxc_ast/src/generated/visit.rs index 274c2765f238c..a42a3dacb807a 100644 --- a/crates/oxc_ast/src/generated/visit.rs +++ b/crates/oxc_ast/src/generated/visit.rs @@ -3888,7 +3888,7 @@ pub mod walk { it: &TSModuleDeclarationName<'a>, ) { match it { - TSModuleDeclarationName::Identifier(it) => visitor.visit_identifier_name(it), + TSModuleDeclarationName::Identifier(it) => visitor.visit_binding_identifier(it), TSModuleDeclarationName::StringLiteral(it) => visitor.visit_string_literal(it), } } diff --git a/crates/oxc_ast/src/generated/visit_mut.rs b/crates/oxc_ast/src/generated/visit_mut.rs index eb5460f88fbc4..fa9eda505882c 100644 --- a/crates/oxc_ast/src/generated/visit_mut.rs +++ b/crates/oxc_ast/src/generated/visit_mut.rs @@ -4109,7 +4109,7 @@ pub mod walk_mut { it: &mut TSModuleDeclarationName<'a>, ) { match it { - TSModuleDeclarationName::Identifier(it) => visitor.visit_identifier_name(it), + TSModuleDeclarationName::Identifier(it) => visitor.visit_binding_identifier(it), TSModuleDeclarationName::StringLiteral(it) => visitor.visit_string_literal(it), } } diff --git a/crates/oxc_parser/src/ts/statement.rs b/crates/oxc_parser/src/ts/statement.rs index 9d7039e899134..ecf6a898ad4f5 100644 --- a/crates/oxc_parser/src/ts/statement.rs +++ b/crates/oxc_parser/src/ts/statement.rs @@ -300,7 +300,7 @@ impl<'a> ParserImpl<'a> { ); let id = match self.cur_kind() { Kind::Str => self.parse_literal_string().map(TSModuleDeclarationName::StringLiteral), - _ => self.parse_identifier_name().map(TSModuleDeclarationName::Identifier), + _ => self.parse_binding_identifier().map(TSModuleDeclarationName::Identifier), }?; let body = if self.eat(Kind::Dot) { diff --git a/crates/oxc_semantic/src/binder.rs b/crates/oxc_semantic/src/binder.rs index c43a45767e832..4d48f273f72af 100644 --- a/crates/oxc_semantic/src/binder.rs +++ b/crates/oxc_semantic/src/binder.rs @@ -416,13 +416,19 @@ impl<'a> Binder<'a> for TSModuleDeclaration<'a> { // At declaration time a module has no value declaration it is only when a value declaration // is made inside a the scope of a module that the symbol is modified let ambient = if self.declare { SymbolFlags::Ambient } else { SymbolFlags::None }; - // FIXME: insert symbol_id into TSModuleDeclarationName AST node - let _symbol_id = builder.declare_symbol( + let symbol_id = builder.declare_symbol( self.id.span(), self.id.name().as_str(), SymbolFlags::NameSpaceModule | ambient, SymbolFlags::None, ); + + // do not bind `global` for `declare global { ... }` + if !self.kind.is_global() { + if let TSModuleDeclarationName::Identifier(id) = &self.id { + id.symbol_id.set(Some(symbol_id)); + } + } } } diff --git a/crates/oxc_transformer/src/typescript/namespace.rs b/crates/oxc_transformer/src/typescript/namespace.rs index 7d21a173e06e2..f26b4c7f60236 100644 --- a/crates/oxc_transformer/src/typescript/namespace.rs +++ b/crates/oxc_transformer/src/typescript/namespace.rs @@ -153,7 +153,8 @@ impl<'a, 'ctx> TypeScriptNamespace<'a, 'ctx> { let mut names: FxHashSet> = FxHashSet::default(); - let TSModuleDeclarationName::Identifier(IdentifierName { name: real_name, .. }) = decl.id + let TSModuleDeclarationName::Identifier(BindingIdentifier { name: real_name, .. }) = + decl.id else { return None; }; diff --git a/crates/oxc_traverse/src/generated/walk.rs b/crates/oxc_traverse/src/generated/walk.rs index 775af069fbc36..61c6a408cca98 100644 --- a/crates/oxc_traverse/src/generated/walk.rs +++ b/crates/oxc_traverse/src/generated/walk.rs @@ -4987,7 +4987,7 @@ pub(crate) unsafe fn walk_ts_module_declaration_name<'a, Tr: Traverse<'a>>( traverser.enter_ts_module_declaration_name(&mut *node, ctx); match &mut *node { TSModuleDeclarationName::Identifier(node) => { - walk_identifier_name(traverser, node as *mut _, ctx) + walk_binding_identifier(traverser, node as *mut _, ctx) } TSModuleDeclarationName::StringLiteral(node) => { walk_string_literal(traverser, node as *mut _, ctx) diff --git a/tasks/coverage/snapshots/parser_typescript.snap b/tasks/coverage/snapshots/parser_typescript.snap index dc5ed72ff1b9f..7d54ace1ace7c 100644 --- a/tasks/coverage/snapshots/parser_typescript.snap +++ b/tasks/coverage/snapshots/parser_typescript.snap @@ -3,7 +3,7 @@ commit: a709f989 parser_typescript Summary: AST Parsed : 6470/6479 (99.86%) Positive Passed: 6459/6479 (99.69%) -Negative Passed: 1234/5715 (21.59%) +Negative Passed: 1235/5715 (21.61%) Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration10.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration11.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ClassDeclaration13.ts @@ -1906,7 +1906,6 @@ Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/statics.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/staticsNotInScopeInClodule.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/strictFunctionTypesErrors.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/strictModeInConstructor.ts -Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/strictModeReservedWordInModuleDeclaration.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/strictNullEmptyDestructuring.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/strictNullNotNullIndexTypeNoLib.ts Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/strictOptionalProperties3.ts @@ -12021,6 +12020,37 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/salsa/private · ────── ╰──── + × The keyword 'public' is reserved + ╭─[typescript/tests/cases/compiler/strictModeReservedWordInModuleDeclaration.ts:2:8] + 1 │ "use strict" + 2 │ module public { } + · ────── + 3 │ module private { } + ╰──── + + × The keyword 'private' is reserved + ╭─[typescript/tests/cases/compiler/strictModeReservedWordInModuleDeclaration.ts:3:8] + 2 │ module public { } + 3 │ module private { } + · ─────── + 4 │ module public.whatever { + ╰──── + + × The keyword 'public' is reserved + ╭─[typescript/tests/cases/compiler/strictModeReservedWordInModuleDeclaration.ts:4:8] + 3 │ module private { } + 4 │ module public.whatever { + · ────── + 5 │ } + ╰──── + + × The keyword 'private' is reserved + ╭─[typescript/tests/cases/compiler/strictModeReservedWordInModuleDeclaration.ts:6:8] + 5 │ } + 6 │ module private.public.foo { } + · ─────── + ╰──── + × The keyword 'package' is reserved ╭─[typescript/tests/cases/compiler/strictModeWordInImportDeclaration.ts:2:13] 1 │ "use strict" @@ -13041,21 +13071,20 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/salsa/private 3 │ ╰──── - × Expected a semicolon or an implicit semicolon after a statement, but found none - ╭─[typescript/tests/cases/conformance/ambient/ambientModuleDeclarationWithReservedIdentifierInDottedPath.ts:11:8] - 10 │ - 11 │ declare module debugger {} // still an error - · ▲ - ╰──── - help: Try insert a semicolon here + × Unexpected token + ╭─[typescript/tests/cases/conformance/ambient/ambientModuleDeclarationWithReservedIdentifierInDottedPath.ts:3:23] + 2 │ + 3 │ declare module chrome.debugger { + · ──────── + 4 │ declare var tabId: number; + ╰──── - × Expected a semicolon or an implicit semicolon after a statement, but found none - ╭─[typescript/tests/cases/conformance/ambient/ambientModuleDeclarationWithReservedIdentifierInDottedPath2.ts:9:8] - 8 │ - 9 │ declare namespace debugger {} // still an error - · ▲ + × Unexpected token + ╭─[typescript/tests/cases/conformance/ambient/ambientModuleDeclarationWithReservedIdentifierInDottedPath2.ts:1:26] + 1 │ declare namespace chrome.debugger { + · ──────── + 2 │ declare var tabId: number; ╰──── - help: Try insert a semicolon here × Cannot use `await` as an identifier in an async context ╭─[typescript/tests/cases/conformance/async/es2017/asyncArrowFunction/asyncArrowFunction5_es2017.ts:1:18]