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.
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
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
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
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
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
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
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
@@ -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
@@ -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
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
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
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
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.