Skip to content

Commit 484355e

Browse files
Brooooooklynclaude
andcommitted
Angular compiler alignment: 100% match rate (651/651 components)
This commit achieves full alignment between the Oxc Angular compiler and the TypeScript Angular compiler for all 651 tested components from the bitwarden-clients project. Key fixes in this session (95.5% -> 100%): - Nullish coalescing parentheses around Conditional LHS - ngFor listener context variables (LiteralMap/LiteralArray handling) - HTML entity encoding (UTF-8 character handling in expression lexer) - Animation binding const index (skip animations in attribute extraction) - Interpolated binding sanitizers (security context for interpolated properties) - Const pool ordering (single-pass pooling matching TypeScript) - .bind() target resolution (ThisReceiver resolves to proper context variable) - Template literal variable resolution (local alias references) - Context alias inlining (fixed double-counting in variable optimization) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 9e07537 commit 484355e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+977
-476
lines changed

crates/oxc_angular_compiler/src/ir/enums.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,8 @@ pub enum ExpressionKind {
258258
Typeof,
259259
/// Void expression (void expr).
260260
Void,
261+
/// Template literal with resolved expressions (used after name resolution).
262+
ResolvedTemplateLiteral,
261263
}
262264

263265
/// Flags for semantic variables.

crates/oxc_angular_compiler/src/ir/expression.rs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,10 @@ pub enum IrExpression<'a> {
174174
/// Void expression (void expr).
175175
/// Used to preserve pipe bindings inside void expressions during ingest.
176176
Void(Box<'a, VoidExpr<'a>>),
177+
/// Template literal with resolved expressions (created during name resolution).
178+
/// Used when a template literal like `\`bwi ${menuItem.icon}\`` has expressions
179+
/// that need to be resolved to variables in scope.
180+
ResolvedTemplateLiteral(Box<'a, ResolvedTemplateLiteralExpr<'a>>),
177181
}
178182

179183
impl<'a> IrExpression<'a> {
@@ -242,6 +246,8 @@ impl<'a> IrExpression<'a> {
242246
IrExpression::Typeof(_) => ExpressionKind::Typeof,
243247
// Void is a void expression
244248
IrExpression::Void(_) => ExpressionKind::Void,
249+
// ResolvedTemplateLiteral is a template literal with resolved expressions
250+
IrExpression::ResolvedTemplateLiteral(_) => ExpressionKind::ResolvedTemplateLiteral,
245251
}
246252
}
247253
}
@@ -705,6 +711,27 @@ impl<'a> IrExpression<'a> {
705711
},
706712
allocator,
707713
)),
714+
IrExpression::ResolvedTemplateLiteral(e) => {
715+
let mut elements = Vec::with_capacity_in(e.elements.len(), allocator);
716+
for elem in e.elements.iter() {
717+
elements.push(IrTemplateLiteralElement {
718+
text: elem.text.clone(),
719+
source_span: elem.source_span,
720+
});
721+
}
722+
let mut expressions = Vec::with_capacity_in(e.expressions.len(), allocator);
723+
for expr in e.expressions.iter() {
724+
expressions.push(expr.clone_in(allocator));
725+
}
726+
IrExpression::ResolvedTemplateLiteral(Box::new_in(
727+
ResolvedTemplateLiteralExpr {
728+
elements,
729+
expressions,
730+
source_span: e.source_span,
731+
},
732+
allocator,
733+
))
734+
}
708735
}
709736
}
710737
}
@@ -890,6 +917,30 @@ pub struct ResolvedSafePropertyReadExpr<'a> {
890917
pub source_span: Option<Span>,
891918
}
892919

