From 0d7ebf5729acc7ac04477217a48bee4dfad9c03c Mon Sep 17 00:00:00 2001 From: MichaelMitchell-at <=> Date: Sun, 7 Jul 2024 03:44:44 -0400 Subject: [PATCH] fix(isolated_declarations): Emit computed properties when they are well known symbols --- crates/oxc_isolated_declarations/src/class.rs | 11 +- .../src/global_symbol_binding_tracker.rs | 164 ++++++++++++++++++ crates/oxc_isolated_declarations/src/lib.rs | 8 + crates/oxc_isolated_declarations/src/types.rs | 2 +- .../tests/fixtures/well-known-symbols.ts | 31 ++++ .../tests/snapshots/well-known-symbols.snap | 59 +++++++ tasks/coverage/transpile.snap | 44 +---- 7 files changed, 278 insertions(+), 41 deletions(-) create mode 100644 crates/oxc_isolated_declarations/src/global_symbol_binding_tracker.rs create mode 100644 crates/oxc_isolated_declarations/tests/fixtures/well-known-symbols.ts create mode 100644 crates/oxc_isolated_declarations/tests/snapshots/well-known-symbols.snap diff --git a/crates/oxc_isolated_declarations/src/class.rs b/crates/oxc_isolated_declarations/src/class.rs index d7f6862c6c4d81..aed96f705c1853 100644 --- a/crates/oxc_isolated_declarations/src/class.rs +++ b/crates/oxc_isolated_declarations/src/class.rs @@ -32,8 +32,15 @@ impl<'a> IsolatedDeclarations<'a> { pub fn report_property_key(&self, key: &PropertyKey<'a>, computed: bool) -> bool { if computed && !self.is_literal_key(key) { - self.error(computed_property_name(key.span())); - true + if self + .global_symbol_binding_tracker + .does_computed_property_reference_well_known_symbol(key) + { + false + } else { + self.error(computed_property_name(key.span())); + true + } } else { false } diff --git a/crates/oxc_isolated_declarations/src/global_symbol_binding_tracker.rs b/crates/oxc_isolated_declarations/src/global_symbol_binding_tracker.rs new file mode 100644 index 00000000000000..8f1d91172fe999 --- /dev/null +++ b/crates/oxc_isolated_declarations/src/global_symbol_binding_tracker.rs @@ -0,0 +1,164 @@ +use std::cell::Cell; + +#[allow(clippy::wildcard_imports)] +use oxc_ast::ast::*; +#[allow(clippy::wildcard_imports)] +use oxc_ast::{visit::walk::*, Visit}; +use oxc_span::{Atom, GetSpan, Span}; +use oxc_syntax::scope::{ScopeFlags, ScopeId}; +use rustc_hash::FxHashSet; + +/// Linear tree of declaration scopes. +pub struct GlobalSymbolBindingTracker { + depth: u8, + symbol_binding_depth: Option, + global_this_binding_depth: Option, + computed_properties_using_non_global_symbol: FxHashSet, + computed_properties_using_non_global_global_this: FxHashSet, +} + +impl GlobalSymbolBindingTracker { + pub fn new() -> Self { + Self { + depth: 0, + symbol_binding_depth: None, + global_this_binding_depth: None, + computed_properties_using_non_global_symbol: FxHashSet::default(), + computed_properties_using_non_global_global_this: FxHashSet::default(), + } + } + + fn does_computed_property_reference_non_global_symbol(&self, key: &PropertyKey) -> bool { + self.computed_properties_using_non_global_symbol.contains(&key.span()) + } + + fn does_computed_property_reference_non_global_global_this(&self, key: &PropertyKey) -> bool { + self.computed_properties_using_non_global_global_this.contains(&key.span()) + } + + pub fn does_computed_property_reference_well_known_symbol(&self, key: &PropertyKey) -> bool { + if let PropertyKey::StaticMemberExpression(expr) = key { + if let Expression::Identifier(identifier) = &expr.object { + identifier.name == "Symbol" + && !self.does_computed_property_reference_non_global_symbol(key) + } else if let Expression::StaticMemberExpression(static_member) = &expr.object { + if let Expression::Identifier(identifier) = &static_member.object { + identifier.name == "globalThis" + && static_member.property.name == "Symbol" + && !self.does_computed_property_reference_non_global_global_this(key) + } else { + false + } + } else { + false + } + } else { + false + } + } + + fn handle_name_binding(&mut self, name: &Atom) { + match name.as_str() { + "Symbol" if self.symbol_binding_depth.is_none() => { + self.symbol_binding_depth = Some(self.depth); + } + "globalThis" if self.global_this_binding_depth.is_none() => { + self.global_this_binding_depth = Some(self.depth); + } + _ => {} + } + } +} + +impl<'a> Visit<'a> for GlobalSymbolBindingTracker { + fn enter_scope(&mut self, _: ScopeFlags, _: &Cell>) { + self.depth += 1; + } + + fn leave_scope(&mut self) { + if self.symbol_binding_depth == Some(self.depth) { + self.symbol_binding_depth = None; + } + if self.global_this_binding_depth == Some(self.depth) { + self.global_this_binding_depth = None; + } + self.depth -= 1; + } + + fn visit_ts_type(&mut self, _: &TSType<'a>) { + // Optimization: we don't need to traverse down into types. + } + + fn visit_binding_pattern(&mut self, pattern: &BindingPattern<'a>) { + if let BindingPatternKind::BindingIdentifier(ident) = &pattern.kind { + self.handle_name_binding(&ident.name); + } + walk_binding_pattern(self, pattern); + } + + fn visit_declaration(&mut self, declaration: &Declaration<'a>) { + match declaration { + Declaration::VariableDeclaration(_) | Declaration::UsingDeclaration(_) => { + // handled in BindingPattern + } + Declaration::FunctionDeclaration(decl) => { + if let Some(id) = decl.id.as_ref() { + self.handle_name_binding(&id.name); + } + } + Declaration::ClassDeclaration(decl) => { + if let Some(id) = decl.id.as_ref() { + self.handle_name_binding(&id.name); + } + } + Declaration::TSModuleDeclaration(decl) => { + if let TSModuleDeclarationName::Identifier(ident) = &decl.id { + self.handle_name_binding(&ident.name); + } + } + Declaration::TSImportEqualsDeclaration(decl) => { + self.handle_name_binding(&decl.id.name); + return; + } + Declaration::TSEnumDeclaration(decl) => { + if !decl.r#const { + self.handle_name_binding(&decl.id.name); + } + return; + } + Declaration::TSTypeAliasDeclaration(_) | Declaration::TSInterfaceDeclaration(_) => { + return + } + } + walk_declaration(self, declaration); + } + + fn visit_object_property(&mut self, prop: &ObjectProperty<'a>) { + if prop.computed { + if let PropertyKey::StaticMemberExpression(expr) = &prop.key { + if self.symbol_binding_depth.is_some() { + if let Expression::Identifier(identifier) = &expr.object { + if identifier.name == "Symbol" { + self.computed_properties_using_non_global_symbol.insert(expr.span); + } + } + } + + if self.global_this_binding_depth.is_some() { + if let Expression::StaticMemberExpression(static_member) = &expr.object { + if let Expression::Identifier(identifier) = &static_member.object { + if identifier.name == "globalThis" + && static_member.property.name == "Symbol" + { + self.computed_properties_using_non_global_global_this + .insert(expr.span); + } + } + } + } + } + } + + walk_object_property(self, prop); + } +} diff --git a/crates/oxc_isolated_declarations/src/lib.rs b/crates/oxc_isolated_declarations/src/lib.rs index ea3830adaed83a..0d2b152e985c92 100644 --- a/crates/oxc_isolated_declarations/src/lib.rs +++ b/crates/oxc_isolated_declarations/src/lib.rs @@ -11,6 +11,7 @@ mod diagnostics; mod r#enum; mod formal_parameter_binding_pattern; mod function; +mod global_symbol_binding_tracker; mod inferrer; mod literal; mod module; @@ -28,6 +29,7 @@ use oxc_diagnostics::OxcDiagnostic; use oxc_span::{Atom, SourceType, SPAN}; use rustc_hash::FxHashSet; +use crate::global_symbol_binding_tracker::GlobalSymbolBindingTracker; use crate::scope::ScopeTree; pub struct IsolatedDeclarationsReturn<'a> { @@ -39,6 +41,7 @@ pub struct IsolatedDeclarations<'a> { ast: AstBuilder<'a>, // state scope: ScopeTree<'a>, + global_symbol_binding_tracker: GlobalSymbolBindingTracker, errors: RefCell>, } @@ -47,6 +50,7 @@ impl<'a> IsolatedDeclarations<'a> { Self { ast: AstBuilder::new(allocator), scope: ScopeTree::new(allocator), + global_symbol_binding_tracker: GlobalSymbolBindingTracker::new(), errors: RefCell::new(vec![]), } } @@ -77,6 +81,10 @@ impl<'a> IsolatedDeclarations<'a> { &mut self, program: &Program<'a>, ) -> oxc_allocator::Vec<'a, Statement<'a>> { + // Collect information about global Symbol usage within computed + // properties before performing any transformations. + self.global_symbol_binding_tracker.visit_program(program); + let has_import_or_export = program.body.iter().any(|stmt| { matches!( stmt, diff --git a/crates/oxc_isolated_declarations/src/types.rs b/crates/oxc_isolated_declarations/src/types.rs index 5647c743907ad8..a207c6d9b7187f 100644 --- a/crates/oxc_isolated_declarations/src/types.rs +++ b/crates/oxc_isolated_declarations/src/types.rs @@ -120,7 +120,7 @@ impl<'a> IsolatedDeclarations<'a> { let property_signature = self.ast.ts_signature_property_signature( object.span, - false, + object.computed, false, is_const, self.ast.copy(&object.key), diff --git a/crates/oxc_isolated_declarations/tests/fixtures/well-known-symbols.ts b/crates/oxc_isolated_declarations/tests/fixtures/well-known-symbols.ts new file mode 100644 index 00000000000000..d5936035d6439c --- /dev/null +++ b/crates/oxc_isolated_declarations/tests/fixtures/well-known-symbols.ts @@ -0,0 +1,31 @@ +// Correct +export const foo = { + [Symbol.iterator]: (): void => {}, + [Symbol.asyncIterator]: async (): Promise => {}, + [globalThis.Symbol.iterator]: (): void => {}, +} + +export abstract class Foo { + [Symbol.iterator](): void {} + async [Symbol.asyncIterator](): Promise {} + [globalThis.Symbol.iterator](): void {} +} + +// Incorrect + +export namespace Foo { + const Symbol = {}; + const globalThis = {}; + + export const foo = { + [Symbol.iterator]: (): void => {}, + [globalThis.Symbol.iterator]: (): void => {}, + } +} + +export function bar(Symbol: {}, globalThis: {}) { + return { + [Symbol.iterator]: (): void => {}, + [globalThis.Symbol.iterator]: (): void => {}, + } +} \ No newline at end of file diff --git a/crates/oxc_isolated_declarations/tests/snapshots/well-known-symbols.snap b/crates/oxc_isolated_declarations/tests/snapshots/well-known-symbols.snap new file mode 100644 index 00000000000000..138a9ab27dd8c0 --- /dev/null +++ b/crates/oxc_isolated_declarations/tests/snapshots/well-known-symbols.snap @@ -0,0 +1,59 @@ +--- +source: crates/oxc_isolated_declarations/tests/mod.rs +input_file: crates/oxc_isolated_declarations/tests/fixtures/well-known-symbols.ts +--- +==================== .D.TS ==================== + +export declare const foo: { + [Symbol.iterator]: () => void; + [Symbol.asyncIterator]: () => Promise; + [globalThis.Symbol.iterator]: () => void; +}; +export declare abstract class Foo { + [Symbol.iterator](): void; + [Symbol.asyncIterator](): Promise; + [globalThis.Symbol.iterator](): void; +} +export declare namespace Foo { + export const foo: {}; +} +export declare function bar(Symbol: {}, globalThis: {}): {}; + + +==================== Errors ==================== + + x TS9038: Computed property names on class or object literals cannot be + | inferred with --isolatedDeclarations. + ,-[21:6] + 20 | export const foo = { + 21 | [Symbol.iterator]: (): void => {}, + : ^^^^^^^^^^^^^^^ + 22 | [globalThis.Symbol.iterator]: (): void => {}, + `---- + + x TS9038: Computed property names on class or object literals cannot be + | inferred with --isolatedDeclarations. + ,-[22:6] + 21 | [Symbol.iterator]: (): void => {}, + 22 | [globalThis.Symbol.iterator]: (): void => {}, + : ^^^^^^^^^^^^^^^^^^^^^^^^^^ + 23 | } + `---- + + x TS9038: Computed property names on class or object literals cannot be + | inferred with --isolatedDeclarations. + ,-[28:6] + 27 | return { + 28 | [Symbol.iterator]: (): void => {}, + : ^^^^^^^^^^^^^^^ + 29 | [globalThis.Symbol.iterator]: (): void => {}, + `---- + + x TS9038: Computed property names on class or object literals cannot be + | inferred with --isolatedDeclarations. + ,-[29:6] + 28 | [Symbol.iterator]: (): void => {}, + 29 | [globalThis.Symbol.iterator]: (): void => {}, + : ^^^^^^^^^^^^^^^^^^^^^^^^^^ + 30 | } + `---- diff --git a/tasks/coverage/transpile.snap b/tasks/coverage/transpile.snap index 0b5ef5fba7757d..bf841c63ea71ab 100644 --- a/tasks/coverage/transpile.snap +++ b/tasks/coverage/transpile.snap @@ -98,12 +98,16 @@ export interface B { [Math.random() > 0.5 ? "f1" : "f2"]: number; } export declare class C { + [Symbol.iterator]: number; + [globalThis.Symbol.toStringTag]: number; [1]: number; ["2"]: number; } export declare const D: { - 1: number; - "2": number; + [Symbol.iterator]: number; + [globalThis.Symbol.toStringTag]: number; + [1]: number; + ["2"]: number; }; export {}; x TS9010: Variable must have an explicit type annotation with @@ -214,24 +218,6 @@ export {}; 39 | [Symbol.iterator]: number = 1; `---- - x TS9038: Computed property names on class or object literals cannot be - | inferred with --isolatedDeclarations. - ,-[declarationComputedPropertyNames.d.ts:39:6] - 38 | [presentNs.a]: number = 1; - 39 | [Symbol.iterator]: number = 1; - : ^^^^^^^^^^^^^^^ - 40 | [globalThis.Symbol.toStringTag]: number = 1; - `---- - - x TS9038: Computed property names on class or object literals cannot be - | inferred with --isolatedDeclarations. - ,-[declarationComputedPropertyNames.d.ts:40:6] - 39 | [Symbol.iterator]: number = 1; - 40 | [globalThis.Symbol.toStringTag]: number = 1; - : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - 41 | [(globalThis.Symbol).unscopables]: number = 1; - `---- - x TS9038: Computed property names on class or object literals cannot be | inferred with --isolatedDeclarations. ,-[declarationComputedPropertyNames.d.ts:41:6] @@ -295,24 +281,6 @@ export {}; 53 | [Symbol.iterator]: 1, `---- - x TS9038: Computed property names on class or object literals cannot be - | inferred with --isolatedDeclarations. - ,-[declarationComputedPropertyNames.d.ts:53:6] - 52 | [presentNs.a]: 1, - 53 | [Symbol.iterator]: 1, - : ^^^^^^^^^^^^^^^ - 54 | [globalThis.Symbol.toStringTag]: 1, - `---- - - x TS9038: Computed property names on class or object literals cannot be - | inferred with --isolatedDeclarations. - ,-[declarationComputedPropertyNames.d.ts:54:6] - 53 | [Symbol.iterator]: 1, - 54 | [globalThis.Symbol.toStringTag]: 1, - : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - 55 | [(globalThis.Symbol).unscopables]: 1, - `---- - x TS9038: Computed property names on class or object literals cannot be | inferred with --isolatedDeclarations. ,-[declarationComputedPropertyNames.d.ts:55:6]