From 91c87dd380c4ccd35c0040fff56349b17ef33b6d Mon Sep 17 00:00:00 2001 From: Boshen <1430279+Boshen@users.noreply.github.com> Date: Sat, 12 Oct 2024 04:56:43 +0000 Subject: [PATCH] refactor(codegen)!: remove `Codegen::enableSourceMap` API (#6452) --- crates/oxc/src/compiler.rs | 7 ++-- crates/oxc_codegen/examples/sourcemap.rs | 7 ++-- crates/oxc_codegen/src/lib.rs | 29 +++++++++------- crates/oxc_codegen/src/sourcemap_builder.rs | 38 ++++++++------------- napi/transform/src/isolated_declaration.rs | 13 +++---- tasks/benchmark/benches/codegen.rs | 11 ++++-- tasks/benchmark/benches/sourcemap.rs | 14 ++++++-- 7 files changed, 67 insertions(+), 52 deletions(-) diff --git a/crates/oxc/src/compiler.rs b/crates/oxc/src/compiler.rs index 8e789d05b36f7..8c95baf044340 100644 --- a/crates/oxc/src/compiler.rs +++ b/crates/oxc/src/compiler.rs @@ -283,11 +283,10 @@ pub trait CompilerInterface { mangler: Option, 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 index 8518287972b3f..78b16aafc1629 100644 --- 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 index fc0a1f83f633f..1ca4269e1852c 100644 --- 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, } 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, } @@ -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) -> 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 index 31ddb113d7588..a520ef9c3b738 100644 --- 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 = 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 index 89645cebcfea0..88a0df50840ac 100644 --- 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 index f7fe082b1ec49..c2e327e2c008f 100644 --- 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 index a5fcef03c201b..12110b9847b62 100644 --- 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(&[