Skip to content

Commit

Permalink
fix(compiler): Don't set expression text to synthetic $implicit whe…
Browse files Browse the repository at this point in the history
…n empty (#40583)

When parsing interpolations, if we encounter an empty interpolation
(`{{}}`), the current code uses a "pretend" value of `$implicit` for the
name as if the interplotion were really `{{$implicit}}`. This is
problematic because the spans are then incorrect downstream since they
are based off of the `$implicit` text.

This commit changes the interpretation of empty interpolations so that
the text is simply an empty string.

Fixes angular/vscode-ng-language-service#1077
Fixes angular/vscode-ng-language-service#1078

PR Close #40583
  • Loading branch information
atscott authored and mhevery committed Jan 28, 2021
1 parent daa9b7e commit 9d396f8
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 13 deletions.
15 changes: 10 additions & 5 deletions packages/compiler/src/expression_parser/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,14 +243,12 @@ export class Parser {
const fullEnd = exprEnd + interpEnd.length;

const text = input.substring(exprStart, exprEnd);
if (text.trim().length > 0) {
expressions.push({text, start: fullStart, end: fullEnd});
} else {
if (text.trim().length === 0) {
this._reportError(
'Blank expressions are not allowed in interpolated strings', input,
`at column ${i} in`, location);
expressions.push({text: '$implicit', start: fullStart, end: fullEnd});
}
expressions.push({text, start: fullStart, end: fullEnd});
offsets.push(exprStart);

i = fullEnd;
Expand Down Expand Up @@ -571,7 +569,14 @@ export class _ParseAST {
this.error(`Unexpected token '${this.next}'`);
}
}
if (exprs.length == 0) return new EmptyExpr(this.span(start), this.sourceSpan(start));
if (exprs.length == 0) {
// We have no expressions so create an empty expression that spans the entire input length
const artificialStart = this.offset;
const artificialEnd = this.offset + this.inputLength;
return new EmptyExpr(
this.span(artificialStart, artificialEnd),
this.sourceSpan(artificialStart, artificialEnd));
}
if (exprs.length == 1) return exprs[0];
return new Chain(this.span(start), this.sourceSpan(start), exprs);
}
Expand Down
8 changes: 7 additions & 1 deletion packages/compiler/test/expression_parser/parser_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {AbsoluteSourceSpan, ASTWithSource, BindingPipe, Interpolation, ParserError, TemplateBinding, VariableBinding} from '@angular/compiler/src/expression_parser/ast';
import {AbsoluteSourceSpan, ASTWithSource, BindingPipe, EmptyExpr, Interpolation, ParserError, TemplateBinding, VariableBinding} from '@angular/compiler/src/expression_parser/ast';
import {Lexer} from '@angular/compiler/src/expression_parser/lexer';
import {IvyParser, Parser, SplitInterpolation} from '@angular/compiler/src/expression_parser/parser';
import {expect} from '@angular/platform-browser/testing/src/matchers';
Expand Down Expand Up @@ -910,6 +910,12 @@ describe('parser', () => {
'Parser Error: Blank expressions are not allowed in interpolated strings');
});

it('should produce an empty expression ast for empty interpolations', () => {
const parsed = parseInterpolation('{{}}')!.ast as Interpolation;
expect(parsed.expressions.length).toBe(1);
expect(parsed.expressions[0]).toBeAnInstanceOf(EmptyExpr);
});

it('should parse conditional expression', () => {
checkInterpolation('{{ a < b ? a : b }}');
});
Expand Down
16 changes: 9 additions & 7 deletions packages/language-service/ivy/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,12 +220,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {

const {componentContext, templateContext} = completions;

let replacementSpan: ts.TextSpan|undefined = undefined;
// Non-empty nodes get replaced with the completion.
if (!(this.node instanceof EmptyExpr || this.node instanceof LiteralPrimitive ||
this.node instanceof BoundEvent)) {
replacementSpan = makeReplacementSpanFromAst(this.node);
}
const replacementSpan = makeReplacementSpanFromAst(this.node);

// Merge TS completion results with results from the template scope.
let entries: ts.CompletionEntry[] = [];
Expand Down Expand Up @@ -631,7 +626,14 @@ function makeReplacementSpanFromParseSourceSpan(span: ParseSourceSpan): ts.TextS
}

function makeReplacementSpanFromAst(node: PropertyRead|PropertyWrite|MethodCall|SafePropertyRead|
SafeMethodCall|BindingPipe): ts.TextSpan {
SafeMethodCall|BindingPipe|EmptyExpr|LiteralPrimitive|
BoundEvent): ts.TextSpan|undefined {
if ((node instanceof EmptyExpr || node instanceof LiteralPrimitive ||
node instanceof BoundEvent)) {
// empty nodes do not replace any existing text
return undefined;
}

return {
start: node.nameSpan.start,
length: node.nameSpan.end - node.nameSpan.start,
Expand Down

0 comments on commit 9d396f8

Please sign in to comment.