From 8d90c1ad97439ad657b14557c93e809c32ec31ac Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 28 Oct 2020 21:35:06 +0000 Subject: [PATCH] refactor(compiler): store the `fullStart` location on `ParseSourceSpan`s (#39486) The lexer is able to skip leading trivia in the `start` location of tokens. This makes the source-span more friendly since things like elements appear to begin at the start of the opening tag, rather than at the start of any leading whitespace, which could include newlines. But some tooling requires the full source-span to be available, such as when tokenizing a text span into an Angular expression. This commit simply adds the `fullStart` location to the `ParseSourceSpan` class, and ensures that places where such spans are cloned, this property flows through too. PR Close #39486 --- .../compiler/src/compiler_facade_interface.ts | 1 + .../src/compiler_util/expression_converter.ts | 3 ++- packages/compiler/src/ml_parser/parser.ts | 19 +++++++++----- packages/compiler/src/parse_util.ts | 26 ++++++++++++++++++- .../src/render3/r3_template_transform.ts | 2 +- .../src/render3/view/i18n/localize_utils.ts | 3 ++- .../src/template_parser/binding_parser.ts | 4 ++- .../src/template_parser/template_parser.ts | 2 +- .../src/compiler/compiler_facade_interface.ts | 1 + 9 files changed, 49 insertions(+), 12 deletions(-) diff --git a/packages/compiler/src/compiler_facade_interface.ts b/packages/compiler/src/compiler_facade_interface.ts index dc0253771c..dd46e3adaf 100644 --- 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 index 58017f5c73..ecda7aa218 100644 --- 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/ml_parser/parser.ts b/packages/compiler/src/ml_parser/parser.ts index 1194a8b7ba..2bd172921e 100644 --- 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 index 25ec56e797..190233f413 100644 --- 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 index 8f058b676c..4d2525d817 100644 --- 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 index b57593e4d8..85b4178edf 100644 --- 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); } /** diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts index 81e0b69b1d..f810d55099 100644 --- 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 index 19dfe8f384..098e8f4979 100644 --- 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/core/src/compiler/compiler_facade_interface.ts b/packages/core/src/compiler/compiler_facade_interface.ts index dc0253771c..dd46e3adaf 100644 --- 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; }