fix(language-service): Only provide Angular property completions in templates (#41278)

When possible, the @angular/language-service should only provide
information related to Angular. When there is an embedded language, like
inline templates, editor extensions should have the ability to create
virtual documents and forward the requests to the relevant providers for
that language type (see https://github.com/angular/vscode-ng-language-service/pull/1212).

This commit removes all dom schema completions in both inline and
external templates and provides only the Angular syntax for property completions
on elements.

PR Close #41278
This commit is contained in:
Andrew Scott 2021-03-17 11:31:57 -07:00 committed by Alex Rickabaugh
parent 3470ea17d5
commit 0226a11c18
4 changed files with 34 additions and 53 deletions

View File

@ -68,9 +68,11 @@ export interface DomAttributeCompletion {
attribute: string;
/**
* Whether this attribute is also a DOM property.
* Whether this attribute is also a DOM property. Note that this is required to be `true` because
* we only want to provide DOM attributes when there is an Angular syntax associated with them
* (`[propertyName]=""`).
*/
isAlsoProperty: boolean;
isAlsoProperty: true;
}
/**
@ -180,8 +182,7 @@ export type AttributeCompletion = DomAttributeCompletion|DomPropertyCompletion|
*/
export function buildAttributeCompletionTable(
component: ts.ClassDeclaration, element: TmplAstElement|TmplAstTemplate,
checker: TemplateTypeChecker,
includeDomSchemaAttributes: boolean): Map<string, AttributeCompletion> {
checker: TemplateTypeChecker): Map<string, AttributeCompletion> {
const table = new Map<string, AttributeCompletion>();
// Use the `ElementSymbol` or `TemplateSymbol` to iterate over directives present on the node, and
@ -333,22 +334,16 @@ export function buildAttributeCompletionTable(
}
// Finally, add any DOM attributes not already covered by inputs.
if (element instanceof TmplAstElement && includeDomSchemaAttributes) {
if (element instanceof TmplAstElement) {
for (const {attribute, property} of checker.getPotentialDomBindings(element.name)) {
const isAlsoProperty = attribute === property;
if (!table.has(attribute)) {
if (!table.has(attribute) && isAlsoProperty) {
table.set(attribute, {
kind: AttributeCompletionKind.DomAttribute,
attribute,
isAlsoProperty,
});
}
if (!isAlsoProperty && !table.has(property)) {
table.set(property, {
kind: AttributeCompletionKind.DomProperty,
property,
});
}
}
}
@ -449,30 +444,14 @@ export function addAttributeCompletionEntries(
break;
}
case AttributeCompletionKind.DomAttribute: {
if (isAttributeContext) {
// Offer a completion of an attribute binding.
entries.push({
kind: unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.ATTRIBUTE),
name: completion.attribute,
sortText: completion.attribute,
replacementSpan,
});
if (completion.isAlsoProperty) {
// Offer a completion of a property binding to the DOM property.
entries.push({
kind: unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.PROPERTY),
name: `[${completion.attribute}]`,
// In the case of DOM attributes, the property binding should sort after the attribute
// binding.
sortText: completion.attribute + '_1',
replacementSpan,
});
}
} else if (completion.isAlsoProperty) {
if (isAttributeContext && completion.isAlsoProperty) {
// Offer a completion of a property binding to the DOM property.
entries.push({
kind: unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.PROPERTY),
name: completion.attribute,
sortText: completion.attribute,
name: `[${completion.attribute}]`,
// In the case of DOM attributes, the property binding should sort after the attribute
// binding.
sortText: completion.attribute + '_1',
replacementSpan,
});
}

View File

@ -57,7 +57,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
constructor(
private readonly tsLS: ts.LanguageService, private readonly compiler: NgCompiler,
private readonly component: ts.ClassDeclaration, private readonly node: N,
private readonly targetDetails: TemplateTarget, private readonly inlineTemplate: boolean) {}
private readonly targetDetails: TemplateTarget) {}
/**
* Analogue for `ts.LanguageService.getCompletionsAtPosition`.
@ -371,11 +371,9 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
const replacementSpan: ts.TextSpan = {start, length};
let potentialTags = Array.from(templateTypeChecker.getPotentialElementTags(this.component));
if (!this.inlineTemplate) {
// If we are in an external template, don't provide non-Angular tags (directive === null)
// because we expect other extensions (i.e. Emmet) to provide those for HTML files.
potentialTags = potentialTags.filter(([_, directive]) => directive !== null);
}
// Don't provide non-Angular tags (directive === null) because we expect other extensions (i.e.
// Emmet) to provide those for HTML files.
potentialTags = potentialTags.filter(([_, directive]) => directive !== null);
const entries: ts.CompletionEntry[] = potentialTags.map(([tag, directive]) => ({
kind: tagCompletionKind(directive),
name: tag,
@ -462,7 +460,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
}
const attrTable = buildAttributeCompletionTable(
this.component, element, this.compiler.getTemplateTypeChecker(), this.inlineTemplate);
this.component, element, this.compiler.getTemplateTypeChecker());
let entries: ts.CompletionEntry[] = [];
@ -536,7 +534,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
}
const attrTable = buildAttributeCompletionTable(
this.component, element, this.compiler.getTemplateTypeChecker(), this.inlineTemplate);
this.component, element, this.compiler.getTemplateTypeChecker());
if (!attrTable.has(name)) {
return undefined;
@ -603,7 +601,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
}
const attrTable = buildAttributeCompletionTable(
this.component, element, this.compiler.getTemplateTypeChecker(), this.inlineTemplate);
this.component, element, this.compiler.getTemplateTypeChecker());
if (!attrTable.has(name)) {
return undefined;

View File

@ -211,8 +211,7 @@ export class LanguageService {
positionDetails.context.nodes[0] :
positionDetails.context.node;
return new CompletionBuilder(
this.tsLS, compiler, templateInfo.component, node, positionDetails,
isTypeScriptFile(fileName));
this.tsLS, compiler, templateInfo.component, node, positionDetails);
}
getCompletionsAtPosition(

View File

@ -308,11 +308,11 @@ describe('completions', () => {
['div', 'span']);
});
it('should return DOM completions', () => {
it('should not return DOM completions for inline template', () => {
const {appFile} = setupInlineTemplate(`<div>`, '');
appFile.moveCursorToText('<div¦>');
const completions = appFile.getCompletionsAtPosition();
expectContain(
expectDoesNotContain(
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.ELEMENT),
['div', 'span']);
});
@ -431,20 +431,25 @@ describe('completions', () => {
describe('element attribute scope', () => {
describe('dom completions', () => {
it('should not return completions dom completions in external template', () => {
it('should return dom property completions in external template', () => {
const {templateFile} = setup(`<input >`, '');
templateFile.moveCursorToText('<input ¦>');
const completions = templateFile.getCompletionsAtPosition();
expect(completions?.entries.length).toBe(0);
expectDoesNotContain(
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.ATTRIBUTE),
['value']);
expectContain(
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.PROPERTY),
['[value]']);
});
it('should return completions for a new element attribute', () => {
it('should return completions for a new element property', () => {
const {appFile} = setupInlineTemplate(`<input >`, '');
appFile.moveCursorToText('<input ¦>');
const completions = appFile.getCompletionsAtPosition();
expectContain(
expectDoesNotContain(
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.ATTRIBUTE),
['value']);
expectContain(
@ -457,7 +462,7 @@ describe('completions', () => {
appFile.moveCursorToText('<input val¦>');
const completions = appFile.getCompletionsAtPosition();
expectContain(
expectDoesNotContain(
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.ATTRIBUTE),
['value']);
expectContain(
@ -477,7 +482,7 @@ describe('completions', () => {
expectDoesNotContain(
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.PROPERTY),
['[value]']);
expectContain(
expectDoesNotContain(
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.PROPERTY),
['value']);
expectReplacementText(completions, appFile.contents, 'val');