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
This commit is contained in:
Keen Yee Liau 2019-10-24 14:48:43 -07:00 committed by Andrew Kushnir
parent 6323a35468
commit a78b70178e
3 changed files with 81 additions and 69 deletions

View File

@ -12,23 +12,38 @@ import {getExpressionScope} from '@angular/compiler-cli/src/language_services';
import {AstResult} from './common'; import {AstResult} from './common';
import {getExpressionCompletions} from './expressions'; import {getExpressionCompletions} from './expressions';
import {attributeNames, elementNames, eventNames, propertyNames} from './html_info'; import {attributeNames, elementNames, eventNames, propertyNames} from './html_info';
import {InlineTemplate} from './template';
import * as ng from './types'; import * as ng from './types';
import {diagnosticInfoFromTemplateInfo, findTemplateAstAt, getSelectors, hasTemplateReference, inSpan, spanOf} from './utils'; import {diagnosticInfoFromTemplateInfo, findTemplateAstAt, getSelectors, hasTemplateReference, inSpan, spanOf} from './utils';
const TEMPLATE_ATTR_PREFIX = '*'; const TEMPLATE_ATTR_PREFIX = '*';
const HIDDEN_HTML_ELEMENTS: ReadonlySet<string> =
const hiddenHtmlElements = { new Set(['html', 'script', 'noscript', 'base', 'body', 'title', 'head', 'link']);
html: true, const HTML_ELEMENTS: ReadonlyArray<ng.CompletionEntry> =
script: true, elementNames().filter(name => !HIDDEN_HTML_ELEMENTS.has(name)).map(name => {
noscript: true, return {
base: true, name,
body: true, kind: ng.CompletionKind.HTML_ELEMENT,
title: true, sortText: name,
head: true, };
link: true, });
}; const ANGULAR_ELEMENTS: ReadonlyArray<ng.CompletionEntry> = [
{
const ANGULAR_ELEMENTS: ReadonlyArray<string> = ['ng-container', 'ng-content', 'ng-template']; 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( export function getTemplateCompletions(
templateInfo: AstResult, position: number): ng.CompletionEntry[] { templateInfo: AstResult, position: number): ng.CompletionEntry[] {
@ -39,7 +54,7 @@ export function getTemplateCompletions(
const path = findNode(htmlAst, templatePosition); const path = findNode(htmlAst, templatePosition);
const mostSpecific = path.tail; const mostSpecific = path.tail;
if (path.empty || !mostSpecific) { if (path.empty || !mostSpecific) {
result = elementCompletions(templateInfo, path); result = elementCompletions(templateInfo);
} else { } else {
const astPosition = templatePosition - mostSpecific.sourceSpan.start.offset; const astPosition = templatePosition - mostSpecific.sourceSpan.start.offset;
mostSpecific.visit( mostSpecific.visit(
@ -50,7 +65,7 @@ export function getTemplateCompletions(
// + 1 for the opening angle bracket // + 1 for the opening angle bracket
if (templatePosition <= startTagSpan.start + tagLen + 1) { if (templatePosition <= startTagSpan.start + tagLen + 1) {
// If we are in the tag then return the element completions. // If we are in the tag then return the element completions.
result = elementCompletions(templateInfo, path); result = elementCompletions(templateInfo);
} else if (templatePosition < startTagSpan.end) { } else if (templatePosition < startTagSpan.end) {
// We are in the attribute section of the element (but not in an attribute). // We are in the attribute section of the element (but not in an attribute).
// Return the attribute completions. // Return the attribute completions.
@ -78,14 +93,14 @@ export function getTemplateCompletions(
result = voidElementAttributeCompletions(templateInfo, path); result = voidElementAttributeCompletions(templateInfo, path);
if (!result.length) { if (!result.length) {
// If the element can hold content, show element completions. // If the element can hold content, show element completions.
result = elementCompletions(templateInfo, path); result = elementCompletions(templateInfo);
} }
} }
} else { } else {
// If no element container, implies parsable data so show elements. // If no element container, implies parsable data so show elements.
result = voidElementAttributeCompletions(templateInfo, path); result = voidElementAttributeCompletions(templateInfo, path);
if (!result.length) { if (!result.length) {
result = elementCompletions(templateInfo, path); result = elementCompletions(templateInfo);
} }
} }
}, },
@ -110,13 +125,15 @@ function attributeCompletionsForElement(
info: AstResult, elementName: string): ng.CompletionEntry[] { info: AstResult, elementName: string): ng.CompletionEntry[] {
const results: ng.CompletionEntry[] = []; const results: ng.CompletionEntry[] = [];
// Add html attributes if (info.template instanceof InlineTemplate) {
for (const name of attributeNames(elementName)) { // Provide HTML attributes completion only for inline templates
results.push({ for (const name of attributeNames(elementName)) {
name, results.push({
kind: ng.CompletionKind.HTML_ATTRIBUTE, name,
sortText: name, kind: ng.CompletionKind.HTML_ATTRIBUTE,
}); sortText: name,
});
}
} }
// Add html properties // Add html properties
@ -166,38 +183,29 @@ function attributeValueCompletions(
return visitor.result || []; return visitor.result || [];
} }
function elementCompletions(info: AstResult, path: AstPath<HtmlAst>): ng.CompletionEntry[] { function elementCompletions(info: AstResult): ng.CompletionEntry[] {
const htmlNames = elementNames().filter(name => !(name in hiddenHtmlElements)); const results: ng.CompletionEntry[] = [...ANGULAR_ELEMENTS];
if (info.template instanceof InlineTemplate) {
// Provide HTML elements completion only for inline templates
results.push(...HTML_ELEMENTS);
}
// Collect the elements referenced by the selectors // Collect the elements referenced by the selectors
const directiveElements = getSelectors(info) const components = new Set<string>();
.selectors.map(selector => selector.element) for (const selector of getSelectors(info).selectors) {
.filter(name => !!name) as string[]; const name = selector.element;
if (name && !components.has(name)) {
components.add(name);
results.push({
name,
kind: ng.CompletionKind.COMPONENT,
sortText: name,
});
}
}
const components = directiveElements.map(name => { return results;
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,
};
});
// Return components and html elements
return uniqueByName([...htmlElements, ...components, ...angularElements]);
} }
/** /**
@ -419,8 +427,6 @@ function getSourceText(template: ng.TemplateSource, span: ng.Span): string {
return template.source.substring(span.start, span.end); return template.source.substring(span.start, span.end);
} }
const templateAttr = /^(\w+:)?(template$|^\*)/;
function lowerName(name: string): string { function lowerName(name: string): string {
return name && (name[0].toLowerCase() + name.substr(1)); return name && (name[0].toLowerCase() + name.substr(1));
} }

View File

@ -268,6 +268,7 @@ export enum CompletionKind {
ELEMENT = 'element', ELEMENT = 'element',
ENTITY = 'entity', ENTITY = 'entity',
HTML_ATTRIBUTE = 'html attribute', HTML_ATTRIBUTE = 'html attribute',
HTML_ELEMENT = 'html element',
KEY = 'key', KEY = 'key',
METHOD = 'method', METHOD = 'method',
PIPE = 'pipe', PIPE = 'pipe',

View File

@ -36,7 +36,7 @@ describe('completions', () => {
for (const location of locations) { for (const location of locations) {
const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, location); const marker = mockHost.getLocationMarkerFor(APP_COMPONENT, location);
const completions = ngLS.getCompletionsAt(APP_COMPONENT, marker.start); 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', () => { it('should be able to return attributes of an incomplete element', () => {
const m1 = mockHost.getLocationMarkerFor(PARSING_CASES, 'incomplete-open-lt'); const m1 = mockHost.getLocationMarkerFor(PARSING_CASES, 'incomplete-open-lt');
const c1 = ngLS.getCompletionsAt(PARSING_CASES, m1.start); 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 m2 = mockHost.getLocationMarkerFor(PARSING_CASES, 'incomplete-open-a');
const c2 = ngLS.getCompletionsAt(PARSING_CASES, m2.start); 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 m3 = mockHost.getLocationMarkerFor(PARSING_CASES, 'incomplete-open-attr');
const c3 = ngLS.getCompletionsAt(PARSING_CASES, m3.start); 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', () => { it('should be able to return completions with a missing closing tag', () => {
const marker = mockHost.getLocationMarkerFor(PARSING_CASES, 'missing-closing'); const marker = mockHost.getLocationMarkerFor(PARSING_CASES, 'missing-closing');
const completions = ngLS.getCompletionsAt(PARSING_CASES, marker.start); 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', () => { it('should be able to return common attributes of an unknown tag', () => {
@ -166,16 +166,21 @@ describe('completions', () => {
expectContain(completions, CompletionKind.ENTITY, ['&amp;', '&gt;', '&lt;', '&iota;']); expectContain(completions, CompletionKind.ENTITY, ['&amp;', '&gt;', '&lt;', '&iota;']);
}); });
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']; const locations = ['empty', 'start-tag-h1', 'h1-content', 'start-tag', 'start-tag-after-h'];
for (const location of locations) { for (const location of locations) {
const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, location); const marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, location);
const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start); 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 marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'empty');
const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start); const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start);
expectContain(completions, CompletionKind.COMPONENT, [ 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 marker = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'h1-after-space');
const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start); const completions = ngLS.getCompletionsAt(TEST_TEMPLATE, marker.start);
expectContain(completions, CompletionKind.HTML_ATTRIBUTE, [ expect(completions).toBeDefined();
'class', const {entries} = completions !;
'id', expect(entries).not.toContain(jasmine.objectContaining({name: 'class'}));
'onclick', expect(entries).not.toContain(jasmine.objectContaining({name: 'id'}));
'onmouseup', 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', () => { it('should be able to find common angular attributes', () => {