From 5139974dc55e9a905b7d80daa447c97bc455a511 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 9 Mar 2026 20:31:38 +0800 Subject: [PATCH] fix: populate source map in transformAngularFile when sourcemap option is enabled The edit-based transform pipeline never generated source maps, leaving `TransformResult.map` as `None` even when `sourcemap: true`. Add `apply_edits_with_sourcemap` that produces a source map by analyzing which original byte ranges survive in the output, and wire it into the AOT, JIT, and no-Angular-classes code paths. - Close https://github.com/voidzero-dev/oxc-angular-compiler/issues/99 Co-Authored-By: Claude Opus 4.6 --- .../src/component/transform.rs | 29 ++- .../oxc_angular_compiler/src/optimizer/mod.rs | 193 ++++++++++++++++++ .../tests/integration_test.rs | 147 +++++++++++++ 3 files changed, 363 insertions(+), 6 deletions(-) diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index 6c1516459..954ad387b 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -15,7 +15,7 @@ use oxc_parser::Parser; use oxc_span::{Atom, GetSpan, SourceType, Span}; use rustc_hash::FxHashMap; -use crate::optimizer::{Edit, apply_edits}; +use crate::optimizer::{Edit, apply_edits, apply_edits_with_sourcemap}; #[cfg(feature = "cross_file_elision")] use super::cross_file_elision::CrossFileAnalyzer; @@ -1126,7 +1126,7 @@ fn transform_angular_file_jit( allocator: &Allocator, path: &str, source: &str, - _options: &TransformOptions, + options: &TransformOptions, ) -> TransformResult { let mut result = TransformResult::new(); @@ -1213,7 +1213,13 @@ fn transform_angular_file_jit( if jit_classes.is_empty() { // No Angular classes found, return source as-is - result.code = source.to_string(); + if options.sourcemap { + let (code, map) = apply_edits_with_sourcemap(source, vec![], path); + result.code = code; + result.map = map; + } else { + result.code = source.to_string(); + } return result; } @@ -1359,7 +1365,13 @@ fn transform_angular_file_jit( } // Apply all edits - result.code = apply_edits(source, edits); + if options.sourcemap { + let (code, map) = apply_edits_with_sourcemap(source, edits, path); + result.code = code; + result.map = map; + } else { + result.code = apply_edits(source, edits); + } result } @@ -2125,8 +2137,13 @@ pub fn transform_angular_file( } // Apply all edits in one pass - result.code = apply_edits(source, edits); - result.map = None; + if options.sourcemap { + let (code, map) = apply_edits_with_sourcemap(source, edits, path); + result.code = code; + result.map = map; + } else { + result.code = apply_edits(source, edits); + } result } diff --git a/crates/oxc_angular_compiler/src/optimizer/mod.rs b/crates/oxc_angular_compiler/src/optimizer/mod.rs index 3d5858d5a..854b87582 100644 --- a/crates/oxc_angular_compiler/src/optimizer/mod.rs +++ b/crates/oxc_angular_compiler/src/optimizer/mod.rs @@ -229,6 +229,199 @@ pub fn apply_edits(code: &str, mut edits: Vec) -> String { result } +/// Apply edits to source code and generate a source map. +/// +/// Uses the same edit-application algorithm as `apply_edits`, then generates +/// a source map by finding where unchanged source segments appear in the +/// actual output — guaranteeing the sourcemap is consistent with the output +/// regardless of edit ordering. +pub fn apply_edits_with_sourcemap( + code: &str, + edits: Vec, + filename: &str, +) -> (String, Option) { + // Generate the output using the existing algorithm + let output = apply_edits(code, edits.clone()); + + // Generate sourcemap by finding unchanged source segments in the actual output + let map = generate_sourcemap_from_edits(code, &output, edits, filename); + (output, Some(map)) +} + +/// Generate a source map by finding unchanged source segments in the actual output. +/// +/// Instead of independently modeling how edits transform positions (which could +/// diverge from `apply_edits`'s reverse-order mutating algorithm), this function: +/// 1. Computes which source byte ranges are untouched by any edit +/// 2. Locates each unchanged segment in the actual output string +/// 3. Generates identity mappings for those segments +/// +/// This guarantees the sourcemap is always consistent with the actual output. +fn generate_sourcemap_from_edits( + source: &str, + output: &str, + edits: Vec, + filename: &str, +) -> String { + let mut builder = oxc_sourcemap::SourceMapBuilder::default(); + builder.set_source_and_content(filename, source); + + if edits.is_empty() { + // Identity mapping — every line maps 1:1 + add_line_mappings_for_segment(&mut builder, source, 0, 0, 0, 0); + return builder.into_sourcemap().to_json_string(); + } + + // 1. Collect all edit boundary positions. + // Every edit start/end position is a point where the output may differ from + // the source. We need to split unchanged ranges at ALL edit positions — + // including pure insertions (start == end) — because insertions embed new + // text within what would otherwise be a contiguous source segment, breaking + // `find(segment)` in step 3. + let code_len = source.len() as u32; + let mut boundary_points: Vec = Vec::new(); + let mut deleted_ranges: Vec<(u32, u32)> = Vec::new(); + + for edit in &edits { + if edit.start > code_len || edit.end > code_len || edit.start > edit.end { + continue; + } + boundary_points.push(edit.start); + boundary_points.push(edit.end); + if edit.start < edit.end { + deleted_ranges.push((edit.start, edit.end)); + } + } + + boundary_points.push(0); + boundary_points.push(code_len); + boundary_points.sort_unstable(); + boundary_points.dedup(); + + // Merge overlapping deleted ranges for quick overlap checks + deleted_ranges.sort_by_key(|r| r.0); + let mut merged_deleted: Vec<(u32, u32)> = Vec::new(); + for (s, e) in deleted_ranges { + if let Some(last) = merged_deleted.last_mut() { + if s <= last.1 { + last.1 = last.1.max(e); + continue; + } + } + merged_deleted.push((s, e)); + } + + // 2. Compute unchanged source sub-ranges. + // A sub-range [boundary[i], boundary[i+1]) is unchanged if it doesn't + // overlap with any deletion range. + let mut unchanged: Vec<(u32, u32)> = Vec::new(); + for window in boundary_points.windows(2) { + let (start, end) = (window[0], window[1]); + if start >= end { + continue; + } + // Check if this sub-range overlaps with any merged deletion + let overlaps = merged_deleted.iter().any(|(del_s, del_e)| start < *del_e && end > *del_s); + if !overlaps { + unchanged.push((start, end)); + } + } + + // 3. Compute the output byte offset for each unchanged segment and generate mappings. + // Instead of using string search (which can false-match replacement text for + // short segments like `}`), we compute the exact output position using the + // edit shift formula: + // output_pos(S) = S + Σ (replacement.len() - (end - start)) + // for all edits where end <= S + // This is exact for non-overlapping edits. + + // Precompute edit shifts sorted by end position for efficient prefix-sum lookup + let mut edit_shifts: Vec<(u32, i64)> = edits + .iter() + .filter(|e| e.start <= code_len && e.end <= code_len && e.start <= e.end) + .map(|e| (e.end, e.replacement.len() as i64 - (e.end as i64 - e.start as i64))) + .collect(); + edit_shifts.sort_by_key(|(end, _)| *end); + + for (src_start, src_end) in &unchanged { + let segment = &source[*src_start as usize..*src_end as usize]; + if segment.is_empty() { + continue; + } + // Compute output byte offset: src_start + net shift from all edits ending at or before src_start + let net_shift: i64 = edit_shifts + .iter() + .take_while(|(end, _)| *end <= *src_start) + .map(|(_, shift)| shift) + .sum(); + let output_byte_pos = (*src_start as i64 + net_shift) as usize; + + debug_assert!( + output_byte_pos + segment.len() <= output.len() + && &output[output_byte_pos..output_byte_pos + segment.len()] == segment, + "Sourcemap: computed output position {output_byte_pos} does not match \ + segment {:?} (src {}..{})", + &segment[..segment.len().min(20)], + src_start, + src_end, + ); + + let (src_line, src_col) = byte_offset_to_line_col_utf16(source, *src_start as usize); + let (out_line, out_col) = byte_offset_to_line_col_utf16(output, output_byte_pos); + add_line_mappings_for_segment(&mut builder, segment, out_line, out_col, src_line, src_col); + } + + builder.into_sourcemap().to_json_string() +} + +/// Compute line and column (UTF-16 code units) for a byte offset in a string. +/// +/// Source map columns must be in UTF-16 code units per the spec and `oxc_sourcemap` +/// convention. For ASCII this equals byte offset; for multi-byte characters +/// (e.g., `ɵ` U+0275 = 2 UTF-8 bytes but 1 UTF-16 code unit) the values differ. +fn byte_offset_to_line_col_utf16(source: &str, offset: usize) -> (u32, u32) { + let mut line: u32 = 0; + let mut col: u32 = 0; + for (i, ch) in source.char_indices() { + if i >= offset { + break; + } + if ch == '\n' { + line += 1; + col = 0; + } else { + col += ch.len_utf16() as u32; + } + } + (line, col) +} + +/// Add source map mappings for an unchanged segment of source code. +/// +/// Adds a mapping at the start of the segment and at the beginning of each new line. +fn add_line_mappings_for_segment( + builder: &mut oxc_sourcemap::SourceMapBuilder, + segment: &str, + mut out_line: u32, + mut out_col: u32, + mut src_line: u32, + mut src_col: u32, +) { + // Add mapping at the start of this segment + builder.add_token(out_line, out_col, src_line, src_col, Some(0), None); + + for ch in segment.chars() { + if ch == '\n' { + out_line += 1; + out_col = 0; + src_line += 1; + src_col = 0; + // Add mapping at the start of each new line + builder.add_token(out_line, out_col, src_line, src_col, Some(0), None); + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index c13568deb..16c93e0dd 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -6356,3 +6356,150 @@ export class TestComponent { insta::assert_snapshot!("jit_union_type_ctor_params", result.code); } + +// ========================================================================= +// Source map tests +// ========================================================================= + +#[test] +fn test_sourcemap_aot_mode() { + // Issue #99: transformAngularFile should return a source map when sourcemap: true + let allocator = Allocator::default(); + let source = r#"import { Component } from '@angular/core'; + +@Component({ + selector: 'app-test', + template: '

