fix(compiler): correct spans when parsing bindings with comments (#44678)

alxhub · atscott · commit 59eef29a6c5d · 2022-01-11T19:16:19.000Z
When parsing a binding with a comment at the end of the expression, the parser previously had logic to offset the parsed spans by the length of the comment. This logic seemed not to serve any useful purpose, and instead resulted in the corruption of the spans. For example, in the expression `{{foo // comment}}`, the parser would map the parsed `foo` `PropertyRead` node at the location of the characters 'ent' from the comment suffix. This commit removes that logic, correcting the parsed spans of such nodes in the presence of comments. Removing this logic does not seem to have caused any tests to fail. PR Close #44678
diff --git a/packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts b/packages/compiler-cli/src/ngtsc/indexer/test/template_spec.ts
@@ -21,6 +21,19 @@ function bind(template: string) {
 
 runInEachFileSystem(() => {
   describe('getTemplateIdentifiers', () => {
+    it('should handle comments in interpolations', () => {
+      const template = '{{foo // comment}}';
+      const refs = getTemplateIdentifiers(bind(template));
+
+      const [ref] = Array.from(refs);
+      expect(ref).toEqual({
+        name: 'foo',
+        kind: IdentifierKind.Property,
+        span: new AbsoluteSourceSpan(2, 5),
+        target: null,
+      });
+    });
+
     it('should generate nothing in empty template', () => {
       const template = '';
       const refs = getTemplateIdentifiers(bind(template));
diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts
@@ -39,11 +39,11 @@ export class Parser {
       interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): ASTWithSource {
     this._checkNoInterpolation(input, location, interpolationConfig);
     const sourceToLex = this._stripComments(input);
-    const tokens = this._lexer.tokenize(this._stripComments(input));
-    const ast = new _ParseAST(
-                    input, location, absoluteOffset, tokens, sourceToLex.length, true, this.errors,
-                    input.length - sourceToLex.length)
-                    .parseChain();
+    const tokens = this._lexer.tokenize(sourceToLex);
+    const ast =
+        new _ParseAST(
+            input, location, absoluteOffset, tokens, sourceToLex.length, true, this.errors, 0)
+            .parseChain();
     return new ASTWithSource(ast, input, location, absoluteOffset, this.errors);
   }
 
@@ -91,8 +91,7 @@ export class Parser {
     const sourceToLex = this._stripComments(input);
     const tokens = this._lexer.tokenize(sourceToLex);
     return new _ParseAST(
-               input, location, absoluteOffset, tokens, sourceToLex.length, false, this.errors,
-               input.length - sourceToLex.length)
+               input, location, absoluteOffset, tokens, sourceToLex.length, false, this.errors, 0)
         .parseChain();
   }
 
@@ -162,7 +161,7 @@ export class Parser {
       const tokens = this._lexer.tokenize(sourceToLex);
       const ast = new _ParseAST(
                       input, location, absoluteOffset, tokens, sourceToLex.length, false,
-                      this.errors, offsets[i] + (expressionText.length - sourceToLex.length))
+                      this.errors, offsets[i])
                       .parseChain();
       expressionNodes.push(ast);
     }
diff --git a/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts b/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts
@@ -7,10 +7,23 @@
  */
 
 import {AbsoluteSourceSpan} from '@angular/compiler';
+
 import {humanizeExpressionSource} from './util/expression';
 import {parseR3 as parse} from './view/util';
 
 describe('expression AST absolute source spans', () => {
+  it('should handle comment in interpolation', () => {
+    expect(humanizeExpressionSource(parse('{{foo // comment}}', {preserveWhitespaces: true}).nodes))
+        .toContain(['foo', new AbsoluteSourceSpan(2, 5)]);
+  });
+
+  it('should handle comment in an action binding', () => {
+    expect(humanizeExpressionSource(parse('<button (click)="foo = true // comment">Save</button>', {
+                                      preserveWhitespaces: true
+                                    }).nodes))
+        .toContain(['foo = true', new AbsoluteSourceSpan(17, 27)]);
+  });
+
   // TODO(ayazhafiz): duplicate this test without `preserveWhitespaces` once whitespace rewriting is
   // moved to post-R3AST generation.
   it('should provide absolute offsets with arbitrary whitespace', () => {
