refactor(compiler-cli): Add a diagnostic to detect forbiden invocations of required initializers (#63614)

The diagnostic will raise an error when required initializers (input, model, queries) are invoked the context of property initializers and contructors.

Docs will be provided in a follow-up

fixes #63602

PR Close #63614

goldens/public-api/compiler-cli/error_code.api.md

@@ -50,6 +50,7 @@ export enum ErrorCode {
     // (undocumented)
     DUPLICATE_DECORATED_PROPERTIES = 1012,
     DUPLICATE_VARIABLE_DECLARATION = 8006,
+    FORBIDDEN_REQUIRED_INITIALIZER_INVOCATION = 8118,
     HOST_BINDING_PARSE_ERROR = 5001,
     HOST_DIRECTIVE_COMPONENT = 2015,
     HOST_DIRECTIVE_CONFLICTING_ALIAS = 2018,

packages/compiler-cli/src/ngtsc/diagnostics/src/error_code.ts

@@ -611,6 +611,22 @@ export enum ErrorCode {
    */
   UNINVOKED_FUNCTION_IN_TEXT_INTERPOLATION = 8117,
 
+  /**
+   * A required initializer is being invoked in a forbidden context such as a property initializer
+   * or a constructor.
+   *
+   * For example:
+   * ```ts
+   * class MyComponent {
+   *  myInput = input.required();
+   *  somValue = this.myInput(); // Error
+   *
+   *  constructor() {
+   *    this.myInput(); // Error
+   *  }
+   */
+  FORBIDDEN_REQUIRED_INITIALIZER_INVOCATION = 8118,
+
   /**
    * The template type-checking engine would need to generate an inline type check block for a
    * component, but the current type-checking environment doesn't support it.

packages/compiler-cli/src/ngtsc/validation/src/rules/forbidden_required_initializer_invocation_rule.ts

@@ -0,0 +1,129 @@
+/*!
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.dev/license
+ */
+
+import ts from 'typescript';
+
+import {
+  InitializerApiFunction,
+  INPUT_INITIALIZER_FN,
+  MODEL_INITIALIZER_FN,
+  QUERY_INITIALIZER_FNS,
+  tryParseInitializerApi,
+} from '../../../annotations';
+import {ErrorCode, makeDiagnostic} from '../../../diagnostics';
+import {ImportedSymbolsTracker} from '../../../imports';
+import {ReflectionHost} from '../../../reflection';
+
+import {SourceFileValidatorRule} from './api';
+
+/** APIs whose usages should be checked by the rule. */
+const APIS_TO_CHECK: InitializerApiFunction[] = [
+  INPUT_INITIALIZER_FN,
+  MODEL_INITIALIZER_FN,
+  ...QUERY_INITIALIZER_FNS,
+];
+
+/**
+ * Rule that flags forbidden invocations of required initializers in property initializers and constructors.
+ */
+export class ForbiddenRequiredInitializersInvocationRule implements SourceFileValidatorRule {
+  constructor(
+    private reflector: ReflectionHost,
+    private importedSymbolsTracker: ImportedSymbolsTracker,
+  ) {}
+
+  shouldCheck(sourceFile: ts.SourceFile): boolean {
+    // Skip the traversal if there are no imports of the initializer APIs.
+    return APIS_TO_CHECK.some(({functionName, owningModule}) => {
+      return (
+        this.importedSymbolsTracker.hasNamedImport(sourceFile, functionName, owningModule) ||
+        this.importedSymbolsTracker.hasNamespaceImport(sourceFile, owningModule)
+      );
+    });
+  }
+
+  checkNode(node: ts.Node): ts.Diagnostic[] | null {
+    if (!ts.isClassDeclaration(node)) return null;
+
+    const requiredInitializerDeclarations = node.members.filter(
+      (m) => ts.isPropertyDeclaration(m) && this.isPropDeclarationARequiredInitializer(m),
+    );
+
+    const diagnostics: ts.Diagnostic[] = [];
+
+    // Handling of the usages in props initializations
+    for (let decl of node.members) {
+      if (!ts.isPropertyDeclaration(decl)) continue;
+
+      const initiallizerExpr = decl.initializer;
+      if (!initiallizerExpr) continue;
+
+      checkForbiddenInvocation(initiallizerExpr);
+    }
+
+    function checkForbiddenInvocation(node: ts.Node): boolean | undefined {
+      if (ts.isArrowFunction(node) || ts.isFunctionExpression(node)) return;
+
+      if (
+        ts.isPropertyAccessExpression(node) &&
+        node.expression.kind === ts.SyntaxKind.ThisKeyword &&
+        // With the following we make sure we only flag invoked required initializers
+        ts.isCallExpression(node.parent) &&
+        node.parent.expression === node
+      ) {
+        const requiredProp = requiredInitializerDeclarations.find(
+          (prop) => prop.name.getText() === node.name.getText(),
+        );
+        if (requiredProp) {
+          const initializerFn = (
+            requiredProp.initializer.expression as ts.PropertyAccessExpression
+          ).expression.getText();
+          diagnostics.push(
+            makeDiagnostic(
+              ErrorCode.FORBIDDEN_REQUIRED_INITIALIZER_INVOCATION,
+              node,
+              `\`${node.name.getText()}\` is a required \`${initializerFn}\` and does not have a value in this context.`,
+            ),
+          );
+        }
+      }
+
+      return node.forEachChild(checkForbiddenInvocation);
+    }
+
+    const ctor = getConstructorFromClass(node);
+    if (ctor) {
+      checkForbiddenInvocation(ctor);
+    }
+
+    return diagnostics;
+  }
+  private isPropDeclarationARequiredInitializer(
+    node: ts.PropertyDeclaration,
+  ): node is ts.PropertyDeclaration & {initializer: ts.CallExpression} {
+    if (!node.initializer) return false;
+
+    const identifiedInitializer = tryParseInitializerApi(
+      APIS_TO_CHECK,
+      node.initializer,
+      this.reflector,
+      this.importedSymbolsTracker,
+    );
+
+    if (identifiedInitializer === null || !identifiedInitializer.isRequired) return false;
+
+    return true;
+  }
+}
+
+function getConstructorFromClass(node: ts.ClassDeclaration): ts.ConstructorDeclaration | undefined {
+  // We also check for a constructor body to avoid picking up parent constructors.
+  return node.members.find(
+    (m): m is ts.ConstructorDeclaration => ts.isConstructorDeclaration(m) && m.body !== undefined,
+  );
+}

packages/compiler-cli/src/ngtsc/validation/src/source_file_validator.ts

@@ -15,6 +15,7 @@ import {SourceFileValidatorRule} from './rules/api';
 import {InitializerApiUsageRule} from './rules/initializer_api_usage_rule';
 import {UnusedStandaloneImportsRule} from './rules/unused_standalone_imports_rule';
 import {TemplateTypeChecker, TypeCheckingConfig} from '../../typecheck/api';
+import {ForbiddenRequiredInitializersInvocationRule} from './rules/forbidden_required_initializer_invocation_rule';
 
 /**
  * Validates that TypeScript files match a specific set of rules set by the Angular compiler.
@@ -37,6 +38,10 @@ export class SourceFileValidator {
         importedSymbolsTracker,
       ),
     );
+
+    this.rules.push(
+      new ForbiddenRequiredInitializersInvocationRule(reflector, importedSymbolsTracker),
+    );
   }
 
   /**

packages/compiler-cli/test/ngtsc/authoring_diagnostics_spec.ts

@@ -274,5 +274,197 @@ runInEachFileSystem(() => {
         'Unsupported call to the input function. This function can only be used as the initializer of a property on a @Component or @Directive class.',
       );
     });
+
+    describe('required initializer functions validation', () => {
+      it('should report required input being used as property initialized', () => {
+        env.write(
+          'test.ts',
+          `
+        import {input, Component} from '@angular/core';
+
+        function foobar(x:any) {}
+
+        @Component({template: \`\`})
+        export class Test {
+          reqInp = input.required<string>();
+          reqInp2 = input.required<boolean>();
+          smthg = this.reqInp();
+          smthg2 = foobar(this.reqInp2());
+        }
+      `,
+        );
+
+        const diags = env.driveDiagnostics();
+        expect(diags.length).toBe(2);
+        expect(diags[0].messageText).toContain('`reqInp` is a required `input`');
+        expect(diags[1].messageText).toContain('`reqInp2` is a required `input`');
+      });
+
+      it('should report required input being invoked inside a constructor', () => {
+        env.write(
+          'test.ts',
+          `
+        import {input, Component} from '@angular/core';
+
+        function foobar(x:any) {}
+
+        @Component({template: \`\`})
+        export class Test {
+          reqInp = input.required<string>();
+          reqInp2 = input.required<boolean>();
+
+          constructor() {
+            const smthg = this.reqInp();
+            const smthg2 = foobar(this.reqInp2());
+          }
+        }
+      `,
+        );
+
+        const diags = env.driveDiagnostics();
+        expect(diags.length).toBe(2);
+        expect(diags[0].messageText).toContain('`reqInp` is a required `input`');
+        expect(diags[1].messageText).toContain('`reqInp2` is a required `input`');
+      });
+
+      it('should report required model being used as property initialized', () => {
+        env.write(
+          'test.ts',
+          `
+        import {model, Component} from '@angular/core';
+
+        @Component({template: \`\`})
+        export class Test {
+          reqModel = model.required<string>();
+          smthg = this.reqModel();
+        }
+      `,
+        );
+
+        const diags = env.driveDiagnostics();
+        expect(diags.length).toBe(1);
+        expect(diags[0].messageText).toContain('`reqModel` is a required `model`');
+      });
+
+      it('should report required model being invoked in a constructor despite being set before hand', () => {
+        // While this code might be valid at runtime, we still prefer to flag any required model reads.
+        env.write(
+          'test.ts',
+          `
+        import {model, Component} from '@angular/core';
+
+        @Component({template: \`\`})
+        export class Test {
+          reqModel = model.required<string>();
+          constructor() {
+            this.reqModel.set('foobar');
+            const smthg = this.reqModel();
+          }
+        }
+      `,
+        );
+
+        const diags = env.driveDiagnostics();
+        expect(diags.length).toBe(1);
+        expect(diags[0].messageText).toContain('`reqModel` is a required `model`');
+      });
+
+      it('should report required viewChild being used as property initialized', () => {
+        env.write(
+          'test.ts',
+          `
+        import {viewChild, Component} from '@angular/core';
+
+        @Component({template: \`\`})
+        export class Test {
+          reqChild = viewChild.required('someRef');
+          smthg = this.reqChild();
+        }
+      `,
+        );
+
+        const diags = env.driveDiagnostics();
+        expect(diags.length).toBe(1);
+        expect(diags[0].messageText).toContain('`reqChild` is a required `viewChild`');
+      });
+
+      it('should not report required input being invoked inside an effect or a computed', () => {
+        env.write(
+          'test.ts',
+          `
+        import {input, Component, effect, computed} from '@angular/core';
+
+        function foobar(x:any) {}
+
+        @Component({template: \`\`})
+        export class Test {
+          reqInp = input.required<string>();
+          reqInp2 = input.required<boolean>();
+          _ = effect(() => { this.reqInp(); foobar(this.reqInp2()); });
+          __ = effect(function() { this.reqInp(); foobar(this.reqInp2()); });
+          comp = computed(() => this.reqInp());
+
+          constructor() {
+            effect(() => { this.reqInp(); foobar(this.reqInp2()); });
+          }
+        }
+      `,
+        );
+
+        const diags = env.driveDiagnostics();
+        expect(diags.length).toBe(0);
+      });
+
+      it('should not report non-required input being invoked in contructrors or in property initialization', () => {
+        env.write(
+          'test.ts',
+          `
+        import {input, Component, effect, computed} from '@angular/core';
+
+        function foobar(x:any) {}
+
+        @Component({template: \`\`})
+        export class Test {
+          reqInp = input<string>('');
+          reqInp2 = input<boolean>(true);
+          smthg = this.reqInp();
+          smthg2 = foobar(this.reqInp2());
+
+          constructor() {
+            const smthg = this.reqInp();
+            const smthg2 = foobar(this.reqInp2());
+          }
+        }
+      `,
+        );
+
+        const diags = env.driveDiagnostics();
+        expect(diags.length).toBe(0);
+      });
+
+      it('should not report non-invocations of required inputs', () => {
+        env.write(
+          'test.ts',
+          `
+        import {input, Component, effect, computed} from '@angular/core';
+        import {toObservable} from '@angular/core/rxjs-interop';
+
+
+        @Component({template: \`\`})
+        export class Test {
+          reqInp = input.required<string>();
+          obs$ = toObservable(this.reqInp);
+
+          constructor() {
+            const obs$ = toObservable(this.reqInp);
+          }
+        }
+      `,
+        );
+
+        const diags = env.driveDiagnostics();
+        expect(diags.length).toBe(0);
+      });
+    });
   });
 });

packages/core/test/acceptance/authoring/model_inputs_spec.ts

@@ -359,29 +359,6 @@ describe('model inputs', () => {
     expect(emittedValues).toEqual([2]);
   });
 
-  it('should throw if a required model input is accessed too early', () => {
-    @Directive({selector: '[dir]'})
-    class Dir {
-      value = model.required<number>();
-
-      constructor() {
-        this.value();
-      }
-    }
-
-    @Component({
-      template: '<div [(value)]="value" dir></div>',
-      imports: [Dir],
-    })
-    class App {
-      value = 1;
-    }
-
-    expect(() => TestBed.createComponent(App)).toThrowError(
-      /Model is required but no value is available yet/,
-    );
-  });
-
   it('should throw if a required model input is updated too early', () => {
     @Directive({selector: '[dir]'})
     class Dir {

packages/core/test/acceptance/authoring/signal_inputs_spec.ts

@@ -180,32 +180,6 @@ describe('signal inputs', () => {
     expect(transformRunCount).toBe(1);
   });
 
-  it('should throw error if a required input is accessed too early', () => {
-    @Component({
-      selector: 'input-comp',
-      template: 'input:{{input()}}',
-    })
-    class InputComp {
-      input = input.required<number>({debugName: 'input'});
-
-      constructor() {
-        this.input();
-      }
-    }
-
-    @Component({
-      template: `<input-comp [input]="value" />`,
-      imports: [InputComp],
-    })
-    class TestCmp {
-      value = 1;
-    }
-
-    expect(() => TestBed.createComponent(TestCmp)).toThrowError(
-      /Input "input" is required but no value is available yet/,
-    );
-  });
-
   it('should be possible to bind to an inherited input', () => {
     @Directive()
     class BaseDir {

packages/core/test/acceptance/authoring/signal_queries_spec.ts

@@ -83,24 +83,6 @@ describe('queries as signals', () => {
       expect(fixture.componentInstance.foundEl()).toBeTrue();
     });
 
-    it('should throw if required query is read in the constructor', () => {
-      @Component({
-        template: `<div #el></div>`,
-      })
-      class AppComponent {
-        divEl = viewChild.required<ElementRef<HTMLDivElement>>('el');
-
-        constructor() {
-          this.divEl();
-        }
-      }
-
-      // non-required query results are undefined before we run creation mode on the view queries
-      expect(() => {
-        TestBed.createComponent(AppComponent);
-      }).toThrowError(/NG0951: Child query result is required but no value is available/);
-    });
-
     it('should query for multiple elements in a template', () => {
       @Component({
         template: `

packages/core/test/acceptance/component_spec.ts

@@ -21,10 +21,12 @@ import {
   Injector,
   input,
   Input,
+  model,
   NgModule,
   OnDestroy,
   reflectComponentType,
   Renderer2,
+  viewChild,
   ViewChild,
   ViewContainerRef,
   ViewEncapsulation,
@@ -922,4 +924,74 @@ describe('component', () => {
       forbidOrphanRendering: true,
     });
   });
+
+  describe('required initiliazers', () => {
+    // The following tests are specifically not in the authoring subdirectory to ensure that AOT doesn't check (and throws) forbidden required reads.
+    it('should throw error if a required input is accessed too early', () => {
+      @Component({
+        selector: 'input-comp',
+        template: 'input:{{input()}}',
+      })
+      class InputComp {
+        input = input.required<number>({debugName: 'input'});
+
+        constructor() {
+          this.input();
+        }
+      }
+
+      @Component({
+        template: `<input-comp [input]="value" />`,
+        imports: [InputComp],
+      })
+      class TestCmp {
+        value = 1;
+      }
+
+      expect(() => TestBed.createComponent(TestCmp)).toThrowError(
+        /Input "input" is required but no value is available yet/,
+      );
+    });
+
+    it('should throw if a required model input is accessed too early', () => {
+      @Directive({selector: '[dir]'})
+      class Dir {
+        value = model.required<number>();
+
+        constructor() {
+          this.value();
+        }
+      }
+
+      @Component({
+        template: '<div [(value)]="value" dir></div>',
+        imports: [Dir],
+      })
+      class App {
+        value = 1;
+      }
+
+      expect(() => TestBed.createComponent(App)).toThrowError(
+        /Model is required but no value is available yet/,
+      );
+    });
+
+    it('should throw if required query is read in the constructor', () => {
+      @Component({
+        template: `<div #el></div>`,
+      })
+      class AppComponent {
+        divEl = viewChild.required<ElementRef<HTMLDivElement>>('el');
+
+        constructor() {
+          this.divEl();
+        }
+      }
+
+      // non-required query results are undefined before we run creation mode on the view queries
+      expect(() => {
+        TestBed.createComponent(AppComponent);
+      }).toThrowError(/NG0951: Child query result is required but no value is available/);
+    });
+  });
 });