diff --git a/Cargo.lock b/Cargo.lock index b148c22685..a400c3a36a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1887,6 +1887,7 @@ dependencies = [ "oxc_data_structures", "oxc_ecmascript", "oxc_index", + "oxc_mangler", "oxc_parser", "oxc_semantic", "oxc_sourcemap", diff --git a/crates/oxc_codegen/Cargo.toml b/crates/oxc_codegen/Cargo.toml index 74f280ca1b..2957d902f6 100644 --- a/crates/oxc_codegen/Cargo.toml +++ b/crates/oxc_codegen/Cargo.toml @@ -42,6 +42,7 @@ smallvec = { workspace = true } [dev-dependencies] insta = { workspace = true } +oxc_mangler = { workspace = true } oxc_parser = { workspace = true } pico-args = { workspace = true } diff --git a/crates/oxc_codegen/src/sourcemap_builder.rs b/crates/oxc_codegen/src/sourcemap_builder.rs index ef3c2bef36..3cda644143 100644 --- a/crates/oxc_codegen/src/sourcemap_builder.rs +++ b/crates/oxc_codegen/src/sourcemap_builder.rs @@ -54,6 +54,7 @@ pub struct SourcemapBuilder<'a> { tokens: Vec, last_generated_update: usize, last_position: Option, + last_token_has_name: bool, line_offset_tables: LineOffsetTables, generated_line: u32, generated_column: u32, @@ -76,6 +77,7 @@ impl<'a> SourcemapBuilder<'a> { tokens: Vec::with_capacity(source_len / 4), last_generated_update: 0, last_position: None, + last_token_has_name: false, line_offset_tables, generated_line: 0, generated_column: 0, @@ -137,6 +139,21 @@ impl<'a> SourcemapBuilder<'a> { pub fn add_source_mapping(&mut self, output: &[u8], position: u32, name: Option<&'a str>) { if self.last_position == Some(position) { + if let Some(name) = name + && !self.last_token_has_name + { + let name_id = self.add_name(name); + let token = self.tokens.last_mut().unwrap(); + *token = oxc_sourcemap::Token::new( + token.get_dst_line(), + token.get_dst_col(), + token.get_src_line(), + token.get_src_col(), + token.get_source_id(), + Some(name_id), + ); + self.last_token_has_name = true; + } return; } let (original_line, original_column) = self.search_original_line_and_column(position); @@ -151,6 +168,7 @@ impl<'a> SourcemapBuilder<'a> { name_id, )); self.last_position = Some(position); + self.last_token_has_name = name_id.is_some(); } fn add_name(&mut self, name: &'a str) -> u32 { @@ -663,6 +681,34 @@ mod test { ); } + #[test] + fn duplicate_source_mappings_preserve_names_and_coordinates() { + let mut builder = SourcemapBuilder::new(Path::new("x.js"), "first second"); + builder.add_source_mapping(b"", 0, None); + builder.add_source_mapping(b"(a", 0, Some("first")); + builder.add_source_mapping(b"(a", 0, Some("ignored")); + builder.add_source_mapping(b"(a\n", 6, None); + builder.add_source_mapping(b"(a\nb", 6, Some("second")); + builder.add_source_mapping(b"(a\nb", 6, None); + builder.add_source_mapping(b"(a\nb", 6, Some("ignored")); + + let sm = builder.into_sourcemap(); + let tokens = sm + .get_source_view_tokens() + .map(|token| { + ( + token.get_dst_line(), + token.get_dst_col(), + token.get_src_line(), + token.get_src_col(), + token.get_name(), + ) + }) + .collect::>(); + assert_eq!(tokens, [(0, 0, 0, 0, Some("first")), (1, 0, 0, 6, Some("second"))]); + assert_eq!(sm.get_names().collect::>(), ["first", "second"]); + } + #[test] fn add_source_mapping_for_name_from_private_identifier() { // A transform can print a plain identifier where the source had a private one. diff --git a/crates/oxc_codegen/tests/integration/snapshots/stacktrace_is_correct.snap b/crates/oxc_codegen/tests/integration/snapshots/stacktrace_is_correct.snap index 0eb6a666bc..ae4292b0fc 100644 --- a/crates/oxc_codegen/tests/integration/snapshots/stacktrace_is_correct.snap +++ b/crates/oxc_codegen/tests/integration/snapshots/stacktrace_is_correct.snap @@ -262,3 +262,37 @@ make().prop; Error at Object.get prop (/project/input.js:4:15) at (/project/input.js:7:7) + +------------------------------------------------------ +## Mangled identifier source-map names +["longFunctionName", "longName"] + +## Input +export {}; +function longFunctionName(longName) { + (longName = () => { + Error.stackTraceLimit = 2; + throw new Error(); + })(); +} +longFunctionName(0); + +## Output +export {}; +function e(e) { + (e = () => { + Error.stackTraceLimit = 2; + throw new Error(); + })(); +} +e(0); + + +## Stderr +/project/input.js:6 + throw new Error(); + ^ + +Error + at longName (/project/input.js:6:15) + at e (/project/input.js:7:7) diff --git a/crates/oxc_codegen/tests/integration/sourcemap.rs b/crates/oxc_codegen/tests/integration/sourcemap.rs index 4a96a29db7..7504c0d77f 100644 --- a/crates/oxc_codegen/tests/integration/sourcemap.rs +++ b/crates/oxc_codegen/tests/integration/sourcemap.rs @@ -5,6 +5,7 @@ use oxc_allocator::Allocator; use oxc_ast::ast::{Expression, Statement}; use oxc_codegen::{Codegen, CodegenOptions}; use oxc_index::IndexVec; +use oxc_mangler::{MangleOptions, Mangler}; use oxc_parser::Parser; use oxc_span::{SourceType, Span}; use oxc_str::CompactStr; @@ -440,6 +441,22 @@ const make = () => ({ make().prop", ]; + // The identifier is first mapped by its enclosing AssignmentExpression. The map must retain + // the later identifier mapping's original name so Node can restore the inferred function name. + // `export {}` makes this a module, allowing the enclosing function to be mangled as well. + let mangled_case = r" +export {}; +function longFunctionName(longName) { + (longName = () => { + Error.stackTraceLimit = 2; + throw new Error(); + })(); +} +longFunctionName(0); +"; + let (mangled_output, mangled_sourcemap_url, mangled_names) = codegen_mangled(mangled_case); + assert_eq!(mangled_names, ["longFunctionName", "longName"]); + let node_version = std::process::Command::new("node").arg("--version").output().map_or_else( |_| "unknown".to_string(), |output| String::from_utf8_lossy(&output.stdout).trim().to_string(), @@ -454,7 +471,13 @@ make().prop", cases.iter().map(|s| { let (output, sourcemap_url) = codegen(s); format!("## Input\n{}\n\n## Output\n{}\n\n## Stderr\n{}", s, output, execute_with_node(&output, &sourcemap_url)) - }).collect::>().join("\n------------------------------------------------------\n") + }).chain(std::iter::once(format!( + "## Mangled identifier source-map names\n{:?}\n\n## Input\n{}\n\n## Output\n{}\n\n## Stderr\n{}", + mangled_names, + mangled_case.trim(), + mangled_output, + execute_with_node(&mangled_output, &mangled_sourcemap_url), + ))).collect::>().join("\n------------------------------------------------------\n") ) ); }); @@ -511,6 +534,27 @@ fn private_identifier_is_named_only_when_mangled() { assert_eq!(mangled_private_names(source_text, &[("ab", "a")]), vec!["#ab".to_string()]); } +fn codegen_mangled(code: &str) -> (String, String, Vec) { + let allocator = Allocator::default(); + let ret = Parser::new(&allocator, code, SourceType::mjs()).parse(); + assert!(ret.diagnostics.is_empty()); + let program = ret.program; + let mangler = Mangler::new() + .with_options(MangleOptions { top_level: Some(true), ..MangleOptions::default() }) + .build(&program); + let ret = Codegen::new() + .with_options(CodegenOptions { + source_map_path: Some(PathBuf::from("input.js")), + ..Default::default() + }) + .with_scoping(Some(mangler.scoping)) + .with_private_member_mappings(Some(mangler.class_private_mappings)) + .build(&program); + let map = ret.map.unwrap(); + let names = map.get_names().map(str::to_owned).collect(); + (ret.code, map.to_data_url(), names) +} + fn execute_with_node(code: &str, sourcemap_url: &str) -> String { let cwd = env::current_dir().unwrap().join("input.js"); let cwd = cwd.to_str().unwrap(); diff --git a/packages/codegen/src-js/print/write.ts b/packages/codegen/src-js/print/write.ts index cc61a1c483..85f6d3b1b3 100644 --- a/packages/codegen/src-js/print/write.ts +++ b/packages/codegen/src-js/print/write.ts @@ -518,12 +518,14 @@ function markMapNamed( const { sourceText } = state; if (start > sourceText.length) return; - // `oxc_codegen` suppresses consecutive source positions as it records them. Do this before - // recovering a name or retaining the mapping, since member-level marks commonly duplicate keys. + // `oxc_codegen` suppresses consecutive source positions as it records them. A named child can + // still enrich its unnamed parent mapping, but a mapping which already has a name is complete. // On the first mapping the read is index -1, which on a typed array is `undefined` - never equal. let { mapPositions } = state; - const { mapPositionsLen } = state; - if (mapPositions[mapPositionsLen - 1] === start) return; + const { mapNames, mapPositionsLen } = state; + const mappingIndex = mapPositionsLen >> 1; + const duplicatePosition = mapPositions[mapPositionsLen - 1] === start; + if (duplicatePosition && mapNames[mapNames.length - 2] === mappingIndex - 1) return; // A mapping carries a name only when the identifier printed differs from the one in the source. // When possible, the mapping records the name from source, but if the source range is invalid, @@ -563,17 +565,19 @@ function markMapNamed( // A transformed or hand-authored AST can carry ranges unrelated to `sourceText`. // Preserve the existing fallback in that case instead of recording an arbitrary source substring. if (originalName === undefined || !isSameToken(originalName, printedName, hashLength)) { - state.mapNames.push( - mapPositionsLen >> 1, + mapNames.push( + duplicatePosition ? mappingIndex - 1 : mappingIndex, originalName === undefined ? printedName : originalName, ); } } - if (mapPositionsLen === mapPositions.length) mapPositions = state.growMapPositions(); - mapPositions[mapPositionsLen] = state.spilledOutputLength + state.output.length; - mapPositions[mapPositionsLen + 1] = start; - state.mapPositionsLen = mapPositionsLen + 2; + if (!duplicatePosition) { + if (mapPositionsLen === mapPositions.length) mapPositions = state.growMapPositions(); + mapPositions[mapPositionsLen] = state.spilledOutputLength + state.output.length; + mapPositions[mapPositionsLen + 1] = start; + state.mapPositionsLen = mapPositionsLen + 2; + } } /**