feat(language-service): specific suggestions for template context diags (#34751)

This commit elaborates diagnostics produced for invalid template
contexts by including the name of the embedded template type using the
template context, and in the common case that the implicity property is
being referenced (e.g. in a `for .. of ..` expression), suggesting to
refine the type of the context. This suggestion is provided because
users will sometimes use a base class as the type of the context in the
embedded view, and a more specific context later on (e.g. in an
`ngOnChanges` method).

Closes angular/vscode-ng-language-service#251

PR Close #34751

Author: ayazhafiz

packages/language-service/src/expression_diagnostics.ts

@@ -269,10 +269,12 @@ class ExpressionDiagnosticsVisitor extends RecursiveTemplateAstVisitor {
       if (context && !context.has(ast.value)) {
         if (ast.value === '$implicit') {
           this.reportError(
-              'The template context does not have an implicit value', spanOf(ast.sourceSpan));
+              `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 does not define a member called '${ast.value}'`,
+              `The template context of '${directive.type.reference.name}' does not define a member called '${ast.value}'`,
               spanOf(ast.sourceSpan));
         }
       }

packages/language-service/test/diagnostics_spec.ts

@@ -25,7 +25,6 @@ import {MockTypescriptHost} from './test_utils';
 
 const EXPRESSION_CASES = '/app/expression-cases.ts';
 const NG_FOR_CASES = '/app/ng-for-cases.ts';
-const NG_IF_CASES = '/app/ng-if-cases.ts';
 const TEST_TEMPLATE = '/app/test.ng';
 const APP_COMPONENT = '/app/app.component.ts';
 
@@ -250,11 +249,6 @@ describe('diagnostics', () => {
           `and element references do not contain such a member`);
     });
 
-    it('should report an unknown context reference', () => {
-      const diags = ngLS.getSemanticDiagnostics(NG_FOR_CASES).map(d => d.messageText);
-      expect(diags).toContain(`The template context does not define a member called 'even_1'`);
-    });
-
     it('should report an unknown value in a key expression', () => {
       const diags = ngLS.getSemanticDiagnostics(NG_FOR_CASES).map(d => d.messageText);
       expect(diags).toContain(
@@ -264,10 +258,37 @@ describe('diagnostics', () => {
     });
   });
 
-  describe('in ng-if-cases.ts', () => {
-    it('should report an implicit context reference', () => {
-      const diags = ngLS.getSemanticDiagnostics(NG_IF_CASES).map(d => d.messageText);
-      expect(diags).toContain(`The template context does not define a member called 'unknown'`);
+  describe('embedded templates', () => {
+    it('should suggest refining a template context missing a property', () => {
+      mockHost.override(
+          TEST_TEMPLATE,
+          `<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];
+      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.`, );
+
+      const span = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'emb');
+      expect(start).toBe(span.start);
+      expect(length).toBe(span.length);
+    });
+
+    it('should report an unknown context reference', () => {
+      mockHost.override(
+          TEST_TEMPLATE,
+          `<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];
+      expect(messageText)
+          .toBe(`The template context of 'NgForOf' does not define a member called 'even_1'`);
+
+      const span = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'emb');
+      expect(start).toBe(span.start);
+      expect(length).toBe(span.length);
     });
   });
 

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

@@ -33,6 +33,7 @@ import * as ParsingCases from './parsing-cases';
     ParsingCases.CaseIncompleteOpen,
     ParsingCases.CaseMissingClosing,
     ParsingCases.CaseUnknown,
+    ParsingCases.CounterDirective,
     ParsingCases.EmptyInterpolation,
     ParsingCases.HintModel,
     ParsingCases.NoValueAttribute,

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

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {Component, Directive, EventEmitter, Input, Output} from '@angular/core';
+import {Component, Directive, EventEmitter, Input, OnChanges, Output, SimpleChanges, TemplateRef, ViewContainerRef} from '@angular/core';
 
 import {Hero} from './app.component';
 
@@ -109,6 +109,24 @@ export class AsyncForUsingComponent {
 export class References {
 }
 
+class CounterDirectiveContext<T> {
+  constructor(public $implicit: T) {}
+}
+
+@Directive({selector: '[counterOf]'})
+export class CounterDirective implements OnChanges {
+  // Object does not have an "$implicit" property.
+  constructor(private container: ViewContainerRef, private template: TemplateRef<Object>) {}
+
+  @Input('counterOf') counter: number = 0;
+  ngOnChanges(_changes: SimpleChanges) {
+    this.container.clear();
+    for (let i = 0; i < this.counter; ++i) {
+      this.container.createEmbeddedView(this.template, new CounterDirectiveContext<number>(i + 1));
+    }
+  }
+}
+
 /**
  * This Component provides the `test-comp` selector.
  */