From 0bc539af29a79604f904b770d0469717793a77de Mon Sep 17 00:00:00 2001 From: Joey Perrott Date: Wed, 14 Apr 2021 08:16:47 -0700 Subject: [PATCH] Revert "fix(compiler-cli): autocomplete literal types in templates. (#41456)" (#41623) This reverts commit 1d12c50f63f90c91636185b2287e31e9c0291121. PR Close #41623 --- .../src/ngtsc/typecheck/api/checker.ts | 5 +- .../src/ngtsc/typecheck/api/completion.ts | 6 - .../src/ngtsc/typecheck/src/checker.ts | 7 +- .../src/ngtsc/typecheck/src/completion.ts | 120 ++++++------------ .../test/type_checker__completion_spec.ts | 2 +- packages/language-service/ivy/completions.ts | 51 +------- .../ivy/test/completions_spec.ts | 28 +--- 7 files changed, 53 insertions(+), 166 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts index d2b022f603..da1d0bb294 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/checker.ts @@ -105,9 +105,8 @@ export interface TemplateTypeChecker { * include completions from the template's context component, as well as any local references or * template variables which are in scope for that expression. */ - getGlobalCompletions( - context: TmplAstTemplate|null, component: ts.ClassDeclaration, - node: AST|TmplAstNode): GlobalCompletion|null; + getGlobalCompletions(context: TmplAstTemplate|null, component: ts.ClassDeclaration): + GlobalCompletion|null; /** diff --git a/packages/compiler-cli/src/ngtsc/typecheck/api/completion.ts b/packages/compiler-cli/src/ngtsc/typecheck/api/completion.ts index 47d09ddb26..7fb996cfb4 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/api/completion.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/api/completion.ts @@ -71,10 +71,4 @@ export interface GlobalCompletion { * the same name (from the `componentContext` completions). */ templateContext: Map; - - /** - * A location within the type-checking shim where TypeScript's completion APIs can be used to - * access completions for the AST node of the cursor position (primitive constants). - */ - nodeContext: ShimLocation|null; } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts index bcb5960ad3..2490ad92bb 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts @@ -257,15 +257,14 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker { return this.getLatestComponentState(component).tcb; } - getGlobalCompletions( - context: TmplAstTemplate|null, component: ts.ClassDeclaration, - node: AST|TmplAstNode): GlobalCompletion|null { + getGlobalCompletions(context: TmplAstTemplate|null, component: ts.ClassDeclaration): + GlobalCompletion|null { const engine = this.getOrCreateCompletionEngine(component); if (engine === null) { return null; } return this.perf.inPhase( - PerfPhase.TtcAutocompletion, () => engine.getGlobalCompletions(context, node)); + PerfPhase.TtcAutocompletion, () => engine.getGlobalCompletions(context)); } getExpressionCompletionLocation( diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts index 473e5b8a6d..6157a663d7 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/completion.ts @@ -7,7 +7,7 @@ */ import {TmplAstReference, TmplAstTemplate} from '@angular/compiler'; -import {AST, EmptyExpr, MethodCall, PropertyRead, PropertyWrite, SafeMethodCall, SafePropertyRead, TmplAstNode} from '@angular/compiler/src/compiler'; +import {MethodCall, PropertyRead, PropertyWrite, SafeMethodCall, SafePropertyRead} from '@angular/compiler/src/compiler'; import * as ts from 'typescript'; import {AbsoluteFsPath} from '../../file_system'; @@ -23,77 +23,65 @@ import {TemplateData} from './context'; * surrounding TS program have changed. */ export class CompletionEngine { - private componentContext: ShimLocation|null; - /** - * Cache of completions for various levels of the template, including the root template (`null`). - * Memoizes `getTemplateContextCompletions`. + * Cache of `GlobalCompletion`s for various levels of the template, including the root template + * (`null`). */ - private templateContextCache = - new Map>(); + private globalCompletionCache = new Map(); private expressionCompletionCache = new Map(); + constructor(private tcb: ts.Node, private data: TemplateData, private shimPath: AbsoluteFsPath) {} + + /** + * Get global completions within the given template context - either a `TmplAstTemplate` embedded + * view, or `null` for the root template context. + */ + getGlobalCompletions(context: TmplAstTemplate|null): GlobalCompletion|null { + if (this.globalCompletionCache.has(context)) { + return this.globalCompletionCache.get(context)!; + } - constructor(private tcb: ts.Node, private data: TemplateData, private shimPath: AbsoluteFsPath) { // Find the component completion expression within the TCB. This looks like: `ctx. /* ... */;` const globalRead = findFirstMatchingNode(this.tcb, { filter: ts.isPropertyAccessExpression, withExpressionIdentifier: ExpressionIdentifier.COMPONENT_COMPLETION }); - if (globalRead !== null) { - this.componentContext = { + if (globalRead === null) { + return null; + } + + const completion: GlobalCompletion = { + componentContext: { shimPath: this.shimPath, // `globalRead.name` is an empty `ts.Identifier`, so its start position immediately follows // the `.` in `ctx.`. TS autocompletion APIs can then be used to access completion results // for the component context. positionInShimFile: globalRead.name.getStart(), - }; - } else { - this.componentContext = null; - } - } + }, + templateContext: new Map(), + }; - /** - * Get global completions within the given template context and AST node. - * - * @param context the given template context - either a `TmplAstTemplate` embedded view, or `null` - * for the root - * template context. - * @param node the given AST node - */ - getGlobalCompletions(context: TmplAstTemplate|null, node: AST|TmplAstNode): GlobalCompletion - |null { - if (this.componentContext === null) { - return null; - } - - const templateContext = this.getTemplateContextCompletions(context); - if (templateContext === null) { - return null; - } - - let nodeContext: ShimLocation|null = null; - if (node instanceof EmptyExpr) { - const nodeLocation = findFirstMatchingNode(this.tcb, { - filter: ts.isIdentifier, - withSpan: node.sourceSpan, - }); - if (nodeLocation !== null) { - nodeContext = { - shimPath: this.shimPath, - positionInShimFile: nodeLocation.getStart(), - }; + // The bound template already has details about the references and variables in scope in the + // `context` template - they just need to be converted to `Completion`s. + for (const node of this.data.boundTarget.getEntitiesInTemplateScope(context)) { + if (node instanceof TmplAstReference) { + completion.templateContext.set(node.name, { + kind: CompletionKind.Reference, + node, + }); + } else { + completion.templateContext.set(node.name, { + kind: CompletionKind.Variable, + node, + }); } } - return { - componentContext: this.componentContext, - templateContext, - nodeContext, - }; + this.globalCompletionCache.set(context, completion); + return completion; } getExpressionCompletionLocation(expr: PropertyRead|PropertyWrite|MethodCall| @@ -143,36 +131,4 @@ export class CompletionEngine { this.expressionCompletionCache.set(expr, res); return res; } - - /** - * Get global completions within the given template context - either a `TmplAstTemplate` embedded - * view, or `null` for the root context. - */ - private getTemplateContextCompletions(context: TmplAstTemplate|null): - Map|null { - if (this.templateContextCache.has(context)) { - return this.templateContextCache.get(context)!; - } - - const templateContext = new Map(); - - // The bound template already has details about the references and variables in scope in the - // `context` template - they just need to be converted to `Completion`s. - for (const node of this.data.boundTarget.getEntitiesInTemplateScope(context)) { - if (node instanceof TmplAstReference) { - templateContext.set(node.name, { - kind: CompletionKind.Reference, - node, - }); - } else { - templateContext.set(node.name, { - kind: CompletionKind.Variable, - node, - }); - } - } - - this.templateContextCache.set(context, templateContext); - return templateContext; - } } diff --git a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__completion_spec.ts b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__completion_spec.ts index d86e0bc8ac..104b3ed279 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__completion_spec.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/test/type_checker__completion_spec.ts @@ -141,7 +141,7 @@ function setupCompletions( context = tmpl; } - const completions = templateTypeChecker.getGlobalCompletions(context, SomeCmp, null!)!; + const completions = templateTypeChecker.getGlobalCompletions(context, SomeCmp)!; expect(completions).toBeDefined(); return { completions, diff --git a/packages/language-service/ivy/completions.ts b/packages/language-service/ivy/completions.ts index 84f89938f2..69a0a8bd75 100644 --- a/packages/language-service/ivy/completions.ts +++ b/packages/language-service/ivy/completions.ts @@ -216,21 +216,21 @@ export class CompletionBuilder { options: ts.GetCompletionsAtPositionOptions| undefined): ts.WithMetadata|undefined { const completions = - this.templateTypeChecker.getGlobalCompletions(this.template, this.component, this.node); + this.templateTypeChecker.getGlobalCompletions(this.template, this.component); if (completions === null) { return undefined; } - const {componentContext, templateContext, nodeContext: astContext} = completions; + const {componentContext, templateContext} = completions; const replacementSpan = makeReplacementSpanFromAst(this.node); // Merge TS completion results with results from the template scope. let entries: ts.CompletionEntry[] = []; - const componentCompletions = this.tsLS.getCompletionsAtPosition( + const tsLsCompletions = this.tsLS.getCompletionsAtPosition( componentContext.shimPath, componentContext.positionInShimFile, options); - if (componentCompletions !== undefined) { - for (const tsCompletion of componentCompletions.entries) { + if (tsLsCompletions !== undefined) { + for (const tsCompletion of tsLsCompletions.entries) { // Skip completions that are shadowed by a template entity definition. if (templateContext.has(tsCompletion.name)) { continue; @@ -244,24 +244,6 @@ export class CompletionBuilder { } } - // Merge TS completion results with results from the ast context. - if (astContext !== null) { - const nodeCompletions = this.tsLS.getCompletionsAtPosition( - astContext.shimPath, astContext.positionInShimFile, options); - if (nodeCompletions !== undefined) { - for (const tsCompletion of nodeCompletions.entries) { - if (this.isValidNodeContextCompletion(tsCompletion)) { - entries.push({ - ...tsCompletion, - // Substitute the TS completion's `replacementSpan` (which uses offsets within the - // TCB) with the `replacementSpan` within the template source. - replacementSpan, - }); - } - } - } - } - for (const [name, entity] of templateContext) { entries.push({ name, @@ -294,7 +276,7 @@ export class CompletionBuilder { formatOptions: ts.FormatCodeOptions|ts.FormatCodeSettings|undefined, preferences: ts.UserPreferences|undefined): ts.CompletionEntryDetails|undefined { const completions = - this.templateTypeChecker.getGlobalCompletions(this.template, this.component, this.node); + this.templateTypeChecker.getGlobalCompletions(this.template, this.component); if (completions === null) { return undefined; } @@ -335,7 +317,7 @@ export class CompletionBuilder { private getGlobalPropertyExpressionCompletionSymbol( this: PropertyExpressionCompletionBuilder, entryName: string): ts.Symbol|undefined { const completions = - this.templateTypeChecker.getGlobalCompletions(this.template, this.component, this.node); + this.templateTypeChecker.getGlobalCompletions(this.template, this.component); if (completions === null) { return undefined; } @@ -656,25 +638,6 @@ export class CompletionBuilder { isNewIdentifierLocation: false, }; } - - /** - * From the AST node of the cursor position, include completion of string literals, number - * literals, `true`, `false`, `null`, and `undefined`. - */ - private isValidNodeContextCompletion(completion: ts.CompletionEntry): boolean { - if (completion.kind === ts.ScriptElementKind.string) { - // 'string' kind includes both string literals and number literals - return true; - } - if (completion.kind === ts.ScriptElementKind.keyword) { - return completion.name === 'true' || completion.name === 'false' || - completion.name === 'null'; - } - if (completion.kind === ts.ScriptElementKind.variableElement) { - return completion.name === 'undefined'; - } - return false; - } } function makeReplacementSpanFromParseSourceSpan(span: ParseSourceSpan): ts.TextSpan { diff --git a/packages/language-service/ivy/test/completions_spec.ts b/packages/language-service/ivy/test/completions_spec.ts index 5fa39aa5c8..b881a614e7 100644 --- a/packages/language-service/ivy/test/completions_spec.ts +++ b/packages/language-service/ivy/test/completions_spec.ts @@ -24,18 +24,6 @@ const DIR_WITH_INPUT = { ` }; -const DIR_WITH_UNION_TYPE_INPUT = { - 'Dir': ` - @Directive({ - selector: '[dir]', - inputs: ['myInput'] - }) - export class Dir { - myInput!: 'foo'|42|null|undefined - } - ` -}; - const DIR_WITH_OUTPUT = { 'Dir': ` @Directive({ @@ -215,18 +203,6 @@ describe('completions', () => { const completions = templateFile.getCompletionsAtPosition(); expectContain(completions, ts.ScriptElementKind.memberVariableElement, ['title']); }); - - it('should return completions of string literals, number literals, `true`, `false`, `null` and `undefined`', - () => { - const {templateFile} = setup(``, '', DIR_WITH_UNION_TYPE_INPUT); - templateFile.moveCursorToText('dir [myInput]="¦">'); - - const completions = templateFile.getCompletionsAtPosition(); - expectContain(completions, ts.ScriptElementKind.string, [`'foo'`, '42']); - expectContain(completions, ts.ScriptElementKind.keyword, ['null']); - expectContain(completions, ts.ScriptElementKind.variableElement, ['undefined']); - expectDoesNotContain(completions, ts.ScriptElementKind.parameterElement, ['ctx']); - }); }); describe('in an expression scope', () => { @@ -813,7 +789,7 @@ function setup( export class AppCmp { ${classContents} } - + ${otherDirectiveClassDecls} @NgModule({ @@ -846,7 +822,7 @@ function setupInlineTemplate( export class AppCmp { ${classContents} } - + ${otherDirectiveClassDecls} @NgModule({