From 57f310d7f768cc613d26f2970ec1122169632690 Mon Sep 17 00:00:00 2001 From: Gabriel Silva Date: Mon, 26 Oct 2026 19:16:56 -0300 Subject: [PATCH 1/3] fix(parser): read non-ASCII selectors by byte offset, and don't panic on a microsyntax value that fails to parse --- .../src/parser/expression/parser.rs | 4 ++- .../src/pipeline/selector.rs | 30 ++++++++++++++----- .../tests/expression_parser_test.rs | 13 ++++++++ 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/crates/oxc_angular_compiler/src/parser/expression/parser.rs b/crates/oxc_angular_compiler/src/parser/expression/parser.rs index 53075d135..c593c67b0 100644 --- a/crates/oxc_angular_compiler/src/parser/expression/parser.rs +++ b/crates/oxc_angular_compiler/src/parser/expression/parser.rs @@ -752,7 +752,9 @@ impl<'a> Parser<'a> { self.peek().map(|t| t.index).unwrap_or(self.source.len() as u32) }; - let value_source = &self.source[expr_start as usize..value_end as usize]; + // A value that fails to parse ends before it starts: Angular's `substring` swaps the two. + let value_source = + &self.source[expr_start.min(value_end) as usize..expr_start.max(value_end) as usize]; let ast_with_source = ASTWithSource { ast: value, source: Some(Ident::from_in(value_source, &self.allocator)), diff --git a/crates/oxc_angular_compiler/src/pipeline/selector.rs b/crates/oxc_angular_compiler/src/pipeline/selector.rs index df9a40588..6b79d3b15 100644 --- a/crates/oxc_angular_compiler/src/pipeline/selector.rs +++ b/crates/oxc_angular_compiler/src/pipeline/selector.rs @@ -150,6 +150,10 @@ impl CssSelector { let chars: Vec = selector.chars().collect(); let len = chars.len(); + // `i` counts chars; slices of `selector` take the byte offset of each. + let offsets: Vec = + selector.char_indices().map(|(at, _)| at).chain([selector.len()]).collect(); + let text = |start: usize, end: usize| &selector[offsets[start]..offsets[end]]; let mut i = 0; while i < len { @@ -177,7 +181,7 @@ impl CssSelector { } // Handle :not( - if c == ':' && i + 4 < len && &selector[i..i + 5] == ":not(" { + if c == ':' && i + 4 < len && text(i, i + 5) == ":not(" { // Nesting :not in a selector is not allowed - return what we have if in_not { Self::finalize_selector(&mut results, &mut current, false, CssSelector::new()); @@ -211,7 +215,7 @@ impl CssSelector { i += 1; } if i > start { - target.add_class_name(&selector[start..i]); + target.add_class_name(text(start, i)); } continue; } @@ -224,7 +228,7 @@ impl CssSelector { i += 1; } if i > start { - target.add_attribute("id", Some(&selector[start..i])); + target.add_attribute("id", Some(text(start, i))); } continue; } @@ -237,7 +241,7 @@ impl CssSelector { while i < len && chars[i] != '=' && chars[i] != ']' { i += 1; } - let raw_attr_name = selector[attr_start..i].trim(); + let raw_attr_name = text(attr_start, i).trim(); // Unescape attribute name (like Angular's unescapeAttribute) let attr_name = Self::unescape_attribute(raw_attr_name); @@ -257,7 +261,7 @@ impl CssSelector { while i < len && chars[i] != q { i += 1; } - let attr_value = &selector[value_start..i]; + let attr_value = text(value_start, i); target.add_attribute(&attr_name, Some(attr_value)); if i < len { i += 1; // Skip closing quote @@ -266,7 +270,7 @@ impl CssSelector { while i < len && chars[i] != ']' { i += 1; } - let attr_value = selector[value_start..i].trim(); + let attr_value = text(value_start, i).trim(); target.add_attribute(&attr_name, Some(attr_value)); } } else { @@ -286,7 +290,7 @@ impl CssSelector { while i < len && Self::is_ident_char(chars[i]) { i += 1; } - target.set_element(&selector[start..i]); + target.set_element(text(start, i)); continue; } @@ -544,6 +548,18 @@ mod tests { assert_eq!(selectors[0].attrs, vec!["name", "value"]); } + #[test] + fn test_parse_non_ascii() { + let selectors = CssSelector::parse("[slot=título]"); + assert_eq!(selectors[0].attrs, vec!["slot", "título"]); + let selectors = CssSelector::parse("[lang='ção'].ação#é"); + assert_eq!(selectors[0].attrs, vec!["lang", "ção", "id", "é"]); + assert_eq!(selectors[0].class_names, vec!["ação"]); + let selectors = CssSelector::parse("x-é:not([é=ü])"); + assert_eq!(selectors[0].element, Some("x-é".to_string())); + assert_eq!(selectors[0].not_selectors[0].attrs, vec!["é", "ü"]); + } + #[test] fn test_parse_comma_separated() { let selectors = CssSelector::parse("div, span"); diff --git a/crates/oxc_angular_compiler/tests/expression_parser_test.rs b/crates/oxc_angular_compiler/tests/expression_parser_test.rs index 21f928fcb..e8b856937 100644 --- a/crates/oxc_angular_compiler/tests/expression_parser_test.rs +++ b/crates/oxc_angular_compiler/tests/expression_parser_test.rs @@ -1138,6 +1138,19 @@ mod parse_template_bindings { result.bindings } + #[test] + fn should_report_a_value_that_fails_to_parse() { + for value in ["b; in/", "let é of ü; trackBy: ñ"] { + let allocator = Allocator::default(); + let key = TemplateBindingIdentifier { + source: oxc_str::Ident::from("a"), + span: oxc_angular_compiler::ast::expression::AbsoluteSourceSpan::new(0, 1), + }; + let result = Parser::new(&allocator, value).parse_template_bindings(key); + assert!(!result.errors.is_empty(), "{value}"); + } + } + // NOTE: Many tests below document the CURRENT behavior of the Rust implementation. // Some behaviors differ from Angular's TypeScript implementation. // Tests marked with TODO comments indicate expected Angular behavior for future fixes. From d0be913cfe372f8b1a077e3fc89e3c3687ba0650 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 28 Oct 2026 10:16:49 +0800 Subject: [PATCH 2/3] fix(parser): report a keyword value that starts with an unexpected token `*ngFor="let i of xs; trackBy: }"` panicked on main; with the swapped slice it compiled with no error and dropped the `trackBy` binding. A token that starts no expression is left unread and the value is empty. Angular's `parsePrimary` reports it ("Unexpected token }") and skips it, which also gives the value text Angular takes from the AST's span. Co-Authored-By: Claude Opus 5.5 --- .../src/parser/expression/parser.rs | 8 +++++++ .../tests/expression_parser_test.rs | 22 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/crates/oxc_angular_compiler/src/parser/expression/parser.rs b/crates/oxc_angular_compiler/src/parser/expression/parser.rs index c593c67b0..c6222c0aa 100644 --- a/crates/oxc_angular_compiler/src/parser/expression/parser.rs +++ b/crates/oxc_angular_compiler/src/parser/expression/parser.rs @@ -736,6 +736,14 @@ impl<'a> Parser<'a> { } let value = self.parse_pipe_for_template_binding(); + // A token that starts no expression is left unread, and the value is empty. Angular's + // `parsePrimary` reports it and skips it. + if self.current_end_index() < expr_start + && let Some(token) = self.peek() + { + let token_str = token.str_value.to_string(); + self.error(&format!("Unexpected token {token_str}")); + } // Value source is just the expression (up to current_end_index) let value_end = self.current_end_index(); diff --git a/crates/oxc_angular_compiler/tests/expression_parser_test.rs b/crates/oxc_angular_compiler/tests/expression_parser_test.rs index e8b856937..99e3b8471 100644 --- a/crates/oxc_angular_compiler/tests/expression_parser_test.rs +++ b/crates/oxc_angular_compiler/tests/expression_parser_test.rs @@ -1151,6 +1151,28 @@ mod parse_template_bindings { } } + #[test] + fn should_report_a_token_that_starts_no_value() { + for (value, token) in [ + ("let i of xs; trackBy: }", "}"), + ("b; c: )", ")"), + ("b; c ]", "]"), + ("let i of xs; trackBy: =>", "=>"), + ] { + let allocator = Allocator::default(); + let key = TemplateBindingIdentifier { + source: oxc_str::Ident::from("a"), + span: oxc_angular_compiler::ast::expression::AbsoluteSourceSpan::new(0, 1), + }; + let result = Parser::new(&allocator, value).parse_template_bindings(key); + let messages: Vec<_> = result.errors.iter().map(String::as_str).collect(); + assert!( + messages.iter().any(|m| m.starts_with(&format!("Unexpected token {token} "))), + "{value}: {messages:?}" + ); + } + } + // NOTE: Many tests below document the CURRENT behavior of the Rust implementation. // Some behaviors differ from Angular's TypeScript implementation. // Tests marked with TODO comments indicate expected Angular behavior for future fixes. From 1fb9530e5a47fb700e00fd159d3674804d1e7f0e Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 28 Oct 2026 10:16:49 +0800 Subject: [PATCH 3/3] fix(selector): read names and attributes as Angular's selector regexp does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Angular parses selectors with `_SELECTOR_REGEXP`, whose `\w` has no `u` flag: a name is a run of ASCII `[-\w]`, so `.ação` is the class `a` and then the element `o`, and `` looks up `a|href` for its sanitizer. An attribute matches only `\[([-.\w*\\$]+)(?:=(["']?)([^\]"']*)\5)?\]`; otherwise the regexp goes on from the char after the `[`. An unquoted value is no longer trimmed. The linker escaping test used `[t='a"b']`, which Angular reads as the element `b`: a quote never reaches a selector value. It now escapes a backslash, which does. Co-Authored-By: Claude Opus 5.5 --- .../src/pipeline/selector.rs | 129 ++++++++++-------- .../oxc_angular_compiler/tests/linker_test.rs | 4 +- 2 files changed, 75 insertions(+), 58 deletions(-) diff --git a/crates/oxc_angular_compiler/src/pipeline/selector.rs b/crates/oxc_angular_compiler/src/pipeline/selector.rs index 6b79d3b15..3ae705684 100644 --- a/crates/oxc_angular_compiler/src/pipeline/selector.rs +++ b/crates/oxc_angular_compiler/src/pipeline/selector.rs @@ -34,6 +34,9 @@ impl SelectorFlags { } } +/// A `start..end` range of char indices into a selector. +type CharRange = (usize, usize); + /// A parsed CSS selector. #[derive(Debug, Default)] pub struct CssSelector { @@ -233,59 +236,20 @@ impl CssSelector { continue; } - // Handle attribute selector - if c == '[' { - i += 1; - let attr_start = i; - // Find attribute name (may contain escape sequences like \$) - while i < len && chars[i] != '=' && chars[i] != ']' { - i += 1; - } - let raw_attr_name = text(attr_start, i).trim(); + // Handle attribute selector. When it does not match, Angular's regexp goes on from + // the char after the `[`, as an unknown char. + if c == '[' + && let Some((name, value, end)) = Self::attribute_at(&chars, i + 1) + { // Unescape attribute name (like Angular's unescapeAttribute) - let attr_name = Self::unescape_attribute(raw_attr_name); - - if i < len && chars[i] == '=' { - i += 1; - // Skip optional quote - let quote = if i < len && (chars[i] == '"' || chars[i] == '\'') { - let q = chars[i]; - i += 1; - Some(q) - } else { - None - }; - - let value_start = i; - if let Some(q) = quote { - while i < len && chars[i] != q { - i += 1; - } - let attr_value = text(value_start, i); - target.add_attribute(&attr_name, Some(attr_value)); - if i < len { - i += 1; // Skip closing quote - } - } else { - while i < len && chars[i] != ']' { - i += 1; - } - let attr_value = text(value_start, i).trim(); - target.add_attribute(&attr_name, Some(attr_value)); - } - } else { - target.add_attribute(&attr_name, None); - } - - // Skip closing bracket - if i < len && chars[i] == ']' { - i += 1; - } + let attr_name = Self::unescape_attribute(text(name.0, name.1)); + target.add_attribute(&attr_name, value.map(|(start, end)| text(start, end))); + i = end; continue; } // Handle element name (identifier at current position) - if Self::is_ident_start(c) { + if Self::is_ident_char(c) { let start = i; while i < len && Self::is_ident_char(chars[i]) { i += 1; @@ -344,12 +308,49 @@ impl CssSelector { result } - fn is_ident_start(c: char) -> bool { - c.is_ascii_alphabetic() || c == '_' || c == '-' || c > '\x7f' + /// `[-\w]`, the chars of a name in Angular's selector regexp. `\w` has no `u` flag there, so + /// it is ASCII only: `.ação` is the class `a` and then the element `o`. + fn is_ident_char(c: char) -> bool { + c.is_ascii_alphanumeric() || c == '_' || c == '-' } - fn is_ident_char(c: char) -> bool { - c.is_ascii_alphanumeric() || c == '_' || c == '-' || c > '\x7f' + /// Angular's attribute production, `\[([-.\w*\\$]+)(?:=(["']?)([^\]"']*)\5)?\]`, from just + /// after the `[`: the char ranges of the name and of the value, and the index after the `]`. + fn attribute_at(chars: &[char], start: usize) -> Option<(CharRange, Option, usize)> { + let mut i = start; + while chars + .get(i) + .is_some_and(|&c| Self::is_ident_char(c) || matches!(c, '.' | '*' | '\\' | '$')) + { + i += 1; + } + if i == start { + return None; + } + let name = (start, i); + match chars.get(i)? { + ']' => Some((name, None, i + 1)), + '=' => { + i += 1; + let quote = chars.get(i).copied().filter(|c| matches!(c, '"' | '\'')); + if quote.is_some() { + i += 1; + } + let value_start = i; + while chars.get(i).is_some_and(|c| !matches!(c, ']' | '"' | '\'')) { + i += 1; + } + let value = (value_start, i); + if let Some(quote) = quote { + if chars.get(i) != Some("e) { + return None; + } + i += 1; + } + (chars.get(i) == Some(&']')).then_some((name, Some(value), i + 1)) + } + _ => None, + } } fn finalize_selector( @@ -548,16 +549,32 @@ mod tests { assert_eq!(selectors[0].attrs, vec!["name", "value"]); } + /// As Angular's selector regexp reads them: values keep any char, names only `[-\w]` (ASCII). #[test] fn test_parse_non_ascii() { let selectors = CssSelector::parse("[slot=título]"); assert_eq!(selectors[0].attrs, vec!["slot", "título"]); let selectors = CssSelector::parse("[lang='ção'].ação#é"); - assert_eq!(selectors[0].attrs, vec!["lang", "ção", "id", "é"]); - assert_eq!(selectors[0].class_names, vec!["ação"]); + assert_eq!(selectors[0].attrs, vec!["lang", "ção"]); + assert_eq!(selectors[0].class_names, vec!["a"]); + assert_eq!(selectors[0].element, Some("o".to_string())); let selectors = CssSelector::parse("x-é:not([é=ü])"); - assert_eq!(selectors[0].element, Some("x-é".to_string())); - assert_eq!(selectors[0].not_selectors[0].attrs, vec!["é", "ü"]); + assert_eq!(selectors[0].element, Some("x-".to_string())); + let not = &selectors[0].not_selectors[0]; + assert!(not.element.is_none() && not.attrs.is_empty() && not.class_names.is_empty()); + } + + /// An attribute that does not match Angular's production is read on from the char after `[`. + #[test] + fn test_parse_attribute_like_angular() { + let selectors = CssSelector::parse("[slot= olá ]"); + assert_eq!(selectors[0].attrs, vec!["slot", " olá "]); + let selectors = CssSelector::parse("[ a ]"); + assert_eq!(selectors[0].element, Some("a".to_string())); + assert!(selectors[0].attrs.is_empty()); + let selectors = CssSelector::parse("[a=\"b]"); + assert_eq!(selectors[0].element, Some("b".to_string())); + assert!(selectors[0].attrs.is_empty()); } #[test] diff --git a/crates/oxc_angular_compiler/tests/linker_test.rs b/crates/oxc_angular_compiler/tests/linker_test.rs index 65948a3a3..5d16b48e2 100644 --- a/crates/oxc_angular_compiler/tests/linker_test.rs +++ b/crates/oxc_angular_compiler/tests/linker_test.rs @@ -643,14 +643,14 @@ fn test_link_escapes_metadata_strings() { export class P {} P.ɵpipe = i0.ɵɵngDeclarePipe({ minVersion: "14.0.0", version: "21.0.0", ngImport: i0, type: P, isStandalone: true, name: 'a"+x+"' }); export class D {} -D.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "21.0.0", ngImport: i0, type: D, isStandalone: true, selector: "[t='a\"b']", inputs: { 'a.b"c': "c" }, host: { classAttribute: 'k"+x+"' }, controlCreate: { passThroughInput: 'f"+x+"' } }); +D.ɵdir = i0.ɵɵngDeclareDirective({ minVersion: "14.0.0", version: "21.0.0", ngImport: i0, type: D, isStandalone: true, selector: "[t='a\\b']", inputs: { 'a.b"c': "c" }, host: { classAttribute: 'k"+x+"' }, controlCreate: { passThroughInput: 'f"+x+"' } }); "#; let allocator = Allocator::default(); let result = link(&allocator, code, "test.mjs"); assert!(result.linked); for expected in [ r#"name: "a\"+x+\"""#, - r#"selectors: [["", "t", "a\"b"]]"#, + r#"selectors: [["", "t", "a\\b"]]"#, r#"inputs: { "a.b\"c": "c" }"#, r#"hostAttrs: [1, "k\"+x+\""]"#, r#"ɵɵControlFeature("f\"+x+\"")"#,