Visitar URL original
fix(vscode-extension): handle template literals when detecting suppor… · angular/angular@f2cf65a · GitHub
Skip to content

Commit f2cf65a

Browse files
aparziatscott
aparzi
authored and
atscott
committed
fix(vscode-extension): handle template literals when detecting supported decorator fields
The client decides whether a position in a TypeScript file belongs to a supported `@Component` field (`template`, `styles`, ...) with a lightweight `ts.Scanner` loop before forwarding requests to the language server and the HTML/CSS providers. `ts.Scanner#scan` doesn't handle template substitutions on its own: the `}` closing a `${...}` substitution is reported as a plain `CloseBraceToken` and the rest of the template literal is scanned as the start of a new one, swallowing everything up to the next backtick. As a result, once a template literal with substitutions appears in a file, every position after it is considered to be inside a template string, and TypeScript completions get polluted with HTML completions. The scanner is now driven the same way the TypeScript parser does it, re-scanning the brace that closes a substitution as a `TemplateMiddle`/`TemplateTail` token. In addition, the property assignment context previously only ended at a terminator found at the top level of the file, so it leaked from `template:` to every string literal that followed it inside the same decorator and class body. The context now ends at the first terminator found at the nesting depth where it was entered. Two kinds of strings were only recognized thanks to that leak, and only when declared after `template`: inline `styles` and the arguments of `@HostBinding`/`@HostListener` decorators, both of which the language service supports. They are now recognized explicitly, regardless of where they appear. Fixes #65494
1 parent 467b37b commit f2cf65a

2 files changed

Lines changed: 262 additions & 30 deletions

File tree

‎vscode-ng-language-service/client/src/embedded_support.ts‎

Lines changed: 123 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,14 @@ import * as vscode from 'vscode';
1111
const ANGULAR_PROPERTY_ASSIGNMENTS = new Set([
1212
'template',
1313
'templateUrl',
14+
'styles',
1415
'styleUrls',
1516
'styleUrl',
1617
'host',
1718
]);
1819

20+
const ANGULAR_HOST_BINDING_DECORATORS = new Set(['HostBinding', 'HostListener']);
21+
1922
/**
2023
* Determines if the position is inside a decorator
2124
* property that supports language service features.
@@ -27,10 +30,11 @@ export function isNotTypescriptOrSupportedDecoratorField(
2730
if (document.languageId !== 'typescript') {
2831
return true;
2932
}
30-
return isPropertyAssignmentToStringOrStringInArray(
33+
return isInsideAngularStringLiteral(
3134
document.getText(),
3235
document.offsetAt(position),
3336
ANGULAR_PROPERTY_ASSIGNMENTS,
37+
ANGULAR_HOST_BINDING_DECORATORS,
3438
);
3539
}
3640

@@ -65,27 +69,21 @@ export function isInsideStringLiteral(
6569
return true;
6670
}
6771
const offset = document.offsetAt(position);
68-
const scanner = ts.createScanner(ts.ScriptTarget.ESNext, true /* skipTrivia */);
69-
scanner.setText(document.getText());
72+
const {scanner, scan} = createTemplateAwareScanner(document.getText());
7073

71-
let token: ts.SyntaxKind = scanner.scan();
74+
let token: ts.SyntaxKind = scan();
7275
while (token !== ts.SyntaxKind.EndOfFileToken && scanner.getStartPos() < offset) {
73-
const isStringToken =
74-
token === ts.SyntaxKind.StringLiteral ||
75-
token === ts.SyntaxKind.NoSubstitutionTemplateLiteral;
76-
const isCursorInToken =
77-
scanner.getStartPos() <= offset &&
78-
scanner.getStartPos() + scanner.getTokenText().length >= offset;
79-
if (isCursorInToken && isStringToken) {
76+
if (isStringToken(token) && isCursorInToken(scanner, offset)) {
8077
return true;
8178
}
82-
token = scanner.scan();
79+
token = scan();
8380
}
8481
return false;
8582
}
8683

