From 68ec77193bab6ec5f353764cebf77fa536168eee Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 16 Apr 2024 12:10:59 -0400 Subject: [PATCH 1/2] fix: do not panic doing scope analysis before emit --- src/parsing.rs | 2 +- src/transpiling/mod.rs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/parsing.rs b/src/parsing.rs index 5d5d5b9..8f39776 100644 --- a/src/parsing.rs +++ b/src/parsing.rs @@ -188,7 +188,7 @@ fn scope_analysis_transform_inner( let globals = Globals::new(); crate::swc::common::GLOBALS.set(&globals, || { let unresolved_mark = Mark::new(); - let top_level_mark = Mark::new(); + let top_level_mark = Mark::fresh(Mark::root()); let program = program.fold_with(&mut resolver(unresolved_mark, top_level_mark, true)); diff --git a/src/transpiling/mod.rs b/src/transpiling/mod.rs index 444d41a..b3132ec 100644 --- a/src/transpiling/mod.rs +++ b/src/transpiling/mod.rs @@ -1600,4 +1600,38 @@ const a = _jsx(Foo, { .unwrap(); assert_eq!(&emit_result.text, "const foo = \"bar\";\n"); } + + #[test] + fn should_not_panic_with_scope_analysis() { + let specifier = + ModuleSpecifier::parse("https://deno.land/x/mod.ts").unwrap(); + let source = r#" +const inspect: () => void = eval(); + +export function defaultFormatter(record: Record): string { + for (let i = 0; i < 10; i++) { + inspect(record); + } +} + +export function formatter(record: Record) { +} +"#; + let module = parse_module(ParseParams { + specifier, + text_info: SourceTextInfo::from_string(source.to_string()), + media_type: MediaType::TypeScript, + capture_tokens: false, + maybe_syntax: None, + scope_analysis: true, + }) + .unwrap(); + + let emit_result = module + .transpile( + &TranspileOptions::default(), + &EmitOptions::default(), + ); + assert!(emit_result.is_ok()); + } } From b1c2d3679a67026638c26abb4603b1f18c6535af Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 16 Apr 2024 15:53:12 -0400 Subject: [PATCH 2/2] Store globals in ParsedSource. --- src/parsed_source.rs | 50 ++++++++++++++++++++++++++++++++- src/parsing.rs | 64 ++++++++++++++++++++++++------------------ src/transpiling/mod.rs | 43 +++++++++++++++------------- 3 files changed, 108 insertions(+), 49 deletions(-) diff --git a/src/parsed_source.rs b/src/parsed_source.rs index 874370f..272cd84 100644 --- a/src/parsed_source.rs +++ b/src/parsed_source.rs @@ -3,6 +3,8 @@ use std::fmt; use std::sync::Arc; +use swc_common::Mark; + use crate::comments::MultiThreadedComments; use crate::scope_analysis_transform; use crate::swc::ast::Module; @@ -17,6 +19,42 @@ use crate::ParseDiagnostic; use crate::SourceRangedForSpanned; use crate::SourceTextInfo; +#[derive(Debug, Clone)] +pub struct Marks { + pub unresolved: Mark, + pub top_level: Mark, +} + +#[derive(Clone)] +pub struct Globals { + marks: Marks, + globals: Arc, +} + +impl Default for Globals { + fn default() -> Self { + let globals = crate::swc::common::Globals::new(); + let marks = crate::swc::common::GLOBALS.set(&globals, || Marks { + unresolved: Mark::new(), + top_level: Mark::fresh(Mark::root()), + }); + Self { + marks, + globals: Arc::new(globals), + } + } +} + +impl Globals { + pub fn with(&self, action: impl FnOnce(&Marks) -> T) -> T { + crate::swc::common::GLOBALS.set(&self.globals, || action(&self.marks)) + } + + pub fn marks(&self) -> &Marks { + &self.marks + } +} + #[derive(Clone)] pub(crate) struct SyntaxContexts { pub unresolved: SyntaxContext, @@ -30,6 +68,7 @@ pub(crate) struct ParsedSourceInner { pub comments: MultiThreadedComments, pub program: Arc, pub tokens: Option>>, + pub globals: Globals, pub syntax_contexts: Option, pub diagnostics: Vec, } @@ -51,6 +90,7 @@ impl ParsedSource { comments: MultiThreadedComments, program: Arc, tokens: Option>>, + globals: Globals, syntax_contexts: Option, diagnostics: Vec, ) -> Self { @@ -62,6 +102,7 @@ impl ParsedSource { comments, program, tokens, + globals, syntax_contexts, diagnostics, }), @@ -118,6 +159,11 @@ impl ParsedSource { &self.inner.comments } + /// Wrapper around globals that swc uses for transpiling. + pub fn globals(&self) -> &Globals { + &self.inner.globals + } + /// Get the source's leading comments, where triple slash directives might /// be located. pub fn get_leading_comments(&self) -> Option<&Vec> { @@ -156,13 +202,15 @@ impl ParsedSource { tokens: arc_inner.tokens.clone(), syntax_contexts: arc_inner.syntax_contexts.clone(), diagnostics: arc_inner.diagnostics.clone(), + globals: arc_inner.globals.clone(), }, }; let program = match Arc::try_unwrap(inner.program) { Ok(program) => program, Err(program) => (*program).clone(), }; - let (program, context) = scope_analysis_transform(program); + let (program, context) = + scope_analysis_transform(program, &inner.globals); inner.program = Arc::new(program); inner.syntax_contexts = context; ParsedSource { diff --git a/src/parsing.rs b/src/parsing.rs index 8f39776..eba64f1 100644 --- a/src/parsing.rs +++ b/src/parsing.rs @@ -15,6 +15,7 @@ use crate::swc::parser::token::TokenAndSpan; use crate::swc::parser::EsConfig; use crate::swc::parser::Syntax; use crate::swc::parser::TsConfig; +use crate::Globals; use crate::MediaType; use crate::ModuleSpecifier; use crate::ParseDiagnostic; @@ -48,7 +49,7 @@ pub struct ParseParams { pub fn parse_program( params: ParseParams, ) -> Result { - parse(params, ParseMode::Program, |p| p) + parse(params, ParseMode::Program, |p, _| p) } /// Parses the provided information as a program with the option of providing some @@ -66,7 +67,7 @@ pub fn parse_program( /// maybe_syntax: None, /// scope_analysis: false, /// }, -/// |program| { +/// |program, _globals| { /// // do something with the program here before it gets stored /// program /// }, @@ -74,7 +75,7 @@ pub fn parse_program( /// ``` pub fn parse_program_with_post_process( params: ParseParams, - post_process: impl FnOnce(Program) -> Program, + post_process: impl FnOnce(Program, &Globals) -> Program, ) -> Result { parse(params, ParseMode::Program, post_process) } @@ -83,36 +84,44 @@ pub fn parse_program_with_post_process( pub fn parse_module( params: ParseParams, ) -> Result { - parse(params, ParseMode::Module, |p| p) + parse(params, ParseMode::Module, |p, _| p) } /// Parses a module with post processing (see docs on `parse_program_with_post_process`). pub fn parse_module_with_post_process( params: ParseParams, - post_process: impl FnOnce(Module) -> Module, + post_process: impl FnOnce(Module, &Globals) -> Module, ) -> Result { - parse(params, ParseMode::Module, |program| match program { - Program::Module(module) => Program::Module(post_process(module)), - Program::Script(_) => unreachable!(), - }) + parse( + params, + ParseMode::Module, + |program, globals| match program { + Program::Module(module) => Program::Module(post_process(module, globals)), + Program::Script(_) => unreachable!(), + }, + ) } /// Parses the provided information to a script. pub fn parse_script( params: ParseParams, ) -> Result { - parse(params, ParseMode::Script, |p| p) + parse(params, ParseMode::Script, |p, _| p) } /// Parses a script with post processing (see docs on `parse_program_with_post_process`). pub fn parse_script_with_post_process( params: ParseParams, - post_process: impl FnOnce(Script) -> Script, + post_process: impl FnOnce(Script, &Globals) -> Script, ) -> Result { - parse(params, ParseMode::Script, |program| match program { - Program::Module(_) => unreachable!(), - Program::Script(script) => Program::Script(post_process(script)), - }) + parse( + params, + ParseMode::Script, + |program, globals| match program { + Program::Module(_) => unreachable!(), + Program::Script(script) => Program::Script(post_process(script, globals)), + }, + ) } enum ParseMode { @@ -124,7 +133,7 @@ enum ParseMode { fn parse( params: ParseParams, parse_mode: ParseMode, - post_process: impl FnOnce(Program) -> Program, + post_process: impl FnOnce(Program, &Globals) -> Program, ) -> Result { let source = params.text_info; let specifier = params.specifier; @@ -142,10 +151,11 @@ fn parse( .into_iter() .map(|err| ParseDiagnostic::from_swc_error(err, &specifier, source.clone())) .collect(); - let program = post_process(program); + let globals = Globals::default(); + let program = post_process(program, &globals); let (program, syntax_contexts) = if params.scope_analysis { - scope_analysis_transform(program) + scope_analysis_transform(program, &globals) } else { (program, None) }; @@ -157,6 +167,7 @@ fn parse( MultiThreadedComments::from_single_threaded(comments), Arc::new(program), tokens.map(Arc::new), + globals, syntax_contexts, diagnostics, )) @@ -164,10 +175,11 @@ fn parse( pub(crate) fn scope_analysis_transform( _program: Program, + _globals: &crate::Globals, ) -> (Program, Option) { #[cfg(feature = "transforms")] { - scope_analysis_transform_inner(_program) + scope_analysis_transform_inner(_program, _globals) } #[cfg(not(feature = "transforms"))] panic!( @@ -178,25 +190,21 @@ pub(crate) fn scope_analysis_transform( #[cfg(feature = "transforms")] fn scope_analysis_transform_inner( program: Program, + globals: &crate::Globals, ) -> (Program, Option) { - use crate::swc::common::Globals; - use crate::swc::common::Mark; use crate::swc::common::SyntaxContext; use crate::swc::transforms::resolver; use crate::swc::visit::FoldWith; - let globals = Globals::new(); - crate::swc::common::GLOBALS.set(&globals, || { - let unresolved_mark = Mark::new(); - let top_level_mark = Mark::fresh(Mark::root()); + globals.with(|marks| { let program = - program.fold_with(&mut resolver(unresolved_mark, top_level_mark, true)); + program.fold_with(&mut resolver(marks.unresolved, marks.top_level, true)); ( program, Some(crate::SyntaxContexts { - unresolved: SyntaxContext::empty().apply_mark(unresolved_mark), - top_level: SyntaxContext::empty().apply_mark(top_level_mark), + unresolved: SyntaxContext::empty().apply_mark(marks.unresolved), + top_level: SyntaxContext::empty().apply_mark(marks.top_level), }), ) }) diff --git a/src/transpiling/mod.rs b/src/transpiling/mod.rs index c6a0758..9bb2c37 100644 --- a/src/transpiling/mod.rs +++ b/src/transpiling/mod.rs @@ -12,8 +12,6 @@ use crate::swc::ast::Program; use crate::swc::common::chain; use crate::swc::common::comments::SingleThreadedComments; use crate::swc::common::errors::Diagnostic as SwcDiagnostic; -use crate::swc::common::Globals; -use crate::swc::common::Mark; use crate::swc::parser::error::SyntaxError; use crate::swc::transforms::fixer; use crate::swc::transforms::helpers; @@ -27,6 +25,8 @@ use crate::swc::visit::FoldWith; use crate::EmitError; use crate::EmitOptions; use crate::EmittedSource; +use crate::Globals; +use crate::Marks; use crate::ModuleSpecifier; use crate::ParseDiagnostic; use crate::ParseDiagnosticsError; @@ -217,6 +217,11 @@ impl ParsedSource { program, // we need the comments to be mutable, so make it single threaded self.comments().as_single_threaded(), + // todo(dsherret): this might be a bug where multiple transpiles could + // end up with globals from a different transpile, so this should probably + // do a deep clone of the globals, but that requires changes in swc and this + // is unlikely to be a problem in practice + self.globals(), transpile_options, emit_options, self.diagnostics(), @@ -245,6 +250,7 @@ impl ParsedSource { tokens: inner.tokens, syntax_contexts: inner.syntax_contexts, diagnostics: inner.diagnostics, + globals: inner.globals, }), }) } @@ -255,6 +261,7 @@ impl ParsedSource { program, // we need the comments to be mutable, so make it single threaded inner.comments.into_single_threaded(), + &inner.globals, transpile_options, emit_options, &inner.diagnostics, @@ -262,11 +269,13 @@ impl ParsedSource { } } +#[allow(clippy::too_many_arguments)] fn transpile( specifier: ModuleSpecifier, source: String, program: Program, comments: SingleThreadedComments, + globals: &Globals, transpile_options: &TranspileOptions, emit_options: &EmitOptions, diagnostics: &[ParseDiagnostic], @@ -279,15 +288,13 @@ fn transpile( let source_map = SourceMap::single(specifier, source); - let globals = Globals::new(); - let program = crate::swc::common::GLOBALS.set(&globals, || { - let top_level_mark = Mark::fresh(Mark::root()); + let program = globals.with(|marks| { fold_program( program, transpile_options, &source_map, &comments, - top_level_mark, + marks, diagnostics, ) })?; @@ -331,15 +338,14 @@ pub fn fold_program( options: &TranspileOptions, source_map: &SourceMap, comments: &SingleThreadedComments, - top_level_mark: Mark, + marks: &Marks, diagnostics: &[ParseDiagnostic], ) -> Result { ensure_no_fatal_diagnostics(diagnostics)?; - let unresolved_mark = Mark::new(); let mut passes = chain!( Optional::new(transforms::StripExportsFolder, options.var_decl_imports), - resolver(unresolved_mark, top_level_mark, true), + resolver(marks.unresolved, marks.top_level, true), Optional::new( proposal::decorators::decorators(proposal::decorators::Config { legacy: true, @@ -354,7 +360,7 @@ pub fn fold_program( options.use_decorators_proposal, ), proposal::explicit_resource_management::explicit_resource_management(), - helpers::inject_helpers(top_level_mark), + helpers::inject_helpers(marks.top_level), // transform imports to var decls before doing the typescript pass // so that swc doesn't do any optimizations on the import declarations Optional::new( @@ -362,7 +368,7 @@ pub fn fold_program( options.var_decl_imports ), Optional::new( - typescript::typescript(options.as_typescript_config(), top_level_mark), + typescript::typescript(options.as_typescript_config(), marks.top_level), !options.transform_jsx ), Optional::new( @@ -371,7 +377,7 @@ pub fn fold_program( options.as_typescript_config(), options.as_tsx_config(), comments, - top_level_mark + marks.top_level, ), options.transform_jsx ), @@ -408,8 +414,8 @@ pub fn fold_program( throw_if_namespace: Some(false), use_spread: None, }, - top_level_mark, - unresolved_mark, + marks.top_level, + marks.unresolved, ), options.transform_jsx ), @@ -1648,7 +1654,7 @@ const a = _jsx(Foo, { }; assert_eq!(&emit_result.text, "const foo = \"bar\";\n"); } - + #[test] fn should_not_panic_with_scope_analysis() { let specifier = @@ -1675,11 +1681,8 @@ export function formatter(record: Record) { }) .unwrap(); - let emit_result = module - .transpile( - &TranspileOptions::default(), - &EmitOptions::default(), - ); + let emit_result = + module.transpile(&TranspileOptions::default(), &EmitOptions::default()); assert!(emit_result.is_ok()); } }