angular · petebacondarwin · Oct 28, 2020 · Oct 28, 2020 · Oct 28, 2020 · Oct 29, 2020
diff --git a/packages/compiler/src/compiler_facade_interface.ts b/packages/compiler/src/compiler_facade_interface.ts
@@ -193,4 +193,5 @@ export interface ParseSourceSpan {
   start: any;
   end: any;
   details: any;
+  fullStart: any;
 }
diff --git a/packages/compiler/src/compiler_util/expression_converter.ts b/packages/compiler/src/compiler_util/expression_converter.ts
@@ -917,7 +917,8 @@ class _AstToIrVisitor implements cdAst.AstVisitor {
     if (this.baseSourceSpan) {
       const start = this.baseSourceSpan.start.moveBy(span.start);
       const end = this.baseSourceSpan.start.moveBy(span.end);
-      return new ParseSourceSpan(start, end);
+      const fullStart = this.baseSourceSpan.fullStart.moveBy(span.start);
+      return new ParseSourceSpan(start, end, fullStart);
     } else {
       return null;
     }

diff --git a/packages/compiler/src/i18n/i18n_parser.ts b/packages/compiler/src/i18n/i18n_parser.ts
@@ -203,7 +203,7 @@ class _I18nVisitor implements html.Visitor {
 
 function getOffsetSourceSpan(
     sourceSpan: ParseSourceSpan, {start, end}: {start: number, end: number}): ParseSourceSpan {
-  return new ParseSourceSpan(sourceSpan.start.moveBy(start), sourceSpan.start.moveBy(end));
+  return new ParseSourceSpan(sourceSpan.fullStart.moveBy(start), sourceSpan.fullStart.moveBy(end));
 }
 
 const _CUSTOM_PH_EXP =

diff --git a/packages/compiler/src/ml_parser/lexer.ts b/packages/compiler/src/ml_parser/lexer.ts
@@ -917,19 +917,20 @@ class PlainCharacterCursor implements CharacterCursor {
 
   getSpan(start?: this, leadingTriviaCodePoints?: number[]): ParseSourceSpan {
     start = start || this;
-    let cloned = false;
+    let fullStart = start;
     if (leadingTriviaCodePoints) {
       while (this.diff(start) > 0 && leadingTriviaCodePoints.indexOf(start.peek()) !== -1) {
-        if (!cloned) {
+        if (fullStart === start) {
           start = start.clone() as this;
-          cloned = true;
         }
         start.advance();
       }
     }
-    return new ParseSourceSpan(
-        new ParseLocation(start.file, start.state.offset, start.state.line, start.state.column),
-        new ParseLocation(this.file, this.state.offset, this.state.line, this.state.column));
+    const startLocation = this.locationFromCursor(start);
+    const endLocation = this.locationFromCursor(this);
+    const fullStartLocation =
+        fullStart !== start ? this.locationFromCursor(fullStart) : startLocation;
+    return new ParseSourceSpan(startLocation, endLocation, fullStartLocation);
   }
 
   getChars(start: this): string {
@@ -959,6 +960,11 @@ class PlainCharacterCursor implements CharacterCursor {
   protected updatePeek(state: CursorState): void {
     state.peek = state.offset >= this.end ? chars.$EOF : this.charAt(state.offset);
   }
+
+  private locationFromCursor(cursor: this): ParseLocation {
+    return new ParseLocation(
+        cursor.file, cursor.state.offset, cursor.state.line, cursor.state.column);
+  }
 }
 
 class EscapedCharacterCursor extends PlainCharacterCursor {

diff --git a/packages/compiler/src/ml_parser/parser.ts b/packages/compiler/src/ml_parser/parser.ts
@@ -129,7 +129,8 @@ class _TreeBuilder {
           TreeError.create(null, this._peek.sourceSpan, `Invalid ICU message. Missing '}'.`));
       return;
     }
-    const sourceSpan = new ParseSourceSpan(token.sourceSpan.start, this._peek.sourceSpan.end);
+    const sourceSpan = new ParseSourceSpan(
+        token.sourceSpan.start, this._peek.sourceSpan.end, token.sourceSpan.fullStart);
     this._addToParent(new html.Expansion(
         switchValue.parts[0], type.parts[0], cases, sourceSpan, switchValue.sourceSpan));
 
@@ -163,8 +164,10 @@ class _TreeBuilder {
       return null;
     }
 
-    const sourceSpan = new ParseSourceSpan(value.sourceSpan.start, end.sourceSpan.end);
-    const expSourceSpan = new ParseSourceSpan(start.sourceSpan.start, end.sourceSpan.end);
+    const sourceSpan =
+        new ParseSourceSpan(value.sourceSpan.start, end.sourceSpan.end, value.sourceSpan.fullStart);
+    const expSourceSpan =
+        new ParseSourceSpan(start.sourceSpan.start, end.sourceSpan.end, start.sourceSpan.fullStart);
     return new html.ExpansionCase(
         value.parts[0], expansionCaseParser.rootNodes, sourceSpan, value.sourceSpan, expSourceSpan);
   }
@@ -257,9 +260,11 @@ class _TreeBuilder {
       selfClosing = false;
     }
     const end = this._peek.sourceSpan.start;
-    const span = new ParseSourceSpan(startTagToken.sourceSpan.start, end);
+    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.
-    const startSpan = new ParseSourceSpan(startTagToken.sourceSpan.start, end);
+    const startSpan = new ParseSourceSpan(
+        startTagToken.sourceSpan.start, end, startTagToken.sourceSpan.fullStart);
     const el = new html.Element(fullName, attrs, [], span, startSpan, undefined);
     this._pushElement(el);
     if (selfClosing) {
@@ -347,7 +352,9 @@ class _TreeBuilder {
       end = quoteToken.sourceSpan.end;
     }
     return new html.Attribute(
-        fullName, value, new ParseSourceSpan(attrName.sourceSpan.start, end), valueSpan);
+        fullName, value,
+        new ParseSourceSpan(attrName.sourceSpan.start, end, attrName.sourceSpan.fullStart),
+        valueSpan);
   }
 
   private _getParentElement(): html.Element|null {

diff --git a/packages/compiler/src/parse_util.ts b/packages/compiler/src/parse_util.ts
@@ -100,8 +100,32 @@ export class ParseSourceFile {
 }
 
 export class ParseSourceSpan {
+  /**
+   * Create an object that holds information about spans of tokens/nodes captured during
+   * lexing/parsing of text.
+   *
+   * @param start
+   * The location of the start of the span (having skipped leading trivia).
+   * Skipping leading trivia makes source-spans more "user friendly", since things like HTML
+   * elements will appear to begin at the start of the opening tag, rather than at the start of any
+   * leading trivia, which could include newlines.
+   *
+   * @param end
+   * The location of the end of the span.
+   *
+   * @param fullStart
+   * The start of the token without skipping the leading trivia.
+   * This is used by tooling that splits tokens further, such as extracting Angular interpolations
+   * from text tokens. Such tooling creates new source-spans relative to the original token's
+   * source-span. If leading trivia characters have been skipped then the new source-spans may be
+   * incorrectly offset.
+   *
+   * @param details
+   * Additional information (such as identifier names) that should be associated with the span.
+   */
   constructor(
-      public start: ParseLocation, public end: ParseLocation, public details: string|null = null) {}
+      public start: ParseLocation, public end: ParseLocation,
+      public fullStart: ParseLocation = start, public details: string|null = null) {}
 
   toString(): string {
     return this.start.file.content.substring(this.start.offset, this.end.offset);

diff --git a/packages/compiler/src/render3/r3_template_transform.ts b/packages/compiler/src/render3/r3_template_transform.ts
@@ -331,7 +331,7 @@ class HtmlAstToIvyAst implements html.Visitor {
       const normalizationAdjustment = attribute.name.length - name.length;
       const keySpanStart = srcSpan.start.moveBy(prefix.length + normalizationAdjustment);
       const keySpanEnd = keySpanStart.moveBy(identifier.length);
-      return new ParseSourceSpan(keySpanStart, keySpanEnd, identifier);
+      return new ParseSourceSpan(keySpanStart, keySpanEnd, keySpanStart, identifier);
     }
 
     const bindParts = name.match(BIND_NAME_REGEXP);

diff --git a/packages/compiler/src/render3/view/i18n/localize_utils.ts b/packages/compiler/src/render3/view/i18n/localize_utils.ts
@@ -90,7 +90,8 @@ function getSourceSpan(message: i18n.Message): ParseSourceSpan {
   const startNode = message.nodes[0];
   const endNode = message.nodes[message.nodes.length - 1];
   return new ParseSourceSpan(
-      startNode.sourceSpan.start, endNode.sourceSpan.end, startNode.sourceSpan.details);
+      startNode.sourceSpan.start, endNode.sourceSpan.end, startNode.sourceSpan.fullStart,
+      startNode.sourceSpan.details);
 }
 
 /**
@@ -120,7 +121,7 @@ function processMessagePieces(pieces: o.MessagePiece[]):
       placeHolders.push(part);
       if (pieces[i - 1] instanceof o.PlaceholderPiece) {
         // There were two placeholders in a row, so we need to add an empty message part.
-        messageParts.push(createEmptyMessagePart(part.sourceSpan.end));
+        messageParts.push(createEmptyMessagePart(pieces[i - 1].sourceSpan.end));
       }
     }
   }

diff --git a/packages/compiler/src/render3/view/template.ts b/packages/compiler/src/render3/view/template.ts
@@ -55,7 +55,7 @@ const EVENT_BINDING_SCOPE_GLOBALS = new Set<string>(['$event']);
 const GLOBAL_TARGET_RESOLVERS = new Map<string, o.ExternalReference>(
     [['window', R3.resolveWindow], ['document', R3.resolveDocument], ['body', R3.resolveBody]]);
 
-const LEADING_TRIVIA_CHARS = [' ', '\n', '\r', '\t'];
+export const LEADING_TRIVIA_CHARS = [' ', '\n', '\r', '\t'];
 
 //  if (rf & flags) { .. }
 export function renderFlagCheckIfStmt(

diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts
@@ -585,5 +585,7 @@ function moveParseSourceSpan(
   // The difference of two absolute offsets provide the relative offset
   const startDiff = absoluteSpan.start - sourceSpan.start.offset;
   const endDiff = absoluteSpan.end - sourceSpan.end.offset;
-  return new ParseSourceSpan(sourceSpan.start.moveBy(startDiff), sourceSpan.end.moveBy(endDiff));
+  return new ParseSourceSpan(
+      sourceSpan.start.moveBy(startDiff), sourceSpan.end.moveBy(endDiff),
+      sourceSpan.fullStart.moveBy(startDiff), sourceSpan.details);
 }
diff --git a/packages/compiler/src/template_parser/template_parser.ts b/packages/compiler/src/template_parser/template_parser.ts
@@ -566,7 +566,7 @@ class TemplateParseVisitor implements html.Visitor {
 
     const directiveAsts = directives.map((directive) => {
       const sourceSpan = new ParseSourceSpan(
-          elementSourceSpan.start, elementSourceSpan.end,
+          elementSourceSpan.start, elementSourceSpan.end, elementSourceSpan.fullStart,
           `Directive ${identifierName(directive.type)}`);
 
       if (directive.isComponent) {

diff --git a/packages/compiler/test/ml_parser/lexer_spec.ts b/packages/compiler/test/ml_parser/lexer_spec.ts
@@ -54,14 +54,14 @@ import {ParseLocation, ParseSourceFile, ParseSourceSpan} from '../../src/parse_u
       });
 
       it('should skip over leading trivia for source-span start', () => {
-        expect(tokenizeAndHumanizeLineColumn(
-                   '<t>\n \t a</t>', {leadingTriviaChars: ['\n', ' ', '\t']}))
+        expect(
+            tokenizeAndHumanizeFullStart('<t>\n \t a</t>', {leadingTriviaChars: ['\n', ' ', '\t']}))
             .toEqual([
-              [lex.TokenType.TAG_OPEN_START, '0:0'],
-              [lex.TokenType.TAG_OPEN_END, '0:2'],
-              [lex.TokenType.TEXT, '1:3'],
-              [lex.TokenType.TAG_CLOSE, '1:4'],
-              [lex.TokenType.EOF, '1:8'],
+              [lex.TokenType.TAG_OPEN_START, '0:0', '0:0'],
+              [lex.TokenType.TAG_OPEN_END, '0:2', '0:2'],
+              [lex.TokenType.TEXT, '1:3', '0:3'],
+              [lex.TokenType.TAG_CLOSE, '1:4', '1:4'],
+              [lex.TokenType.EOF, '1:8', '1:8'],
             ]);
       });
     });
@@ -1560,6 +1560,14 @@ function tokenizeAndHumanizeLineColumn(input: string, options?: lex.TokenizeOpti
       .tokens.map(token => [<any>token.type, humanizeLineColumn(token.sourceSpan.start)]);
 }
 
+function tokenizeAndHumanizeFullStart(input: string, options?: lex.TokenizeOptions): any[] {
+  return tokenizeWithoutErrors(input, options)
+      .tokens.map(
+          token =>
+              [<any>token.type, humanizeLineColumn(token.sourceSpan.start),
+               humanizeLineColumn(token.sourceSpan.fullStart)]);
+}
+
 function tokenizeAndHumanizeErrors(input: string, options?: lex.TokenizeOptions): any[] {
   return lex.tokenize(input, 'someUrl', getHtmlTagDefinition, options)
       .errors.map(e => [<any>e.tokenType, e.msg, humanizeLineColumn(e.span.start)]);

diff --git a/packages/compiler/test/render3/view/i18n_spec.ts b/packages/compiler/test/render3/view/i18n_spec.ts
@@ -18,6 +18,7 @@ import {serializeIcuNode} from '../../../src/render3/view/i18n/icu_serializer';
 import {serializeI18nMessageForLocalize} from '../../../src/render3/view/i18n/localize_utils';
 import {I18nMeta, parseI18nMeta} from '../../../src/render3/view/i18n/meta';
 import {formatI18nPlaceholderName} from '../../../src/render3/view/i18n/util';
+import {LEADING_TRIVIA_CHARS} from '../../../src/render3/view/template';
 
 import {parseR3 as parse} from './util';
 
@@ -386,7 +387,7 @@ describe('serializeI18nMessageForGetMsg', () => {
 
 describe('serializeI18nMessageForLocalize', () => {
   const serialize = (input: string) => {
-    const tree = parse(`<div i18n>${input}</div>`);
+    const tree = parse(`<div i18n>${input}</div>`, {leadingTriviaChars: LEADING_TRIVIA_CHARS});
     const root = tree.nodes[0] as t.Element;
     return serializeI18nMessageForLocalize(root.i18n as i18n.Message);
   };
@@ -477,7 +478,7 @@ describe('serializeI18nMessageForLocalize', () => {
        expect(messageParts[3].text).toEqual('');
        expect(messageParts[3].sourceSpan.toString()).toEqual('');
        expect(messageParts[4].text).toEqual(' D');
-       expect(messageParts[4].sourceSpan.toString()).toEqual(' D');
+       expect(messageParts[4].sourceSpan.toString()).toEqual('D');
 
        expect(placeHolders[0].text).toEqual('START_TAG_SPAN');
        expect(placeHolders[0].sourceSpan.toString()).toEqual('<span>');
@@ -489,14 +490,53 @@ describe('serializeI18nMessageForLocalize', () => {
        expect(placeHolders[3].sourceSpan.toString()).toEqual('</span>');
      });
 
+  it('should create the correct source-spans when there are two placeholders next to each other',
+     () => {
+       const {messageParts, placeHolders} = serialize('<b>{{value}}</b>');
+       expect(messageParts[0].text).toEqual('');
+       expect(humanizeSourceSpan(messageParts[0].sourceSpan)).toEqual('"" (10-10)');
+       expect(messageParts[1].text).toEqual('');
+       expect(humanizeSourceSpan(messageParts[1].sourceSpan)).toEqual('"" (13-13)');
+       expect(messageParts[2].text).toEqual('');
+       expect(humanizeSourceSpan(messageParts[2].sourceSpan)).toEqual('"" (22-22)');
+       expect(messageParts[3].text).toEqual('');
+       expect(humanizeSourceSpan(messageParts[3].sourceSpan)).toEqual('"" (26-26)');
+
+       expect(placeHolders[0].text).toEqual('START_BOLD_TEXT');
+       expect(humanizeSourceSpan(placeHolders[0].sourceSpan)).toEqual('"<b>" (10-13)');
+       expect(placeHolders[1].text).toEqual('INTERPOLATION');
+       expect(humanizeSourceSpan(placeHolders[1].sourceSpan)).toEqual('"{{value}}" (13-22)');
+       expect(placeHolders[2].text).toEqual('CLOSE_BOLD_TEXT');
+       expect(humanizeSourceSpan(placeHolders[2].sourceSpan)).toEqual('"</b>" (22-26)');
+     });
+
+  it('should create the correct placeholder source-spans when there is skipped leading whitespace',
+     () => {
+       const {messageParts, placeHolders} = serialize('<b>   {{value}}</b>');
+       expect(messageParts[0].text).toEqual('');
+       expect(humanizeSourceSpan(messageParts[0].sourceSpan)).toEqual('"" (10-10)');
+       expect(messageParts[1].text).toEqual('   ');
+       expect(humanizeSourceSpan(messageParts[1].sourceSpan)).toEqual('"   " (13-16)');
+       expect(messageParts[2].text).toEqual('');
+       expect(humanizeSourceSpan(messageParts[2].sourceSpan)).toEqual('"" (25-25)');
+       expect(messageParts[3].text).toEqual('');
+       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(placeHolders[1].text).toEqual('INTERPOLATION');
+       expect(humanizeSourceSpan(placeHolders[1].sourceSpan)).toEqual('"{{value}}" (16-25)');
+       expect(placeHolders[2].text).toEqual('CLOSE_BOLD_TEXT');
+       expect(humanizeSourceSpan(placeHolders[2].sourceSpan)).toEqual('"</b>" (25-29)');
+     });
+
   it('should serialize simple ICU for `$localize()`', () => {
     expect(serialize('{age, plural, 10 {ten} other {other}}')).toEqual({
       messageParts: [literal('{VAR_PLURAL, plural, 10 {ten} other {other}}')],
       placeHolders: []
     });
   });
 
-
   it('should serialize nested ICUs for `$localize()`', () => {
     expect(serialize(
                '{age, plural, 10 {ten {size, select, 1 {one} 2 {two} other {2+}}} other {other}}'))
@@ -509,7 +549,6 @@ describe('serializeI18nMessageForLocalize', () => {
         });
   });
 
-
   it('should serialize ICU with embedded HTML for `$localize()`', () => {
     expect(serialize('{age, plural, 10 {<b>ten</b>} other {<div class="A">other</div>}}')).toEqual({
       messageParts: [
@@ -594,4 +633,8 @@ function literal(text: string, span: any = jasmine.any(ParseSourceSpan)): o.Lite
 
 function placeholder(name: string, span: any = jasmine.any(ParseSourceSpan)): o.PlaceholderPiece {
   return new o.PlaceholderPiece(name, span);
-}
+}
+
+function humanizeSourceSpan(span: ParseSourceSpan): string {
+  return `"${span.toString()}" (${span.start.offset}-${span.end.offset})`;
+}
diff --git a/packages/compiler/test/render3/view/util.ts b/packages/compiler/test/render3/view/util.ts
@@ -78,11 +78,13 @@ export function toStringExpression(expr: e.AST): string {
 
 // Parse an html string to IVY specific info
 export function parseR3(
-    input: string, options: {preserveWhitespaces?: boolean} = {}): Render3ParseResult {
+    input: string, options: {preserveWhitespaces?: boolean, leadingTriviaChars?: string[]} = {}):
+    Render3ParseResult {
   const htmlParser = new HtmlParser();
 
-  const parseResult =
-      htmlParser.parse(input, 'path:://to/template', {tokenizeExpansionForms: true});
+  const parseResult = htmlParser.parse(
+      input, 'path:://to/template',
+      {tokenizeExpansionForms: true, leadingTriviaChars: options.leadingTriviaChars});
 
   if (parseResult.errors.length > 0) {
     const msg = parseResult.errors.map(e => e.toString()).join('\n');

diff --git a/packages/core/src/compiler/compiler_facade_interface.ts b/packages/core/src/compiler/compiler_facade_interface.ts
@@ -193,4 +193,5 @@ export interface ParseSourceSpan {
   start: any;
   end: any;
   details: any;
+  fullStart: any;
 }