From da4eb9128302bc173cb3b933a40d911efdfde36d Mon Sep 17 00:00:00 2001 From: ayazhafiz Date: Thu, 24 Oct 2019 21:29:24 -0400 Subject: [PATCH] feat(language-service): add support for text replacement (#33091) Adds a `replacementSpan` field on a completion that will allow typed text to be replaced with the suggested completion value if a user selects the completion. Previously, the completion value would simply be appended to the text already typed. E.g. if we had ``` {{ti}} ``` typed in a template and `title` was recommended as a completion and selected, the template would become ``` {{tititle}} ``` With `replacementSpan`, the original text `ti` will be replaced for `title`. PR Close #33091 --- packages/language-service/src/completions.ts | 79 +++++++- .../language-service/test/completions_spec.ts | 189 +++++++++++++++++- .../language-service/test/ts_plugin_spec.ts | 1 + 3 files changed, 267 insertions(+), 2 deletions(-) diff --git a/packages/language-service/src/completions.ts b/packages/language-service/src/completions.ts index 5054fd7d711cd..cebaa6a23c592 100644 --- a/packages/language-service/src/completions.ts +++ b/packages/language-service/src/completions.ts @@ -8,6 +8,7 @@ import {AST, AstPath, Attribute, BoundDirectivePropertyAst, BoundElementPropertyAst, BoundEventAst, BoundTextAst, CssSelector, Element, ElementAst, ImplicitReceiver, NAMED_ENTITIES, Node as HtmlAst, NullTemplateVisitor, ParseSpan, PropertyRead, TagContentType, Text, findNode, getHtmlTagDefinition} from '@angular/compiler'; import {getExpressionScope} from '@angular/compiler-cli/src/language_services'; +import {$$, $_, isAsciiLetter, isDigit} from '@angular/compiler/src/chars'; import {AstResult} from './common'; import {getExpressionCompletions} from './expressions'; @@ -45,6 +46,76 @@ const ANGULAR_ELEMENTS: ReadonlyArray = [ }, ]; +function isIdentifierPart(code: number) { + // Identifiers consist of alphanumeric characters, '_', or '$'. + return isAsciiLetter(code) || isDigit(code) || code == $$ || code == $_; +} + +/** + * Gets the span of word in a template that surrounds `position`. If there is no word around + * `position`, nothing is returned. + */ +function getBoundedWordSpan(templateInfo: AstResult, position: number): ts.TextSpan|undefined { + const {template} = templateInfo; + const templateSrc = template.source; + + if (!templateSrc) return; + + // TODO(ayazhafiz): A solution based on word expansion will always be expensive compared to one + // based on ASTs. Whatever penalty we incur is probably manageable for small-length (i.e. the + // majority of) identifiers, but the current solution involes a number of branchings and we can't + // control potentially very long identifiers. Consider moving to an AST-based solution once + // existing difficulties with AST spans are more clearly resolved (see #31898 for discussion of + // known problems, and #33091 for how they affect text replacement). + // + // `templatePosition` represents the right-bound location of a cursor in the template. + // key.ent|ry + // ^---- cursor, at position `r` is at. + // A cursor is not itself a character in the template; it has a left (lower) and right (upper) + // index bound that hugs the cursor itself. + let templatePosition = position - template.span.start; + // To perform word expansion, we want to determine the left and right indices that hug the cursor. + // There are three cases here. + let left, right; + if (templatePosition === 0) { + // 1. Case like + // |rest of template + // the cursor is at the start of the template, hugged only by the right side (0-index). + left = right = 0; + } else if (templatePosition === templateSrc.length) { + // 2. Case like + // rest of template| + // the cursor is at the end of the template, hugged only by the left side (last-index). + left = right = templateSrc.length - 1; + } else { + // 3. Case like + // wo|rd + // there is a clear left and right index. + left = templatePosition - 1; + right = templatePosition; + } + + if (!isIdentifierPart(templateSrc.charCodeAt(left)) && + !isIdentifierPart(templateSrc.charCodeAt(right))) { + // Case like + // .|. + // left ---^ ^--- right + // There is no word here. + return; + } + + // Expand on the left and right side until a word boundary is hit. Back up one expansion on both + // side to stay inside the word. + while (left >= 0 && isIdentifierPart(templateSrc.charCodeAt(left))) --left; + ++left; + while (right < templateSrc.length && isIdentifierPart(templateSrc.charCodeAt(right))) ++right; + --right; + + const absoluteStartPosition = position - (templatePosition - left); + const length = right - left + 1; + return {start: absoluteStartPosition, length}; +} + export function getTemplateCompletions( templateInfo: AstResult, position: number): ng.CompletionEntry[] { let result: ng.CompletionEntry[] = []; @@ -110,7 +181,13 @@ export function getTemplateCompletions( }, null); } - return result; + + const replacementSpan = getBoundedWordSpan(templateInfo, position); + return result.map(entry => { + return { + ...entry, replacementSpan, + }; + }); } function attributeCompletions(info: AstResult, path: AstPath): ng.CompletionEntry[] { diff --git a/packages/language-service/test/completions_spec.ts b/packages/language-service/test/completions_spec.ts index 42ce895fb2925..6795b97a50909 100644 --- a/packages/language-service/test/completions_spec.ts +++ b/packages/language-service/test/completions_spec.ts @@ -9,7 +9,7 @@ import * as ts from 'typescript'; import {createLanguageService} from '../src/language_service'; -import {CompletionKind} from '../src/types'; +import {CompletionKind, LanguageService} from '../src/types'; import {TypeScriptServiceHost} from '../src/typescript_host'; import {MockTypescriptHost} from './test_utils'; @@ -372,6 +372,193 @@ describe('completions', () => { }); }); +describe('replace completions correctly', () => { + const mockHost = new MockTypescriptHost(['/app/main.ts']); + let ngLS: LanguageService; + + beforeEach(() => { + mockHost.reset(); + const tsLS = ts.createLanguageService(mockHost); + const ngHost = new TypeScriptServiceHost(mockHost, tsLS); + ngLS = createLanguageService(ngHost); + }); + + it('should not generate replacement entries for zero-length replacements', () => { + const fileName = mockHost.addCode(` + @Component({ + selector: 'foo-component', + template: \` +
{{obj.~{key}}}
+ \`, + }) + export class FooComponent { + obj: {key: 'value'}; + } + `); + const location = mockHost.getLocationMarkerFor(fileName, 'key'); + const completions = ngLS.getCompletionsAt(fileName, location.start) !; + expect(completions).toBeDefined(); + const completion = completions.entries.find(entry => entry.name === 'key') !; + expect(completion).toBeDefined(); + expect(completion.kind).toBe('property'); + expect(completion.replacementSpan).toBeUndefined(); + }); + + it('should work for start of template', () => { + const fileName = mockHost.addCode(` + @Component({ + selector: 'foo-component', + template: \`~{start}abc\`, + }) + export class FooComponent {} + `); + const location = mockHost.getLocationMarkerFor(fileName, 'start'); + const completions = ngLS.getCompletionsAt(fileName, location.start) !; + expect(completions).toBeDefined(); + const completion = completions.entries.find(entry => entry.name === 'acronym') !; + expect(completion).toBeDefined(); + expect(completion.kind).toBe('html element'); + expect(completion.replacementSpan).toEqual({start: location.start, length: 3}); + }); + + it('should work for end of template', () => { + const fileName = mockHost.addCode(` + @Component({ + selector: 'foo-component', + template: \`acro~{end}\`, + }) + export class FooComponent {} + `); + const location = mockHost.getLocationMarkerFor(fileName, 'end'); + const completions = ngLS.getCompletionsAt(fileName, location.start) !; + expect(completions).toBeDefined(); + const completion = completions.entries.find(entry => entry.name === 'acronym') !; + expect(completion).toBeDefined(); + expect(completion.kind).toBe('html element'); + expect(completion.replacementSpan).toEqual({start: location.start - 4, length: 4}); + }); + + it('should work for middle-word replacements', () => { + const fileName = mockHost.addCode(` + @Component({ + selector: 'foo-component', + template: \` +
{{obj.ke~{key}key}}
+ \`, + }) + export class FooComponent { + obj: {key: 'value'}; + } + `); + const location = mockHost.getLocationMarkerFor(fileName, 'key'); + const completions = ngLS.getCompletionsAt(fileName, location.start) !; + expect(completions).toBeDefined(); + const completion = completions.entries.find(entry => entry.name === 'key') !; + expect(completion).toBeDefined(); + expect(completion.kind).toBe('property'); + expect(completion.replacementSpan).toEqual({start: location.start - 2, length: 5}); + }); + + it('should work for all kinds of identifier characters', () => { + const fileName = mockHost.addCode(` + @Component({ + selector: 'foo-component', + template: \` +
{{~{field}$title_1}}
+ \`, + }) + export class FooComponent { + $title_1: string; + } + `); + const location = mockHost.getLocationMarkerFor(fileName, 'field'); + const completions = ngLS.getCompletionsAt(fileName, location.start) !; + expect(completions).toBeDefined(); + const completion = completions.entries.find(entry => entry.name === '$title_1') !; + expect(completion).toBeDefined(); + expect(completion.kind).toBe('property'); + expect(completion.replacementSpan).toEqual({start: location.start, length: 8}); + }); + + it('should work for attributes', () => { + const fileName = mockHost.addCode(` + @Component({ + selector: 'foo-component', + template: \` +
+ \`, + }) + export class FooComponent {} + `); + const location = mockHost.getLocationMarkerFor(fileName, 'click'); + const completions = ngLS.getCompletionsAt(fileName, location.start) !; + expect(completions).toBeDefined(); + const completion = completions.entries.find(entry => entry.name === '(click)') !; + expect(completion).toBeDefined(); + expect(completion.kind).toBe('attribute'); + expect(completion.replacementSpan).toEqual({start: location.start - 2, length: 2}); + }); + + it('should work for events', () => { + const fileName = mockHost.addCode(` + @Component({ + selector: 'foo-component', + template: \` +
+ \`, + }) + export class FooComponent { + handleClick() {} + } + `); + const location = mockHost.getLocationMarkerFor(fileName, 'handleClick'); + const completions = ngLS.getCompletionsAt(fileName, location.start) !; + expect(completions).toBeDefined(); + const completion = completions.entries.find(entry => entry.name === 'handleClick') !; + expect(completion).toBeDefined(); + expect(completion.kind).toBe('method'); + expect(completion.replacementSpan).toEqual({start: location.start - 3, length: 3}); + }); + + it('should work for element names', () => { + const fileName = mockHost.addCode(` + @Component({ + selector: 'foo-component', + template: \` + + \`, + }) + export class FooComponent {} + `); + const location = mockHost.getLocationMarkerFor(fileName, 'div'); + const completions = ngLS.getCompletionsAt(fileName, location.start) !; + expect(completions).toBeDefined(); + const completion = completions.entries.find(entry => entry.name === 'div') !; + expect(completion).toBeDefined(); + expect(completion.kind).toBe('html element'); + expect(completion.replacementSpan).toEqual({start: location.start - 2, length: 2}); + }); + + it('should work for bindings', () => { + const fileName = mockHost.addCode(` + @Component({ + selector: 'foo-component', + template: \` + + \`, + }) + export class FooComponent {} + `); + const location = mockHost.getLocationMarkerFor(fileName, 'model'); + const completions = ngLS.getCompletionsAt(fileName, location.start) !; + expect(completions).toBeDefined(); + const completion = completions.entries.find(entry => entry.name === '[(ngModel)]') !; + expect(completion).toBeDefined(); + expect(completion.kind).toBe('attribute'); + expect(completion.replacementSpan).toEqual({start: location.start - 5, length: 5}); + }); +}); + function expectContain( completions: ts.CompletionInfo | undefined, kind: CompletionKind, names: string[]) { expect(completions).toBeDefined(); diff --git a/packages/language-service/test/ts_plugin_spec.ts b/packages/language-service/test/ts_plugin_spec.ts index 6cadb918306b5..740614af67fa9 100644 --- a/packages/language-service/test/ts_plugin_spec.ts +++ b/packages/language-service/test/ts_plugin_spec.ts @@ -127,6 +127,7 @@ describe('plugin', () => { name: 'children', kind: CompletionKind.PROPERTY as any, sortText: 'children', + replacementSpan: {start: 182, length: 8}, }, ]); });