From 3fd78d74514dc6b631dc6098cd5d3bcefa090aba Mon Sep 17 00:00:00 2001 From: camc314 <18101008+camc314@users.noreply.github.com> Date: Tue, 3 Nov 2026 02:22:04 +0000 Subject: [PATCH] fix(codegen): preserve names in duplicate source mappings (#26051) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an unnamed parent node and a mangled identifier begin at the same source position, codegen deduplicated the identifier mapping. This retained location data but discarded the original identifier name, leaving consumers unable to restore inferred function names in source-mapped Node stack traces. ```mermaid flowchart LR A[AssignmentExpression mapping
unnamed] --> B[Token at source position P
name: none] C[Identifier mapping
name: longName] --> D{Same source position P?} D -->|before| E[Drop duplicate
names: empty] D -->|after| F[Enrich existing token
name: longName] ``` ### Changes - Cache whether the last emitted token has a name, keeping duplicate-position handling cheap in the source-map builder hot path. - Enrich an unnamed duplicate token when a later identifier mapping supplies the original name. - Add a unit regression test for the builder behavior. - Extend the existing Node source-map stack-trace snapshot test with a mangled ES module fixture. The Node fixture verifies: ```text function longFunctionName(longName) { ... } ↓ mangled function e(e) { ... } ``` and asserts that the source map includes both original names while Node restores the inferred inner frame as: ```text at longName (/project/input.js:6:15) ``` --- Cargo.lock | 1 + crates/oxc_codegen/Cargo.toml | 1 + crates/oxc_codegen/src/sourcemap_builder.rs | 46 +++++++++++++++++++ .../snapshots/stacktrace_is_correct.snap | 34 ++++++++++++++ .../tests/integration/sourcemap.rs | 46 ++++++++++++++++++- packages/codegen/src-js/print/write.ts | 24 ++++++---- 6 files changed, 141 insertions(+), 11 deletions(-) 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; + } } /**