Skip to content

Commit

Permalink
fix(isolated_declarations): Emit computed properties when they are we…
Browse files Browse the repository at this point in the history
…ll known symbols
  • Loading branch information
MichaelMitchell-at committed Jul 15, 2024
1 parent b94540d commit 14de332
Show file tree
Hide file tree
Showing 7 changed files with 290 additions and 41 deletions.
11 changes: 9 additions & 2 deletions crates/oxc_isolated_declarations/src/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
176 changes: 176 additions & 0 deletions crates/oxc_isolated_declarations/src/global_symbol_binding_tracker.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
use std::cell::Cell;

use oxc_allocator::{Allocator, Vec};
#[allow(clippy::wildcard_imports)]
use oxc_ast::ast::*;
use oxc_ast::AstBuilder;
#[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;

/// Declaration scope.
#[derive(Debug)]
struct Scope {
has_symbol_binding: bool,
has_global_this_binding: bool,
}

impl Scope {
fn new() -> Self {
Self { has_symbol_binding: false, has_global_this_binding: false }
}
}

/// Linear tree of declaration scopes.
pub struct GlobalSymbolBindingTracker<'a> {
levels: Vec<'a, Scope>,
computed_properties_using_non_global_symbol: FxHashSet<Span>,
computed_properties_using_non_global_global_this: FxHashSet<Span>,
}

impl<'a> GlobalSymbolBindingTracker<'a> {
pub fn new(allocator: &'a Allocator) -> Self {
let ast = AstBuilder::new(allocator);
let levels = ast.vec1(Scope::new());
Self {
levels,
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 has_in_scope_symbol_binding(&self) -> bool {
self.levels.iter().any(|level| level.has_symbol_binding)
}

fn has_in_scope_global_this_binding(&self) -> bool {
self.levels.iter().any(|level| level.has_global_this_binding)
}

fn handle_name_binding(&mut self, name: &Atom) {
match name.as_str() {
"Symbol" => {
let scope = self.levels.last_mut().unwrap();
scope.has_symbol_binding = true;
}
"globalThis" => {
let scope = self.levels.last_mut().unwrap();
scope.has_global_this_binding = true;
}
_ => {}
}
}
}

impl<'a> Visit<'a> for GlobalSymbolBindingTracker<'a> {
fn enter_scope(&mut self, _: ScopeFlags, _: &Cell<Option<ScopeId>>) {
let scope = Scope::new();
self.levels.push(scope);
}

fn leave_scope(&mut self) {
self.levels.pop();
}

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);
}
Declaration::TSTypeAliasDeclaration(_)
| Declaration::TSInterfaceDeclaration(_)
| Declaration::TSEnumDeclaration(_) => 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.has_in_scope_symbol_binding() {
if let Expression::Identifier(identifier) = &expr.object {
if identifier.name == "Symbol" {
self.computed_properties_using_non_global_symbol
.insert(prop.key.span());
}
}
}

if self.has_in_scope_global_this_binding() {
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(prop.key.span());
}
}
}
}
}
}

walk_object_property(self, prop);
}
}
8 changes: 8 additions & 0 deletions crates/oxc_isolated_declarations/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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> {
Expand All @@ -39,6 +41,7 @@ pub struct IsolatedDeclarations<'a> {
ast: AstBuilder<'a>,
// state
scope: ScopeTree<'a>,
global_symbol_binding_tracker: GlobalSymbolBindingTracker<'a>,
errors: RefCell<Vec<OxcDiagnostic>>,
}

Expand All @@ -47,6 +50,7 @@ impl<'a> IsolatedDeclarations<'a> {
Self {
ast: AstBuilder::new(allocator),
scope: ScopeTree::new(allocator),
global_symbol_binding_tracker: GlobalSymbolBindingTracker::new(allocator),
errors: RefCell::new(vec![]),
}
}
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_isolated_declarations/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Correct
export const foo = {
[Symbol.iterator]: (): void => {},
[Symbol.asyncIterator]: async (): Promise<void> => {},
[globalThis.Symbol.iterator]: (): void => {},
}

export abstract class Foo {
[Symbol.iterator](): void {}
async [Symbol.asyncIterator](): Promise<void> {}
[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 => {},
}
}
Original file line number Diff line number Diff line change
@@ -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<void>;
[globalThis.Symbol.iterator]: () => void;
};
export declare abstract class Foo {
[Symbol.iterator](): void;
[Symbol.asyncIterator](): Promise<void>;
[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 | }
`----
Loading

0 comments on commit 14de332

Please sign in to comment.