refactor(compiler-cli): Return symbols for all matching inputs (#40144)

This commit ensures that the template type checker returns symbols for
all inputs if an attribute binds to more than one.

PR Close #40144
This commit is contained in:
Andrew Scott 2020-12-15 15:55:54 -08:00 committed by atscott
parent 13020f904f
commit da2be4b710
5 changed files with 108 additions and 78 deletions

View File

@ -13,7 +13,7 @@ import {AbsoluteFsPath} from '../../file_system';
import {ClassDeclaration} from '../../reflection'; import {ClassDeclaration} from '../../reflection';
import {ComponentScopeReader} from '../../scope'; import {ComponentScopeReader} from '../../scope';
import {isAssignment} from '../../util/src/typescript'; import {isAssignment} from '../../util/src/typescript';
import {DirectiveSymbol, DomBindingSymbol, ElementSymbol, ExpressionSymbol, InputBindingSymbol, OutputBindingSymbol, PipeSymbol, ReferenceSymbol, ShimLocation, Symbol, SymbolKind, TemplateSymbol, TsNodeSymbolInfo, TypeCheckableDirectiveMeta, VariableSymbol} from '../api'; import {BindingSymbol, DirectiveSymbol, DomBindingSymbol, ElementSymbol, ExpressionSymbol, InputBindingSymbol, OutputBindingSymbol, PipeSymbol, ReferenceSymbol, ShimLocation, Symbol, SymbolKind, TemplateSymbol, TsNodeSymbolInfo, TypeCheckableDirectiveMeta, VariableSymbol} from '../api';
import {ExpressionIdentifier, findAllMatchingNodes, findFirstMatchingNode, hasExpressionIdentifier} from './comments'; import {ExpressionIdentifier, findAllMatchingNodes, findFirstMatchingNode, hasExpressionIdentifier} from './comments';
import {TemplateData} from './context'; import {TemplateData} from './context';
@ -253,31 +253,35 @@ export class SymbolBuilder {
return host !== null ? {kind: SymbolKind.DomBinding, host} : null; return host !== null ? {kind: SymbolKind.DomBinding, host} : null;
} }
const node = findFirstMatchingNode( const nodes = findAllMatchingNodes(
this.typeCheckBlock, {withSpan: binding.sourceSpan, filter: isAssignment}); this.typeCheckBlock, {withSpan: binding.sourceSpan, filter: isAssignment});
if (node === null || !isAccessExpression(node.left)) { const bindings: BindingSymbol[] = [];
return null; for (const node of nodes) {
if (!isAccessExpression(node.left)) {
continue;
} }
const symbolInfo = this.getSymbolOfTsNode(node.left); const symbolInfo = this.getSymbolOfTsNode(node.left);
if (symbolInfo === null || symbolInfo.tsSymbol === null) { if (symbolInfo === null || symbolInfo.tsSymbol === null) {
return null; continue;
} }
const target = this.getDirectiveSymbolForAccessExpression(node.left, consumer); const target = this.getDirectiveSymbolForAccessExpression(node.left, consumer);
if (target === null) { if (target === null) {
return null; continue;
} }
bindings.push({
return {
kind: SymbolKind.Input,
bindings: [{
...symbolInfo, ...symbolInfo,
tsSymbol: symbolInfo.tsSymbol, tsSymbol: symbolInfo.tsSymbol,
kind: SymbolKind.Binding, kind: SymbolKind.Binding,
target, target,
}], });
}; }
if (bindings.length === 0) {
return null;
}
return {kind: SymbolKind.Input, bindings};
} }
private getDirectiveSymbolForAccessExpression( private getDirectiveSymbolForAccessExpression(

View File

@ -1136,7 +1136,7 @@ runInEachFileSystem(() => {
.toEqual('TestDir'); .toEqual('TestDir');
}); });
it('returns the first directive match when two directives have the same input', () => { it('returns the all inputs when two directives have the same input', () => {
const fileName = absoluteFrom('/main.ts'); const fileName = absoluteFrom('/main.ts');
const dirFile = absoluteFrom('/dir.ts'); const dirFile = absoluteFrom('/dir.ts');
const templateString = `<div dir otherDir [inputA]="'my input A'"></div>`; const templateString = `<div dir otherDir [inputA]="'my input A'"></div>`;
@ -1178,12 +1178,12 @@ runInEachFileSystem(() => {
const inputAbinding = (nodes[0] as TmplAstElement).inputs[0]; const inputAbinding = (nodes[0] as TmplAstElement).inputs[0];
const symbol = templateTypeChecker.getSymbolOfNode(inputAbinding, cmp)!; const symbol = templateTypeChecker.getSymbolOfNode(inputAbinding, cmp)!;
assertInputBindingSymbol(symbol); assertInputBindingSymbol(symbol);
expect( expect(new Set(symbol.bindings.map(
(symbol.bindings[0].tsSymbol!.declarations[0] as ts.PropertyDeclaration).name.getText()) b => (b.tsSymbol!.declarations[0] as ts.PropertyDeclaration).name.getText())))
.toEqual('inputA'); .toEqual(new Set(['inputA', 'otherDirInputA']));
expect((symbol.bindings[0].tsSymbol!.declarations[0] as ts.PropertyDeclaration) expect(new Set(symbol.bindings.map(
.parent.name?.text) b => (b.tsSymbol!.declarations[0] as ts.PropertyDeclaration).parent.name?.text)))
.toEqual('TestDir'); .toEqual(new Set(['TestDir', 'OtherDir']));
}); });
}); });

