fix(language-service): Always attempt HTML AST to template AST conversion for LS (#41068)

The current logic in the compiler is to bail when there are errors when
parsing a template into an HTML AST or when there are errors in the i18n
metadata. As a result, a template with these types of parse errors
_will not have any information for the language service_. This is because we
never attempt to conver the HTML AST to a template AST in these
scenarios, so there are no template AST nodes for the language service
to look at for information. In addition, this also means that the errors
are never displayed in the template to the user because there are no
nodes to map the error to.

This commit adds an option to the template parser to temporarily ignore
the html parse and i18n meta errors and always perform the template AST
conversion. At the end, the i18n and HTML parse errors are appended to
the returned errors list. While this seems risky, it at least provides
us with more information than we had before (which was 0) and it's only
done in the context of the language service, when the compiler is
configured to use poisoned data (HTML parse and i18n meta errors can be
interpreted as a "poisoned" template).

fixes angular/vscode-ng-language-service#1140

PR Close #41068
This commit is contained in:
Andrew Scott 2021-03-02 13:00:45 -08:00 committed by Zach Arend
parent 1e3c870ee6
commit 0847a0353b
6 changed files with 97 additions and 5 deletions

View File

@ -975,6 +975,7 @@ export class ComponentDecoratorHandler implements
enableI18nLegacyMessageIdFormat: this.enableI18nLegacyMessageIdFormat,
i18nNormalizeLineEndingsInICUs,
isInline: template.isInline,
alwaysAttemptHtmlToR3AstConversion: this.usePoisonedData,
});
// Unfortunately, the primary parse of the template above may not contain accurate source map
@ -1002,6 +1003,7 @@ export class ComponentDecoratorHandler implements
i18nNormalizeLineEndingsInICUs,
leadingTriviaChars: [],
isInline: template.isInline,
alwaysAttemptHtmlToR3AstConversion: this.usePoisonedData,
});
return {

View File

@ -2096,6 +2096,22 @@ export interface ParseTemplateOptions {
* Whether the template was inline.
*/
isInline?: boolean;
/**
* Whether to always attempt to convert the parsed HTML AST to an R3 AST, despite HTML or i18n
* Meta parse errors.
*
*
* This option is useful in the context of the language service, where we want to get as much
* information as possible, despite any errors in the HTML. As an example, a user may be adding
* a new tag and expecting autocomplete on that tag. In this scenario, the HTML is in an errored
* state, as there is an incomplete open tag. However, we're still able to convert the HTML AST
* nodes to R3 AST nodes in order to provide information for the language service.
*
* Note that even when `true` the HTML parse and i18n errors are still appended to the errors
* output, but this is done after converting the HTML AST to R3 AST.
*/
alwaysAttemptHtmlToR3AstConversion?: boolean;
}
/**
@ -2115,9 +2131,8 @@ export function parseTemplate(
template, templateUrl,
{leadingTriviaChars: LEADING_TRIVIA_CHARS, ...options, tokenizeExpansionForms: true});
if (parseResult.errors && parseResult.errors.length > 0) {
// TODO(ayazhafiz): we may not always want to bail out at this point (e.g. in
// the context of a language service).
if (!options.alwaysAttemptHtmlToR3AstConversion && parseResult.errors &&
parseResult.errors.length > 0) {
return {
interpolationConfig,
preserveWhitespaces,
@ -2143,7 +2158,8 @@ export function parseTemplate(
enableI18nLegacyMessageIdFormat);
const i18nMetaResult = i18nMetaVisitor.visitAllWithErrors(rootNodes);
if (i18nMetaResult.errors && i18nMetaResult.errors.length > 0) {
if (!options.alwaysAttemptHtmlToR3AstConversion && i18nMetaResult.errors &&
i18nMetaResult.errors.length > 0) {
return {
interpolationConfig,
preserveWhitespaces,
@ -2175,6 +2191,7 @@ export function parseTemplate(
const {nodes, errors, styleUrls, styles, ngContentSelectors} =
htmlAstToRender3Ast(rootNodes, bindingParser);
errors.push(...parseResult.errors, ...i18nMetaResult.errors);
return {
interpolationConfig,

View File

@ -116,6 +116,19 @@ describe('R3 template transform', () => {
});
describe('Nodes without binding', () => {
it('should parse incomplete tags terminated by EOF', () => {
expectFromHtml('<a', true /* ignoreError */).toEqual([
['Element', 'a'],
]);
});
it('should parse incomplete tags terminated by another tag', () => {
expectFromHtml('<a <span></span>', true /* ignoreError */).toEqual([
['Element', 'a'],
['Element', 'span'],
]);
});
it('should parse text nodes', () => {
expectFromHtml('a').toEqual([
['Text', 'a'],

View File

@ -353,6 +353,23 @@ describe('completions', () => {
expect(ts.displayPartsToString(details.documentation!)).toEqual('This is another component.');
});
it('should return completions for an incomplete tag', () => {
const OTHER_CMP = {
'OtherCmp': `
/** This is another component. */
@Component({selector: 'other-cmp', template: 'unimportant'})
export class OtherCmp {}
`,
};
const {templateFile} = setup(`<other`, '', OTHER_CMP);
templateFile.moveCursorToText('<other¦');
const completions = templateFile.getCompletionsAtPosition();
expectContain(
completions, unsafeCastDisplayInfoKindToScriptElementKind(DisplayInfoKind.COMPONENT),
['other-cmp']);
});
it('should return completions with a blank open tag', () => {
const OTHER_CMP = {
'OtherCmp': `
@ -397,6 +414,10 @@ describe('completions', () => {
const completions = templateFile.getCompletionsAtPosition();
expect(completions).toBeUndefined();
const details = templateFile.getCompletionEntryDetails('other-cmp')!;
expect(details).toBeUndefined();
});
describe('element attribute scope', () => {

View File

@ -19,7 +19,7 @@ describe('getSemanticDiagnostics', () => {
env = LanguageServiceTestEnv.setup();
});
it('should not produce error for a minimal component defintion', () => {
it('should not produce error for a minimal component definition', () => {
const files = {
'app.ts': `
import {Component, NgModule} from '@angular/core';
@ -124,6 +124,34 @@ describe('getSemanticDiagnostics', () => {
`Parser Error: Bindings cannot contain assignments at column 8 in [{{nope = true}}]`);
});
it('reports html parse errors along with typecheck errors as diagnostics', () => {
const files = {
'app.ts': `
import {Component, NgModule} from '@angular/core';
@Component({
templateUrl: './app.html'
})
export class AppComponent {
nope = false;
}
`,
'app.html': '<dne'
};
const project = createModuleAndProjectWithDeclarations(env, 'test', files);
const diags = project.getDiagnosticsForFile('app.html');
expect(diags.length).toBe(2);
expect(diags[0].category).toBe(ts.DiagnosticCategory.Error);
expect(diags[0].file?.fileName).toBe('/test/app.html');
expect(diags[0].messageText).toContain(`'dne' is not a known element`);
expect(diags[1].category).toBe(ts.DiagnosticCategory.Error);
expect(diags[1].file?.fileName).toBe('/test/app.html');
expect(diags[1].messageText).toContain(`Opening tag "dne" not terminated.`);
});
it('should report parse errors of components defined in the same ts file', () => {
const files = {
'app.ts': `

View File

@ -35,12 +35,23 @@ function parse(template: string): ParseResult {
// `ComponentDecoratorHandler._parseTemplate`.
leadingTriviaChars: [],
preserveWhitespaces: true,
alwaysAttemptHtmlToR3AstConversion: true,
}),
position,
};
}
describe('getTargetAtPosition for template AST', () => {
it('should locate incomplete tag', () => {
const {errors, nodes, position} = parse(`<div¦`);
expect(errors?.length).toBe(1);
expect(errors![0].msg).toContain('Opening tag "div" not terminated.');
const {context} = getTargetAtPosition(nodes, position)!;
const {node} = context as SingleNodeTarget;
expect(isTemplateNode(node!)).toBe(true);
expect(node).toBeInstanceOf(t.Element);
});
it('should locate element in opening tag', () => {
const {errors, nodes, position} = parse(`<di¦v></div>`);
expect(errors).toBe(null);