Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions crates/oxc_angular_compiler/src/ir/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1228,8 +1228,9 @@ pub struct RepeaterVarNames<'a> {
pub item: Option<Atom<'a>>,
/// Alias for $count.
pub count: Option<Atom<'a>>,
/// Alias for $index.
pub index: Option<Atom<'a>>,
/// Aliases for $index (can be multiple, e.g. `let i = $index, j = $index`).
/// Angular stores these in a `Set<string>`.
pub index: Vec<'a, Atom<'a>>,
/// Alias for $first.
pub first: Option<Atom<'a>>,
/// Alias for $last.
Expand Down
5 changes: 3 additions & 2 deletions crates/oxc_angular_compiler/src/pipeline/ingest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2502,10 +2502,11 @@ fn ingest_for_block<'a>(
});

// Build var_names for the repeater, tracking user-defined aliases
#[allow(clippy::needless_update)]
let mut var_names = RepeaterVarNames {
item: Some(for_block.item.name.clone()),
count: None,
index: None,
index: oxc_allocator::Vec::new_in(allocator),
first: None,
last: None,
even: None,
Expand All @@ -2525,7 +2526,7 @@ fn ingest_for_block<'a>(
// This is used for track expression variable replacement
for var in for_block.context_variables.iter() {
if var.value.as_str() == "$index" {
var_names.index = Some(var.name.clone());
var_names.index.push(var.name.clone());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,7 @@ pub fn generate_track_variables(job: &mut ComponentCompilationJob<'_>) {
if let CreateOp::RepeaterCreate(rep) = op {
// Get $index names and $implicit name for this repeater
let index_names: Vec<Atom<'_>> = {
let mut names = Vec::new();
if let Some(ref index) = rep.var_names.index {
names.push(index.clone());
}
let mut names: Vec<Atom<'_>> = rep.var_names.index.iter().cloned().collect();
// Also include '$index' itself
names.push(Atom::from("$index"));
names
Expand Down
6 changes: 3 additions & 3 deletions crates/oxc_angular_compiler/src/transform/control_flow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -531,9 +531,9 @@ fn parse_let_parameter<'a>(
}

// Check for duplicate alias name
let already_has_name = context_variables
.iter()
.any(|v| v.name.as_str() == name && v.name.as_str() != v.value.as_str());
// Angular checks all existing names including implicit context vars (e.g. $index).
// Reference: r3_control_flow.ts line 479
let already_has_name = context_variables.iter().any(|v| v.name.as_str() == name);
if already_has_name {
errors.push(format!("Duplicate \"let\" parameter variable \"{}\"", variable_name));
current_offset += part.len() as u32 + 1;
Expand Down
7 changes: 7 additions & 0 deletions crates/oxc_angular_compiler/src/transform/html_to_r3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2370,12 +2370,19 @@ impl<'a> HtmlToR3Transform<'a> {
}

// Validate @case: must have exactly one parameter
// Angular pushes invalid @case blocks into unknownBlocks for language service support.
// Reference: r3_control_flow.ts line 242
let is_case = child_block.block_type == BlockType::Case;
if is_case && child_block.parameters.len() != 1 {
self.report_error(
"@case block must have exactly one parameter",
child_block.start_span,
);
unknown_blocks.push(crate::ast::r3::R3UnknownBlock {
name: child_block.name.clone(),
source_span: child_block.span,
name_span: child_block.name_span,
});
continue;
}

Expand Down
24 changes: 24 additions & 0 deletions crates/oxc_angular_compiler/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5126,3 +5126,27 @@ export class MixedQueryComponent {
}
}
}

#[test]
fn test_for_loop_multiple_index_aliases_in_track() {
// When multiple aliases reference $index (e.g., `let i = $index, j = $index`),
// ALL aliases must be rewritten to $index in the track expression.
// Angular stores all $index aliases in a Set<string> and checks membership,
// while a bug in OXC previously stored only the last alias (overwriting earlier ones).
// Reference: Angular's ingest.ts uses `indexVarNames = new Set<string>()` and `.add()`.
let js = compile_template_to_js(
r#"@for (item of items; track i + j; let i = $index, j = $index) { {{item}} }"#,
"TestComponent",
);
// The track function should rewrite both `i` and `j` to `$index`.
// Expected: ($index,$item)=>($index + $index)
// Bug behavior: ($index,$item)=>(this.i + $index) (only `j` rewritten, `i` left as `this.i`)
assert!(
!js.contains("this.i"),
"Track expression should rewrite all $index aliases, but `i` was not rewritten.\nGenerated JS:\n{js}"
);
assert!(
!js.contains("this.j"),
"Track expression should rewrite all $index aliases, but `j` was not rewritten.\nGenerated JS:\n{js}"
);
}
74 changes: 74 additions & 0 deletions crates/oxc_angular_compiler/tests/r3_template_transform_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2052,3 +2052,77 @@ mod switch_validation {
);
}
}

// ============================================================================
// Tests: @for duplicate let parameter validation
// ============================================================================

mod switch_invalid_case_unknown_blocks {
use super::*;

/// Helper that parses HTML to R3 AST and returns the unknown_blocks from the first SwitchBlock.
fn get_switch_unknown_block_names(html: &str) -> Vec<String> {
let allocator = Box::new(Allocator::default());
let allocator_ref: &'static Allocator =
unsafe { &*std::ptr::from_ref::<Allocator>(allocator.as_ref()) };

let parser = HtmlParser::new(allocator_ref, html, "test.html");
let html_result = parser.parse();

let options = TransformOptions { collect_comment_nodes: false };
let transformer = HtmlToR3Transform::new(allocator_ref, html, options);
let r3_result = transformer.transform(&html_result.nodes);

// Find the first SwitchBlock and return its unknown_blocks names
for node in r3_result.nodes.iter() {
if let R3Node::SwitchBlock(b) = node {
return b.unknown_blocks.iter().map(|ub| ub.name.to_string()).collect();
}
}
Vec::new()
}

#[test]
fn should_add_invalid_case_with_no_params_to_unknown_blocks() {
// Angular pushes @case blocks with invalid parameters into unknownBlocks
// for language service autocompletion support.
// Reference: r3_control_flow.ts line 242
let unknown_names =
get_switch_unknown_block_names("@switch (expr) { @case { a } @case (1) { b } }");
assert!(
unknown_names.contains(&"case".to_string()),
"Expected invalid @case (no params) to be in unknown_blocks, got: {unknown_names:?}"
);
}
}

mod for_loop_duplicate_let_validation {
use super::*;

#[test]
fn should_report_duplicate_let_for_implicit_var_aliased_to_itself() {
// Angular test: `let $index = $index` should be rejected as a duplicate
// because $index is already pre-populated as an implicit context variable.
// Reference: r3_template_transform_spec.ts line 2340
let errors = get_transform_errors(
"@for (item of items.foo.bar; track item.id; let $index = $index) {}",
);
assert!(
errors.iter().any(|e| e.contains("Duplicate \"let\" parameter variable")),
"Expected duplicate let parameter error for `let $index = $index`, got: {errors:?}"
);
}

#[test]
fn should_report_duplicate_let_for_explicit_alias_used_twice() {
// Angular test: `let i = $index` used twice should be rejected
// Reference: r3_template_transform_spec.ts line 2340
let errors = get_transform_errors(
"@for (item of items.foo.bar; track item.id; let i = $index, f = $first, i = $index) {}",
);
assert!(
errors.iter().any(|e| e.contains("Duplicate \"let\" parameter variable")),
"Expected duplicate let parameter error for duplicate alias `i`, got: {errors:?}"
);
}
}
Loading