Skip to content
Permalink
Browse files

fix(language-service): Do not produce diagnostics if metadata for NgM…

…odule 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 angular/vscode-ng-language-service#463

PR Close #34113
  • Loading branch information
kyliau authored and mhevery committed Nov 27, 2019
1 parent bfeb4d2 commit 29de8d3ab1e4d0640469883c803212087bee8047
@@ -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');
}
@@ -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));
}
}

@@ -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);
});
}
@@ -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.
@@ -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,
};
}

/**
@@ -837,4 +837,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([]);
});
});

0 comments on commit 29de8d3

Please sign in to comment.
You can’t perform that action at this time.