Skip to content

Commit

Permalink
refactor(compiler-cli): identify structural directives (#40032)
Browse files Browse the repository at this point in the history
This commit introduces an `isStructural` flag on directive metadata, which
is `true` if the directive injects `TemplateRef` (and thus is at least
theoretically usable as a structural directive). The flag is not used for
anything currently, but will be utilized by the Language Service to offer
better autocompletion results for structural directives.

PR Close #40032
  • Loading branch information
alxhub committed Dec 14, 2020
1 parent cbb6eae commit c55bf4a
Show file tree
Hide file tree
Showing 18 changed files with 227 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ export class ComponentDecoratorHandler implements
baseClass: analysis.baseClass,
...analysis.typeCheckMeta,
isPoisoned: analysis.isPoisoned,
isStructural: false,
});

this.resourceRegistry.registerResources(analysis.resources, node);
Expand Down
18 changes: 17 additions & 1 deletion packages/compiler-cli/src/ngtsc/annotations/src/directive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {compileDeclareDirectiveFromMetadata, compileDirectiveFromMetadata, ConstantPool, Expression, Identifiers, makeBindingParser, ParsedHostBindings, ParseError, parseHostBindings, R3DependencyMetadata, R3DirectiveDef, R3DirectiveMetadata, R3FactoryTarget, R3QueryMetadata, Statement, verifyHostBindings, WrappedNodeExpr} from '@angular/compiler';
import {compileDeclareDirectiveFromMetadata, compileDirectiveFromMetadata, ConstantPool, Expression, ExternalExpr, Identifiers, makeBindingParser, ParsedHostBindings, ParseError, parseHostBindings, R3DependencyMetadata, R3DirectiveDef, R3DirectiveMetadata, R3FactoryTarget, R3QueryMetadata, R3ResolvedDependencyType, Statement, verifyHostBindings, WrappedNodeExpr} from '@angular/compiler';
import * as ts from 'typescript';

import {ErrorCode, FatalDiagnosticError} from '../../diagnostics';
Expand Down Expand Up @@ -42,6 +42,7 @@ export interface DirectiveHandlerData {
inputs: ClassPropertyMapping;
outputs: ClassPropertyMapping;
isPoisoned: boolean;
isStructural: boolean;
}

export class DirectiveDecoratorHandler implements
Expand Down Expand Up @@ -109,6 +110,7 @@ export class DirectiveDecoratorHandler implements
typeCheckMeta: extractDirectiveTypeCheckMeta(node, directiveResult.inputs, this.reflector),
providersRequiringFactory,
isPoisoned: false,
isStructural: directiveResult.isStructural,
}
};
}
Expand All @@ -129,6 +131,7 @@ export class DirectiveDecoratorHandler implements
baseClass: analysis.baseClass,
...analysis.typeCheckMeta,
isPoisoned: analysis.isPoisoned,
isStructural: analysis.isStructural,
});

