Skip to content

Commit 5dab521

Browse files
atscottmhevery
authored andcommitted
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 angular/vscode-ng-language-service#1077 Fixes angular/vscode-ng-language-service#1078 PR Close #40583
1 parent bf12f52 commit 5dab521

File tree

3 files changed

+26
-13
lines changed

3 files changed

+26
-13
lines changed

packages/compiler/src/expression_parser/parser.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -243,14 +243,12 @@ export class Parser {
243243
const fullEnd = exprEnd + interpEnd.length;
244244

245245
const text = input.substring(exprStart, exprEnd);
246-
if (text.trim().length > 0) {
247-
expressions.push({text, start: fullStart, end: fullEnd});
248-
} else {
246+
if (text.trim().length === 0) {
249247
this._reportError(
250248
'Blank expressions are not allowed in interpolated strings', input,
251249
`at column ${i} in`, location);
252-
expressions.push({text: '$implicit', start: fullStart, end: fullEnd});
253250
}
251+
expressions.push({text, start: fullStart, end: fullEnd});
254252
offsets.push(exprStart);
255253

256254
i = fullEnd;
@@ -571,7 +569,14 @@ export class _ParseAST {
571569
this.error(`Unexpected token '${this.next}'`);
572570
}
573571
}
574-
if (exprs.length == 0) return new EmptyExpr(this.span(start), this.sourceSpan(start));
572+
if (exprs.length == 0) {
573+
// We have no expressions so create an empty expression that spans the entire input length
574+
const artificialStart = this.offset;
575+
const artificialEnd = this.offset + this.inputLength;
576+
return new EmptyExpr(
577+
this.span(artificialStart, artificialEnd),
578+
this.sourceSpan(artificialStart, artificialEnd));
579+
}
575580
if (exprs.length == 1) return exprs[0];
576581
return new Chain(this.span(start), this.sourceSpan(start), exprs);
577582
}

packages/compiler/test/expression_parser/parser_spec.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

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

913+
it('should produce an empty expression ast for empty interpolations', () => {
914+
const parsed = parseInterpolation('{{}}')!.ast as Interpolation;
915+
expect(parsed.expressions.length).toBe(1);
916+
expect(parsed.expressions[0]).toBeAnInstanceOf(EmptyExpr);
917+
});
918+
913919
it('should parse conditional expression', () => {
914920
checkInterpolation('{{ a < b ? a : b }}');
915921
});

packages/language-service/ivy/completions.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -220,12 +220,7 @@ export class CompletionBuilder<N extends TmplAstNode|AST> {
220220

221221
const {componentContext, templateContext} = completions;
222222

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

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

633628
function makeReplacementSpanFromAst(node: PropertyRead|PropertyWrite|MethodCall|SafePropertyRead|
634-
SafeMethodCall|BindingPipe): ts.TextSpan {
629+
SafeMethodCall|BindingPipe|EmptyExpr|LiteralPrimitive|
630+
BoundEvent): ts.TextSpan|undefined {
631+
if ((node instanceof EmptyExpr || node instanceof LiteralPrimitive ||
632+
node instanceof BoundEvent)) {
633+
// empty nodes do not replace any existing text
634+
return undefined;
635+
}
636+
635637
return {
636638
start: node.nameSpan.start,
637639
length: node.nameSpan.end - node.nameSpan.start,

0 commit comments

Comments
 (0)