Hello World

', + standalone: true, +}) +export class TestComponent { +} +"#; + + let options = ComponentTransformOptions { sourcemap: true, ..Default::default() }; + + let result = transform_angular_file(&allocator, "app.component.ts", source, &options, None); + + assert!( + result.map.is_some(), + "AOT mode should return a source map when sourcemap: true, but map was None" + ); + + let map = result.map.unwrap(); + // Verify it's valid JSON + assert!( + map.starts_with('{'), + "Source map should be valid JSON, got: {}", + &map[..50.min(map.len())] + ); + // Verify it contains expected sourcemap fields + assert!(map.contains("\"version\":3"), "Source map should have version 3"); + assert!(map.contains("\"mappings\""), "Source map should have mappings"); + assert!(map.contains("app.component.ts"), "Source map should reference the source file"); +} + +#[test] +fn test_sourcemap_jit_mode() { + // Issue #99: JIT mode should also return a source map when sourcemap: true + let allocator = Allocator::default(); + let source = r#"import { Component } from '@angular/core'; + +@Component({ + selector: 'app-test', + template: '

Hello World

', + standalone: true, +}) +export class TestComponent { +} +"#; + + let options = ComponentTransformOptions { sourcemap: true, jit: true, ..Default::default() }; + + let result = transform_angular_file(&allocator, "app.component.ts", source, &options, None); + + assert!( + result.map.is_some(), + "JIT mode should return a source map when sourcemap: true, but map was None" + ); + + let map = result.map.unwrap(); + assert!(map.starts_with('{'), "Source map should be valid JSON"); + assert!(map.contains("\"version\":3"), "Source map should have version 3"); +} + +#[test] +fn test_sourcemap_disabled_by_default() { + // When sourcemap is false (default), map should be None + let allocator = Allocator::default(); + let source = r#"import { Component } from '@angular/core'; + +@Component({ + selector: 'app-test', + template: '

Hello

', + standalone: true, +}) +export class TestComponent { +} +"#; + + let result = transform_angular_file( + &allocator, + "app.component.ts", + source, + &ComponentTransformOptions::default(), + None, + ); + + assert!(result.map.is_none(), "Source map should be None when sourcemap option is false"); +} + +#[test] +fn test_sourcemap_with_external_template() { + // Source map should work with resolved external templates + let allocator = Allocator::default(); + let source = r#"import { Component } from '@angular/core'; + +@Component({ + selector: 'app-test', + templateUrl: './app.html', + standalone: true, +}) +export class TestComponent { +} +"#; + + let mut templates = std::collections::HashMap::new(); + templates.insert("./app.html".to_string(), "

Hello World

".to_string()); + let resolved = ResolvedResources { templates, styles: std::collections::HashMap::new() }; + + let options = ComponentTransformOptions { sourcemap: true, ..Default::default() }; + + let result = + transform_angular_file(&allocator, "app.component.ts", source, &options, Some(&resolved)); + + assert!( + result.map.is_some(), + "AOT with external template should return a source map when sourcemap: true" + ); +} + +#[test] +fn test_sourcemap_no_angular_classes() { + // A file with no Angular classes should still return a source map if requested + let allocator = Allocator::default(); + let source = r#"export class PlainService { + getData() { return 42; } +} +"#; + + let options = ComponentTransformOptions { sourcemap: true, ..Default::default() }; + + let result = transform_angular_file(&allocator, "plain.ts", source, &options, None); + + // Even for files with no Angular components, if sourcemap is requested, + // a trivial identity source map should be returned + assert!( + result.map.is_some(), + "Should return a source map even for files with no Angular classes when sourcemap: true" + ); +}