refactor(codegen)!: remove Codegen::enableSourceMap API

Boshen · Boshen · commit 5b1e7fbac917 · 2024-10-11T17:51:13.000+08:00
diff --git a/crates/oxc/src/compiler.rs b/crates/oxc/src/compiler.rs
@@ -283,11 +283,10 @@ pub trait CompilerInterface {
         mangler: Option<Mangler>,
         options: CodegenOptions,
     ) -> CodegenReturn {
-        let mut codegen = CodeGenerator::new().with_options(options).with_mangler(mangler);
+        let mut options = options;
         if self.enable_sourcemap() {
-            codegen = codegen
-                .enable_source_map(source_path.to_string_lossy().as_ref(), program.source_text);
+            options.source_map_path = Some(source_path.to_path_buf());
         }
-        codegen.build(program)
+        CodeGenerator::new().with_options(options).with_mangler(mangler).build(program)
     }
 }
diff --git a/crates/oxc_codegen/examples/sourcemap.rs b/crates/oxc_codegen/examples/sourcemap.rs
@@ -3,7 +3,7 @@ use std::{env, path::Path};
 
 use base64::{prelude::BASE64_STANDARD, Engine};
 use oxc_allocator::Allocator;
-use oxc_codegen::{CodeGenerator, CodegenReturn};
+use oxc_codegen::{CodeGenerator, CodegenOptions, CodegenReturn};
 use oxc_parser::Parser;
 use oxc_span::SourceType;
 
@@ -28,7 +28,10 @@ fn main() -> std::io::Result<()> {
     }
 
     let CodegenReturn { code, map } = CodeGenerator::new()
