From f65db20c6d499b5cbaf6f3d3b2c118c791d06980 Mon Sep 17 00:00:00 2001 From: Ayaz Hafiz Date: Tue, 16 Jul 2019 12:18:32 -0700 Subject: [PATCH] feat(ivy): record absolute position of template expressions (#31391) Currently, template expressions and statements have their location recorded relative to the HTML element they are in, with no handle to absolute location in a source file except for a line/column location. However, the line/column location is also not entirely accurate, as it points an entire semantic expression, and not necessarily the start of an expression recorded by the expression parser. To support record of the source code expressions originate from, add a new `sourceSpan` field to `ASTWithSource` that records the absolute byte offset of an expression within a source code. Implement part 2 of [refactoring template parsing for stability](https://hackmd.io/@X3ECPVy-RCuVfba-pnvIpw/BkDUxaW84/%2FMA1oxh6jRXqSmZBcLfYdyw?type=book). PR Close #31391 --- .../src/ngtsc/annotations/src/base_def.ts | 4 +- .../src/ngtsc/annotations/src/directive.ts | 7 +- .../compiler/src/expression_parser/ast.ts | 12 ++- .../compiler/src/expression_parser/parser.ts | 49 ++++++------ packages/compiler/src/parse_util.ts | 5 +- .../src/render3/r3_template_transform.ts | 26 ++++--- .../src/template_parser/binding_parser.ts | 58 +++++++++------ .../src/template_parser/template_parser.ts | 24 +++--- .../test/expression_parser/parser_spec.ts | 32 ++++---- .../test/render3/r3_ast_absolute_span_spec.ts | 74 +++++++++++++++++++ .../compiler/test/render3/view/i18n_spec.ts | 12 +-- packages/language-service/src/completions.ts | 2 +- 12 files changed, 210 insertions(+), 95 deletions(-) create mode 100644 packages/compiler/test/render3/r3_ast_absolute_span_spec.ts diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts b/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts index 2fb78c0d93..2e8e33b8b0 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/base_def.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ConstantPool, R3BaseRefMetaData, WrappedNodeExpr, compileBaseDefFromMetadata, makeBindingParser} from '@angular/compiler'; +import {ConstantPool, EMPTY_SOURCE_SPAN, R3BaseRefMetaData, WrappedNodeExpr, compileBaseDefFromMetadata, makeBindingParser} from '@angular/compiler'; import {PartialEvaluator} from '../../partial_evaluator'; import {ClassDeclaration, ClassMember, Decorator, ReflectionHost} from '../../reflection'; @@ -94,7 +94,7 @@ export class BaseDefDecoratorHandler implements const analysis: R3BaseRefMetaData = { name: node.name.text, type: new WrappedNodeExpr(node.name), - typeSourceSpan: null ! + typeSourceSpan: EMPTY_SOURCE_SPAN, }; if (metadata.inputs) { diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts index 307970172d..b5e27c50a4 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/directive.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ConstantPool, Expression, ParseError, ParsedHostBindings, R3DirectiveMetadata, R3QueryMetadata, Statement, WrappedNodeExpr, compileDirectiveFromMetadata, makeBindingParser, parseHostBindings, verifyHostBindings} from '@angular/compiler'; +import {ConstantPool, EMPTY_SOURCE_SPAN, Expression, ParseError, ParsedHostBindings, R3DirectiveMetadata, R3QueryMetadata, Statement, WrappedNodeExpr, compileDirectiveFromMetadata, makeBindingParser, parseHostBindings, verifyHostBindings} from '@angular/compiler'; import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; @@ -227,7 +227,7 @@ export function extractDirectiveMetadata( outputs: {...outputsFromMeta, ...outputsFromFields}, queries, viewQueries, selector, type: new WrappedNodeExpr(clazz.name), typeArgumentCount: reflector.getGenericArityOfClass(clazz) || 0, - typeSourceSpan: null !, usesInheritance, exportAs, providers + typeSourceSpan: EMPTY_SOURCE_SPAN, usesInheritance, exportAs, providers }; return {decoratedElements, decorator: directive, metadata}; } @@ -503,7 +503,8 @@ export function extractHostBindings( const bindings = parseHostBindings(hostMetadata); // TODO: create and provide proper sourceSpan to make error message more descriptive (FW-995) - const errors = verifyHostBindings(bindings, /* sourceSpan */ null !); + // For now, pass an incorrect (empty) but valid sourceSpan. + const errors = verifyHostBindings(bindings, EMPTY_SOURCE_SPAN); if (errors.length > 0) { throw new FatalDiagnosticError( // TODO: provide more granular diagnostic and output specific host expression that triggered diff --git a/packages/compiler/src/expression_parser/ast.ts b/packages/compiler/src/expression_parser/ast.ts index 7756fde607..545c9e2a21 100644 --- a/packages/compiler/src/expression_parser/ast.ts +++ b/packages/compiler/src/expression_parser/ast.ts @@ -203,11 +203,21 @@ export class FunctionCall extends AST { } } +/** + * Records the absolute position of a text span in a source file, where `start` and `end` are the + * starting and ending byte offsets, respectively, of the text span in a source file. + */ +export class AbsoluteSourceSpan { + constructor(public start: number, public end: number) {} +} + export class ASTWithSource extends AST { + public sourceSpan: AbsoluteSourceSpan; constructor( - public ast: AST, public source: string|null, public location: string, + public ast: AST, public source: string|null, public location: string, absoluteOffset: number, public errors: ParserError[]) { super(new ParseSpan(0, source == null ? 0 : source.length)); + this.sourceSpan = new AbsoluteSourceSpan(absoluteOffset, absoluteOffset + this.span.end); } visit(visitor: AstVisitor, context: any = null): any { if (visitor.visitASTWithSource) { diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts index c6be7f5886..01b3092d49 100644 --- a/packages/compiler/src/expression_parser/parser.ts +++ b/packages/compiler/src/expression_parser/parser.ts @@ -34,35 +34,35 @@ export class Parser { constructor(private _lexer: Lexer) {} parseAction( - input: string, location: any, + input: string, location: any, absoluteOffset: number, 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, tokens, sourceToLex.length, true, this.errors, + input, location, absoluteOffset, tokens, sourceToLex.length, true, this.errors, input.length - sourceToLex.length) .parseChain(); - return new ASTWithSource(ast, input, location, this.errors); + return new ASTWithSource(ast, input, location, absoluteOffset, this.errors); } parseBinding( - input: string, location: any, + input: string, location: any, absoluteOffset: number, interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): ASTWithSource { - const ast = this._parseBindingAst(input, location, interpolationConfig); - return new ASTWithSource(ast, input, location, this.errors); + const ast = this._parseBindingAst(input, location, absoluteOffset, interpolationConfig); + return new ASTWithSource(ast, input, location, absoluteOffset, this.errors); } parseSimpleBinding( - input: string, location: string, + input: string, location: string, absoluteOffset: number, interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): ASTWithSource { - const ast = this._parseBindingAst(input, location, interpolationConfig); + const ast = this._parseBindingAst(input, location, absoluteOffset, interpolationConfig); const errors = SimpleExpressionChecker.check(ast); if (errors.length > 0) { this._reportError( `Host binding expression cannot contain ${errors.join(' ')}`, input, location); } - return new ASTWithSource(ast, input, location, this.errors); + return new ASTWithSource(ast, input, location, absoluteOffset, this.errors); } private _reportError(message: string, input: string, errLocation: string, ctxLocation?: any) { @@ -70,7 +70,8 @@ export class Parser { } private _parseBindingAst( - input: string, location: string, interpolationConfig: InterpolationConfig): AST { + input: string, location: string, absoluteOffset: number, + interpolationConfig: InterpolationConfig): AST { // Quotes expressions use 3rd-party expression language. We don't want to use // our lexer or parser for that, so we check for that ahead of time. const quote = this._parseQuote(input, location); @@ -83,7 +84,7 @@ export class Parser { const sourceToLex = this._stripComments(input); const tokens = this._lexer.tokenize(sourceToLex); return new _ParseAST( - input, location, tokens, sourceToLex.length, false, this.errors, + input, location, absoluteOffset, tokens, sourceToLex.length, false, this.errors, input.length - sourceToLex.length) .parseChain(); } @@ -98,15 +99,16 @@ export class Parser { return new Quote(new ParseSpan(0, input.length), prefix, uninterpretedExpression, location); } - parseTemplateBindings(tplKey: string, tplValue: string, location: any): + parseTemplateBindings(tplKey: string, tplValue: string, location: any, absoluteOffset: number): TemplateBindingParseResult { const tokens = this._lexer.tokenize(tplValue); - return new _ParseAST(tplValue, location, tokens, tplValue.length, false, this.errors, 0) + return new _ParseAST( + tplValue, location, absoluteOffset, tokens, tplValue.length, false, this.errors, 0) .parseTemplateBindings(tplKey); } parseInterpolation( - input: string, location: any, + input: string, location: any, absoluteOffset: number, interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): ASTWithSource|null { const split = this.splitInterpolation(input, location, interpolationConfig); if (split == null) return null; @@ -118,8 +120,8 @@ export class Parser { const sourceToLex = this._stripComments(expressionText); const tokens = this._lexer.tokenize(sourceToLex); const ast = new _ParseAST( - input, location, tokens, sourceToLex.length, false, this.errors, - split.offsets[i] + (expressionText.length - sourceToLex.length)) + input, location, absoluteOffset, tokens, sourceToLex.length, false, + this.errors, split.offsets[i] + (expressionText.length - sourceToLex.length)) .parseChain(); expressions.push(ast); } @@ -127,7 +129,7 @@ export class Parser { return new ASTWithSource( new Interpolation( new ParseSpan(0, input == null ? 0 : input.length), split.strings, expressions), - input, location, this.errors); + input, location, absoluteOffset, this.errors); } splitInterpolation( @@ -166,10 +168,10 @@ export class Parser { return new SplitInterpolation(strings, expressions, offsets); } - wrapLiteralPrimitive(input: string|null, location: any): ASTWithSource { + wrapLiteralPrimitive(input: string|null, location: any, absoluteOffset: number): ASTWithSource { return new ASTWithSource( new LiteralPrimitive(new ParseSpan(0, input == null ? 0 : input.length), input), input, - location, this.errors); + location, absoluteOffset, this.errors); } private _stripComments(input: string): string { @@ -228,9 +230,9 @@ export class _ParseAST { index: number = 0; constructor( - public input: string, public location: any, public tokens: Token[], - public inputLength: number, public parseAction: boolean, private errors: ParserError[], - private offset: number) {} + public input: string, public location: any, public absoluteOffset: number, + public tokens: Token[], public inputLength: number, public parseAction: boolean, + private errors: ParserError[], private offset: number) {} peek(offset: number): Token { const i = this.index + offset; @@ -716,7 +718,8 @@ export class _ParseAST { const start = this.inputIndex; const ast = this.parsePipe(); const source = this.input.substring(start - this.offset, this.inputIndex - this.offset); - expression = new ASTWithSource(ast, source, this.location, this.errors); + expression = + new ASTWithSource(ast, source, this.location, this.absoluteOffset, this.errors); } bindings.push(new TemplateBinding(this.span(start), key, isVar, name, expression)); diff --git a/packages/compiler/src/parse_util.ts b/packages/compiler/src/parse_util.ts index 4e1fc7d5c5..8935dfc26a 100644 --- a/packages/compiler/src/parse_util.ts +++ b/packages/compiler/src/parse_util.ts @@ -109,6 +109,9 @@ export class ParseSourceSpan { } } +export const EMPTY_PARSE_LOCATION = new ParseLocation(new ParseSourceFile('', ''), 0, 0, 0); +export const EMPTY_SOURCE_SPAN = new ParseSourceSpan(EMPTY_PARSE_LOCATION, EMPTY_PARSE_LOCATION); + export enum ParseErrorLevel { WARNING, ERROR, @@ -154,4 +157,4 @@ export function r3JitTypeSourceSpan( const sourceFile = new ParseSourceFile('', sourceFileName); return new ParseSourceSpan( new ParseLocation(sourceFile, -1, -1, -1), new ParseLocation(sourceFile, -1, -1, -1)); -} \ No newline at end of file +} diff --git a/packages/compiler/src/render3/r3_template_transform.ts b/packages/compiler/src/render3/r3_template_transform.ts index 83521634e5..ab22c7d2aa 100644 --- a/packages/compiler/src/render3/r3_template_transform.ts +++ b/packages/compiler/src/render3/r3_template_transform.ts @@ -141,9 +141,11 @@ class HtmlAstToIvyAst implements html.Visitor { const templateKey = normalizedName.substring(TEMPLATE_ATTR_PREFIX.length); const parsedVariables: ParsedVariable[] = []; + const absoluteOffset = attribute.valueSpan ? attribute.valueSpan.start.offset : + attribute.sourceSpan.start.offset; this.bindingParser.parseInlineTemplateBinding( - templateKey, templateValue, attribute.sourceSpan, [], templateParsedProperties, - parsedVariables); + templateKey, templateValue, attribute.sourceSpan, absoluteOffset, [], + templateParsedProperties, parsedVariables); templateVariables.push( ...parsedVariables.map(v => new t.Variable(v.name, v.value, v.sourceSpan))); } else { @@ -285,6 +287,8 @@ class HtmlAstToIvyAst implements html.Visitor { const name = normalizeAttributeName(attribute.name); const value = attribute.value; const srcSpan = attribute.sourceSpan; + const absoluteOffset = + attribute.valueSpan ? attribute.valueSpan.start.offset : srcSpan.start.offset; const bindParts = name.match(BIND_NAME_REGEXP); let hasBinding = false; @@ -293,7 +297,8 @@ class HtmlAstToIvyAst implements html.Visitor { hasBinding = true; if (bindParts[KW_BIND_IDX] != null) { this.bindingParser.parsePropertyBinding( - bindParts[IDENT_KW_IDX], value, false, srcSpan, matchableAttributes, parsedProperties); + bindParts[IDENT_KW_IDX], value, false, srcSpan, absoluteOffset, matchableAttributes, + parsedProperties); } else if (bindParts[KW_LET_IDX]) { if (isTemplateElement) { @@ -315,26 +320,27 @@ class HtmlAstToIvyAst implements html.Visitor { addEvents(events, boundEvents); } else if (bindParts[KW_BINDON_IDX]) { this.bindingParser.parsePropertyBinding( - bindParts[IDENT_KW_IDX], value, false, srcSpan, matchableAttributes, parsedProperties); + bindParts[IDENT_KW_IDX], value, false, srcSpan, absoluteOffset, matchableAttributes, + parsedProperties); this.parseAssignmentEvent( bindParts[IDENT_KW_IDX], value, srcSpan, attribute.valueSpan, matchableAttributes, boundEvents); } else if (bindParts[KW_AT_IDX]) { this.bindingParser.parseLiteralAttr( - name, value, srcSpan, matchableAttributes, parsedProperties); + name, value, srcSpan, absoluteOffset, matchableAttributes, parsedProperties); } else if (bindParts[IDENT_BANANA_BOX_IDX]) { this.bindingParser.parsePropertyBinding( - bindParts[IDENT_BANANA_BOX_IDX], value, false, srcSpan, matchableAttributes, - parsedProperties); + bindParts[IDENT_BANANA_BOX_IDX], value, false, srcSpan, absoluteOffset, + matchableAttributes, parsedProperties); this.parseAssignmentEvent( bindParts[IDENT_BANANA_BOX_IDX], value, srcSpan, attribute.valueSpan, matchableAttributes, boundEvents); } else if (bindParts[IDENT_PROPERTY_IDX]) { this.bindingParser.parsePropertyBinding( - bindParts[IDENT_PROPERTY_IDX], value, false, srcSpan, matchableAttributes, - parsedProperties); + bindParts[IDENT_PROPERTY_IDX], value, false, srcSpan, absoluteOffset, + matchableAttributes, parsedProperties); } else if (bindParts[IDENT_EVENT_IDX]) { const events: ParsedEvent[] = []; @@ -450,4 +456,4 @@ function textContents(node: html.Element): string|null { } else { return (node.children[0] as html.Text).value; } -} \ No newline at end of file +} diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts index 1940825967..999197252f 100644 --- a/packages/compiler/src/template_parser/binding_parser.ts +++ b/packages/compiler/src/template_parser/binding_parser.ts @@ -56,7 +56,8 @@ export class BindingParser { Object.keys(dirMeta.hostProperties).forEach(propName => { const expression = dirMeta.hostProperties[propName]; if (typeof expression === 'string') { - this.parsePropertyBinding(propName, expression, true, sourceSpan, [], boundProps); + this.parsePropertyBinding( + propName, expression, true, sourceSpan, sourceSpan.start.offset, [], boundProps); } else { this._reportError( `Value of the host property binding "${propName}" needs to be a string representing an expression but got "${expression}" (${typeof expression})`, @@ -100,20 +101,20 @@ export class BindingParser { const sourceInfo = sourceSpan.start.toString(); try { - const ast = - this._exprParser.parseInterpolation(value, sourceInfo, this._interpolationConfig) !; + const ast = this._exprParser.parseInterpolation( + value, sourceInfo, sourceSpan.start.offset, 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); + return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo, sourceSpan.start.offset); } } // Parse an inline template binding. ie `` parseInlineTemplateBinding( - tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan, + tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan, absoluteOffset: number, targetMatchableAttrs: string[][], targetProps: ParsedProperty[], targetVars: ParsedVariable[]) { const bindings = this._parseTemplateBindings(tplKey, tplValue, sourceSpan); @@ -127,7 +128,8 @@ export class BindingParser { binding.key, binding.expression, sourceSpan, targetMatchableAttrs, targetProps); } else { targetMatchableAttrs.push([binding.key, '']); - this.parseLiteralAttr(binding.key, null, sourceSpan, targetMatchableAttrs, targetProps); + this.parseLiteralAttr( + binding.key, null, sourceSpan, absoluteOffset, targetMatchableAttrs, targetProps); } } } @@ -137,7 +139,8 @@ export class BindingParser { const sourceInfo = sourceSpan.start.toString(); try { - const bindingsResult = this._exprParser.parseTemplateBindings(tplKey, tplValue, sourceInfo); + const bindingsResult = this._exprParser.parseTemplateBindings( + tplKey, tplValue, sourceInfo, sourceSpan.start.offset); this._reportExpressionParserErrors(bindingsResult.errors, sourceSpan); bindingsResult.templateBindings.forEach((binding) => { if (binding.expression) { @@ -154,7 +157,7 @@ export class BindingParser { } parseLiteralAttr( - name: string, value: string|null, sourceSpan: ParseSourceSpan, + name: string, value: string|null, sourceSpan: ParseSourceSpan, absoluteOffset: number, targetMatchableAttrs: string[][], targetProps: ParsedProperty[]) { if (isAnimationLabel(name)) { name = name.substring(1); @@ -164,17 +167,18 @@ export class BindingParser { ` Use property bindings (e.g. [@prop]="exp") or use an attribute without a value (e.g. @prop) instead.`, sourceSpan, ParseErrorLevel.ERROR); } - this._parseAnimation(name, value, sourceSpan, targetMatchableAttrs, targetProps); + this._parseAnimation( + name, value, sourceSpan, absoluteOffset, targetMatchableAttrs, targetProps); } else { targetProps.push(new ParsedProperty( - name, this._exprParser.wrapLiteralPrimitive(value, ''), ParsedPropertyType.LITERAL_ATTR, - sourceSpan)); + name, this._exprParser.wrapLiteralPrimitive(value, '', absoluteOffset), + ParsedPropertyType.LITERAL_ATTR, sourceSpan)); } } parsePropertyBinding( name: string, expression: string, isHost: boolean, sourceSpan: ParseSourceSpan, - targetMatchableAttrs: string[][], targetProps: ParsedProperty[]) { + absoluteOffset: number, targetMatchableAttrs: string[][], targetProps: ParsedProperty[]) { let isAnimationProp = false; if (name.startsWith(ANIMATE_PROP_PREFIX)) { isAnimationProp = true; @@ -185,10 +189,11 @@ export class BindingParser { } if (isAnimationProp) { - this._parseAnimation(name, expression, sourceSpan, targetMatchableAttrs, targetProps); + this._parseAnimation( + name, expression, sourceSpan, absoluteOffset, targetMatchableAttrs, targetProps); } else { this._parsePropertyAst( - name, this._parseBinding(expression, isHost, sourceSpan), sourceSpan, + name, this._parseBinding(expression, isHost, sourceSpan, absoluteOffset), sourceSpan, targetMatchableAttrs, targetProps); } } @@ -212,30 +217,33 @@ export class BindingParser { } private _parseAnimation( - name: string, expression: string|null, sourceSpan: ParseSourceSpan, + name: string, expression: string|null, sourceSpan: ParseSourceSpan, absoluteOffset: number, targetMatchableAttrs: string[][], targetProps: ParsedProperty[]) { // This will occur when a @trigger is not paired with an expression. // For animations it is valid to not have an expression since */void // states will be applied by angular when the element is attached/detached - const ast = this._parseBinding(expression || 'undefined', false, sourceSpan); + const ast = this._parseBinding(expression || 'undefined', false, sourceSpan, absoluteOffset); targetMatchableAttrs.push([name, ast.source !]); targetProps.push(new ParsedProperty(name, ast, ParsedPropertyType.ANIMATION, sourceSpan)); } - private _parseBinding(value: string, isHostBinding: boolean, sourceSpan: ParseSourceSpan): - ASTWithSource { + private _parseBinding( + value: string, isHostBinding: boolean, sourceSpan: ParseSourceSpan, + absoluteOffset: number): ASTWithSource { const sourceInfo = (sourceSpan && sourceSpan.start || '(unknown)').toString(); try { const ast = isHostBinding ? - this._exprParser.parseSimpleBinding(value, sourceInfo, this._interpolationConfig) : - this._exprParser.parseBinding(value, sourceInfo, this._interpolationConfig); + this._exprParser.parseSimpleBinding( + value, sourceInfo, absoluteOffset, this._interpolationConfig) : + this._exprParser.parseBinding( + 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); + return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo, absoluteOffset); } } @@ -362,21 +370,23 @@ export class BindingParser { private _parseAction(value: string, sourceSpan: ParseSourceSpan): ASTWithSource { const sourceInfo = (sourceSpan && sourceSpan.start || '(unknown').toString(); + const absoluteOffset = (sourceSpan && sourceSpan.start) ? sourceSpan.start.offset : 0; try { - const ast = this._exprParser.parseAction(value, sourceInfo, this._interpolationConfig); + const ast = this._exprParser.parseAction( + value, sourceInfo, absoluteOffset, this._interpolationConfig); if (ast) { this._reportExpressionParserErrors(ast.errors, sourceSpan); } if (!ast || ast.ast instanceof EmptyExpr) { this._reportError(`Empty expressions are not allowed`, sourceSpan); - return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo); + return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo, absoluteOffset); } this._checkPipes(ast, sourceSpan); return ast; } catch (e) { this._reportError(`${e}`, sourceSpan); - return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo); + return this._exprParser.wrapLiteralPrimitive('ERROR', sourceInfo, absoluteOffset); } } diff --git a/packages/compiler/src/template_parser/template_parser.ts b/packages/compiler/src/template_parser/template_parser.ts index f7af554933..a1de634d35 100644 --- a/packages/compiler/src/template_parser/template_parser.ts +++ b/packages/compiler/src/template_parser/template_parser.ts @@ -306,8 +306,8 @@ class TemplateParseVisitor implements html.Visitor { hasInlineTemplates = true; const parsedVariables: ParsedVariable[] = []; this._bindingParser.parseInlineTemplateBinding( - templateKey !, templateValue !, attr.sourceSpan, templateMatchableAttrs, - templateElementOrDirectiveProps, parsedVariables); + templateKey !, templateValue !, attr.sourceSpan, attr.sourceSpan.start.offset, + templateMatchableAttrs, templateElementOrDirectiveProps, parsedVariables); templateElementVars.push(...parsedVariables.map(v => t.VariableAst.fromParsedVariable(v))); } @@ -416,6 +416,7 @@ class TemplateParseVisitor implements html.Visitor { const name = this._normalizeAttributeName(attr.name); const value = attr.value; const srcSpan = attr.sourceSpan; + const absoluteOffset = attr.valueSpan ? attr.valueSpan.start.offset : srcSpan.start.offset; const boundEvents: ParsedEvent[] = []; const bindParts = name.match(BIND_NAME_REGEXP); @@ -425,7 +426,8 @@ class TemplateParseVisitor implements html.Visitor { hasBinding = true; if (bindParts[KW_BIND_IDX] != null) { this._bindingParser.parsePropertyBinding( - bindParts[IDENT_KW_IDX], value, false, srcSpan, targetMatchableAttrs, targetProps); + bindParts[IDENT_KW_IDX], value, false, srcSpan, absoluteOffset, targetMatchableAttrs, + targetProps); } else if (bindParts[KW_LET_IDX]) { if (isTemplateElement) { @@ -446,27 +448,28 @@ class TemplateParseVisitor implements html.Visitor { } else if (bindParts[KW_BINDON_IDX]) { this._bindingParser.parsePropertyBinding( - bindParts[IDENT_KW_IDX], value, false, srcSpan, targetMatchableAttrs, targetProps); + bindParts[IDENT_KW_IDX], value, false, srcSpan, absoluteOffset, targetMatchableAttrs, + targetProps); this._parseAssignmentEvent( bindParts[IDENT_KW_IDX], value, srcSpan, attr.valueSpan || srcSpan, targetMatchableAttrs, boundEvents); } else if (bindParts[KW_AT_IDX]) { this._bindingParser.parseLiteralAttr( - name, value, srcSpan, targetMatchableAttrs, targetProps); + name, value, srcSpan, absoluteOffset, targetMatchableAttrs, targetProps); } else if (bindParts[IDENT_BANANA_BOX_IDX]) { this._bindingParser.parsePropertyBinding( - bindParts[IDENT_BANANA_BOX_IDX], value, false, srcSpan, targetMatchableAttrs, - targetProps); + bindParts[IDENT_BANANA_BOX_IDX], value, false, srcSpan, absoluteOffset, + targetMatchableAttrs, targetProps); this._parseAssignmentEvent( bindParts[IDENT_BANANA_BOX_IDX], value, srcSpan, attr.valueSpan || srcSpan, targetMatchableAttrs, boundEvents); } else if (bindParts[IDENT_PROPERTY_IDX]) { this._bindingParser.parsePropertyBinding( - bindParts[IDENT_PROPERTY_IDX], value, false, srcSpan, targetMatchableAttrs, - targetProps); + bindParts[IDENT_PROPERTY_IDX], value, false, srcSpan, absoluteOffset, + targetMatchableAttrs, targetProps); } else if (bindParts[IDENT_EVENT_IDX]) { this._bindingParser.parseEvent( @@ -479,7 +482,8 @@ class TemplateParseVisitor implements html.Visitor { } if (!hasBinding) { - this._bindingParser.parseLiteralAttr(name, value, srcSpan, targetMatchableAttrs, targetProps); + this._bindingParser.parseLiteralAttr( + name, value, srcSpan, absoluteOffset, targetMatchableAttrs, targetProps); } targetEvents.push(...boundEvents.map(e => t.BoundEventAst.fromParsedEvent(e))); diff --git a/packages/compiler/test/expression_parser/parser_spec.ts b/packages/compiler/test/expression_parser/parser_spec.ts index e028ea9c3e..fb5ac23530 100644 --- a/packages/compiler/test/expression_parser/parser_spec.ts +++ b/packages/compiler/test/expression_parser/parser_spec.ts @@ -434,7 +434,8 @@ describe('parser', () => { it('should support custom interpolation', () => { const parser = new Parser(new Lexer()); - const ast = parser.parseInterpolation('{% a %}', null, {start: '{%', end: '%}'}) !.ast as any; + const ast = + parser.parseInterpolation('{% a %}', null, 0, {start: '{%', end: '%}'}) !.ast as any; expect(ast.strings).toEqual(['', '']); expect(ast.expressions.length).toEqual(1); expect(ast.expressions[0].name).toEqual('a'); @@ -492,7 +493,8 @@ describe('parser', () => { describe('wrapLiteralPrimitive', () => { it('should wrap a literal primitive', () => { - expect(unparse(validate(createParser().wrapLiteralPrimitive('foo', null)))).toEqual('"foo"'); + expect(unparse(validate(createParser().wrapLiteralPrimitive('foo', null, 0)))) + .toEqual('"foo"'); }); }); @@ -528,33 +530,35 @@ function createParser() { return new Parser(new Lexer()); } -function parseAction(text: string, location: any = null): ASTWithSource { - return createParser().parseAction(text, location); +function parseAction(text: string, location: any = null, offset: number = 0): ASTWithSource { + return createParser().parseAction(text, location, offset); } -function parseBinding(text: string, location: any = null): ASTWithSource { - return createParser().parseBinding(text, location); +function parseBinding(text: string, location: any = null, offset: number = 0): ASTWithSource { + return createParser().parseBinding(text, location, offset); } function parseTemplateBindingsResult( - key: string, value: string, location: any = null): TemplateBindingParseResult { - return createParser().parseTemplateBindings(key, value, location); + key: string, value: string, location: any = null, + offset: number = 0): TemplateBindingParseResult { + return createParser().parseTemplateBindings(key, value, location, offset); } function parseTemplateBindings( - key: string, value: string, location: any = null): TemplateBinding[] { + key: string, value: string, location: any = null, offset: number = 0): TemplateBinding[] { return parseTemplateBindingsResult(key, value, location).templateBindings; } -function parseInterpolation(text: string, location: any = null): ASTWithSource|null { - return createParser().parseInterpolation(text, location); +function parseInterpolation(text: string, location: any = null, offset: number = 0): ASTWithSource| + null { + return createParser().parseInterpolation(text, location, offset); } function splitInterpolation(text: string, location: any = null): SplitInterpolation|null { return createParser().splitInterpolation(text, location); } -function parseSimpleBinding(text: string, location: any = null): ASTWithSource { - return createParser().parseSimpleBinding(text, location); +function parseSimpleBinding(text: string, location: any = null, offset: number = 0): ASTWithSource { + return createParser().parseSimpleBinding(text, location, offset); } function checkInterpolation(exp: string, expected?: string) { @@ -595,4 +599,4 @@ function expectActionError(text: string, message: string) { function expectBindingError(text: string, message: string) { expectError(validate(parseBinding(text)), message); -} \ No newline at end of file +} diff --git a/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts b/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts new file mode 100644 index 0000000000..96fa208c3c --- /dev/null +++ b/packages/compiler/test/render3/r3_ast_absolute_span_spec.ts @@ -0,0 +1,74 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {ASTWithSource, AbsoluteSourceSpan, NullAstVisitor} from '@angular/compiler'; +import * as t from '../../src/render3/r3_ast'; +import {parseR3 as parse} from './view/util'; + +class ExpressionLocationHumanizer extends NullAstVisitor implements t.Visitor { + result: any[] = []; + + visitASTWithSource(ast: ASTWithSource) { this.result.push([ast.source, ast.sourceSpan]); } + + visitTemplate(ast: t.Template) { t.visitAll(this, ast.children); } + visitElement(ast: t.Element) { + t.visitAll(this, ast.children); + t.visitAll(this, ast.inputs); + t.visitAll(this, ast.outputs); + } + visitReference(ast: t.Reference) {} + visitVariable(ast: t.Variable) {} + visitEvent(ast: t.BoundEvent) { ast.handler.visit(this); } + visitTextAttribute(ast: t.TextAttribute) {} + visitBoundAttribute(ast: t.BoundAttribute) { ast.value.visit(this); } + visitBoundEvent(ast: t.BoundEvent) { ast.handler.visit(this); } + visitBoundText(ast: t.BoundText) { ast.value.visit(this); } + visitContent(ast: t.Content) {} + visitText(ast: t.Text) {} + visitIcu(ast: t.Icu) {} +} + +function humanizeExpressionLocation(templateAsts: t.Node[]): any[] { + const humanizer = new ExpressionLocationHumanizer(); + t.visitAll(humanizer, templateAsts); + return humanizer.result; +} + +describe('expression AST absolute source spans', () => { + // TODO(ayazhafiz): duplicate this test without `preserveWhitespaces` once whitespace rewriting is + // moved to post-R3AST generation. + it('should provide absolute offsets with arbitrary whitespace', () => { + expect(humanizeExpressionLocation( + parse('
\n \n{{foo}}
', {preserveWhitespaces: true}).nodes)) + .toContain(['\n \n{{foo}}', new AbsoluteSourceSpan(5, 16)]); + }); + + it('should provide absolute offsets of an expression in a bound text', () => { + expect(humanizeExpressionLocation(parse('
{{foo}}
').nodes)).toContain([ + '{{foo}}', new AbsoluteSourceSpan(5, 12) + ]); + }); + + it('should provide absolute offsets of an expression in a bound event', () => { + expect(humanizeExpressionLocation(parse('
').nodes)) + .toContain(['foo();bar();', new AbsoluteSourceSpan(14, 26)]); + + expect(humanizeExpressionLocation(parse('
').nodes)) + .toContain(['foo();bar();', new AbsoluteSourceSpan(15, 27)]); + }); + + it('should provide absolute offsets of an expression in a bound attribute', () => { + expect( + humanizeExpressionLocation(parse('').nodes)) + .toContain(['condition ? true : false', new AbsoluteSourceSpan(19, 43)]); + + expect(humanizeExpressionLocation( + parse('').nodes)) + .toContain(['condition ? true : false', new AbsoluteSourceSpan(22, 46)]); + }); +}); diff --git a/packages/compiler/test/render3/view/i18n_spec.ts b/packages/compiler/test/render3/view/i18n_spec.ts index fa1a1a5214..1ce50ed7d0 100644 --- a/packages/compiler/test/render3/view/i18n_spec.ts +++ b/packages/compiler/test/render3/view/i18n_spec.ts @@ -48,8 +48,8 @@ describe('I18nContext', () => { // binding collection checks expect(ctx.bindings.size).toBe(0); - ctx.appendBinding(expressionParser.parseInterpolation('{{ valueA }}', '') as AST); - ctx.appendBinding(expressionParser.parseInterpolation('{{ valueB }}', '') as AST); + ctx.appendBinding(expressionParser.parseInterpolation('{{ valueA }}', '', 0) as AST); + ctx.appendBinding(expressionParser.parseInterpolation('{{ valueB }}', '', 0) as AST); expect(ctx.bindings.size).toBe(2); }); @@ -76,7 +76,7 @@ describe('I18nContext', () => { // set data for root ctx ctx.appendBoundText(i18nOf(boundTextA)); - ctx.appendBinding(expressionParser.parseInterpolation('{{ valueA }}', '') as AST); + ctx.appendBinding(expressionParser.parseInterpolation('{{ valueA }}', '', 0) as AST); ctx.appendElement(i18nOf(elementA), 0); ctx.appendTemplate(i18nOf(templateA), 1); ctx.appendElement(i18nOf(elementA), 0, true); @@ -92,11 +92,11 @@ describe('I18nContext', () => { // set data for child context childCtx.appendElement(i18nOf(elementB), 0); childCtx.appendBoundText(i18nOf(boundTextB)); - childCtx.appendBinding(expressionParser.parseInterpolation('{{ valueB }}', '') as AST); + childCtx.appendBinding(expressionParser.parseInterpolation('{{ valueB }}', '', 0) as AST); childCtx.appendElement(i18nOf(elementC), 1); childCtx.appendElement(i18nOf(elementC), 1, true); childCtx.appendBoundText(i18nOf(boundTextC)); - childCtx.appendBinding(expressionParser.parseInterpolation('{{ valueC }}', '') as AST); + childCtx.appendBinding(expressionParser.parseInterpolation('{{ valueC }}', '', 0) as AST); childCtx.appendElement(i18nOf(elementB), 0, true); expect(childCtx.bindings.size).toBe(2); @@ -255,4 +255,4 @@ describe('Serializer', () => { cases.forEach(([input, output]) => { expect(serialize(input)).toEqual(output); }); }); -}); \ No newline at end of file +}); diff --git a/packages/language-service/src/completions.ts b/packages/language-service/src/completions.ts index 5a663bd045..c595c8db1a 100644 --- a/packages/language-service/src/completions.ts +++ b/packages/language-service/src/completions.ts @@ -313,7 +313,7 @@ class ExpressionVisitor extends NullTemplateVisitor { selectors.filter(s => s.attrs.some((attr, i) => i % 2 == 0 && attr == key))[0]; const templateBindingResult = - this.info.expressionParser.parseTemplateBindings(key, this.attr.value, null); + this.info.expressionParser.parseTemplateBindings(key, this.attr.value, null, 0); // find the template binding that contains the position if (!this.attr.valueSpan) return;