From 2fd5c2a53b71858526aa8837176e430e75891a05 Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Fri, 20 Sep 2024 10:18:09 +0000 Subject: [PATCH] refactor(isolated-declarations): pre-filter statements that do not need to be transformed (#5909) We only transform `Declaration` and `ModuleDeclaration` in `IsolatedDeclaration`. We pre-filter statements that need to transform and then use `clone_in` which will avoid producing overhead for clone `Statements`. --- crates/oxc_isolated_declarations/src/lib.rs | 82 +++++++++++-------- crates/oxc_isolated_declarations/src/scope.rs | 12 ++- .../tests/snapshots/expando-function.snap | 20 ++--- 3 files changed, 64 insertions(+), 50 deletions(-) diff --git a/crates/oxc_isolated_declarations/src/lib.rs b/crates/oxc_isolated_declarations/src/lib.rs index 80d62e068e2fd..c987e8c2bc38e 100644 --- a/crates/oxc_isolated_declarations/src/lib.rs +++ b/crates/oxc_isolated_declarations/src/lib.rs @@ -22,7 +22,7 @@ mod types; use std::{cell::RefCell, collections::VecDeque, mem}; use diagnostics::function_with_assigning_properties; -use oxc_allocator::Allocator; +use oxc_allocator::{Allocator, CloneIn}; #[allow(clippy::wildcard_imports)] use oxc_ast::{ast::*, AstBuilder, Trivias, Visit, NONE}; use oxc_diagnostics::OxcDiagnostic; @@ -147,30 +147,39 @@ impl<'a> IsolatedDeclarations<'a> { &mut self, stmts: &oxc_allocator::Vec<'a, Statement<'a>>, ) -> oxc_allocator::Vec<'a, Statement<'a>> { - let mut new_ast_stmts = self.ast.vec::>(); - // SAFETY: `ast.copy` is unsound! We need to fix. - for stmt in Self::remove_function_overloads_implementation(unsafe { self.ast.copy(stmts) }) - { - if let Some(decl) = stmt.as_declaration() { + self.report_error_for_expando_function(stmts); + + let filtered_stmts = stmts.iter().filter_map(|stmt| { + if stmt.is_declaration() { + Some(stmt.clone_in(self.ast.allocator)) + } else { + None + } + }); + + let filtered_stmts = Self::remove_function_overloads_implementation(filtered_stmts); + + let mut stmts = self.ast.vec_from_iter(filtered_stmts); + for stmt in stmts.iter_mut() { + if let Some(decl) = stmt.as_declaration_mut() { if self.has_internal_annotation(decl.span()) { continue; } - if let Some(decl) = self.transform_declaration(decl, false) { - new_ast_stmts.push(Statement::from(decl)); - } else { - // SAFETY: `ast.copy` is unsound! We need to fix. - new_ast_stmts.push(Statement::from(unsafe { self.ast.copy(decl) })); + if let Some(new_decl) = self.transform_declaration(decl, false) { + *decl = new_decl; } } } - self.report_error_for_expando_function(stmts); - new_ast_stmts + + stmts } pub fn transform_statements_on_demand( &mut self, stmts: &oxc_allocator::Vec<'a, Statement<'a>>, ) -> oxc_allocator::Vec<'a, Statement<'a>> { + self.report_error_for_expando_function(stmts); + // https://github.com/microsoft/TypeScript/pull/58912 let mut need_empty_export_marker = true; @@ -178,13 +187,22 @@ impl<'a> IsolatedDeclarations<'a> { let mut variables_declarations = VecDeque::new(); let mut variable_transformed_indexes = VecDeque::new(); let mut transformed_indexes = FxHashSet::default(); + + let filtered_stmts = stmts.iter().filter_map(|stmt| { + if stmt.is_declaration() || stmt.is_module_declaration() { + Some(stmt.clone_in(self.ast.allocator)) + } else { + None + } + }); + + let filtered_stmts = Self::remove_function_overloads_implementation(filtered_stmts); + // 1. Collect all declarations, module declarations // 2. Transform export declarations // 3. Collect all bindings / reference from module declarations // 4. Collect transformed indexes - // SAFETY: `ast.copy` is unsound! We need to fix. - for stmt in Self::remove_function_overloads_implementation(unsafe { self.ast.copy(stmts) }) - { + for mut stmt in filtered_stmts { match stmt { match_declaration!(Statement) => { match stmt.to_declaration() { @@ -219,7 +237,7 @@ impl<'a> IsolatedDeclarations<'a> { } } match_module_declaration!(Statement) => { - match stmt.to_module_declaration() { + match stmt.to_module_declaration_mut() { ModuleDeclaration::ExportDefaultDeclaration(decl) => { if self.has_internal_annotation(decl.span) { continue; @@ -229,7 +247,6 @@ impl<'a> IsolatedDeclarations<'a> { self.transform_export_default_declaration(decl) { if let Some(var_decl) = var_decl { - need_empty_export_marker = false; self.scope.visit_variable_declaration(&var_decl); new_stmts.push(Statement::VariableDeclaration( self.ast.alloc(var_decl), @@ -238,10 +255,7 @@ impl<'a> IsolatedDeclarations<'a> { } self.scope.visit_export_default_declaration(&new_decl); - new_stmts.push(Statement::ExportDefaultDeclaration( - self.ast.alloc(new_decl), - )); - continue; + *decl = self.ast.alloc(new_decl); } need_empty_export_marker = false; @@ -254,16 +268,10 @@ impl<'a> IsolatedDeclarations<'a> { } transformed_indexes.insert(new_stmts.len()); if let Some(new_decl) = self.transform_export_named_declaration(decl) { - self.scope.visit_declaration( - new_decl.declaration.as_ref().unwrap_or_else(|| unreachable!()), - ); - - new_stmts.push(Statement::ExportNamedDeclaration( - self.ast.alloc(new_decl), - )); - continue; + *decl = self.ast.alloc(new_decl); + } else { + need_empty_export_marker = false; } - need_empty_export_marker = false; self.scope.visit_export_named_declaration(decl); } ModuleDeclaration::ImportDeclaration(_) => { @@ -382,17 +390,19 @@ impl<'a> IsolatedDeclarations<'a> { .push(Statement::from(ModuleDeclaration::ExportNamedDeclaration(empty_export))); } - self.report_error_for_expando_function(stmts); new_ast_stmts } - pub fn remove_function_overloads_implementation( - stmts: oxc_allocator::Vec<'a, Statement<'a>>, - ) -> impl Iterator> + '_ { + pub fn remove_function_overloads_implementation( + stmts: T, + ) -> impl Iterator> + where + T: Iterator>, + { let mut last_function_name: Option> = None; let mut is_export_default_function_overloads = false; - stmts.into_iter().filter_map(move |stmt| match stmt { + stmts.filter_map(move |stmt| match stmt { Statement::FunctionDeclaration(ref func) => { let name = &func .id diff --git a/crates/oxc_isolated_declarations/src/scope.rs b/crates/oxc_isolated_declarations/src/scope.rs index 921c48f6a976f..da894b7079828 100644 --- a/crates/oxc_isolated_declarations/src/scope.rs +++ b/crates/oxc_isolated_declarations/src/scope.rs @@ -140,10 +140,14 @@ impl<'a> Visit<'a> for ScopeTree<'a> { } fn visit_export_named_declaration(&mut self, decl: &ExportNamedDeclaration<'a>) { - for specifier in &decl.specifiers { - if let Some(name) = specifier.local.identifier_name() { - self.add_type_reference(name.clone()); - self.add_value_reference(name); + if let Some(declaration) = &decl.declaration { + walk_declaration(self, declaration); + } else { + for specifier in &decl.specifiers { + if let Some(name) = specifier.local.identifier_name() { + self.add_type_reference(name.clone()); + self.add_value_reference(name); + } } } } diff --git a/crates/oxc_isolated_declarations/tests/snapshots/expando-function.snap b/crates/oxc_isolated_declarations/tests/snapshots/expando-function.snap index 7bd2565b320e0..8eb57c0bc5425 100644 --- a/crates/oxc_isolated_declarations/tests/snapshots/expando-function.snap +++ b/crates/oxc_isolated_declarations/tests/snapshots/expando-function.snap @@ -21,16 +21,6 @@ export default qux; ==================== Errors ==================== - x TS9023: Assigning properties to functions without declaring them is not - | supported with --isolatedDeclarations. Add an explicit declaration for the - | properties assigned to this function. - ,-[10:3] - 9 | export const goo = (): void => {} - 10 | goo.length = 10 - : ^^^^^^^^^^ - 11 | } - `---- - x TS9023: Assigning properties to functions without declaring them is not | supported with --isolatedDeclarations. Add an explicit declaration for the | properties assigned to this function. @@ -60,3 +50,13 @@ export default qux; : ^^^^^^^ 20 | foo.baz = 100; `---- + + x TS9023: Assigning properties to functions without declaring them is not + | supported with --isolatedDeclarations. Add an explicit declaration for the + | properties assigned to this function. + ,-[10:3] + 9 | export const goo = (): void => {} + 10 | goo.length = 10 + : ^^^^^^^^^^ + 11 | } + `----