From e5869b8826bdaf86c5e212186331bb4415320358 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 21 Oct 2026 13:46:13 +0800 Subject: [PATCH 1/2] fix(directive): compile input and output names with lone surrogates oxc 0.155 stores string values as `JSStr`, which has no `&str` form for a lone surrogate (`'x-\uD800'`). Such an input or output member was dropped from the definition, and such an alias was an error. ngtsc reads them like any string and prints each lone code unit as a `\uXXXX` escape. Names, aliases and evaluated metadata strings now keep each lone code unit as two characters: the noncharacter U+FDD0, then U+10F800 + offset. The JS, `.d.ts` and JIT string printers turn that pair back into `\uXXXX`; either character on its own is printed as itself. A property name holding one is written as a key (`ctx["q-\uD800"]`), and an `ngAcceptInputType_` member holding one is quoted, where ngtsc's output is not valid JS / TS. A selector holding a lone surrogate is still an error: the selector parser can't read it. Closes #505 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/class_metadata/builders.rs | 19 +- .../src/component/transform.rs | 108 +++---- .../src/directive/decorator.rs | 45 +-- .../src/directive/dts_type.rs | 2 +- .../src/directive/evaluator.rs | 54 +--- .../oxc_angular_compiler/src/directive/mod.rs | 4 +- .../src/directive/property_decorators.rs | 125 +++----- crates/oxc_angular_compiler/src/dts.rs | 32 +- .../src/output/emitter.rs | 28 +- .../src/util/lone_surrogate.rs | 128 ++++++++ crates/oxc_angular_compiler/src/util/mod.rs | 1 + .../tests/integration_test.rs | 58 ++-- .../tests/jit_signal_initializer_test.rs | 41 ++- .../tests/lone_surrogate_names_test.rs | 289 ++++++++++++++++++ 14 files changed, 652 insertions(+), 282 deletions(-) create mode 100644 crates/oxc_angular_compiler/src/util/lone_surrogate.rs create mode 100644 crates/oxc_angular_compiler/tests/lone_surrogate_names_test.rs diff --git a/crates/oxc_angular_compiler/src/class_metadata/builders.rs b/crates/oxc_angular_compiler/src/class_metadata/builders.rs index 050d459bd..e4c7fd7b1 100644 --- a/crates/oxc_angular_compiler/src/class_metadata/builders.rs +++ b/crates/oxc_angular_compiler/src/class_metadata/builders.rs @@ -25,6 +25,7 @@ use crate::output::ast::{ use crate::output::oxc_converter::{ convert_oxc_expression, convert_plain_properties, is_plain_property, make_raw_source, }; +use crate::util::lone_surrogate::js_str_to_ident; /// Build the decorators metadata array expression. /// @@ -593,7 +594,7 @@ pub fn build_prop_decorators_metadata_in<'a>( continue; } - let Some((prop_name, quoted)) = member_key_name(key) else { + let Some((prop_name, quoted)) = member_key_name(allocator, key) else { continue; }; @@ -679,7 +680,7 @@ fn build_initializer_api_prop_decorators<'a>( ) -> Option> { let mut decorators = AllocVec::new_in(&allocator); - if let Some(input) = try_parse_signal_input(value, property_name.clone(), consts) { + if let Some(input) = try_parse_signal_input(allocator, value, property_name.clone(), consts) { // input() / input.required() → `Input({ isSignal, alias, required })` decorators.push(build_signal_input_decorator(allocator, namespace_registry, &input)); } else if let Some(model) = @@ -693,7 +694,8 @@ fn build_initializer_api_prop_decorators<'a>( "Output", model.output.1.clone(), )); - } else if let Some((_, binding)) = try_parse_signal_output(value, property_name.clone(), consts) + } else if let Some((_, binding)) = + try_parse_signal_output(allocator, value, property_name.clone(), consts) { // output() / outputFromObservable() → `Output("")` decorators.push(build_core_decorator_with_string_arg( @@ -1403,10 +1405,13 @@ fn get_property_key_name<'a>(key: &PropertyKey<'a>) -> Option> { /// string-literal key (`'x-y'`, `'x'`) is always quoted in the emitted object /// literal; a bare identifier is not. A no-substitution template literal /// counts too; private (`#x`), computed and numeric keys have no usable name -/// and return `None`, as does a string key holding a lone surrogate (it has -/// no `&str` form, so the member is left out). -fn member_key_name<'a>(key: &PropertyKey<'a>) -> Option<(Ident<'a>, bool)> { - let quoted = |value: oxc_str::JSStr<'a>| value.as_str().map(|name| (Ident::from(name), true)); +/// and return `None`. A lone surrogate (`'\uD800'`) is stored as +/// [`crate::util::lone_surrogate`] describes. +fn member_key_name<'a>( + allocator: &'a Allocator, + key: &PropertyKey<'a>, +) -> Option<(Ident<'a>, bool)> { + let quoted = |value: oxc_str::JSStr<'a>| Some((js_str_to_ident(value, allocator), true)); match key { PropertyKey::StaticIdentifier(id) => Some((id.name.into(), false)), PropertyKey::StringLiteral(lit) => quoted(lit.value), diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index f8d536ae5..54d1ff031 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -18,6 +18,7 @@ use oxc_span::{GetSpan, SourceType, Span}; use oxc_str::Ident; use crate::r3::CORE; +use crate::util::lone_surrogate::{Piece, js_str_to_str, pieces}; use rustc_hash::FxHashMap; use crate::optimizer::{Edit, apply_edits, apply_edits_with_sourcemap}; @@ -1287,7 +1288,6 @@ fn extract_all_jit_member_decorators( class: &oxc_ast::ast::Class<'_>, string_consts: &crate::directive::StringConsts<'_>, core_namespace: &str, - diagnostics: &mut std::vec::Vec, ) -> (std::vec::Vec, std::vec::Vec) { use oxc_ast::ast::{ClassElement, MethodDefinitionKind, PropertyKey}; @@ -1295,17 +1295,16 @@ fn extract_all_jit_member_decorators( let mut non_angular_members: std::vec::Vec = std::vec::Vec::new(); for element in &class.body.body { - // A string key with a lone surrogate (`'\uD800'`) has no `&str` form; - // like a computed or numeric key, it is skipped. + // A lone surrogate (`'\uD800'`) in a string key is stored as + // [`crate::util::lone_surrogate`] describes. let (member_name, quoted_key, is_static, is_property, decorators, initializer) = match element { ClassElement::PropertyDefinition(prop) => { let (name, quoted) = match &prop.key { PropertyKey::StaticIdentifier(id) => (id.name.to_string(), false), - PropertyKey::StringLiteral(s) => match s.value.as_str() { - Some(name) => (name.to_string(), true), - None => continue, - }, + PropertyKey::StringLiteral(s) => { + (js_str_to_str(s.value).into_owned(), true) + } _ => continue, }; (name, quoted, prop.r#static, true, &prop.decorators, prop.value.as_ref()) @@ -1316,10 +1315,9 @@ fn extract_all_jit_member_decorators( } let (name, quoted) = match &method.key { PropertyKey::StaticIdentifier(id) => (id.name.to_string(), false), - PropertyKey::StringLiteral(s) => match s.value.as_str() { - Some(name) => (name.to_string(), true), - None => continue, - }, + PropertyKey::StringLiteral(s) => { + (js_str_to_str(s.value).into_owned(), true) + } _ => continue, }; (name, quoted, method.r#static, false, &method.decorators, None) @@ -1327,10 +1325,9 @@ fn extract_all_jit_member_decorators( ClassElement::AccessorProperty(accessor) => { let (name, quoted) = match &accessor.key { PropertyKey::StaticIdentifier(id) => (id.name.to_string(), false), - PropertyKey::StringLiteral(s) => match s.value.as_str() { - Some(name) => (name.to_string(), true), - None => continue, - }, + PropertyKey::StringLiteral(s) => { + (js_str_to_str(s.value).into_owned(), true) + } _ => continue, }; (name, quoted, accessor.r#static, false, &accessor.decorators, None) @@ -1401,7 +1398,6 @@ fn extract_all_jit_member_decorators( &explicit_field_decorators, string_consts, core_namespace, - diagnostics, ); // Upstream inserts the synthesized decorator ahead of the member's // existing decorators (`[newDecorator, ...member.node.modifiers]`), @@ -1664,23 +1660,17 @@ fn synthesize_signal_api_decorators( existing: &rustc_hash::FxHashSet, string_consts: &crate::directive::StringConsts<'_>, core_namespace: &str, - diagnostics: &mut std::vec::Vec, ) -> std::vec::Vec { let unwrapped = unwrap_jit_initializer(initializer); let Expression::CallExpression(call) = unwrapped else { return std::vec::Vec::new() }; let Some(kind) = classify_initializer_api(&call.callee, string_consts) else { return std::vec::Vec::new(); }; - // The `alias` option, or the field name; `None` (after reporting the - // error) when the alias can't be read. - let mut alias_or_field_name = - |options_index: usize| match extract_string_option(call, options_index, "alias") { - Ok(alias) => Some(alias.unwrap_or_else(|| field_name.to_string())), - Err(error) => { - diagnostics.push(error); - None - } - }; + // The `alias` option, or the field name. + let alias_or_field_name = |options_index: usize| { + extract_string_option(call, options_index, "alias") + .unwrap_or_else(|| field_name.to_string()) + }; match kind { InitializerApiKind::Input | InitializerApiKind::InputRequired => { @@ -1690,9 +1680,7 @@ fn synthesize_signal_api_decorators( let required = matches!(kind, InitializerApiKind::InputRequired); // input(initial, options?) → options is args[1]; input.required(options?) → args[0]. let options_index = if required { 0 } else { 1 }; - let Some(alias) = alias_or_field_name(options_index) else { - return std::vec::Vec::new(); - }; + let alias = alias_or_field_name(options_index); let args = format!( "{{ isSignal: true, alias: \"{alias}\", required: {required}, transform: undefined }}", alias = escape_js_string(&alias), @@ -1710,9 +1698,7 @@ fn synthesize_signal_api_decorators( // output(options?) → args[0]; outputFromObservable(source, options?) → args[1]. let options_index = if matches!(kind, InitializerApiKind::OutputFromObservable) { 1 } else { 0 }; - let Some(alias) = alias_or_field_name(options_index) else { - return std::vec::Vec::new(); - }; + let alias = alias_or_field_name(options_index); let args = format!("\"{}\"", escape_js_string(&alias)); std::vec::Vec::from([JitParamDecorator { name: format!("{core_namespace}.Output"), @@ -1727,9 +1713,7 @@ fn synthesize_signal_api_decorators( let required = matches!(kind, InitializerApiKind::ModelRequired); // Same arg layout as input. let options_index = if required { 0 } else { 1 }; - let Some(alias) = alias_or_field_name(options_index) else { - return std::vec::Vec::new(); - }; + let alias = alias_or_field_name(options_index); let input_args = format!( "{{ isSignal: true, alias: \"{alias}\", required: {required}, transform: undefined }}", alias = escape_js_string(&alias), @@ -1808,17 +1792,15 @@ fn jit_classes_need_angular_core_namespace( } /// Pull a string option (e.g. `alias`) from the object literal at `args[options_index]`. -/// Returns `Ok(None)` if the argument isn't an object literal or the option is missing/non-string. -/// -/// A string with a lone surrogate (`'\uD800'`) has no `&str` form: it is an -/// error, as the AOT path's `literal_alias_error` reports it. +/// Returns `None` if the argument isn't an object literal or the option is missing/non-string. +/// A lone surrogate (`'\uD800'`) is stored as [`crate::util::lone_surrogate`] describes. fn extract_string_option( call: &oxc_ast::ast::CallExpression<'_>, options_index: usize, option_name: &str, -) -> Result, OxcDiagnostic> { - let Some(arg) = call.arguments.get(options_index) else { return Ok(None) }; - let Argument::ObjectExpression(obj) = arg else { return Ok(None) }; +) -> Option { + let arg = call.arguments.get(options_index)?; + let Argument::ObjectExpression(obj) = arg else { return None }; for prop in &obj.properties { let ObjectPropertyKind::ObjectProperty(prop) = prop else { continue }; let key_matches = match &prop.key { @@ -1830,22 +1812,28 @@ fn extract_string_option( continue; } if let Expression::StringLiteral(s) = &prop.value { - return match s.value.as_str() { - Some(value) => Ok(Some(value.to_string())), - None => Err(OxcDiagnostic::error( - "Alias needs to be a string that is statically analyzable.", - ) - .with_label(s.span)), - }; + return Some(js_str_to_str(s.value).into_owned()); } } - Ok(None) + None } /// Minimal JS-string escape for values embedded in synthesized propDecorator text. -/// Property names are TS identifiers or string literals, so only `\` and `"` matter. +/// Property names are TS identifiers or string literals, so only `\` and `"` +/// matter, and a stored lone surrogate is printed as its `\uXXXX` escape. fn escape_js_string(s: &str) -> String { - s.replace('\\', "\\\\").replace('"', "\\\"") + let mut out = String::with_capacity(s.len()); + for piece in pieces(s) { + match piece { + Piece::Char('\\') => out.push_str("\\\\"), + Piece::Char('"') => out.push_str("\\\""), + Piece::Char(c) => out.push(c), + Piece::Lone(unit) => { + let _ = std::fmt::Write::write_fmt(&mut out, format_args!("\\u{unit:04X}")); + } + } + } + out } /// Build the propDecorators static property text for JIT member decorator metadata. @@ -2542,7 +2530,6 @@ fn collect_jit_class_info<'a>( resource_imports: &mut std::vec::Vec<(String, String)>, string_consts: &crate::directive::StringConsts<'a>, core_namespace: &str, - diagnostics: &mut std::vec::Vec, ) -> JitClassInfo { let mut all_class_decorator_spans: std::vec::Vec = std::vec::Vec::new(); let mut all_class_decorator_texts: std::vec::Vec = std::vec::Vec::new(); @@ -2577,13 +2564,8 @@ fn collect_jit_class_info<'a>( extract_jit_ctor_params(source, class, string_consts); // Extract Angular and non-Angular member decorators - let (member_decorators, non_angular_member_decorators) = extract_all_jit_member_decorators( - source, - class, - string_consts, - core_namespace, - diagnostics, - ); + let (member_decorators, non_angular_member_decorators) = + extract_all_jit_member_decorators(source, class, string_consts, core_namespace); JitClassInfo { class_name, @@ -2735,7 +2717,7 @@ fn jit_class_edits( "__decorate([{}], {}, \"{}\", {});\n", member_dec.decorator_texts.join(", "), target, - member_dec.member_name, + escape_js_string(&member_dec.member_name), desc )); } @@ -2895,7 +2877,6 @@ fn transform_angular_file_jit( &mut resource_imports, &string_consts, core_namespace.as_str(), - &mut result.diagnostics, )); result.component_count += @@ -3376,7 +3357,6 @@ pub fn transform_angular_file( &mut jit_resource_imports, &string_consts, core_namespace.as_str(), - &mut result.diagnostics, ); // A co-located @Injectable still compiles: upstream's diff --git a/crates/oxc_angular_compiler/src/directive/decorator.rs b/crates/oxc_angular_compiler/src/directive/decorator.rs index 2c2cae5fd..482884037 100644 --- a/crates/oxc_angular_compiler/src/directive/decorator.rs +++ b/crates/oxc_angular_compiler/src/directive/decorator.rs @@ -25,6 +25,7 @@ use crate::output::ast::{OutputAstBuilder, OutputExpression, ReadVarExpr}; use crate::output::oxc_converter::convert_oxc_expression; use crate::r3::CORE; use crate::util::is_metadata_property; +use crate::util::lone_surrogate::has_lone_surrogate; /// Find the @Directive decorator in a list of decorators. With the file's /// `consts`, only Angular's (imported from `@angular/core`, see @@ -776,14 +777,6 @@ fn io_error(field: &str, message: impl FnOnce() -> String, value: &Value<'_>) -> value_error(&format!("@Directive.{field}"), message, value) } -/// Whether an object property's value is dynamic because it reads a string -/// with a lone surrogate (`'\uD800'`), as opposed to being absent or truly -/// dynamic. -fn reads_lone_surrogate<'a>(prop: &super::evaluator::Prop<'a>, consts: &StringConsts<'a>) -> bool { - prop.value.is_dynamic() - && prop.expr.is_some_and(|expr| Evaluator::reads_lone_surrogate(consts, expr)) -} - /// oxc's error for a transform written inside a function that the metadata /// calls, using that function's parameters (`transform: (v) => v + name`), so /// it would mean something else, or nothing, where the metadata is compiled. @@ -1007,13 +1000,6 @@ fn parse_input_object<'a>( return Some((io_error("inputs", String::new, value), span)); } } - // Nor an alias with a lone surrogate (`'\uD800'`), which evaluates as - // dynamic: an unreadable string item gets this error too. - if item.prop("alias").is_some_and(|alias| reads_lone_surrogate(alias, consts)) { - let message = - || "@Directive.inputs array can only contain strings or object literals".into(); - return Some((io_error("inputs", message, &Value::Dynamic), span)); - } let alias = item.prop("alias").and_then(|p| p.value.as_str()).unwrap_or(name); let required = matches!(item.prop("required").map(|p| &p.value), Some(Value::Bool(true))); let (transform_function, error) = match item.prop("transform") { @@ -1212,7 +1198,7 @@ pub fn decorator_io_errors<'a>( if let Some(error) = initializer_api_access_error(api, level, IO_ACCESS, call) { return Some(at(error)); } - match literal_alias_error(call, usize::from(!is_required)) { + match literal_alias_error(allocator, call, usize::from(!is_required)) { Ok(alias) => signal_alias = alias, Err(error) => return Some(at(error)), } @@ -1225,7 +1211,7 @@ pub fn decorator_io_errors<'a>( if let Some(error) = initializer_api_access_error(api, level, IO_ACCESS, call) { return Some(at(error)); } - match literal_alias_error(call, usize::from(!is_required)) { + match literal_alias_error(allocator, call, usize::from(!is_required)) { Ok(alias) => model_alias = alias, Err(error) => return Some(at(error)), } @@ -1363,7 +1349,7 @@ pub fn decorator_io_errors<'a>( return Some(at(error)); } let options_index = usize::from(api == OUTPUT_FROM_OBSERVABLE_API); - match literal_alias_error(call, options_index) { + match literal_alias_error(allocator, call, options_index) { Ok(alias) => output_alias = alias, Err(error) => return Some(at(error)), } @@ -1380,7 +1366,7 @@ pub fn decorator_io_errors<'a>( if let Some(error) = initializer_api_access_error(api, level, IO_ACCESS, call) { return Some(at(error)); } - match literal_alias_error(call, usize::from(!is_required)) { + match literal_alias_error(allocator, call, usize::from(!is_required)) { Ok(alias) => model_alias = alias, Err(error) => return Some(at(error)), } @@ -1526,11 +1512,18 @@ fn directive_selector_error<'a>( ) -> Option<(String, Span, Option)> { let expr = config.and_then(|config| config_property(config, "selector", consts))?; let resolved = evaluator.evaluate(expr); - let Value::String(selector) = &resolved else { + // oxc's selector parser can't read a lone surrogate (`'[a\uD800]'`), so + // such a selector stays an unreadable value rather than matching nothing. + let readable = match &resolved { + Value::String(selector) if !has_lone_surrogate(selector) => Some(selector), + _ => None, + }; + let Some(selector) = readable else { + let value = if matches!(resolved, Value::String(_)) { &Value::Dynamic } else { &resolved }; let message = value_error( &format!("@{decorator_name}.selector"), || "selector must be a string".to_string(), - &resolved, + value, ); return Some((message, expr.span(), None)); }; @@ -1724,16 +1717,6 @@ fn input_decorator_error<'a>( ); return Some((message, decorator.span)); } - // An `alias` with a lone surrogate (`'\uD800'`) evaluates as dynamic, - // which would read as no alias: report it like `@Input('\uD800')` rather - // than compile the class property name. - if options.prop("alias").is_some_and(|alias| reads_lone_surrogate(alias, consts)) { - let message = format!( - "{subject} decorator argument must resolve to a string or an object literal{}", - Value::Dynamic.wrong_type_suffix() - ); - return Some((message, decorator.span)); - } let transform = options.prop("transform")?; if let Some(error) = imported_transform_error(&subject, transform, span) { return Some(error); diff --git a/crates/oxc_angular_compiler/src/directive/dts_type.rs b/crates/oxc_angular_compiler/src/directive/dts_type.rs index 112c19db2..1231c115c 100644 --- a/crates/oxc_angular_compiler/src/directive/dts_type.rs +++ b/crates/oxc_angular_compiler/src/directive/dts_type.rs @@ -2085,7 +2085,7 @@ fn line_break_width(text: &str) -> usize { /// escapes for control characters and everything outside ASCII (as UTF-16 /// code units). pub(crate) fn quote(s: &str) -> String { - quote_units(s.encode_utf16()) + quote_units(crate::util::lone_surrogate::encode_utf16(s)) } /// [`quote`] for a string literal from the source, whose value can hold lone diff --git a/crates/oxc_angular_compiler/src/directive/evaluator.rs b/crates/oxc_angular_compiler/src/directive/evaluator.rs index 9ec8660f3..84dc567f6 100644 --- a/crates/oxc_angular_compiler/src/directive/evaluator.rs +++ b/crates/oxc_angular_compiler/src/directive/evaluator.rs @@ -12,7 +12,6 @@ use std::cell::{Cell, RefCell}; use std::collections::{HashMap, HashSet}; -use oxc_ast::StaticPropertyName; use oxc_ast::ast::{ Argument, ArrayExpression, ArrayExpressionElement, ArrowFunctionBody, ArrowFunctionExpression, BinaryExpression, BindingPattern, BlockStatement, CallExpression, CatchClause, ChainElement, @@ -38,6 +37,7 @@ use oxc_syntax::operator::{BinaryOperator, LogicalOperator, UnaryOperator}; use oxc_syntax::scope::ScopeFlags; use crate::output::emitter::format_number_like_js; +use crate::util::lone_surrogate::js_str_to_str; /// Everything declared at the top level of a file that the evaluator can resolve. #[derive(Default)] @@ -1642,45 +1642,17 @@ pub(crate) struct Evaluator<'s, 'a> { /// reference dynamic (ngtsc overflows its stack on those). variables: RefCell, Option<(Value<'a>, u32)>>>, fuel: Cell, - /// Set once a string with a lone surrogate (`'\uD800'`) is read: it has - /// no `&str` form and evaluates as dynamic. - lone_surrogate: Cell, } impl<'s, 'a> Evaluator<'s, 'a> { pub(crate) fn new(consts: &'s super::StringConsts<'a>) -> Self { - Self { - consts, - variables: RefCell::default(), - fuel: Cell::new(FUEL), - lone_surrogate: Cell::new(false), - } - } - - /// Whether evaluating `expr` reads a string with a lone surrogate - /// (`'\uD800'`), directly or through a `const`. Such a string evaluates - /// as dynamic; a reader that takes a dynamic value as absent (an input's - /// `alias`) reports it instead, so a string the source gives is never - /// silently replaced. - pub(crate) fn reads_lone_surrogate( - consts: &'s super::StringConsts<'a>, - expr: &'a Expression<'a>, - ) -> bool { - // A fresh evaluator, whose cache can't hold the `const` already. - let evaluator = Evaluator::new(consts); - evaluator.evaluate(expr); - evaluator.lone_surrogate.get() + Self { consts, variables: RefCell::default(), fuel: Cell::new(FUEL) } } - /// A string value, or dynamic for one with a lone surrogate. + /// A string value; a lone surrogate (`'\uD800'`) is stored as + /// [`crate::util::lone_surrogate`] describes. fn string(&self, value: JSStr<'_>) -> Value<'a> { - match value.as_str() { - Some(value) => Value::String(value.to_string()), - None => { - self.lone_surrogate.set(true); - Value::Dynamic - } - } + Value::String(js_str_to_str(value).into_owned()) } pub(crate) fn evaluate(&self, expr: &'a Expression<'a>) -> Value<'a> { @@ -1861,11 +1833,7 @@ impl<'s, 'a> Evaluator<'s, 'a> { let mut out = String::new(); for (i, quasi) in tpl.quasis.iter().enumerate() { let Some(cooked) = quasi.value.cooked else { return Value::Dynamic }; - let Some(cooked) = cooked.as_str() else { - self.lone_surrogate.set(true); - return Value::Dynamic; - }; - out.push_str(cooked); + out.push_str(&js_str_to_str(cooked)); if let Some(e) = tpl.expressions.get(i) { match literal(self.eval(e, depth, frame)) { Value::Dynamic => return Value::Dynamic, @@ -2825,7 +2793,7 @@ pub(crate) fn transform_error<'a>( && el .property_key() .and_then(PropertyKey::static_name) - .is_some_and(|n| n == conflicting.as_str()) + .is_some_and(|n| js_str_to_str(n.as_js_str()) == conflicting.as_str()) }) .then(|| { ( @@ -3651,14 +3619,14 @@ pub fn input_transform_types<'a>( ClassElement::MethodDefinition(m) => (&m.key, &m.decorators), _ => continue, }; - let Some(name) = key.static_name().and_then(StaticPropertyName::into_cow_str) else { - continue; - }; + // Named like the input, whose lone surrogates (`'\uD800'`) are stored + // as [`crate::util::lone_surrogate`] describes. + let Some(name) = key.static_name() else { continue }; if let Some(options) = super::property_decorators::input_decorator_options(decorators, consts) && let Some(transform) = evaluator.evaluate(options).prop("transform") { - record(name.to_string(), &transform.value); + record(js_str_to_str(name.as_js_str()).into_owned(), &transform.value); } } diff --git a/crates/oxc_angular_compiler/src/directive/mod.rs b/crates/oxc_angular_compiler/src/directive/mod.rs index a0725e1ed..7886d6030 100644 --- a/crates/oxc_angular_compiler/src/directive/mod.rs +++ b/crates/oxc_angular_compiler/src/directive/mod.rs @@ -41,9 +41,7 @@ pub use decorator::{ }; pub(crate) use decorator::{merge_by_class_property, parse_decorator_io}; pub use definition::{DirectiveDefinitions, generate_directive_definitions}; -pub(crate) use dts_type::{ - quote as ts_string_literal, quote_literal as quote_string_literal, quote_units, -}; +pub(crate) use dts_type::{quote as ts_string_literal, quote_literal as quote_string_literal}; pub use evaluator::input_transform_types; #[cfg(feature = "cross_file_elision")] pub(crate) use evaluator::{Evaluator, ImportValueResolver, StaticValue}; diff --git a/crates/oxc_angular_compiler/src/directive/property_decorators.rs b/crates/oxc_angular_compiler/src/directive/property_decorators.rs index 1066abc0b..fd453d00b 100644 --- a/crates/oxc_angular_compiler/src/directive/property_decorators.rs +++ b/crates/oxc_angular_compiler/src/directive/property_decorators.rs @@ -25,6 +25,7 @@ use crate::output::ast::OutputExpression; use crate::output::oxc_converter::{convert_oxc_expression, make_raw_source}; use crate::r3::CORE; use crate::util::is_metadata_property; +use crate::util::lone_surrogate::{js_str_to_ident, js_str_to_str}; // ============================================================================ // Helper Functions @@ -296,7 +297,9 @@ pub(crate) fn member_name(element: &ClassElement<'_>) -> Option { } match key { PropertyKey::StaticIdentifier(id) => Some(id.name.to_string()), - PropertyKey::StringLiteral(lit) => lit.value.as_str().map(str::to_string), + // A lone surrogate (`'\uD800'`) is stored as + // [`crate::util::lone_surrogate`] describes. + PropertyKey::StringLiteral(lit) => Some(js_str_to_str(lit.value).into_owned()), PropertyKey::PrivateIdentifier(id) => Some(format!("#{}", id.name)), _ => None, } @@ -357,6 +360,7 @@ pub(crate) fn initializer_api_access_error( /// no-substitution template literal — a resolved `const` doesn't count. /// `Ok(None)` when there is no such argument or no `alias`. pub(crate) fn literal_alias_error<'a>( + allocator: &'a Allocator, call: &'a CallExpression<'a>, options_index: usize, ) -> Result>, (String, Span)> { @@ -380,7 +384,7 @@ pub(crate) fn literal_alias_error<'a>( { continue; } - return match extract_string_value(&prop.value) { + return match extract_name_value(allocator, &prop.value) { Some(alias) => Ok(Some(alias)), None => Err(( "Alias needs to be a string that is statically analyzable.".to_string(), @@ -391,36 +395,6 @@ pub(crate) fn literal_alias_error<'a>( Ok(None) } -/// The predicate for a string selector with a lone surrogate (`'\uD800'`), -/// which has no `&str`: the selector list the emitter builds from a string -/// (split on commas, each part trimmed), each part with its exact UTF-16 -/// escapes. -fn lone_surrogate_selectors<'a>( - allocator: &'a Allocator, - lit: &oxc_ast::ast::StringLiteral<'_>, -) -> QueryPredicate<'a> { - use crate::output::ast::{LiteralArrayExpr, RawSourceExpr}; - - let units: std::vec::Vec = lit.value.encode_utf16().collect(); - let is_space = |unit: &u16| char::from_u32(u32::from(*unit)).is_some_and(char::is_whitespace); - let mut entries = Vec::new_in(&allocator); - for part in units.split(|&unit| unit == u16::from(b',')) { - let start = part.iter().position(|unit| !is_space(unit)).unwrap_or(part.len()); - let end = part.iter().rposition(|unit| !is_space(unit)).map_or(start, |i| i + 1); - if start < end { - let source = allocator.alloc_str(&super::quote_units(part[start..end].iter().copied())); - entries.push(OutputExpression::RawSource(oxc_allocator::Box::new_in( - RawSourceExpr { source: Ident::from(&*source), source_span: None }, - &allocator, - ))); - } - } - QueryPredicate::Type(OutputExpression::LiteralArray(oxc_allocator::Box::new_in( - LiteralArrayExpr { entries, source_span: None }, - &allocator, - ))) -} - /// Get the property key name as an Atom. /// /// Handles both identifier keys and string literal keys. @@ -445,6 +419,19 @@ fn extract_string_value<'a>(expr: &Expression<'a>) -> Option> { } } +/// [`extract_string_value`] for a name ngtsc reads like any string: a lone +/// surrogate (`'\uD800'`) is stored as [`crate::util::lone_surrogate`] +/// describes. +fn extract_name_value<'a>(allocator: &'a Allocator, expr: &Expression<'a>) -> Option> { + match expr { + Expression::StringLiteral(lit) => Some(js_str_to_ident(lit.value, allocator)), + Expression::TemplateLiteral(tpl) if tpl.expressions.is_empty() => { + tpl.quasis.first().and_then(|q| q.value.cooked).map(|c| js_str_to_ident(c, allocator)) + } + _ => None, + } +} + /// Extract a boolean value from an expression. fn extract_boolean_value(expr: &Expression<'_>) -> Option { match expr { @@ -604,7 +591,7 @@ fn parse_literal_input_config<'a>( match key_name.as_str() { "alias" => { - config.alias = extract_string_value(&prop.value); + config.alias = extract_name_value(allocator, &prop.value); } "required" => { config.required = extract_boolean_value(&prop.value).unwrap_or(false); @@ -663,7 +650,7 @@ pub(crate) fn try_parse_signal_model<'a>( // model(initialValue, options?) / model.required(options?) let options_arg_index = if is_required { 0 } else { 1 }; - let alias = options_alias(call_expr.arguments.get(options_arg_index)); + let alias = options_alias(allocator, call_expr.arguments.get(options_arg_index)); let binding_property_name = alias.unwrap_or_else(|| property_name.clone()); // Output binding name is always `bindingPropertyName + "Change"` @@ -683,7 +670,10 @@ pub(crate) fn try_parse_signal_model<'a>( } /// The string `alias` of an initializer API's options object literal. -fn options_alias<'a>(options: Option<&Argument<'a>>) -> Option> { +fn options_alias<'a>( + allocator: &'a Allocator, + options: Option<&Argument<'a>>, +) -> Option> { let Some(Argument::ObjectExpression(obj)) = options else { return None }; let mut alias = None; for prop in &obj.properties { @@ -694,7 +684,7 @@ fn options_alias<'a>(options: Option<&Argument<'a>>) -> Option> { && !prop.computed && get_property_key_name(&prop.key).is_some_and(|key| key == "alias") { - alias = extract_string_value(&prop.value); + alias = extract_name_value(allocator, &prop.value); } } alias @@ -715,6 +705,7 @@ fn options_alias<'a>(options: Option<&Argument<'a>>) -> Option> { /// /// Based on Angular's `output_function.ts` in the compiler-cli. pub(crate) fn try_parse_signal_output<'a>( + allocator: &'a Allocator, value: &Expression<'a>, property_name: Ident<'a>, consts: Option<&super::StringConsts<'_>>, @@ -732,7 +723,7 @@ pub(crate) fn try_parse_signal_output<'a>( // output() → options at index 0; outputFromObservable(obs, options?) → options at index 1 let options_idx = if api == OUTPUT_FROM_OBSERVABLE_API { 1 } else { 0 }; - let alias = options_alias(call_expr.arguments.get(options_idx)); + let alias = options_alias(allocator, call_expr.arguments.get(options_idx)); let binding_property_name = alias.unwrap_or_else(|| property_name.clone()); Some((property_name, binding_property_name)) @@ -768,6 +759,7 @@ pub(crate) fn unwrap_initializer_api_expr<'a, 'b>(expr: &'b Expression<'a>) -> & /// /// Based on Angular's `input_function.ts` in the compiler-cli. pub(crate) fn try_parse_signal_input<'a>( + allocator: &'a Allocator, value: &Expression<'a>, property_name: Ident<'a>, consts: Option<&super::StringConsts<'_>>, @@ -777,7 +769,7 @@ pub(crate) fn try_parse_signal_input<'a>( // input(initialValue, options?) / input.required(options?) let options_arg_index = if is_required { 0 } else { 1 }; - let alias = options_alias(call_expr.arguments.get(options_arg_index)); + let alias = options_alias(allocator, call_expr.arguments.get(options_arg_index)); let binding_property_name = alias.unwrap_or_else(|| property_name.clone()); Some(R3InputMetadata { @@ -862,7 +854,7 @@ pub fn extract_input_metadata_in<'a>( } // Then check for input() else if let Some(signal_input) = - try_parse_signal_input(value, property_name, consts) + try_parse_signal_input(allocator, value, property_name, consts) { inputs.push(signal_input); } @@ -1031,7 +1023,7 @@ pub fn extract_output_metadata_in<'a>( if let Some(property_name) = member_name_ident(allocator, element) { // Check for output() signal first if let Some(output_mapping) = - try_parse_signal_output(value, property_name.clone(), consts) + try_parse_signal_output(allocator, value, property_name.clone(), consts) { outputs.push(output_mapping); } @@ -1193,18 +1185,11 @@ fn parse_query_config<'a>( config.is_forward_ref = unwrapped.is_some(); let node = unwrapped.unwrap_or(first_arg); config.predicate = match node { - Expression::StringLiteral(lit) => match lit.value.as_str() { - Some(selector) => { - let mut list = Vec::new_in(&allocator); - list.push(Ident::from(selector)); - Some(QueryPredicate::Selectors(list)) - } - None => { - // Like a selector, a string isn't wrapped in `forwardRef`. - config.is_forward_ref = false; - Some(lone_surrogate_selectors(allocator, lit)) - } - }, + Expression::StringLiteral(lit) => { + let mut list = Vec::new_in(&allocator); + list.push(js_str_to_ident(lit.value, allocator)); + Some(QueryPredicate::Selectors(list)) + } _ => convert_oxc_expression(allocator, node, source_text) .or_else(|| make_raw_source(allocator, source_text, node.span())) .map(QueryPredicate::Type), @@ -1318,14 +1303,11 @@ fn try_parse_signal_query<'a>( let mut is_forward_ref = false; let predicate = match predicate_arg { // String selector: viewChild('myRef') - Argument::StringLiteral(lit) => match lit.value.as_str() { - Some(selector) => { - let mut selectors = oxc_allocator::Vec::new_in(&allocator); - selectors.push(Ident::from(selector)); - QueryPredicate::Selectors(selectors) - } - None => lone_surrogate_selectors(allocator, lit), - }, + Argument::StringLiteral(lit) => { + let mut selectors = oxc_allocator::Vec::new_in(&allocator); + selectors.push(js_str_to_ident(lit.value, allocator)); + QueryPredicate::Selectors(selectors) + } // Type predicate: viewChild(TemplateRef) or viewChild(forwardRef(() => MyClass)) _ => { // ngtsc emits a non-string locator as written (`WrappedNodeExpr`); @@ -2452,7 +2434,7 @@ fn decorator_query<'a>( return Err((format!("@{name} must have arguments"), span)); }; let unwrapped = try_unwrap_forward_ref(first, Some(consts)); - let mut is_forward_ref = unwrapped.is_some(); + let is_forward_ref = unwrapped.is_some(); let node = unwrapped.unwrap_or(first); let at_node = |message: String| (message, node.span()); let predicate = match evaluator.evaluate(node) { @@ -2460,21 +2442,12 @@ fn decorator_query<'a>( // evaluated is wrapped in a `WrappedNodeExpr` and emitted as written, // so the source text is the fallback for nodes the output AST can't // represent (`class Foo {}`, an import, ...). - Value::Reference { .. } | Value::Dynamic | Value::Function(_) => match node { - // A string with a lone surrogate (`'\uD800'`) has no `&str`, so - // the evaluator can't read it, but it is still a selector. Like - // one, it isn't wrapped in `forwardRef`. - Expression::StringLiteral(lit) if lit.value.has_lone_surrogate() => { - is_forward_ref = false; - lone_surrogate_selectors(allocator, lit) - } - _ => { - let expr = convert_oxc_expression(allocator, node, source_text) - .or_else(|| make_raw_source(allocator, source_text, node.span())) - .ok_or_else(|| at_node(format!("@{name} predicate cannot be interpreted")))?; - QueryPredicate::Type(expr) - } - }, + Value::Reference { .. } | Value::Dynamic | Value::Function(_) => { + let expr = convert_oxc_expression(allocator, node, source_text) + .or_else(|| make_raw_source(allocator, source_text, node.span())) + .ok_or_else(|| at_node(format!("@{name} predicate cannot be interpreted")))?; + QueryPredicate::Type(expr) + } Value::String(s) => { let mut selectors = Vec::new_in(&allocator); selectors.push(Ident::from(allocator.alloc_str(&s))); diff --git a/crates/oxc_angular_compiler/src/dts.rs b/crates/oxc_angular_compiler/src/dts.rs index 4f50cd203..e45a4fb41 100644 --- a/crates/oxc_angular_compiler/src/dts.rs +++ b/crates/oxc_angular_compiler/src/dts.rs @@ -23,6 +23,7 @@ use crate::injectable::InjectableMetadata; use crate::ng_module::NgModuleMetadata; use crate::pipe::PipeMetadata; use crate::service::ServiceMetadata; +use crate::util::lone_surrogate::{Piece, has_lone_surrogate, pieces}; use oxc_str::Ident; use std::collections::HashMap; @@ -637,8 +638,14 @@ fn generate_input_transform_fields( let name = input.class_property_name.as_str(); let field = format!("ngAcceptInputType_{name}"); // ngtsc quotes the name only when Angular's `isUnsafeObjectKey` - // (`/[-.]/`) matches; any other name is printed as written. - let field = if field.contains(['-', '.']) { ts_string_literal(&field) } else { field }; + // (`/[-.]/`) matches; any other name is printed as written. A lone + // surrogate is quoted too: ngtsc writes it raw, as U+FFFD, which + // is not valid TypeScript. + let field = if field.contains(['-', '.']) || has_lone_surrogate(&field) { + ts_string_literal(&field) + } else { + field + }; let ty = accept_types.get(name).map_or("unknown", String::as_str); members.push_str(&format!("\nstatic {field}: {ty};")); } @@ -823,11 +830,22 @@ fn extract_directive_name_from_expr(expr: &crate::output::ast::OutputExpression) /// Escape a string for use in a TypeScript `.d.ts` string literal type. fn escape_dts_string(s: &str) -> String { - s.replace('\\', "\\\\") - .replace('"', "\\\"") - .replace('\n', "\\n") - .replace('\r', "\\r") - .replace('\t', "\\t") + let mut out = String::with_capacity(s.len()); + for piece in pieces(s) { + match piece { + Piece::Char('\\') => out.push_str("\\\\"), + Piece::Char('"') => out.push_str("\\\""), + Piece::Char('\n') => out.push_str("\\n"), + Piece::Char('\r') => out.push_str("\\r"), + Piece::Char('\t') => out.push_str("\\t"), + Piece::Char(c) => out.push(c), + // A stored lone surrogate is printed as its `\uXXXX` escape. + Piece::Lone(unit) => { + let _ = std::fmt::Write::write_fmt(&mut out, format_args!("\\u{unit:04X}")); + } + } + } + out } #[cfg(test)] diff --git a/crates/oxc_angular_compiler/src/output/emitter.rs b/crates/oxc_angular_compiler/src/output/emitter.rs index feef6736e..edf5a5bd3 100644 --- a/crates/oxc_angular_compiler/src/output/emitter.rs +++ b/crates/oxc_angular_compiler/src/output/emitter.rs @@ -23,6 +23,7 @@ use super::ast::{ ArrowFunctionBody, BinaryOperator, DeclareVarStmt, DynamicImportUrl, FnParam, LeadingComment, LiteralValue, OutputExpression, OutputStatement, UnaryOperator, }; +use crate::util::lone_surrogate::{Piece, has_lone_surrogate, pieces}; use crate::util::{ParseSourceFile, ParseSourceSpan}; // ============================================================================ @@ -663,14 +664,19 @@ impl JsEmitter { } OutputExpression::ReadProp(e) => { self.visit_expression(&e.receiver, ctx); - if e.optional { - ctx.print("?."); - } else { - ctx.print("."); - } // Map the property name to its source location let prop_span = e.source_span.and_then(|span| ctx.span_to_source_span(span)); - ctx.print_with_span(&e.name, prop_span); + // A name with a stored lone surrogate (`'q\uD800'`) can only be + // written as a key (ngtsc prints it raw, as U+FFFD). + if has_lone_surrogate(&e.name) { + ctx.print(if e.optional { "?.[" } else { "[" }); + let key = escape_string(&e.name, self.escape_dollar_in_strings); + ctx.print_with_span(&key, prop_span); + ctx.print("]"); + } else { + ctx.print(if e.optional { "?." } else { "." }); + ctx.print_with_span(&e.name, prop_span); + } } OutputExpression::ReadKey(e) => { self.visit_expression(&e.receiver, ctx); @@ -1279,7 +1285,15 @@ fn is_nullish_coalesce(expr: &OutputExpression<'_>) -> bool { pub(crate) fn escape_string(input: &str, escape_dollar: bool) -> String { let mut result = String::with_capacity(input.len() + 2); result.push('"'); - for c in input.chars() { + for piece in pieces(input) { + let c = match piece { + // A stored lone surrogate is printed as its `\uXXXX` escape. + Piece::Lone(unit) => { + push_unicode_escape(&mut result, u32::from(unit)); + continue; + } + Piece::Char(c) => c, + }; match c { '"' => result.push_str("\\\""), '\\' => result.push_str("\\\\"), diff --git a/crates/oxc_angular_compiler/src/util/lone_surrogate.rs b/crates/oxc_angular_compiler/src/util/lone_surrogate.rs new file mode 100644 index 000000000..68e48e04d --- /dev/null +++ b/crates/oxc_angular_compiler/src/util/lone_surrogate.rs @@ -0,0 +1,128 @@ +//! Strings with lone surrogates (`'\uD800'`). +//! +//! A JavaScript string can hold a lone surrogate code unit, which a Rust +//! `&str` can't. ngtsc reads such a string like any other and prints it as a +//! `\uXXXX` escape. To carry one through the compiler's `&str` names, each lone +//! code unit `0xD800 + k` is stored as two characters: the noncharacter +//! U+FDD0, which Unicode reserves for a program's internal use, then the +//! private-use character `U+10F800 + k`. The string printers turn that pair +//! back into `\uXXXX`; either character on its own is printed as itself. + +use std::borrow::Cow; + +use oxc_allocator::Allocator; +use oxc_str::{Ident, JSStr}; + +/// The first character of a stored lone surrogate. +const MARK: char = '\u{FDD0}'; +/// The second character of a stored lone code unit `0xD800`. +const BASE: u32 = 0x10_F800; + +/// `value` as a `&str`, with each lone surrogate stored as described above. +pub(crate) fn js_str_to_str(value: JSStr<'_>) -> Cow<'_, str> { + if let Some(value) = value.as_str() { + return Cow::Borrowed(value); + } + let mut out = String::with_capacity(value.len() + 4); + for c in value.chars() { + match c.to_char() { + Some(c) => out.push(c), + None => { + out.push(MARK); + out.push(char::from_u32(BASE + (c.to_u32() - 0xD800)).unwrap_or(MARK)); + } + } + } + Cow::Owned(out) +} + +/// [`js_str_to_str`] as an [`Ident`]. +pub(crate) fn js_str_to_ident<'a>(value: JSStr<'a>, allocator: &'a Allocator) -> Ident<'a> { + match js_str_to_str(value) { + Cow::Borrowed(value) => Ident::from(value), + Cow::Owned(value) => Ident::from(allocator.alloc_str(&value)), + } +} + +/// A character of a string, or a stored lone surrogate. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum Piece { + Char(char), + Lone(u16), +} + +/// The pieces of `s`: its characters, with each stored lone surrogate as one +/// [`Piece::Lone`]. +pub(crate) fn pieces(s: &str) -> impl Iterator + '_ { + let mut chars = s.chars().peekable(); + std::iter::from_fn(move || { + let c = chars.next()?; + if c == MARK + && let Some(unit) = chars.peek().and_then(|&next| stored_unit(next)) + { + chars.next(); + return Some(Piece::Lone(unit)); + } + Some(Piece::Char(c)) + }) +} + +/// Whether `s` stores a lone surrogate. +pub(crate) fn has_lone_surrogate(s: &str) -> bool { + s.contains(MARK) && pieces(s).any(|piece| matches!(piece, Piece::Lone(_))) +} + +/// The UTF-16 code units of `s`, with each stored lone surrogate as its unit. +pub(crate) fn encode_utf16(s: &str) -> impl Iterator + '_ { + pieces(s) + .flat_map(|piece| match piece { + Piece::Lone(unit) => [Some(unit), None], + Piece::Char(c) => { + let mut buf = [0; 2]; + let units = c.encode_utf16(&mut buf); + [Some(units[0]), units.get(1).copied()] + } + }) + .flatten() +} + +/// The lone code unit the second character `c` of a stored one stands for. +fn stored_unit(c: char) -> Option { + // `BASE + 0x7FF` is `U+10FFFF`, the last code point. + let offset = u16::try_from((c as u32).checked_sub(BASE)?).ok()?; + Some(0xD800 + offset) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn round_trips_lone_surrogates() { + let allocator = Allocator::default(); + let mut builder = oxc_str::JSStrBuilder::new_in(&&allocator); + builder.push_str("x-"); + builder.push_code_unit(0xD800); + builder.push('-'); + builder.push_code_unit(0xDFFF); + let value = builder.into_js_str(); + let text = js_str_to_str(value); + assert!(has_lone_surrogate(&text)); + assert_eq!(encode_utf16(&text).collect::>(), [0x78, 0x2D, 0xD800, 0x2D, 0xDFFF]); + } + + #[test] + fn leaves_other_strings_alone() { + assert!(matches!(js_str_to_str(JSStr::from("a😀\u{FFFD}")), Cow::Borrowed("a😀\u{FFFD}"))); + // Either character on its own, or in the other order, is itself. + for text in + ["\u{10F800}", "\u{FDD0}", "\u{FDD0}a", "\u{10F800}\u{FDD0}", "\u{FDD0}\u{10F7FF}"] + { + assert!(!has_lone_surrogate(text), "{text:?}"); + assert_eq!( + encode_utf16(text).collect::>(), + text.encode_utf16().collect::>() + ); + } + } +} diff --git a/crates/oxc_angular_compiler/src/util/mod.rs b/crates/oxc_angular_compiler/src/util/mod.rs index 4a58bd698..c58741771 100644 --- a/crates/oxc_angular_compiler/src/util/mod.rs +++ b/crates/oxc_angular_compiler/src/util/mod.rs @@ -2,6 +2,7 @@ pub mod chars; mod deferred_time; +pub(crate) mod lone_surrogate; mod parse_util; mod type_extract; diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 837ba7251..7e04db03b 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -17665,10 +17665,9 @@ fn test_component_with_lone_surrogate_key_still_compiles() { #[test] fn test_lone_surrogate_query_selector_kept_exact() { - // A string query predicate with a lone surrogate (`'\uD800'`) has no - // `&str` form: it still becomes a selector list (split on commas, each - // part trimmed), with exact escapes, not a lossy rendering or a bare - // string token. + // A string query predicate with a lone surrogate (`'\uD800'`) becomes a + // selector list like any other (split on commas, each part trimmed, + // pooled), with exact escapes, as ngtsc emits `const _c0 = ["\uD800"]`. let result = expect_result( "import { Component, ViewChild, viewChild } from '@angular/core';\n\ @Component({ selector: 'app-c', template: '' })\n\ @@ -17678,30 +17677,24 @@ fn test_lone_surrogate_query_selector_kept_exact() { }", ); assert!(result.diagnostics.is_empty(), "{:?}", result.diagnostics); - assert!( - result.code.contains(r#"i0.ɵɵviewQuerySignal(ctx.b,["x","\uDC00"],5);"#), - "{}", - result.code - ); - assert!(result.code.contains(r#"i0.ɵɵviewQuery(["\uD800"],5);"#), "{}", result.code); -} - -#[test] -fn test_lone_surrogate_input_alias_is_diagnostic() { - // An `alias` with a lone surrogate (`'\uD800'`) has no `&str` form. It is - // present but unreadable, not absent: report it like the positional - // `@Input('\uD800')` instead of compiling the class property name. - let input_error = "@Input decorator argument must resolve to a string or an object \ - literal Value could not be determined statically."; - let inputs_error = "@Directive.inputs array can only contain strings or object literals \ - Value could not be determined statically."; - for (decorator, member, expected) in [ - ("", r"@Input('\uD800') value: any;", input_error), - ("", r"@Input({ alias: '\uD800', required: true }) value: any;", input_error), - ("", r"@Input({ alias: `\uD800` }) value: any;", input_error), - ("", "@Input({ alias: C }) value: any;", input_error), - ("", "@Input(OPTS) value: any;", input_error), - (r", inputs: [{ name: 'value', alias: '\uD800' }]", "value: any;", inputs_error), + assert!(result.code.contains(r#"const _c0 = ["x","\uDC00"];"#), "{}", result.code); + assert!(result.code.contains(r#"const _c1 = ["\uD800"];"#), "{}", result.code); + assert!(result.code.contains("i0.ɵɵviewQuerySignal(ctx.b,_c0,5);"), "{}", result.code); + assert!(result.code.contains("i0.ɵɵviewQuery(_c1,5);"), "{}", result.code); +} + +#[test] +fn test_lone_surrogate_input_alias_is_compiled() { + // An `alias` with a lone surrogate (`'\uD800'`) is read like any string, + // directly or through a `const`, and printed as its escape, as ngtsc + // does: `inputs: { value: [0, "\uD800", "value"] }`. + for (decorator, member) in [ + ("", r"@Input('\uD800') value: any;"), + ("", r"@Input({ alias: '\uD800', required: true }) value: any;"), + ("", r"@Input({ alias: `\uD800` }) value: any;"), + ("", "@Input({ alias: C }) value: any;"), + ("", "@Input(OPTS) value: any;"), + (r", inputs: [{ name: 'value', alias: '\uD800' }]", "value: any;"), ] { let source = format!( "import {{ Directive, Input }} from '@angular/core';\n\ @@ -17710,7 +17703,14 @@ fn test_lone_surrogate_input_alias_is_diagnostic() { @Directive({{ selector: '[d]'{decorator} }})\n\ export class D {{\n {member}\n}}" ); - assert_eq!(expect_diagnostics(&source), [expected], "`{decorator}` `{member}`"); + let result = expect_result(&source); + assert!(result.diagnostics.is_empty(), "`{member}`: {:?}", result.diagnostics); + let compact: String = result.code.chars().filter(|c| !c.is_whitespace()).collect(); + assert!( + compact.contains(r#"inputs:{value:[0,"\uD800","value"]}"#), + "`{decorator}` `{member}`\n{}", + result.code + ); } // A truly dynamic alias still reads as no alias, like ngtsc. let diagnostics = expect_diagnostics( diff --git a/crates/oxc_angular_compiler/tests/jit_signal_initializer_test.rs b/crates/oxc_angular_compiler/tests/jit_signal_initializer_test.rs index 12a68d6c5..848ba636e 100644 --- a/crates/oxc_angular_compiler/tests/jit_signal_initializer_test.rs +++ b/crates/oxc_angular_compiler/tests/jit_signal_initializer_test.rs @@ -417,13 +417,29 @@ fn synthesized_decorator_precedes_explicit_member_decorators() { } #[test] -fn lone_surrogate_alias_is_rejected_like_aot() { - // An alias with a lone surrogate (`'\uD800'`) has no `&str` form: JIT - // rejects it with the AOT path's error rather than lowering a lossy alias. - for (field, imports) in [ - (r" readonly value = input(0, { alias: '\uD800' });", "input"), - (r" readonly ready = output({ alias: '\uDC00' });", "output"), - (r" readonly m = model.required({ alias: '\uDBFF' });", "model"), +fn lone_surrogate_alias_is_lowered_exactly() { + // An alias with a lone surrogate (`'\uD800'`) is read like any string, in + // JIT as in AOT, and printed as its escape (JIT output is reprinted by + // oxc's codegen, which writes lowercase hex). + for (field, imports, jit_alias, aot_alias) in [ + ( + r" readonly value = input(0, { alias: '\uD800' });", + "input", + r#"alias:"\ud800""#, + r#"inputs:{value:[1,"\uD800","value"]}"#, + ), + ( + r" readonly ready = output({ alias: '\uDC00' });", + "output", + r#"args:["\udc00"]"#, + r#"outputs:{ready:"\uDC00"}"#, + ), + ( + r" readonly m = model.required({ alias: '\uDBFF' });", + "model", + r#"args:["\udbffChange"]"#, + r#"outputs:{m:"\uDBFFChange"}"#, + ), ] { let source = component(field, imports); for jit in [true, false] { @@ -436,13 +452,10 @@ fn lone_surrogate_alias_is_rejected_like_aot() { Some(&options), None, ); - let messages: Vec = result.diagnostics.iter().map(|d| format!("{d}")).collect(); - assert_eq!( - messages, - ["Alias needs to be a string that is statically analyzable."], - "jit: {jit}, `{field}`\n{}", - result.code - ); + assert!(result.diagnostics.is_empty(), "jit: {jit}: {:?}", result.diagnostics); + let compact: String = result.code.chars().filter(|c| !c.is_whitespace()).collect(); + let expected = if jit { jit_alias } else { aot_alias }; + assert!(compact.contains(expected), "jit: {jit}, `{field}`\n{}", result.code); } } } diff --git a/crates/oxc_angular_compiler/tests/lone_surrogate_names_test.rs b/crates/oxc_angular_compiler/tests/lone_surrogate_names_test.rs new file mode 100644 index 000000000..c2e21c35a --- /dev/null +++ b/crates/oxc_angular_compiler/tests/lone_surrogate_names_test.rs @@ -0,0 +1,289 @@ +//! Input and output names holding a lone surrogate (`'x-\uD800'`, #505). +//! +//! ngtsc reads them like any other string and prints each lone code unit as +//! a `\uXXXX` escape. The expected fragments below are ngtsc 22.2's output +//! with the whitespace removed. + +use oxc_allocator::Allocator; +use oxc_angular_compiler::{ + CompilationMode, TransformOptions, TransformResult, transform_angular_file, +}; + +fn compile(source: &str, options: TransformOptions) -> TransformResult { + let allocator = Allocator::default(); + let result = transform_angular_file(&allocator, "t.ts", source, Some(&options), None); + assert!(result.diagnostics.is_empty(), "{:?}", result.diagnostics); + result +} + +fn compact(text: &str) -> String { + text.chars().filter(|c| !c.is_whitespace()).collect() +} + +/// The `.d.ts` members of `class_name`, whitespace removed. +fn dts(result: &TransformResult, class_name: &str) -> String { + let decl = result.dts_declarations.iter().find(|d| d.class_name == class_name).unwrap(); + compact(&decl.members) +} + +fn assert_has(haystack: &str, needles: &[&str]) { + for needle in needles { + assert!(haystack.contains(needle), "missing `{needle}` in\n{haystack}"); + } +} + +const DECORATORS: &str = r"import { Directive, EventEmitter, Input, Output } from '@angular/core'; +@Directive({ selector: '[d]' }) +export class D { + @Input() 'x-\uD800': any; + @Input({ transform: (v: string) => 1 }) 'y-\uD800': number = 0; + @Input('z-\uDC00') z: any; + @Input({ alias: 'w-\uD800' }) w: any; + @Output() 'o-\uD800' = new EventEmitter(); + @Output('p-\uD800') p = new EventEmitter(); +} +"; + +#[test] +fn decorator_inputs_and_outputs() { + let result = compile(DECORATORS, TransformOptions::default()); + assert_has( + &compact(&result.code), + &[ + r#"inputs:{"x-\uD800":"x-\uD800","y-\uD800":[2,"y-\uD800","y-\uD800",(v)=>1],z:[0,"z-\uDC00","z"],w:[0,"w-\uD800","w"]}"#, + r#"outputs:{"o-\uD800":"o-\uD800",p:"p-\uD800"}"#, + // `setClassMetadata` keys; the decorator arguments are the source text. + r#"{"x-\uD800":[{type:Input}],"y-\uD800":[{type:Input,args:[{transform:(v)=>1}]}],z:[{type:Input,args:['z-\uDC00']}]"#, + r#""o-\uD800":[{type:Output}],p:[{type:Output,args:['p-\uD800']}]"#, + ], + ); + assert_has( + &dts(&result, "D"), + &[ + r#"{"x-\uD800":{"alias":"x-\uD800";"required":false;};"y-\uD800":{"alias":"y-\uD800";"required":false;};"z":{"alias":"z-\uDC00";"required":false;};"w":{"alias":"w-\uD800";"required":false;};},{"o-\uD800":"o-\uD800";"p":"p-\uD800";}"#, + r#"static"ngAcceptInputType_y-\uD800":string;"#, + ], + ); +} + +#[test] +fn partial_declaration() { + let options = + TransformOptions { compilation_mode: CompilationMode::Partial, ..Default::default() }; + let result = compile(DECORATORS, options); + assert_has( + &compact(&result.code), + &[ + r#"inputs:{"x-\uD800":"x-\uD800","y-\uD800":["y-\uD800","y-\uD800",(v)=>1],z:["z-\uDC00","z"],w:["w-\uD800","w"]}"#, + r#"outputs:{"o-\uD800":"o-\uD800",p:"p-\uD800"}"#, + ], + ); +} + +#[test] +fn signal_apis() { + let result = compile( + r"import { Directive, input, model, output } from '@angular/core'; +@Directive({ selector: '[s]' }) +export class S { + 'a-\uD800' = input(); + b = input('', { alias: 'b-\uD800' }); + 'm-\uD800' = model(0); + c = output({ alias: 'c-\uD800' }); +} +", + TransformOptions::default(), + ); + assert_has( + &compact(&result.code), + &[ + r#"inputs:{"a-\uD800":[1,"a-\uD800"],b:[1,"b-\uD800","b"],"m-\uD800":[1,"m-\uD800"]}"#, + r#"outputs:{"m-\uD800":"m-\uD800Change",c:"c-\uD800"}"#, + ], + ); + assert_has( + &dts(&result, "S"), + &[r#""a-\uD800":{"alias":"a-\uD800";"required":false;"isSignal":true;}"#], + ); +} + +#[test] +fn metadata_arrays() { + let result = compile( + r"import { Directive, EventEmitter } from '@angular/core'; +@Directive({ + selector: '[e]', + inputs: ['q-\uD800', 'r: r-\uD800', { name: 's', alias: 's-\uD800' }], + outputs: ['t-\uD800'], +}) +export class E { 'q-\uD800': any; r: any; s: any; 't-\uD800' = new EventEmitter(); } +", + TransformOptions::default(), + ); + assert_has( + &compact(&result.code), + &[ + r#"inputs:{"q-\uD800":"q-\uD800",r:[0,"r-\uD800","r"],s:[0,"s-\uD800","s"]}"#, + r#"outputs:{"t-\uD800":"t-\uD800"}"#, + ], + ); +} + +#[test] +fn escape_case_and_position() { + // A lowercase escape prints uppercase, as ngtsc's generated strings do. A + // key starting or ending with a lone unit stays quoted (ngtsc writes it + // unquoted, as U+FFFD, which is not valid JavaScript). + let result = compile( + r"import { Directive, Input } from '@angular/core'; +@Directive({ selector: '[m]' }) +export class M { + @Input() 'l-\ud800': any; + @Input() '\uD800start': any; + @Input() 'end\uDC00': any; + @Input('lc-\ud800') lc: any; +} +", + TransformOptions::default(), + ); + assert_has( + &compact(&result.code), + &[ + r#"inputs:{"l-\uD800":"l-\uD800","\uD800start":"\uD800start","end\uDC00":"end\uDC00",lc:[0,"lc-\uD800","lc"]}"#, + r#"lc:[{type:Input,args:['lc-\ud800']}]"#, + ], + ); +} + +#[test] +fn jit_member_names() { + let result = compile( + r"import { Directive, Input, input } from '@angular/core'; +@Directive({ selector: '[j]' }) +export class J { + @Input() 'x-\uD800': any; + 'a-\uD800' = input(); +} +", + TransformOptions { jit: true, ..Default::default() }, + ); + // JIT output is reprinted by oxc's codegen, which writes lowercase hex. + assert_has( + &compact(&result.code), + &[ + r#""x-\ud800":[{type:Input}]"#, + r#""a-\ud800":[{type:i0.Input,args:[{isSignal:true,alias:"a-\ud800""#, + ], + ); +} + +#[test] +fn host_attribute_value() { + // Metadata strings are read the same way: ngtsc emits + // `hostAttrs: ["title", "v\uD800"]`. + let result = compile( + r"import { Directive } from '@angular/core'; +@Directive({ selector: '[h]', host: { title: 'v\uD800' } }) +export class H {} +", + TransformOptions::default(), + ); + assert_has(&compact(&result.code), &[r#"hostAttrs:["title","v\uD800"]"#]); +} + +#[test] +fn selector_with_a_lone_surrogate_is_an_error() { + // oxc's selector parser can't read it, so it is reported rather than + // compiled to a directive that matches nothing. + let allocator = Allocator::default(); + let result = transform_angular_file( + &allocator, + "t.ts", + r"import { Directive } from '@angular/core'; +@Directive({ selector: '[k\uD800]' }) +export class K {} +", + None, + None, + ); + let messages: Vec = result.diagnostics.iter().map(|d| d.message.to_string()).collect(); + assert_eq!(messages, ["selector must be a string Value could not be determined statically."]); +} + +#[test] +fn private_use_characters_are_printed_as_themselves() { + // A real U+10F800 (private use) or U+FDD0 (noncharacter) is not a stored + // lone surrogate, alone or next to each other the other way round. + let source = "import { Component, Input } from '@angular/core'; +@Component({ selector: 'p', template: '\u{10F800}\u{FDD0}' }) +export class P { + @Input() 'a\u{10F800}': any; + @Input() 'b\u{FDD0}': any; + @Input() 'c\u{10F800}\u{FDD0}': any; +} +"; + let result = compile(source, TransformOptions::default()); + assert_has( + &compact(&result.code), + &[ + "\"a\u{10F800}\":\"a\u{10F800}\"", + "\"b\u{FDD0}\":\"b\u{FDD0}\"", + "\"c\u{10F800}\u{FDD0}\":\"c\u{10F800}\u{FDD0}\"", + "\"\u{10F800}\u{FDD0}\"", + ], + ); + assert!(!compact(&result.code).contains(r"\uD800"), "{}", result.code); +} + +#[test] +fn accept_type_member_is_quoted() { + // ngtsc prints `ngAcceptInputType_`, which is not valid TS. + let result = compile( + r"import { Directive, Input } from '@angular/core'; +@Directive({ selector: '[a]' }) +export class A { @Input({ transform: (v: string) => 1 }) '\uD800': number = 0; } +", + TransformOptions::default(), + ); + assert_has(&dts(&result, "A"), &[r#"static"ngAcceptInputType_\uD800":string;"#]); +} + +#[test] +fn query_writes_use_a_key() { + // ngtsc prints `ctx.q-`, which is not valid JS. + let result = compile( + r"import { Component, ViewChild, viewChild } from '@angular/core'; +@Component({ selector: 'q', template: '' }) +export class Q { @ViewChild('r') 'q-\uD800': any; 'v\uD800' = viewChild('r'); } +", + TransformOptions::default(), + ); + assert_has( + &compact(&result.code), + &[r#"ɵɵviewQuerySignal(ctx["v\uD800"],"#, r#"(ctx["q-\uD800"]=_t.first)"#], + ); +} + +#[test] +fn accept_type_member_clash_is_reported() { + let allocator = Allocator::default(); + let result = transform_angular_file( + &allocator, + "t.ts", + r"import { Directive, Input } from '@angular/core'; +@Directive({ selector: '[c]' }) +export class C { + @Input({ transform: (v: string) => 1 }) 'q\uD800': number = 0; + static 'ngAcceptInputType_q\uD800': string; +} +", + None, + None, + ); + assert_eq!(result.diagnostics.len(), 1, "{:?}", result.diagnostics); + assert!( + result.diagnostics[0].message.contains("Class cannot have both a transform function"), + "{:?}", + result.diagnostics + ); +} From c6da2db7c54749ac99ef00954dea0722b8611091 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 21 Oct 2026 13:55:37 +0800 Subject: [PATCH 2/2] fix(directive): compare evaluated strings by their real code units Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/directive/evaluator.rs | 7 ++++--- .../src/util/lone_surrogate.rs | 4 ++++ .../tests/lone_surrogate_names_test.rs | 16 ++++++++++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/crates/oxc_angular_compiler/src/directive/evaluator.rs b/crates/oxc_angular_compiler/src/directive/evaluator.rs index 84dc567f6..f24bc9605 100644 --- a/crates/oxc_angular_compiler/src/directive/evaluator.rs +++ b/crates/oxc_angular_compiler/src/directive/evaluator.rs @@ -37,7 +37,7 @@ use oxc_syntax::operator::{BinaryOperator, LogicalOperator, UnaryOperator}; use oxc_syntax::scope::ScopeFlags; use crate::output::emitter::format_number_like_js; -use crate::util::lone_surrogate::js_str_to_str; +use crate::util::lone_surrogate::{encode_utf16, js_str_to_str}; /// Everything declared at the top level of a file that the evaluator can resolve. #[derive(Default)] @@ -3526,7 +3526,7 @@ fn binary<'a>(operator: BinaryOperator, left: &Value<'a>, right: &Value<'a>) -> /// `a < b` (or `a <= b`): strings compare by UTF-16 code units, anything else as numbers. fn compare(a: &Value<'_>, b: &Value<'_>, or_equal: bool) -> bool { let ordering = match (a, b) { - (Value::String(a), Value::String(b)) => Some(a.encode_utf16().cmp(b.encode_utf16())), + (Value::String(a), Value::String(b)) => Some(encode_utf16(a).cmp(encode_utf16(b))), _ => to_number(a).partial_cmp(&to_number(b)), }; ordering.is_some_and(|o| o.is_lt() || (or_equal && o.is_eq())) @@ -3537,7 +3537,8 @@ fn strict_equals(a: &Value<'_>, b: &Value<'_>) -> bool { (Value::Null, Value::Null) | (Value::Undefined, Value::Undefined) => true, (Value::Bool(a), Value::Bool(b)) => a == b, (Value::Number(a), Value::Number(b)) => a == b, - (Value::String(a), Value::String(b)) => a == b, + // By code units: `'\uD800' + '\uDC00'` is `'😀'`. + (Value::String(a), Value::String(b)) => encode_utf16(a).eq(encode_utf16(b)), _ => false, } } diff --git a/crates/oxc_angular_compiler/src/util/lone_surrogate.rs b/crates/oxc_angular_compiler/src/util/lone_surrogate.rs index 68e48e04d..6099a85cb 100644 --- a/crates/oxc_angular_compiler/src/util/lone_surrogate.rs +++ b/crates/oxc_angular_compiler/src/util/lone_surrogate.rs @@ -7,6 +7,10 @@ //! U+FDD0, which Unicode reserves for a program's internal use, then the //! private-use character `U+10F800 + k`. The string printers turn that pair //! back into `\uXXXX`; either character on its own is printed as itself. +//! +//! A source string that really holds U+FDD0 followed by a character from +//! U+10F800 to U+10FFFF reads as a stored lone surrogate too. Both are +//! reserved code points, so no real name is expected to have that pair. use std::borrow::Cow; diff --git a/crates/oxc_angular_compiler/tests/lone_surrogate_names_test.rs b/crates/oxc_angular_compiler/tests/lone_surrogate_names_test.rs index c2e21c35a..a6997ea15 100644 --- a/crates/oxc_angular_compiler/tests/lone_surrogate_names_test.rs +++ b/crates/oxc_angular_compiler/tests/lone_surrogate_names_test.rs @@ -287,3 +287,19 @@ export class C { result.diagnostics ); } + +#[test] +fn evaluated_strings_compare_by_code_unit() { + // `'\uD800' < ''` in JS, and the two halves of a pair joined are + // the pair itself. + let result = compile( + r"import { Directive, Input } from '@angular/core'; +const LESS = '\uD800' < '' ? 'lt' : 'ge'; +const SAME = '\uD83D' + '\uDE00' === '😀' ? 'eq' : 'ne'; +@Directive({ selector: '[o]' }) +export class O { @Input(LESS) a: any; @Input(SAME) b: any; } +", + TransformOptions::default(), + ); + assert_has(&compact(&result.code), &[r#"inputs:{a:[0,"lt","a"],b:[0,"eq","b"]}"#]); +}