fix(compiler): check ctor param decorators (arity + unexpected) like di.ts

crates/oxc_angular_compiler/src/component/transform.rs

@@ -42,7 +42,7 @@ use crate::directive::collect_string_consts;
 use crate::directive::{
     R3QueryMetadata, create_content_queries_function, create_view_queries_function,
     decorator_io_errors, extract_class_queries, extract_directive_metadata,
-    find_directive_decorator, generate_directive_definitions,
+    find_directive_decorator, generate_directive_definitions, param_decorator_errors,
 };
 use crate::dts;
 use crate::injectable::{
@@ -2910,12 +2910,17 @@ pub fn transform_angular_file(
             let implicit_standalone = options.implicit_standalone();
 
             // `inputs:`/`outputs:` forms ngtsc rejects, rather than dropping them silently.
-            result.diagnostics.extend(decorator_io_errors(
-                allocator,
-                class,
-                Some(source),
-                &string_consts,
-            ));
+            let io_errors = decorator_io_errors(allocator, class, Some(source), &string_consts);
+            result.diagnostics.extend(io_errors.iter().cloned());
+            if io_errors.is_empty() {
+                // Constructor parameter decorators: ngtsc's
+                // `getConstructorDependencies` (arity, unexpected decorators)
+                // runs only once the io/query checks passed. Upstream also
+                // throws earlier — malformed decorator args, a missing
+                // selector — for shapes oxc doesn't diagnose yet, so this
+                // can be the first error where ngtsc would report another.
+                result.diagnostics.extend(param_decorator_errors(class, &string_consts));
+            }
 
             if let Some(mut metadata) = extract_component_metadata(
                 &allocator,

crates/oxc_angular_compiler/src/directive/decorator.rs

@@ -1268,6 +1268,91 @@ pub fn decorator_io_errors<'a>(
         .collect()
 }
 
+/// 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
+/// import (`@core.Inject()`), the local name otherwise.
+fn angular_decorator_name<'a>(decorator: &'a Decorator<'a>, consts: &StringConsts<'a>) -> &'a str {
+    let callee = match &decorator.expression {
+        Expression::CallExpression(call) => &call.callee,
+        expr => expr,
+    };
+    match callee {
+        Expression::Identifier(id) => consts
+            .scope()
+            .import(&id.name)
+            .and_then(|import| import.imported)
+            // A default import has no imported name upstream
+            // (`getExportedName` falls back to the local identifier).
+            .filter(|name| *name != "default")
+            .unwrap_or(id.name.as_str()),
+        Expression::StaticMemberExpression(m) => m.property.name.as_str(),
+        _ => "",
+    }
+}
+
+/// The first error ngtsc's `getConstructorDependencies` (`di.ts`) raises for a
+/// class's constructor parameters: `@Inject()` / `@Attribute()` with anything
+/// but one argument (`DECORATOR_ARITY_WRONG`), or another `@angular/core`
+/// decorator on a parameter (`Unexpected decorator X on parameter.`).
+///
+/// Runs for every class a DI handler compiles — `@Component`, `@Directive`,
+/// `@Pipe`, `@NgModule` and `@Injectable` all call it — after the io/query
+/// checks, which is where it sits inside `extractDirectiveMetadata`.
+/// `getConstructorDependencies` only sees the constructor's parameters, so
+/// decorators on other parameters are not checked.
+pub fn param_decorator_errors<'a>(
+    class: &'a Class<'a>,
+    consts: &StringConsts<'a>,
+) -> std::vec::Vec<OxcDiagnostic> {
+    // Not a class any DI handler compiles: per-class analysis never looks
+    // at its parameters. (Upstream's `InjectableClassRegistry` can still
+    // reach undecorated provider/base classes through `getConstructorDependencies`;
+    // oxc has no equivalent registry checks.)
+    if !class.decorators.iter().any(|d| super::angular_class_decorator(d, consts).is_some()) {
+        return std::vec::Vec::new();
+    }
+
+    class
+        .body
+        .body
+        .iter()
+        .filter_map(|element| match element {
+            // Upstream picks the first constructor *with a body*
+            // (typescript.ts `findConstructorDeclaration`): an overload
+            // signature's parameters are not checked.
+            ClassElement::MethodDefinition(m)
+                if m.kind == MethodDefinitionKind::Constructor && m.value.body.is_some() =>
+            {
+                Some(m)
+            }
+            _ => None,
+        })
+        .flat_map(|ctor| ctor.value.params.items.iter())
+        .flat_map(|param| param.decorators.iter())
+        .filter(|d| super::is_angular_core_decorator(d, Some(consts)))
+        .find_map(|decorator| {
+            let args: Option<&oxc_allocator::Vec<'_, Argument<'_>>> = match &decorator.expression {
+                Expression::CallExpression(call) => Some(&call.arguments),
+                _ => None,
+            };
+            let error = match angular_decorator_name(decorator, consts) {
+                name @ ("Inject" | "Attribute") => {
+                    if args.is_none_or(|a| a.len() != 1) {
+                        Some(format!("Unexpected number of arguments to @{name}()."))
+                    } else {
+                        None
+                    }
+                }
+                "Optional" | "Self" | "SkipSelf" | "Host" => None,
+                name => Some(format!("Unexpected decorator {name} on parameter.")),
+            };
+            error.map(|message| OxcDiagnostic::error(message).with_label(decorator.span))
+        })
+        .into_iter()
+        .collect()
+}
+
 /// ngtsc's error for the `@Input(...)` decorator of the member `name`
 /// (`tryParseInputFieldMapping`): more than one argument, an argument that
 /// isn't `null`, a string or an object, or a `transform` it can't use. Options