this.injectableRegistry.registerInjectable(node);
Expand Down Expand Up @@ -226,6 +229,7 @@ export function extractDirectiveMetadata(
metadata: R3DirectiveMetadata,
inputs: ClassPropertyMapping,
outputs: ClassPropertyMapping,
isStructural: boolean;
}|undefined {
let directive: Map<string, ts.Expression>;
if (decorator === null || decorator.args === null || decorator.args.length === 0) {
Expand Down Expand Up @@ -352,6 +356,17 @@ export function extractDirectiveMetadata(
ctorDeps = unwrapConstructorDependencies(rawCtorDeps);
}

const isStructural = ctorDeps !== null && ctorDeps !== 'invalid' && ctorDeps.some(dep => {
if (dep.resolved !== R3ResolvedDependencyType.Token || !(dep.token instanceof ExternalExpr)) {
return false;
}
if (dep.token.value.moduleName !== '@angular/core' || dep.token.value.name !== 'TemplateRef') {
return false;
}

return true;
});

// Detect if the component inherits from another class
const usesInheritance = reflector.hasBaseClass(clazz);
const type = wrapTypeReference(reflector, clazz);
Expand Down Expand Up @@ -386,6 +401,7 @@ export function extractDirectiveMetadata(
metadata,
inputs,
outputs,
isStructural,
};
}

Expand Down
25 changes: 25 additions & 0 deletions packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ runInEachFileSystem(() => {
isComponent: false,
name: 'Dir',
selector: '[dir]',
isStructural: false,
};
matcher.addSelectables(CssSelector.parse('[dir]'), dirMeta);

Expand All @@ -118,6 +119,30 @@ runInEachFileSystem(() => {
// and field names.
expect(propBindingConsumer).toBe(dirMeta);
});

it('should identify a structural directive', () => {
const src = `
import {Directive, TemplateRef} from '@angular/core';
@Directive({selector: 'test-dir'})
export class TestDir {
constructor(private ref: TemplateRef) {}
}
`;
const {program} = makeProgram([
{
name: _('/node_modules/@angular/core/index.d.ts'),
contents: 'export const Directive: any; export declare class TemplateRef {}',
},
{
name: _('/entry.ts'),
contents: src,
},
]);

const analysis = analyzeDirective(program, 'TestDir');
expect(analysis.isStructural).toBeTrue();
});
});

// Helpers
Expand Down
1 change: 1 addition & 0 deletions packages/compiler-cli/src/ngtsc/indexer/test/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export function getBoundTemplate(
inputs: ClassPropertyMapping.fromMappedObject({}),
outputs: ClassPropertyMapping.fromMappedObject({}),
exportAs: null,
isStructural: false,
});
});
const binder = new R3TargetBinder(matcher);
Expand Down
5 changes: 5 additions & 0 deletions packages/compiler-cli/src/ngtsc/metadata/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@ export interface DirectiveMeta extends T2DirectiveMeta, DirectiveTypeCheckMeta {
* and reliable metadata.
*/
isPoisoned: boolean;

/**
* Whether the directive is likely a structural directive (injects `TemplateRef`).
*/
isStructural: boolean;
}

