mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
fix(codegen): preserve names in duplicate source mappings (#26051)
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<br/>unnamed] --> B[Token at source position P<br/>name: none]
C[Identifier mapping<br/>name: longName] --> D{Same source position P?}
D -->|before| E[Drop duplicate<br/>names: empty]
D -->|after| F[Enrich existing token<br/>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)
```
This commit is contained in:
1 parent
406bc50d1d
commit
3fd78d7451
6 files changed
+141
-11
No files matched your search
Generated
+1
@@ -1887,6 +1887,7 @@ dependencies = [
|
||||
"oxc_data_structures",
|
||||
"oxc_ecmascript",
|
||||
"oxc_index",
|
||||
"oxc_mangler",
|
||||
"oxc_parser",
|
||||
"oxc_semantic",
|
||||
"oxc_sourcemap",
|
||||
|
||||
@@ -42,6 +42,7 @@ smallvec = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
insta = { workspace = true }
|
||||
oxc_mangler = { workspace = true }
|
||||
oxc_parser = { workspace = true }
|
||||
pico-args = { workspace = true }
|
||||
|
||||
|
||||
@@ -54,6 +54,7 @@ pub struct SourcemapBuilder<'a> {
|
||||
tokens: Vec<oxc_sourcemap::Token>,
|
||||
last_generated_update: usize,
|
||||
last_position: Option<u32>,
|
||||
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::<Vec<_>>();
|
||||
assert_eq!(tokens, [(0, 0, 0, 0, Some("first")), (1, 0, 0, 6, Some("second"))]);
|
||||
assert_eq!(sm.get_names().collect::<Vec<_>>(), ["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.
|
||||
|
||||
@@ -262,3 +262,37 @@ make().prop;
|
||||
Error
|
||||
at Object.get prop (/project/input.js:4:15)
|
||||
at <anonymous> (/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)
|
||||
@@ -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::<Vec<_>>().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::<Vec<_>>().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<String>) {
|
||||
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();
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in new issue
Block a user