From a78b70178e1fbce888eefbeee6d94ce01707fae1 Mon Sep 17 00:00:00 2001 From: Keen Yee Liau Date: Thu, 24 Oct 2019 14:48:43 -0700 Subject: [PATCH] fix(language-service): Do not show HTML elements and attrs for ext template (#33388) This commit removes HTML elements and HTML attributes from the completions list for external template. This is because these completions should be handled by the native HTML extension, and not Angular. Once we setup TextMate grammar for inline templates, we could remove the HTML completions completely. PR closes https://github.com/angular/vscode-ng-language-service/issues/370 PR Close #33388 --- packages/language-service/src/completions.ts | 116 +++++++++--------- packages/language-service/src/types.ts | 1 + .../language-service/test/completions_spec.ts | 33 ++--- 3 files changed, 81 insertions(+), 69 deletions(-) diff --git a/packages/language-service/src/completions.ts b/packages/language-service/src/completions.ts index 297fbf32b2d1e..36824deb9ba82 100644 --- a/packages/language-service/src/completions.ts +++ b/packages/language-service/src/completions.ts @@ -12,23 +12,38 @@ import {getExpressionScope} from '@angular/compiler-cli/src/language_services'; import {AstResult} from './common'; import {getExpressionCompletions} from './expressions'; import {attributeNames, elementNames, eventNames, propertyNames} from './html_info'; +import {InlineTemplate} from './template'; import * as ng from './types'; import {diagnosticInfoFromTemplateInfo, findTemplateAstAt, getSelectors, hasTemplateReference, inSpan, spanOf} from './utils'; const TEMPLATE_ATTR_PREFIX = '*'; - -const hiddenHtmlElements = { - html: true, - script: true, - noscript: true, - base: true, - body: true, - title: true, - head: true, - link: true, -}; - -const ANGULAR_ELEMENTS: ReadonlyArray = ['ng-container', 'ng-content', 'ng-template']; +const HIDDEN_HTML_ELEMENTS: ReadonlySet = + new Set(['html', 'script', 'noscript', 'base', 'body', 'title', 'head', 'link']); +const HTML_ELEMENTS: ReadonlyArray = + elementNames().filter(name => !HIDDEN_HTML_ELEMENTS.has(name)).map(name => { + return { + name, + kind: ng.CompletionKind.HTML_ELEMENT, + sortText: name, + }; + }); +const ANGULAR_ELEMENTS: ReadonlyArray = [ + { + name: 'ng-container', + kind: ng.CompletionKind.ANGULAR_ELEMENT, + sortText: 'ng-container', + }, + { + name: 'ng-content', + kind: ng.CompletionKind.ANGULAR_ELEMENT, + sortText: 'ng-content', + }, + { + name: 'ng-template', + kind: ng.CompletionKind.ANGULAR_ELEMENT, + sortText: 'ng-template', + }, +]; export function getTemplateCompletions( templateInfo: AstResult, position: number): ng.CompletionEntry[] { @@ -39,7 +54,7 @@ export function getTemplateCompletions( const path = findNode(htmlAst, templatePosition); const mostSpecific = path.tail; if (path.empty || !mostSpecific) { - result = elementCompletions(templateInfo, path); + result = elementCompletions(templateInfo); } else { const astPosition = templatePosition - mostSpecific.sourceSpan.start.offset; mostSpecific.visit( @@ -50,7 +65,7 @@ export function getTemplateCompletions( // + 1 for the opening angle bracket if (templatePosition <= startTagSpan.start + tagLen + 1) { // If we are in the tag then return the element completions. - result = elementCompletions(templateInfo, path); + result = elementCompletions(templateInfo); } else if (templatePosition < startTagSpan.end) { // We are in the attribute section of the element (but not in an attribute). // Return the attribute completions. @@ -78,14 +93,14 @@ export function getTemplateCompletions( result = voidElementAttributeCompletions(templateInfo, path); if (!result.length) { // If the element can hold content, show element completions. - result = elementCompletions(templateInfo, path); + result = elementCompletions(templateInfo); } } } else { // If no element container, implies parsable data so show elements. result = voidElementAttributeCompletions(templateInfo, path); if (!result.length) { - result = elementCompletions(templateInfo, path); + result = elementCompletions(templateInfo); } } }, @@ -110,13 +125,15 @@ function attributeCompletionsForElement( info: AstResult, elementName: string): ng.CompletionEntry[] { const results: ng.CompletionEntry[] = []; - // Add html attributes - for (const name of attributeNames(elementName)) { - results.push({ - name, - kind: ng.CompletionKind.HTML_ATTRIBUTE, - sortText: name, - }); + if (info.template instanceof InlineTemplate) { + // Provide HTML attributes completion only for inline templates + for (const name of attributeNames(elementName)) { + results.push({ + name, + kind: ng.CompletionKind.HTML_ATTRIBUTE, + sortText: name, + }); + } } // Add html properties @@ -166,38 +183,29 @@ function attributeValueCompletions( return visitor.result || []; } -function elementCompletions(info: AstResult, path: AstPath): ng.CompletionEntry[] { - const htmlNames = elementNames().filter(name => !(name in hiddenHtmlElements)); +function elementCompletions(info: AstResult): ng.CompletionEntry[] { + const results: ng.CompletionEntry[] = [...ANGULAR_ELEMENTS]; - // Collect the elements referenced by the selectors - const directiveElements = getSelectors(info) - .selectors.map(selector => selector.element) - .filter(name => !!name) as string[]; + if (info.template instanceof InlineTemplate) { + // Provide HTML elements completion only for inline templates + results.push(...HTML_ELEMENTS); + } - const components = directiveElements.map(name => { - return { - name, - kind: ng.CompletionKind.COMPONENT, - sortText: name, - }; - }); - const htmlElements = htmlNames.map(name => { - return { - name, - kind: ng.CompletionKind.ELEMENT, - sortText: name, - }; - }); - const angularElements = ANGULAR_ELEMENTS.map(name => { - return { - name, - kind: ng.CompletionKind.ANGULAR_ELEMENT, - sortText: name, - }; - }); + // Collect the elements referenced by the selectors + const components = new Set(); + for (const selector of getSelectors(info).selectors) { + const name = selector.element; + if (name && !components.has(name)) { + components.add(name); + results.push({ + name, + kind: ng.CompletionKind.COMPONENT, + sortText: name, + }); + } + } - // Return components and html elements - return uniqueByName([...htmlElements, ...components, ...angularElements]); + return results; } /** @@ -419,8 +427,6 @@ function getSourceText(template: ng.TemplateSource, span: ng.Span): string { return template.source.substring(span.start, span.end); } -const templateAttr = /^(\w+:)?(template$|^\*)/; - function lowerName(name: string): string { return name && (name[0].toLowerCase() + name.substr(1)); } diff --git a/packages/language-service/src/types.ts b/packages/language-service/src/types.ts index c1d6a66d31a0d..a6972fbcc5766 100644 --- a/packages/language-service/src/types.ts +++ b/packages/language-service/src/types.ts @@ -268,6 +268,7 @@ export enum CompletionKind { ELEMENT = 'element', ENTITY = 'entity', HTML_ATTRIBUTE = 'html attribute', + HTML_ELEMENT = 'html element', KEY = 'key', METHOD = 'method', PIPE = 'pipe', diff --git a/packages/language-service/test/completions_spec.ts b/packages/language-service/test/completions_spec.ts index 2ae39da15edcd..42ce895fb2925 100644 --- a/packages/language-service/test/completions_spec.ts +++ b/packages/language-service/test/completions_spec.ts @@ -36,7 +36,7 @@ describe('completions', () => { for (const location of locations) { const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, location); const completions = ngLS.getCompletionsAt(APP_COMPONENT, marker.start); - expectContain(completions, CompletionKind.ELEMENT, ['div', 'h1', 'h2', 'span']); + expectContain(completions, CompletionKind.HTML_ELEMENT, ['div', 'h1', 'h2', 'span']); } }); @@ -124,11 +124,11 @@ describe('completions', () => { it('should be able to return attributes of an incomplete element', () => { const m1 = mockHost.getLocationMarkerFor(PARSING_CASES, 'incomplete-open-lt'); const c1 = ngLS.getCompletionsAt(PARSING_CASES, m1.start); - expectContain(c1, CompletionKind.ELEMENT, ['a', 'div', 'p', 'span']); + expectContain(c1, CompletionKind.HTML_ELEMENT, ['a', 'div', 'p', 'span']); const m2 = mockHost.getLocationMarkerFor(PARSING_CASES, 'incomplete-open-a'); const c2 = ngLS.getCompletionsAt(PARSING_CASES, m2.start); - expectContain(c2, CompletionKind.ELEMENT, ['a', 'div', 'p', 'span']); + expectContain(c2, CompletionKind.HTML_ELEMENT, ['a', 'div', 'p', 'span']); const m3 = mockHost.getLocationMarkerFor(PARSING_CASES, 'incomplete-open-attr'); const c3 = ngLS.getCompletionsAt(PARSING_CASES, m3.start); @@ -138,7 +138,7 @@ describe('completions', () => { it('should be able to return completions with a missing closing tag', () => { const marker = mockHost.getLocationMarkerFor(PARSING_CASES, 'missing-closing'); const completions = ngLS.getCompletionsAt(PARSING_CASES, marker.start); - expectContain(completions, CompletionKind.ELEMENT, ['a', 'div', 'p', 'span', 'h1', 'h2']); + expectContain(completions, CompletionKind.HTML_ELEMENT, ['a', 'div', 'p', 'span', 'h1', 'h2']); }); it('should be able to return common attributes of an unknown tag', () => { @@ -166,16 +166,21 @@ describe('completions', () => { expectContain(completions, CompletionKind.ENTITY, ['&', '>', '<', 'ι']); }); - it('should be able to return html elements', () => { + it('should not return html elements', () => { const locations = ['empty', 'start-tag-h1', 'h1-content', 'start-tag', 'start-tag-after-h']; for (const location of locations) { const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, location); const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start); - expectContain(completions, CompletionKind.ELEMENT, ['div', 'h1', 'h2', 'span']); + expect(completions).toBeDefined(); + const {entries} = completions !; + expect(entries).not.toContain(jasmine.objectContaining({name: 'div'})); + expect(entries).not.toContain(jasmine.objectContaining({name: 'h1'})); + expect(entries).not.toContain(jasmine.objectContaining({name: 'h2'})); + expect(entries).not.toContain(jasmine.objectContaining({name: 'span'})); } }); - it('should be able to return element diretives', () => { + it('should be able to return element directives', () => { const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'empty'); const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start); expectContain(completions, CompletionKind.COMPONENT, [ @@ -186,15 +191,15 @@ describe('completions', () => { ]); }); - it('should be able to return h1 attributes', () => { + it('should not return html attributes', () => { const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'h1-after-space'); const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start); - expectContain(completions, CompletionKind.HTML_ATTRIBUTE, [ - 'class', - 'id', - 'onclick', - 'onmouseup', - ]); + expect(completions).toBeDefined(); + const {entries} = completions !; + expect(entries).not.toContain(jasmine.objectContaining({name: 'class'})); + expect(entries).not.toContain(jasmine.objectContaining({name: 'id'})); + expect(entries).not.toContain(jasmine.objectContaining({name: 'onclick'})); + expect(entries).not.toContain(jasmine.objectContaining({name: 'onmouseup'})); }); it('should be able to find common angular attributes', () => {