/**
Expand Down
18 changes: 16 additions & 2 deletions packages/compiler-cli/src/ngtsc/metadata/src/dts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import * as ts from 'typescript';

import {Reference} from '../../imports';
import {ClassDeclaration, isNamedClassDeclaration, ReflectionHost} from '../../reflection';
import {ClassDeclaration, isNamedClassDeclaration, ReflectionHost, TypeValueReferenceKind} from '../../reflection';

import {DirectiveMeta, MetadataReader, NgModuleMeta, PipeMeta} from './api';
import {ClassPropertyMapping} from './property_mapping';
Expand Down Expand Up @@ -77,14 +77,27 @@ export class DtsMetadataReader implements MetadataReader {
return null;
}

const isComponent = def.name === 'ɵcmp';

const ctorParams = this.reflector.getConstructorParameters(clazz);

// A directive is considered to be structural if:
// 1) it's a directive, not a component, and
// 2) it injects `TemplateRef`
const isStructural = !isComponent && ctorParams !== null && ctorParams.some(param => {
return param.typeValueReference.kind === TypeValueReferenceKind.IMPORTED &&
param.typeValueReference.moduleName === '@angular/core' &&
param.typeValueReference.importedName === 'TemplateRef';
});

const inputs =
ClassPropertyMapping.fromMappedObject(readStringMapType(def.type.typeArguments[3]));
const outputs =
ClassPropertyMapping.fromMappedObject(readStringMapType(def.type.typeArguments[4]));
return {
ref,
name: clazz.name.text,
isComponent: def.name === 'ɵcmp',
isComponent,
selector: readStringType(def.type.typeArguments[1]),
exportAs: readStringArrayType(def.type.typeArguments[2]),
inputs,
Expand All @@ -93,6 +106,7 @@ export class DtsMetadataReader implements MetadataReader {
...extractDirectiveTypeCheckMeta(clazz, inputs, this.reflector),
baseClass: readBaseClass(clazz, this.checker, this.reflector),
isPoisoned: false,
isStructural,
};
}

Expand Down
4 changes: 4 additions & 0 deletions packages/compiler-cli/src/ngtsc/metadata/src/inheritance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export function flattenInheritedDirectiveMetadata(
let isDynamic = false;
let inputs = ClassPropertyMapping.empty();
let outputs = ClassPropertyMapping.empty();
let isStructural: boolean = false;

const addMetadata = (meta: DirectiveMeta): void => {
if (meta.baseClass === 'dynamic') {
Expand All @@ -51,6 +52,8 @@ export function flattenInheritedDirectiveMetadata(
}
}

isStructural = isStructural || meta.isStructural;

inputs = ClassPropertyMapping.merge(inputs, meta.inputs);
outputs = ClassPropertyMapping.merge(outputs, meta.outputs);

Expand Down Expand Up @@ -79,5 +82,6 @@ export function flattenInheritedDirectiveMetadata(
restrictedInputFields,
stringLiteralInputFields,
baseClass: isDynamic ? 'dynamic' : null,
isStructural,
};
}
35 changes: 35 additions & 0 deletions packages/compiler-cli/src/ngtsc/metadata/test/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
load("//tools:defaults.bzl", "jasmine_node_test", "ts_library")

package(default_visibility = ["//visibility:public"])

ts_library(
name = "test_lib",
testonly = True,
srcs = glob([
"**/*.ts",
]),
deps =
[
"//packages:types",
"//packages/compiler",
"//packages/compiler-cli/src/ngtsc/file_system",
"//packages/compiler-cli/src/ngtsc/file_system/testing",
"//packages/compiler-cli/src/ngtsc/imports",
"//packages/compiler-cli/src/ngtsc/metadata",
"//packages/compiler-cli/src/ngtsc/reflection",
"//packages/compiler-cli/src/ngtsc/testing",
"@npm//typescript",
],
)

jasmine_node_test(
name = "test",
bootstrap = ["//tools/testing:node_no_angular_es5"],
data = [
"//packages/compiler-cli/src/ngtsc/testing/fake_core:npm_package",
],
deps =
[
":test_lib",
],
)
93 changes: 93 additions & 0 deletions packages/compiler-cli/src/ngtsc/metadata/test/dts_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/**
* @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.io/license
*/
import {ExternalExpr} from '@angular/compiler';
import * as ts from 'typescript';

