fix(ivy): allow abstract directives to have an invalid constructor (#…

…32987)

For abstract directives, i.e. directives without a selector, it may
happen that their constructor is called explicitly from a subclass,
hence its parameters are not required to be valid for Angular's DI
purposes. Prior to this commit however, having an abstract directive
with a constructor that has parameters that are not eligible for
Angular's DI would produce a compilation error.

A similar scenario may occur for `@Injectable`s, where an explicit
`use*` definition allows for the constructor to be irrelevant. For
example, the situation where `useFactory` is specified allows for the
constructor to be called explicitly with any value, so its constructor
parameters are not required to be valid. For `@Injectable`s this is
handled by generating a DI factory function that throws.

This commit implements the same solution for abstract directives, such
that a compilation error is avoided while still producing an error at
runtime if the type is instantiated implicitly by Angular's DI
mechanism.

Fixes #32981

PR Close #32987

packages/compiler-cli/src/ngtsc/annotations/src/component.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ConstantPool, CssSelector, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, ExternalExpr, Identifiers, InterpolationConfig, LexerRange, ParseError, ParseSourceFile, ParseTemplateOptions, R3ComponentMetadata, R3TargetBinder, SchemaMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr, compileComponentFromMetadata, makeBindingParser, parseTemplate} from '@angular/compiler';
+import {ConstantPool, CssSelector, DEFAULT_INTERPOLATION_CONFIG, DomElementSchemaRegistry, Expression, ExternalExpr, Identifiers, InterpolationConfig, LexerRange, ParseError, ParseSourceFile, ParseTemplateOptions, R3ComponentMetadata, R3FactoryTarget, R3TargetBinder, SchemaMetadata, SelectorMatcher, Statement, TmplAstNode, WrappedNodeExpr, compileComponentFromMetadata, makeBindingParser, parseTemplate} from '@angular/compiler';
 import * as ts from 'typescript';
 
 import {CycleAnalyzer} from '../../cycles';
@@ -490,7 +490,8 @@ export class ComponentDecoratorHandler implements
       CompileResult[] {
     const meta = analysis.meta;
     const res = compileComponentFromMetadata(meta, pool, makeBindingParser());
-    const factoryRes = compileNgFactoryDefField({...meta, injectFn: Identifiers.directiveInject});
+    const factoryRes = compileNgFactoryDefField(
+        {...meta, injectFn: Identifiers.directiveInject, target: R3FactoryTarget.Component});
     if (analysis.metadataStmt !== null) {
       factoryRes.statements.push(analysis.metadataStmt);
     }

packages/compiler-cli/src/ngtsc/annotations/src/directive.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ConstantPool, EMPTY_SOURCE_SPAN, Expression, Identifiers, ParseError, ParsedHostBindings, R3DirectiveMetadata, R3QueryMetadata, Statement, WrappedNodeExpr, compileDirectiveFromMetadata, makeBindingParser, parseHostBindings, verifyHostBindings} from '@angular/compiler';
+import {ConstantPool, EMPTY_SOURCE_SPAN, Expression, Identifiers, ParseError, ParsedHostBindings, R3DependencyMetadata, R3DirectiveMetadata, R3FactoryTarget, R3QueryMetadata, Statement, WrappedNodeExpr, compileDirectiveFromMetadata, makeBindingParser, parseHostBindings, verifyHostBindings} from '@angular/compiler';
 import * as ts from 'typescript';
 
 import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
@@ -19,7 +19,7 @@ import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerFl
 
 import {compileNgFactoryDefField} from './factory';
 import {generateSetClassMetadataCall} from './metadata';
-import {findAngularDecorator, getValidConstructorDependencies, readBaseClass, unwrapExpression, unwrapForwardRef} from './util';
+import {findAngularDecorator, getConstructorDependencies, readBaseClass, unwrapConstructorDependencies, unwrapExpression, unwrapForwardRef, validateConstructorDependencies} from './util';
 
 const EMPTY_OBJECT: {[key: string]: string} = {};
 