-        .enable_source_map(path.to_string_lossy().as_ref(), &source_text)
+        .with_options(CodegenOptions {
+            source_map_path: Some(path.to_path_buf()),
+            ..CodegenOptions::default()
+        })
         .build(&ret.program);
 
     if let Some(source_map) = map {
diff --git a/crates/oxc_codegen/src/lib.rs b/crates/oxc_codegen/src/lib.rs
@@ -10,7 +10,7 @@ mod gen;
 mod operator;
 mod sourcemap_builder;
 
-use std::borrow::Cow;
+use std::{borrow::Cow, path::PathBuf};
 
 use oxc_ast::ast::{
     BindingIdentifier, BlockStatement, Expression, IdentifierReference, Program, Statement,
@@ -35,7 +35,7 @@ pub use crate::{
 /// Code generator without whitespace removal.
 pub type CodeGenerator<'a> = Codegen<'a>;
 
-#[derive(Clone, Copy)]
+#[derive(Debug, Clone)]
 pub struct CodegenOptions {
     /// Use single quotes instead of double quotes.
     ///
@@ -58,16 +58,24 @@ pub struct CodegenOptions {
     ///
     /// Default is `false`.
     pub annotation_comments: bool,
+
+    pub source_map_path: Option<PathBuf>,
 }
 
 impl Default for CodegenOptions {
     fn default() -> Self {
-        Self { single_quote: false, minify: false, comments: true, annotation_comments: false }
+        Self {
+            single_quote: false,
+            minify: false,
+            comments: true,
+            annotation_comments: false,
+            source_map_path: None,
+        }
     }
 }
 
 impl CodegenOptions {
-    fn print_annotation_comments(self) -> bool {
+    fn print_annotation_comments(&self) -> bool {
         !self.minify && (self.comments || self.annotation_comments)
     }
 }
@@ -78,6 +86,8 @@ pub struct CodegenReturn {
     pub code: String,
 
     /// The source map from the input source code to the generated source code.
+    ///
+    /// You must set [`CodegenOptions::source_map_path`] for this to be [`Some`].
     pub map: Option<oxc_sourcemap::SourceMap>,
 }
 
@@ -185,14 +195,6 @@ impl<'a> Codegen<'a> {
         self
     }
 
-    #[must_use]
-    pub fn enable_source_map(mut self, source_name: &str, source_text: &str) -> Self {
-        let mut sourcemap_builder = SourcemapBuilder::default();
-        sourcemap_builder.with_name_and_source(source_name, source_text);
-        self.sourcemap_builder = Some(sourcemap_builder);
-        self
-    }
-
     #[must_use]
     pub fn with_mangler(mut self, mangler: Option<Mangler>) -> Self {
         self.mangler = mangler;
@@ -207,6 +209,9 @@ impl<'a> Codegen<'a> {
         if self.options.print_annotation_comments() {
             self.build_comments(&program.comments);
         }
+        if let Some(path) = &self.options.source_map_path {
+            self.sourcemap_builder = Some(SourcemapBuilder::new(path, program.source_text));
+        }
 
         program.print(&mut self, Context::default());
         let code = self.into_source_text();
diff --git a/crates/oxc_codegen/src/sourcemap_builder.rs b/crates/oxc_codegen/src/sourcemap_builder.rs
@@ -1,4 +1,4 @@
-use std::sync::Arc;
+use std::{path::Path, sync::Arc};
 
 use nonmax::NonMaxU32;
 use oxc_index::{Idx, IndexVec};
@@ -73,27 +73,23 @@ pub struct SourcemapBuilder {
     generated_column: u32,
 }
 
-impl Default for SourcemapBuilder {
-    fn default() -> Self {
+impl SourcemapBuilder {
+    pub fn new(path: &Path, source_text: &str) -> Self {
+        let mut sourcemap_builder = oxc_sourcemap::SourceMapBuilder::default();
+        let line_offset_tables = Self::generate_line_offset_tables(source_text);
+        let source_id =
+            sourcemap_builder.set_source_and_content(path.to_string_lossy().as_ref(), source_text);
         Self {
-            source_id: 0,
-            original_source: "".into(),
+            source_id,
+            original_source: Arc::from(source_text),
             last_generated_update: 0,
             last_position: None,
-            line_offset_tables: LineOffsetTables::default(),
-            sourcemap_builder: oxc_sourcemap::SourceMapBuilder::default(),
+            line_offset_tables,
+            sourcemap_builder,
             generated_line: 0,
             generated_column: 0,
         }
     }
-}
-
-impl SourcemapBuilder {
-    pub fn with_name_and_source(&mut self, name: &str, source: &str) {
-        self.line_offset_tables = Self::generate_line_offset_tables(source);
-        self.source_id = self.sourcemap_builder.set_source_and_content(name, source);
-        self.original_source = source.into();
-    }
 
     pub fn into_sourcemap(self) -> oxc_sourcemap::SourceMap {
         self.sourcemap_builder.into_sourcemap()
@@ -392,8 +388,7 @@ mod test {
     }
 
     fn assert_mapping(source: &str, mappings: &[(u32, u32, u32)]) {
-        let mut builder = SourcemapBuilder::default();
-        builder.with_name_and_source("x.js", source);
+        let mut builder = SourcemapBuilder::new(Path::new("x.js"), source);
         for (position, expected_line, expected_col) in mappings.iter().copied() {
             let (line, col) = builder.search_original_line_and_column(position);
             assert_eq!(
@@ -407,8 +402,7 @@ mod test {
     #[test]
     fn add_source_mapping() {
         fn create_mappings(source: &str, line: u32, column: u32) {
-            let mut builder = SourcemapBuilder::default();
-            builder.with_name_and_source("x.js", source);
+            let mut builder = SourcemapBuilder::new(Path::new("x.js"), source);
             let output: Vec<u8> = source.as_bytes().into();
             for (i, _ch) in source.char_indices() {
                 #[allow(clippy::cast_possible_truncation)]
@@ -444,8 +438,7 @@ mod test {
     #[test]
     fn add_source_mapping_for_name() {
         let output = "ac".as_bytes();
-        let mut builder = SourcemapBuilder::default();
-        builder.with_name_and_source("x.js", "ab");
+        let mut builder = SourcemapBuilder::new(Path::new("x.js"), "ab");
         builder.add_source_mapping_for_name(output, Span::new(0, 1), "a");
         builder.add_source_mapping_for_name(output, Span::new(1, 2), "c");
         let sm = builder.into_sourcemap();
@@ -464,8 +457,7 @@ mod test {
     #[test]
     fn add_source_mapping_for_unordered_position() {
         let output = "".as_bytes();
-        let mut builder = SourcemapBuilder::default();
-        builder.with_name_and_source("x.js", "ab");
+        let mut builder = SourcemapBuilder::new(Path::new("x.js"), "ab");
         builder.add_source_mapping(output, 1, None);
         builder.add_source_mapping(output, 0, None);
         let sm = builder.into_sourcemap();
diff --git a/napi/transform/src/isolated_declaration.rs b/napi/transform/src/isolated_declaration.rs
@@ -4,7 +4,7 @@ use napi_derive::napi;
 
 use oxc::{
     allocator::Allocator,
-    codegen::CodeGenerator,
+    codegen::{CodeGenerator, CodegenOptions},
     isolated_declarations::IsolatedDeclarations,
     napi::{
         isolated_declarations::{IsolatedDeclarationsOptions, IsolatedDeclarationsResult},
@@ -39,11 +39,12 @@ pub fn isolated_declaration(
     )
     .build(&ret.program);
 
-    let mut codegen = CodeGenerator::new();
-    if options.sourcemap == Some(true) {
-        codegen = codegen.enable_source_map(&filename, &source_text);
-    }
-    let codegen_ret = codegen.build(&transformed_ret.program);
+    let codegen_ret = CodeGenerator::new()
+        .with_options(CodegenOptions {
+            source_map_path: Some(source_path.to_path_buf()),
+            ..CodegenOptions::default()
+        })
+        .build(&transformed_ret.program);
 
     let errors = ret.errors.into_iter().chain(transformed_ret.errors).collect();
     let errors = wrap_diagnostics(source_path, source_type, &source_text, errors);
diff --git a/tasks/benchmark/benches/codegen.rs b/tasks/benchmark/benches/codegen.rs
@@ -1,6 +1,8 @@
+use std::path::PathBuf;
+
 use oxc_allocator::Allocator;
 use oxc_benchmark::{criterion_group, criterion_main, BenchmarkId, Criterion};
-use oxc_codegen::CodeGenerator;
+use oxc_codegen::{CodeGenerator, CodegenOptions};
 use oxc_parser::Parser;
 use oxc_span::SourceType;
 use oxc_tasks_common::TestFiles;
@@ -22,7 +24,12 @@ fn bench_codegen(criterion: &mut Criterion) {
         let mut group = criterion.benchmark_group("codegen_sourcemap");
         group.bench_with_input(id, &ret.program, |b, program| {
             b.iter_with_large_drop(|| {
-                CodeGenerator::new().enable_source_map(&file.file_name, source_text).build(program)
+                CodeGenerator::new()
+                    .with_options(CodegenOptions {
+                        source_map_path: Some(PathBuf::from(&file.file_name)),
+                        ..CodegenOptions::default()
+                    })
+                    .build(program)
             });
         });
         group.finish();
diff --git a/tasks/benchmark/benches/sourcemap.rs b/tasks/benchmark/benches/sourcemap.rs
@@ -1,6 +1,8 @@
+use std::path::PathBuf;
+
 use oxc_allocator::Allocator;
 use oxc_benchmark::{criterion_group, criterion_main, BenchmarkId, Criterion};
-use oxc_codegen::{CodeGenerator, CodegenReturn};
+use oxc_codegen::{CodeGenerator, CodegenOptions, CodegenReturn};
 use oxc_parser::Parser;
 use oxc_sourcemap::ConcatSourceMapBuilder;
 use oxc_span::SourceType;
@@ -18,13 +20,19 @@ fn bench_sourcemap(criterion: &mut Criterion) {
             let ret = Parser::new(&allocator, source_text, source_type).parse();
 
             let CodegenReturn { code: output_txt, .. } = CodeGenerator::new()
-                .enable_source_map(file.file_name.as_str(), source_text)
+                .with_options(CodegenOptions {
+                    source_map_path: Some(PathBuf::from(&file.file_name)),
+                    ..CodegenOptions::default()
+                })
                 .build(&ret.program);
             let lines = output_txt.matches('\n').count() as u32;
 
             b.iter(|| {
                 let CodegenReturn { map, .. } = CodeGenerator::new()
-                    .enable_source_map(file.file_name.as_str(), source_text)
+                    .with_options(CodegenOptions {
+                        source_map_path: Some(PathBuf::from(&file.file_name)),
+                        ..CodegenOptions::default()
+                    })
                     .build(&ret.program);
                 if let Some(sourcemap) = map {
                     let concat_sourcemap_builder = ConcatSourceMapBuilder::from_sourcemaps(&[

Original file line number	Diff line number	Diff line change
`@@ -283,11 +283,10 @@ pub trait CompilerInterface {`
`283`	`283`	`mangler: Option<Mangler>,`
`284`	`284`	`options: CodegenOptions,`
`285`	`285`	`) -> CodegenReturn {`
`286`		`- let mut codegen = CodeGenerator::new().with_options(options).with_mangler(mangler);`
	`286`	`+ let mut options = options;`
`287`	`287`	`if self.enable_sourcemap() {`
`288`		`- codegen = codegen`
`289`		`- .enable_source_map(source_path.to_string_lossy().as_ref(), program.source_text);`
	`288`	`+ options.source_map_path = Some(source_path.to_path_buf());`
`290`	`289`	`}`
`291`		`- codegen.build(program)`
	`290`	`+ CodeGenerator::new().with_options(options).with_mangler(mangler).build(program)`
`292`	`291`	`}`
`293`	`292`	`}`