From 0847a0353b87cd181ef2a257e36126a888f02d75 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 2 Mar 2021 13:00:45 -0800 Subject: [PATCH] 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 --- .../src/ngtsc/annotations/src/component.ts | 2 ++ .../compiler/src/render3/view/template.ts | 25 +++++++++++++--- .../render3/r3_template_transform_spec.ts | 13 ++++++++ .../ivy/test/completions_spec.ts | 21 +++++++++++++ .../ivy/test/diagnostic_spec.ts | 30 ++++++++++++++++++- .../ivy/test/legacy/template_target_spec.ts | 11 +++++++ 6 files changed, 97 insertions(+), 5 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 0dfddae248..55c9597c9a 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -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 { diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts index 88ad359364..4103a3f06b 100644 --- a/packages/compiler/src/render3/view/template.ts +++ b/packages/compiler/src/render3/view/template.ts @@ -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, diff --git a/packages/compiler/test/render3/r3_template_transform_spec.ts b/packages/compiler/test/render3/r3_template_transform_spec.ts index cf1cfb40bf..d27e7e51d4 100644 --- a/packages/compiler/test/render3/r3_template_transform_spec.ts +++ b/packages/compiler/test/render3/r3_template_transform_spec.ts @@ -116,6 +116,19 @@ describe('R3 template transform', () => { }); describe('Nodes without binding', () => { + it('should parse incomplete tags terminated by EOF', () => { + expectFromHtml(' { + expectFromHtml('', true /* ignoreError */).toEqual([ + ['Element', 'a'], + ['Element', 'span'], + ]); + }); + it('should parse text nodes', () => { expectFromHtml('a').toEqual([ ['Text', 'a'], diff --git a/packages/language-service/ivy/test/completions_spec.ts b/packages/language-service/ivy/test/completions_spec.ts index eac4d9816e..62d4139dec 100644 --- a/packages/language-service/ivy/test/completions_spec.ts +++ b/packages/language-service/ivy/test/completions_spec.ts @@ -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(` { 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', () => { diff --git a/packages/language-service/ivy/test/diagnostic_spec.ts b/packages/language-service/ivy/test/diagnostic_spec.ts index 97ccf9dc1d..5288e06814 100644 --- a/packages/language-service/ivy/test/diagnostic_spec.ts +++ b/packages/language-service/ivy/test/diagnostic_spec.ts @@ -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': ' { const files = { 'app.ts': ` diff --git a/packages/language-service/ivy/test/legacy/template_target_spec.ts b/packages/language-service/ivy/test/legacy/template_target_spec.ts index 73457a9d2e..01623e9d7c 100644 --- a/packages/language-service/ivy/test/legacy/template_target_spec.ts +++ b/packages/language-service/ivy/test/legacy/template_target_spec.ts @@ -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(` { const {errors, nodes, position} = parse(``); expect(errors).toBe(null);