920+
/// Template literal with resolved expressions.
921+
///
922+
/// Created during name resolution when a template literal like `\`bwi ${menuItem.icon}\``
923+
/// has expressions that reference variables in scope. The expressions are resolved to
924+
/// `ReadVariable` or `ResolvedPropertyRead` expressions.
925+
#[derive(Debug)]
926+
pub struct ResolvedTemplateLiteralExpr<'a> {
927+
/// Template literal text elements (the static parts between expressions).
928+
pub elements: Vec<'a, IrTemplateLiteralElement<'a>>,
929+
/// Resolved expressions (the dynamic parts inside ${...}).
930+
pub expressions: Vec<'a, IrExpression<'a>>,
931+
/// Source span.
932+
pub source_span: Option<Span>,
933+
}
934+
935+
/// Template literal element (text part) for IR expressions.
936+
#[derive(Debug, Clone)]
937+
pub struct IrTemplateLiteralElement<'a> {
938+
/// The text content.
939+
pub text: Atom<'a>,
940+
/// Source span.
941+
pub source_span: Option<Span>,
942+
}
943+
893944
/// Derived literal array for pure function bodies.
894945
/// This is used when a literal array contains non-constant entries that need
895946
/// to be replaced with PureFunctionParameter references.
@@ -1488,6 +1539,11 @@ pub fn transform_expressions_in_expression<'a, F>(
14881539
IrExpression::Void(e) => {
14891540
transform_expressions_in_expression(&mut e.expr, transform, flags);
14901541
}
1542+
IrExpression::ResolvedTemplateLiteral(e) => {
1543+
for expr in e.expressions.iter_mut() {
1544+
transform_expressions_in_expression(expr, transform, flags);
1545+
}
1546+
}
14911547
// These expressions have no internal expressions
14921548
IrExpression::LexicalRead(_)
14931549
| IrExpression::Reference(_)
@@ -1653,6 +1709,11 @@ pub fn visit_expressions_in_expression<'a, F>(
16531709
IrExpression::Void(e) => {
16541710
visit_expressions_in_expression(&e.expr, visitor, flags);
16551711
}
1712+
IrExpression::ResolvedTemplateLiteral(e) => {
1713+
for expr in e.expressions.iter() {
1714+
visit_expressions_in_expression(expr, visitor, flags);
1715+
}
1716+
}
16561717
// These expressions have no internal expressions
16571718
IrExpression::LexicalRead(_)
16581719
| IrExpression::Reference(_)
@@ -2644,6 +2705,11 @@ pub fn vars_used_by_ir_expression(expr: &IrExpression<'_>) -> u32 {
26442705
// Void expression: vars used by inner expression
26452706
IrExpression::Void(void_expr) => vars_used_by_ir_expression(&void_expr.expr),
26462707

2708+
// ResolvedTemplateLiteral: vars used by all expressions inside
2709+
IrExpression::ResolvedTemplateLiteral(rtl) => {
2710+
rtl.expressions.iter().map(vars_used_by_ir_expression).sum()
2711+
}
2712+
26472713
// All other expressions don't directly consume variable slots
26482714
IrExpression::LexicalRead(_)
26492715
| IrExpression::Reference(_)

crates/oxc_angular_compiler/src/output/emitter.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,7 +597,20 @@ impl JsEmitter {
597597
}
598598
OutputExpression::BinaryOperator(e) => {
599599
ctx.print("(");
600+
// When the operator is NullishCoalesce and the LHS is a Conditional,
601+
// TypeScript requires an extra set of parentheses around the LHS.
602+
// This is because TypeScript generates incorrect code if they're missing:
603+
// `(a ? b : c) ?? d` must keep the parentheses.
604+
// See: angular/packages/compiler/src/template/pipeline/src/phases/strip_nonrequired_parentheses.ts:91-96
605+
let lhs_needs_extra_parens = matches!(e.operator, BinaryOperator::NullishCoalesce)
606+
&& matches!(e.lhs.as_ref(), OutputExpression::Conditional(_));
607+
if lhs_needs_extra_parens {
608+
ctx.print("(");
609+
}
600610
self.visit_expression(&e.lhs, ctx);
611+
if lhs_needs_extra_parens {
612+
ctx.print(")");
613+
}
601614
ctx.print(" ");
602615
ctx.print(binary_operator_to_str(e.operator));
603616
ctx.print(" ");

crates/oxc_angular_compiler/src/parser/expression/lexer.rs

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -401,11 +401,9 @@ pub struct Lexer<'a> {
401401
allocator: &'a Allocator,
402402
/// The source text.
403403
input: &'a str,
404-
/// The source bytes.
405-
bytes: &'a [u8],
406-
/// The input length.
404+
/// The input length (in bytes).
407405
length: u32,
408-
/// The current position.
406+
/// The current byte position.
409407
index: u32,
410408
/// The generated tokens.
411409
tokens: std::vec::Vec<Token<'a>>,
@@ -417,7 +415,6 @@ impl<'a> Lexer<'a> {
417415
Self {
418416
allocator,
419417
input,
420-
bytes: input.as_bytes(),
421418
length: input.len() as u32,
422419
index: 0,
423420
tokens: std::vec::Vec::new(),
@@ -434,20 +431,31 @@ impl<'a> Lexer<'a> {
434431
}
435432

436433
/// Peeks at the current character.
434+
/// Returns the next UTF-8 character or EOF if at end of input.
437435
fn peek(&self) -> char {
438-
if self.index >= self.length { chars::EOF } else { self.bytes[self.index as usize] as char }
436+
if self.index >= self.length {
437+
chars::EOF
438+
} else {
439+
self.input[self.index as usize..].chars().next().unwrap_or(chars::EOF)
440+
}
439441
}
440442

441443
/// Peeks at a character at offset from current position.
444+
/// Note: offset is in characters, not bytes. For multi-byte UTF-8 characters,
445+
/// this iterates through the characters to find the one at the given offset.
442446
fn peek_at(&self, offset: u32) -> char {
443-
let pos = self.index + offset;
444-
if pos >= self.length { chars::EOF } else { self.bytes[pos as usize] as char }
447+
if self.index >= self.length {
448+
return chars::EOF;
449+
}
450+
self.input[self.index as usize..].chars().nth(offset as usize).unwrap_or(chars::EOF)
445451
}
446452

447-
/// Advances the index and returns the current character.
453+
/// Advances the index by the current character's byte length and returns the character.
448454
fn advance(&mut self) -> char {
449455
let ch = self.peek();
450-
self.index += 1;
456+
if ch != chars::EOF {
457+
self.index += ch.len_utf8() as u32;
458+
}
451459
ch
452460
}
453461

@@ -721,10 +729,11 @@ impl<'a> Lexer<'a> {
721729
/// Scans a number literal, including support for numeric separators.
722730
fn scan_number(&mut self, start: u32) {
723731
// Check if number starts with a period (e.g., .5)
724-
let mut is_float = self.bytes.get(start as usize) == Some(&b'.');
732+
let start_char = self.input[start as usize..].chars().next();
733+
let mut is_float = start_char == Some('.');
725734

726735
// Check for hex, octal, or binary
727-
if self.bytes.get(start as usize) == Some(&b'0') {
736+
if start_char == Some('0') {
728737
let next = self.peek();
729738
if next == 'x' || next == 'X' {
730739
self.advance();

crates/oxc_angular_compiler/src/pipeline/emit.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1188,6 +1188,33 @@ fn convert_pure_function_body<'a>(
11881188
allocator,
11891189
))
11901190
}
1191+
1192+
// ResolvedTemplateLiteral: convert to template literal with resolved expressions
1193+
IrExpression::ResolvedTemplateLiteral(rtl) => {
1194+
let mut elements = OxcVec::new_in(allocator);
1195+
let mut expressions = OxcVec::new_in(allocator);
1196+
1197+
for elem in rtl.elements.iter() {
1198+
elements.push(crate::output::ast::TemplateLiteralElement {
1199+
text: elem.text.clone(),
1200+
raw_text: elem.text.clone(),
1201+
source_span: elem.source_span,
1202+
});
1203+
}
1204+
1205+
for expr in rtl.expressions.iter() {
1206+
expressions.push(convert_pure_function_body(allocator, expr, params));
1207+
}
1208+
1209+
OutputExpression::TemplateLiteral(Box::new_in(
1210+
crate::output::ast::TemplateLiteralExpr {
1211+
elements,
1212+
expressions,
1213+
source_span: rtl.source_span,
1214+
},
1215+
allocator,
1216+
))
1217+
}
11911218
}
11921219
}
11931220

crates/oxc_angular_compiler/src/pipeline/ingest.rs

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ use crate::ast::r3::{
3030
R3Variable, SecurityContext,
3131
};
3232
use crate::ir::enums::{
33-
BindingKind, DeferOpModifierKind, DeferTriggerKind, Namespace, SemanticVariableKind,
34-
TemplateKind,
33+
AnimationKind, BindingKind, DeferOpModifierKind, DeferTriggerKind, Namespace,
34+
SemanticVariableKind, TemplateKind,
3535
};
3636
use crate::ir::expression::{
3737
BinaryExpr, ConditionalCaseExpr, EmptyExpr, IrBinaryOperator, IrExpression, LexicalReadExpr,
@@ -1368,6 +1368,22 @@ fn ingest_listener_owned<'a>(
13681368
}
13691369
}
13701370

1371+
// Determine if this is an animation listener and extract animation phase
1372+
let (is_animation_listener, animation_phase) = match output.event_type {
1373+
ParsedEventType::Animation => (true, None),
1374+
ParsedEventType::LegacyAnimation => {
1375+
// For legacy animations, parse the phase from the output
1376+
// Phase can be "start" or "done"
1377+
let phase = output.phase.as_ref().and_then(|p| match p.as_str() {
1378+
"start" => Some(AnimationKind::Enter),
1379+
"done" => Some(AnimationKind::Leave),
1380+
_ => None,
1381+
});
1382+
(true, phase)
1383+
}
1384+
_ => (false, None),
1385+
};
1386+
13711387
CreateOp::Listener(ListenerOp {
13721388
base: CreateOpBase { source_span: Some(output.source_span), ..Default::default() },
13731389
target: element_xref,
@@ -1379,8 +1395,8 @@ fn ingest_listener_owned<'a>(
13791395
handler_ops,
13801396
handler_fn_name: None,
13811397
consume_fn_name: None,
1382-
is_animation_listener: false,
1383-
animation_phase: None,
1398+
is_animation_listener,
1399+
animation_phase,
13841400
event_target: output.target,
13851401
consumes_dollar_event: false, // Set during resolve_dollar_event phase
13861402
})
@@ -3442,11 +3458,12 @@ fn ingest_control_flow_insertion_point<'a, 'b>(
34423458
let mut root: Option<RootNodeRef<'a, 'b>> = None;
34433459

34443460
for child in children {
3445-
// Skip over comment nodes, @let declarations, and whitespace-only text nodes
3446-
// since it doesn't matter where they end up in the DOM.
3461+
// Skip over comment nodes and @let declarations since
3462+
// it doesn't matter where they end up in the DOM.
3463+
// NOTE: TypeScript does NOT skip whitespace-only text nodes here,
3464+
// so we must not skip them either to match the behavior.
34473465
match child {
34483466
R3Node::Comment(_) | R3Node::LetDeclaration(_) => continue,
3449-
R3Node::Text(text) if text.value.as_str().trim().is_empty() => continue,
34503467
R3Node::Element(elem) => {
34513468
// We can only infer the tag name/attributes if there's a single root node.
34523469
if root.is_some() {

crates/oxc_angular_compiler/src/pipeline/phases/attribute_extraction.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,15 @@ fn process_view_attributes<'a>(
101101
}
102102
}
103103
UpdateOp::Property(prop_op) => {
104+
// Skip animation bindings - they don't participate in directive matching
105+
// and are handled separately via property() instruction at runtime.
106+
if matches!(
107+
prop_op.binding_kind,
108+
BindingKind::Animation | BindingKind::LegacyAnimation
109+
) {
110+
continue;
111+
}
112+
104113
// Properties also generate extracted attributes for directive matching
105114
// Note: Property ops are NOT removed - they still need runtime updates
106115
let extracted = ExtractedAttributeOp {
@@ -148,6 +157,15 @@ fn process_view_attributes<'a>(
148157
// These are created for content projection and should be extracted if they're
149158
// text attributes or have constant expressions.
150159
UpdateOp::Binding(binding_op) => {
160+
// Animation bindings are NOT extractable - they're handled separately
161+
// by the property() instruction at runtime.
162+
if matches!(
163+
binding_op.kind,
164+
BindingKind::Animation | BindingKind::LegacyAnimation
165+
) {
166+
continue;
167+
}
168+
151169
// Check if this binding is extractable:
152170
// - Text attributes (static attributes from template) are always extractable
153171
// - Non-interpolation constant expressions are also extractable

0 commit comments

Comments
 (0)