View File

@ -6,19 +6,18 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {absoluteFrom} from '@angular/compiler-cli/src/ngtsc/file_system'; import {absoluteFrom, AbsoluteFsPath} from '@angular/compiler-cli/src/ngtsc/file_system';
import {initMockFileSystem, TestFile} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; import {initMockFileSystem} from '@angular/compiler-cli/src/ngtsc/file_system/testing';
import {LanguageServiceTestEnvironment} from './env'; import {extractCursorInfo, LanguageServiceTestEnvironment} from './env';
import {humanizeDefinitionInfo} from './test_utils'; import {assertFileNames, createModuleWithDeclarations, humanizeDefinitionInfo} from './test_utils';
describe('definitions', () => { describe('definitions', () => {
let env: LanguageServiceTestEnvironment;
it('returns the pipe class as definition when checkTypeOfPipes is false', () => { it('returns the pipe class as definition when checkTypeOfPipes is false', () => {
initMockFileSystem('Native'); initMockFileSystem('Native');
const testFiles: TestFile[] = [ const {cursor, text} = extractCursorInfo('{{"1/1/2020" | dat¦e}}');
{ const templateFile = {contents: text, name: absoluteFrom('/app.html')};
const appFile = {
name: absoluteFrom('/app.ts'), name: absoluteFrom('/app.ts'),
contents: ` contents: `
import {Component, NgModule} from '@angular/core'; import {Component, NgModule} from '@angular/core';
@ -26,43 +25,74 @@ describe('definitions', () => {
@Component({templateUrl: 'app.html'}) @Component({templateUrl: 'app.html'})
export class AppCmp {} export class AppCmp {}
@NgModule({declarations: [AppCmp], imports: [CommonModule]})
export class AppModule {}
`, `,
isRoot: true };
}, const env = createModuleWithDeclarations([appFile], [templateFile]);
{
name: absoluteFrom('/app.html'),
contents: `Will be overridden`,
}
];
// checkTypeOfPipes is set to false when strict templates is false // checkTypeOfPipes is set to false when strict templates is false
env = LanguageServiceTestEnvironment.setup(testFiles, {strictTemplates: false}); const {textSpan, definitions} =
const definitions = getDefinitionsAndAssertBoundSpan( getDefinitionsAndAssertBoundSpan(env, absoluteFrom('/app.html'), cursor);
{templateOverride: '{{"1/1/2020" | dat¦e}}', expectedSpanText: 'date'}); expect(text.substr(textSpan.start, textSpan.length)).toEqual('date');
expect(definitions!.length).toEqual(1);
expect(definitions.length).toEqual(1);
const [def] = definitions; const [def] = definitions;
expect(def.textSpan).toContain('DatePipe'); expect(def.textSpan).toContain('DatePipe');
expect(def.contextSpan).toContain('DatePipe'); expect(def.contextSpan).toContain('DatePipe');
}); });
it('gets definitions for all inputs when attribute matches more than one', () => {
initMockFileSystem('Native');
const {cursor, text} = extractCursorInfo('<div dir inpu¦tA="abc"></div>');
const templateFile = {contents: text, name: absoluteFrom('/app.html')};
const dirFile = {
name: absoluteFrom('/dir.ts'),
contents: `
import {Directive, Input} from '@angular/core';
@Directive({selector: '[dir]'})
export class MyDir {
@Input() inputA!: any;
}`,
};
const dirFile2 = {
name: absoluteFrom('/dir2.ts'),
contents: `
import {Directive, Input} from '@angular/core';
@Directive({selector: '[dir]'})
export class MyDir2 {
@Input() inputA!: any;
}`,
};
const appFile = {
name: absoluteFrom('/app.ts'),
contents: `
import {Component, NgModule} from '@angular/core';
import {CommonModule} from '@angular/common';
@Component({templateUrl: 'app.html'})
export class AppCmp {}
`
};
const env = createModuleWithDeclarations([appFile, dirFile, dirFile2], [templateFile]);
const {textSpan, definitions} =
getDefinitionsAndAssertBoundSpan(env, absoluteFrom('/app.html'), cursor);
expect(text.substr(textSpan.start, textSpan.length)).toEqual('inputA');
expect(definitions.length).toEqual(2);
const [def, def2] = definitions;
expect(def.textSpan).toContain('inputA');
expect(def2.textSpan).toContain('inputA');
// TODO(atscott): investigate why the text span includes more than just 'inputA'
// assertTextSpans([def, def2], ['inputA']);
assertFileNames([def, def2], ['dir2.ts', 'dir.ts']);
});
function getDefinitionsAndAssertBoundSpan( function getDefinitionsAndAssertBoundSpan(
{templateOverride, expectedSpanText}: {templateOverride: string, expectedSpanText: string}): env: LanguageServiceTestEnvironment, fileName: AbsoluteFsPath, cursor: number) {
Array<{textSpan: string, contextSpan: string | undefined, fileName: string}> {
const {cursor, text} =
env.overrideTemplateWithCursor(absoluteFrom('/app.ts'), 'AppCmp', templateOverride);
env.expectNoSourceDiagnostics(); env.expectNoSourceDiagnostics();
env.expectNoTemplateDiagnostics(absoluteFrom('/app.ts'), 'AppCmp'); const definitionAndBoundSpan = env.ngLS.getDefinitionAndBoundSpan(fileName, cursor);
const definitionAndBoundSpan =
env.ngLS.getDefinitionAndBoundSpan(absoluteFrom('/app.html'), cursor);
const {textSpan, definitions} = definitionAndBoundSpan!; const {textSpan, definitions} = definitionAndBoundSpan!;
expect(text.substring(textSpan.start, textSpan.start + textSpan.length))
.toEqual(expectedSpanText);
expect(definitions).toBeTruthy(); expect(definitions).toBeTruthy();
const overrides = new Map<string, string>(); return {textSpan, definitions: definitions!.map(d => humanizeDefinitionInfo(d, env.host))};
overrides.set(absoluteFrom('/app.ts'), text);
return definitions!.map(d => humanizeDefinitionInfo(d, env.host, overrides));
} }
}); });

View File

@ -11,7 +11,7 @@ import {initMockFileSystem, TestFile} from '@angular/compiler-cli/src/ngtsc/file
import * as ts from 'typescript/lib/tsserverlibrary'; import * as ts from 'typescript/lib/tsserverlibrary';
import {extractCursorInfo, LanguageServiceTestEnvironment} from './env'; import {extractCursorInfo, LanguageServiceTestEnvironment} from './env';
import {createModuleWithDeclarations, getText} from './test_utils'; import {assertFileNames, assertTextSpans, createModuleWithDeclarations, getText} from './test_utils';
describe('find references', () => { describe('find references', () => {
let env: LanguageServiceTestEnvironment; let env: LanguageServiceTestEnvironment;
@ -832,21 +832,6 @@ describe('find references', () => {
} }
}); });
function assertFileNames(refs: Array<{fileName: string}>, expectedFileNames: string[]) {
const actualPaths = refs.map(r => r.fileName);
const actualFileNames = actualPaths.map(p => last(p.split('/')));
expect(new Set(actualFileNames)).toEqual(new Set(expectedFileNames));
}
function assertTextSpans(refs: Array<{textSpan: string}>, expectedTextSpans: string[]) {
const actualSpans = refs.map(ref => ref.textSpan);
expect(new Set(actualSpans)).toEqual(new Set(expectedTextSpans));
}
function last<T>(array: T[]): T {
return array[array.length - 1];
}
type Stringy<T> = { type Stringy<T> = {
[P in keyof T]: string; [P in keyof T]: string;
}; };

View File

@ -75,3 +75,14 @@ export function humanizeDefinitionInfo(
undefined, undefined,
}; };
} }
export function assertFileNames(refs: Array<{fileName: string}>, expectedFileNames: string[]) {
const actualPaths = refs.map(r => r.fileName);
const actualFileNames = actualPaths.map(p => last(p.split('/')));
expect(new Set(actualFileNames)).toEqual(new Set(expectedFileNames));
}
export function assertTextSpans(items: Array<{textSpan: string}>, expectedTextSpans: string[]) {
const actualSpans = items.map(item => item.textSpan);
expect(new Set(actualSpans)).toEqual(new Set(expectedTextSpans));
}