From 1d329e0761fdd9638187cd20c624963503db30d5 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 21 Oct 2024 19:16:09 -0400 Subject: [PATCH 1/6] feat: use Program instead of Module more --- src/cjs_parse.rs | 8 +++ src/dep.rs | 18 +++-- src/emit.rs | 15 ++-- src/parsed_source.rs | 112 +++++++++++++++++++++++++++--- src/transpiling/jsx_precompile.rs | 7 +- src/transpiling/mod.rs | 7 +- 6 files changed, 138 insertions(+), 29 deletions(-) diff --git a/src/cjs_parse.rs b/src/cjs_parse.rs index a06adc3..f10deb3 100644 --- a/src/cjs_parse.rs +++ b/src/cjs_parse.rs @@ -12,6 +12,7 @@ use crate::swc::visit::noop_visit_type; use crate::swc::visit::Visit; use crate::swc::visit::VisitWith; use crate::ParsedSource; +use crate::ProgramRef; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct CjsAnalysis { @@ -30,7 +31,14 @@ impl ParsedSource { } let mut visitor = CjsVisitor::default(); +<<<<<<< Updated upstream visitor.visit_script(self.script()); +======= + match self.program_ref() { + ProgramRef::Module(n) => visitor.visit_module(n), + ProgramRef::Script(n) => visitor.visit_script(n), + }; +>>>>>>> Stashed changes visitor.take_result() } } diff --git a/src/dep.rs b/src/dep.rs index 96c9016..bfd807d 100644 --- a/src/dep.rs +++ b/src/dep.rs @@ -3,7 +3,6 @@ use std::collections::HashMap; use ast::BinExpr; -use ast::Module; use serde::Deserialize; use serde::Serialize; @@ -16,6 +15,7 @@ use crate::swc::visit::Visit; use crate::swc::visit::VisitWith; use crate::MultiThreadedComments; use crate::ParsedSource; +use crate::ProgramRef; use crate::SourcePos; use crate::SourceRange; use crate::SourceRangedForSpanned; @@ -23,24 +23,22 @@ use crate::SourceRangedForSpanned; impl ParsedSource { /// Analyzes the module for a list of its imports and exports. pub fn analyze_dependencies(&self) -> Vec { - match self.program_ref() { - ast::Program::Module(module) => { - analyze_module_dependencies(module, self.comments()) - } - ast::Program::Script(_) => vec![], - } + analyze_program_dependencies(self.program_ref(), self.comments()) } } -pub fn analyze_module_dependencies( - module: &Module, +pub fn analyze_program_dependencies( + program: ProgramRef, comments: &MultiThreadedComments, ) -> Vec { let mut v = DependencyCollector { comments, items: vec![], }; - module.visit_with(&mut v); + match program { + ProgramRef::Module(n) => n.visit_with(&mut v), + ProgramRef::Script(n) => n.visit_with(&mut v), + } v.items } diff --git a/src/emit.rs b/src/emit.rs index 362fcb4..7bc2215 100644 --- a/src/emit.rs +++ b/src/emit.rs @@ -5,11 +5,11 @@ use std::string::FromUtf8Error; use base64::Engine; use thiserror::Error; -use crate::swc::ast::Program; use crate::swc::codegen::text_writer::JsWriter; use crate::swc::codegen::Node; use crate::swc::common::FileName; use crate::ModuleSpecifier; +use crate::ProgramRef; use crate::SourceMap; #[derive(Default, Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -92,7 +92,7 @@ pub enum EmitError { /// Emits the program as a string of JavaScript code, possibly with the passed /// comments, and optionally also a source map. pub fn emit( - program: &Program, + program: ProgramRef, comments: &dyn crate::swc::common::comments::Comments, source_map: &SourceMap, emit_options: &EmitOptions, @@ -119,9 +119,14 @@ pub fn emit( cm: source_map.clone(), wr: writer, }; - program - .emit_with(&mut emitter) - .map_err(EmitError::SwcEmit)?; + match program { + ProgramRef::Module(n) => { + n.emit_with(&mut emitter).map_err(EmitError::SwcEmit)?; + } + ProgramRef::Script(n) => { + n.emit_with(&mut emitter).map_err(EmitError::SwcEmit)?; + } + } } let mut map: Option> = None; diff --git a/src/parsed_source.rs b/src/parsed_source.rs index ca9bfbe..147920a 100644 --- a/src/parsed_source.rs +++ b/src/parsed_source.rs @@ -9,6 +9,9 @@ use dprint_swc_ext::common::SourceTextInfo; use dprint_swc_ext::common::SourceTextProvider; use dprint_swc_ext::common::StartSourcePos; use swc_common::Mark; +use swc_ecma_ast::ModuleDecl; +use swc_ecma_ast::ModuleItem; +use swc_ecma_ast::Stmt; use crate::comments::MultiThreadedComments; use crate::scope_analysis_transform; @@ -59,6 +62,92 @@ impl Globals { } } +/// A reference to a Program. +/// +/// It is generally preferable for functions to accept this over `&Program` +/// because it doesn't require cloning when only owning a `Module` or `Script`. +#[derive(Debug, Clone, Copy)] +pub enum ProgramRef<'a> { + Module(&'a Module), + Script(&'a Script), +} + +impl<'a> From<&'a Program> for ProgramRef<'a> { + fn from(program: &'a Program) -> Self { + match program { + Program::Module(module) => ProgramRef::Module(module), + Program::Script(script) => ProgramRef::Script(script), + } + } +} + +impl<'a> ProgramRef<'a> { + pub fn shebang(&self) -> &Option { + match self { + ProgramRef::Module(m) => &m.shebang, + ProgramRef::Script(s) => &s.shebang, + } + } + + pub fn body(&self) -> impl Iterator> { + match self { + ProgramRef::Module(m) => Box::new(m.body.iter().map(|n| n.into())) + as Box>>, + ProgramRef::Script(s) => Box::new(s.body.iter().map(ModuleItemRef::Stmt)), + } + } + + pub fn to_owned(&self) -> Program { + match self { + ProgramRef::Module(m) => Program::Module((*m).clone()), + ProgramRef::Script(s) => Program::Script((*s).clone()), + } + } +} + +impl swc_common::Spanned for ProgramRef<'_> { + // ok because we're implementing Spanned + #[allow(clippy::disallowed_methods)] + #[allow(clippy::disallowed_types)] + fn span(&self) -> swc_common::Span { + match self { + Self::Module(m) => m.span, + Self::Script(s) => s.span, + } + } +} + +/// Reference to a ModuleDecl or Stmt in a Program. +/// +/// This is used to allow using the same API for the top level +/// statements when working with a ProgramRef. +#[derive(Debug, Clone, Copy)] +pub enum ModuleItemRef<'a> { + ModuleDecl(&'a ModuleDecl), + Stmt(&'a Stmt), +} + +impl swc_common::Spanned for ModuleItemRef<'_> { + // ok because we're implementing Spanned + #[allow(clippy::disallowed_methods)] + #[allow(clippy::disallowed_types)] + fn span(&self) -> swc_common::Span { + match self { + Self::ModuleDecl(n) => n.span(), + Self::Stmt(n) => n.span(), + } + } +} + +impl<'a> From<&'a ModuleItem> for ModuleItemRef<'a> { + fn from(item: &'a ModuleItem) -> Self { + match item { + ModuleItem::ModuleDecl(n) => ModuleItemRef::ModuleDecl(n), + ModuleItem::Stmt(n) => ModuleItemRef::Stmt(n), + } + } +} + #[derive(Clone)] pub(crate) struct SyntaxContexts { pub unresolved: SyntaxContext, @@ -126,8 +215,11 @@ impl ParsedSource { } /// Gets the parsed program as a reference. - pub fn program_ref(&self) -> &Program { - &self.0.program + pub fn program_ref(&self) -> ProgramRef<'_> { + match self.0.program.as_ref() { + Program::Module(module) => ProgramRef::Module(module), + Program::Script(script) => ProgramRef::Script(script), + } } /// Gets the parsed module. @@ -135,8 +227,8 @@ impl ParsedSource { /// This will panic if the source is not a module. pub fn module(&self) -> &Module { match self.program_ref() { - Program::Module(module) => module, - Program::Script(_) => panic!("Cannot get a module when the source was a script. Use `.program()` instead."), + ProgramRef::Module(module) => module, + ProgramRef::Script(_) => panic!("Cannot get a module when the source was a script. Use `.program()` instead."), } } @@ -145,8 +237,8 @@ impl ParsedSource { /// This will panic if the source is not a script. pub fn script(&self) -> &Script { match self.program_ref() { - Program::Script(script) => script, - Program::Module(_) => panic!("Cannot get a script when the source was a module. Use `.program()` instead."), + ProgramRef::Script(script) => script, + ProgramRef::Module(_) => panic!("Cannot get a script when the source was a module. Use `.program()` instead."), } } @@ -245,12 +337,12 @@ impl ParsedSource { /// Gets if this source is a module. pub fn is_module(&self) -> bool { - matches!(self.program_ref(), Program::Module(_)) + matches!(self.program_ref(), ProgramRef::Module(_)) } /// Gets if this source is a script. pub fn is_script(&self) -> bool { - matches!(self.program_ref(), Program::Script(_)) + matches!(self.program_ref(), ProgramRef::Script(_)) } } @@ -298,8 +390,8 @@ impl ParsedSource { ) -> T { let program_info = crate::view::ProgramInfo { program: match self.program_ref() { - Program::Module(module) => crate::view::ProgramRef::Module(module), - Program::Script(script) => crate::view::ProgramRef::Script(script), + ProgramRef::Module(module) => crate::view::ProgramRef::Module(module), + ProgramRef::Script(script) => crate::view::ProgramRef::Script(script), }, text_info: Some(self.text_info_lazy()), tokens: self.0.tokens.as_ref().map(|t| t as &[TokenAndSpan]), diff --git a/src/transpiling/jsx_precompile.rs b/src/transpiling/jsx_precompile.rs index b651fc6..88e0786 100644 --- a/src/transpiling/jsx_precompile.rs +++ b/src/transpiling/jsx_precompile.rs @@ -1650,6 +1650,7 @@ mod tests { use crate::swc::visit::FoldWith; use crate::EmitOptions; use crate::ModuleSpecifier; + use crate::ProgramRef; use crate::SourceMap; use pretty_assertions::assert_eq; use swc_common::comments::SingleThreadedComments; @@ -3018,7 +3019,7 @@ const a = _jsxTemplate($$_tpl_1, _jsxAttr("class", "foo"), _jsxAttr("className", ) { let (source_map, module) = parse(src); let mut transform_folder = as_folder(transform); - let output = print(&source_map, module.fold_with(&mut transform_folder)); + let output = print(&source_map, &module.fold_with(&mut transform_folder)); assert_eq!(output, format!("{}\n", expected_output)); } @@ -3037,9 +3038,9 @@ const a = _jsxTemplate($$_tpl_1, _jsxAttr("class", "foo"), _jsxAttr("className", (source_map, parser.parse_module().unwrap()) } - fn print(source_map: &SourceMap, module: Module) -> String { + fn print(source_map: &SourceMap, module: &Module) -> String { crate::emit::emit( - &Program::Module(module), + ProgramRef::Module(module), &SingleThreadedComments::default(), source_map, &EmitOptions { diff --git a/src/transpiling/mod.rs b/src/transpiling/mod.rs index c6286be..b4797a6 100644 --- a/src/transpiling/mod.rs +++ b/src/transpiling/mod.rs @@ -348,7 +348,12 @@ fn transpile( ) })?; - Ok(emit(&program, &comments, &source_map, emit_options)?) + Ok(emit( + (&program).into(), + &comments, + &source_map, + emit_options, + )?) } #[derive(Default, Clone)] From 219027b939b9fb05f7f7d648aae4f7570356c6b8 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 21 Oct 2024 19:17:27 -0400 Subject: [PATCH 2/6] fix merge conflict --- src/cjs_parse.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/cjs_parse.rs b/src/cjs_parse.rs index f10deb3..3d7f8ad 100644 --- a/src/cjs_parse.rs +++ b/src/cjs_parse.rs @@ -31,14 +31,10 @@ impl ParsedSource { } let mut visitor = CjsVisitor::default(); -<<<<<<< Updated upstream - visitor.visit_script(self.script()); -======= match self.program_ref() { ProgramRef::Module(n) => visitor.visit_module(n), ProgramRef::Script(n) => visitor.visit_script(n), }; ->>>>>>> Stashed changes visitor.take_result() } } From 0f98ca8cd932ba3c46652fb4b5526bd733e90627 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 21 Oct 2024 19:23:45 -0400 Subject: [PATCH 3/6] fix description to be less confusing --- src/dep.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/dep.rs b/src/dep.rs index bfd807d..0ed6932 100644 --- a/src/dep.rs +++ b/src/dep.rs @@ -21,7 +21,9 @@ use crate::SourceRange; use crate::SourceRangedForSpanned; impl ParsedSource { - /// Analyzes the module for a list of its imports and exports. + /// Analyzes the module for a list of its static and dynamic imports. + /// + /// Note: This will also include `require` calls as dynamic imports. pub fn analyze_dependencies(&self) -> Vec { analyze_program_dependencies(self.program_ref(), self.comments()) } From 95924861b6134faf830887b6b95a1afd38abc2a6 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 21 Oct 2024 20:08:01 -0400 Subject: [PATCH 4/6] add VisitWith impl --- src/parsed_source.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/parsed_source.rs b/src/parsed_source.rs index 147920a..a2ce517 100644 --- a/src/parsed_source.rs +++ b/src/parsed_source.rs @@ -81,6 +81,24 @@ impl<'a> From<&'a Program> for ProgramRef<'a> { } } +impl<'a, T: swc_ecma_visit::Visit> swc_ecma_visit::VisitWith + for ProgramRef<'a> +{ + fn visit_with(&self, visitor: &mut T) { + match self { + ProgramRef::Module(n) => n.visit_with(visitor), + ProgramRef::Script(n) => n.visit_with(visitor), + } + } + + fn visit_children_with(&self, visitor: &mut T) { + match self { + ProgramRef::Module(n) => n.visit_children_with(visitor), + ProgramRef::Script(n) => n.visit_children_with(visitor), + } + } +} + impl<'a> ProgramRef<'a> { pub fn shebang(&self) -> &Option { match self { From 54f0693607e36f797e597f9617cfa24314b39f5a Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 22 Oct 2024 11:39:29 -0400 Subject: [PATCH 5/6] use Option<&T> --- src/parsed_source.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/parsed_source.rs b/src/parsed_source.rs index a2ce517..bc0518d 100644 --- a/src/parsed_source.rs +++ b/src/parsed_source.rs @@ -100,10 +100,10 @@ impl<'a, T: swc_ecma_visit::Visit> swc_ecma_visit::VisitWith } impl<'a> ProgramRef<'a> { - pub fn shebang(&self) -> &Option { + pub fn shebang(&self) -> Option<&swc_atoms::Atom> { match self { - ProgramRef::Module(m) => &m.shebang, - ProgramRef::Script(s) => &s.shebang, + ProgramRef::Module(m) => m.shebang.as_ref(), + ProgramRef::Script(s) => s.shebang.as_ref(), } } From 5aebfc77a6ab8ea0bca8285f5279b8ffdb5fce32 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 22 Oct 2024 11:49:56 -0400 Subject: [PATCH 6/6] make it harder to use module() or script() --- src/dep.rs | 2 +- src/parsed_source.rs | 79 ++++++++++++++++++++++---------------------- src/parsing.rs | 4 +-- 3 files changed, 42 insertions(+), 43 deletions(-) diff --git a/src/dep.rs b/src/dep.rs index 0ed6932..1f9ebfb 100644 --- a/src/dep.rs +++ b/src/dep.rs @@ -567,7 +567,7 @@ mod tests { maybe_syntax: None, }) .unwrap(); - (source.module().start(), source.analyze_dependencies()) + (source.program_ref().start(), source.analyze_dependencies()) } #[test] diff --git a/src/parsed_source.rs b/src/parsed_source.rs index bc0518d..db22c75 100644 --- a/src/parsed_source.rs +++ b/src/parsed_source.rs @@ -72,34 +72,25 @@ pub enum ProgramRef<'a> { Script(&'a Script), } -impl<'a> From<&'a Program> for ProgramRef<'a> { - fn from(program: &'a Program) -> Self { - match program { - Program::Module(module) => ProgramRef::Module(module), - Program::Script(script) => ProgramRef::Script(script), - } - } -} - -impl<'a, T: swc_ecma_visit::Visit> swc_ecma_visit::VisitWith - for ProgramRef<'a> -{ - fn visit_with(&self, visitor: &mut T) { +impl<'a> ProgramRef<'a> { + pub fn unwrap_module(&self) -> &Module { match self { - ProgramRef::Module(n) => n.visit_with(visitor), - ProgramRef::Script(n) => n.visit_with(visitor), + ProgramRef::Module(m) => m, + ProgramRef::Script(_) => { + panic!("Cannot get a module when the source was a script.") + } } } - fn visit_children_with(&self, visitor: &mut T) { + pub fn unwrap_script(&self) -> &Script { match self { - ProgramRef::Module(n) => n.visit_children_with(visitor), - ProgramRef::Script(n) => n.visit_children_with(visitor), + ProgramRef::Module(_) => { + panic!("Cannot get a script when the source was a module.") + } + ProgramRef::Script(s) => s, } } -} -impl<'a> ProgramRef<'a> { pub fn shebang(&self) -> Option<&swc_atoms::Atom> { match self { ProgramRef::Module(m) => m.shebang.as_ref(), @@ -123,6 +114,34 @@ impl<'a> ProgramRef<'a> { } } +impl<'a> From<&'a Program> for ProgramRef<'a> { + fn from(program: &'a Program) -> Self { + match program { + Program::Module(module) => ProgramRef::Module(module), + Program::Script(script) => ProgramRef::Script(script), + } + } +} + +#[cfg(feature = "visit")] +impl<'a, T: swc_ecma_visit::Visit> swc_ecma_visit::VisitWith + for ProgramRef<'a> +{ + fn visit_with(&self, visitor: &mut T) { + match self { + ProgramRef::Module(n) => n.visit_with(visitor), + ProgramRef::Script(n) => n.visit_with(visitor), + } + } + + fn visit_children_with(&self, visitor: &mut T) { + match self { + ProgramRef::Module(n) => n.visit_children_with(visitor), + ProgramRef::Script(n) => n.visit_children_with(visitor), + } + } +} + impl swc_common::Spanned for ProgramRef<'_> { // ok because we're implementing Spanned #[allow(clippy::disallowed_methods)] @@ -240,26 +259,6 @@ impl ParsedSource { } } - /// Gets the parsed module. - /// - /// This will panic if the source is not a module. - pub fn module(&self) -> &Module { - match self.program_ref() { - ProgramRef::Module(module) => module, - ProgramRef::Script(_) => panic!("Cannot get a module when the source was a script. Use `.program()` instead."), - } - } - - /// Gets the parsed script. - /// - /// This will panic if the source is not a script. - pub fn script(&self) -> &Script { - match self.program_ref() { - ProgramRef::Script(script) => script, - ProgramRef::Module(_) => panic!("Cannot get a script when the source was a module. Use `.program()` instead."), - } - } - /// Gets the comments found in the source file. pub fn comments(&self) -> &MultiThreadedComments { &self.0.comments diff --git a/src/parsing.rs b/src/parsing.rs index 90dece4..dfe962b 100644 --- a/src/parsing.rs +++ b/src/parsing.rs @@ -387,7 +387,7 @@ mod test { assert_eq!(program.text().as_ref(), "// 1\n1 + 1\n// 2"); assert_eq!(program.media_type(), MediaType::JavaScript); assert!(matches!( - program.script().body[0], + program.program_ref().unwrap_script().body[0], crate::swc::ast::Stmt::Expr(..) )); assert_eq!(program.get_leading_comments().unwrap().len(), 1); @@ -408,7 +408,7 @@ mod test { }) .unwrap(); assert!(matches!( - program.module().body[0], + program.program_ref().unwrap_module().body[0], crate::swc::ast::ModuleItem::Stmt(..) )); }