diff --git a/packages/compiler/src/expression_parser/ast.ts b/packages/compiler/src/expression_parser/ast.ts index 94803c7f61..fb4e05bf07 100644 --- a/packages/compiler/src/expression_parser/ast.ts +++ b/packages/compiler/src/expression_parser/ast.ts @@ -283,10 +283,58 @@ export class ASTWithSource extends AST { toString(): string { return `${this.source} in ${this.location}`; } } -export class TemplateBinding { +/** + * TemplateBinding refers to a particular key-value pair in a microsyntax + * expression. A few examples are: + * + * |---------------------|--------------|---------|--------------| + * | expression | key | value | binding type | + * |---------------------|--------------|---------|--------------| + * | 1. let item | item | null | variable | + * | 2. of items | ngForOf | items | expression | + * | 3. let x = y | x | y | variable | + * | 4. index as i | i | index | variable | + * | 5. trackBy: func | ngForTrackBy | func | expression | + * | 6. *ngIf="cond" | ngIf | cond | expression | + * |---------------------|--------------|---------|--------------| + * + * (6) is a notable exception because it is a binding from the template key in + * the LHS of a HTML attribute to the expression in the RHS. All other bindings + * in the example above are derived solely from the RHS. + */ +export type TemplateBinding = VariableBinding | ExpressionBinding; + +export class VariableBinding { + /** + * @param sourceSpan entire span of the binding. + * @param key name of the LHS along with its span. + * @param value optional value for the RHS along with its span. + */ constructor( - public span: ParseSpan, sourceSpan: AbsoluteSourceSpan, public key: string, - public keyIsVar: boolean, public name: string, public value: ASTWithSource|null) {} + public readonly sourceSpan: AbsoluteSourceSpan, + public readonly key: TemplateBindingIdentifier, + public readonly value: TemplateBindingIdentifier|null) {} +} + +export class ExpressionBinding { + /** + * @param sourceSpan entire span of the binding. + * @param key binding name, like ngForOf, ngForTrackBy, ngIf, along with its + * span. Note that the length of the span may not be the same as + * `key.source.length`. For example, + * 1. key.source = ngFor, key.span is for "ngFor" + * 2. key.source = ngForOf, key.span is for "of" + * 3. key.source = ngForTrackBy, key.span is for "trackBy" + * @param value optional expression for the RHS. + */ + constructor( + public readonly sourceSpan: AbsoluteSourceSpan, + public readonly key: TemplateBindingIdentifier, public readonly value: ASTWithSource|null) {} +} + +export interface TemplateBindingIdentifier { + source: string; + span: AbsoluteSourceSpan; } export interface AstVisitor { diff --git a/packages/compiler/src/expression_parser/parser.ts b/packages/compiler/src/expression_parser/parser.ts index 363aa0545d..bf8da66c8f 100644 --- a/packages/compiler/src/expression_parser/parser.ts +++ b/packages/compiler/src/expression_parser/parser.ts @@ -10,7 +10,7 @@ import * as chars from '../chars'; import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/interpolation_config'; import {escapeRegExp} from '../util'; -import {AST, ASTWithSource, AbsoluteSourceSpan, AstVisitor, Binary, BindingPipe, Chain, Conditional, EmptyExpr, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralMapKey, LiteralPrimitive, MethodCall, NonNullAssert, ParseSpan, ParserError, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead, TemplateBinding} from './ast'; +import {AST, ASTWithSource, AbsoluteSourceSpan, AstVisitor, Binary, BindingPipe, Chain, Conditional, EmptyExpr, ExpressionBinding, FunctionCall, ImplicitReceiver, Interpolation, KeyedRead, KeyedWrite, LiteralArray, LiteralMap, LiteralMapKey, LiteralPrimitive, MethodCall, NonNullAssert, ParseSpan, ParserError, PrefixNot, PropertyRead, PropertyWrite, Quote, SafeMethodCall, SafePropertyRead, TemplateBinding, TemplateBindingIdentifier, VariableBinding} from './ast'; import {EOF, Lexer, Token, TokenType, isIdentifier, isQuote} from './lexer'; export class SplitInterpolation { @@ -125,7 +125,8 @@ export class Parser { * For example, * ``` *
- * ^ `absoluteOffset` for `tplValue` + * ^ ^ absoluteValueOffset for `templateValue` + * absoluteKeyOffset for `templateKey` * ``` * contains three bindings: * 1. ngFor -> null @@ -140,16 +141,20 @@ export class Parser { * @param templateKey name of directive, without the * prefix. For example: ngIf, ngFor * @param templateValue RHS of the microsyntax attribute * @param templateUrl template filename if it's external, component filename if it's inline - * @param absoluteOffset absolute offset of the `tplValue` + * @param absoluteKeyOffset start of the `templateKey` + * @param absoluteValueOffset start of the `templateValue` */ parseTemplateBindings( - templateKey: string, templateValue: string, templateUrl: string, - absoluteOffset: number): TemplateBindingParseResult { + templateKey: string, templateValue: string, templateUrl: string, absoluteKeyOffset: number, + absoluteValueOffset: number): TemplateBindingParseResult { const tokens = this._lexer.tokenize(templateValue); - return new _ParseAST( - templateValue, templateUrl, absoluteOffset, tokens, templateValue.length, - false /* parseAction */, this.errors, 0 /* relative offset */) - .parseTemplateBindings(templateKey); + const parser = new _ParseAST( + templateValue, templateUrl, absoluteValueOffset, tokens, templateValue.length, + false /* parseAction */, this.errors, 0 /* relative offset */); + return parser.parseTemplateBindings({ + source: templateKey, + span: new AbsoluteSourceSpan(absoluteKeyOffset, absoluteKeyOffset + templateKey.length), + }); } parseInterpolation( @@ -302,6 +307,11 @@ export class _ParseAST { this.inputLength + this.offset; } + /** + * Returns the absolute offset of the start of the current token. + */ + get currentAbsoluteOffset(): number { return this.absoluteOffset + this.inputIndex; } + span(start: number) { return new ParseSpan(start, this.inputIndex); } sourceSpan(start: number): AbsoluteSourceSpan { @@ -747,12 +757,13 @@ export class _ParseAST { } /** - * Parses an identifier, a keyword, a string with an optional `-` in between. + * Parses an identifier, a keyword, a string with an optional `-` in between, + * and returns the string along with its absolute source span. */ - expectTemplateBindingKey(): {key: string, keySpan: ParseSpan} { + expectTemplateBindingKey(): TemplateBindingIdentifier { let result = ''; let operatorFound = false; - const start = this.inputIndex; + const start = this.currentAbsoluteOffset; do { result += this.expectIdentifierOrKeywordOrString(); operatorFound = this.consumeOptionalOperator('-'); @@ -761,8 +772,8 @@ export class _ParseAST { } } while (operatorFound); return { - key: result, - keySpan: new ParseSpan(start, start + result.length), + source: result, + span: new AbsoluteSourceSpan(start, start + result.length), }; } @@ -784,16 +795,16 @@ export class _ParseAST { * For a full description of the microsyntax grammar, see * https://gist.github.com/mhevery/d3530294cff2e4a1b3fe15ff75d08855 * - * @param templateKey name of the microsyntax directive, like ngIf, ngFor, without the * + * @param templateKey name of the microsyntax directive, like ngIf, ngFor, + * without the *, along with its absolute span. */ - parseTemplateBindings(templateKey: string): TemplateBindingParseResult { + parseTemplateBindings(templateKey: TemplateBindingIdentifier): TemplateBindingParseResult { const bindings: TemplateBinding[] = []; // The first binding is for the template key itself // In *ngFor="let item of items", key = "ngFor", value = null // In *ngIf="cond | pipe", key = "ngIf", value = "cond | pipe" - bindings.push(...this.parseDirectiveKeywordBindings( - templateKey, new ParseSpan(0, templateKey.length), this.absoluteOffset)); + bindings.push(...this.parseDirectiveKeywordBindings(templateKey)); while (this.index < this.tokens.length) { // If it starts with 'let', then this must be variable declaration @@ -805,18 +816,17 @@ export class _ParseAST { // "directive-keyword expression". We don't know which case, but both // "value" and "directive-keyword" are template binding key, so consume // the key first. - const {key, keySpan} = this.expectTemplateBindingKey(); + const key = this.expectTemplateBindingKey(); // Peek at the next token, if it is "as" then this must be variable // declaration. - const binding = this.parseAsBinding(key, keySpan, this.absoluteOffset); + const binding = this.parseAsBinding(key); if (binding) { bindings.push(binding); } else { // Otherwise the key must be a directive keyword, like "of". Transform // the key to actual key. Eg. of -> ngForOf, trackBy -> ngForTrackBy - const actualKey = templateKey + key[0].toUpperCase() + key.substring(1); - bindings.push( - ...this.parseDirectiveKeywordBindings(actualKey, keySpan, this.absoluteOffset)); + key.source = templateKey.source + key.source[0].toUpperCase() + key.source.substring(1); + bindings.push(...this.parseDirectiveKeywordBindings(key)); } } this.consumeStatementTerminator(); @@ -832,33 +842,33 @@ export class _ParseAST { * There could be an optional "as" binding that follows the expression. * For example, * ``` - * *ngFor="let item of items | slice:0:1 as collection".` - * ^^ ^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^ - * keyword bound target optional 'as' binding + * *ngFor="let item of items | slice:0:1 as collection". + * ^^ ^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^ + * keyword bound target optional 'as' binding * ``` * - * @param key binding key, for example, ngFor, ngIf, ngForOf - * @param keySpan span of the key in the expression. keySpan might be different - * from `key.length`. For example, the span for key "ngForOf" is "of". - * @param absoluteOffset absolute offset of the attribute value + * @param key binding key, for example, ngFor, ngIf, ngForOf, along with its + * absolute span. */ - private parseDirectiveKeywordBindings(key: string, keySpan: ParseSpan, absoluteOffset: number): - TemplateBinding[] { + private parseDirectiveKeywordBindings(key: TemplateBindingIdentifier): TemplateBinding[] { const bindings: TemplateBinding[] = []; this.consumeOptionalCharacter(chars.$COLON); // trackBy: trackByFunction - const valueExpr = this.getDirectiveBoundTarget(); - const span = new ParseSpan(keySpan.start, this.inputIndex); - bindings.push(new TemplateBinding( - span, span.toAbsolute(absoluteOffset), key, false /* keyIsVar */, valueExpr?.source || '', valueExpr)); + const value = this.getDirectiveBoundTarget(); + let spanEnd = this.currentAbsoluteOffset; // The binding could optionally be followed by "as". For example, // *ngIf="cond | pipe as x". In this case, the key in the "as" binding // is "x" and the value is the template key itself ("ngIf"). Note that the // 'key' in the current context now becomes the "value" in the next binding. - const asBinding = this.parseAsBinding(key, keySpan, absoluteOffset); + const asBinding = this.parseAsBinding(key); + if (!asBinding) { + this.consumeStatementTerminator(); + spanEnd = this.currentAbsoluteOffset; + } + const sourceSpan = new AbsoluteSourceSpan(key.span.start, spanEnd); + bindings.push(new ExpressionBinding(sourceSpan, key, value)); if (asBinding) { bindings.push(asBinding); } - this.consumeStatementTerminator(); return bindings; } @@ -866,10 +876,10 @@ export class _ParseAST { * Return the expression AST for the bound target of a directive keyword * binding. For example, * ``` - * *ngIf="condition | pipe". - * ^^^^^^^^^^^^^^^^ bound target for "ngIf" - * *ngFor="let item of items" - * ^^^^^ bound target for "ngForOf" + * *ngIf="condition | pipe" + * ^^^^^^^^^^^^^^^^ bound target for "ngIf" + * *ngFor="let item of items" + * ^^^^^ bound target for "ngForOf" * ``` */ private getDirectiveBoundTarget(): ASTWithSource|null { @@ -877,7 +887,11 @@ export class _ParseAST { return null; } const ast = this.parsePipe(); // example: "condition | async" - const {start, end} = ast.span; + const {start} = ast.span; + // Getting the end of the last token removes trailing whitespace. + // If ast has the correct end span then no need to peek at last token. + // TODO(ayazhafiz): Remove this in https://github.com/angular/angular/pull/34690 + const {end} = this.peek(-1); const value = this.input.substring(start, end); return new ASTWithSource(ast, value, this.location, this.absoluteOffset + start, this.errors); } @@ -886,35 +900,30 @@ export class _ParseAST { * Return the binding for a variable declared using `as`. Note that the order * of the key-value pair in this declaration is reversed. For example, * ``` - * *ngFor="let item of items; index as i" - * ^^^^^ ^ - * value key + * *ngFor="let item of items; index as i" + * ^^^^^ ^ + * value key * ``` * - * @param value name of the value in the declaration, "ngIf" in the example above - * @param valueSpan span of the value in the declaration - * @param absoluteOffset absolute offset of `value` + * @param value name of the value in the declaration, "ngIf" in the example + * above, along with its absolute span. */ - private parseAsBinding(value: string, valueSpan: ParseSpan, absoluteOffset: number): - TemplateBinding|null { + private parseAsBinding(value: TemplateBindingIdentifier): TemplateBinding|null { if (!this.peekKeywordAs()) { return null; } this.advance(); // consume the 'as' keyword - const {key} = this.expectTemplateBindingKey(); - const valueAst = new AST(valueSpan, valueSpan.toAbsolute(absoluteOffset)); - const valueExpr = new ASTWithSource( - valueAst, value, this.location, absoluteOffset + valueSpan.start, this.errors); - const span = new ParseSpan(valueSpan.start, this.inputIndex); - return new TemplateBinding( - span, span.toAbsolute(absoluteOffset), key, true /* keyIsVar */, value, valueExpr); + const key = this.expectTemplateBindingKey(); + this.consumeStatementTerminator(); + const sourceSpan = new AbsoluteSourceSpan(value.span.start, this.currentAbsoluteOffset); + return new VariableBinding(sourceSpan, key, value); } /** * Return the binding for a variable declared using `let`. For example, * ``` - * *ngFor="let item of items; let i=index;" - * ^^^^^^^^ ^^^^^^^^^^^ + * *ngFor="let item of items; let i=index;" + * ^^^^^^^^ ^^^^^^^^^^^ * ``` * In the first binding, `item` is bound to `NgForOfContext.$implicit`. * In the second binding, `i` is bound to `NgForOfContext.index`. @@ -923,20 +932,16 @@ export class _ParseAST { if (!this.peekKeywordLet()) { return null; } - const spanStart = this.inputIndex; + const spanStart = this.currentAbsoluteOffset; this.advance(); // consume the 'let' keyword - const {key} = this.expectTemplateBindingKey(); - let valueExpr: ASTWithSource|null = null; + const key = this.expectTemplateBindingKey(); + let value: TemplateBindingIdentifier|null = null; if (this.consumeOptionalOperator('=')) { - const {key: value, keySpan: valueSpan} = this.expectTemplateBindingKey(); - const ast = new AST(valueSpan, valueSpan.toAbsolute(this.absoluteOffset)); - valueExpr = new ASTWithSource( - ast, value, this.location, this.absoluteOffset + valueSpan.start, this.errors); + value = this.expectTemplateBindingKey(); } - const spanEnd = this.inputIndex; - const span = new ParseSpan(spanStart, spanEnd); - return new TemplateBinding( - span, span.toAbsolute(this.absoluteOffset), key, true /* keyIsVar */, valueExpr?.source || '$implicit', valueExpr); + this.consumeStatementTerminator(); + const sourceSpan = new AbsoluteSourceSpan(spanStart, this.currentAbsoluteOffset); + return new VariableBinding(sourceSpan, key, value); } /** diff --git a/packages/compiler/src/template_parser/binding_parser.ts b/packages/compiler/src/template_parser/binding_parser.ts index bfb311a9a6..19dc950b00 100644 --- a/packages/compiler/src/template_parser/binding_parser.ts +++ b/packages/compiler/src/template_parser/binding_parser.ts @@ -8,7 +8,7 @@ import {CompileDirectiveSummary, CompilePipeSummary} from '../compile_metadata'; import {SecurityContext} from '../core'; -import {ASTWithSource, BindingPipe, BindingType, BoundElementProperty, EmptyExpr, ParsedEvent, ParsedEventType, ParsedProperty, ParsedPropertyType, ParsedVariable, ParserError, RecursiveAstVisitor, TemplateBinding} from '../expression_parser/ast'; +import {ASTWithSource, BindingPipe, BindingType, BoundElementProperty, EmptyExpr, ParsedEvent, ParsedEventType, ParsedProperty, ParsedPropertyType, ParsedVariable, ParserError, RecursiveAstVisitor, TemplateBinding, VariableBinding} from '../expression_parser/ast'; import {Parser} from '../expression_parser/parser'; import {InterpolationConfig} from '../ml_parser/interpolation_config'; import {mergeNsAndName} from '../ml_parser/tags'; @@ -114,8 +114,9 @@ export class BindingParser { } /** - * Parses an inline template binding, e.g. - * + * Parses the bindings in a microsyntax expression, and converts them to + * `ParsedProperty` or `ParsedVariable`. + * * @param tplKey template binding name * @param tplValue template binding value * @param sourceSpan span of template binding relative to entire the template @@ -128,43 +129,51 @@ export class BindingParser { tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan, absoluteValueOffset: number, targetMatchableAttrs: string[][], targetProps: ParsedProperty[], targetVars: ParsedVariable[]) { - const bindings = this._parseTemplateBindings(tplKey, tplValue, sourceSpan, absoluteValueOffset); + const absoluteKeyOffset = sourceSpan.start.offset; + const bindings = this._parseTemplateBindings( + tplKey, tplValue, sourceSpan, absoluteKeyOffset, absoluteValueOffset); for (let i = 0; i < bindings.length; i++) { const binding = bindings[i]; - if (binding.keyIsVar) { - targetVars.push(new ParsedVariable(binding.key, binding.name, sourceSpan)); + const key = binding.key.source; + if (binding instanceof VariableBinding) { + const value = binding.value ? binding.value.source : '$implicit'; + targetVars.push(new ParsedVariable(key, value, sourceSpan)); } else if (binding.value) { this._parsePropertyAst( - binding.key, binding.value, sourceSpan, undefined, targetMatchableAttrs, targetProps); + key, binding.value, sourceSpan, undefined, targetMatchableAttrs, targetProps); } else { - targetMatchableAttrs.push([binding.key, '']); + targetMatchableAttrs.push([key, '']); this.parseLiteralAttr( - binding.key, null, sourceSpan, absoluteValueOffset, undefined, targetMatchableAttrs, + key, null, sourceSpan, absoluteValueOffset, undefined, targetMatchableAttrs, targetProps); } } } /** - * Parses the bindings in an inline template binding, e.g. + * Parses the bindings in a microsyntax expression, e.g. + * ``` * + * ``` + * * @param tplKey template binding name * @param tplValue template binding value * @param sourceSpan span of template binding relative to entire the template - * @param absoluteValueOffset start of the tplValue relative to the entire template + * @param absoluteKeyOffset start of the `tplKey` + * @param absoluteValueOffset start of the `tplValue` */ private _parseTemplateBindings( - tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan, + tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan, absoluteKeyOffset: number, absoluteValueOffset: number): TemplateBinding[] { const sourceInfo = sourceSpan.start.toString(); try { - const bindingsResult = - this._exprParser.parseTemplateBindings(tplKey, tplValue, sourceInfo, absoluteValueOffset); + const bindingsResult = this._exprParser.parseTemplateBindings( + tplKey, tplValue, sourceInfo, absoluteKeyOffset, absoluteValueOffset); this._reportExpressionParserErrors(bindingsResult.errors, sourceSpan); bindingsResult.templateBindings.forEach((binding) => { - if (binding.value) { + if (binding.value instanceof ASTWithSource) { this._checkPipes(binding.value, sourceSpan); } }); diff --git a/packages/compiler/test/expression_parser/parser_spec.ts b/packages/compiler/test/expression_parser/parser_spec.ts index 86ba149418..c03aebbd99 100644 --- a/packages/compiler/test/expression_parser/parser_spec.ts +++ b/packages/compiler/test/expression_parser/parser_spec.ts @@ -6,9 +6,9 @@ * found in the LICENSE file at https://angular.io/license */ -import {ASTWithSource, BindingPipe, Interpolation, ParserError, TemplateBinding} from '@angular/compiler/src/expression_parser/ast'; +import {ASTWithSource, BindingPipe, Interpolation, ParserError, TemplateBinding, VariableBinding} from '@angular/compiler/src/expression_parser/ast'; import {Lexer} from '@angular/compiler/src/expression_parser/lexer'; -import {Parser, SplitInterpolation, TemplateBindingParseResult} from '@angular/compiler/src/expression_parser/parser'; +import {Parser, SplitInterpolation} from '@angular/compiler/src/expression_parser/parser'; import {expect} from '@angular/platform-browser/testing/src/matchers'; @@ -247,200 +247,309 @@ describe('parser', () => { }); describe('parseTemplateBindings', () => { - - function keys(templateBindings: TemplateBinding[]) { - return templateBindings.map(binding => binding.key); - } - - function keyValues(templateBindings: TemplateBinding[]) { - return templateBindings.map(binding => { - if (binding.keyIsVar) { - return 'let ' + binding.key + (binding.name == null ? '=null' : '=' + binding.name); - } else { - return binding.key + (binding.value == null ? '' : `=${binding.value}`); - } - }); - } - - function keySpans(source: string, templateBindings: TemplateBinding[]) { - return templateBindings.map( - binding => source.substring(binding.span.start, binding.span.end)); - } - - function exprSources(templateBindings: TemplateBinding[]) { - return templateBindings.map(binding => binding.value != null ? binding.value.source : null); - } - function humanize(bindings: TemplateBinding[]): Array<[string, string | null, boolean]> { return bindings.map(binding => { - const {key, value: expression, name, keyIsVar} = binding; - const value = keyIsVar ? name : (expression ? expression.source : expression); + const key = binding.key.source; + const value = binding.value ? binding.value.source : null; + const keyIsVar = binding instanceof VariableBinding; return [key, value, keyIsVar]; }); } - it('should parse a key without a value', - () => { expect(keys(parseTemplateBindings('a', ''))).toEqual(['a']); }); + function humanizeSpans( + bindings: TemplateBinding[], attr: string): Array<[string, string, string | null]> { + return bindings.map(binding => { + const {sourceSpan, key, value} = binding; + const sourceStr = attr.substring(sourceSpan.start, sourceSpan.end); + const keyStr = attr.substring(key.span.start, key.span.end); + let valueStr = null; + if (value) { + const {start, end} = value instanceof ASTWithSource ? value.ast.sourceSpan : value.span; + valueStr = attr.substring(start, end); + } + return [sourceStr, keyStr, valueStr]; + }); + } - it('should allow string including dashes as keys', () => { - let bindings = parseTemplateBindings('a', 'b'); - expect(keys(bindings)).toEqual(['a']); - - bindings = parseTemplateBindings('a-b', 'c'); - expect(keys(bindings)).toEqual(['a-b']); + it('should parse key and value', () => { + const cases: Array<[string, string, string | null, boolean, string, string, string | null]> = + [ + // expression, key, value, VariableBinding, source span, key span, value span + ['*a=""', 'a', null, false, 'a="', 'a', null], + ['*a="b"', 'a', 'b', false, 'a="b', 'a', 'b'], + ['*a-b="c"', 'a-b', 'c', false, 'a-b="c', 'a-b', 'c'], + ['*a="1+1"', 'a', '1+1', false, 'a="1+1', 'a', '1+1'], + ]; + for (const [attr, key, value, keyIsVar, sourceSpan, keySpan, valueSpan] of cases) { + const bindings = parseTemplateBindings(attr); + expect(humanize(bindings)).toEqual([ + [key, value, keyIsVar], + ]); + expect(humanizeSpans(bindings, attr)).toEqual([ + [sourceSpan, keySpan, valueSpan], + ]); + } }); - it('should detect expressions as value', () => { - let bindings = parseTemplateBindings('a', 'b'); - expect(exprSources(bindings)).toEqual(['b']); - - bindings = parseTemplateBindings('a', '1+1'); - expect(exprSources(bindings)).toEqual(['1+1']); - }); - - it('should detect names as value', () => { - const bindings = parseTemplateBindings('a', 'let b'); - expect(keyValues(bindings)).toEqual(['a', 'let b=$implicit']); - }); - - it('should allow space and colon as separators', () => { - let bindings = parseTemplateBindings('a', 'b'); - expect(keys(bindings)).toEqual(['a']); - expect(exprSources(bindings)).toEqual(['b']); + it('should variable declared via let', () => { + const bindings = parseTemplateBindings('*a="let b"'); + expect(humanize(bindings)).toEqual([ + // key, value, VariableBinding + ['a', null, false], + ['b', null, true], + ]); }); it('should allow multiple pairs', () => { - const bindings = parseTemplateBindings('a', '1 b 2'); - expect(keys(bindings)).toEqual(['a', 'aB']); - expect(exprSources(bindings)).toEqual(['1 ', '2']); + const bindings = parseTemplateBindings('*a="1 b 2"'); + expect(humanize(bindings)).toEqual([ + // key, value, VariableBinding + ['a', '1', false], + ['aB', '2', false], + ]); }); - it('should store the sources in the result', () => { - const bindings = parseTemplateBindings('a', '1,b 2'); - expect(bindings[0].value !.source).toEqual('1'); - expect(bindings[1].value !.source).toEqual('2'); + it('should allow space and colon as separators', () => { + const bindings = parseTemplateBindings('*a="1,b 2"'); + expect(humanize(bindings)).toEqual([ + // key, value, VariableBinding + ['a', '1', false], + ['aB', '2', false], + ]); }); - it('should store the passed-in location', () => { - const bindings = parseTemplateBindings('a', '1,b 2', 'location'); - expect(bindings[0].value !.location).toEqual('location'); + it('should store the templateUrl', () => { + const bindings = parseTemplateBindings('*a="1,b 2"', '/foo/bar.html'); + expect(humanize(bindings)).toEqual([ + // key, value, VariableBinding + ['a', '1', false], + ['aB', '2', false], + ]); + expect((bindings[0].value as ASTWithSource).location).toEqual('/foo/bar.html'); }); it('should support common usage of ngIf', () => { - const bindings = parseTemplateBindings('ngIf', 'cond | pipe as foo, let x; ngIf as y'); + const bindings = parseTemplateBindings('*ngIf="cond | pipe as foo, let x; ngIf as y"'); expect(humanize(bindings)).toEqual([ - // [ key, value, keyIsVar ] - ['ngIf', 'cond | pipe ', false], + // [ key, value, VariableBinding ] + ['ngIf', 'cond | pipe', false], ['foo', 'ngIf', true], - ['x', '$implicit', true], + ['x', null, true], ['y', 'ngIf', true], ]); }); it('should support common usage of ngFor', () => { let bindings: TemplateBinding[]; - bindings = parseTemplateBindings( - 'ngFor', 'let item; of items | slice:0:1 as collection, trackBy: func; index as i'); + bindings = parseTemplateBindings('*ngFor="let person of people"'); expect(humanize(bindings)).toEqual([ - // [ key, value, keyIsVar ] + // [ key, value, VariableBinding ] ['ngFor', null, false], - ['item', '$implicit', true], - ['ngForOf', 'items | slice:0:1 ', false], + ['person', null, true], + ['ngForOf', 'people', false], + ]); + + + bindings = parseTemplateBindings( + '*ngFor="let item; of items | slice:0:1 as collection, trackBy: func; index as i"'); + expect(humanize(bindings)).toEqual([ + // [ key, value, VariableBinding ] + ['ngFor', null, false], + ['item', null, true], + ['ngForOf', 'items | slice:0:1', false], ['collection', 'ngForOf', true], ['ngForTrackBy', 'func', false], ['i', 'index', true], ]); bindings = parseTemplateBindings( - 'ngFor', 'let item, of: [1,2,3] | pipe as items; let i=index, count as len'); + '*ngFor="let item, of: [1,2,3] | pipe as items; let i=index, count as len"'); expect(humanize(bindings)).toEqual([ - // [ key, value, keyIsVar ] + // [ key, value, VariableBinding ] ['ngFor', null, false], - ['item', '$implicit', true], - ['ngForOf', '[1,2,3] | pipe ', false], + ['item', null, true], + ['ngForOf', '[1,2,3] | pipe', false], ['items', 'ngForOf', true], ['i', 'index', true], ['len', 'count', true], ]); }); - it('should support let notation', () => { - let bindings = parseTemplateBindings('key', 'let i'); - expect(keyValues(bindings)).toEqual(['key', 'let i=$implicit']); - - bindings = parseTemplateBindings('key', 'let a; let b'); - expect(keyValues(bindings)).toEqual([ - 'key', - 'let a=$implicit', - 'let b=$implicit', - ]); - - bindings = parseTemplateBindings('key', 'let a; let b;'); - expect(keyValues(bindings)).toEqual([ - 'key', - 'let a=$implicit', - 'let b=$implicit', - ]); - - bindings = parseTemplateBindings('key', 'let i-a = k-a'); - expect(keyValues(bindings)).toEqual([ - 'key', - 'let i-a=k-a', - ]); - - bindings = parseTemplateBindings('key', 'let item; let i = k'); - expect(keyValues(bindings)).toEqual([ - 'key', - 'let item=$implicit', - 'let i=k', - ]); - - bindings = parseTemplateBindings('directive', 'let item in expr; let a = b', 'location'); - expect(keyValues(bindings)).toEqual([ - 'directive', - 'let item=$implicit', - 'directiveIn=expr in location', - 'let a=b', - ]); - }); - - it('should support as notation', () => { - let bindings = parseTemplateBindings('ngIf', 'exp as local', 'location'); - expect(keyValues(bindings)).toEqual(['ngIf=exp in location', 'let local=ngIf']); - - bindings = parseTemplateBindings('ngFor', 'let item of items as iter; index as i', 'L'); - expect(keyValues(bindings)).toEqual([ - 'ngFor', 'let item=$implicit', 'ngForOf=items in L', 'let iter=ngForOf', 'let i=index' - ]); - }); - it('should parse pipes', () => { - const bindings = parseTemplateBindings('key', 'value|pipe'); - const ast = bindings[0].value !.ast; - expect(ast).toBeAnInstanceOf(BindingPipe); + const bindings = parseTemplateBindings('*key="value|pipe "'); + expect(humanize(bindings)).toEqual([ + // [ key, value, VariableBinding ] + ['key', 'value|pipe', false], + ]); + const {value} = bindings[0]; + expect(value).toBeAnInstanceOf(ASTWithSource); + expect((value as ASTWithSource).ast).toBeAnInstanceOf(BindingPipe); }); - describe('spans', () => { - it('should should support let', () => { - const source = 'let i'; - expect(keySpans(source, parseTemplateBindings('key', 'let i'))).toEqual(['', 'let i']); - }); - - it('should support multiple lets', () => { - const source = 'let item; let i=index; let e=even;'; - expect(keySpans(source, parseTemplateBindings('key', source))).toEqual([ - '', 'let item', 'let i=index', 'let e=even' + describe('"let" binding', () => { + it('should support single declaration', () => { + const bindings = parseTemplateBindings('*key="let i"'); + expect(humanize(bindings)).toEqual([ + // [ key, value, VariableBinding ] + ['key', null, false], + ['i', null, true], ]); }); - it('should support a prefix', () => { - const source = 'let person of people'; - const prefix = 'ngFor'; - const bindings = parseTemplateBindings(prefix, source); - expect(keyValues(bindings)).toEqual([ - 'ngFor', 'let person=$implicit', 'ngForOf=people in null' + it('should support multiple declarations', () => { + const bindings = parseTemplateBindings('*key="let a; let b"'); + expect(humanize(bindings)).toEqual([ + // [ key, value, VariableBinding ] + ['key', null, false], + ['a', null, true], + ['b', null, true], + ]); + }); + + it('should support empty string assignment', () => { + const bindings = parseTemplateBindings(`*key="let a=''; let b='';"`); + expect(humanize(bindings)).toEqual([ + // [ key, value, VariableBinding ] + ['key', null, false], + ['a', '', true], + ['b', '', true], + ]); + }); + + it('should support key and value names with dash', () => { + const bindings = parseTemplateBindings('*key="let i-a = j-a,"'); + expect(humanize(bindings)).toEqual([ + // [ key, value, VariableBinding ] + ['key', null, false], + ['i-a', 'j-a', true], + ]); + }); + + it('should support declarations with or without value assignment', () => { + const bindings = parseTemplateBindings('*key="let item; let i = k"'); + expect(humanize(bindings)).toEqual([ + // [ key, value, VariableBinding ] + ['key', null, false], + ['item', null, true], + ['i', 'k', true], + ]); + }); + + it('should support declaration before an expression', () => { + const bindings = parseTemplateBindings('*directive="let item in expr; let a = b"'); + expect(humanize(bindings)).toEqual([ + // [ key, value, VariableBinding ] + ['directive', null, false], + ['item', null, true], + ['directiveIn', 'expr', false], + ['a', 'b', true], + ]); + }); + }); + + describe('"as" binding', () => { + it('should support single declaration', () => { + const bindings = parseTemplateBindings('*ngIf="exp as local"'); + expect(humanize(bindings)).toEqual([ + // [ key, value, VariableBinding ] + ['ngIf', 'exp', false], + ['local', 'ngIf', true], + ]); + }); + + it('should support declaration after an expression', () => { + const bindings = parseTemplateBindings('*ngFor="let item of items as iter; index as i"'); + expect(humanize(bindings)).toEqual([ + // [ key, value, VariableBinding ] + ['ngFor', null, false], + ['item', null, true], + ['ngForOf', 'items', false], + ['iter', 'ngForOf', true], + ['i', 'index', true], + ]); + }); + + it('should support key and value names with dash', () => { + const bindings = parseTemplateBindings('*key="foo, k-b as l-b;"'); + expect(humanize(bindings)).toEqual([ + // [ key, value, VariableBinding ] + ['key', 'foo', false], + ['l-b', 'k-b', true], + ]); + }); + }); + + describe('source, key, value spans', () => { + it('should map empty expression', () => { + const attr = '*ngIf=""'; + const bindings = parseTemplateBindings(attr); + expect(humanizeSpans(bindings, attr)).toEqual([ + // source span, key span, value span + ['ngIf="', 'ngIf', null], + ]); + }); + + it('should map variable declaration via "let"', () => { + const attr = '*key="let i"'; + const bindings = parseTemplateBindings(attr); + expect(humanizeSpans(bindings, attr)).toEqual([ + // source span, key span, value span + ['key="', 'key', null], // source span stretches till next binding + ['let i', 'i', null], + ]); + }); + + it('shoud map multiple variable declarations via "let"', () => { + const attr = '*key="let item; let i=index; let e=even;"'; + const bindings = parseTemplateBindings(attr); + expect(humanizeSpans(bindings, attr)).toEqual([ + // source span, key span, value span + ['key="', 'key', null], + ['let item; ', 'item', null], + ['let i=index; ', 'i', 'index'], + ['let e=even;', 'e', 'even'], + ]); + }); + + it('shoud map expression with pipe', () => { + const attr = '*ngIf="cond | pipe as foo, let x; ngIf as y"'; + const bindings = parseTemplateBindings(attr); + expect(humanizeSpans(bindings, attr)).toEqual([ + // source span, key span, value span + ['ngIf="cond | pipe ', 'ngIf', 'cond | pipe '], + ['ngIf="cond | pipe as foo, ', 'foo', 'ngIf'], + ['let x; ', 'x', null], + ['ngIf as y', 'y', 'ngIf'], + ]); + }); + + it('should map variable declaration via "as"', () => { + const attr = + '*ngFor="let item; of items | slice:0:1 as collection, trackBy: func; index as i"'; + const bindings = parseTemplateBindings(attr); + expect(humanizeSpans(bindings, attr)).toEqual([ + // source span, key span, value span + ['ngFor="', 'ngFor', null], + ['let item; ', 'item', null], + ['of items | slice:0:1 ', 'of', 'items | slice:0:1 '], + ['of items | slice:0:1 as collection, ', 'collection', 'of'], + ['trackBy: func; ', 'trackBy', 'func'], + ['index as i', 'i', 'index'], + ]); + }); + + it('should map literal array', () => { + const attr = '*ngFor="let item, of: [1,2,3] | pipe as items; let i=index, count as len, "'; + const bindings = parseTemplateBindings(attr); + expect(humanizeSpans(bindings, attr)).toEqual([ + // source span, key span, value span + ['ngFor="', 'ngFor', null], + ['let item, ', 'item', null], + ['of: [1,2,3] | pipe ', 'of', '[1,2,3] | pipe '], + ['of: [1,2,3] | pipe as items; ', 'items', 'of'], + ['let i=index, ', 'i', 'index'], + ['count as len, ', 'len', 'count'], ]); - expect(keySpans(source, bindings)).toEqual(['', 'let person ', 'of people']); }); }); }); @@ -585,14 +694,18 @@ function parseBinding(text: string, location: any = null, offset: number = 0): A return createParser().parseBinding(text, location, offset); } -function parseTemplateBindingsResult( - 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, offset: number = 0): TemplateBinding[] { - return parseTemplateBindingsResult(key, value, location).templateBindings; +function parseTemplateBindings(attribute: string, templateUrl = 'foo.html'): TemplateBinding[] { + const match = attribute.match(/^\*(.+)="(.*)"$/); + expect(match).toBeTruthy(`failed to extract key and value from ${attribute}`); + const [_, key, value] = match; + const absKeyOffset = 1; // skip the * prefix + const absValueOffset = attribute.indexOf('=') + '="'.length; + const parser = createParser(); + const result = + parser.parseTemplateBindings(key, value, templateUrl, absKeyOffset, absValueOffset); + expect(result.errors).toEqual([]); + expect(result.warnings).toEqual([]); + return result.templateBindings; } function parseInterpolation(text: string, location: any = null, offset: number = 0): ASTWithSource| diff --git a/packages/language-service/src/completions.ts b/packages/language-service/src/completions.ts index 3fcf830d6a..6233f013b9 100644 --- a/packages/language-service/src/completions.ts +++ b/packages/language-service/src/completions.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, AstPath, AttrAst, Attribute, BoundDirectivePropertyAst, BoundElementPropertyAst, BoundEventAst, BoundTextAst, Element, ElementAst, HtmlAstPath, NAMED_ENTITIES, Node as HtmlAst, NullTemplateVisitor, ReferenceAst, TagContentType, TemplateBinding, Text, getHtmlTagDefinition} from '@angular/compiler'; +import {AST, ASTWithSource, AstPath, AttrAst, Attribute, BoundDirectivePropertyAst, BoundElementPropertyAst, BoundEventAst, BoundTextAst, Element, ElementAst, HtmlAstPath, NAMED_ENTITIES, Node as HtmlAst, NullTemplateVisitor, ReferenceAst, TagContentType, TemplateBinding, Text, VariableBinding, getHtmlTagDefinition} from '@angular/compiler'; import {$$, $_, isAsciiLetter, isDigit} from '@angular/compiler/src/chars'; import {AstResult} from './common'; @@ -71,6 +71,8 @@ enum ATTR { // Group 10 = identifier inside () IDENT_EVENT_IDX = 10, } +// Microsyntax template starts with '*'. See https://angular.io/api/core/TemplateRef +const TEMPLATE_ATTR_PREFIX = '*'; function isIdentifierPart(code: number) { // Identifiers consist of alphanumeric characters, '_', or '$'. @@ -231,8 +233,7 @@ function attributeCompletions(info: AstResult, path: AstPath): ng.Compl // bind parts for cases like [()|] // ^ cursor is here const bindParts = attr.name.match(BIND_NAME_REGEXP); - // TemplateRef starts with '*'. See https://angular.io/api/core/TemplateRef - const isTemplateRef = attr.name.startsWith('*'); + const isTemplateRef = attr.name.startsWith(TEMPLATE_ATTR_PREFIX); const isBinding = bindParts !== null || isTemplateRef; if (!isBinding) { @@ -450,15 +451,21 @@ class ExpressionVisitor extends NullTemplateVisitor { } visitAttr(ast: AttrAst) { - if (ast.name.startsWith('*')) { + if (ast.name.startsWith(TEMPLATE_ATTR_PREFIX)) { // This a template binding given by micro syntax expression. // First, verify the attribute consists of some binding we can give completions for. + // The sourceSpan of AttrAst points to the RHS of the attribute + const templateKey = ast.name.substring(TEMPLATE_ATTR_PREFIX.length); + const templateValue = ast.sourceSpan.toString(); + const templateUrl = ast.sourceSpan.start.file.url; + // TODO(kyliau): We are unable to determine the absolute offset of the key + // but it is okay here, because we are only looking at the RHS of the attr + const absKeyOffset = 0; + const absValueOffset = ast.sourceSpan.start.offset; const {templateBindings} = this.info.expressionParser.parseTemplateBindings( - ast.name, ast.value, ast.sourceSpan.toString(), ast.sourceSpan.start.offset); - // Find where the cursor is relative to the start of the attribute value. - const valueRelativePosition = this.position - ast.sourceSpan.start.offset; + templateKey, templateValue, templateUrl, absKeyOffset, absValueOffset); // Find the template binding that contains the position. - const binding = templateBindings.find(b => inSpan(valueRelativePosition, b.span)); + const binding = templateBindings.find(b => inSpan(this.position, b.sourceSpan)); if (!binding) { return; @@ -549,7 +556,10 @@ class ExpressionVisitor extends NullTemplateVisitor { const valueRelativePosition = this.position - attr.sourceSpan.start.offset; - if (binding.keyIsVar) { + if (binding instanceof VariableBinding) { + // TODO(kyliau): With expression sourceSpan we shouldn't have to search + // the attribute value string anymore. Just check if position is in the + // expression source span. const equalLocation = attr.value.indexOf('='); if (equalLocation > 0 && valueRelativePosition > equalLocation) { // We are after the '=' in a let clause. The valid values here are the members of the @@ -566,9 +576,8 @@ class ExpressionVisitor extends NullTemplateVisitor { } } } - - if (binding.value && inSpan(valueRelativePosition, binding.value.ast.span)) { - this.processExpressionCompletions(binding.value.ast); + else if (inSpan(valueRelativePosition, binding.value?.ast.span)) { + this.processExpressionCompletions(binding.value !.ast); return; } diff --git a/packages/language-service/src/locate_symbol.ts b/packages/language-service/src/locate_symbol.ts index 57f4b4bae2..138d31e4e5 100644 --- a/packages/language-service/src/locate_symbol.ts +++ b/packages/language-service/src/locate_symbol.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AST, Attribute, BoundDirectivePropertyAst, CssSelector, DirectiveAst, ElementAst, EmbeddedTemplateAst, RecursiveTemplateAstVisitor, SelectorMatcher, StaticSymbol, TemplateAst, TemplateAstPath, templateVisitAll, tokenReference} from '@angular/compiler'; +import {AST, Attribute, BoundDirectivePropertyAst, CssSelector, DirectiveAst, ElementAst, EmbeddedTemplateAst, ExpressionBinding, RecursiveTemplateAstVisitor, SelectorMatcher, StaticSymbol, TemplateAst, TemplateAstPath, VariableBinding, templateVisitAll, tokenReference} from '@angular/compiler'; import * as tss from 'typescript/lib/tsserverlibrary'; import {AstResult} from './common'; @@ -200,33 +200,44 @@ function getSymbolInMicrosyntax(info: AstResult, path: TemplateAstPath, attribut if (!attribute.valueSpan) { return; } + const absValueOffset = attribute.valueSpan.start.offset; let result: {symbol: Symbol, span: Span}|undefined; const {templateBindings} = info.expressionParser.parseTemplateBindings( attribute.name, attribute.value, attribute.sourceSpan.toString(), - attribute.valueSpan.start.offset); - // Find where the cursor is relative to the start of the attribute value. - const valueRelativePosition = path.position - attribute.valueSpan.start.offset; + attribute.sourceSpan.start.offset, attribute.valueSpan.start.offset); // Find the symbol that contains the position. - templateBindings.filter(tb => !tb.keyIsVar).forEach(tb => { - if (inSpan(valueRelativePosition, tb.value?.ast.span)) { + for (const tb of templateBindings) { + if (tb instanceof VariableBinding) { + // TODO(kyliau): if binding is variable we should still look for the value + // of the key. For example, "let i=index" => "index" should point to + // NgForOfContext.index + continue; + } + if (inSpan(path.position, tb.value?.ast.sourceSpan)) { const dinfo = diagnosticInfoFromTemplateInfo(info); const scope = getExpressionScope(dinfo, path); result = getExpressionSymbol(scope, tb.value !, path.position, info.template.query); - } else if (inSpan(valueRelativePosition, tb.span)) { + } else if (inSpan(path.position, tb.sourceSpan)) { const template = path.first(EmbeddedTemplateAst); if (template) { // One element can only have one template binding. const directiveAst = template.directives[0]; if (directiveAst) { - const symbol = findInputBinding(info, tb.key.substring(1), directiveAst); + const symbol = findInputBinding(info, tb.key.source.substring(1), directiveAst); if (symbol) { - result = {symbol, span: tb.span}; + result = { + symbol, + // the span here has to be relative to the start of the template + // value so deduct the absolute offset. + // TODO(kyliau): Use absolute source span throughout completions. + span: offsetSpan(tb.key.span, -absValueOffset), + }; } } } } - }); + } return result; } diff --git a/packages/language-service/test/definitions_spec.ts b/packages/language-service/test/definitions_spec.ts index 43bdf3e561..69d34a2f38 100644 --- a/packages/language-service/test/definitions_spec.ts +++ b/packages/language-service/test/definitions_spec.ts @@ -310,9 +310,7 @@ describe('definitions', () => { }); it('should be able to find the directive property', () => { - mockHost.override( - TEST_TEMPLATE, - `
`); + mockHost.override(TEST_TEMPLATE, `
`); // Get the marker for trackBy in the code added above. const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'trackBy'); @@ -322,8 +320,7 @@ describe('definitions', () => { const {textSpan, definitions} = result !; // Get the marker for bounded text in the code added above - const boundedText = mockHost.getLocationMarkerFor(TEST_TEMPLATE, 'my'); - expect(textSpan).toEqual(boundedText); + expect(textSpan).toEqual(marker); expect(definitions).toBeDefined(); // The two definitions are setter and getter of 'ngForTrackBy'. diff --git a/packages/language-service/test/hover_spec.ts b/packages/language-service/test/hover_spec.ts index 9f144a6624..dc3bb7098b 100644 --- a/packages/language-service/test/hover_spec.ts +++ b/packages/language-service/test/hover_spec.ts @@ -118,9 +118,8 @@ describe('hover', () => { }); it('should work for structural directive inputs', () => { - mockHost.override( - TEST_TEMPLATE, `
`); - const marker = mockHost.getDefinitionMarkerFor(TEST_TEMPLATE, 'trackBy'); + mockHost.override(TEST_TEMPLATE, `
`); + const marker = mockHost.getReferenceMarkerFor(TEST_TEMPLATE, 'trackBy'); const quickInfo = ngLS.getQuickInfoAtPosition(TEST_TEMPLATE, marker.start); expect(quickInfo).toBeTruthy(); const {textSpan, displayParts} = quickInfo !;