Skip to content
Permalink
Browse files

fix(language-service): Remove completions for let and of in ngFor (#3…

…4434)

`let` and `of` should be considered reserved keywords in template syntax
and thus should not be part of the autocomplete suggestions.

For reference, TypeScript does not provide such completions.

This commit removes these results and cleans up the code.

PR Close #34434
  • Loading branch information
kyliau authored and kara committed Dec 16, 2019
1 parent 7916b1e commit ab614802e41478cc2202212c3fcc7cf5ffae939f
@@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {AST, AstPath, Attribute, BoundDirectivePropertyAst, BoundElementPropertyAst, BoundEventAst, BoundTextAst, CssSelector, Element, ElementAst, ImplicitReceiver, NAMED_ENTITIES, Node as HtmlAst, NullTemplateVisitor, ParseSpan, PropertyRead, TagContentType, TemplateBinding, Text, findNode, getHtmlTagDefinition} from '@angular/compiler';
import {AST, AstPath, Attribute, BoundDirectivePropertyAst, BoundElementPropertyAst, BoundEventAst, BoundTextAst, Element, ElementAst, ImplicitReceiver, NAMED_ENTITIES, Node as HtmlAst, NullTemplateVisitor, ParseSpan, PropertyRead, TagContentType, TemplateBinding, Text, findNode, getHtmlTagDefinition} from '@angular/compiler';
import {$$, $_, isAsciiLetter, isDigit} from '@angular/compiler/src/chars';

import {AstResult} from './common';
@@ -246,19 +246,7 @@ function attributeValueCompletions(
const visitor =
new ExpressionVisitor(info, position, () => getExpressionScope(dinfo, path, false), attr);
path.tail.visit(visitor, null);
const {results} = visitor;
if (results.length) {
return results;
}
// Try allowing widening the path
const widerPath = findTemplateAstAt(info.templateAst, position, /* allowWidening */ true);
if (widerPath.tail) {
const widerVisitor = new ExpressionVisitor(
info, position, () => getExpressionScope(dinfo, widerPath, false), attr);
widerPath.tail.visit(widerVisitor, null);
return widerVisitor.results;
}
return results;
return visitor.results;
}

function elementCompletions(info: AstResult): ng.CompletionEntry[] {
@@ -415,24 +403,6 @@ class ExpressionVisitor extends NullTemplateVisitor {
}
}

private addKeysToCompletions(selector: CssSelector, key: string) {
if (key !== 'ngFor') {
return;
}
this.completions.set('let', {
name: 'let',
kind: ng.CompletionKind.KEY,
sortText: 'let',
});
if (selector.attrs.some(attr => attr === 'ngForOf')) {
this.completions.set('of', {
name: 'of',
kind: ng.CompletionKind.KEY,
sortText: 'of',
});
}
}

private addSymbolsToCompletions(symbols: ng.Symbol[]) {
for (const s of symbols) {
if (s.name.startsWith('__') || !s.public || this.completions.has(s.name)) {
@@ -507,8 +477,6 @@ class ExpressionVisitor extends NullTemplateVisitor {
this.addAttributeValuesToCompletions(binding.expression.ast, this.position);
return;
}

this.addKeysToCompletions(selector, key);
}
}

@@ -101,15 +101,14 @@ export function diagnosticInfoFromTemplateInfo(info: AstResult): DiagnosticTempl
};
}

export function findTemplateAstAt(
ast: TemplateAst[], position: number, allowWidening: boolean = false): TemplateAstPath {
export function findTemplateAstAt(ast: TemplateAst[], position: number): TemplateAstPath {
const path: TemplateAst[] = [];
const visitor = new class extends RecursiveTemplateAstVisitor {
visit(ast: TemplateAst, context: any): any {
let span = spanOf(ast);
if (inSpan(position, span)) {
const len = path.length;
if (!len || allowWidening || isNarrower(span, spanOf(path[len - 1]))) {
if (!len || isNarrower(span, spanOf(path[len - 1]))) {
path.push(ast);
}
} else {
@@ -296,12 +296,6 @@ describe('completions', () => {
});

describe('with a *ngFor', () => {
it('should include a let for empty attribute', () => {
const marker = mockHost.getLocationMarkerFor(PARSING_CASES, 'for-empty');
const completions = ngLS.getCompletionsAt(PARSING_CASES, marker.start);
expectContain(completions, CompletionKind.KEY, ['let', 'of']);
});

it('should suggest NgForRow members for let initialization expression', () => {
const marker = mockHost.getLocationMarkerFor(PARSING_CASES, 'for-let-i-equal');
const completions = ngLS.getCompletionsAt(PARSING_CASES, marker.start);
@@ -317,18 +311,6 @@ describe('completions', () => {
]);
});

it('should include a let', () => {
const marker = mockHost.getLocationMarkerFor(PARSING_CASES, 'for-let');
const completions = ngLS.getCompletionsAt(PARSING_CASES, marker.start);
expectContain(completions, CompletionKind.KEY, ['let', 'of']);
});

it('should include an "of"', () => {
const marker = mockHost.getLocationMarkerFor(PARSING_CASES, 'for-of');
const completions = ngLS.getCompletionsAt(PARSING_CASES, marker.start);
expectContain(completions, CompletionKind.KEY, ['let', 'of']);
});

it('should include field reference', () => {
const marker = mockHost.getLocationMarkerFor(PARSING_CASES, 'for-people');
const completions = ngLS.getCompletionsAt(PARSING_CASES, marker.start);
@@ -38,7 +38,6 @@ import * as ParsingCases from './parsing-cases';
ParsingCases.EventBinding,
ParsingCases.FooComponent,
ParsingCases.ForLetIEqual,
ParsingCases.ForOfEmpty,
ParsingCases.ForOfLetEmpty,
ParsingCases.ForUsingComponent,
ParsingCases.NoValueAttribute,
@@ -111,12 +111,6 @@ interface Person {
street: string;
}

@Component({
template: '<div *ngFor="~{for-empty}"></div>',
})
export class ForOfEmpty {
}

@Component({
template: '<div *ngFor="let ~{for-let-empty}"></div>',
})
@@ -131,7 +125,7 @@ export class ForLetIEqual {

@Component({
template: `
<div *ngFor="~{for-let}let ~{for-person}person ~{for-of}of ~{for-people}people">
<div *ngFor="let ~{for-person}person of ~{for-people}people">
<span>Name: {{~{for-interp-person}person.~{for-interp-name}name}}</span>
<span>Age: {{person.~{for-interp-age}age}}</span>
</div>`,
@@ -94,7 +94,7 @@ describe('TypeScriptServiceHost', () => {
const tsLS = ts.createLanguageService(tsLSHost);
const ngLSHost = new TypeScriptServiceHost(tsLSHost, tsLS);
const templates = ngLSHost.getTemplates('/app/parsing-cases.ts');
expect(templates.length).toBe(18);
expect(templates.length).toBe(17);
});

it('should be able to find external template', () => {

0 comments on commit ab61480

Please sign in to comment.
You can’t perform that action at this time.