From 1425e630292e2c2167c567b9f71a923d0605224d Mon Sep 17 00:00:00 2001
From: ayazhafiz <ayaz.hafiz.1@gmail.com>
Date: Wed, 27 Nov 2019 11:29:37 -0600
Subject: [PATCH] fix(language-service): determine correct type for ngFor
 exported values (#34089)

Currently, variables of an unknown type in an `*ngFor` expression are
refined to have the type of the iterable binding of the `*ngFor`
expression. Unfortunately, this is a bug for variables aliasing
[values exported by
`*ngFor`](https://angular.io/api/common/NgForOf#local-variables),
including `index` and `first`, because they are also given the type of
the binding expression, but they are not of the binding type. For
example, in

```typescript
@Component({
  selector: 'test',
  template: `
    <div *ngFor="let hero of heroes; let i = index; let isFirst = first">
      {{ hero }}
    </div>
  `
})
export class TestComponent {
  heroes: Hero[];
}
```

The local variables `i` and `isFirst` are determined to have a type of
`Hero`, when actually their types are `number` and `boolean`,
respectively.

This commit fixes this bug by checking if the value of a variable in an
`*ngFor` expression is known to be an export and assigning the variable
the type of that export value. Only if the variable does not alias an
export is it typed with the binding value of the `*ngFor` expression.

Closes https://github.com/angular/vscode-ng-language-service/issues/460

PR Close #34089
---
 .../src/expression_diagnostics.ts             | 40 +++++++++++++++----
 .../language-service/test/diagnostics_spec.ts | 25 ++++++++++++
 2 files changed, 58 insertions(+), 7 deletions(-)

diff --git a/packages/language-service/src/expression_diagnostics.ts b/packages/language-service/src/expression_diagnostics.ts
index 8acc2db926..159a014120 100644
--- a/packages/language-service/src/expression_diagnostics.ts
+++ b/packages/language-service/src/expression_diagnostics.ts
@@ -115,7 +115,7 @@ function getVarDeclarations(
           // that have been declared so far are also in scope.
           info.query.createSymbolTable(results),
         ]);
-        symbol = refinedVariableType(symbolsInScope, info.query, current);
+        symbol = refinedVariableType(variable.value, symbolsInScope, info.query, current);
       }
       results.push({
         name: variable.name,
@@ -127,16 +127,37 @@ function getVarDeclarations(
   return results;
 }
 
+/**
+ * Gets the type of an ngFor exported value, as enumerated in
+ * https://angular.io/api/common/NgForOfContext
+ * @param value exported value name
+ * @param query type symbol query
+ */
+function getNgForExportedValueType(value: string, query: SymbolQuery): Symbol|undefined {
+  switch (value) {
+    case 'index':
+    case 'count':
+      return query.getBuiltinType(BuiltinType.Number);
+    case 'first':
+    case 'last':
+    case 'even':
+    case 'odd':
+      return query.getBuiltinType(BuiltinType.Boolean);
+  }
+}
+
 /**
  * Resolve a more specific type for the variable in `templateElement` by inspecting
  * all variables that are in scope in the `mergedTable`. This function is a special
  * case for `ngFor` and `ngIf`. If resolution fails, return the `any` type.
+ * @param value variable value name
  * @param mergedTable symbol table for all variables in scope
  * @param query
  * @param templateElement
  */
 function refinedVariableType(
-    mergedTable: SymbolTable, query: SymbolQuery, templateElement: EmbeddedTemplateAst): Symbol {
+    value: string, mergedTable: SymbolTable, query: SymbolQuery,
+    templateElement: EmbeddedTemplateAst): Symbol {
   // Special case the ngFor directive
   const ngForDirective = templateElement.directives.find(d => {
     const name = identifierName(d.directive.type);
@@ -145,12 +166,17 @@ function refinedVariableType(
   if (ngForDirective) {
     const ngForOfBinding = ngForDirective.inputs.find(i => i.directiveName == 'ngForOf');
     if (ngForOfBinding) {
+      // Check if the variable value is a type exported by the ngFor statement.
+      let result = getNgForExportedValueType(value, query);
+
+      // Otherwise, check if there is a known type for the ngFor binding.
       const bindingType = new AstType(mergedTable, query, {}).getType(ngForOfBinding.value);
-      if (bindingType) {
-        const result = query.getElementType(bindingType);
-        if (result) {
-          return result;
-        }
+      if (!result && bindingType) {
+        result = query.getElementType(bindingType);
+      }
+
+      if (result) {
+        return result;
       }
     }
   }
diff --git a/packages/language-service/test/diagnostics_spec.ts b/packages/language-service/test/diagnostics_spec.ts
index 5a21eb43a2..2317a85382 100644
--- a/packages/language-service/test/diagnostics_spec.ts
+++ b/packages/language-service/test/diagnostics_spec.ts
@@ -119,6 +119,31 @@ describe('diagnostics', () => {
     expect(diagnostics).toEqual([]);
   });
 
+  describe('diagnostics for ngFor exported values', () => {
+    it('should report errors for mistmatched exported types', () => {
+      mockHost.override(TEST_TEMPLATE, `
+        <div *ngFor="let hero of heroes; let i = index; let isFirst = first">
+            'i' is a number; 'isFirst' is a boolean
+          {{ i === isFirst }}
+        </div>
+      `);
+      const diags = ngLS.getDiagnostics(TEST_TEMPLATE);
+      expect(diags.length).toBe(1);
+      expect(diags[0].messageText).toBe(`Expected the operants to be of similar type or any`);
+    });
+
+    it('should not report errors for matching exported type', () => {
+      mockHost.override(TEST_TEMPLATE, `
+        <div *ngFor="let hero of heroes; let i = index">
+            'i' is a number
+          {{ i < 2 }}
+        </div>
+      `);
+      const diags = ngLS.getDiagnostics(TEST_TEMPLATE);
+      expect(diags.length).toBe(0);
+    });
+  });
+
   describe('diagnostics for invalid indexed type property access', () => {
     it('should work with numeric index signatures (arrays)', () => {
       mockHost.override(TEST_TEMPLATE, `