fix(compiler): exclude trailing whitespace from element source spans (#40513)

If the template parse option `leadingTriviaChars` is configured to
consider whitespace as trivia, any trailing whitespace of an element
would be considered as leading trivia of the subsequent element, such
that its `start` span would start _after_ the whitespace. This means
that the start span cannot be used to mark the end of the current
element, as its trailing whitespace would then be included in its span.
Instead, the full start of the subsequent element should be used.

To harden the tests that for the Ivy parser, the test utility `parseR3`
has been adjusted to use the same configuration for `leadingTriviaChars`
as would be the case in its production counterpart `parseTemplate`. This
uncovered another bug in offset handling of the interpolation parser,
where the absolute offset was computed from the start source span
(which excludes leading trivia) whereas the interpolation expression
would include the leading trivia. As such, the absolute offset now also
uses the full start span.

Fixes #39148

PR Close #40513

Author: JoostK

packages/compiler-cli/src/ngtsc/annotations/src/component.ts

@@ -879,6 +879,9 @@ export class ComponentDecoratorHandler implements
     //
     // In order to guarantee the correctness of diagnostics, templates are parsed a second time
     // with the above options set to preserve source mappings.
+    //
+    // Note: template parse options should be aligned with `template_target_spec.ts` and
+    // `TemplateTypeCheckerImpl.overrideComponentTemplate`.
 
     const {nodes: diagNodes} = parseTemplate(templateStr, template.sourceMapUrl, {
       preserveWhitespaces: true,

packages/compiler-cli/src/ngtsc/typecheck/src/checker.ts

@@ -154,6 +154,11 @@ export class TemplateTypeCheckerImpl implements TemplateTypeChecker {
   overrideComponentTemplate(component: ts.ClassDeclaration, template: string):
       {nodes: TmplAstNode[], errors: ParseError[]|null} {
     const {nodes, errors} = parseTemplate(template, 'override.html', {
+      // Set `leadingTriviaChars` and `preserveWhitespaces` such that whitespace is not stripped
+      // and fully accounted for in source spans. Without these flags the source spans can be
+      // inaccurate.
+      // Note: template parse options should be aligned with `template_target_spec.ts` and the
+      // `diagNodes` in `ComponentDecoratorHandler._parseTemplate`.
       preserveWhitespaces: true,
       leadingTriviaChars: [],
     });

packages/compiler-cli/test/compliance/test_cases/source_mapping/external_templates/escaped_chars.js

@@ -1,4 +1,4 @@
-i0.ɵɵelementStart(0, "div") // SOURCE: "/escaped_chars.html" <div>\r\n  
+i0.ɵɵelementStart(0, "div") // SOURCE: "/escaped_chars.html" <div>
 …
 // NOTE: the `\\r\\n` at the end of the next line will be unescaped to `\r\n`. If it was just `\r\n` it would get unescaped to the actual characters.
 i0.ɵɵtext(1, " Some Message Encoded character: \uD83D\uDE80\\n") // SOURCE: "/escaped_chars.html" Some Message\r\n  Encoded character: 🚀\\r\\n

packages/compiler-cli/test/compliance/test_cases/source_mapping/inline_templates/i18n_message_element_whitespace.js

@@ -9,9 +9,9 @@
 }:CLOSE_PARAGRAPH: post-p\n` // SOURCE: "/i18n_message_element_whitespace.ts" post-p\\n
 …
 
-i0.ɵɵelementStart(0, "div") // SOURCE: "/i18n_message_element_whitespace.ts" <div i18n>\\n  
+i0.ɵɵelementStart(0, "div") // SOURCE: "/i18n_message_element_whitespace.ts" <div i18n>
 …
-i0.ɵɵi18nStart(1, 0) // SOURCE: "/i18n_message_element_whitespace.ts" <div i18n>\\n  
+i0.ɵɵi18nStart(1, 0) // SOURCE: "/i18n_message_element_whitespace.ts" <div i18n>
 …
 i0.ɵɵelement(2, "p") // SOURCE: "/i18n_message_element_whitespace.ts" <p>\\n    
 …

packages/compiler-cli/test/ngtsc/template_mapping_spec.ts

@@ -432,7 +432,7 @@ runInEachFileSystem((os) => {
              const mappings = compileAndMap(
                  `<div i18n title="  pre-title {{name}}  post-title" i18n-title>  pre-body {{greeting}}  post-body</div>`);
              expectMapping(mappings, {
-               source: '<div i18n title="  pre-title {{name}}  post-title" i18n-title>  ',
+               source: '<div i18n title="  pre-title {{name}}  post-title" i18n-title>',
                generated: 'i0.ɵɵelementStart(0, "div", 0)',
                sourceUrl: '../test.ts',
              });
@@ -491,12 +491,12 @@ runInEachFileSystem((os) => {
              // ivy instructions
              expectMapping(mappings, {
                sourceUrl: '../test.ts',
-               source: '<div i18n>\n  ',
+               source: '<div i18n>',
                generated: 'i0.ɵɵelementStart(0, "div")',
              });
              expectMapping(mappings, {
                sourceUrl: '../test.ts',
-               source: '<div i18n>\n  ',
+               source: '<div i18n>',
                generated: 'i0.ɵɵi18nStart(1, 0)',
              });
              expectMapping(mappings, {

packages/compiler/src/ml_parser/parser.ts

@@ -259,7 +259,7 @@ class _TreeBuilder {
       this._advance();
       selfClosing = false;
     }
-    const end = this._peek.sourceSpan.start;
+    const end = this._peek.sourceSpan.fullStart;
     const span = new ParseSourceSpan(
         startTagToken.sourceSpan.start, end, startTagToken.sourceSpan.fullStart);
     // Create a separate `startSpan` because `span` will be modified when there is an `end` span.

packages/compiler/src/template_parser/binding_parser.ts

@@ -120,16 +120,17 @@ export class BindingParser {
 
   parseInterpolation(value: string, sourceSpan: ParseSourceSpan): ASTWithSource {
     const sourceInfo = sourceSpan.start.toString();
+    const absoluteOffset = sourceSpan.fullStart.offset;
 
     try {
       const ast = this._exprParser.parseInterpolation(
-          value, sourceInfo, sourceSpan.start.offset, this._interpolationConfig)!;
+          value, sourceInfo, absoluteOffset, this._interpolationConfig)!;
       if (ast) this._reportExpressionParserErrors(ast.errors, sourceSpan);
       this._checkPipes(ast, sourceSpan);
       return ast;
     } catch (e) {
       this._reportError(`${e}`, sourceSpan);
-      return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo, sourceSpan.start.offset);
+      return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo, absoluteOffset);
     }
   }
 
@@ -140,16 +141,17 @@ export class BindingParser {
    */
   parseInterpolationExpression(expression: string, sourceSpan: ParseSourceSpan): ASTWithSource {
     const sourceInfo = sourceSpan.start.toString();
+    const absoluteOffset = sourceSpan.start.offset;
 
     try {
-      const ast = this._exprParser.parseInterpolationExpression(
-          expression, sourceInfo, sourceSpan.start.offset);
+      const ast =
+          this._exprParser.parseInterpolationExpression(expression, sourceInfo, absoluteOffset);
       if (ast) this._reportExpressionParserErrors(ast.errors, sourceSpan);
       this._checkPipes(ast, sourceSpan);
       return ast;
     } catch (e) {
       this._reportError(`${e}`, sourceSpan);
-      return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo, sourceSpan.start.offset);
+      return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo, absoluteOffset);
     }
   }
 

packages/compiler/test/ml_parser/html_parser_spec.ts

@@ -709,6 +709,23 @@ import {humanizeDom, humanizeDomSourceSpans, humanizeLineColumn, humanizeNodes}
           ]);
         });
 
+        it('should set the end source span excluding trailing whitespace whitespace', () => {
+          expect(humanizeDomSourceSpans(
+                     parser.parse('<input type="text" />\n\n\n  <span>\n</span>', 'TestComp', {
+                       leadingTriviaChars: [' ', '\n', '\r', '\t'],
+                     })))
+              .toEqual([
+                [
+                  html.Element, 'input', 0, '<input type="text" />', '<input type="text" />',
+                  '<input type="text" />'
+                ],
+                [html.Attribute, 'type', 'text', 'type="text"'],
+                [html.Text, '\n\n\n  ', 0, ''],
+                [html.Element, 'span', 0, '<span>\n</span>', '<span>', '</span>'],
+                [html.Text, '\n', 1, ''],
+              ]);
+        });
+
         it('should not set the end source span for elements that are implicitly closed', () => {
           expect(humanizeDomSourceSpans(parser.parse('<div><p></div>', 'TestComp'))).toEqual([
             [html.Element, 'div', 0, '<div><p></div>', '<div>', '</div>'],

packages/compiler/test/render3/r3_ast_spans_spec.ts

@@ -148,6 +148,13 @@ describe('R3 AST source spans', () => {
         ['TextAttribute', 'a', 'a', '<empty>'],
       ]);
     });
+
+    it('is correct for self-closing elements with trailing whitespace', () => {
+      expectFromHtml('<input />\n  <span>\n</span>').toEqual([
+        ['Element', '<input />', '<input />', '<input />'],
+        ['Element', '<span>\n</span>', '<span>', '</span>'],
+      ]);
+    });
   });
 
   describe('bound text nodes', () => {

packages/compiler/test/render3/view/i18n_spec.ts

@@ -523,7 +523,7 @@ describe('serializeI18nMessageForLocalize', () => {
        expect(humanizeSourceSpan(messageParts[3].sourceSpan)).toEqual('"" (29-29)');
 
        expect(placeHolders[0].text).toEqual('START_BOLD_TEXT');
-       expect(humanizeSourceSpan(placeHolders[0].sourceSpan)).toEqual('"<b>   " (10-16)');
+       expect(humanizeSourceSpan(placeHolders[0].sourceSpan)).toEqual('"<b>" (10-13)');
        expect(placeHolders[1].text).toEqual('INTERPOLATION');
        expect(humanizeSourceSpan(placeHolders[1].sourceSpan)).toEqual('"{{value}}" (16-25)');
        expect(placeHolders[2].text).toEqual('CLOSE_BOLD_TEXT');