diff --git a/packages/compiler-cli/src/ngtsc/indexer/src/api.ts b/packages/compiler-cli/src/ngtsc/indexer/src/api.ts index e0e3ba10f3..2841620fab 100644 --- a/packages/compiler-cli/src/ngtsc/indexer/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/indexer/src/api.ts @@ -15,13 +15,8 @@ import * as ts from 'typescript'; export enum IdentifierKind { Property, Method, -} - -/** - * Describes the absolute byte offsets of a text anchor in a source code. - */ -export class AbsoluteSourceSpan { - constructor(public start: number, public end: number) {} + Element, + Attribute, } /** @@ -34,6 +29,43 @@ export interface TemplateIdentifier { kind: IdentifierKind; } +/** Describes a property accessed in a template. */ +export interface PropertyIdentifier extends TemplateIdentifier { kind: IdentifierKind.Property; } + +/** Describes a method accessed in a template. */ +export interface MethodIdentifier extends TemplateIdentifier { kind: IdentifierKind.Method; } + +/** Describes an element attribute in a template. */ +export interface AttributeIdentifier extends TemplateIdentifier { kind: IdentifierKind.Attribute; } + +/** + * Describes an indexed element in a template. The name of an `ElementIdentifier` is the entire + * element tag, which can be parsed by an indexer to determine where used directives should be + * referenced. + */ +export interface ElementIdentifier extends TemplateIdentifier { + kind: IdentifierKind.Element; + + /** Attributes on an element. */ + attributes: Set; + + /** Directives applied to an element. */ + usedDirectives: Set; +} + +/** + * Identifiers recorded at the top level of the template, without any context about the HTML nodes + * they were discovered in. + */ +export type TopLevelIdentifier = PropertyIdentifier | MethodIdentifier | ElementIdentifier; + +/** + * Describes the absolute byte offsets of a text anchor in a source code. + */ +export class AbsoluteSourceSpan { + constructor(public start: number, public end: number) {} +} + /** * Describes an analyzed, indexed component and its template. */ @@ -42,7 +74,7 @@ export interface IndexedComponent { selector: string|null; file: ParseSourceFile; template: { - identifiers: Set, + identifiers: Set, usedComponents: Set, isInline: boolean, file: ParseSourceFile; diff --git a/packages/compiler-cli/src/ngtsc/indexer/src/template.ts b/packages/compiler-cli/src/ngtsc/indexer/src/template.ts index 5aa38ee2ba..0948429a78 100644 --- a/packages/compiler-cli/src/ngtsc/indexer/src/template.ts +++ b/packages/compiler-cli/src/ngtsc/indexer/src/template.ts @@ -6,9 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, BoundTarget, DirectiveMeta, ImplicitReceiver, MethodCall, PropertyRead, RecursiveAstVisitor} from '@angular/compiler'; +import {AST, BoundTarget, ImplicitReceiver, MethodCall, PropertyRead, RecursiveAstVisitor} from '@angular/compiler'; import {BoundText, Element, Node, RecursiveVisitor as RecursiveTemplateVisitor, Template} from '@angular/compiler/src/render3/r3_ast'; -import {AbsoluteSourceSpan, IdentifierKind, TemplateIdentifier} from './api'; +import {AbsoluteSourceSpan, AttributeIdentifier, ElementIdentifier, IdentifierKind, MethodIdentifier, PropertyIdentifier, TemplateIdentifier, TopLevelIdentifier} from './api'; +import {ComponentMeta} from './context'; /** * A parsed node in a template, which may have a name (if it is a selector) or @@ -19,20 +20,22 @@ interface HTMLNode extends Node { name?: string; } +type ExpressionIdentifier = PropertyIdentifier | MethodIdentifier; + /** * Visits the AST of an Angular template syntax expression, finding interesting * entities (variable references, etc.). Creates an array of Entities found in * the expression, with the location of the Entities being relative to the * expression. * - * Visiting `text {{prop}}` will return `[TemplateIdentifier {name: 'prop', span: {start: 7, end: - * 11}}]`. + * Visiting `text {{prop}}` will return + * `[TopLevelIdentifier {name: 'prop', span: {start: 7, end: 11}}]`. */ class ExpressionVisitor extends RecursiveAstVisitor { - readonly identifiers: TemplateIdentifier[] = []; + readonly identifiers: ExpressionIdentifier[] = []; private constructor( - context: Node, private readonly boundTemplate: BoundTarget, + context: Node, private readonly boundTemplate: BoundTarget, private readonly expressionStr = context.sourceSpan.toString(), private readonly absoluteOffset = context.sourceSpan.start.offset) { super(); @@ -46,8 +49,8 @@ class ExpressionVisitor extends RecursiveAstVisitor { * @param boundTemplate bound target of the entire template, which can be used to query for the * entities expressions target. */ - static getIdentifiers(ast: AST, context: Node, boundTemplate: BoundTarget): - TemplateIdentifier[] { + static getIdentifiers(ast: AST, context: Node, boundTemplate: BoundTarget): + TopLevelIdentifier[] { const visitor = new ExpressionVisitor(context, boundTemplate); visitor.visit(ast); return visitor.identifiers; @@ -71,7 +74,8 @@ class ExpressionVisitor extends RecursiveAstVisitor { * @param ast expression AST the identifier is in * @param kind identifier kind */ - private visitIdentifier(ast: AST&{name: string, receiver: AST}, kind: IdentifierKind) { + private visitIdentifier( + ast: AST&{name: string, receiver: AST}, kind: ExpressionIdentifier['kind']) { // The definition of a non-top-level property such as `bar` in `{{foo.bar}}` is currently // impossible to determine by an indexer and unsupported by the indexing module. // The indexing module also does not currently support references to identifiers declared in the @@ -86,6 +90,9 @@ class ExpressionVisitor extends RecursiveAstVisitor { // useful to the indexer. For example, a `MethodCall` `foo(a, b)` will record the span of the // entire method call, but the indexer is interested only in the method identifier. const localExpression = this.expressionStr.substr(ast.span.start, ast.span.end); + if (!localExpression.includes(ast.name)) { + throw new Error(`Impossible state: "${ast.name}" not found in "${localExpression}"`); + } const identifierStart = ast.span.start + localExpression.indexOf(ast.name); // Join the relative position of the expression within a node with the absolute position @@ -93,11 +100,7 @@ class ExpressionVisitor extends RecursiveAstVisitor { const absoluteStart = this.absoluteOffset + identifierStart; const span = new AbsoluteSourceSpan(absoluteStart, absoluteStart + ast.name.length); - this.identifiers.push({ - name: ast.name, - span, - kind, - }); + this.identifiers.push({ name: ast.name, span, kind, } as ExpressionIdentifier); } } @@ -107,7 +110,7 @@ class ExpressionVisitor extends RecursiveAstVisitor { */ class TemplateVisitor extends RecursiveTemplateVisitor { // identifiers of interest found in the template - readonly identifiers = new Set(); + readonly identifiers = new Set(); /** * Creates a template visitor for a bound template target. The bound target can be used when @@ -115,7 +118,7 @@ class TemplateVisitor extends RecursiveTemplateVisitor { * * @param boundTemplate bound template target */ - constructor(private boundTemplate: BoundTarget) { super(); } + constructor(private boundTemplate: BoundTarget) { super(); } /** * Visits a node in the template. @@ -126,8 +129,40 @@ class TemplateVisitor extends RecursiveTemplateVisitor { visitAll(nodes: Node[]) { nodes.forEach(node => this.visit(node)); } + /** + * Add an identifier for an HTML element and visit its children recursively. + * + * @param element + */ visitElement(element: Element) { - this.visitAll(element.attributes); + // Record the element's attributes, which an indexer can later traverse to see if any of them + // specify a used directive on the element. + const attributes = element.attributes.map(({name, value, sourceSpan}): AttributeIdentifier => { + return { + name, + span: new AbsoluteSourceSpan(sourceSpan.start.offset, sourceSpan.end.offset), + kind: IdentifierKind.Attribute, + }; + }); + const usedDirectives = this.boundTemplate.getDirectivesOfNode(element) || []; + const {name, sourceSpan} = element; + // An element's source span can be of the form ``, ``, or + // ``. Only the selector is interesting to the indexer, so the source is + // searched for the first occurrence of the element (selector) name. + const localStr = sourceSpan.toString(); + if (!localStr.includes(name)) { + throw new Error(`Impossible state: "${name}" not found in "${localStr}"`); + } + const start = sourceSpan.start.offset + localStr.indexOf(name); + const elId: ElementIdentifier = { + name, + span: new AbsoluteSourceSpan(start, start + name.length), + kind: IdentifierKind.Element, + attributes: new Set(attributes), + usedDirectives: new Set(usedDirectives.map(dir => dir.ref.node)), + }; + this.identifiers.add(elId); + this.visitAll(element.children); this.visitAll(element.references); } @@ -142,7 +177,7 @@ class TemplateVisitor extends RecursiveTemplateVisitor { /** * Visits a node's expression and adds its identifiers, if any, to the visitor's state. * - * @param curretNode node whose expression to visit + * @param node node whose expression to visit */ private visitExpression(node: Node&{value: AST}) { const identifiers = ExpressionVisitor.getIdentifiers(node.value, node, this.boundTemplate); @@ -156,8 +191,8 @@ class TemplateVisitor extends RecursiveTemplateVisitor { * @param boundTemplate bound template target, which can be used for querying expression targets. * @return identifiers in template */ -export function getTemplateIdentifiers(boundTemplate: BoundTarget): - Set { +export function getTemplateIdentifiers(boundTemplate: BoundTarget): + Set { const visitor = new TemplateVisitor(boundTemplate); if (boundTemplate.target.template !== undefined) { visitor.visitAll(boundTemplate.target.template); diff --git a/packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts b/packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts index 6e016116cd..6da72750c6 100644 --- a/packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts +++ b/packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AbsoluteSourceSpan, IdentifierKind} from '..'; +import {AbsoluteSourceSpan, AttributeIdentifier, ElementIdentifier, IdentifierKind} from '..'; import {runInEachFileSystem} from '../../file_system/testing'; import {getTemplateIdentifiers} from '../src/template'; import * as util from './util'; @@ -20,8 +20,8 @@ function bind(template: string) { runInEachFileSystem(() => { describe('getTemplateIdentifiers', () => { - it('should generate nothing in HTML-only template', () => { - const refs = getTemplateIdentifiers(bind('
')); + it('should generate nothing in empty template', () => { + const refs = getTemplateIdentifiers(bind('')); expect(refs.size).toBe(0); }); @@ -33,14 +33,14 @@ runInEachFileSystem(() => { }); it('should handle arbitrary whitespace', () => { - const template = '
\n\n {{foo}}
'; + const template = '\n\n {{foo}}'; const refs = getTemplateIdentifiers(bind(template)); const [ref] = Array.from(refs); expect(ref).toEqual({ name: 'foo', kind: IdentifierKind.Property, - span: new AbsoluteSourceSpan(12, 15), + span: new AbsoluteSourceSpan(7, 10), }); }); @@ -75,11 +75,11 @@ runInEachFileSystem(() => { const refs = getTemplateIdentifiers(bind(template)); const refArr = Array.from(refs); - expect(refArr).toEqual(jasmine.arrayContaining([{ + expect(refArr).toContain({ name: 'foo', kind: IdentifierKind.Property, span: new AbsoluteSourceSpan(13, 16), - }])); + }); }); it('should ignore identifiers that are not implicitly received by the template', () => { @@ -111,11 +111,11 @@ runInEachFileSystem(() => { const refs = getTemplateIdentifiers(bind(template)); const refArr = Array.from(refs); - expect(refArr).toEqual(jasmine.arrayContaining([{ + expect(refArr).toContain({ name: 'foo', kind: IdentifierKind.Method, span: new AbsoluteSourceSpan(13, 16), - }])); + }); }); it('should ignore identifiers that are not implicitly received by the template', () => { @@ -127,5 +127,127 @@ runInEachFileSystem(() => { expect(ref.name).toBe('foo'); }); }); + + describe('generates identifiers for elements', () => { + it('should record elements as ElementIdentifiers', () => { + const template = ''; + const refs = getTemplateIdentifiers(bind(template)); + expect(refs.size).toBe(1); + + const [ref] = Array.from(refs); + expect(ref.kind).toBe(IdentifierKind.Element); + }); + + it('should record element names as their selector', () => { + const template = ''; + const refs = getTemplateIdentifiers(bind(template)); + expect(refs.size).toBe(1); + + const [ref] = Array.from(refs); + expect(ref as ElementIdentifier).toEqual({ + name: 'test-selector', + kind: IdentifierKind.Element, + span: new AbsoluteSourceSpan(1, 14), + attributes: new Set(), + usedDirectives: new Set(), + }); + }); + + it('should discover selectors in self-closing elements', () => { + const template = ''; + const refs = getTemplateIdentifiers(bind(template)); + expect(refs.size).toBe(1); + + const [ref] = Array.from(refs); + expect(ref as ElementIdentifier).toEqual({ + name: 'img', + kind: IdentifierKind.Element, + span: new AbsoluteSourceSpan(1, 4), + attributes: new Set(), + usedDirectives: new Set(), + }); + }); + + it('should discover selectors in elements with adjacent open and close tags', () => { + const template = ''; + const refs = getTemplateIdentifiers(bind(template)); + expect(refs.size).toBe(1); + + const [ref] = Array.from(refs); + expect(ref as ElementIdentifier).toEqual({ + name: 'test-selector', + kind: IdentifierKind.Element, + span: new AbsoluteSourceSpan(1, 14), + attributes: new Set(), + usedDirectives: new Set(), + }); + }); + + it('should discover selectors in elements with non-adjacent open and close tags', () => { + const template = ' text '; + const refs = getTemplateIdentifiers(bind(template)); + expect(refs.size).toBe(1); + + const [ref] = Array.from(refs); + expect(ref as ElementIdentifier).toEqual({ + name: 'test-selector', + kind: IdentifierKind.Element, + span: new AbsoluteSourceSpan(1, 14), + attributes: new Set(), + usedDirectives: new Set(), + }); + }); + + it('should discover nested selectors', () => { + const template = '
'; + const refs = getTemplateIdentifiers(bind(template)); + + const refArr = Array.from(refs); + expect(refArr).toContain({ + name: 'span', + kind: IdentifierKind.Element, + span: new AbsoluteSourceSpan(6, 10), + attributes: new Set(), + usedDirectives: new Set(), + }); + }); + + it('should generate information about attributes', () => { + const template = '
'; + const refs = getTemplateIdentifiers(bind(template)); + + const [ref] = Array.from(refs); + const attrs = (ref as ElementIdentifier).attributes; + expect(attrs).toEqual(new Set([ + { + name: 'attrA', + kind: IdentifierKind.Attribute, + span: new AbsoluteSourceSpan(5, 10), + }, + { + name: 'attrB', + kind: IdentifierKind.Attribute, + span: new AbsoluteSourceSpan(11, 22), + } + ])); + }); + + it('should generate information about used directives', () => { + const declA = util.getComponentDeclaration('class A {}', 'A'); + const declB = util.getComponentDeclaration('class B {}', 'B'); + const declC = util.getComponentDeclaration('class C {}', 'C'); + const template = ''; + const boundTemplate = util.getBoundTemplate(template, {}, [ + {selector: 'a-selector', declaration: declA}, + {selector: '[b-selector]', declaration: declB}, + {selector: ':not(never-selector)', declaration: declC}, + ]); + + const refs = getTemplateIdentifiers(boundTemplate); + const [ref] = Array.from(refs); + const usedDirectives = (ref as ElementIdentifier).usedDirectives; + expect(usedDirectives).toEqual(new Set([declA, declB, declC])); + }); + }); }); }); diff --git a/packages/compiler-cli/src/ngtsc/indexer/test/transform_spec.ts b/packages/compiler-cli/src/ngtsc/indexer/test/transform_spec.ts index 1f2d270dec..e77763f7c7 100644 --- a/packages/compiler-cli/src/ngtsc/indexer/test/transform_spec.ts +++ b/packages/compiler-cli/src/ngtsc/indexer/test/transform_spec.ts @@ -7,9 +7,8 @@ */ import {BoundTarget, ParseSourceFile} from '@angular/compiler'; import {runInEachFileSystem} from '../../file_system/testing'; -import {DirectiveMeta} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; -import {IndexingContext} from '../src/context'; +import {ComponentMeta, IndexingContext} from '../src/context'; import {getTemplateIdentifiers} from '../src/template'; import {generateAnalysis} from '../src/transform'; import * as util from './util'; @@ -19,7 +18,7 @@ import * as util from './util'; */ function populateContext( context: IndexingContext, component: ClassDeclaration, selector: string, template: string, - boundTemplate: BoundTarget, isInline: boolean = false) { + boundTemplate: BoundTarget, isInline: boolean = false) { context.addComponent({ declaration: component, selector, diff --git a/packages/compiler-cli/src/ngtsc/indexer/test/util.ts b/packages/compiler-cli/src/ngtsc/indexer/test/util.ts index 4d9d11d32a..55bf78fb19 100644 --- a/packages/compiler-cli/src/ngtsc/indexer/test/util.ts +++ b/packages/compiler-cli/src/ngtsc/indexer/test/util.ts @@ -10,9 +10,9 @@ import {BoundTarget, CssSelector, ParseTemplateOptions, R3TargetBinder, Selector import * as ts from 'typescript'; import {AbsoluteFsPath, absoluteFrom} from '../../file_system'; import {Reference} from '../../imports'; -import {DirectiveMeta} from '../../metadata'; import {ClassDeclaration} from '../../reflection'; import {getDeclaration, makeProgram} from '../../testing'; +import {ComponentMeta} from '../src/context'; /** Dummy file URL */ export function getTestFilePath(): AbsoluteFsPath { @@ -41,16 +41,12 @@ export function getComponentDeclaration(componentStr: string, className: string) export function getBoundTemplate( template: string, options: ParseTemplateOptions = {}, components: Array<{selector: string, declaration: ClassDeclaration}> = - []): BoundTarget { - const matcher = new SelectorMatcher(); + []): BoundTarget { + const matcher = new SelectorMatcher(); components.forEach(({selector, declaration}) => { matcher.addSelectables(CssSelector.parse(selector), { ref: new Reference(declaration), selector, - queries: [], - ngTemplateGuards: [], - hasNgTemplateContextGuard: false, - baseClass: null, name: declaration.name.getText(), isComponent: true, inputs: {}, diff --git a/packages/compiler-cli/test/ngtsc/component_indexing_spec.ts b/packages/compiler-cli/test/ngtsc/component_indexing_spec.ts index 554533177f..d7abaad621 100644 --- a/packages/compiler-cli/test/ngtsc/component_indexing_spec.ts +++ b/packages/compiler-cli/test/ngtsc/component_indexing_spec.ts @@ -7,7 +7,7 @@ */ import {AbsoluteFsPath, resolve} from '@angular/compiler-cli/src/ngtsc/file_system'; import {runInEachFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; -import {AbsoluteSourceSpan, IdentifierKind} from '@angular/compiler-cli/src/ngtsc/indexer'; +import {AbsoluteSourceSpan, IdentifierKind, TopLevelIdentifier} from '@angular/compiler-cli/src/ngtsc/indexer'; import {ParseSourceFile} from '@angular/compiler/src/compiler'; import {NgtscTestEnvironment} from './env'; @@ -66,7 +66,7 @@ runInEachFileSystem(() => { const template = indexedComp.template; expect(template).toEqual({ - identifiers: new Set([{ + identifiers: new Set([{ name: 'foo', kind: IdentifierKind.Property, span: new AbsoluteSourceSpan(127, 130), @@ -93,7 +93,7 @@ runInEachFileSystem(() => { const template = indexedComp.template; expect(template).toEqual({ - identifiers: new Set([{ + identifiers: new Set([{ name: 'foo', kind: IdentifierKind.Property, span: new AbsoluteSourceSpan(2, 5), @@ -118,20 +118,20 @@ runInEachFileSystem(() => { }) export class TestCmp { foo = 0; } `); - env.write(testTemplateFile, '
\n {{foo}}
'); + env.write(testTemplateFile, ' \n {{foo}}'); const indexed = env.driveIndexer(); const [[_, indexedComp]] = Array.from(indexed.entries()); const template = indexedComp.template; expect(template).toEqual({ - identifiers: new Set([{ + identifiers: new Set([{ name: 'foo', kind: IdentifierKind.Property, - span: new AbsoluteSourceSpan(12, 15), + span: new AbsoluteSourceSpan(7, 10), }]), usedComponents: new Set(), isInline: false, - file: new ParseSourceFile('
\n {{foo}}
', testTemplateFile), + file: new ParseSourceFile(' \n {{foo}}', testTemplateFile), }); });