import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../file_system';
import {runInEachFileSystem} from '../../file_system/testing';
import {Reference} from '../../imports';
import {isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection';
import {loadFakeCore, makeProgram} from '../../testing';

import {DtsMetadataReader} from '../src/dts';

runInEachFileSystem(() => {
beforeEach(() => {
loadFakeCore(getFileSystem());
});

describe('DtsMetadataReader', () => {
it('should not assume directives are structural', () => {
const mainPath = absoluteFrom('/main.d.ts');
const {program} = makeProgram(
[{
name: mainPath,
contents: `
import {ViewContainerRef} from '@angular/core';
import * as i0 from '@angular/core';
export declare class TestDir {
constructor(p0: ViewContainerRef);
static ɵdir: i0.ɵɵDirectiveDefWithMeta<TestDir, "[test]", never, {}, {}, never>
}
`
}],
{
skipLibCheck: true,
lib: ['es6', 'dom'],
});

const sf = getSourceFileOrError(program, mainPath);
const clazz = sf.statements[2];
if (!isNamedClassDeclaration(clazz)) {
return fail('Expected class declaration');
}

const typeChecker = program.getTypeChecker();
const dtsReader =
new DtsMetadataReader(typeChecker, new TypeScriptReflectionHost(typeChecker));

const meta = dtsReader.getDirectiveMetadata(new Reference(clazz))!;
expect(meta.isStructural).toBeFalse();
});

it('should identify a structural directive by its constructor', () => {
const mainPath = absoluteFrom('/main.d.ts');
const {program} = makeProgram(
[{
name: mainPath,
contents: `
import {TemplateRef, ViewContainerRef} from '@angular/core';
import * as i0 from '@angular/core';
export declare class TestDir {
constructor(p0: ViewContainerRef, p1: TemplateRef);
static ɵdir: i0.ɵɵDirectiveDefWithMeta<TestDir, "[test]", never, {}, {}, never>
}
`
}],
{
skipLibCheck: true,
lib: ['es6', 'dom'],
});

const sf = getSourceFileOrError(program, mainPath);
const clazz = sf.statements[2];
if (!isNamedClassDeclaration(clazz)) {
return fail('Expected class declaration');
}

const typeChecker = program.getTypeChecker();
const dtsReader =
new DtsMetadataReader(typeChecker, new TypeScriptReflectionHost(typeChecker));

const meta = dtsReader.getDirectiveMetadata(new Reference(clazz))!;
expect(meta.isStructural).toBeTrue();
});
});
});
10 changes: 6 additions & 4 deletions packages/compiler-cli/src/ngtsc/reflection/src/typescript.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,14 @@ export class TypeScriptReflectionHost implements ReflectionHost {
getConstructorParameters(clazz: ClassDeclaration): CtorParameter[]|null {
const tsClazz = castDeclarationToClassOrDie(clazz);

// First, find the constructor with a `body`. The constructors without a `body` are overloads
// whereas we want the implementation since it's the one that'll be executed and which can
// have decorators.
const isDeclaration = tsClazz.getSourceFile().isDeclarationFile;
// For non-declaration files, we want to find the constructor with a `body`. The constructors
// without a `body` are overloads whereas we want the implementation since it's the one that'll
// be executed and which can have decorators. For declaration files, we take the first one that
// we get.
const ctor = tsClazz.members.find(
(member): member is ts.ConstructorDeclaration =>
ts.isConstructorDeclaration(member) && member.body !== undefined);
ts.isConstructorDeclaration(member) && (isDeclaration || member.body !== undefined));
if (ctor === undefined) {
return null;
}
Expand Down
1 change: 1 addition & 0 deletions packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ function fakeDirective(ref: Reference<ClassDeclaration>): DirectiveMeta {
isGeneric: false,
baseClass: null,
isPoisoned: false,
isStructural: false,
};
}

Expand Down
3 changes: 2 additions & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import * as ts from 'typescript';
import {FullTemplateMapping, TypeCheckableDirectiveMeta} from './api';
import {GlobalCompletion} from './completion';
import {DirectiveInScope, PipeInScope} from './scope';
import {DirectiveSymbol, ElementSymbol, ShimLocation, Symbol} from './symbols';
import {DirectiveSymbol, ElementSymbol, ShimLocation, Symbol, TemplateSymbol} from './symbols';

/**
* Interface to the Angular Template Type Checker to extract diagnostics and intelligence from the
Expand Down Expand Up @@ -111,6 +111,7 @@ export interface TemplateTypeChecker {
* @see Symbol
*/
getSymbolOfNode(node: TmplAstElement, component: ts.ClassDeclaration): ElementSymbol|null;
getSymbolOfNode(node: TmplAstTemplate, component: ts.ClassDeclaration): TemplateSymbol|null;
getSymbolOfNode(node: AST|TmplAstNode, component: ts.ClassDeclaration): Symbol|null;

/**
Expand Down
5 changes: 5 additions & 0 deletions packages/compiler-cli/src/ngtsc/typecheck/api/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ export interface DirectiveInScope {
* `true` if this directive is a component.
*/
isComponent: boolean;

/**
* `true` if this directive is a structural directive.
*/
isStructural: boolean;
}

/**
Expand Down

0 comments on commit c55bf4a

Please sign in to comment.