crates/oxc_angular_compiler/src/directive/mod.rs

@@ -33,7 +33,7 @@ pub(crate) use decorator::{
 };
 pub use decorator::{
     StringConsts, collect_string_consts, decorator_io_errors, extract_directive_metadata,
-    find_directive_decorator_span,
+    find_directive_decorator_span, param_decorator_errors,
 };
 pub(crate) use decorator::{
     angular_decorator_config, extract_string_value, resolve_template_literal,

crates/oxc_angular_compiler/tests/integration_test.rs

@@ -9386,8 +9386,9 @@ fn test_class_metadata_lists_any_angular_core_decorator() {
     // listed; a same-named decorator from another module is not.
     //
     // Note: ngtsc rejects a non-DI `@angular/core` decorator on a ctor param
-    // in DI analysis (DECORATOR_UNEXPECTED), so upstream never emits this
-    // metadata — oxc lists it, matching the metadata filter's own semantics.
+    // in DI analysis (`Unexpected decorator Injectable on parameter.`),
+    // which oxc now reports too (#550); the metadata emit assertion below
+    // still verifies the filter's own semantics on the emitted code.
     let allocator = Allocator::default();
     let source = r"
 import { Component, Inject, Injectable } from '@angular/core';
@@ -9417,7 +9418,14 @@ export class TestComponent {
     let options = ComponentTransformOptions::default();
     let result =
         transform_angular_file(&allocator, "test.component.ts", source, Some(&options), None);
-    assert!(!result.has_errors(), "Should not have errors: {:?}", result.diagnostics);
+    assert!(
+        result
+            .diagnostics
+            .iter()
+            .any(|d| format!("{d}").contains("Unexpected decorator Injectable on parameter.")),
+        "Should report DECORATOR_UNEXPECTED for @Injectable on a ctor param: {:?}",
+        result.diagnostics
+    );
 
     let compact: String = result.code.chars().filter(|c| !c.is_whitespace()).collect();
 
@@ -16348,6 +16356,132 @@ fn test_static_member_checks_keep_ngtsc_error_order() {
     );
 }
 
+#[test]
+fn test_param_decorator_arity_is_diagnostic() {
+    // ngtsc's `getConstructorDependencies` (di.ts): `@Inject`/`@Attribute` need
+    // exactly one argument; bare (`@Inject`) counts as zero.
+    for (param, expected) in [
+        ("@Inject() x: any", "Unexpected number of arguments to @Inject()."),
+        ("@Inject(A, B) x: any", "Unexpected number of arguments to @Inject()."),
+        ("@Inject x: any", "Unexpected number of arguments to @Inject()."),
+        ("@Attribute() x: any", "Unexpected number of arguments to @Attribute()."),
+        ("@Attribute('a', 'b') x: any", "Unexpected number of arguments to @Attribute()."),
+        ("@Input() x: any", "Unexpected decorator Input on parameter."),
+    ] {
+        let source = format!(
+            "import {{ Directive, Inject, Attribute, Input }} from '@angular/core';\n\
+             @Directive({{ selector: '[d]' }})\n\
+             export class D {{\n    constructor({param}) {{}}\n}}"
+        );
+        let diagnostics = expect_diagnostics(&source);
+        assert!(
+            diagnostics.iter().any(|d| d.contains(expected)),
+            "`{param}` should report {expected:?}. Got: {diagnostics:?}"
+        );
+    }
+}
+
+#[test]
+fn test_param_decorator_checks_respect_import_and_kind() {
+    // Only the constructor's parameters are checked (upstream only visits
+    // ctor params), only `@angular/core` decorators count, and aliases resolve
+    // to the imported name.
+    for (source, expected) in [
+        // Foreign @Inject: no diagnostic.
+        (
+            "import { Directive } from '@angular/core';\n\
+             import { Inject } from 'not-angular';\n\
+             @Directive({ selector: '[d]' })\n\
+             export class D {\n    constructor(@Inject() x: any) {}\n}",
+            None,
+        ),
+        // Aliased import reports the imported name upstream
+        // (`dec.import.name`).
+        (
+            "import { Directive, Inject as Inj } from '@angular/core';\n\
+             @Directive({ selector: '[d]' })\n\
+             export class D {\n    constructor(@Inj() x: any) {}\n}",
+            Some("Unexpected number of arguments to @Inject()."),
+        ),
+        // Namespace import resolves the member name.
+        (
+            "import { Directive } from '@angular/core';\n\
+             import * as core from '@angular/core';\n\
+             @Directive({ selector: '[d]' })\n\
+             export class D {\n    constructor(@core.Inject() x: any) {}\n}",
+            Some("Unexpected number of arguments to @Inject()."),
+        ),
+        // A bad decorator on a non-ctor param is not checked.
+        (
+            "import { Directive, Inject } from '@angular/core';\n\
+             @Directive({ selector: '[d]' })\n\
+             export class D {\n    m(@Inject() x: any) {}\n}",
+            None,
+        ),
+        // An undecorated class is never looked at.
+        (
+            "import { Inject } from '@angular/core';\n\
+             export class D {\n    constructor(@Inject() x: any) {}\n}",
+            None,
+        ),
+        // Valid decorators: arity ok, and flags take no arity check.
+        (
+            "import { Directive, Inject, Attribute, Optional } from '@angular/core';\n\
+             const TOKEN = 't';\n\
+             @Directive({ selector: '[d]' })\n\
+             export class D {\n    constructor(@Inject(TOKEN) a: any, @Attribute('x') b: any, @Optional(1) c: any) {}\n}",
+            None,
+        ),
+        // A bodiless overload signature is not the checked constructor.
+        (
+            "import { Directive, Inject } from '@angular/core';\n\
+             @Directive({ selector: '[d]' })\n\
+             export class D {\n    constructor(@Inject() x: any);\n    constructor(x: any) {}\n}",
+            None,
+        ),
+        // Default import resolves to the local name upstream
+        // (`getExportedName` falls back for non-ImportSpecifier decls).
+        (
+            "import { Directive } from '@angular/core';\n\
+             import Inj from '@angular/core';\n\
+             @Directive({ selector: '[d]' })\n\
+             export class D {\n    constructor(@Inj() x: any) {}\n}",
+            Some("Unexpected decorator Inj on parameter."),
+        ),
+    ] {
+        let diagnostics = expect_diagnostics(source);
+        match expected {
+            Some(expected) => assert!(
+                diagnostics.iter().any(|d| d.contains(expected)),
+                "should report {expected:?}. Got: {diagnostics:?}"
+            ),
+            None => assert!(
+                diagnostics.is_empty(),
+                "should not report a param-decorator error. Got: {diagnostics:?}"
+            ),
+        }
+    }
+}
+
+#[test]
+fn test_param_decorator_error_waits_for_io_checks() {
+    // Upstream throws the io error first: `getConstructorDependencies` runs
+    // last inside `extractDirectiveMetadata`, so a class with both problems
+    // reports only the input error.
+    let source = "import { Directive, Inject, Input } from '@angular/core';\n\
+                  @Directive({ selector: '[d]' })\n\
+                  export class D {\n    @Input() static x = 0;\n    constructor(@Inject() y: any) {}\n}";
+    let diagnostics = expect_diagnostics(source);
+    assert!(
+        diagnostics.iter().any(|d| d.contains("is incorrectly declared as static")),
+        "io error should be reported. Got: {diagnostics:?}"
+    );
+    assert!(
+        !diagnostics.iter().any(|d| d.contains("Unexpected number of arguments")),
+        "param error should be suppressed while an io error exists. Got: {diagnostics:?}"
+    );
+}
+
 #[test]
 fn test_instance_input_output_query_members_have_no_static_diagnostic() {
     // The same declarations without `static` stay valid.