fix(compiler): Don't set expression text to synthetic `$implicit` when 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 https://github.com/angular/vscode-ng-language-service/issues/1077
Fixes https://github.com/angular/vscode-ng-language-service/issues/1078

PR Close #40583
This commit is contained in:
Andrew Scott 2021-01-26 12:45:18 -08:00 committed by Misko Hevery
parent b9f04bf5bb
commit 6b4909c588
3 changed files with 26 additions and 13 deletions

View File

@ -243,14 +243,12 @@ export class Parser {
const fullEnd = exprEnd + interpEnd.length; const fullEnd = exprEnd + interpEnd.length;
const text = input.substring(exprStart, exprEnd); const text = input.substring(exprStart, exprEnd);
if (text.trim().length > 0) { if (text.trim().length === 0) {
expressions.push({text, start: fullStart, end: fullEnd});
} else {
this._reportError( this._reportError(
'Blank expressions are not allowed in interpolated strings', input, 'Blank expressions are not allowed in interpolated strings', input,
`at column ${i} in`, location); `at column ${i} in`, location);
expressions.push({text: '$implicit', start: fullStart, end: fullEnd});
} }
expressions.push({text, start: fullStart, end: fullEnd});
offsets.push(exprStart); offsets.push(exprStart);
i = fullEnd; i = fullEnd;
@ -571,7 +569,14 @@ export class _ParseAST {
this.error(`Unexpected token '${this.next}'`); 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]; if (exprs.length == 1) return exprs[0];
return new Chain(this.span(start), this.sourceSpan(start), exprs); return new Chain(this.span(start), this.sourceSpan(start), exprs);
} }

View File

@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license * 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 {Lexer} from '@angular/compiler/src/expression_parser/lexer';
import {IvyParser, Parser, SplitInterpolation} from '@angular/compiler/src/expression_parser/parser'; import {IvyParser, Parser, SplitInterpolation} from '@angular/compiler/src/expression_parser/parser';
import {expect} from '@angular/platform-browser/testing/src/matchers'; import {expect} from '@angular/platform-browser/testing/src/matchers';
@ -910,6 +910,12 @@ describe('parser', () => {
'Parser Error: Blank expressions are not allowed in interpolated strings'); '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', () => { it('should parse conditional expression', () => {
checkInterpolation('{{ a < b ? a : b }}'); checkInterpolation('{{ a < b ? a : b }}');
}); });

View File

@ -220,12 +220,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
const {componentContext, templateContext} = completions; const {componentContext, templateContext} = completions;
let replacementSpan: ts.TextSpan|undefined = undefined; const replacementSpan = makeReplacementSpanFromAst(this.node);
// 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);
}
// Merge TS completion results with results from the template scope. // Merge TS completion results with results from the template scope.
let entries: ts.CompletionEntry[] = []; let entries: ts.CompletionEntry[] = [];
@ -631,7 +626,14 @@ function makeReplacementSpanFromParseSourceSpan(span: ParseSourceSpan): ts.TextS
} }
function makeReplacementSpanFromAst(node: PropertyRead|PropertyWrite|MethodCall|SafePropertyRead| 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 { return {
start: node.nameSpan.start, start: node.nameSpan.start,
length: node.nameSpan.end - node.nameSpan.start, length: node.nameSpan.end - node.nameSpan.start,