fix(language-service): Do not produce diagnostics if metadata for NgModule not found (#34113)

The language service incorrectly reports an error if it fails to find
NgModule metadata for a particular Component / Directive. In many cases,
the use case is legit, particularly in test.

This commit removes such diagnostic message and cleans up the interface
for `TypeScriptHost.getTemplateAst()`.

PR closes https://github.com/angular/vscode-ng-language-service/issues/463

PR Close #34113

packages/language-service/src/common.ts

@@ -8,7 +8,7 @@
 
 import {CompileDirectiveMetadata, CompileDirectiveSummary, CompilePipeSummary, CssSelector, Node as HtmlAst, ParseError, Parser, TemplateAst} from '@angular/compiler';
 
-import {Diagnostic, TemplateSource} from './types';
+import {TemplateSource} from './types';
 
 export interface AstResult {
   htmlAst: HtmlAst[];
@@ -25,7 +25,3 @@ export type SelectorInfo = {
   selectors: CssSelector[],
   map: Map<CssSelector, CompileDirectiveSummary>
 };
-
-export function isAstResult(result: AstResult | Diagnostic): result is AstResult {
-  return result.hasOwnProperty('templateAst');
-}

packages/language-service/src/language_service.ts

@@ -8,7 +8,6 @@
 
 import * as tss from 'typescript/lib/tsserverlibrary';
 
-import {isAstResult} from './common';
 import {getTemplateCompletions} from './completions';
 import {getDefinitionAndBoundSpan, getTsDefinitionAndBoundSpan} from './definitions';
 import {getDeclarationDiagnostics, getTemplateDiagnostics, ngDiagnosticToTsDiagnostic, uniqueBySpan} from './diagnostics';
@@ -34,11 +33,9 @@ class LanguageServiceImpl implements LanguageService {
     const templates = this.host.getTemplates(fileName);
 
     for (const template of templates) {
-      const astOrDiagnostic = this.host.getTemplateAst(template);
-      if (isAstResult(astOrDiagnostic)) {
-        results.push(...getTemplateDiagnostics(astOrDiagnostic));
-      } else {
-        results.push(astOrDiagnostic);
+      const ast = this.host.getTemplateAst(template);
+      if (ast) {
+        results.push(...getTemplateDiagnostics(ast));
       }
     }
 

packages/language-service/src/template.ts

@@ -8,7 +8,6 @@
 
 import * as ts from 'typescript';
 
-import {isAstResult} from './common';
 import {createGlobalSymbolTable} from './global_symbols';
 import * as ng from './types';
 import {TypeScriptServiceHost} from './typescript_host';
@@ -73,7 +72,7 @@ abstract class BaseTemplate implements ng.TemplateSource {
         // TODO: There is circular dependency here between TemplateSource and
         // TypeScriptHost. Consider refactoring the code to break this cycle.
         const ast = this.host.getTemplateAst(this);
-        const pipes = isAstResult(ast) ? ast.pipes : [];
+        const pipes = (ast && ast.pipes) || [];
         return getPipesTable(sourceFile, program, typeChecker, pipes);
       });
     }

packages/language-service/src/types.ts

@@ -193,7 +193,7 @@ export interface LanguageServiceHost {
   /**
    * Return the AST for both HTML and template for the contextFile.
    */
-  getTemplateAst(template: TemplateSource): AstResult|Diagnostic;
+  getTemplateAst(template: TemplateSource): AstResult|undefined;
 
   /**
    * Return the template AST for the node that corresponds to the position.

packages/language-service/src/typescript_host.ts

@@ -11,11 +11,11 @@ import {SchemaMetadata, ViewEncapsulation, ɵConsole as Console} from '@angular/
 import * as ts from 'typescript';
 import * as tss from 'typescript/lib/tsserverlibrary';
 
-import {AstResult, isAstResult} from './common';
+import {AstResult} from './common';
 import {createLanguageService} from './language_service';
 import {ReflectorHost} from './reflector_host';
 import {ExternalTemplate, InlineTemplate, getClassDeclFromDecoratorProp, getPropertyAssignmentFromValue} from './template';
-import {Declaration, DeclarationError, Diagnostic, DiagnosticKind, DiagnosticMessageChain, LanguageService, LanguageServiceHost, Span, TemplateSource} from './types';
+import {Declaration, DeclarationError, DiagnosticMessageChain, LanguageService, LanguageServiceHost, Span, TemplateSource} from './types';
 import {findTightestNode, getDirectiveClassLike} from './utils';
 
 
@@ -456,11 +456,7 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
     if (!template) {
       return;
     }
-    const astResult = this.getTemplateAst(template);
-    if (!isAstResult(astResult)) {
-      return;
-    }
-    return astResult;
+    return this.getTemplateAst(template);
   }
 
   /**
@@ -505,57 +501,38 @@ export class TypeScriptServiceHost implements LanguageServiceHost {
   }
 
   /**
-   * Parse the `template` and return its AST if there's no error. Otherwise
-   * return a Diagnostic message.
+   * Parse the `template` and return its AST, if any.
    * @param template template to be parsed
    */
-  getTemplateAst(template: TemplateSource): AstResult|Diagnostic {
+  getTemplateAst(template: TemplateSource): AstResult|undefined {
     const {type: classSymbol, fileName} = template;
-    try {
-      const data = this.resolver.getNonNormalizedDirectiveMetadata(classSymbol);
-      if (!data) {
-        return {
-          kind: DiagnosticKind.Error,
-          message: `No metadata found for '${classSymbol.name}' in ${fileName}.`,
-          span: template.span,
-        };
-      }
-      const htmlParser = new I18NHtmlParser(new HtmlParser());
-      const expressionParser = new Parser(new Lexer());
-      const parser = new TemplateParser(
-          new CompilerConfig(), this.reflector, expressionParser, new DomElementSchemaRegistry(),
-          htmlParser,
-          null !,  // console
-          []       // tranforms
-          );
-      const htmlResult = htmlParser.parse(template.source, fileName, {
-        tokenizeExpansionForms: true,
-        preserveLineEndings: true,  // do not convert CRLF to LF
-      });
-      const {directives, pipes, schemas} = this.getModuleMetadataForDirective(classSymbol);
-      const parseResult =
-          parser.tryParseHtml(htmlResult, data.metadata, directives, pipes, schemas);
-      if (!parseResult.templateAst) {
-        return {
-          kind: DiagnosticKind.Error,
-          message: `Failed to parse template for '${classSymbol.name}' in ${fileName}`,
-          span: template.span,
-        };
-      }
-      return {
-        htmlAst: htmlResult.rootNodes,
-        templateAst: parseResult.templateAst,
-        directive: data.metadata, directives, pipes,
-        parseErrors: parseResult.errors, expressionParser, template,
-      };
-    } catch (e) {
-      return {
-        kind: DiagnosticKind.Error,
-        message: e.message,
-        span:
-            e.fileName === fileName && template.query.getSpanAt(e.line, e.column) || template.span,
-      };
+    const data = this.resolver.getNonNormalizedDirectiveMetadata(classSymbol);
+    if (!data) {
+      return;
     }
+    const htmlParser = new I18NHtmlParser(new HtmlParser());
+    const expressionParser = new Parser(new Lexer());
+    const parser = new TemplateParser(
+        new CompilerConfig(), this.reflector, expressionParser, new DomElementSchemaRegistry(),
+        htmlParser,
+        null !,  // console
+        []       // tranforms
+        );
+    const htmlResult = htmlParser.parse(template.source, fileName, {
+      tokenizeExpansionForms: true,
+      preserveLineEndings: true,  // do not convert CRLF to LF
+    });
+    const {directives, pipes, schemas} = this.getModuleMetadataForDirective(classSymbol);
+    const parseResult = parser.tryParseHtml(htmlResult, data.metadata, directives, pipes, schemas);
+    if (!parseResult.templateAst) {
+      return;
+    }
+    return {
+      htmlAst: htmlResult.rootNodes,
+      templateAst: parseResult.templateAst,
+      directive: data.metadata, directives, pipes,
+      parseErrors: parseResult.errors, expressionParser, template,
+    };
   }
 
   /**

packages/language-service/test/diagnostics_spec.ts

@@ -829,4 +829,20 @@ describe('diagnostics', () => {
       expect(content.substring(start !, start ! + length !)).toBe(`line${i}`);
     }
   });
+
+  it('should not produce diagnostics for non-exported directives', () => {
+    const fileName = '/app/test.component.ts';
+    mockHost.addScript(fileName, `
+      import {Component} from '@angular/core';
+
+      @Component({
+        template: '<test-comp></test-comp>'
+      })
+      class TestHostComponent {}
+    `);
+    const tsDiags = tsLS.getSemanticDiagnostics(fileName);
+    expect(tsDiags).toEqual([]);
+    const ngDiags = ngLS.getDiagnostics(fileName);
+    expect(ngDiags).toEqual([]);
+  });
 });