fix(ivy): include directive base class metadata when generating TCBs (#29698)

Previously the template type-checking code only considered the metadata of
directive classes actually referenced in the template. If those directives
had base classes, any inputs/outputs/etc of the base classes were not
tracked when generating the TCB. This resulted in bindings to those inputs
being incorrectly attributed to the host component or element.

This commit uses the new metadata package to follow directive inheritance
chains and use the full metadata for a directive for TCB generation.

Testing strategy: Template type-checking tests included.

PR Close #29698

packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts

@@ -14,7 +14,7 @@ import * as ts from 'typescript';
 import {BaseDefDecoratorHandler, ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, PipeDecoratorHandler, ReferencesRegistry, ResourceLoader} from '../../../src/ngtsc/annotations';
 import {CycleAnalyzer, ImportGraph} from '../../../src/ngtsc/cycles';
 import {AbsoluteModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NOOP_DEFAULT_IMPORT_RECORDER, ReferenceEmitter} from '../../../src/ngtsc/imports';
-import {CompoundMetadataRegistry, DtsMetadataReader, LocalMetadataRegistry} from '../../../src/ngtsc/metadata';
+import {CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, LocalMetadataRegistry} from '../../../src/ngtsc/metadata';
 import {PartialEvaluator} from '../../../src/ngtsc/partial_evaluator';
 import {AbsoluteFsPath, LogicalFileSystem} from '../../../src/ngtsc/path';
 import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../../src/ngtsc/scope';
@@ -68,6 +68,7 @@ export class DecorationAnalyzer {
   resourceManager = new NgccResourceLoader();
   metaRegistry = new LocalMetadataRegistry();
   dtsMetaReader = new DtsMetadataReader(this.typeChecker, this.reflectionHost);
+  fullMetaReader = new CompoundMetadataReader([this.metaRegistry, this.dtsMetaReader]);
   refEmitter = new ReferenceEmitter([
     new LocalIdentifierStrategy(),
     new AbsoluteModuleStrategy(this.program, this.typeChecker, this.options, this.host),
@@ -88,8 +89,9 @@ export class DecorationAnalyzer {
   handlers: DecoratorHandler<any, any>[] = [
     new BaseDefDecoratorHandler(this.reflectionHost, this.evaluator, this.isCore),
     new ComponentDecoratorHandler(
-        this.reflectionHost, this.evaluator, this.fullRegistry, this.scopeRegistry, this.isCore,
-        this.resourceManager, this.rootDirs, /* defaultPreserveWhitespaces */ false,
+        this.reflectionHost, this.evaluator, this.fullRegistry, this.fullMetaReader,
+        this.scopeRegistry, this.isCore, this.resourceManager, this.rootDirs,
+        /* defaultPreserveWhitespaces */ false,
         /* i18nUseExternalIds */ true, this.moduleResolver, this.cycleAnalyzer, this.refEmitter,
         NOOP_DEFAULT_IMPORT_RECORDER),
     new DirectiveDecoratorHandler(

packages/compiler-cli/ngcc/src/utils.ts

@@ -48,5 +48,6 @@ export function findAll<T>(node: ts.Node, test: (node: ts.Node) => node is ts.No
  */
 export function hasNameIdentifier(declaration: ts.Declaration): declaration is ts.Declaration&
     {name: ts.Identifier} {
-  return ts.isIdentifier((declaration as any).name);
+  const namedDeclaration: ts.Declaration & {name?: ts.Node} = declaration;
+  return namedDeclaration.name !== undefined && ts.isIdentifier(namedDeclaration.name);
 }

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

@@ -13,7 +13,8 @@ import * as ts from 'typescript';
 import {CycleAnalyzer} from '../../cycles';
 import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
 import {DefaultImportRecorder, ModuleResolver, Reference, ReferenceEmitter} from '../../imports';
-import {DirectiveMeta, MetadataRegistry, extractDirectiveGuards} from '../../metadata';
+import {DirectiveMeta, MetadataReader, MetadataRegistry, extractDirectiveGuards} from '../../metadata';
+import {flattenInheritedDirectiveMetadata} from '../../metadata/src/inheritance';
 import {EnumValue, PartialEvaluator} from '../../partial_evaluator';
 import {ClassDeclaration, Decorator, ReflectionHost, filterToMembersWithDecorator, reflectObjectLiteral} from '../../reflection';
 import {LocalModuleScopeRegistry} from '../../scope';
@@ -24,7 +25,7 @@ import {tsSourceMapBug29300Fixed} from '../../util/src/ts_source_map_bug_29300';
 import {ResourceLoader} from './api';
 import {extractDirectiveMetadata, extractQueriesFromDecorator, parseFieldArrayValue, queriesFromFields} from './directive';
 import {generateSetClassMetadataCall} from './metadata';
-import {findAngularDecorator, isAngularCoreReference, isExpressionForwardReference, unwrapExpression} from './util';
+import {findAngularDecorator, isAngularCoreReference, isExpressionForwardReference, readBaseClass, unwrapExpression} from './util';
 
 const EMPTY_MAP = new Map<string, Expression>();
 const EMPTY_ARRAY: any[] = [];
@@ -42,8 +43,9 @@ export class ComponentDecoratorHandler implements
     DecoratorHandler<ComponentHandlerData, Decorator> {
   constructor(
       private reflector: ReflectionHost, private evaluator: PartialEvaluator,
-      private metaRegistry: MetadataRegistry, private scopeRegistry: LocalModuleScopeRegistry,
-      private isCore: boolean, private resourceLoader: ResourceLoader, private rootDirs: string[],
+      private metaRegistry: MetadataRegistry, private metaReader: MetadataReader,
+      private scopeRegistry: LocalModuleScopeRegistry, private isCore: boolean,
+      private resourceLoader: ResourceLoader, private rootDirs: string[],
       private defaultPreserveWhitespaces: boolean, private i18nUseExternalIds: boolean,
       private moduleResolver: ModuleResolver, private cycleAnalyzer: CycleAnalyzer,
       private refEmitter: ReferenceEmitter, private defaultImportRecorder: DefaultImportRecorder) {}
@@ -221,7 +223,7 @@ export class ComponentDecoratorHandler implements
         outputs: metadata.outputs,
         queries: metadata.queries.map(query => query.propertyName),
         isComponent: true, ...extractDirectiveGuards(node, this.reflector),
-        baseClass: null,
+        baseClass: readBaseClass(node, this.reflector, this.evaluator),
       });
     }
 
@@ -307,7 +309,8 @@ export class ComponentDecoratorHandler implements
     const matcher = new SelectorMatcher<DirectiveMeta>();
     if (scope !== null) {
       for (const meta of scope.compilation.directives) {
-        matcher.addSelectables(CssSelector.parse(meta.selector), meta);
+        const extMeta = flattenInheritedDirectiveMetadata(this.metaReader, meta.ref);
+        matcher.addSelectables(CssSelector.parse(meta.selector), extMeta);
       }
       const bound = new R3TargetBinder(matcher).bind({template: meta.parsedTemplate});
       ctx.addTemplate(node, bound);

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

@@ -19,7 +19,7 @@ import {LocalModuleScopeRegistry} from '../../scope/src/local';
 import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform';
 
 import {generateSetClassMetadataCall} from './metadata';
-import {findAngularDecorator, getValidConstructorDependencies, unwrapExpression, unwrapForwardRef} from './util';
+import {findAngularDecorator, getValidConstructorDependencies, readBaseClass, unwrapExpression, unwrapForwardRef} from './util';
 
 const EMPTY_OBJECT: {[key: string]: string} = {};
 
@@ -69,7 +69,7 @@ export class DirectiveDecoratorHandler implements
         outputs: analysis.outputs,
         queries: analysis.queries.map(query => query.propertyName),
         isComponent: false, ...extractDirectiveGuards(node, this.reflector),
-        baseClass: null,
+        baseClass: readBaseClass(node, this.reflector, this.evaluator),
       });
     }
 

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

@@ -11,8 +11,8 @@ import * as ts from 'typescript';
 
 import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
 import {DefaultImportRecorder, ImportMode, Reference, ReferenceEmitter} from '../../imports';
-import {ForeignFunctionResolver} from '../../partial_evaluator';
-import {ClassDeclaration, CtorParameter, Decorator, Import, ReflectionHost, TypeValueReference} from '../../reflection';
+import {ForeignFunctionResolver, PartialEvaluator} from '../../partial_evaluator';
+import {ClassDeclaration, CtorParameter, Decorator, Import, ReflectionHost, TypeValueReference, isNamedClassDeclaration} from '../../reflection';
 
 export enum ConstructorDepErrorKind {
   NO_SUITABLE_TOKEN,
@@ -294,3 +294,28 @@ export function isExpressionForwardReference(
 export function isWrappedTsNodeExpr(expr: Expression): expr is WrappedNodeExpr<ts.Node> {
   return expr instanceof WrappedNodeExpr;
 }
+
+export function readBaseClass(
+    node: ClassDeclaration, reflector: ReflectionHost,
+    evaluator: PartialEvaluator): Reference<ClassDeclaration>|'dynamic'|null {
+  if (!isNamedClassDeclaration(node)) {
+    // If the node isn't a ts.ClassDeclaration, consider any base class to be dynamic for now.
+    return reflector.hasBaseClass(node) ? 'dynamic' : null;
+  }
+
+  if (node.heritageClauses !== undefined) {
+    for (const clause of node.heritageClauses) {
+      if (clause.token === ts.SyntaxKind.ExtendsKeyword) {
+        // The class has a base class. Figure out whether it's resolvable or not.
+        const baseClass = evaluator.evaluate(clause.types[0].expression);
+        if (baseClass instanceof Reference && isNamedClassDeclaration(baseClass.node)) {
+          return baseClass as Reference<ClassDeclaration>;
+        } else {
+          return 'dynamic';
+        }
+      }
+    }
+  }
+
+  return null;
+}

packages/compiler-cli/src/ngtsc/annotations/test/component_spec.ts

@@ -11,7 +11,7 @@ import * as ts from 'typescript';
 import {CycleAnalyzer, ImportGraph} from '../../cycles';
 import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
 import {ModuleResolver, NOOP_DEFAULT_IMPORT_RECORDER, ReferenceEmitter} from '../../imports';
-import {DtsMetadataReader, LocalMetadataRegistry} from '../../metadata';
+import {CompoundMetadataReader, DtsMetadataReader, LocalMetadataRegistry} from '../../metadata';
 import {PartialEvaluator} from '../../partial_evaluator';
 import {TypeScriptReflectionHost, isNamedClassDeclaration} from '../../reflection';
 import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../scope';
@@ -54,11 +54,13 @@ describe('ComponentDecoratorHandler', () => {
     const scopeRegistry = new LocalModuleScopeRegistry(
         metaRegistry, new MetadataDtsModuleScopeResolver(dtsReader, null), new ReferenceEmitter([]),
         null);
+    const metaReader = new CompoundMetadataReader([metaRegistry, dtsReader]);
     const refEmitter = new ReferenceEmitter([]);
 
     const handler = new ComponentDecoratorHandler(
-        reflectionHost, evaluator, metaRegistry, scopeRegistry, false, new NoopResourceLoader(),
-        [''], false, true, moduleResolver, cycleAnalyzer, refEmitter, NOOP_DEFAULT_IMPORT_RECORDER);
+        reflectionHost, evaluator, metaRegistry, metaReader, scopeRegistry, false,
+        new NoopResourceLoader(), [''], false, true, moduleResolver, cycleAnalyzer, refEmitter,
+        NOOP_DEFAULT_IMPORT_RECORDER);
     const TestCmp = getDeclaration(program, 'entry.ts', 'TestCmp', isNamedClassDeclaration);
     const detected = handler.detect(TestCmp, reflectionHost.getDecoratorsOfDeclaration(TestCmp));
     if (detected === undefined) {

packages/compiler-cli/src/ngtsc/metadata/index.ts

@@ -9,4 +9,4 @@
 export * from './src/api';
 export {DtsMetadataReader} from './src/dts';
 export {CompoundMetadataRegistry, LocalMetadataRegistry} from './src/registry';
-export {extractDirectiveGuards} from './src/util';
+export {extractDirectiveGuards, CompoundMetadataReader} from './src/util';

packages/compiler-cli/src/ngtsc/metadata/src/dts.ts

@@ -9,7 +9,7 @@
 import * as ts from 'typescript';
 
 import {Reference} from '../../imports';
-import {ClassDeclaration, ReflectionHost} from '../../reflection';
+import {ClassDeclaration, ReflectionHost, isNamedClassDeclaration} from '../../reflection';
 
 import {DirectiveMeta, MetadataReader, NgModuleMeta, PipeMeta} from './api';
 import {extractDirectiveGuards, extractReferencesFromType, readStringArrayType, readStringMapType, readStringType} from './util';
@@ -88,7 +88,7 @@ export class DtsMetadataReader implements MetadataReader {
       outputs: readStringMapType(def.type.typeArguments[4]),
       queries: readStringArrayType(def.type.typeArguments[5]),
       ...extractDirectiveGuards(clazz, this.reflector),
-      baseClass: null,
+      baseClass: readBaseClass(clazz, this.checker, this.reflector),
     };
   }
 
@@ -116,3 +116,33 @@ export class DtsMetadataReader implements MetadataReader {
     return {ref, name};
   }
 }
+
+function readBaseClass(clazz: ClassDeclaration, checker: ts.TypeChecker, reflector: ReflectionHost):
+    Reference<ClassDeclaration>|'dynamic'|null {
+  if (!isNamedClassDeclaration(clazz)) {
+    // Technically this is an error in a .d.ts file, but for the purposes of finding the base class
+    // it's ignored.
+    return reflector.hasBaseClass(clazz) ? 'dynamic' : null;
+  }
+
+  if (clazz.heritageClauses !== undefined) {
+    for (const clause of clazz.heritageClauses) {
+      if (clause.token === ts.SyntaxKind.ExtendsKeyword) {
+        const baseExpr = clause.types[0].expression;
+        let symbol = checker.getSymbolAtLocation(baseExpr);
+        if (symbol === undefined) {
+          return 'dynamic';
+        } else if (symbol.flags & ts.SymbolFlags.Alias) {
+          symbol = checker.getAliasedSymbol(symbol);
+        }
+        if (symbol.valueDeclaration !== undefined &&
+            isNamedClassDeclaration(symbol.valueDeclaration)) {
+          return new Reference(symbol.valueDeclaration);
+        } else {
+          return 'dynamic';
+        }
+      }
+    }
+  }
+  return null;
+}

packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts

@@ -0,0 +1,56 @@
+/**
+ * @license
+ * Copyright Google Inc. 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.io/license
+ */
+
+import {Reference} from '../../imports';
+import {DirectiveMeta, MetadataReader} from '../../metadata';
+import {ClassDeclaration} from '../../reflection';
+
+/**
+ * Given a reference to a directive, return a flattened version of its `DirectiveMeta` metadata
+ * which includes metadata from its entire inheritance chain.
+ *
+ * The returned `DirectiveMeta` will either have `baseClass: null` if the inheritance chain could be
+ * fully resolved, or `baseClass: 'dynamic'` if the inheritance chain could not be completely
+ * followed.
+ */
+export function flattenInheritedDirectiveMetadata(
+    reader: MetadataReader, dir: Reference<ClassDeclaration>): DirectiveMeta {
+  const topMeta = reader.getDirectiveMetadata(dir);
+  if (topMeta === null) {
+    throw new Error(`Metadata not found for directive: ${dir.debugName}`);
+  }
+
+  let inputs: {[key: string]: string | [string, string]} = {};
+  let outputs: {[key: string]: string} = {};
+  let isDynamic = false;
+
+  const addMetadata = (meta: DirectiveMeta): void => {
+    if (meta.baseClass === 'dynamic') {
+      isDynamic = true;
+    } else if (meta.baseClass !== null) {
+      const baseMeta = reader.getDirectiveMetadata(meta.baseClass);
+      if (baseMeta !== null) {
+        addMetadata(baseMeta);
+      } else {
+        // Missing metadata for the base class means it's effectively dynamic.
+        isDynamic = true;
+      }
+    }
+    inputs = {...inputs, ...meta.inputs};
+    outputs = {...outputs, ...meta.outputs};
+  };
+
+  addMetadata(topMeta);
+
+  return {
+    ...topMeta,
+    inputs,
+    outputs,
+    baseClass: isDynamic ? 'dynamic' : null,
+  };
+}

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

@@ -12,6 +12,8 @@ import {Reference} from '../../imports';
 import {ClassDeclaration, ClassMemberKind, ReflectionHost, isNamedClassDeclaration, reflectTypeEntityToDeclaration} from '../../reflection';
 import {nodeDebugInfo} from '../../util/src/typescript';
 
+import {DirectiveMeta, MetadataReader, NgModuleMeta, PipeMeta} from './api';
+
 export function extractReferencesFromType(
     checker: ts.TypeChecker, def: ts.TypeNode, ngModuleImportedFrom: string | null,
     resolutionContext: string): Reference<ClassDeclaration>[] {
@@ -93,3 +95,43 @@ function nodeStaticMethodNames(node: ClassDeclaration, reflector: ReflectionHost
       .filter(member => member.kind === ClassMemberKind.Method && member.isStatic)
       .map(member => member.name);
 }
+
+/**
+ * A `MetadataReader` that reads from an ordered set of child readers until it obtains the requested
+ * metadata.
+ *
+ * This is used to combine `MetadataReader`s that read from different sources (e.g. from a registry
+ * and from .d.ts files).
+ */
+export class CompoundMetadataReader implements MetadataReader {
+  constructor(private readers: MetadataReader[]) {}
+
+  getDirectiveMetadata(node: Reference<ClassDeclaration<ts.Declaration>>): DirectiveMeta|null {
+    for (const reader of this.readers) {
+      const meta = reader.getDirectiveMetadata(node);
+      if (meta !== null) {
+        return meta;
+      }
+    }
+    return null;
+  }
+
+  getNgModuleMetadata(node: Reference<ClassDeclaration<ts.Declaration>>): NgModuleMeta|null {
+    for (const reader of this.readers) {
+      const meta = reader.getNgModuleMetadata(node);
+      if (meta !== null) {
+        return meta;
+      }
+    }
+    return null;
+  }
+  getPipeMetadata(node: Reference<ClassDeclaration<ts.Declaration>>): PipeMeta|null {
+    for (const reader of this.readers) {
+      const meta = reader.getPipeMetadata(node);
+      if (meta !== null) {
+        return meta;
+      }
+    }
+    return null;
+  }
+}

packages/compiler-cli/src/ngtsc/program.ts

@@ -19,7 +19,7 @@ import {ErrorCode, ngErrorCode} from './diagnostics';
 import {FlatIndexGenerator, ReferenceGraph, checkForPrivateExports, findFlatIndexEntryPoint} from './entry_point';
 import {AbsoluteModuleStrategy, AliasGenerator, AliasStrategy, DefaultImportTracker, FileToModuleHost, FileToModuleStrategy, ImportRewriter, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NoopImportRewriter, R3SymbolsImportRewriter, Reference, ReferenceEmitter} from './imports';
 import {IncrementalState} from './incremental';
-import {CompoundMetadataRegistry, DtsMetadataReader, LocalMetadataRegistry} from './metadata';
+import {CompoundMetadataReader, CompoundMetadataRegistry, DtsMetadataReader, LocalMetadataRegistry, MetadataReader} from './metadata';
 import {PartialEvaluator} from './partial_evaluator';
 import {AbsoluteFsPath, LogicalFileSystem} from './path';
 import {NOOP_PERF_RECORDER, PerfRecorder, PerfTracker} from './perf';
@@ -56,6 +56,7 @@ export class NgtscProgram implements api.Program {
   private constructionDiagnostics: ts.Diagnostic[] = [];
   private moduleResolver: ModuleResolver;
   private cycleAnalyzer: CycleAnalyzer;
+  private metaReader: MetadataReader|null = null;
 
   private refEmitter: ReferenceEmitter|null = null;
   private fileToModuleHost: FileToModuleHost|null = null;
@@ -421,6 +422,8 @@ export class NgtscProgram implements api.Program {
         localMetaRegistry, depScopeReader, this.refEmitter, aliasGenerator);
     const metaRegistry = new CompoundMetadataRegistry([localMetaRegistry, scopeRegistry]);
 
+    this.metaReader = new CompoundMetadataReader([localMetaRegistry, dtsReader]);
+
 
     // If a flat module entrypoint was specified, then track references via a `ReferenceGraph`
     // in
@@ -441,8 +444,8 @@ export class NgtscProgram implements api.Program {
     const handlers = [
       new BaseDefDecoratorHandler(this.reflector, evaluator, this.isCore),
       new ComponentDecoratorHandler(
-          this.reflector, evaluator, metaRegistry, scopeRegistry, this.isCore, this.resourceManager,
-          this.rootDirs, this.options.preserveWhitespaces || false,
+          this.reflector, evaluator, metaRegistry, this.metaReader !, scopeRegistry, this.isCore,
+          this.resourceManager, this.rootDirs, this.options.preserveWhitespaces || false,
           this.options.i18nUseExternalIds !== false, this.moduleResolver, this.cycleAnalyzer,
           this.refEmitter, this.defaultImportTracker),
       new DirectiveDecoratorHandler(

packages/compiler-cli/src/ngtsc/typecheck/BUILD.bazel

@@ -9,6 +9,7 @@ ts_library(
         "//packages:types",
         "//packages/compiler",
         "//packages/compiler-cli/src/ngtsc/imports",
+        "//packages/compiler-cli/src/ngtsc/metadata",
         "//packages/compiler-cli/src/ngtsc/reflection",
         "//packages/compiler-cli/src/ngtsc/translator",
         "//packages/compiler-cli/src/ngtsc/util",

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

@@ -164,4 +164,46 @@ describe('ngtsc type checking', () => {
     expect(diags.length).toBe(1);
     expect(diags[0].messageText).toContain('does_not_exist');
   });
+
+  it('should properly type-check inherited directives', () => {
+    env.write('test.ts', `
+    import {Component, Directive, Input, NgModule} from '@angular/core';
+
+    @Directive({
+      selector: '[base]',
+    })
+    class BaseDir {
+      @Input() fromBase!: string;
+    }
+
+    @Directive({
+      selector: '[child]',
+    })
+    class ChildDir extends BaseDir {
+      @Input() fromChild!: boolean;
+    }
+
+    @Component({
+      selector: 'test',
+      template: '<div child [fromBase]="3" [fromChild]="4"></div>',
+    })
+    class TestCmp {}
+
+    @NgModule({
+      declarations: [TestCmp, ChildDir],
+    })
+    class Module {}
+    `);
+
+    const diags = env.driveDiagnostics();
+    expect(diags.length).toBe(2);
+
+    // Error from the binding to [fromBase].
+    expect(diags[0].messageText)
+        .toBe(`Type 'number' is not assignable to type 'string | undefined'.`);
+
+    // Error from the binding to [fromChild].
+    expect(diags[1].messageText)
+        .toBe(`Type 'number' is not assignable to type 'boolean | undefined'.`);
+  });
 });