From 2b74a05a6539b78ed6682e29d4c3b5a6052a9cdd Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 3 Nov 2020 16:49:30 -0800 Subject: [PATCH] refactor(compiler-cli): Enable pipe information when checkTypeOfPipes=false (#39555) When `checkTypeOfPipes` is set to `false`, the configuration is meant to ignore the signature of the pipe's `transform` method for diagnostics. However, we still should produce some information about the pipe for the `TemplateTypeChecker`. This change refactors the returned symbol for pipes so that it also includes information about the pipe's class instance as it appears in the TCB. PR Close #39555 --- .../src/ngtsc/typecheck/api/symbols.ts | 37 +++++++- .../src/ngtsc/typecheck/src/context.ts | 2 +- .../src/ngtsc/typecheck/src/tcb_util.ts | 10 +- .../typecheck/src/template_symbol_builder.ts | 47 +++++++++- .../ngtsc/typecheck/src/type_check_block.ts | 3 +- .../typecheck/test/type_check_block_spec.ts | 3 +- ...ecker__get_symbol_of_template_node_spec.ts | 94 ++++++++++++------- packages/language-service/ivy/definitions.ts | 18 ++++ packages/language-service/ivy/quick_info.ts | 26 +++-- packages/language-service/ivy/references.ts | 1 + .../ivy/test/definitions_spec.ts | 68 ++++++++++++++ .../ivy/test/quick_info_spec.ts | 9 ++ .../language-service/ivy/test/test_utils.ts | 24 +++++ .../ivy/test/type_definitions_spec.ts | 63 +++++++++++++ 14 files changed, 349 insertions(+), 56 deletions(-) create mode 100644 packages/language-service/ivy/test/definitions_spec.ts create mode 100644 packages/language-service/ivy/test/type_definitions_spec.ts diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts index 5f8bcd7b8b..f6b3cf92c6 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/symbols.ts @@ -24,13 +24,14 @@ export enum SymbolKind { Template, Expression, DomBinding, + Pipe, } /** * A representation of an entity in the `TemplateAst`. */ export type Symbol = InputBindingSymbol|OutputBindingSymbol|ElementSymbol|ReferenceSymbol| - VariableSymbol|ExpressionSymbol|DirectiveSymbol|TemplateSymbol|DomBindingSymbol; + VariableSymbol|ExpressionSymbol|DirectiveSymbol|TemplateSymbol|DomBindingSymbol|PipeSymbol; /** * A `Symbol` which declares a new named entity in the template scope. @@ -276,3 +277,37 @@ export interface DomBindingSymbol { /** The symbol for the element or template of the text attribute. */ host: ElementSymbol|TemplateSymbol; } + +/** + * A representation for a call to a pipe's transform method in the TCB. + */ +export interface PipeSymbol { + kind: SymbolKind.Pipe; + + /** The `ts.Type` of the transform node. */ + tsType: ts.Type; + + /** + * The `ts.Symbol` for the transform call. This could be `null` when `checkTypeOfPipes` is set to + * `false` because the transform call would be of the form `(_pipe1 as any).transform()` + */ + tsSymbol: ts.Symbol|null; + + /** The position of the transform call in the template. */ + shimLocation: ShimLocation; + + /** The symbol for the pipe class as an instance that appears in the TCB. */ + classSymbol: ClassSymbol; +} + +/** Represents an instance of a class found in the TCB, i.e. `var _pipe1: MyPipe = null!; */ +export interface ClassSymbol { + /** The `ts.Type` of class. */ + tsType: ts.Type; + + /** The `ts.Symbol` for class. */ + tsSymbol: ts.Symbol; + + /** The position for the variable declaration for the class instance. */ + shimLocation: ShimLocation; +} diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts index e122c3e94b..c9c0cc7723 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/context.ts @@ -257,7 +257,7 @@ export class TypeCheckContextImpl implements TypeCheckContext { boundTarget, }); - const tcbRequiresInline = requiresInlineTypeCheckBlock(ref.node); + const tcbRequiresInline = requiresInlineTypeCheckBlock(ref.node, pipes); // If inlining is not supported, but is required for either the TCB or one of its directive // dependencies, then exit here with an error. diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts index ac4f3cfb5e..43b944d4e2 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/tcb_util.ts @@ -10,6 +10,7 @@ import {AbsoluteSourceSpan, ParseSourceSpan} from '@angular/compiler'; import {ClassDeclaration} from '@angular/compiler-cli/src/ngtsc/reflection'; import * as ts from 'typescript'; +import {Reference} from '../../imports'; import {getTokenAtPosition} from '../../util/src/typescript'; import {FullTemplateMapping, SourceLocation, TemplateId, TemplateSourceMapping} from '../api'; @@ -37,7 +38,9 @@ export interface TemplateSourceResolver { toParseSourceSpan(id: TemplateId, span: AbsoluteSourceSpan): ParseSourceSpan|null; } -export function requiresInlineTypeCheckBlock(node: ClassDeclaration): boolean { +export function requiresInlineTypeCheckBlock( + node: ClassDeclaration, + usedPipes: Map>>): boolean { // In order to qualify for a declared TCB (not inline) two conditions must be met: // 1) the class must be exported // 2) it must not have constrained generic types @@ -47,6 +50,11 @@ export function requiresInlineTypeCheckBlock(node: ClassDeclaration !checkIfClassIsExported(pipeRef.node))) { + // If one of the pipes used by the component is not exported, a non-inline TCB will not be able + // to import it, so this requires an inline TCB. + return true; } else { return false; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts index 86cb34e882..a3c2e51336 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/template_symbol_builder.ts @@ -13,7 +13,7 @@ import {AbsoluteFsPath} from '../../file_system'; import {ClassDeclaration} from '../../reflection'; import {ComponentScopeReader} from '../../scope'; import {isAssignment} from '../../util/src/typescript'; -import {DirectiveSymbol, DomBindingSymbol, ElementSymbol, ExpressionSymbol, InputBindingSymbol, OutputBindingSymbol, ReferenceSymbol, ShimLocation, Symbol, SymbolKind, TemplateSymbol, TsNodeSymbolInfo, TypeCheckableDirectiveMeta, VariableSymbol} from '../api'; +import {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 {TemplateData} from './context'; @@ -61,6 +61,8 @@ export class SymbolBuilder { symbol = this.getSymbolOfVariable(node); } else if (node instanceof TmplAstReference) { symbol = this.getSymbolOfReference(node); + } else if (node instanceof BindingPipe) { + symbol = this.getSymbolOfPipe(node); } else if (node instanceof AST) { symbol = this.getSymbolOfTemplateExpression(node); } else { @@ -397,6 +399,45 @@ export class SymbolBuilder { } } + private getSymbolOfPipe(expression: BindingPipe): PipeSymbol|null { + const node = findFirstMatchingNode( + this.typeCheckBlock, {withSpan: expression.sourceSpan, filter: ts.isCallExpression}); + if (node === null || !ts.isPropertyAccessExpression(node.expression)) { + return null; + } + + const methodAccess = node.expression; + // Find the node for the pipe variable from the transform property access. This will be one of + // two forms: `_pipe1.transform` or `(_pipe1 as any).transform`. + const pipeVariableNode = ts.isParenthesizedExpression(methodAccess.expression) && + ts.isAsExpression(methodAccess.expression.expression) ? + methodAccess.expression.expression.expression : + methodAccess.expression; + const pipeDeclaration = this.getTypeChecker().getSymbolAtLocation(pipeVariableNode); + if (pipeDeclaration === undefined || pipeDeclaration.valueDeclaration === undefined) { + return null; + } + + const pipeInstance = this.getSymbolOfTsNode(pipeDeclaration.valueDeclaration); + if (pipeInstance === null || pipeInstance.tsSymbol === null) { + return null; + } + + const symbolInfo = this.getSymbolOfTsNode(methodAccess); + if (symbolInfo === null) { + return null; + } + + return { + kind: SymbolKind.Pipe, + ...symbolInfo, + classSymbol: { + ...pipeInstance, + tsSymbol: pipeInstance.tsSymbol, + }, + }; + } + private getSymbolOfTemplateExpression(expression: AST): VariableSymbol|ReferenceSymbol |ExpressionSymbol|null { if (expression instanceof ASTWithSource) { @@ -446,10 +487,6 @@ export class SymbolBuilder { // still get the type of the whole conditional expression to include `|undefined`. tsType: this.getTypeChecker().getTypeAtLocation(node) }; - } else if (expression instanceof BindingPipe && ts.isCallExpression(node)) { - // TODO(atscott): Create a PipeSymbol to include symbol for the Pipe class - const symbolInfo = this.getSymbolOfTsNode(node.expression); - return symbolInfo === null ? null : {...symbolInfo, kind: SymbolKind.Expression}; } else { const symbolInfo = this.getSymbolOfTsNode(node); return symbolInfo === null ? null : {...symbolInfo, kind: SymbolKind.Expression}; diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index ba3a23d083..3db0a4eacf 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -1599,7 +1599,8 @@ class TcbExpressionTranslator { pipe = this.tcb.env.pipeInst(pipeRef); } else { // Use an 'any' value when not checking the type of the pipe. - pipe = NULL_AS_ANY; + pipe = ts.createAsExpression( + this.tcb.env.pipeInst(pipeRef), ts.createKeywordTypeNode(ts.SyntaxKind.AnyKeyword)); } const args = ast.args.map(arg => this.translate(arg)); const methodAccess = ts.createPropertyAccess(pipe, 'transform'); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts index 8e2b580d7a..6de3126cb0 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts @@ -973,7 +973,8 @@ describe('type check blocks', () => { it('should not check types of pipes when disabled', () => { const DISABLED_CONFIG: TypeCheckingConfig = {...BASE_CONFIG, checkTypeOfPipes: false}; const block = tcb(TEMPLATE, PIPES, DISABLED_CONFIG); - expect(block).toContain('(null as any).transform(((ctx).a), ((ctx).b), ((ctx).c))'); + expect(block).toContain( + '((null as TestPipe) as any).transform(((ctx).a), ((ctx).b), ((ctx).c))'); }); }); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts index 23bb5912e7..effba31830 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__get_symbol_of_template_node_spec.ts @@ -12,7 +12,7 @@ import * as ts from 'typescript'; import {absoluteFrom, AbsoluteFsPath, getSourceFileOrError} from '../../file_system'; import {runInEachFileSystem} from '../../file_system/testing'; import {ClassDeclaration} from '../../reflection'; -import {DirectiveSymbol, DomBindingSymbol, ElementSymbol, ExpressionSymbol, InputBindingSymbol, OutputBindingSymbol, ReferenceSymbol, Symbol, SymbolKind, TemplateSymbol, TemplateTypeChecker, TypeCheckingConfig, VariableSymbol} from '../api'; +import {DirectiveSymbol, DomBindingSymbol, ElementSymbol, ExpressionSymbol, InputBindingSymbol, OutputBindingSymbol, PipeSymbol, ReferenceSymbol, Symbol, SymbolKind, TemplateSymbol, TemplateTypeChecker, TypeCheckingConfig, VariableSymbol} from '../api'; import {getClass, ngForDeclaration, ngForTypeCheckTarget, setup as baseTestSetup, TypeCheckingTarget} from './test_utils'; @@ -710,34 +710,35 @@ runInEachFileSystem(() => { }); }); - describe('pipes', () => { let templateTypeChecker: TemplateTypeChecker; let cmp: ClassDeclaration; let binding: BindingPipe; let program: ts.Program; - beforeEach(() => { + function setupPipesTest(checkTypeOfPipes = true) { const fileName = absoluteFrom('/main.ts'); const templateString = `
`; - const testValues = setup([ - { - fileName, - templates: {'Cmp': templateString}, - source: ` + const testValues = setup( + [ + { + fileName, + templates: {'Cmp': templateString}, + source: ` export class Cmp { a: string; b: number; c: boolean } export class TestPipe { transform(value: string, repeat: number, commaSeparate: boolean): string[] { } } `, - declarations: [{ - type: 'pipe', - name: 'TestPipe', - pipeName: 'test', - }], - }, - ]); + declarations: [{ + type: 'pipe', + name: 'TestPipe', + pipeName: 'test', + }], + }, + ], + {checkTypeOfPipes}); program = testValues.program; templateTypeChecker = testValues.templateTypeChecker; const sf = getSourceFileOrError(testValues.program, fileName); @@ -745,38 +746,55 @@ runInEachFileSystem(() => { binding = (getAstElements(templateTypeChecker, cmp)[0].inputs[0].value as ASTWithSource).ast as BindingPipe; - }); + } it('should get symbol for pipe', () => { + setupPipesTest(); const pipeSymbol = templateTypeChecker.getSymbolOfNode(binding, cmp)!; - assertExpressionSymbol(pipeSymbol); + assertPipeSymbol(pipeSymbol); expect(program.getTypeChecker().symbolToString(pipeSymbol.tsSymbol!)) .toEqual('transform'); - expect( - (pipeSymbol.tsSymbol!.declarations[0].parent as ts.ClassDeclaration).name!.getText()) + expect(program.getTypeChecker().symbolToString(pipeSymbol.classSymbol.tsSymbol)) .toEqual('TestPipe'); - expect(program.getTypeChecker().typeToString(pipeSymbol.tsType)) + expect(program.getTypeChecker().typeToString(pipeSymbol.tsType!)) .toEqual('(value: string, repeat: number, commaSeparate: boolean) => string[]'); }); - it('should get symbols for pipe expression and args', () => { - const aSymbol = templateTypeChecker.getSymbolOfNode(binding.exp, cmp)!; - assertExpressionSymbol(aSymbol); - expect(program.getTypeChecker().symbolToString(aSymbol.tsSymbol!)).toEqual('a'); - expect(program.getTypeChecker().typeToString(aSymbol.tsType)).toEqual('string'); - - const bSymbol = templateTypeChecker.getSymbolOfNode(binding.args[0], cmp)!; - assertExpressionSymbol(bSymbol); - expect(program.getTypeChecker().symbolToString(bSymbol.tsSymbol!)).toEqual('b'); - expect(program.getTypeChecker().typeToString(bSymbol.tsType)).toEqual('number'); - - const cSymbol = templateTypeChecker.getSymbolOfNode(binding.args[1], cmp)!; - assertExpressionSymbol(cSymbol); - expect(program.getTypeChecker().symbolToString(cSymbol.tsSymbol!)).toEqual('c'); - expect(program.getTypeChecker().typeToString(cSymbol.tsType)).toEqual('boolean'); + it('should get symbol for pipe, checkTypeOfPipes: false', () => { + setupPipesTest(false); + const pipeSymbol = templateTypeChecker.getSymbolOfNode(binding, cmp)!; + assertPipeSymbol(pipeSymbol); + expect(pipeSymbol.tsSymbol).toBeNull(); + expect(program.getTypeChecker().typeToString(pipeSymbol.tsType!)).toEqual('any'); + expect(program.getTypeChecker().symbolToString(pipeSymbol.classSymbol.tsSymbol)) + .toEqual('TestPipe'); + expect(program.getTypeChecker().typeToString(pipeSymbol.classSymbol.tsType)) + .toEqual('TestPipe'); }); - }); + for (const checkTypeOfPipes of [true, false]) { + describe(`checkTypeOfPipes: ${checkTypeOfPipes}`, () => { + // Because the args are property reads, we still need information about them. + it(`should get symbols for pipe expression and args`, () => { + setupPipesTest(checkTypeOfPipes); + const aSymbol = templateTypeChecker.getSymbolOfNode(binding.exp, cmp)!; + assertExpressionSymbol(aSymbol); + expect(program.getTypeChecker().symbolToString(aSymbol.tsSymbol!)).toEqual('a'); + expect(program.getTypeChecker().typeToString(aSymbol.tsType)).toEqual('string'); + + const bSymbol = templateTypeChecker.getSymbolOfNode(binding.args[0], cmp)!; + assertExpressionSymbol(bSymbol); + expect(program.getTypeChecker().symbolToString(bSymbol.tsSymbol!)).toEqual('b'); + expect(program.getTypeChecker().typeToString(bSymbol.tsType)).toEqual('number'); + + const cSymbol = templateTypeChecker.getSymbolOfNode(binding.args[1], cmp)!; + assertExpressionSymbol(cSymbol); + expect(program.getTypeChecker().symbolToString(cSymbol.tsSymbol!)).toEqual('c'); + expect(program.getTypeChecker().typeToString(cSymbol.tsType)).toEqual('boolean'); + }); + }); + } + }); it('should get a symbol for PropertyWrite expressions', () => { const fileName = absoluteFrom('/main.ts'); @@ -1515,6 +1533,10 @@ function assertExpressionSymbol(tSymbol: Symbol): asserts tSymbol is ExpressionS expect(tSymbol.kind).toEqual(SymbolKind.Expression); } +function assertPipeSymbol(tSymbol: Symbol): asserts tSymbol is PipeSymbol { + expect(tSymbol.kind).toEqual(SymbolKind.Pipe); +} + function assertElementSymbol(tSymbol: Symbol): asserts tSymbol is ElementSymbol { expect(tSymbol.kind).toEqual(SymbolKind.Element); } diff --git a/packages/language-service/ivy/definitions.ts b/packages/language-service/ivy/definitions.ts index de0b8ce78a..ca8861e1d7 100644 --- a/packages/language-service/ivy/definitions.ts +++ b/packages/language-service/ivy/definitions.ts @@ -67,6 +67,15 @@ export class DefinitionBuilder { // LS users to "go to definition" on an item in the template that maps to a class and be // taken to the directive or HTML class. return this.getTypeDefinitionsForTemplateInstance(symbol, node); + case SymbolKind.Pipe: { + if (symbol.tsSymbol !== null) { + return this.getDefinitionsForSymbols(symbol); + } else { + // If there is no `ts.Symbol` for the pipe transform, we want to return the + // type definition (the pipe class). + return this.getTypeDefinitionsForSymbols(symbol.classSymbol); + } + } case SymbolKind.Output: case SymbolKind.Input: { const bindingDefs = this.getDefinitionsForSymbols(...symbol.bindings); @@ -135,6 +144,15 @@ export class DefinitionBuilder { node, definitionMeta.parent, templateInfo.component); return [...bindingDefs, ...directiveDefs]; } + case SymbolKind.Pipe: { + if (symbol.tsSymbol !== null) { + return this.getTypeDefinitionsForSymbols(symbol); + } else { + // If there is no `ts.Symbol` for the pipe transform, we want to return the + // type definition (the pipe class). + return this.getTypeDefinitionsForSymbols(symbol.classSymbol); + } + } case SymbolKind.Reference: return this.getTypeDefinitionsForSymbols({shimLocation: symbol.targetLocation}); case SymbolKind.Expression: diff --git a/packages/language-service/ivy/quick_info.ts b/packages/language-service/ivy/quick_info.ts index 32d99e65fd..fb40c07dcb 100644 --- a/packages/language-service/ivy/quick_info.ts +++ b/packages/language-service/ivy/quick_info.ts @@ -5,13 +5,13 @@ * 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 {AST, BindingPipe, ImplicitReceiver, MethodCall, ThisReceiver, TmplAstBoundAttribute, TmplAstNode, TmplAstTextAttribute} from '@angular/compiler'; +import {AST, ImplicitReceiver, MethodCall, ThisReceiver, TmplAstBoundAttribute, TmplAstNode, TmplAstTextAttribute} from '@angular/compiler'; import {NgCompiler} from '@angular/compiler-cli/src/ngtsc/core'; -import {DirectiveSymbol, DomBindingSymbol, ElementSymbol, ExpressionSymbol, InputBindingSymbol, OutputBindingSymbol, ReferenceSymbol, ShimLocation, Symbol, SymbolKind, VariableSymbol} from '@angular/compiler-cli/src/ngtsc/typecheck/api'; +import {DirectiveSymbol, DomBindingSymbol, ElementSymbol, InputBindingSymbol, OutputBindingSymbol, PipeSymbol, ReferenceSymbol, ShimLocation, Symbol, SymbolKind, VariableSymbol} from '@angular/compiler-cli/src/ngtsc/typecheck/api'; import * as ts from 'typescript'; import {createDisplayParts, DisplayInfoKind, SYMBOL_PUNC, SYMBOL_SPACE, SYMBOL_TEXT, unsafeCastDisplayInfoKindToScriptElementKind} from './display_parts'; -import {filterAliasImports, getDirectiveMatchesForAttribute, getDirectiveMatchesForElementTag, getTemplateInfoAtPosition, getTextSpanOfNode} from './utils'; +import {filterAliasImports, getDirectiveMatchesForAttribute, getDirectiveMatchesForElementTag, getTextSpanOfNode} from './utils'; export class QuickInfoBuilder { private readonly typeChecker = this.compiler.getNextProgram().getTypeChecker(); @@ -47,10 +47,10 @@ export class QuickInfoBuilder { return this.getQuickInfoForDomBinding(symbol); case SymbolKind.Directive: return this.getQuickInfoAtShimLocation(symbol.shimLocation); + case SymbolKind.Pipe: + return this.getQuickInfoForPipeSymbol(symbol); case SymbolKind.Expression: - return this.node instanceof BindingPipe ? - this.getQuickInfoForPipeSymbol(symbol) : - this.getQuickInfoAtShimLocation(symbol.shimLocation); + return this.getQuickInfoAtShimLocation(symbol.shimLocation); } } @@ -93,10 +93,16 @@ export class QuickInfoBuilder { undefined /* containerName */, this.typeChecker.typeToString(symbol.tsType), documentation); } - private getQuickInfoForPipeSymbol(symbol: ExpressionSymbol): ts.QuickInfo|undefined { - const quickInfo = this.getQuickInfoAtShimLocation(symbol.shimLocation); - return quickInfo === undefined ? undefined : - updateQuickInfoKind(quickInfo, DisplayInfoKind.PIPE); + private getQuickInfoForPipeSymbol(symbol: PipeSymbol): ts.QuickInfo|undefined { + if (symbol.tsSymbol !== null) { + const quickInfo = this.getQuickInfoAtShimLocation(symbol.shimLocation); + return quickInfo === undefined ? undefined : + updateQuickInfoKind(quickInfo, DisplayInfoKind.PIPE); + } else { + return createQuickInfo( + this.typeChecker.typeToString(symbol.classSymbol.tsType), DisplayInfoKind.PIPE, + getTextSpanOfNode(this.node)); + } } private getQuickInfoForDomBinding(symbol: DomBindingSymbol) { diff --git a/packages/language-service/ivy/references.ts b/packages/language-service/ivy/references.ts index cf72980e17..661a77c2d7 100644 --- a/packages/language-service/ivy/references.ts +++ b/packages/language-service/ivy/references.ts @@ -95,6 +95,7 @@ export class ReferenceBuilder { const {shimPath, positionInShimFile} = symbol.bindings[0].shimLocation; return this.getReferencesAtTypescriptPosition(shimPath, positionInShimFile); } + case SymbolKind.Pipe: case SymbolKind.Expression: { const {shimPath, positionInShimFile} = symbol.shimLocation; return this.getReferencesAtTypescriptPosition(shimPath, positionInShimFile); diff --git a/packages/language-service/ivy/test/definitions_spec.ts b/packages/language-service/ivy/test/definitions_spec.ts new file mode 100644 index 0000000000..80eadd2bca --- /dev/null +++ b/packages/language-service/ivy/test/definitions_spec.ts @@ -0,0 +1,68 @@ +/** + * @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 {absoluteFrom} from '@angular/compiler-cli/src/ngtsc/file_system'; +import {initMockFileSystem, TestFile} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; + +import {LanguageServiceTestEnvironment} from './env'; +import {humanizeDefinitionInfo} from './test_utils'; + +describe('definitions', () => { + let env: LanguageServiceTestEnvironment; + + it('returns the pipe class as definition when checkTypeOfPipes is false', () => { + initMockFileSystem('Native'); + const testFiles: TestFile[] = [ + { + name: absoluteFrom('/app.ts'), + contents: ` + import {Component, NgModule} from '@angular/core'; + import {CommonModule} from '@angular/common'; + + @Component({templateUrl: 'app.html'}) + export class AppCmp {} + + @NgModule({declarations: [AppCmp], imports: [CommonModule]}) + export class AppModule {} + `, + isRoot: true + }, + { + name: absoluteFrom('/app.html'), + contents: `Will be overridden`, + } + ]; + // checkTypeOfPipes is set to false when strict templates is false + env = LanguageServiceTestEnvironment.setup(testFiles, {strictTemplates: false}); + const definitions = getDefinitionsAndAssertBoundSpan( + {templateOverride: '{{"1/1/2020" | dat¦e}}', expectedSpanText: 'date'}); + expect(definitions!.length).toEqual(1); + + const [def] = definitions; + expect(def.textSpan).toContain('DatePipe'); + expect(def.contextSpan).toContain('DatePipe'); + }); + + function getDefinitionsAndAssertBoundSpan( + {templateOverride, expectedSpanText}: {templateOverride: string, expectedSpanText: string}): + Array<{textSpan: string, contextSpan: string | undefined, fileName: string}> { + const {cursor, text} = + env.overrideTemplateWithCursor(absoluteFrom('/app.ts'), 'AppCmp', templateOverride); + env.expectNoSourceDiagnostics(); + env.expectNoTemplateDiagnostics(absoluteFrom('/app.ts'), 'AppCmp'); + const definitionAndBoundSpan = + env.ngLS.getDefinitionAndBoundSpan(absoluteFrom('/app.html'), cursor); + const {textSpan, definitions} = definitionAndBoundSpan!; + expect(text.substring(textSpan.start, textSpan.start + textSpan.length)) + .toEqual(expectedSpanText); + expect(definitions).toBeTruthy(); + const overrides = new Map(); + overrides.set(absoluteFrom('/app.ts'), text); + return definitions!.map(d => humanizeDefinitionInfo(d, env.host, overrides)); + } +}); diff --git a/packages/language-service/ivy/test/quick_info_spec.ts b/packages/language-service/ivy/test/quick_info_spec.ts index b6c91786b1..8a23f4b5a9 100644 --- a/packages/language-service/ivy/test/quick_info_spec.ts +++ b/packages/language-service/ivy/test/quick_info_spec.ts @@ -507,6 +507,15 @@ describe('quick info', () => { expectedDisplayString: '(event) TestComponent.testEvent: EventEmitter' }); }); + + it('should work for pipes even if checkTypeOfPipes is false', () => { + initMockFileSystem('Native'); + // checkTypeOfPipes is set to false when strict templates is false + env = LanguageServiceTestEnvironment.setup(quickInfoSkeleton(), {strictTemplates: false}); + const templateOverride = `

The hero's birthday is {{birthday | da¦te: "MM/dd/yy"}}

`; + expectQuickInfo( + {templateOverride, expectedSpanText: 'date', expectedDisplayString: '(pipe) DatePipe'}); + }); }); function expectQuickInfo( diff --git a/packages/language-service/ivy/test/test_utils.ts b/packages/language-service/ivy/test/test_utils.ts index 2d7253f91b..6c414ed064 100644 --- a/packages/language-service/ivy/test/test_utils.ts +++ b/packages/language-service/ivy/test/test_utils.ts @@ -10,6 +10,8 @@ import {absoluteFrom as _} from '@angular/compiler-cli/src/ngtsc/file_system'; import {TestFile} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; import {LanguageServiceTestEnvironment} from '@angular/language-service/ivy/test/env'; +import {MockServerHost} from './mock_host'; + export function getText(contents: string, textSpan: ts.TextSpan) { return contents.substr(textSpan.start, textSpan.length); } @@ -51,3 +53,25 @@ export function createModuleWithDeclarations( return LanguageServiceTestEnvironment.setup( [moduleFile, ...filesWithClassDeclarations, ...externalResourceFiles]); } + +export interface HumanizedDefinitionInfo { + fileName: string; + textSpan: string; + contextSpan: string|undefined; +} + +export function humanizeDefinitionInfo( + def: ts.DefinitionInfo, host: MockServerHost, + overrides: Map = new Map()): HumanizedDefinitionInfo { + const contents = (overrides.get(def.fileName) !== undefined ? overrides.get(def.fileName) : + host.readFile(def.fileName)) ?? + ''; + + return { + fileName: def.fileName, + textSpan: contents.substr(def.textSpan.start, def.textSpan.start + def.textSpan.length), + contextSpan: def.contextSpan ? + contents.substr(def.contextSpan.start, def.contextSpan.start + def.contextSpan.length) : + undefined, + }; +} diff --git a/packages/language-service/ivy/test/type_definitions_spec.ts b/packages/language-service/ivy/test/type_definitions_spec.ts new file mode 100644 index 0000000000..0fbe43830c --- /dev/null +++ b/packages/language-service/ivy/test/type_definitions_spec.ts @@ -0,0 +1,63 @@ +/** + * @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 {absoluteFrom} from '@angular/compiler-cli/src/ngtsc/file_system'; +import {initMockFileSystem, TestFile} from '@angular/compiler-cli/src/ngtsc/file_system/testing'; + +import {LanguageServiceTestEnvironment} from './env'; +import {HumanizedDefinitionInfo, humanizeDefinitionInfo} from './test_utils'; + +describe('type definitions', () => { + let env: LanguageServiceTestEnvironment; + + it('returns the pipe class as definition when checkTypeOfPipes is false', () => { + initMockFileSystem('Native'); + const testFiles: TestFile[] = [ + { + name: absoluteFrom('/app.ts'), + contents: ` + import {Component, NgModule} from '@angular/core'; + import {CommonModule} from '@angular/common'; + + @Component({templateUrl: 'app.html'}) + export class AppCmp {} + + @NgModule({declarations: [AppCmp], imports: [CommonModule]}) + export class AppModule {} + `, + isRoot: true + }, + { + name: absoluteFrom('/app.html'), + contents: `Will be overridden`, + } + ]; + // checkTypeOfPipes is set to false when strict templates is false + env = LanguageServiceTestEnvironment.setup(testFiles, {strictTemplates: false}); + const definitions = + getTypeDefinitionsAndAssertBoundSpan({templateOverride: '{{"1/1/2020" | dat¦e}}'}); + expect(definitions!.length).toEqual(1); + + const [def] = definitions; + expect(def.textSpan).toContain('DatePipe'); + expect(def.contextSpan).toContain('DatePipe'); + }); + + function getTypeDefinitionsAndAssertBoundSpan({templateOverride}: {templateOverride: string}): + HumanizedDefinitionInfo[] { + const {cursor, text} = + env.overrideTemplateWithCursor(absoluteFrom('/app.ts'), 'AppCmp', templateOverride); + env.expectNoSourceDiagnostics(); + env.expectNoTemplateDiagnostics(absoluteFrom('/app.ts'), 'AppCmp'); + const defs = env.ngLS.getTypeDefinitionAtPosition(absoluteFrom('/app.html'), cursor); + expect(defs).toBeTruthy(); + const overrides = new Map(); + overrides.set(absoluteFrom('/app.html'), text); + return defs!.map(d => humanizeDefinitionInfo(d, env.host, overrides)); + } +});