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

`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

packages/language-service/src/completions.ts

@@ -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;
+  return visitor.results;
-  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;
 }
 
 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);
   }
 }
 

packages/language-service/src/utils.ts

@@ -101,15 +101,14 @@ export function diagnosticInfoFromTemplateInfo(info: AstResult): DiagnosticTempl
   };
 }
 
-export function findTemplateAstAt(
+export function findTemplateAstAt(ast: TemplateAst[], position: number): TemplateAstPath {
-    ast: TemplateAst[], position: number, allowWidening: boolean = false): 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 {

packages/language-service/test/completions_spec.ts

@@ -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);

packages/language-service/test/project/app/main.ts

@@ -38,7 +38,6 @@ import * as ParsingCases from './parsing-cases';
     ParsingCases.EventBinding,
     ParsingCases.FooComponent,
     ParsingCases.ForLetIEqual,
-    ParsingCases.ForOfEmpty,
     ParsingCases.ForOfLetEmpty,
     ParsingCases.ForUsingComponent,
     ParsingCases.NoValueAttribute,

packages/language-service/test/project/app/parsing-cases.ts

@@ -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>`,

packages/language-service/test/typescript_host_spec.ts

@@ -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', () => {