@@ -89,7 +89,8 @@ export class DirectiveDecoratorHandler implements
       CompileResult[] {
     const meta = analysis.meta;
     const res = compileDirectiveFromMetadata(meta, pool, makeBindingParser());
-    const factoryRes = compileNgFactoryDefField({...meta, injectFn: Identifiers.directiveInject});
+    const factoryRes = compileNgFactoryDefField(
+        {...meta, injectFn: Identifiers.directiveInject, target: R3FactoryTarget.Directive});
     if (analysis.metadataStmt !== null) {
       factoryRes.statements.push(analysis.metadataStmt);
     }
@@ -228,11 +229,23 @@ export function extractDirectiveMetadata(
     exportAs = resolved.split(',').map(part => part.trim());
   }
 
+  const rawCtorDeps = getConstructorDependencies(clazz, reflector, defaultImportRecorder, isCore);
+  let ctorDeps: R3DependencyMetadata[]|'invalid'|null;
+
+  // Non-abstract directives (those with a selector) require valid constructor dependencies, whereas
+  // abstract directives are allowed to have invalid dependencies, given that a subclass may call
+  // the constructor explicitly.
+  if (selector !== null) {
+    ctorDeps = validateConstructorDependencies(clazz, rawCtorDeps);
+  } else {
+    ctorDeps = unwrapConstructorDependencies(rawCtorDeps);
+  }
+
   // Detect if the component inherits from another class
   const usesInheritance = reflector.hasBaseClass(clazz);
   const metadata: R3DirectiveMetadata = {
     name: clazz.name.text,
-    deps: getValidConstructorDependencies(clazz, reflector, defaultImportRecorder, isCore), host,
+    deps: ctorDeps, host,
     lifecycle: {
         usesOnChanges,
     },

packages/compiler-cli/src/ngtsc/annotations/src/factory.ts

@@ -6,11 +6,11 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {R3FactoryDefMetadata, compileFactoryFromMetadata} from '@angular/compiler';
+import {R3FactoryMetadata, compileFactoryFunction} from '@angular/compiler';
 
 import {CompileResult} from '../../transform';
 
-export function compileNgFactoryDefField(metadata: R3FactoryDefMetadata): CompileResult {
-  const res = compileFactoryFromMetadata(metadata);
+export function compileNgFactoryDefField(metadata: R3FactoryMetadata): CompileResult {
+  const res = compileFactoryFunction(metadata);
   return {name: 'ɵfac', initializer: res.factory, statements: res.statements, type: res.type};
 }

packages/compiler-cli/src/ngtsc/annotations/src/injectable.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Expression, Identifiers, LiteralExpr, R3DependencyMetadata, R3InjectableMetadata, R3ResolvedDependencyType, Statement, WrappedNodeExpr, compileInjectable as compileIvyInjectable} from '@angular/compiler';
+import {Expression, Identifiers, LiteralExpr, R3DependencyMetadata, R3FactoryTarget, R3InjectableMetadata, R3ResolvedDependencyType, Statement, WrappedNodeExpr, compileInjectable as compileIvyInjectable} from '@angular/compiler';
 import * as ts from 'typescript';
 
 import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
@@ -16,7 +16,7 @@ import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPr
 
 import {compileNgFactoryDefField} from './factory';
 import {generateSetClassMetadataCall} from './metadata';
-import {findAngularDecorator, getConstructorDependencies, getValidConstructorDependencies, isAngularCore, unwrapForwardRef, validateConstructorDependencies} from './util';
+import {findAngularDecorator, getConstructorDependencies, getValidConstructorDependencies, isAngularCore, unwrapConstructorDependencies, unwrapForwardRef, validateConstructorDependencies} from './util';
 
 export interface InjectableHandlerData {
   meta: R3InjectableMetadata;
@@ -83,7 +83,8 @@ export class InjectableDecoratorHandler implements
         type: meta.type,
         typeArgumentCount: meta.typeArgumentCount,
         deps: analysis.ctorDeps,
-        injectFn: Identifiers.inject
+        injectFn: Identifiers.inject,
+        target: R3FactoryTarget.Injectable,
       });
       if (analysis.metadataStmt !== null) {
         factoryRes.statements.push(analysis.metadataStmt);
@@ -216,45 +217,25 @@ function extractInjectableCtorDeps(
     // Angular's DI.
     //
     // To deal with this, @Injectable() without an argument is more lenient, and if the
-    // constructor signature does not work for DI then a provider def (ɵprov) that throws.
+    // constructor signature does not work for DI then a factory definition (ɵfac) that throws is
+    // generated.
     if (strictCtorDeps) {
       ctorDeps = getValidConstructorDependencies(clazz, reflector, defaultImportRecorder, isCore);
     } else {
-      const possibleCtorDeps =
-          getConstructorDependencies(clazz, reflector, defaultImportRecorder, isCore);
-      if (possibleCtorDeps !== null) {
-        if (possibleCtorDeps.deps !== null) {
-          // This use of @Injectable has valid constructor dependencies.
-          ctorDeps = possibleCtorDeps.deps;
-        } else {
-          // This use of @Injectable is technically invalid. Generate a factory function which
-          // throws
-          // an error.
-          // TODO(alxhub): log warnings for the bad use of @Injectable.
-          ctorDeps = 'invalid';
-        }
-      }
+      ctorDeps = unwrapConstructorDependencies(
+          getConstructorDependencies(clazz, reflector, defaultImportRecorder, isCore));
     }
 
     return ctorDeps;
   } else if (decorator.args.length === 1) {
     const rawCtorDeps = getConstructorDependencies(clazz, reflector, defaultImportRecorder, isCore);
 
-    // rawCtorDeps will be null if the class has no constructor.
-    if (rawCtorDeps !== null) {
-      if (rawCtorDeps.deps !== null) {
-        // A constructor existed and had valid dependencies.
-        ctorDeps = rawCtorDeps.deps;
-      } else {
-        // A constructor existed but had invalid dependencies.
-        ctorDeps = 'invalid';
-      }
-    }
-
-    if (strictCtorDeps && !meta.useValue && !meta.useExisting && !meta.useClass &&
-        !meta.useFactory) {
+    if (strictCtorDeps && meta.useValue === undefined && meta.useExisting === undefined &&
+        meta.useClass === undefined && meta.useFactory === undefined) {
       // Since use* was not provided, validate the deps according to strictCtorDeps.
-      validateConstructorDependencies(clazz, rawCtorDeps);
+      ctorDeps = validateConstructorDependencies(clazz, rawCtorDeps);
+    } else {
+      ctorDeps = unwrapConstructorDependencies(rawCtorDeps);
     }
   }
 

packages/compiler-cli/src/ngtsc/annotations/src/pipe.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Identifiers, R3PipeMetadata, Statement, WrappedNodeExpr, compilePipeFromMetadata} from '@angular/compiler';
+import {Identifiers, R3FactoryTarget, R3PipeMetadata, Statement, WrappedNodeExpr, compilePipeFromMetadata} from '@angular/compiler';
 import * as ts from 'typescript';
 
 import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
@@ -114,7 +114,7 @@ export class PipeDecoratorHandler implements DecoratorHandler<PipeHandlerData, D
     const factoryRes = compileNgFactoryDefField({
       ...meta,
       injectFn: Identifiers.directiveInject,
-      isPipe: true,
+      target: R3FactoryTarget.Pipe,
     });
     if (analysis.metadataStmt !== null) {
       factoryRes.statements.push(analysis.metadataStmt);

packages/compiler-cli/src/ngtsc/annotations/src/util.ts

@@ -134,13 +134,39 @@ export function valueReferenceToExpression(
   }
 }
 
+/**
+ * Convert `ConstructorDeps` into the `R3DependencyMetadata` array for those deps if they're valid,
+ * or into an `'invalid'` signal if they're not.
+ *
+ * This is a companion function to `validateConstructorDependencies` which accepts invalid deps.
+ */
+export function unwrapConstructorDependencies(deps: ConstructorDeps | null): R3DependencyMetadata[]|
+    'invalid'|null {
+  if (deps === null) {
+    return null;
+  } else if (deps.deps !== null) {
+    // These constructor dependencies are valid.
+    return deps.deps;
+  } else {
+    // These deps are invalid.
+    return 'invalid';
+  }
+}
+
 export function getValidConstructorDependencies(
     clazz: ClassDeclaration, reflector: ReflectionHost,
     defaultImportRecorder: DefaultImportRecorder, isCore: boolean): R3DependencyMetadata[]|null {
   return validateConstructorDependencies(
       clazz, getConstructorDependencies(clazz, reflector, defaultImportRecorder, isCore));
 }
 
+/**
+ * Validate that `ConstructorDeps` does not have any invalid dependencies and convert them into the
+ * `R3DependencyMetadata` array if so, or raise a diagnostic if some deps are invalid.
+ *
+ * This is a companion function to `unwrapConstructorDependencies` which does not accept invalid
+ * deps.
+ */
 export function validateConstructorDependencies(
     clazz: ClassDeclaration, deps: ConstructorDeps | null): R3DependencyMetadata[]|null {
   if (deps === null) {

packages/compiler-cli/src/transformers/api.ts

@@ -66,8 +66,7 @@ export interface CompilerOptions extends ts.CompilerOptions {
   // be determined. When this value option is not provided or is `false`, constructor
   // parameters of classes marked with `@Injectable` whose type cannot be resolved will
   // produce a warning. With this option `true`, they produce an error. When this option is
-  // not provided is treated as if it were `false`. In Angular 6.0, if this option is not
-  // provided, it will be treated as `true`.
+  // not provided is treated as if it were `false`.
   strictInjectionParameters?: boolean;
 
   // Whether to generate a flat module index of the given name and the corresponding