From b608fa55f0325e238566fcdf32d26d28f799f53f Mon Sep 17 00:00:00 2001
From: Ayaz Hafiz <ayazhafiz@lyft.com>
Date: Wed, 29 Jan 2020 10:17:36 -0800
Subject: [PATCH] fix(language-service): warn, not error, on missing context
 members (#35036)

The language service reports an error when a directive's template
context is missing a member that is being used in a template (e.g. if
`$implicit` is being used with a template context typed as `any`).
While this diagnostic message is valuable, typing template contexts
loosely as `any` or `object` is very widespread in community packages,
and often still compiles correctly, so reporting the diagnostic as an
error may be misleading to users.

This commit changes the diagnostic to be a warning, and adds additional
information about how the user can eliminate the warning entirely -- by
refining the template context type.

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

PR Close #35036
---
 .../src/expression_diagnostics.ts             | 24 +++++++------------
 .../language-service/test/diagnostics_spec.ts | 12 ++++++----
 2 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/packages/language-service/src/expression_diagnostics.ts b/packages/language-service/src/expression_diagnostics.ts
index 1d5023d57e..b9f717a0d0 100644
--- a/packages/language-service/src/expression_diagnostics.ts
+++ b/packages/language-service/src/expression_diagnostics.ts
@@ -292,16 +292,12 @@ class ExpressionDiagnosticsVisitor extends RecursiveTemplateAstVisitor {
     if (directive && ast.value) {
       const context = this.info.query.getTemplateContext(directive.type.reference) !;
       if (context && !context.has(ast.value)) {
-        if (ast.value === '$implicit') {
-          this.reportError(
-              `The template context of '${directive.type.reference.name}' does not define an implicit value.\n` +
-                  `If the context type is a base type, consider refining it to a more specific type.`,
-              spanOf(ast.sourceSpan));
-        } else {
-          this.reportError(
-              `The template context of '${directive.type.reference.name}' does not define a member called '${ast.value}'`,
-              spanOf(ast.sourceSpan));
-        }
+        const missingMember =
+            ast.value === '$implicit' ? 'an implicit value' : `a member called '${ast.value}'`;
+        this.reportDiagnostic(
+            `The template context of '${directive.type.reference.name}' does not define ${missingMember}.\n` +
+                `If the context type is a base type or 'any', consider refining it to a more specific type.`,
+            spanOf(ast.sourceSpan), ts.DiagnosticCategory.Suggestion);
       }
     }
   }
@@ -353,11 +349,9 @@ class ExpressionDiagnosticsVisitor extends RecursiveTemplateAstVisitor {
 
   private pop() { this.path.pop(); }
 
-  private reportError(message: string, span: Span|undefined) {
-    if (span) {
-      this.diagnostics.push(
-          {span: offsetSpan(span, this.info.offset), kind: ts.DiagnosticCategory.Error, message});
-    }
+  private reportDiagnostic(
+      message: string, span: Span, kind: ts.DiagnosticCategory = ts.DiagnosticCategory.Error) {
+    this.diagnostics.push({span: offsetSpan(span, this.info.offset), kind, message});
   }
 }
 
diff --git a/packages/language-service/test/diagnostics_spec.ts b/packages/language-service/test/diagnostics_spec.ts
index 1340bd8c81..3cccaa1f2b 100644
--- a/packages/language-service/test/diagnostics_spec.ts
+++ b/packages/language-service/test/diagnostics_spec.ts
@@ -257,11 +257,12 @@ describe('diagnostics', () => {
           `<button type="button" ~{start-emb}*counter="let hero of heroes"~{end-emb}></button>`);
       const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
       expect(diags.length).toBe(1);
-      const {messageText, start, length} = diags[0];
+      const {messageText, start, length, category} = diags[0];
+      expect(category).toBe(ts.DiagnosticCategory.Suggestion);
       expect(messageText)
           .toBe(
               `The template context of 'CounterDirective' does not define an implicit value.\n` +
-                  `If the context type is a base type, consider refining it to a more specific type.`, );
+                  `If the context type is a base type or 'any', consider refining it to a more specific type.`, );
 
       const span = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'emb');
       expect(start).toBe(span.start);
@@ -274,9 +275,12 @@ describe('diagnostics', () => {
           `<div ~{start-emb}*ngFor="let hero of heroes; let e = even_1"~{end-emb}></div>`);
       const diags = ngLS.getSemanticDiagnostics(TEST_TEMPLATE);
       expect(diags.length).toBe(1);
-      const {messageText, start, length} = diags[0];
+      const {messageText, start, length, category} = diags[0];
+      expect(category).toBe(ts.DiagnosticCategory.Suggestion);
       expect(messageText)
-          .toBe(`The template context of 'NgForOf' does not define a member called 'even_1'`);
+          .toBe(
+              `The template context of 'NgForOf' does not define a member called 'even_1'.\n` +
+              `If the context type is a base type or 'any', consider refining it to a more specific type.`);
 
       const span = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'emb');
       expect(start).toBe(span.start);