Skip to content

Commit

Permalink
fix(language-service): Do not show HTML elements and attrs for ext te…
Browse files Browse the repository at this point in the history 
…mplate (#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 angular/vscode-ng-language-service#370

PR Close #33388
  • Loading branch information
@kyliau @AndrewKushnir
kyliau authored and AndrewKushnir committed Oct 25, 2019
1 parent 6323a35 commit a78b701
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 69 deletions.
116 changes: 61 additions & 55 deletions packages/language-service/src/completions.ts
View file
Expand Up @@ -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<string> = ['ng-container', 'ng-content', 'ng-template'];
const HIDDEN_HTML_ELEMENTS: ReadonlySet<string> =
new Set(['html', 'script', 'noscript', 'base', 'body', 'title', 'head', 'link']);
const HTML_ELEMENTS: ReadonlyArray<ng.CompletionEntry> =
elementNames().filter(name => !HIDDEN_HTML_ELEMENTS.has(name)).map(name => {
return {
name,
kind: ng.CompletionKind.HTML_ELEMENT,
sortText: name,
};
});
const ANGULAR_ELEMENTS: ReadonlyArray<ng.CompletionEntry> = [
{
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[] {
Expand All @@ -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(
Expand All @@ -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.
Expand Down Expand Up @@ -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);
}
}
},
Expand All @@ -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
Expand Down Expand Up @@ -166,38 +183,29 @@ function attributeValueCompletions(
return visitor.result || [];
}

function elementCompletions(info: AstResult, path: AstPath<HtmlAst>): 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<string>();
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;
}

/**
Expand Down Expand Up @@ -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));
}
Expand Down
1 change: 1 addition & 0 deletions packages/language-service/src/types.ts
View file
Expand Up @@ -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',
Expand Down
33 changes: 19 additions & 14 deletions packages/language-service/test/completions_spec.ts
View file
Expand Up @@ -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']);
}
});

Expand Down Expand Up @@ -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);
Expand All @@ -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', () => {
Expand Down Expand Up @@ -166,16 +166,21 @@ describe('completions', () => {
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'];
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, [
Expand All @@ -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', () => {
Expand Down

0 comments on commit a78b701

Please sign in to comment.