From 0001908c9e7e5b1ab3376a141750ba3d729058f9 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 8 Oct 2026 11:05:49 +0800 Subject: [PATCH 1/3] fix(compiler): report missing ngtsc diagnostics for inputs, outputs, queries and signal APIs Implements the ngtsc diagnostics issue #508 lists (vendored Angular 22.0.0 semantics): - NG8110 source-file rule for initializer APIs used outside a class member initializer (new src/validation.rs), incl. upstream quirks: parens/`as` wrapping escapes the check, `!`/`satisfies` trigger it, `.required` shows in the message, and callee identifiers are resolved through oxc_semantic so shadowed locals and params do not false-positive. - NG1053 disallowed member visibility (private / ES private) for signal input/model/output/query initializers; queries keep allowing TS `private`. - NG1010 for signal options: a non-literal `alias` and a non-object options argument. - NG1054 duplicate input/output binding names, with the first binding labeled as related. - NG1006 for combining multiple query decorators or a query decorator with an `@Input`-named decorator. - NG2004 `selector: ''` on @Directive; `selector must be a string` for both components and directives. - `@Output` on setters/getters/methods now emits its `outputs` entry. - Members whose names can't be reflected (computed/numeric keys) are skipped in the io/query passes and emit paths like upstream's reflectClassMember; `#x` members keep their checks. Existing checks were reordered where upstream throws them in a different sequence (signal access/options errors before decorator collisions, duplicate-binding before static for inputs but after for outputs, decorator collisions before the member-kind query check). --- .../src/class_metadata/builders.rs | 26 +- .../src/component/transform.rs | 14 + .../src/directive/decorator.rs | 382 ++++++-- .../oxc_angular_compiler/src/directive/mod.rs | 10 +- .../src/directive/property_decorators.rs | 342 +++++-- crates/oxc_angular_compiler/src/lib.rs | 1 + crates/oxc_angular_compiler/src/validation.rs | 256 ++++++ .../tests/integration_test.rs | 832 +++++++++++++++++- 8 files changed, 1701 insertions(+), 162 deletions(-) create mode 100644 crates/oxc_angular_compiler/src/validation.rs diff --git a/crates/oxc_angular_compiler/src/class_metadata/builders.rs b/crates/oxc_angular_compiler/src/class_metadata/builders.rs index 510e2cf96..c4667f1b7 100644 --- a/crates/oxc_angular_compiler/src/class_metadata/builders.rs +++ b/crates/oxc_angular_compiler/src/class_metadata/builders.rs @@ -8,6 +8,7 @@ use oxc_ast::ast::{ Class, ClassElement, Decorator, Expression, FormalParameter, MethodDefinitionKind, ObjectPropertyKind, PropertyKey, TSType, TSTypeName, }; +use oxc_span::GetSpan; use oxc_str::Ident; use crate::component::{ImportMap, NamespaceRegistry, R3DependencyMetadata}; @@ -20,7 +21,7 @@ use crate::output::ast::{ LiteralMapExpr, LiteralValue, OutputExpression, ReadPropExpr, ReadVarExpr, }; use crate::output::oxc_converter::{ - convert_oxc_expression, convert_plain_properties, is_plain_property, + convert_oxc_expression, convert_plain_properties, is_plain_property, make_raw_source, }; /// Build the decorators metadata array expression. @@ -158,7 +159,14 @@ pub fn build_decorator_metadata_array<'a>( if !args_emitted && let Expression::CallExpression(call) = &decorator.expression { for (arg_idx, arg) in call.arguments.iter().enumerate() { - let expr = arg.to_expression(); + // A `...args` spread isn't an `Expression`; its raw source + // (`...X`) emits the same spread upstream copies verbatim. + let Some(expr) = arg.as_expression() else { + if let Some(raw) = make_raw_source(allocator, source_text, arg.span()) { + args.push(raw); + } + continue; + }; let converted = convert_oxc_expression(allocator, expr, source_text); if let Some(mut converted) = converted { // Same template-literal drop as the transformed path, applied @@ -752,9 +760,13 @@ fn build_signal_query_decorator<'a>( // Predicate: the first positional argument (required), reused as-is. A query with // no locator is invalid (ngc errors); skip synthesis rather than emit a malformed - // decorator. - let predicate = - convert_oxc_expression(allocator, call.arguments.first()?.to_expression(), source_text)?; + // decorator. A `...args` spread isn't an `Expression`; like the compiled + // query's `WrappedNodeExpr` fallback, only the raw source can stand in. + let first = call.arguments.first()?; + let predicate = match first.as_expression() { + Some(expr) => convert_oxc_expression(allocator, expr, source_text)?, + None => make_raw_source(allocator, source_text, first.span())?, + }; let mut args = AllocVec::new_in(&allocator); args.push(predicate); @@ -763,8 +775,8 @@ fn build_signal_query_decorator<'a>( // which preserves any options expression, object literal or not. let mut options = AllocVec::new_in(&allocator); if let Some(second) = call.arguments.get(1) - && let Some(source_options) = - convert_oxc_expression(allocator, second.to_expression(), source_text) + && let Some(expr) = second.as_expression() + && let Some(source_options) = convert_oxc_expression(allocator, expr, source_text) { options.push(LiteralMapEntry::spread(source_options)); } diff --git a/crates/oxc_angular_compiler/src/component/transform.rs b/crates/oxc_angular_compiler/src/component/transform.rs index def0ba48f..5a5de6b81 100644 --- a/crates/oxc_angular_compiler/src/component/transform.rs +++ b/crates/oxc_angular_compiler/src/component/transform.rs @@ -2722,6 +2722,13 @@ fn transform_angular_file_jit( // / `styleUrls`, matching the AOT metadata extraction path. let string_consts = collect_string_consts(allocator, &parser_ret.program); + // ngtsc's source-file rules (validation/): initializer APIs used outside a + // class member initializer (UNSUPPORTED_INITIALIZER_API_USAGE). + result.diagnostics.extend(crate::validation::initializer_api_usage_errors( + &parser_ret.program, + &string_consts, + )); + // Build an import map so `find_angular_decorator` can verify that a bare // `@Service()` is actually Angular's, not a same-named decorator from // another library. @@ -3126,6 +3133,13 @@ pub fn transform_angular_file( )); } + // ngtsc's source-file rules (validation/): initializer APIs used outside a + // class member initializer (UNSUPPORTED_INITIALIZER_API_USAGE). + result.diagnostics.extend(crate::validation::initializer_api_usage_errors( + &parser_ret.program, + &string_consts, + )); + #[cfg(feature = "cross_file_elision")] let mut import_map = build_import_map(allocator, &parser_ret.program.body, options.resolved_imports.as_ref()); diff --git a/crates/oxc_angular_compiler/src/directive/decorator.rs b/crates/oxc_angular_compiler/src/directive/decorator.rs index 2da04b76d..60f805cb2 100644 --- a/crates/oxc_angular_compiler/src/directive/decorator.rs +++ b/crates/oxc_angular_compiler/src/directive/decorator.rs @@ -1091,11 +1091,11 @@ pub(crate) fn angular_decorator_config<'a>( Some((config, name)) } -/// The first error ngtsc raises for the inputs, outputs and queries of a -/// `@Component` / `@Directive` on `class`, in the order it checks them +/// The first error ngtsc raises for the inputs, outputs, queries and selector +/// of a `@Component` / `@Directive` on `class`, in the order it checks them /// (`extractDirectiveMetadata`): `inputs:`, `@Input` members, `outputs:`, -/// output members, query members (`@ViewChild`, ...), then `queries:`. ngtsc -/// stops at the first one. +/// output members, query members (`@ViewChild`, ...), then `queries:`, then +/// `selector`. ngtsc stops at the first one. /// /// Each error points where ngtsc's does: the `inputs:` / `outputs:` value, the /// member, or the part of a query at fault. `source_text` is the file's, as @@ -1106,6 +1106,13 @@ pub fn decorator_io_errors<'a>( source_text: Option<&'a str>, consts: &StringConsts<'a>, ) -> std::vec::Vec { + use super::property_decorators::{ + IO_ACCESS, decorator_argument, initializer_api_access_error, literal_alias_error, + member_access_level, member_name, + }; + + // (message, primary span, span of the first binding a duplicate points at). + let at = |(message, span): (String, Span)| (message, span, None); let Some((config, decorator_name)) = angular_decorator_config(class, consts) else { return std::vec::Vec::new(); }; @@ -1120,153 +1127,292 @@ pub fn decorator_io_errors<'a>( let evaluator = Evaluator::new(consts); let class_name = class.id.as_ref().map_or(String::new(), |id| id.name.to_string()); + // `parseInputFields`' `bindings` map: binding property name to the first + // member bound to it (its name and node), for DUPLICATE_BINDING_NAME. + let mut input_bindings = HashMap::::new(); let input_members = || { class.body.body.iter().find_map(|element| { - let (key, decorators, value, is_static) = match element { - ClassElement::PropertyDefinition(p) => { - (&p.key, &p.decorators, p.value.as_ref(), p.r#static) - } + let (key, decorators, value, is_static, accessibility, readonly) = match element { + ClassElement::PropertyDefinition(p) => ( + &p.key, + &p.decorators, + p.value.as_ref(), + p.r#static, + p.accessibility, + p.readonly, + ), ClassElement::AccessorProperty(p) => { - (&p.key, &p.decorators, p.value.as_ref(), p.r#static) + (&p.key, &p.decorators, p.value.as_ref(), p.r#static, p.accessibility, false) + } + ClassElement::MethodDefinition(m) => { + (&m.key, &m.decorators, None, m.r#static, m.accessibility, false) } - ClassElement::MethodDefinition(m) => (&m.key, &m.decorators, None, m.r#static), _ => return None, }; - let name = key.static_name()?; + // A member `reflectClassMember` can't name (`['k']`, `0:`) never + // reaches `tryParseInputFieldMapping`. + let member_name = member_name(element)?; + // ngtsc parses the signal mappings inside `tryParseInputFieldMapping` + // — input first, then model — before the `@Input` collision and the + // decorator itself, so their own errors (member visibility, then + // the options literal) come first. + let signal_input = + value.and_then(|value| initializer_api_call(value, Some(consts), &[INPUT_API])); + let mut signal_alias = None; + if let Some((api, is_required, call)) = signal_input { + let level = member_access_level(key, accessibility, readonly); + 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)) { + Ok(alias) => signal_alias = alias, + Err(error) => return Some(at(error)), + } + } + let model_input = + value.and_then(|value| initializer_api_call(value, Some(consts), &[MODEL_API])); + let mut model_alias = None; + if let Some((api, is_required, call)) = model_input { + let level = member_access_level(key, accessibility, readonly); + 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)) { + Ok(alias) => model_alias = alias, + Err(error) => return Some(at(error)), + } + } // `@Input(...)`, as ngtsc's `tryParseInputFieldMapping` reads it. let decorator = super::property_decorators::member_decorator(decorators, "Input", consts); // ngtsc rejects `@Input` on a signal input or model before reading // the decorator. - if let (Some(decorator), Some(value)) = (decorator, value) { - let message = if is_initializer_api_call(value, consts, &[INPUT_API]) { + if let (Some(decorator), Some(_)) = (decorator, value) { + let message = if signal_input.is_some() { Some("Using @Input with a signal input is not allowed.") - } else if is_initializer_api_call(value, consts, &[MODEL_API]) { + } else if model_input.is_some() { Some("Using @Input with a model input is not allowed.") } else { None }; if let Some(message) = message { - return Some((message.to_string(), decorator.span)); + return Some((message.to_string(), decorator.span, None)); } } - let error = - decorator.and_then(|d| input_decorator_error(d, &name, class, consts, &evaluator)); - if error.is_some() { - return error; + let error = decorator + .and_then(|d| input_decorator_error(d, &member_name, class, consts, &evaluator)); + if let Some(error) = error { + return Some(at(error)); + } + // With the member's mapping parsed, `parseInputFields` rejects a + // second member bound to the same name — before its static check. + let binding_name = if let Some(decorator) = decorator { + let options = decorator_argument(decorator).map(|arg| evaluator.evaluate(arg)); + Some(match &options { + Some(Value::String(alias)) => alias.clone(), + Some(options) => options + .prop("alias") + .and_then(|prop| prop.value.as_str()) + .map_or_else(|| member_name.clone(), str::to_string), + None => member_name.clone(), + }) + } else if signal_input.is_some() { + Some(signal_alias.map_or_else(|| member_name.clone(), |a| a.to_string())) + } else if model_input.is_some() { + Some(model_alias.map_or_else(|| member_name.clone(), |a| a.to_string())) + } else { + None + }; + if let Some(binding_name) = binding_name { + if let Some((first_name, first_span)) = input_bindings.get(&binding_name) { + let message = format!( + "Input '{binding_name}' is bound to both '{first_name}' and \ + '{member_name}'." + ); + return Some((message, element.span(), Some(*first_span))); + } + input_bindings.insert(binding_name, (member_name.clone(), element.span())); } // ngtsc's `parseInputFields` rejects an input on a static member // once the mapping parsed (INCORRECTLY_DECLARED_ON_STATIC_MEMBER): // an `@Input` decorator or an `input()`/`model()` initializer. if is_static { - let mapped = decorator.is_some() - || value.is_some_and(|value| { - is_initializer_api_call(value, consts, &[INPUT_API, MODEL_API]) - }); + let mapped = decorator.is_some() || signal_input.is_some() || model_input.is_some(); if mapped { let message = format!( - "Input \"{name}\" is incorrectly declared as static member of \ + "Input \"{member_name}\" is incorrectly declared as static member of \ \"{class_name}\"." ); - return Some((message, element.span())); + return Some((message, element.span(), None)); } } // A signal input only collides with a metadata entry of the same name. - let value = value.filter(|_| meta_inputs.contains(&name.as_ref()))?; - let is_input = is_initializer_api_call(value, consts, &[INPUT_API, MODEL_API]); - is_input.then(|| { + if !meta_inputs.contains(&member_name.as_str()) { + return None; + } + (signal_input.is_some() || model_input.is_some()).then(|| { let message = format!( - "Input \"{name}\" is also declared as non-signal in @{decorator_name}." + "Input \"{member_name}\" is also declared as non-signal in @{decorator_name}." ); - (message, element.span()) + (message, element.span(), None) }) }) }; + // `parseOutputFields`' `bindings` map, like `input_bindings` above. + let mut output_bindings = HashMap::::new(); let output_members = || { class.body.body.iter().find_map(|element| { // `@Output(...)`, as ngtsc's `tryParseDecoratorOutput` reads it; it // accepts any member kind, so a decorated (static) method counts. - let (decorators, value, is_static) = match element { - ClassElement::PropertyDefinition(p) => { - (Some(&p.decorators), p.value.as_ref(), p.r#static) + let (key, decorators, value, is_static, accessibility, readonly) = match element { + ClassElement::PropertyDefinition(p) => ( + &p.key, + &p.decorators, + p.value.as_ref(), + p.r#static, + p.accessibility, + p.readonly, + ), + ClassElement::AccessorProperty(p) => ( + &p.key, + &p.decorators, + p.value.as_ref(), + p.r#static, + p.accessibility, + false, + ), + ClassElement::MethodDefinition(m) => { + (&m.key, &m.decorators, None, m.r#static, m.accessibility, false) } - ClassElement::AccessorProperty(p) => { - (Some(&p.decorators), p.value.as_ref(), p.r#static) - } - ClassElement::MethodDefinition(m) => (Some(&m.decorators), None, m.r#static), - _ => (None, None, false), + _ => return None, }; - let decorator = decorators.and_then(|decorators| { - super::property_decorators::member_decorator(decorators, "Output", consts) - }); + // A member `reflectClassMember` can't name (`['k']`, `0:`) never + // reaches `parseOutputFields`. + let member_name = member_name(element)?; + let decorator = super::property_decorators::member_decorator( + decorators, "Output", consts, + ); if let Some(error) = decorator.and_then(|d| output_decorator_error(d, &evaluator)) { - return Some(error); + return Some(at(error)); } - // ngtsc's `tryParseInitializerBasedOutput` rejects `output.required()` - // while parsing the member, before the checks below. Members - // without an initializer (incl. every method) fall through to the - // static-member and @Output-on-signal checks. - if let Some(value) = value - && let Some((_, true, call)) = initializer_api_call( - value, - Some(consts), - &[OUTPUT_API, OUTPUT_FROM_OBSERVABLE_API], - ) - { - return Some(("Output does not support \".required()\".".to_string(), call.span)); + // `tryParseInitializerBasedOutput`: `output.required()` is rejected + // first, then the member's visibility, then its options. Members + // without an initializer (incl. every method) fall through. + let signal_output = value.and_then(|value| { + initializer_api_call(value, Some(consts), &[OUTPUT_API, OUTPUT_FROM_OBSERVABLE_API]) + }); + let mut output_alias = None; + if let Some((api, is_required, call)) = signal_output { + if is_required { + return Some(( + "Output does not support \".required()\".".to_string(), + call.span, + None, + )); + } + let level = member_access_level(key, accessibility, readonly); + if let Some(error) = initializer_api_access_error(api, level, IO_ACCESS, call) { + return Some(at(error)); + } + let options_index = usize::from(api == OUTPUT_FROM_OBSERVABLE_API); + match literal_alias_error(call, options_index) { + Ok(alias) => output_alias = alias, + Err(error) => return Some(at(error)), + } + } + // `parseOutputFields` parses a model here too; the input pass + // reports its errors first, so these are unreachable, mirrored + // anyway. + let model_mapping = value.and_then(|value| { + initializer_api_call(value, Some(consts), &[MODEL_API]) + }); + let mut model_alias = None; + if let Some((api, is_required, call)) = model_mapping { + let level = member_access_level(key, accessibility, readonly); + 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)) { + Ok(alias) => model_alias = alias, + Err(error) => return Some(at(error)), + } } // Then `@Output` on an `output()` or a model, like ngtsc's // `parseOutputFields`. - if let (Some(decorator), Some(value)) = (decorator, value) { - let apis = [OUTPUT_API, OUTPUT_FROM_OBSERVABLE_API]; - let message = if is_initializer_api_call(value, consts, &apis) { + if let (Some(decorator), Some(_)) = (decorator, value) { + let message = if signal_output.is_some() { Some("Using \"@Output\" with \"output()\" is not allowed.") - } else if is_initializer_api_call(value, consts, &[MODEL_API]) { + } else if model_mapping.is_some() { Some("Using @Output with a model input is not allowed.") } else { None }; if let Some(message) = message { - return Some((message.to_string(), decorator.span)); + return Some((message.to_string(), decorator.span, None)); } } // ngtsc's `parseOutputFields` rejects an output on a static member // (INCORRECTLY_DECLARED_ON_STATIC_MEMBER): an `@Output` decorator // or an `output()`/`outputFromObservable()`/`model()` initializer, - // on the decorator or the call. + // on the decorator or the call — before the binding check. if is_static { - let apis = [OUTPUT_API, OUTPUT_FROM_OBSERVABLE_API, MODEL_API]; let node = decorator.map(|d| d.span).or_else(|| { - value - .and_then(|value| initializer_api_call(value, Some(consts), &apis)) - .map(|(_, _, call)| call.span) + signal_output.map(|(_, _, call)| call.span) + }).or_else(|| { + model_mapping.map(|(_, _, call)| call.span) }); if let Some(span) = node { let message = "Output is incorrectly declared on a static class member.".to_string(); - return Some((message, span)); + return Some((message, span, None)); } } - let ClassElement::PropertyDefinition(prop) = element else { return None }; - let (value, name) = (prop.value.as_ref()?, prop.key.static_name()?); - if !meta_outputs.contains(&name.as_ref()) { + // Then a second member bound to the same name + // (DUPLICATE_BINDING_NAME), before the metadata collision check. + let binding_name = if let Some(decorator) = decorator { + let alias = decorator_argument(decorator) + .and_then(|arg| match evaluator.evaluate(arg) { + Value::String(alias) => Some(alias), + _ => None, + }); + Some(alias.unwrap_or_else(|| member_name.clone())) + } else if signal_output.is_some() { + Some(output_alias.map_or_else(|| member_name.clone(), |a| a.to_string())) + } else if model_mapping.is_some() { + let binding = model_alias.map_or_else(|| member_name.clone(), |a| a.to_string()); + Some(format!("{binding}Change")) + } else { + None + }; + if let Some(binding_name) = binding_name { + if let Some((first_name, first_span)) = output_bindings.get(&binding_name) { + let message = format!( + "Output '{binding_name}' is bound to both '{first_name}' and \ + '{member_name}'." + ); + return Some((message, element.span(), Some(*first_span))); + } + output_bindings.insert(binding_name, (member_name.clone(), element.span())); + } + // `outputsFromMeta.hasOwnProperty(member.name)`: any member kind + // whose initializer is an output collides with `outputs:` naming + // it (`#x` members carry the '#' in their name). + if !meta_outputs.contains(&member_name.as_str()) { return None; } - let is_output = is_initializer_api_call( - value, - consts, - &[OUTPUT_API, OUTPUT_FROM_OBSERVABLE_API, MODEL_API], - ); + let is_output = signal_output.is_some() || model_mapping.is_some(); is_output.then(|| { let message = format!( - "Output \"{name}\" is unexpectedly declared in @{decorator_name} as well." + "Output \"{member_name}\" is unexpectedly declared in @{decorator_name} as well." ); - (message, prop.span) + (message, element.span(), None) }) }) }; - let member_queries = - || super::property_decorators::member_query_error(allocator, class, source_text, consts); + let member_queries = || { + super::property_decorators::member_query_error(allocator, class, source_text, consts) + .map(at) + }; // With the source text, like the compiled queries: a predicate is emitted // as written, which some expressions (functions) need it for. let queries = || { @@ -1279,21 +1425,66 @@ pub fn decorator_io_errors<'a>( consts, decorator_name, ); - queries.error + queries.error.map(at) }; + // The selector is the last thing `extractDirectiveMetadata` reads. + let selector = + || directive_selector_error(config, decorator_name, &class_name, &evaluator, consts); io.as_ref() .and_then(|io| io.input_error.clone()) + .map(at) .or_else(input_members) - .or_else(|| io.as_ref().and_then(|io| io.output_error.clone())) + .or_else(|| io.as_ref().and_then(|io| io.output_error.clone()).map(at)) .or_else(output_members) .or_else(member_queries) .or_else(queries) - .map(|(message, span)| OxcDiagnostic::error(message).with_label(span)) + .or_else(selector) + .map(|(message, span, related)| { + let diagnostic = OxcDiagnostic::error(message).with_label(span); + match related { + Some(first) => { + diagnostic.and_label(first.label("The first binding is declared here.")) + } + None => diagnostic, + } + }) .into_iter() .collect() } +/// ngtsc's `selector` check at the end of `extractDirectiveMetadata`, which +/// both `@Component` and `@Directive` run: a `selector` property that isn't +/// a string (`createValueHasWrongTypeError`). An empty one errors only on a +/// directive (DIRECTIVE_MISSING_SELECTOR) — a component's `defaultSelector` +/// (its element name) fills in instead. +fn directive_selector_error<'a>( + config: Option<&'a ObjectExpression<'a>>, + decorator_name: &str, + class_name: &str, + evaluator: &Evaluator<'_, 'a>, + consts: &StringConsts<'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 { + let message = value_error( + &format!("@{decorator_name}.selector"), + || "selector must be a string".to_string(), + &resolved, + ); + return Some((message, expr.span(), None)); + }; + if selector.is_empty() && decorator_name == "Directive" { + return Some(( + format!("Directive {class_name} has no selector, please add it!"), + expr.span(), + None, + )); + } + None +} + /// The name upstream sees for a `@angular/core` decorator (`dec.import?.name /// ?? dec.name`): the imported name for an aliased import (`@Inj(...)` for /// `import { Inject as Inj }` is `Inject`), the member name for a namespace @@ -1395,6 +1586,18 @@ fn input_decorator_error<'a>( if let Some(error) = decorator_arity_error(decorator, &subject) { return Some(error); } + // ngtsc evaluates `decorator.args[0]` as an expression; a `...args` + // spread isn't analyzable, so it hits the wrong-type error. + if has_spread_argument(decorator) { + return Some(( + format!( + "{subject} decorator argument must resolve to a string or an \ + object literal{}", + Value::Dynamic.wrong_type_suffix() + ), + decorator.span, + )); + } let options = super::property_decorators::decorator_argument(decorator)?; let span = options.span(); let options = evaluator.evaluate(options); @@ -1428,6 +1631,15 @@ fn input_decorator_error<'a>( ) } +/// Whether a `@X(...)` decorator call's first argument is a spread — the one +/// `Argument` kind that isn't an `Expression`. +fn has_spread_argument(decorator: &Decorator<'_>) -> bool { + let Expression::CallExpression(call) = &decorator.expression else { + return false; + }; + call.arguments.first().is_some_and(|arg| arg.as_expression().is_none()) +} + /// ngtsc's error for a member decorator called with more than one argument, /// named `subject` (`@Input`, or `@In` for `import { Input as In }`). fn decorator_arity_error(decorator: &Decorator<'_>, subject: &str) -> Option<(String, Span)> { @@ -1450,6 +1662,16 @@ fn output_decorator_error<'a>( if let Some(error) = decorator_arity_error(decorator, "@Output") { return Some(error); } + // Like the `@Input` decorator: a `...args` spread is not analyzable. + if has_spread_argument(decorator) { + return Some(( + format!( + "@Output decorator argument must resolve to a string{}", + Value::Dynamic.wrong_type_suffix() + ), + decorator.span, + )); + } let argument = super::property_decorators::decorator_argument(decorator)?; match evaluator.evaluate(argument) { Value::String(_) => None, diff --git a/crates/oxc_angular_compiler/src/directive/mod.rs b/crates/oxc_angular_compiler/src/directive/mod.rs index 3471422c6..761cae123 100644 --- a/crates/oxc_angular_compiler/src/directive/mod.rs +++ b/crates/oxc_angular_compiler/src/directive/mod.rs @@ -28,8 +28,8 @@ pub use compiler::{ }; pub(crate) use decorator::find_directive_decorator; pub(crate) use decorator::{ - INPUT_API, MODEL_API, OUTPUT_API, OUTPUT_FROM_OBSERVABLE_API, QUERY_APIS, initializer_api, - initializer_api_call, + INPUT_API, InitializerApi, MODEL_API, OUTPUT_API, OUTPUT_FROM_OBSERVABLE_API, QUERY_APIS, + initializer_api, initializer_api_call, }; pub use decorator::{ StringConsts, collect_string_consts, decorator_io_errors, extract_directive_metadata, @@ -50,9 +50,9 @@ pub use metadata::{ }; pub(crate) use property_decorators::{ angular_class_decorator, angular_member_decorator, angular_param_decorator, - extract_host_bindings_in, extract_host_listeners_in, is_angular_core_decorator, - parse_decorator_queries, try_parse_signal_input, try_parse_signal_model, - try_parse_signal_output, unwrap_initializer_api_expr, + decorator_written_name, extract_host_bindings_in, extract_host_listeners_in, + is_angular_core_decorator, parse_decorator_queries, try_parse_signal_input, + try_parse_signal_model, try_parse_signal_output, unwrap_initializer_api_expr, }; pub use property_decorators::{ extract_class_queries, extract_content_queries, extract_host_bindings, extract_host_listeners, diff --git a/crates/oxc_angular_compiler/src/directive/property_decorators.rs b/crates/oxc_angular_compiler/src/directive/property_decorators.rs index 4f36fd6ef..b8e7d2782 100644 --- a/crates/oxc_angular_compiler/src/directive/property_decorators.rs +++ b/crates/oxc_angular_compiler/src/directive/property_decorators.rs @@ -13,8 +13,8 @@ use oxc_allocator::{Allocator, Vec}; use oxc_ast::ast::{ - Argument, ArrayExpressionElement, Class, ClassElement, Decorator, Expression, - MethodDefinitionKind, ObjectPropertyKind, PropertyKey, + Argument, ArrayExpressionElement, CallExpression, Class, ClassElement, Decorator, Expression, + MethodDefinitionKind, ObjectPropertyKind, PropertyKey, TSAccessibility, }; use oxc_span::{GetSpan, Span}; use oxc_str::Ident; @@ -235,6 +235,162 @@ pub(crate) fn decorator_written_name<'a>(decorator: &'a Decorator<'a>) -> &'a st } } +/// ngtsc's `ClassMemberAccessLevel`: how visible a member is to code outside +/// the class, which the initializer APIs restrict their use to. +#[derive(Clone, Copy, PartialEq, Eq)] +pub enum AccessLevel { + PublicWritable, + PublicReadonly, + Protected, + Private, + EsPrivate, +} + +impl AccessLevel { + /// ngtsc's `classMemberAccessLevelToString`. + fn name(self) -> &'static str { + match self { + Self::PublicWritable => "public", + Self::PublicReadonly => "public readonly", + Self::Protected => "protected", + Self::Private => "private", + Self::EsPrivate => "ES private", + } + } +} + +/// ngtsc's `ClassMemberAccessLevel` for a class member: an ES private name +/// (`#x`) is private whatever its modifiers; a modifier wins over `readonly` +/// (`private readonly` is `private`); everything else is public. +pub(crate) fn member_access_level( + key: &PropertyKey<'_>, + accessibility: Option, + readonly: bool, +) -> AccessLevel { + if key.is_private_identifier() { + AccessLevel::EsPrivate + } else { + match accessibility { + Some(TSAccessibility::Private) => AccessLevel::Private, + Some(TSAccessibility::Protected) => AccessLevel::Protected, + _ if readonly => AccessLevel::PublicReadonly, + _ => AccessLevel::PublicWritable, + } + } +} + +/// ngtsc's `ClassMember.name`: the member name `reflectClassMember` reports — +/// an identifier or string literal, and a `#x` private name *including* the +/// '#'. `None` when the member has no reflectable name (a computed `['k']`, +/// a numeric `0:` key, ...), which drops it from `getMembersOfClass` +/// entirely: it can raise none of the member diagnostics. +pub(crate) fn member_name(element: &ClassElement<'_>) -> Option { + let (key, computed) = match element { + ClassElement::PropertyDefinition(p) => (&p.key, p.computed), + ClassElement::AccessorProperty(p) => (&p.key, p.computed), + ClassElement::MethodDefinition(m) => (&m.key, m.computed), + _ => return None, + }; + if computed { + return None; + } + match key { + PropertyKey::StaticIdentifier(id) => Some(id.name.to_string()), + PropertyKey::StringLiteral(lit) => Some(lit.value.to_string()), + PropertyKey::PrivateIdentifier(id) => Some(format!("#{}", id.name)), + _ => None, + } +} + +/// [`member_name`] as an `Ident`, for the extraction passes that emit the +/// name: a member it can't name — a computed `['k']`, a numeric `0:` — isn't +/// a member upstream, so no input/output/query is extracted from it. +fn member_name_ident<'a>( + allocator: &'a Allocator, + element: &ClassElement<'a>, +) -> Option> { + member_name(element).map(|name| Ident::from(allocator.alloc_str(&name))) +} + +/// The `allowedAccessLevels` of ngtsc's `input` / `model` / `output` / +/// `outputFromObservable` initializer functions. +pub const IO_ACCESS: &[AccessLevel] = + &[AccessLevel::PublicWritable, AccessLevel::PublicReadonly, AccessLevel::Protected]; + +/// The `allowedAccessLevels` of ngtsc's signal query initializer functions, +/// which also allow a TS `private` member (unlike an ES private `#x`). +pub const QUERY_ACCESS: &[AccessLevel] = &[ + AccessLevel::PublicWritable, + AccessLevel::PublicReadonly, + AccessLevel::Protected, + AccessLevel::Private, +]; + +/// ngtsc's `validateAccessOfInitializerApiMember` +/// (INITIALIZER_API_DISALLOWED_MEMBER_VISIBILITY): the diagnostic it throws +/// when `api`'s call can't sit on a member of `level`, on the call. +pub(crate) fn initializer_api_access_error( + api: super::decorator::InitializerApi, + level: AccessLevel, + allowed: &[AccessLevel], + call: &CallExpression<'_>, +) -> Option<(String, Span)> { + if allowed.contains(&level) { + return None; + } + let allowed_names = allowed.iter().map(|l| l.name()).collect::>().join(", "); + Some(( + format!( + "Cannot use \"{}\" on a class member that is declared as {}.\n\ + Update the class field to be either: {allowed_names}", + api.0, + level.name(), + ), + call.span, + )) +} + +/// The `alias` of an initializer API's options argument, read like ngtsc's +/// `parseAndValidateInputAndOutputOptions`: the argument at `options_index` +/// must be an object literal (an identifier doesn't count, however +/// analyzable), and its `alias`, if given, must be a string or +/// 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>( + call: &'a CallExpression<'a>, + options_index: usize, +) -> Result>, (String, Span)> { + let Some(argument) = call.arguments.get(options_index) else { + return Ok(None); + }; + let Argument::ObjectExpression(options) = argument else { + return Err(( + "Argument needs to be an object literal that is statically analyzable.".to_string(), + argument.span(), + )); + }; + // `reflectObjectLiteral`: a repeated key's last property wins. + for prop in options.properties.iter().rev() { + let ObjectPropertyKind::ObjectProperty(prop) = prop else { continue }; + // `propertyNameToString` skips a computed key, so `['alias']` is not + // an `alias` upstream. + if !is_metadata_property(prop) + || prop.computed + || !get_property_key_name(&prop.key).is_some_and(|key| key == "alias") + { + continue; + } + return match extract_string_value(&prop.value) { + Some(alias) => Ok(Some(alias)), + None => Err(( + "Alias needs to be a string that is statically analyzable.".to_string(), + prop.value.span(), + )), + }; + } + Ok(None) +} + /// Get the property key name as an Atom. /// /// Handles both identifier keys and string literal keys. @@ -501,8 +657,11 @@ fn options_alias<'a>(options: Option<&Argument<'a>>) -> Option> { let Some(Argument::ObjectExpression(obj)) = options else { return None }; let mut alias = None; for prop in &obj.properties { + // `propertyNameToString` skips a computed key, so `['alias']` is not + // an `alias` upstream. if let ObjectPropertyKind::ObjectProperty(prop) = prop && is_metadata_property(prop) + && !prop.computed && get_property_key_name(&prop.key).is_some_and(|key| key == "alias") { alias = extract_string_value(&prop.value); @@ -645,7 +804,7 @@ pub fn extract_input_metadata_in<'a>( ClassElement::PropertyDefinition(prop) => { // First check for @Input decorator if let Some(decorator) = find_decorator_by_name(&prop.decorators, "Input", consts) { - let Some(class_property_name) = get_property_key_name(&prop.key) else { + let Some(class_property_name) = member_name_ident(allocator, element) else { continue; }; @@ -664,7 +823,7 @@ pub fn extract_input_metadata_in<'a>( } // Then check for signal-based input (input(), input.required(), model(), model.required()) else if let Some(value) = &prop.value { - if let Some(property_name) = get_property_key_name(&prop.key) { + if let Some(property_name) = member_name_ident(allocator, element) { // Check for model() first since it also creates an input if let Some(model_mapping) = try_parse_signal_model(allocator, value, property_name.clone(), consts) @@ -687,7 +846,7 @@ pub fn extract_input_metadata_in<'a>( continue; }; - let Some(class_property_name) = get_property_key_name(&prop.key) else { + let Some(class_property_name) = member_name_ident(allocator, element) else { continue; }; @@ -712,7 +871,7 @@ pub fn extract_input_metadata_in<'a>( continue; }; - let Some(class_property_name) = get_property_key_name(&method.key) else { + let Some(class_property_name) = member_name_ident(allocator, element) else { continue; }; @@ -826,7 +985,7 @@ pub fn extract_output_metadata_in<'a>( // First check for @Output decorator if let Some(decorator) = find_decorator_by_name(&prop.decorators, "Output", consts) { - let Some(class_property_name) = get_property_key_name(&prop.key) else { + let Some(class_property_name) = member_name_ident(allocator, element) else { continue; }; @@ -839,7 +998,7 @@ pub fn extract_output_metadata_in<'a>( } // Then check for signal-based outputs (output(), model()) else if let Some(value) = &prop.value { - if let Some(property_name) = get_property_key_name(&prop.key) { + 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) @@ -862,7 +1021,27 @@ pub fn extract_output_metadata_in<'a>( continue; }; - let Some(class_property_name) = get_property_key_name(&prop.key) else { + let Some(class_property_name) = member_name_ident(allocator, element) else { + continue; + }; + + let config = parse_output_config(allocator, decorator, evaluator.as_ref()); + + let binding_property_name = + config.alias.unwrap_or_else(|| class_property_name.clone()); + + outputs.push((class_property_name, binding_property_name)); + } + + // `tryParseDecoratorOutput` accepts any member kind, so a + // decorated setter, getter or method is an output too. + ClassElement::MethodDefinition(method) => { + let Some(decorator) = find_decorator_by_name(&method.decorators, "Output", consts) + else { + continue; + }; + + let Some(class_property_name) = member_name_ident(allocator, element) else { continue; }; @@ -1109,16 +1288,23 @@ fn try_parse_signal_query<'a>( } // Type predicate: viewChild(TemplateRef) or viewChild(forwardRef(() => MyClass)) _ => { - let expr = predicate_arg.to_expression(); - // Unwrap forwardRef for evaluation; `is_forward_ref` records it so - // the partial emitter can re-wrap the predicate. - let unwrapped = try_unwrap_forward_ref(expr, consts); - is_forward_ref = unwrapped.is_some(); - let unwrapped_expr = unwrapped.unwrap_or(expr); - // ngtsc emits a non-string locator as written (`WrappedNodeExpr`), - // so fall back to the source text for what can't be converted. - let output_expr = convert_oxc_expression(allocator, unwrapped_expr, source_text) - .or_else(|| make_raw_source(allocator, source_text, unwrapped_expr.span()))?; + // ngtsc emits a non-string locator as written (`WrappedNodeExpr`); + // a spread (`viewChild(...args)`) isn't an `Expression`, so only + // the raw source can stand in for it. + let output_expr = match predicate_arg.as_expression() { + Some(expr) => { + // Unwrap forwardRef for evaluation; `is_forward_ref` + // records it so the partial emitter can re-wrap the + // predicate. + let unwrapped = try_unwrap_forward_ref(expr, consts); + is_forward_ref = unwrapped.is_some(); + let unwrapped_expr = unwrapped.unwrap_or(expr); + convert_oxc_expression(allocator, unwrapped_expr, source_text).or_else( + || make_raw_source(allocator, source_text, unwrapped_expr.span()), + )? + } + None => make_raw_source(allocator, source_text, predicate_arg.span())?, + }; QueryPredicate::Type(output_expr) } }; @@ -1923,20 +2109,21 @@ pub(crate) fn parse_decorator_queries<'a>( // A signal query member for the same property is an error: one calling // Angular's `viewChild()`, `contentChildren()`, ... (not just any function - // named like them). - let signal_queries: std::vec::Vec> = class + // named like them). Any member kind with an initializer counts, and a + // `#x` member's name includes the '#'. + let signal_queries: std::vec::Vec = class .body .body .iter() .filter_map(|element| { - let ClassElement::PropertyDefinition(prop) = element else { return None }; - let name = get_property_key_name(&prop.key)?; - super::decorator::is_initializer_api_call( - prop.value.as_ref()?, - consts, - &super::decorator::QUERY_APIS, - ) - .then_some(name) + let value = match element { + ClassElement::PropertyDefinition(prop) => prop.value.as_ref(), + ClassElement::AccessorProperty(accessor) => accessor.value.as_ref(), + _ => return None, + }; + let name = member_name(element)?; + super::decorator::is_initializer_api_call(value?, consts, &super::decorator::QUERY_APIS) + .then_some(name) }) .collect(); // ngtsc checks the content queries first, and reports the `new` expression. @@ -1945,7 +2132,7 @@ pub(crate) fn parse_decorator_queries<'a>( .iter() .zip(content_exprs) .chain(queries.view.iter().zip(view_exprs)) - .find(|(q, _)| signal_queries.contains(&q.property_name)); + .find(|(q, _)| signal_queries.iter().any(|name| name == q.property_name.as_str())); if let Some((_, span)) = collision { queries.error = Some(( format!( @@ -1993,29 +2180,67 @@ pub(crate) fn member_query_error<'a>( class.body.body.iter().find_map(|element| { // Upstream's `isPropertyTypeMember`: only getters, setters and // properties (incl. auto-accessors) may hold a query decorator. - let (decorators, value, is_static, span, is_property_type) = match element { - ClassElement::PropertyDefinition(prop) => { - (&prop.decorators, prop.value.as_ref(), prop.r#static, prop.span, true) - } - ClassElement::AccessorProperty(accessor) => ( - &accessor.decorators, - accessor.value.as_ref(), - accessor.r#static, - accessor.span, - true, - ), - ClassElement::MethodDefinition(method) => ( - &method.decorators, - None, - method.r#static, - method.span, - matches!(method.kind, MethodDefinitionKind::Get | MethodDefinitionKind::Set), - ), - _ => return None, - }; - let decorator = QUERY_TYPES.iter().find_map(|name| { - find_decorator_by_name(decorators, name, Some(consts)).zip(Some(*name)) - }); + let (key, decorators, value, is_static, accessibility, readonly, span, is_property_type) = + match element { + ClassElement::PropertyDefinition(prop) => ( + &prop.key, + &prop.decorators, + prop.value.as_ref(), + prop.r#static, + prop.accessibility, + prop.readonly, + prop.span, + true, + ), + ClassElement::AccessorProperty(accessor) => ( + &accessor.key, + &accessor.decorators, + accessor.value.as_ref(), + accessor.r#static, + accessor.accessibility, + false, + accessor.span, + true, + ), + ClassElement::MethodDefinition(method) => ( + &method.key, + &method.decorators, + None, + method.r#static, + method.accessibility, + false, + method.span, + matches!(method.kind, MethodDefinitionKind::Get | MethodDefinitionKind::Set), + ), + _ => return None, + }; + // A member `reflectClassMember` can't name (`['k']`, `0:`) is not in + // `getMembersOfClass` upstream, so it raises no query diagnostics. + member_name(element)?; + // ngtsc's `getAngularDecorators` collects every query decorator, in + // member-decorator order. + let query_decorators: std::vec::Vec<(&Decorator<'_>, &'static str)> = decorators + .iter() + .filter_map(|d| { + angular_core_decorator(d, Some(consts), QUERY_TYPES).map(|name| (d, name)) + }) + .collect(); + // `tryGetQueryFromFieldDecorator` reports these collisions first, + // before the member-kind and query metadata checks. + if query_decorators.len() > 1 { + return Some(("Cannot combine multiple query decorators.".to_string(), span)); + } + // `@Input` as written (`decorators.some(v => v.name === 'Input')`): + // an aliased `@In` doesn't count, `@core.Input` does. + if !query_decorators.is_empty() + && decorators.iter().any(|d| decorator_written_name(d) == "Input") + { + return Some(( + "Cannot combine @Input decorators with query decorators".to_string(), + span, + )); + } + let decorator = query_decorators.first().copied(); // Its member-kind check (DECORATOR_UNEXPECTED) runs before the // decorator metadata is parsed upstream (`isPropertyTypeMember` // precedes `extractDecoratorQueryMetadata`). @@ -2037,8 +2262,15 @@ pub(crate) fn member_query_error<'a>( &super::decorator::QUERY_APIS, ) }); - if let Some((_, _, call)) = signal { - if call.arguments.first().and_then(Argument::as_expression).is_none() { + if let Some((api, _, call)) = signal { + // The member-visibility check runs before the locator/options are read. + let level = member_access_level(key, accessibility, readonly); + if let Some(error) = initializer_api_access_error(api, level, QUERY_ACCESS, call) { + return Some(error); + } + // `predicateNode === undefined`: only a call with NO first + // argument errors; a `...args` locator parses like any other. + if call.arguments.is_empty() { return Some(("No locator specified.".to_string(), call.span)); } if let Some(options) = call.arguments.get(1) diff --git a/crates/oxc_angular_compiler/src/lib.rs b/crates/oxc_angular_compiler/src/lib.rs index 00d756263..50a3029ce 100644 --- a/crates/oxc_angular_compiler/src/lib.rs +++ b/crates/oxc_angular_compiler/src/lib.rs @@ -25,6 +25,7 @@ #![warn(missing_docs)] mod util; +mod validation; pub mod ast; pub mod class_debug_info; diff --git a/crates/oxc_angular_compiler/src/validation.rs b/crates/oxc_angular_compiler/src/validation.rs new file mode 100644 index 000000000..e55fe1ddb --- /dev/null +++ b/crates/oxc_angular_compiler/src/validation.rs @@ -0,0 +1,256 @@ +//! ngtsc's source-file validation rules (`ngtsc/validation/`). +//! +//! Unlike the per-class checks in [`crate::directive`], these run on every +//! call expression in the file, wherever it appears. + +use oxc_ast::ast::{ + CallExpression, ClassType, Expression, IdentifierReference, ImportDeclarationSpecifier, + Program, Statement, +}; +use oxc_ast::ast_kind::AstKind; +use oxc_ast_visit::Visit; +use oxc_diagnostics::OxcDiagnostic; +use oxc_semantic::{AstNode, Semantic, SemanticBuilder}; + +use crate::directive::{ + INPUT_API, InitializerApi, MODEL_API, OUTPUT_API, OUTPUT_FROM_OBSERVABLE_API, QUERY_APIS, + StringConsts, decorator_written_name, initializer_api, is_angular_core_decorator, +}; + +/// The APIs ngtsc's `InitializerApiUsageRule` checks (`APIS_TO_CHECK`). +const APIS: [InitializerApi; 8] = [ + INPUT_API, + MODEL_API, + OUTPUT_API, + OUTPUT_FROM_OBSERVABLE_API, + QUERY_APIS[0], + QUERY_APIS[1], + QUERY_APIS[2], + QUERY_APIS[3], +]; + +/// ngtsc's `shouldCheck`: the file imports one of the initializer functions +/// by name (under any alias) from its module, or the module as a namespace. +fn should_check(program: &Program<'_>) -> bool { + program.body.iter().any(|stmt| { + let Statement::ImportDeclaration(import) = stmt else { return false }; + let module = import.source.value.as_str(); + if module != "@angular/core" && module != "@angular/core/rxjs-interop" { + return false; + } + import.specifiers.as_ref().is_some_and(|specifiers| { + specifiers.iter().any(|specifier| match specifier { + ImportDeclarationSpecifier::ImportSpecifier(specifier) => { + APIS.iter().any(|&(name, owning_module)| { + specifier.imported.name().as_str() == name && module == owning_module + }) + } + ImportDeclarationSpecifier::ImportNamespaceSpecifier(_) => true, + ImportDeclarationSpecifier::ImportDefaultSpecifier(_) => false, + }) + }) + }) +} + +/// The module that imports `id`'s declaration — the `ImportDeclaration` +/// containing it, like upstream's `getContainingImportDeclaration`. +fn importing_module<'a>(declaration: &AstNode<'a>, semantic: &Semantic<'a>) -> Option<&'a str> { + let AstKind::ImportDeclaration(import) = semantic.nodes().parent_kind(declaration.id()) else { + return None; + }; + Some(import.source.value.as_str()) +} + +/// The declaration `id`'s reference resolves to when it's the symbol's only +/// declaration: ngtsc's `symbol.declarations.length === 1` — +/// `getSymbolAtLocation` on a shadowed identifier lands on the parameter or +/// local, not the import. +fn sole_declaration<'s, 'a>( + id: &IdentifierReference<'a>, + semantic: &'s Semantic<'a>, +) -> Option<&'s AstNode<'a>> { + let reference_id = id.reference_id.get()?; + let symbol_id = semantic.scoping().get_reference(reference_id).symbol_id()?; + (semantic.scoping().symbol_declarations(symbol_id).count() == 1) + .then(|| semantic.symbol_declaration(symbol_id)) +} + +/// ngtsc's `getDirectImportOfIdentifier`: the `(module, exported name)` `id` +/// imports, when its symbol's only declaration is an import specifier — +/// `import {f}` gives the imported name, `import f` and `import * as f` +/// fall back to the local one (`getExportedName`). Anything else resolves +/// to `None`. +fn direct_import_of<'a>( + id: &IdentifierReference<'a>, + semantic: &Semantic<'a>, +) -> Option<(&'a str, &'a str)> { + let declaration = sole_declaration(id, semantic)?; + let name = match declaration.kind() { + AstKind::ImportSpecifier(specifier) => specifier.imported.name().as_str(), + AstKind::ImportDefaultSpecifier(_) | AstKind::ImportNamespaceSpecifier(_) => { + id.name.as_str() + } + _ => return None, + }; + Some((importing_module(declaration, semantic)?, name)) +} + +/// ngtsc's `getImportOfNamespacedIdentifier`: the module `id`'s symbol is a +/// namespace import of — `ns` in `ns.f`, where the caller checks `f`. +fn namespace_import_of<'a>( + id: &IdentifierReference<'a>, + semantic: &Semantic<'a>, +) -> Option<&'a str> { + let declaration = sole_declaration(id, semantic)?; + if !matches!(declaration.kind(), AstKind::ImportNamespaceSpecifier(_)) { + return None; + } + importing_module(declaration, semantic) +} + +/// ngtsc's last `tryParseInitializerApi` step, `reflector.getImportOfIdentifier` +/// on the API's identifier — "using the type checker ... accounts for things +/// like shadowed variables": `f` (or `f` in `f.required`) only when `id` +/// resolves to an import of `api`'s name from its module; `ns.f` (or `ns.f` +/// in `ns.f.required`) only when `ns` resolves to a namespace import of the +/// module. Any other binding — a parameter, a local, an unrelated import — +/// means the call isn't one of the API's. +fn is_api_reference(callee: &Expression<'_>, api: InitializerApi, semantic: &Semantic<'_>) -> bool { + // `f` — its binding must be the import. + let named = |expr: &Expression<'_>| { + let Expression::Identifier(id) = expr else { return false }; + direct_import_of(id, semantic) + .is_some_and(|(module, name)| module == api.1 && name == api.0) + }; + // `ns.f` — `ns` must be a namespace import of the module. + let namespaced = |expr: &Expression<'_>| { + let Expression::StaticMemberExpression(member) = expr else { return false }; + let Expression::Identifier(ns) = &member.object else { return false }; + member.property.name == api.0 && namespace_import_of(ns, semantic) == Some(api.1) + }; + named(callee) + || namespaced(callee) + || match callee { + // `f.required` / `ns.f.required` — the API reference is the object. + Expression::StaticMemberExpression(member) if member.property.name == "required" => { + named(&member.object) || namespaced(&member.object) + } + _ => false, + } +} + +/// `InitializerApiUsageRule::checkNode` over every call expression. +struct InitializerApiUsage<'c, 'a> { + consts: &'c StringConsts<'a>, + /// The semantic model, for resolving callee identifiers to their binding + /// like `getSymbolAtLocation` does upstream. + semantic: &'c Semantic<'a>, + /// The `AstKind`s enclosing the node being entered, innermost last. + ancestors: std::vec::Vec>, + diagnostics: std::vec::Vec, +} + +impl<'a> InitializerApiUsage<'_, 'a> { + fn check_call(&mut self, call: &'a CallExpression<'a>) { + // The direct parent of the call. If it wraps the call in parentheses + // or an `as` cast, upstream's `node` stops being a call expression + // and the rule reports nothing — `x = (input(0))` and + // `x = input(0) as any` never error. `!`/`satisfies` don't count. + let Some(parent) = self.ancestors.last() else { return }; + if matches!(parent, AstKind::ParenthesizedExpression(_) | AstKind::TSAsExpression(_)) { + return; + } + let Some((api, is_required)) = initializer_api(&call.callee, Some(self.consts), &APIS) + else { + return; + }; + // The file-level import map is only the cheap pre-filter upstream's + // `ImportedSymbolsTracker` plays; a shadowed name still passes it. + if !is_api_reference(&call.callee, api, self.semantic) { + return; + } + let function_name = format!("{}{}", api.0, if is_required { ".required" } else { "" }); + + // The initializer of a class property (or auto-accessor) is the one + // supported position, and only on a `@Component` / `@Directive` class + // declaration. `closestClass` climbs past class expressions upstream, + // to the nearest declaration. + let prop = match parent { + AstKind::PropertyDefinition(prop) => prop.value.as_ref(), + AstKind::AccessorProperty(accessor) => accessor.value.as_ref(), + _ => None, + }; + if let Some(prop) = prop + && matches!(prop, Expression::CallExpression(c) if std::ptr::eq(&raw const **c, call)) + { + let class = self.ancestors.iter().rev().find_map(|kind| match kind { + AstKind::Class(class) if class.r#type == ClassType::ClassDeclaration => { + Some(*class) + } + _ => None, + }); + if let Some(class) = class { + let is_component_or_directive = class.decorators.iter().any(|d| { + matches!(decorator_written_name(d), "Component" | "Directive") + && is_angular_core_decorator(d, Some(self.consts)) + }); + if !is_component_or_directive { + self.diagnostics.push( + OxcDiagnostic::error(format!( + "Unsupported call to the {function_name} function. \ + This function can only be used as the initializer \ + of a property on a @Component or @Directive class." + )) + .with_label(call.span), + ); + } + return; + } + } + self.diagnostics.push( + OxcDiagnostic::error(format!( + "Unsupported call to the {function_name} function. \ + This function can only be called in the initializer of a class member." + )) + .with_label(call.span), + ); + } +} + +impl<'a> Visit<'a> for InitializerApiUsage<'_, 'a> { + fn enter_node(&mut self, kind: AstKind<'a>) { + if let AstKind::CallExpression(call) = kind { + self.check_call(call); + } + self.ancestors.push(kind); + } + + fn leave_node(&mut self, _kind: AstKind<'a>) { + self.ancestors.pop(); + } +} + +/// ngtsc's `InitializerApiUsageRule` (UNSUPPORTED_INITIALIZER_API_USAGE): a +/// diagnostic for every call to an initializer API outside the initializer of +/// a property on a `@Component` / `@Directive` class. +pub fn initializer_api_usage_errors<'a>( + program: &'a Program<'a>, + consts: &StringConsts<'a>, +) -> std::vec::Vec { + if !should_check(program) { + return std::vec::Vec::new(); + } + // Built here so `IdentifierReference.reference_id`s are populated for the + // visitor's binding resolution — upstream's `checker.getSymbolAtLocation`. + // `with_build_nodes` keeps the node store for the declaration kinds and + // their `ImportDeclaration` parents; resolution doesn't need it. + let semantic_ret = SemanticBuilder::new().with_build_nodes(true).build(program); + let mut rule = InitializerApiUsage { + consts, + semantic: &semantic_ret.semantic, + ancestors: std::vec::Vec::new(), + diagnostics: std::vec::Vec::new(), + }; + rule.visit_program(program); + rule.diagnostics +} diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index a7e8b3532..0b0fe3817 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -12480,15 +12480,16 @@ export class AppComponent {} ); } -/// An interpolated `${...}` whose identifier is NOT a known const must NOT -/// crash and must NOT produce a partial/garbage selector — the field is -/// dropped (same fallback as today for any unresolvable identifier). +/// An interpolated `${...}` whose identifier is NOT a known const is a +/// non-string selector upstream (`createValueHasWrongTypeError`, shared.ts: +/// `selector must be a string` for components too): the diagnostic is +/// reported while the emitted `ɵcmp` still falls back to `ng-component`. /// -/// Scope: this test asserts ONLY on the `ɵcmp` selectors field. Since #299 -/// turned `emit_class_metadata` on by default, the raw `${UNRESOLVED}-tag` -/// template literal is intentionally preserved verbatim inside -/// `ɵsetClassMetadata(..., [{ type: Component, args: [...] }], ...)` to -/// mirror ngc's behavior — that's metadata for runtime tooling and is not +/// Scope: this test asserts ONLY on the diagnostic and the `ɵcmp` selectors +/// field. Since #299 turned `emit_class_metadata` on by default, the raw +/// `${UNRESOLVED}-tag` template literal is intentionally preserved verbatim +/// inside `ɵsetClassMetadata(..., [{ type: Component, args: [...] }], ...)` +/// to mirror ngc's behavior — that's metadata for runtime tooling and is not /// the compiled selector itself. #[test] fn component_template_literal_unresolved_identifier_drops_field() { @@ -12504,8 +12505,13 @@ import { Component } from '@angular/core'; export class UnresolvedComponent {} "#; let result = transform_angular_file(&allocator, "u.component.ts", source, None, None); - // Must not crash. - assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + // The unresolved interpolation is reported — but the pipeline still emits. + assert!( + result.diagnostics.iter().any(|d| format!("{d}") + == "selector must be a string Value could not be determined statically."), + "non-string @Component selector should error. Got: {:?}", + result.diagnostics + ); // The unresolved interpolation must not appear inside the `ɵcmp`'s // `selectors:` slot — that's the compiled selector that actually drives @@ -12526,6 +12532,35 @@ export class UnresolvedComponent {} ); } +/// The same `selector must be a string` check for a resolved non-string: +/// `selector: 42` on @Component errors, and the `ɵcmp` still emits the +/// `ng-component` fallback. +#[test] +fn component_non_string_selector_reports_error_and_falls_back() { + let allocator = Allocator::default(); + let source = r#" +import { Component } from '@angular/core'; + +@Component({ selector: 42, template: '' }) +export class C {} +"#; + let result = transform_angular_file(&allocator, "c.component.ts", source, None, None); + assert!( + result.diagnostics.iter().any(|d| format!("{d}").starts_with("selector must be a string")), + "@Component selector: 42 should error. Got: {:?}", + result.diagnostics + ); + + let cmp_start = result.code.find("ɵɵdefineComponent({").expect("ɵcmp missing"); + let cmp_section = &result.code[cmp_start..]; + let cmp_end = cmp_section.find("})").expect("ɵcmp not terminated"); + let cmp_def = &cmp_section[..cmp_end]; + assert!( + cmp_def.contains(r#"selectors:[["ng-component"]]"#), + "Selector should fall back to `ng-component`.\nɵcmp:\n{cmp_def}" + ); +} + // ============================================================================= // Issue #514: `selector: ''` on @Component falls back to `ng-component` // ============================================================================= @@ -12569,10 +12604,8 @@ export class C {} #[test] fn directive_empty_selector_emits_no_invalid_selectors() { // Upstream, `selector: ''` on @Directive resolves to the `null` default - // selector and raises NG2004. Until directive diagnostics land, the best - // OXC can do is not emit an invalid `selectors` array like `[[""]]` — - // and it must NOT fall back to `ng-component` (that fallback is - // component-only). + // selector and raises NG2004 (DIRECTIVE_MISSING_SELECTOR). It must NOT + // fall back to `ng-component` (that fallback is component-only). let allocator = Allocator::default(); let source = r#" import { Directive } from '@angular/core'; @@ -12581,7 +12614,14 @@ import { Directive } from '@angular/core'; export class D {} "#; let result = transform_angular_file(&allocator, "d.directive.ts", source, None, None); - assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics); + assert!( + result + .diagnostics + .iter() + .any(|d| format!("{d}") == "Directive D has no selector, please add it!"), + "Empty @Directive selector should report NG2004. Got: {:?}", + result.diagnostics + ); let dir_start = result.code.find("ɵɵdefineDirective({").expect("ɵdir missing"); let dir_section = &result.code[dir_start..]; @@ -16636,3 +16676,765 @@ export class C { result.diagnostics ); } + +// ============================================================================ +// Issue #508: diagnostics ngtsc reports that were silently dropped +// ============================================================================ +// DUPLICATE_BINDING_NAME (NG1054), DECORATOR_COLLISION on query members +// (NG1006), INITIALIZER_API_DISALLOWED_MEMBER_VISIBILITY (NG1053), the +// signal-options literal checks (NG1010), DIRECTIVE_MISSING_SELECTOR +// (NG2004), @Output on any member kind, and UNSUPPORTED_INITIALIZER_API_USAGE +// (NG8110). + +fn expect_result(source: &str) -> oxc_angular_compiler::TransformResult { + let allocator = Allocator::default(); + transform_angular_file(&allocator, "test.ts", source, None, None) +} + +#[test] +fn test_duplicate_input_binding_name_is_diagnostic() { + // `parseInputFields` maps bindingPropertyName -> member; a second member + // for the same name is an error, named for both members. + let diagnostics = expect_diagnostics( + "import { Directive, Input } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n @Input('y') x: any;\n @Input() y: any;\n}", + ); + assert!( + diagnostics.iter().any(|d| d == "Input 'y' is bound to both 'x' and 'y'."), + "expected the duplicate-binding error. Got: {diagnostics:?}" + ); +} + +#[test] +fn test_duplicate_input_binding_name_labels_first_binding() { + // ngtsc attaches `The first binding is declared here.` to the first + // member's node as related information — a second label on the diagnostic. + let result = expect_result( + "import { Directive, Input } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n @Input('y') x: any;\n @Input() y: any;\n}", + ); + let diagnostic = result + .diagnostics + .iter() + .find(|d| format!("{d}") == "Input 'y' is bound to both 'x' and 'y'.") + .expect("duplicate-binding diagnostic missing"); + assert_eq!( + diagnostic.labels.len(), + 2, + "diagnostic should carry the primary and related labels: {diagnostic:?}" + ); + assert_eq!(diagnostic.labels[1].label(), Some("The first binding is declared here.")); +} + +#[test] +fn test_duplicate_signal_and_model_binding_names() { + // A signal input's literal alias joins the same binding map. + let diagnostics = expect_diagnostics( + "import { Directive, Input, input } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n x = input(0, { alias: 'y' });\n @Input() y: any;\n}", + ); + assert!( + diagnostics.iter().any(|d| d == "Input 'y' is bound to both 'x' and 'y'."), + "signal input alias should collide. Got: {diagnostics:?}" + ); + + // `a = model(0)` binds the output `aChange`, colliding with + // `@Output() aChange`. + let diagnostics = expect_diagnostics( + "import { Directive, Output, model } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n a = model(0);\n @Output() aChange = null;\n}", + ); + assert!( + diagnostics.iter().any(|d| d == "Output 'aChange' is bound to both 'a' and 'aChange'."), + "model output binding should collide. Got: {diagnostics:?}" + ); +} + +#[test] +fn test_duplicate_output_binding_name_is_diagnostic() { + let diagnostics = expect_diagnostics( + "import { Directive, Output } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n @Output('y') x = null;\n @Output() y = null;\n}", + ); + assert!( + diagnostics.iter().any(|d| d == "Output 'y' is bound to both 'x' and 'y'."), + "expected the duplicate-binding error. Got: {diagnostics:?}" + ); +} + +#[test] +fn test_duplicate_binding_check_order_against_static() { + // `parseInputFields` checks the duplicate binding BEFORE the static + // check: a second member binding `y` errors first even when the member + // itself is also static. + let diagnostics = expect_diagnostics( + "import { Directive, Input } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n @Input() y: any;\n @Input('y') static x: any;\n}", + ); + assert!( + diagnostics.iter().any(|d| d == "Input 'y' is bound to both 'y' and 'x'."), + "inputs report the duplicate before the static error. Got: {diagnostics:?}" + ); + + // `parseOutputFields` checks static FIRST: a static member errors before + // a later duplicate is reached. + let diagnostics = expect_diagnostics( + "import { Directive, Output } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n @Output('y') static x = null;\n @Output() y = null;\n}", + ); + assert!( + diagnostics + .iter() + .any(|d| d.contains("Output is incorrectly declared on a static class member.")), + "outputs report the static error before a later duplicate. Got: {diagnostics:?}" + ); +} + +#[test] +fn test_multiple_query_decorators_on_member_is_diagnostic() { + // `tryGetQueryFromFieldDecorator` collects all query decorators; more + // than one is a DECORATOR_COLLISION (trailing period). + for member in [ + "@ViewChild('a') @ViewChildren('b') x: any;", + "@ViewChild('a') @ViewChild('b') x: any;", + "@ContentChild('a') @ContentChildren('b') x: any;", + ] { + let source = format!( + "import {{ Directive, ViewChild, ViewChildren, ContentChild, ContentChildren }} \ + from '@angular/core';\n\ + @Directive({{ selector: '[d]' }})\n\ + export class D {{\n {member}\n}}" + ); + let diagnostics = expect_diagnostics(&source); + assert!( + diagnostics.iter().any(|d| d == "Cannot combine multiple query decorators."), + "`{member}` should report the multi-decorator collision. Got: {diagnostics:?}" + ); + } +} + +#[test] +fn test_input_and_query_decorator_collision_is_diagnostic() { + // `@Input` combined with a query decorator is a DECORATOR_COLLISION, and + // the message has NO trailing period. + let diagnostics = expect_diagnostics( + "import { Directive, Input, ViewChild } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n @Input() @ViewChild('a') x: any;\n}", + ); + assert!( + diagnostics.iter().any(|d| d == "Cannot combine @Input decorators with query decorators"), + "expected the @Input/query collision (no period). Got: {diagnostics:?}" + ); + + // The written name decides, not the resolved import: an aliased @In is + // not 'Input' upstream, so the member parses as both input and query. + let allocator = Allocator::default(); + let result = transform_angular_file( + &allocator, + "test.ts", + "import { Directive, Input as In, ViewChild } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n @In() @ViewChild('a') x: any;\n}", + None, + None, + ); + assert!( + !result.diagnostics.iter().any(|d| format!("{d}").contains("Cannot combine")), + "aliased @In must not trigger the collision. Got: {:?}", + result.diagnostics + ); + + // `@core.Input` IS written 'Input' (the member-access tail). + let diagnostics = expect_diagnostics( + "import { Directive } from '@angular/core';\n\ + import * as core from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n @core.Input() @core.ViewChild('a') x: any;\n}", + ); + assert!( + diagnostics.iter().any(|d| d == "Cannot combine @Input decorators with query decorators"), + "@core.Input should trigger the collision. Got: {diagnostics:?}" + ); +} + +#[test] +fn test_initializer_api_disallowed_member_visibility() { + // INITIALIZER_API_DISALLOWED_MEMBER_VISIBILITY (NG1053): inputs, models + // and outputs disallow private / ES private members. + for (member, expected) in [ + ( + "private x = input(0);", + "Cannot use \"input\" on a class member that is declared as private.\nUpdate the class field to be either: public, public readonly, protected", + ), + ( + "private x = input.required(0);", + "Cannot use \"input\" on a class member that is declared as private.\nUpdate the class field to be either: public, public readonly, protected", + ), + ( + "#x = input(0);", + "Cannot use \"input\" on a class member that is declared as ES private.\nUpdate the class field to be either: public, public readonly, protected", + ), + ( + "private x = output();", + "Cannot use \"output\" on a class member that is declared as private.\nUpdate the class field to be either: public, public readonly, protected", + ), + // The input pass parses model() first and reports 'model'. + ( + "private x = model(0);", + "Cannot use \"model\" on a class member that is declared as private.\nUpdate the class field to be either: public, public readonly, protected", + ), + ] { + let source = format!( + "import {{ Directive, input, model, output }} from '@angular/core';\n\ + @Directive({{ selector: '[d]' }})\n\ + export class D {{\n {member}\n}}" + ); + let diagnostics = expect_diagnostics(&source); + assert!( + diagnostics.iter().any(|d| d == expected), + "`{member}` should report {expected:?}. Got: {diagnostics:?}" + ); + } + + // Query functions additionally allow a TS `private` member — but not an + // ES private `#x`. + let diagnostics = expect_diagnostics( + "import { Directive, viewChild } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n #x = viewChild('a');\n}", + ); + assert!( + diagnostics.iter().any(|d| d + == "Cannot use \"viewChild\" on a class member that is declared as ES private.\nUpdate the class field to be either: public, public readonly, protected, private"), + "#x = viewChild should report ES private. Got: {diagnostics:?}" + ); + // `private` is allowed for queries, so parsing reaches the locator check. + let diagnostics = expect_diagnostics( + "import { Directive, viewChild } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n private x = viewChild();\n}", + ); + assert!( + diagnostics.iter().any(|d| d == "No locator specified."), + "private x = viewChild() should reach the locator check. Got: {diagnostics:?}" + ); + + // `protected` and `readonly` are allowed for inputs: no diagnostic. + let diagnostics = expect_diagnostics( + "import { Directive, input } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n protected a = input(0);\n readonly b = input(0);\n}", + ); + assert!( + diagnostics.is_empty(), + "protected/readonly inputs should compile. Got: {diagnostics:?}" + ); +} + +#[test] +fn test_initializer_api_options_must_be_literals() { + // NG1010: the options argument must be an object literal — a resolved + // const or import is not accepted — and `alias` must be a string literal. + for (member, import, expected) in [ + ( + "x = input(0, OPTS);", + "input", + "Argument needs to be an object literal that is statically analyzable.", + ), + ( + "x = input(0, { alias: ALIAS });", + "input", + "Alias needs to be a string that is statically analyzable.", + ), + ( + "x = output(OBJ);", + "output", + "Argument needs to be an object literal that is statically analyzable.", + ), + ( + "x = outputFromObservable(s, OBJ);", + "output", + "Argument needs to be an object literal that is statically analyzable.", + ), + ] { + let source = format!( + "import {{ Directive, {import} }} from '@angular/core';\n\ + import {{ outputFromObservable }} from '@angular/core/rxjs-interop';\n\ + const OPTS = {{}};\nconst ALIAS = 'a';\nconst OBJ = {{}};\nconst s = null;\n\ + @Directive({{ selector: '[d]' }})\n\ + export class D {{\n {member}\n}}" + ); + let diagnostics = expect_diagnostics(&source); + assert!( + diagnostics.iter().any(|d| d == expected), + "`{member}` should report {expected:?}. Got: {diagnostics:?}" + ); + } + + // A literal alias still binds — two members aliased to the same name + // collide, and input.required takes its options at index 0. + let diagnostics = expect_diagnostics( + "import { Directive, Input, input } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n x = input.required({ alias: 'y' });\n @Input() y: any;\n}", + ); + assert!( + diagnostics.iter().any(|d| d == "Input 'y' is bound to both 'x' and 'y'."), + "literal alias on input.required should bind. Got: {diagnostics:?}" + ); +} + +#[test] +fn test_directive_selector_diagnostics() { + // `selector: 42` on @Directive is not a string — `selector must be a + // string` with the wrong-type suffix. + let diagnostics = expect_diagnostics( + "import { Directive } from '@angular/core';\n\ + @Directive({ selector: 42 })\n\ + export class D {}", + ); + assert!( + diagnostics.iter().any(|d| d.starts_with("selector must be a string")), + "@Directive selector: 42 should error. Got: {diagnostics:?}" + ); + + // `extractDirectiveMetadata` runs the same check for @Component: an + // unresolved or non-string selector is an error there too (the emitted + // `ɵcmp` still falls back to `ng-component`). + for extra in ["selector: 42", "selector: `${UNRESOLVED}-tag`"] { + let source = format!( + "import {{ Component }} from '@angular/core';\n\ + @Component({{ {extra}, template: '' }})\n\ + export class C {{}}" + ); + let diagnostics = expect_diagnostics(&source); + assert!( + diagnostics.iter().any(|d| d.starts_with("selector must be a string")), + "@Component {extra} should report this diagnostic. Got: {diagnostics:?}" + ); + } + + // No `selector` key is fine on both. + let diagnostics = expect_diagnostics( + "import { Directive } from '@angular/core';\n\ + @Directive({})\n\ + export class D {}", + ); + assert!(diagnostics.is_empty(), "missing selector is not an error: {diagnostics:?}"); + + // `selector: ''` is NG2004 on a directive but the default on a component. + let diagnostics = expect_diagnostics( + "import { Directive } from '@angular/core';\n\ + @Directive({ selector: '' })\n\ + export class D {}", + ); + assert!( + diagnostics.iter().any(|d| d == "Directive D has no selector, please add it!"), + "empty directive selector should report NG2004. Got: {diagnostics:?}" + ); + let diagnostics = expect_diagnostics( + "import { Component } from '@angular/core';\n\ + @Component({ selector: '', template: '' })\n\ + export class C {}", + ); + assert!( + diagnostics.is_empty(), + "empty component selector falls back to the default. Got: {diagnostics:?}" + ); + + // An io error wins over the selector check, which is the last thing + // `extractDirectiveMetadata` reads. + let diagnostics = expect_diagnostics( + "import { Directive, Input } from '@angular/core';\n\ + @Directive({ selector: '' })\n\ + export class D {\n @Input() static x = 0;\n}", + ); + assert!( + diagnostics.iter().any(|d| d.contains("is incorrectly declared as static")), + "io error should precede the selector check. Got: {diagnostics:?}" + ); +} + +#[test] +fn test_output_decorator_on_setter_emits_output() { + // `tryParseDecoratorOutput` accepts any member kind upstream: an @Output + // setter is an output, in the definition and the .d.ts. + let result = expect_result( + "import { Directive, Output } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n @Output() set o(v: any) {}\n}", + ); + assert!(!result.has_errors(), "@Output setter errored: {:?}", result.diagnostics); + let code: String = result.code.chars().filter(|c| !c.is_whitespace()).collect(); + assert!( + code.contains("outputs:{o:\"o\"}"), + "@Output setter should emit outputs:{{o:\"o\"}}. Got:\n{}", + result.code + ); + let decl = + result.dts_declarations.iter().find(|d| d.class_name == "D").expect("d.ts missing for D"); + assert!( + decl.members.contains("\"o\": \"o\""), + "d.ts should include the output map. Members:\n{}", + decl.members + ); +} + +#[test] +fn test_computed_output_member_emits_no_output() { + // `reflectClassMember` returns null for a computed name upstream — an + // `@Output` on `['o']` binds nothing. + for member in [ + "@Output() set ['o'](v: any) {}", + "@Output() get ['o']() { return null; }", + "@Output() ['o']() {}", + "@Output() ['o'] = new EventEmitter();", + "['o'] = output();", + ] { + let result = expect_result(&format!( + "import {{ Directive, Output, output }} from '@angular/core';\n\ + @Directive({{ selector: '[d]' }})\n\ + export class D {{\n {member}\n}}", + )); + let code: String = result.code.chars().filter(|c| !c.is_whitespace()).collect(); + assert!( + !code.contains("outputs:{o:"), + "computed member `{member}` must emit no `o` output. Got:\n{}", + result.code + ); + } + + // A non-computed string key still emits, under its quoted name. + let result = expect_result( + "import { Directive, Output } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n @Output() set 'o'(v: any) {}\n}", + ); + let code: String = result.code.chars().filter(|c| !c.is_whitespace()).collect(); + assert!( + code.contains("outputs:{o:\"o\"}"), + "'o' setter should emit outputs:{{o:\"o\"}}. Got:\n{}", + result.code + ); +} + +#[test] +fn test_computed_input_member_emits_no_input() { + // The same `reflectClassMember` null for input members. + for member in ["@Input() set ['i'](v: any) {}", "@Input() ['i'] = 0;", "['i'] = input(0);"] { + let result = expect_result(&format!( + "import {{ Directive, Input, input }} from '@angular/core';\n\ + @Directive({{ selector: '[d]' }})\n\ + export class D {{\n {member}\n}}", + )); + let code: String = result.code.chars().filter(|c| !c.is_whitespace()).collect(); + assert!( + !code.contains("inputs:{i:"), + "computed member `{member}` must emit no `i` input. Got:\n{}", + result.code + ); + } +} + +#[test] +fn test_initializer_api_usage_outside_class_member() { + // UNSUPPORTED_INITIALIZER_API_USAGE (NG8110): a call to an initializer + // API anywhere but a property initializer of an Angular class. + for (snippet, expected) in [ + ( + "export class C {\n x = input(0)!;\n}", + "Unsupported call to the input function. This function can only be called in the initializer of a class member.", + ), + ( + "export class C {\n constructor() { const x = input(0); }\n}", + "Unsupported call to the input function. This function can only be called in the initializer of a class member.", + ), + ( + "export class C {\n x = input(0) satisfies any;\n}", + "Unsupported call to the input function. This function can only be called in the initializer of a class member.", + ), + ( + "function f() { return input(0); }", + "Unsupported call to the input function. This function can only be called in the initializer of a class member.", + ), + ( + "const c = class { x = input(0); };", + "Unsupported call to the input function. This function can only be called in the initializer of a class member.", + ), + ( + "@Directive({ selector: '[d]' })\nclass C {}\nexport class D {\n x = input(0);\n}", + "Unsupported call to the input function. This function can only be used as the initializer of a property on a @Component or @Directive class.", + ), + ( + "@Injectable()\nexport class D {\n x = input(0);\n}", + "Unsupported call to the input function. This function can only be used as the initializer of a property on a @Component or @Directive class.", + ), + // The written name decides: `@Cmp` is not `Component`/`Directive`. + ( + "@Cmp({ selector: 'c', template: '' })\nexport class C {\n x = input(0);\n}", + "Unsupported call to the input function. This function can only be used as the initializer of a property on a @Component or @Directive class.", + ), + ( + "export class C {\n x = input.required(0);\n}", + "Unsupported call to the input.required function. This function can only be used as the initializer of a property on a @Component or @Directive class.", + ), + ] { + let source = format!( + "import {{ Directive, Injectable, Component as Cmp, input }} from '@angular/core';\n\ + {snippet}" + ); + let diagnostics = expect_diagnostics(&source); + assert!( + diagnostics.iter().any(|d| d == expected), + "snippet {snippet:?} should report {expected:?}. Got: {diagnostics:?}" + ); + } + + // Wrapped calls never produce the diagnostic: upstream's `node` is no + // longer the call once parens/`as` unwrap it. + for member in ["x = (input(0));", "x = input(0) as any;"] { + let source = format!( + "import {{ Directive, input }} from '@angular/core';\n\ + @Directive({{ selector: '[d]' }})\n\ + export class D {{\n {member}\n}}" + ); + let diagnostics = expect_diagnostics(&source); + assert!( + !diagnostics.iter().any(|d| d.contains("Unsupported call")), + "`{member}` must not report NG8110. Got: {diagnostics:?}" + ); + } + + // Without the Angular imports the rule doesn't run at all. + let diagnostics = expect_diagnostics( + "declare function input(v: number): number;\n\ + export class C {\n x = input(0);\n}", + ); + assert!( + !diagnostics.iter().any(|d| d.contains("Unsupported call")), + "no Angular import means no NG8110. Got: {diagnostics:?}" + ); +} + +#[test] +fn test_initializer_api_usage_shadowed_identifiers() { + // `getSymbolAtLocation` upstream: a name lexically bound to a parameter + // or local isn't the import, however the file-level import map reads it. + for snippet in [ + "function f(input: any) {\n input();\n}", + "function f(input: any) {\n input.required(0);\n}", + "function f() {\n const input = () => 0;\n input();\n}", + "const input = () => 0;\nfunction f() {\n input();\n}", + "class C {\n constructor(private input: any) {\n input();\n }\n}", + "function f(input: any) {\n @Directive({ selector: '[d]' })\n class D {\n x = input();\n }\n}", + ] { + let source = format!("import {{ Directive, input }} from '@angular/core';\n{snippet}"); + let diagnostics = expect_diagnostics(&source); + assert!( + !diagnostics.iter().any(|d| d.contains("Unsupported call")), + "shadowed `input` in {snippet:?} must not report NG8110. Got: {diagnostics:?}" + ); + } + + // A shadowed namespace import isn't one either. + for snippet in [ + "function g(ngc: any) {\n ngc.input(0);\n}", + "function g() {\n const ngc = { input: (v: any) => v };\n ngc.input(0);\n}", + "function g(ngc: any) {\n ngc.input.required(0);\n}", + ] { + let source = format!("import * as ngc from '@angular/core';\n{snippet}"); + let diagnostics = expect_diagnostics(&source); + assert!( + !diagnostics.iter().any(|d| d.contains("Unsupported call")), + "shadowed `ngc` in {snippet:?} must not report NG8110. Got: {diagnostics:?}" + ); + } + + // ...while the real imports still error, next to the shadowed uses. + let diagnostics = expect_diagnostics( + "import { Directive, input } from '@angular/core';\n\ + function f(input: any) {\n input();\n}\n\ + const x = input(0);", + ); + assert!( + diagnostics.iter().any(|d| d + == "Unsupported call to the input function. This function can only be called in the initializer of a class member."), + "top-level misuse must still report NG8110. Got: {diagnostics:?}" + ); + let diagnostics = expect_diagnostics( + "import * as ngc from '@angular/core';\n\ + function g(ngc: any) {\n ngc.input(0);\n}\n\ + const x = ngc.input(0);", + ); + assert!( + diagnostics.iter().any(|d| d + == "Unsupported call to the input function. This function can only be called in the initializer of a class member."), + "top-level namespaced misuse must still report NG8110. Got: {diagnostics:?}" + ); +} + +#[test] +fn test_computed_alias_key_in_initializer_options_is_ignored() { + // `propertyNameToString` skips a ComputedPropertyName upstream: `{['alias']: ...}` + // is NOT an alias — the member keeps its own binding name and the value + // never hits the "statically analyzable" alias check. + let result = expect_result( + "import { Directive, input } from '@angular/core';\n\ + const ALIAS = 'a';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n x = input(0, { ['alias']: ALIAS });\n}", + ); + assert!( + !result.diagnostics.iter().any(|d| format!("{d}") + .contains("Alias needs to be a string that is statically analyzable.")), + "computed ['alias'] must not hit the alias check. Got: {:?}", + result.diagnostics + ); + let code: String = result.code.chars().filter(|c| !c.is_whitespace()).collect(); + assert!( + code.contains("inputs:{x:[1,\"x\"]}"), + "computed ['alias'] must not bind the input to another name. Got:\n{}", + result.code + ); + + // Same for a literal value — it binds `x`, not `y`. + let result = expect_result( + "import { Directive, output } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n x = output({ ['alias']: 'y' });\n}", + ); + assert!( + !result.diagnostics.iter().any(|d| format!("{d}").contains("Alias")), + "computed ['alias'] on output() must not hit the alias check. Got: {:?}", + result.diagnostics + ); + let code: String = result.code.chars().filter(|c| !c.is_whitespace()).collect(); + assert!( + code.contains("outputs:{x:\"x\"}"), + "computed ['alias'] must not bind the output to 'y'. Got:\n{}", + result.code + ); +} + +#[test] +fn test_spread_locator_and_options_argument_diagnostics() { + // A spread locator is `arguments[0]` upstream, not `undefined`: no + // 'No locator specified.' — the predicate is emitted as written. + let diagnostics = expect_diagnostics( + "import { Directive, viewChild } from '@angular/core';\n\ + const ARGS: any[] = [];\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n x = viewChild(...ARGS);\n}", + ); + assert!( + !diagnostics.iter().any(|d| d == "No locator specified."), + "viewChild(...ARGS) must not report a missing locator. Got: {diagnostics:?}" + ); + + // The no-argument call still reports it. + let diagnostics = expect_diagnostics( + "import { Directive, viewChild } from '@angular/core';\n\ + @Directive({ selector: '[d]' })\n\ + export class D {\n x = viewChild();\n}", + ); + assert!( + diagnostics.iter().any(|d| d == "No locator specified."), + "viewChild() should still report the missing locator. Got: {diagnostics:?}" + ); +} + +#[test] +fn test_spread_decorator_argument_is_wrong_type() { + // `decorator.args[0]` evaluated upstream is a DynamicValue for a spread: + // the wrong-type error, not a silent no-argument decorator. + for (decorator, expected) in [ + ( + "Input", + "@Input decorator argument must resolve to a string or an object literal Value could not be determined statically.", + ), + ( + "Output", + "@Output decorator argument must resolve to a string Value could not be determined statically.", + ), + ] { + let source = format!( + "import {{ Directive, {decorator} }} from '@angular/core';\n\ + const X: any = null;\n\ + @Directive({{ selector: '[d]' }})\n\ + export class D {{\n @{decorator}(...X) x: any;\n}}" + ); + let diagnostics = expect_diagnostics(&source); + assert!( + diagnostics.iter().any(|d| d == expected), + "`@{decorator}(...X)` should report {expected:?}. Got: {diagnostics:?}" + ); + } +} + +#[test] +fn test_outputs_meta_collision_on_all_member_kinds() { + // `outputsFromMeta.hasOwnProperty(member.name)` runs for every reflected + // member kind — an auto-accessor with an output() initializer collides + // with `outputs:` too. (A `#x` member never reaches it: the ES-private + // visibility check fires first.) + let diagnostics = expect_diagnostics( + "import { Directive, output } from '@angular/core';\n\ + @Directive({ selector: '[d]', outputs: ['x'] })\n\ + export class D {\n accessor x = output();\n}", + ); + assert!( + diagnostics + .iter() + .any(|d| d == "Output \"x\" is unexpectedly declared in @Directive as well."), + "`accessor x = output()` + `outputs: ['x']` should collide. Got: {diagnostics:?}" + ); +} + +#[test] +fn test_queries_meta_collision_on_accessor_members() { + // `signalQueryFields` covers every member kind upstream — an auto-accessor + // holding a signal query collides with `queries:` naming it too. (A `#x` + // member never reaches it: the ES-private visibility check fires first.) + let diagnostics = expect_diagnostics( + "import { Directive, ViewChild, viewChild } from '@angular/core';\n\ + @Directive({ selector: '[d]', queries: {x: new ViewChild('a')} })\n\ + export class D {\n accessor x = viewChild('a');\n}", + ); + assert!( + diagnostics.iter().any(|d| d.contains("Query is declared multiple times.")), + "`accessor x = viewChild()` + `queries` naming x should collide. Got: {diagnostics:?}" + ); +} + +#[test] +fn test_unnameable_members_are_silent() { + // `reflectClassMember` drops members whose name can't be reflected + // (computed `['k']`, numeric `0:` keys): no decorator, static, visibility + // or metadata-collision diagnostics for them. + for member in [ + "@Input('y') ['k'] = 0;", + "@Input(5) ['k'] = 0;", + "@Output() ['k'] = null;", + "static ['k'] = output();", + "['k'] = viewChild('a');", + ] { + let source = format!( + "import {{ Directive, Input, Output, output, viewChild }} from '@angular/core';\n\ + @Directive({{ selector: '[d]', outputs: ['k'] }})\n\ + export class D {{\n {member}\n}}" + ); + let diagnostics = expect_diagnostics(&source); + assert!( + diagnostics.is_empty(), + "`{member}` should raise no diagnostics. Got: {diagnostics:?}" + ); + } +} From b7b1204514af8c6c254f078edb3d6b2dd270a2b2 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 8 Oct 2026 11:26:38 +0800 Subject: [PATCH 2/3] fix(compiler): resolve NG8110 class decorators through the semantic model The class-decorator check still used the file-wide import map, so a shadowed `Directive` identifier (e.g. a function parameter) counted as Angular's and suppressed the diagnostic. Resolve the decorator callee to its binding like `is_api_reference` does for the call, matching upstream's `decorator.import?.from`. --- crates/oxc_angular_compiler/src/validation.rs | 24 ++++++++++++++++--- .../tests/integration_test.rs | 22 +++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/crates/oxc_angular_compiler/src/validation.rs b/crates/oxc_angular_compiler/src/validation.rs index e55fe1ddb..ae4e25b5c 100644 --- a/crates/oxc_angular_compiler/src/validation.rs +++ b/crates/oxc_angular_compiler/src/validation.rs @@ -14,7 +14,7 @@ use oxc_semantic::{AstNode, Semantic, SemanticBuilder}; use crate::directive::{ INPUT_API, InitializerApi, MODEL_API, OUTPUT_API, OUTPUT_FROM_OBSERVABLE_API, QUERY_APIS, - StringConsts, decorator_written_name, initializer_api, is_angular_core_decorator, + StringConsts, decorator_written_name, initializer_api, }; /// The APIs ngtsc's `InitializerApiUsageRule` checks (`APIS_TO_CHECK`). @@ -190,9 +190,27 @@ impl<'a> InitializerApiUsage<'_, 'a> { _ => None, }); if let Some(class) = class { + // Upstream checks `decorator.import?.from === '@angular/core'`, + // resolved through the type checker — a shadowed `Directive` + // param doesn't count. Resolve through the semantic model like + // `is_api_reference` does for the call. let is_component_or_directive = class.decorators.iter().any(|d| { - matches!(decorator_written_name(d), "Component" | "Directive") - && is_angular_core_decorator(d, Some(self.consts)) + if !matches!(decorator_written_name(d), "Component" | "Directive") { + return false; + } + let callee = match &d.expression { + Expression::CallExpression(call) => &call.callee, + expr => expr, + }; + match callee { + Expression::Identifier(id) => direct_import_of(id, self.semantic) + .is_some_and(|(module, _)| module == crate::r3::CORE), + Expression::StaticMemberExpression(member) => { + matches!(&member.object, Expression::Identifier(ns) if + namespace_import_of(ns, self.semantic) == Some(crate::r3::CORE)) + } + _ => false, + } }); if !is_component_or_directive { self.diagnostics.push( diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index 0b0fe3817..df948a47f 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -17258,6 +17258,28 @@ fn test_initializer_api_usage_shadowed_identifiers() { ); } + // A class decorator bound to something else (here a parameter) isn't + // `@angular/core`'s, so `input()` in the class's member initializer is an + // error like on any undecorated class — upstream resolves the decorator's + // identifier through the type checker, `decorator.import?.from`. + for snippet in [ + "function f(Directive: any) {\n @Directive({ selector: '[d]' })\n class D {\n x = input(0);\n }\n}", + "function f(ngc: any) {\n @ngc.Directive({ selector: '[d]' })\n class D {\n x = input(0);\n }\n}", + ] { + let source = format!( + "import {{ Directive, input }} from '@angular/core';\n\ + import * as ngc from '@angular/core';\n\ + {snippet}" + ); + let diagnostics = expect_diagnostics(&source); + assert!( + diagnostics.iter().any(|d| d.contains( + "can only be used as the initializer of a property on a @Component or @Directive class." + )), + "shadowed decorator in {snippet:?} must report NG8110. Got: {diagnostics:?}" + ); + } + // ...while the real imports still error, next to the shadowed uses. let diagnostics = expect_diagnostics( "import { Directive, input } from '@angular/core';\n\ From f933d8df3cef0d6f24f50dae7b0281aa1469aa84 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Thu, 8 Oct 2026 11:35:56 +0800 Subject: [PATCH 3/3] fix(compiler): skip unreflectable members in query extraction too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `extract_view_queries_in`/`extract_content_queries_in` still named members with `get_property_key_name`, which accepts a string-literal key on a computed member — `['q'] = viewChild('x')` emitted a query while the new member guards skip validation for it. Use `member_name_ident` (the reflectClassMember-equivalent) in every extraction arm so computed and numeric members produce no query. --- .../src/directive/property_decorators.rs | 24 +++++++------- .../tests/integration_test.rs | 33 +++++++++++++++++++ 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/crates/oxc_angular_compiler/src/directive/property_decorators.rs b/crates/oxc_angular_compiler/src/directive/property_decorators.rs index b8e7d2782..97955ac9e 100644 --- a/crates/oxc_angular_compiler/src/directive/property_decorators.rs +++ b/crates/oxc_angular_compiler/src/directive/property_decorators.rs @@ -1413,10 +1413,10 @@ pub(crate) fn extract_view_queries_in<'a>( } _ => None, }; - if let Some((decorators, key, value)) = prop_like { + if let Some((decorators, _key, value)) = prop_like { // Check for signal-based view queries first (viewChild(), viewChildren()) if let Some(value) = value { - if let Some(property_name) = get_property_key_name(key) { + if let Some(property_name) = member_name_ident(allocator, element) { if let Some((query_type, metadata)) = try_parse_signal_query(allocator, value, property_name, source_text, consts) { @@ -1430,7 +1430,7 @@ pub(crate) fn extract_view_queries_in<'a>( // Check for decorator-based queries (@ViewChild, @ViewChildren) if let Some(decorator) = find_decorator_by_name(decorators, "ViewChild", consts) { - if let Some(property_name) = get_property_key_name(key) { + if let Some(property_name) = member_name_ident(allocator, element) { let config = parse_query_config(allocator, decorator, "ViewChild", source_text, consts); if let Some(predicate) = config.predicate { @@ -1450,7 +1450,7 @@ pub(crate) fn extract_view_queries_in<'a>( } else if let Some(decorator) = find_decorator_by_name(decorators, "ViewChildren", consts) { - if let Some(property_name) = get_property_key_name(key) { + if let Some(property_name) = member_name_ident(allocator, element) { let config = parse_query_config( allocator, decorator, @@ -1482,7 +1482,7 @@ pub(crate) fn extract_view_queries_in<'a>( if let Some(decorator) = find_decorator_by_name(&method.decorators, "ViewChild", consts) { - if let Some(property_name) = get_property_key_name(&method.key) { + if let Some(property_name) = member_name_ident(allocator, element) { let config = parse_query_config( allocator, decorator, @@ -1507,7 +1507,7 @@ pub(crate) fn extract_view_queries_in<'a>( } else if let Some(decorator) = find_decorator_by_name(&method.decorators, "ViewChildren", consts) { - if let Some(property_name) = get_property_key_name(&method.key) { + if let Some(property_name) = member_name_ident(allocator, element) { let config = parse_query_config( allocator, decorator, @@ -1600,10 +1600,10 @@ pub(crate) fn extract_content_queries_in<'a>( } _ => None, }; - if let Some((decorators, key, value)) = prop_like { + if let Some((decorators, _key, value)) = prop_like { // Check for signal-based content queries first (contentChild(), contentChildren()) if let Some(value) = value { - if let Some(property_name) = get_property_key_name(key) { + if let Some(property_name) = member_name_ident(allocator, element) { if let Some((query_type, metadata)) = try_parse_signal_query(allocator, value, property_name, source_text, consts) { @@ -1617,7 +1617,7 @@ pub(crate) fn extract_content_queries_in<'a>( // Check for decorator-based queries (@ContentChild, @ContentChildren) if let Some(decorator) = find_decorator_by_name(decorators, "ContentChild", consts) { - if let Some(property_name) = get_property_key_name(key) { + if let Some(property_name) = member_name_ident(allocator, element) { let config = parse_query_config( allocator, decorator, @@ -1642,7 +1642,7 @@ pub(crate) fn extract_content_queries_in<'a>( } else if let Some(decorator) = find_decorator_by_name(decorators, "ContentChildren", consts) { - if let Some(property_name) = get_property_key_name(key) { + if let Some(property_name) = member_name_ident(allocator, element) { let config = parse_query_config( &allocator, decorator, @@ -1674,7 +1674,7 @@ pub(crate) fn extract_content_queries_in<'a>( if let Some(decorator) = find_decorator_by_name(&method.decorators, "ContentChild", consts) { - if let Some(property_name) = get_property_key_name(&method.key) { + if let Some(property_name) = member_name_ident(allocator, element) { let config = parse_query_config( allocator, decorator, @@ -1699,7 +1699,7 @@ pub(crate) fn extract_content_queries_in<'a>( } else if let Some(decorator) = find_decorator_by_name(&method.decorators, "ContentChildren", consts) { - if let Some(property_name) = get_property_key_name(&method.key) { + if let Some(property_name) = member_name_ident(allocator, element) { let config = parse_query_config( &allocator, decorator, diff --git a/crates/oxc_angular_compiler/tests/integration_test.rs b/crates/oxc_angular_compiler/tests/integration_test.rs index df948a47f..7738daf01 100644 --- a/crates/oxc_angular_compiler/tests/integration_test.rs +++ b/crates/oxc_angular_compiler/tests/integration_test.rs @@ -17460,3 +17460,36 @@ fn test_unnameable_members_are_silent() { ); } } + +#[test] +fn test_unnameable_members_emit_nothing() { + // The same reflectClassMember drop applies to the emit paths: a + // computed-key member can't produce an input, output or query — not just + // no diagnostics. + for (member, meta) in [ + ("@ViewChild('x') ['q'] = null;", "viewQueries"), + ("@ContentChild('x') ['q'] = null;", "queries"), + ("static ['q'] = viewChild('x');", "viewQueries"), + ("['q'] = contentChild('x');", "queries"), + ("@Output() set ['o'](v: any) {}", "outputs"), + ("@Output() ['o']() {}", "outputs"), + ] { + let source = format!( + "import {{ Directive, Output, ViewChild, ContentChild, viewChild, contentChild }} \ + from '@angular/core';\n\ + @Directive({{ selector: '[d]' }})\n\ + export class D {{\n {member}\n}}" + ); + let result = expect_result(&source); + assert!( + result.diagnostics.is_empty(), + "`{member}` should raise no diagnostics. Got: {:?}", + result.diagnostics + ); + assert!( + !result.code.contains(&format!("{meta}:")), + "`{member}` should emit no `{meta}` entry. Got:\n{}", + result.code + ); + } +}