Skip to content

Commit

Permalink
feat(parser)!: use BindingIdentifier for namespace declaration na…
Browse files Browse the repository at this point in the history
…mes (#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.
  • Loading branch information
DonIsaac committed Oct 8, 2024
1 parent 87f700f commit 01b878e
Show file tree
Hide file tree
Showing 10 changed files with 77 additions and 41 deletions.
2 changes: 1 addition & 1 deletion crates/oxc_ast/src/ast/ts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
24 changes: 12 additions & 12 deletions crates/oxc_ast/src/generated/assert_layouts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1091,19 +1091,19 @@ const _: () = {
assert!(size_of::<TSTypePredicateName>() == 16usize);
assert!(align_of::<TSTypePredicateName>() == 8usize);

assert!(size_of::<TSModuleDeclaration>() == 64usize);
assert!(size_of::<TSModuleDeclaration>() == 72usize);
assert!(align_of::<TSModuleDeclaration>() == 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::<TSModuleDeclarationKind>() == 1usize);
assert!(align_of::<TSModuleDeclarationKind>() == 1usize);

assert!(size_of::<TSModuleDeclarationName>() == 32usize);
assert!(size_of::<TSModuleDeclarationName>() == 40usize);
assert!(align_of::<TSModuleDeclarationName>() == 8usize);

assert!(size_of::<TSModuleDeclarationBody>() == 16usize);
Expand Down Expand Up @@ -2628,19 +2628,19 @@ const _: () = {
assert!(size_of::<TSTypePredicateName>() == 12usize);
assert!(align_of::<TSTypePredicateName>() == 4usize);

assert!(size_of::<TSModuleDeclaration>() == 44usize);
assert!(size_of::<TSModuleDeclaration>() == 48usize);
assert!(align_of::<TSModuleDeclaration>() == 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::<TSModuleDeclarationKind>() == 1usize);
assert!(align_of::<TSModuleDeclarationKind>() == 1usize);

assert!(size_of::<TSModuleDeclarationName>() == 20usize);
assert!(size_of::<TSModuleDeclarationName>() == 24usize);
assert!(align_of::<TSModuleDeclarationName>() == 4usize);

assert!(size_of::<TSModuleDeclarationBody>() == 8usize);
Expand Down
12 changes: 6 additions & 6 deletions crates/oxc_ast/src/generated/ast_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11423,27 +11423,27 @@ 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<A>(
pub fn ts_module_declaration_name_binding_identifier<A>(
self,
span: Span,
name: A,
) -> TSModuleDeclarationName<'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<T>(
pub fn ts_module_declaration_name_from_binding_identifier<T>(
self,
inner: T,
) -> TSModuleDeclarationName<'a>
where
T: IntoIn<'a, IdentifierName<'a>>,
T: IntoIn<'a, BindingIdentifier<'a>>,
{
TSModuleDeclarationName::Identifier(inner.into_in(self.allocator))
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_ast/src/generated/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_ast/src/generated/visit_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_parser/src/ts/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 8 additions & 2 deletions crates/oxc_semantic/src/binder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion crates/oxc_transformer/src/typescript/namespace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ impl<'a, 'ctx> TypeScriptNamespace<'a, 'ctx> {

let mut names: FxHashSet<Atom<'a>> = FxHashSet::default();

let TSModuleDeclarationName::Identifier(IdentifierName { name: real_name, .. }) = decl.id
let TSModuleDeclarationName::Identifier(BindingIdentifier { name: real_name, .. }) =
decl.id
else {
return None;
};
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_traverse/src/generated/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
59 changes: 44 additions & 15 deletions tasks/coverage/snapshots/parser_typescript.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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]
1declare 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]
Expand Down

0 comments on commit 01b878e

Please sign in to comment.