Skip to content

Commit 22c3422

Browse files
committed
refactor(client): use SourceFile to detect the Angular context in the client
When using the typescript's scanner to parse the template literals which includes an expression, knowing the right `CloseBraceToken` of the expression to invoke the `reScanTemplateToken` function is a little complex. If don't do this, the rest of the file is temporarily "poisoned". There is an explanation for why typescript's fast-acting lexical classifications don't work for template sting across lines. microsoft/TypeScript#1477 (comment) https://github.com/microsoft/TypeScript/blob/a4d12a46c8b413c65a730c4ad0323459f1fc44ce/src/services/classifier.ts#L114 So this PR uses the `SourceFile` to detect the Angular context in the client.
1 parent 51d907e commit 22c3422

File tree

2 files changed

+107
-100
lines changed

2 files changed

+107
-100
lines changed

client/src/embedded_support.ts

Lines changed: 75 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -11,115 +11,105 @@ import * as vscode from 'vscode';
1111
/** Determines if the position is inside an inline template. */
1212
export function isInsideInlineTemplateRegion(
1313
document: vscode.TextDocument, position: vscode.Position): boolean {
14-
if (document.languageId !== 'typescript') {
15-
return true;
14+
const node = getNodeAtDocumentPosition(document, position);
15+
16+
if (!node) {
17+
return false;
1618
}
17-
return isPropertyAssignmentToStringOrStringInArray(
18-
document.getText(), document.offsetAt(position), ['template']);
19+
20+
return getPropertyAssignmentFromValue(node, 'template') !== null;
1921
}
2022

2123
/** Determines if the position is inside an inline template, templateUrl, or string in styleUrls. */
2224
export function isInsideComponentDecorator(
2325
document: vscode.TextDocument, position: vscode.Position): boolean {
24-
if (document.languageId !== 'typescript') {
25-
return true;
26+
const node = getNodeAtDocumentPosition(document, position);
27+
if (!node) {
28+
return false;
2629
}
27-
return isPropertyAssignmentToStringOrStringInArray(
28-
document.getText(), document.offsetAt(position),
29-
['template', 'templateUrl', 'styleUrls', 'styleUrl']);
30+
const assignment = getPropertyAssignmentFromValue(node, 'template') ??
31+
getPropertyAssignmentFromValue(node, 'templateUrl') ??
32+
// `node.parent` is used because the string is a child of an array element and we want to get
33+
// the property name
34+
getPropertyAssignmentFromValue(node.parent, 'styleUrls') ??
35+
getPropertyAssignmentFromValue(node, 'styleUrl');
36+
return assignment !== null;
3037
}
3138

3239
/**
33-
* Determines if the position is inside a string literal. Returns `true` if the document language is
34-
* not TypeScript.
40+
* Determines if the position is inside a string literal. Returns `true` if the document language
41+
* is not TypeScript.
3542
*/
3643
export function isInsideStringLiteral(
3744
document: vscode.TextDocument, position: vscode.Position): boolean {
38-
if (document.languageId !== 'typescript') {
39-
return true;
40-
}
41-
const offset = document.offsetAt(position);
42-
const scanner = ts.createScanner(ts.ScriptTarget.ESNext, true /* skipTrivia */);
43-
scanner.setText(document.getText());
45+
const node = getNodeAtDocumentPosition(document, position);
4446

45-
let token: ts.SyntaxKind = scanner.scan();
46-
while (token !== ts.SyntaxKind.EndOfFileToken && scanner.getStartPos() < offset) {
47-
const isStringToken = token === ts.SyntaxKind.StringLiteral ||
48-
token === ts.SyntaxKind.NoSubstitutionTemplateLiteral;
49-
const isCursorInToken = scanner.getStartPos() <= offset &&
50-
scanner.getStartPos() + scanner.getTokenText().length >= offset;
51-
if (isCursorInToken && isStringToken) {
52-
return true;
53-
}
54-
token = scanner.scan();
47+
if (!node) {
48+
return false;
5549
}
56-
return false;
50+
51+
return ts.isStringLiteralLike(node);
5752
}
5853

5954
/**
60-
* Basic scanner to determine if we're inside a string of a property with one of the given names.
61-
*
62-
* This scanner is not currently robust or perfect but provides us with an accurate answer _most_ of
63-
* the time.
64-
*
65-
* False positives are OK here. Though this will give some false positives for determining if a
66-
* position is within an Angular context, i.e. an object like `{template: ''}` that is not inside an
67-
* `@Component` or `{styleUrls: [someFunction('stringL¦iteral')]}`, the @angular/language-service
68-
* will always give us the correct answer. This helper gives us a quick win for optimizing the
69-
* number of requests we send to the server.
70-
*
71-
* TODO(atscott): tagged templates don't work: #1872 /
72-
* https://github.com/Microsoft/TypeScript/issues/20055
55+
* Return the node that most tightly encompasses the specified `position`.
56+
* @param node The starting node to start the top-down search.
57+
* @param position The target position within the `node`.
7358
*/
74-
function isPropertyAssignmentToStringOrStringInArray(
75-
documentText: string, offset: number, propertyAssignmentNames: string[]): boolean {
76-
const scanner = ts.createScanner(ts.ScriptTarget.ESNext, true /* skipTrivia */);
77-
scanner.setText(documentText);
59+
function findTightestNodeAtPosition(node: ts.Node, position: number): ts.Node|undefined {
60+
if (node.getStart() <= position && position < node.getEnd()) {
61+
return node.forEachChild(c => findTightestNodeAtPosition(c, position)) ?? node;
62+
}
63+
return undefined;
64+
}
7865

79-
let token: ts.SyntaxKind = scanner.scan();
80-
let lastToken: ts.SyntaxKind|undefined;
81-
let lastTokenText: string|undefined;
82-
let unclosedBraces = 0;
83-
let unclosedBrackets = 0;
84-
let propertyAssignmentContext = false;
85-
while (token !== ts.SyntaxKind.EndOfFileToken && scanner.getStartPos() < offset) {
86-
if (lastToken === ts.SyntaxKind.Identifier && lastTokenText !== undefined &&
87-
propertyAssignmentNames.includes(lastTokenText) && token === ts.SyntaxKind.ColonToken) {
88-
propertyAssignmentContext = true;
89-
token = scanner.scan();
90-
continue;
91-
}
92-
if (unclosedBraces === 0 && unclosedBrackets === 0 && isPropertyAssignmentTerminator(token)) {
93-
propertyAssignmentContext = false;
94-
}
66+
/**
67+
* Returns a property assignment from the assignment value if the property name
68+
* matches the specified `key`, or `null` if there is no match.
69+
*/
70+
function getPropertyAssignmentFromValue(value: ts.Node, key: string): ts.PropertyAssignment|null {
71+
const propAssignment = value.parent;
72+
if (!propAssignment || !ts.isPropertyAssignment(propAssignment) ||
73+
propAssignment.name.getText() !== key) {
74+
return null;
75+
}
76+
return propAssignment;
77+
}
9578

96-
if (token === ts.SyntaxKind.OpenBracketToken) {
97-
unclosedBrackets++;
98-
} else if (token === ts.SyntaxKind.OpenBraceToken) {
99-
unclosedBraces++;
100-
} else if (token === ts.SyntaxKind.CloseBracketToken) {
101-
unclosedBrackets--;
102-
} else if (token === ts.SyntaxKind.CloseBraceToken) {
103-
unclosedBraces--;
104-
}
79+
type NgLSClientSourceFile = ts.SourceFile&{[NgLSClientSourceFileVersion]: number};
10580

106-
const isStringToken = token === ts.SyntaxKind.StringLiteral ||
107-
token === ts.SyntaxKind.NoSubstitutionTemplateLiteral;
108-
const isCursorInToken = scanner.getStartPos() <= offset &&
109-
scanner.getStartPos() + scanner.getTokenText().length >= offset;
110-
if (propertyAssignmentContext && isCursorInToken && isStringToken) {
111-
return true;
112-
}
81+
/**
82+
* The `TextDocument` is not extensible, so the `WeakMap` is used here.
83+
*/
84+
const ngLSClientSourceFileMap = new WeakMap<vscode.TextDocument, NgLSClientSourceFile>();
85+
const NgLSClientSourceFileVersion = Symbol('NgLSClientSourceFileVersion');
11386

114-
lastTokenText = scanner.getTokenText();
115-
lastToken = token;
116-
token = scanner.scan();
87+
/**
88+
*
89+
* Parse the document to `SourceFile` and return the node at the document position.
90+
*/
91+
function getNodeAtDocumentPosition(
92+
document: vscode.TextDocument, position: vscode.Position): ts.Node|undefined {
93+
if (document.languageId !== 'typescript') {
94+
return undefined;
11795
}
96+
const offset = document.offsetAt(position);
11897

119-
return false;
120-
}
98+
let sourceFile = ngLSClientSourceFileMap.get(document);
99+
if (!sourceFile || sourceFile[NgLSClientSourceFileVersion] !== document.version) {
100+
sourceFile =
101+
ts.createSourceFile(
102+
document.fileName, document.getText(), {
103+
languageVersion: ts.ScriptTarget.ESNext,
104+
jsDocParsingMode: ts.JSDocParsingMode.ParseNone,
105+
},
106+
/** setParentNodes */
107+
true /** If not set, the `findTightestNodeAtPosition` will throw an error */) as
108+
NgLSClientSourceFile;
109+
sourceFile[NgLSClientSourceFileVersion] = document.version;
110+
111+
ngLSClientSourceFileMap.set(document, sourceFile);
112+
}
121113

122-
function isPropertyAssignmentTerminator(token: ts.SyntaxKind) {
123-
return token === ts.SyntaxKind.EndOfFileToken || token === ts.SyntaxKind.CommaToken ||
124-
token === ts.SyntaxKind.SemicolonToken || token === ts.SyntaxKind.CloseBraceToken;
114+
return findTightestNodeAtPosition(sourceFile, offset);
125115
}

client/src/tests/embedded_support_spec.ts

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,27 +17,42 @@ describe('embedded language support', () => {
1717
});
1818

1919
it('just after template', () => {
20-
test(`template: '<div></div>'¦`, isInsideInlineTemplateRegion, false);
20+
test(`const foo = {template: '<div></div>'¦}`, isInsideInlineTemplateRegion, false);
2121
});
2222

2323
it('just before template', () => {
2424
// Note that while it seems that this should be `false`, we should still consider this inside
2525
// the string because the visual mode of vim appears to have a position on top of the open
2626
// quote while the cursor position is before it.
27-
test(`template: ¦'<div></div>'`, isInsideInlineTemplateRegion, true);
27+
test(`const foo = {template: ¦'<div></div>'}`, isInsideInlineTemplateRegion, true);
2828
});
2929

3030
it('two spaces before template', () => {
31-
test(`template:¦ '<div></div>'`, isInsideInlineTemplateRegion, false);
31+
test(`const foo = {template:¦ '<div></div>'}`, isInsideInlineTemplateRegion, false);
3232
});
3333

3434
it('at beginning of template', () => {
35-
test(`template: '¦<div></div>'`, isInsideInlineTemplateRegion, true);
35+
test(`const foo = {template: '¦<div></div>'}`, isInsideInlineTemplateRegion, true);
3636
});
3737

3838
it('at end of template', () => {
39-
test(`template: '<div></div>¦'`, isInsideInlineTemplateRegion, true);
39+
test(`const foo = {template: '<div></div>¦'}`, isInsideInlineTemplateRegion, true);
4040
});
41+
42+
it('works for inline templates after a template string', () => {
43+
test(
44+
'const x = `${""}`;\n' +
45+
'const foo = {template: `hello ¦world`}',
46+
isInsideInlineTemplateRegion, true);
47+
});
48+
49+
it('works for inline templates after a tagged template string inside tagged template string',
50+
() => {
51+
test(
52+
'const x = `${`${""}`}`;\n' +
53+
'const foo = {template: `hello ¦world`}',
54+
isInsideInlineTemplateRegion, true);
55+
});
4156
});
4257

4358
describe('isInsideAngularContext', () => {
@@ -46,42 +61,42 @@ describe('embedded language support', () => {
4661
});
4762

4863
it('just after template', () => {
49-
test(`template: '<div></div>'¦`, isInsideComponentDecorator, false);
64+
test(`const foo = {template: '<div></div>'¦}`, isInsideComponentDecorator, false);
5065
});
5166

5267
it('inside template', () => {
53-
test(`template: '<div>¦</div>'`, isInsideComponentDecorator, true);
68+
test(`const foo = {template: '<div>¦</div>'}`, isInsideComponentDecorator, true);
5469
});
5570

5671
it('just after templateUrl', () => {
57-
test(`templateUrl: './abc.html'¦`, isInsideComponentDecorator, false);
72+
test(`const foo = {templateUrl: './abc.html'¦}`, isInsideComponentDecorator, false);
5873
});
5974

6075
it('inside templateUrl', () => {
61-
test(`templateUrl: './abc¦.html'`, isInsideComponentDecorator, true);
76+
test(`const foo = {templateUrl: './abc¦.html'}`, isInsideComponentDecorator, true);
6277
});
6378

6479
it('just after styleUrls', () => {
65-
test(`styleUrls: ['./abc.css']¦`, isInsideComponentDecorator, false);
80+
test(`const foo = {styleUrls: ['./abc.css']¦}`, isInsideComponentDecorator, false);
6681
});
6782

6883
it('inside first item of styleUrls', () => {
69-
test(`styleUrls: ['./abc.c¦ss', 'def.css']`, isInsideComponentDecorator, true);
84+
test(`const foo = {styleUrls: ['./abc.c¦ss', 'def.css']}`, isInsideComponentDecorator, true);
7085
});
7186

7287
it('inside second item of styleUrls', () => {
73-
test(`styleUrls: ['./abc.css', 'def¦.css']`, isInsideComponentDecorator, true);
88+
test(`const foo = {styleUrls: ['./abc.css', 'def¦.css']}`, isInsideComponentDecorator, true);
7489
});
7590

7691
it('inside second item of styleUrls, when first is complicated function', () => {
7792
test(
78-
`styleUrls: [getCss({strict: true, dirs: ['apple', 'banana']}), 'def¦.css']`,
93+
`const foo = {styleUrls: [getCss({strict: true, dirs: ['apple', 'banana']}), 'def¦.css']}`,
7994
isInsideComponentDecorator, true);
8095
});
8196

8297
it('inside non-string item of styleUrls', () => {
8398
test(
84-
`styleUrls: [getCss({strict: true¦, dirs: ['apple', 'banana']}), 'def.css']`,
99+
`const foo = {styleUrls: [getCss({strict: true¦, dirs: ['apple', 'banana']}), 'def.css']}`,
85100
isInsideComponentDecorator, false);
86101
});
87102
});
@@ -92,8 +107,10 @@ function test(
92107
testFn: (doc: vscode.TextDocument, position: vscode.Position) => boolean,
93108
expectation: boolean): void {
94109
const {cursor, text} = extractCursorInfo(fileWithCursor);
95-
const vdoc = TextDocument.create('test' as DocumentUri, 'typescript', 0, text) as {} as
110+
111+
const vdoc = TextDocument.create('test.ts' as DocumentUri, 'typescript', 0, text) as {} as
96112
vscode.TextDocument;
113+
(vdoc as any).fileName = 'test.ts';
97114
const actual = testFn(vdoc, vdoc.positionAt(cursor));
98115
expect(actual).toBe(expectation);
99116
}

0 commit comments

Comments
 (0)