8784
/**
88-
* Basic scanner to determine if we're inside a string of a property with one of the given names.
85+
* Basic scanner to determine if we're inside a string of a property with one of the given names,
86+
* or inside a string argument of a decorator with one of the given names.
8987
*
9088
* This scanner is not currently robust or perfect but provides us with an accurate answer _most_ of
9189
* the time.
@@ -95,24 +93,32 @@ export function isInsideStringLiteral(
9593
* `@Component` or `{styleUrls: [someFunction('stringL¦iteral')]}`, the @angular/language-service
9694
* will always give us the correct answer. This helper gives us a quick win for optimizing the
9795
* number of requests we send to the server.
98-
*
99-
* TODO(atscott): tagged templates don't work: #1872 /
100-
* https://github.com/Microsoft/TypeScript/issues/20055
10196
*/
102-
function isPropertyAssignmentToStringOrStringInArray(
97+
function isInsideAngularStringLiteral(
10398
documentText: string,
10499
offset: number,
105100
propertyAssignmentNames: Set<string>,
101+
decoratorNames: Set<string>,
106102
): boolean {
107-
const scanner = ts.createScanner(ts.ScriptTarget.ESNext, true /* skipTrivia */);
108-
scanner.setText(documentText);
103+
const {scanner, scan} = createTemplateAwareScanner(documentText);
109104

110-
let token: ts.SyntaxKind = scanner.scan();
105+
let token: ts.SyntaxKind = scan();
111106
let lastToken: ts.SyntaxKind | undefined;
112107
let lastTokenText: string | undefined;
108+
let secondLastToken: ts.SyntaxKind | undefined;
113109
let unclosedBraces = 0;
114110
let unclosedBrackets = 0;
111+
let unclosedParens = 0;
115112
let propertyAssignmentContext = false;
113+
// Nesting depth at which the current property assignment context was entered. The context ends
114+
// at the first terminator found at that same depth, i.e. the one that ends the assignment's
115+
// initializer rather than one nested inside of it.
116+
let contextBraces = 0;
117+
let contextBrackets = 0;
118+
let decoratorContext = false;
119+
// Parenthesis depth at which the current decorator context was entered. The context ends at the
120+
// closing parenthesis of the decorator call.
121+
let contextParens = 0;
116122
while (token !== ts.SyntaxKind.EndOfFileToken && scanner.getStartPos() < offset) {
117123
if (
118124
lastToken === ts.SyntaxKind.Identifier &&
@@ -121,36 +127,64 @@ function isPropertyAssignmentToStringOrStringInArray(
121127
propertyAssignmentNames.has(lastTokenText)
122128
) {
123129
propertyAssignmentContext = true;
124-
token = scanner.scan();
130+
contextBraces = unclosedBraces;
131+
contextBrackets = unclosedBrackets;
132+
token = scan();
125133
continue;
126134
}
127-
if (unclosedBraces === 0 && unclosedBrackets === 0 && isPropertyAssignmentTerminator(token)) {
135+
if (
136+
propertyAssignmentContext &&
137+
unclosedBraces === contextBraces &&
138+
unclosedBrackets === contextBrackets &&
139+
isPropertyAssignmentTerminator(token)
140+
) {
128141
propertyAssignmentContext = false;
129142
}
130143

144+
if (
145+
secondLastToken === ts.SyntaxKind.AtToken &&
146+
lastToken === ts.SyntaxKind.Identifier &&
147+
lastTokenText !== undefined &&
148+
token === ts.SyntaxKind.OpenParenToken &&
149+
decoratorNames.has(lastTokenText)
150+
) {
151+
decoratorContext = true;
152+
contextParens = unclosedParens;
153+
}
154+
if (
155+
decoratorContext &&
156+
token === ts.SyntaxKind.CloseParenToken &&
157+
unclosedParens === contextParens + 1
158+
) {
159+
decoratorContext = false;
160+
}
161+
131162
if (token === ts.SyntaxKind.OpenBracketToken) {
132163
unclosedBrackets++;
133164
} else if (token === ts.SyntaxKind.OpenBraceToken) {
134165
unclosedBraces++;
166+
} else if (token === ts.SyntaxKind.OpenParenToken) {
167+
unclosedParens++;
135168
} else if (token === ts.SyntaxKind.CloseBracketToken) {
136169
unclosedBrackets--;
137170
} else if (token === ts.SyntaxKind.CloseBraceToken) {
138171
unclosedBraces--;
172+
} else if (token === ts.SyntaxKind.CloseParenToken) {
173+
unclosedParens--;
139174
}
140175

141-
const isStringToken =
142-
token === ts.SyntaxKind.StringLiteral ||
143-
token === ts.SyntaxKind.NoSubstitutionTemplateLiteral;
144-
const isCursorInToken =
145-
scanner.getStartPos() <= offset &&
146-
scanner.getStartPos() + scanner.getTokenText().length >= offset;
147-
if (propertyAssignmentContext && isCursorInToken && isStringToken) {
176+
if (
177+
(propertyAssignmentContext || decoratorContext) &&
178+
isStringToken(token) &&
179+
isCursorInToken(scanner, offset)
180+
) {
148181
return true;
149182
}
150183

151184
lastTokenText = scanner.getTokenText();
185+
secondLastToken = lastToken;
152186
lastToken = token;
153-
token = scanner.scan();
187+
token = scan();
154188
}
155189

156190
return false;
@@ -164,3 +198,62 @@ function isPropertyAssignmentTerminator(token: ts.SyntaxKind) {
164198
token === ts.SyntaxKind.CloseBraceToken
165199
);
166200
}
201+
202+
function isStringToken(token: ts.SyntaxKind): boolean {
203+
return (
204+
token === ts.SyntaxKind.StringLiteral || token === ts.SyntaxKind.NoSubstitutionTemplateLiteral
205+
);
206+
}
207+
208+
function isCursorInToken(scanner: ts.Scanner, offset: number): boolean {
209+
return (
210+
scanner.getStartPos() <= offset &&
211+
scanner.getStartPos() + scanner.getTokenText().length >= offset
212+
);
213+
}
214+
215+
/**
216+
* Creates a `ts.Scanner` for the given text along with a `scan` function that, unlike
217+
* `ts.Scanner#scan`, handles template literals with substitutions correctly.
218+
*
219+
* The scanner has no notion of template substitutions on its own: it reports the `}` closing a
220+
* `${...}` substitution as a plain `CloseBraceToken` and then treats the remainder of the template
221+
* literal as the beginning of a new one, mis-scanning everything that follows it (see #65494). Just
222+
* like the TypeScript parser, the returned `scan` function re-scans such braces as template
223+
* continuations (`TemplateMiddle`/`TemplateTail`) instead.
224+
*/
225+
function createTemplateAwareScanner(text: string): {
226+
scanner: ts.Scanner;
227+
scan: () => ts.SyntaxKind;
228+
} {
229+
const scanner = ts.createScanner(ts.ScriptTarget.ESNext, true /* skipTrivia */);
230+
scanner.setText(text);
231+
232+
// Brace depth at which each currently open template substitution started, innermost last.
233+
const substitutionDepths: number[] = [];
234+
let braceDepth = 0;
235+
236+
const scan = (): ts.SyntaxKind => {
237+
let token = scanner.scan();
238+
if (token === ts.SyntaxKind.OpenBraceToken) {
239+
braceDepth++;
240+
} else if (token === ts.SyntaxKind.CloseBraceToken) {
241+
if (
242+
substitutionDepths.length > 0 &&
243+
braceDepth === substitutionDepths[substitutionDepths.length - 1]
244+
) {
245+
// This brace closes a template substitution rather than a block or object literal.
246+
substitutionDepths.pop();
247+
token = scanner.reScanTemplateToken(false /* isTaggedTemplate */);
248+
} else {
249+
braceDepth--;
250+
}
251+
}
252+
if (token === ts.SyntaxKind.TemplateHead || token === ts.SyntaxKind.TemplateMiddle) {
253+
substitutionDepths.push(braceDepth);
254+
}
255+
return token;
256+
};
257+
258+
return {scanner, scan};
259+
}

‎vscode-ng-language-service/client/src/tests/embedded_support_spec.ts‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import type * as vscode from 'vscode';
99
import {DocumentUri, TextDocument} from 'vscode-languageserver-textdocument';
1010

1111
import {
12+
isInsideStringLiteral,
1213
isNotTypescriptOrSupportedDecoratorField,
1314
isNotTypescriptOrSupportedDecoratorRange,
1415
} from '../embedded_support';
@@ -90,6 +91,144 @@ describe('embedded language support', () => {
9091
false,
9192
);
9293
});
94+
95+
it('inside styles declared before template', () => {
96+
test(
97+
`const foo = {styles: ['h1 { col¦or: red; }'], template: '<div></div>'}`,
98+
isNotTypescriptOrSupportedDecoratorField,
99+
true,
100+
);
101+
});
102+
103+
it('inside a string of another property after template', () => {
104+
test(
105+
`const foo = {template: '<div></div>', selector: 'app¦-root'}`,
106+
isNotTypescriptOrSupportedDecoratorField,
107+
false,
108+
);
109+
});
110+
111+
it('inside a string in the class body after a template in the decorator', () => {
112+
test(
113+
`@Component({template: '<div></div>'})
114+
export class AppComponent {
115+
private property = 'd¦oe';
116+
}`,
117+
isNotTypescriptOrSupportedDecoratorField,
118+
false,
119+
);
120+
});
121+
122+
it('inside a string of a nested host property', () => {
123+
test(
124+
`const foo = {host: {'[class.foo]': 'is¦Foo'}, selector: 'app-root'}`,
125+
isNotTypescriptOrSupportedDecoratorField,
126+
true,
127+
);
128+
});
129+
130+
it('inside a @HostListener decorator string', () => {
131+
test(
132+
`@Component({template: '<div></div>'})
133+
export class AppComponent {
134+
@HostListener('cl¦ick', ['$event']) onClick(event: Event) {}
135+
}`,
136+
isNotTypescriptOrSupportedDecoratorField,
137+
true,
138+
);
139+
});
140+
141+
it('inside a @HostBinding decorator string of a directive without template', () => {
142+
test(
143+
`@Directive({selector: '[foo]'})
144+
export class FooDirective {
145+
@HostBinding('class.f¦oo') isFoo = true;
146+
}`,
147+
isNotTypescriptOrSupportedDecoratorField,
148+
true,
149+
);
150+
});
151+
152+
it('inside a string after a @HostListener decorator', () => {
153+
test(
154+
`@Component({template: '<div></div>'})
155+
export class AppComponent {
156+
@HostListener('click', ['$event']) onClick(event: Event) {
157+
console.log('cli¦cked');
158+
}
159+
}`,
160+
isNotTypescriptOrSupportedDecoratorField,
161+
false,
162+
);
163+
});
164+
165+
it('inside a string argument of a plain function call named like a decorator', () => {
166+
test(`HostListener('cl¦ick');`, isNotTypescriptOrSupportedDecoratorField, false);
167+
});
168+
169+
it('after a template literal with substitutions', () => {
170+
test(
171+
`@Component({template: \`\`})
172+
export class AppComponent {
173+
private property = 'doe';
174+
private method(): void {
175+
console.log(\`\${this.property}\`);
176+
this.¦
177+
}
178+
}`,
179+
isNotTypescriptOrSupportedDecoratorField,
180+
false,
181+
);
182+
});
183+
184+
it('after a template literal with nested braces in a substitution', () => {
185+
test(
186+
`@Component({template: \`\`})
187+
export class AppComponent {
188+
private method(): void {
189+
console.log(\`\${{a: 1}.a} and \${[1, 2].map((x) => { return x; })}\`);
190+
this.¦
191+
}
192+
}`,
193+
isNotTypescriptOrSupportedDecoratorField,
194+
false,
195+
);
196+
});
197+
198+
it('inside a template literal template with substitutions', () => {
199+
test(
200+
`const foo = {template: \`<div>\${value}</d¦iv>\`}`,
201+
isNotTypescriptOrSupportedDecoratorField,
202+
false,
203+
);
204+
});
205+
206+
it('inside a template literal template after a template literal with substitutions', () => {
207+
test(
208+
`const prefix = \`\${getPrefix()}-\`;
209+
const foo = {template: \`<div>¦</div>\`}`,
210+
isNotTypescriptOrSupportedDecoratorField,
211+
true,
212+
);
213+
});
214+
});
215+
216+
describe('isInsideStringLiteral', () => {
217+
it('inside a string literal', () => {
218+
test(`const foo = 'ba¦r';`, isInsideStringLiteral, true);
219+
});
220+
221+
it('outside of a string literal', () => {
222+
test(`const foo = 'bar'; const baz = 1¦;`, isInsideStringLiteral, false);
223+
});
224+
225+
it('after a template literal with substitutions', () => {
226+
test(`const foo = \`\${bar}\`; const baz = 1¦;`, isInsideStringLiteral, false);
227+
});
228+
229+
it('inside a string literal after a template literal with substitutions', () => {
230+
test(`const foo = \`\${bar}\`; const baz = 'qu¦x';`, isInsideStringLiteral, true);
231+
});
93232
});
94233

95234
describe('isNotTypescriptOrSupportedDecoratorRange', () => {

0 commit comments

Comments
 (0)