Skip to content

Commit 13d9776

Browse files
authored
refactor(client): use SourceFile to detect the Angular context in the client (#2027)
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 13d9776

File tree

2 files changed

+111
-97
lines changed

2 files changed

+111
-97
lines changed

client/src/embedded_support.ts

Lines changed: 79 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,13 @@ export function isInsideInlineTemplateRegion(
1414
if (document.languageId !== 'typescript') {
1515
return true;
1616
}
17-
return isPropertyAssignmentToStringOrStringInArray(
18-
document.getText(), document.offsetAt(position), ['template']);
17+
const node = getNodeAtDocumentPosition(document, position);
18+
19+
if (!node) {
20+
return false;
21+
}
22+
23+
return getPropertyAssignmentFromValue(node, 'template') !== null;
1924
}
2025

2126
/** Determines if the position is inside an inline template, templateUrl, or string in styleUrls. */
@@ -24,102 +29,94 @@ export function isInsideComponentDecorator(
2429
if (document.languageId !== 'typescript') {
2530
return true;
2631
}
27-
return isPropertyAssignmentToStringOrStringInArray(
28-
document.getText(), document.offsetAt(position),
29-
['template', 'templateUrl', 'styleUrls', 'styleUrl']);
32+
33+
const node = getNodeAtDocumentPosition(document, position);
34+
if (!node) {
35+
return false;
36+
}
37+
const assignment = getPropertyAssignmentFromValue(node, 'template') ??
38+
getPropertyAssignmentFromValue(node, 'templateUrl') ??
39+
// `node.parent` is used because the string is a child of an array element and we want to get
40+
// the property name
41+
getPropertyAssignmentFromValue(node.parent, 'styleUrls') ??
42+
getPropertyAssignmentFromValue(node, 'styleUrl');
43+
return assignment !== null;
3044
}
3145

3246
/**
33-
* Determines if the position is inside a string literal. Returns `true` if the document language is
34-
* not TypeScript.
47+
* Determines if the position is inside a string literal. Returns `true` if the document language
48+
* is not TypeScript.
3549
*/
3650
export function isInsideStringLiteral(
3751
document: vscode.TextDocument, position: vscode.Position): boolean {
3852
if (document.languageId !== 'typescript') {
3953
return true;
4054
}
41-
const offset = document.offsetAt(position);
42-
const scanner = ts.createScanner(ts.ScriptTarget.ESNext, true /* skipTrivia */);
43-
scanner.setText(document.getText());
44-
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();
55+
const node = getNodeAtDocumentPosition(document, position);
56+
57+
if (!node) {
58+
return false;
5559
}
56-
return false;
60+
61+
return ts.isStringLiteralLike(node);
5762
}
5863

5964
/**
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
65+
* Return the node that most tightly encompasses the specified `position`.
66+
* @param node The starting node to start the top-down search.
67+
* @param position The target position within the `node`.
7368
*/
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);
78-
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-
}
95-
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-
}
105-
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-
}
113-
114-
lastTokenText = scanner.getTokenText();
115-
lastToken = token;
116-
token = scanner.scan();
69+
function findTightestNodeAtPosition(node: ts.Node, position: number): ts.Node|undefined {
70+
if (node.getStart() <= position && position < node.getEnd()) {
71+
return node.forEachChild(c => findTightestNodeAtPosition(c, position)) ?? node;
11772
}
73+
return undefined;
74+
}
11875

119-
return false;
76+
/**
77+
* Returns a property assignment from the assignment value if the property name
78+
* matches the specified `key`, or `null` if there is no match.
79+
*/
80+
function getPropertyAssignmentFromValue(value: ts.Node, key: string): ts.PropertyAssignment|null {
81+
const propAssignment = value.parent;
82+
if (!propAssignment || !ts.isPropertyAssignment(propAssignment) ||
83+
propAssignment.name.getText() !== key) {
84+
return null;
85+
}
86+
return propAssignment;
12087
}
12188

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;
89+
type NgLSClientSourceFile = ts.SourceFile&{[NgLSClientSourceFileVersion]: number};
90+
91+
/**
92+
* The `TextDocument` is not extensible, so the `WeakMap` is used here.
93+
*/
94+
const ngLSClientSourceFileMap = new WeakMap<vscode.TextDocument, NgLSClientSourceFile>();
95+
const NgLSClientSourceFileVersion = Symbol('NgLSClientSourceFileVersion');
96+
97+
/**
98+
*
99+
* Parse the document to `SourceFile` and return the node at the document position.
100+
*/
101+
function getNodeAtDocumentPosition(
102+
document: vscode.TextDocument, position: vscode.Position): ts.Node|undefined {
103+
const offset = document.offsetAt(position);
104+
105+
let sourceFile = ngLSClientSourceFileMap.get(document);
106+
if (!sourceFile || sourceFile[NgLSClientSourceFileVersion] !== document.version) {
107+
sourceFile =
108+
ts.createSourceFile(
109+
document.fileName, document.getText(), {
110+
languageVersion: ts.ScriptTarget.ESNext,
111+
jsDocParsingMode: ts.JSDocParsingMode.ParseNone,
112+
},
113+
/** setParentNodes */
114+
true /** If not set, the `findTightestNodeAtPosition` will throw an error */) as
115+
NgLSClientSourceFile;
116+
sourceFile[NgLSClientSourceFileVersion] = document.version;
117+
118+
ngLSClientSourceFileMap.set(document, sourceFile);
119+
}
120+
121+
return findTightestNodeAtPosition(sourceFile, offset);
125122
}

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)