Skip to content

refactor(client): use SourceFile to detect the Angular context in t… #2027

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 79 additions & 82 deletions client/src/embedded_support.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,13 @@ export function isInsideInlineTemplateRegion(
if (document.languageId !== 'typescript') {
return true;
}
return isPropertyAssignmentToStringOrStringInArray(
document.getText(), document.offsetAt(position), ['template']);
const node = getNodeAtDocumentPosition(document, position);

if (!node) {
return false;
}

return getPropertyAssignmentFromValue(node, 'template') !== null;
}

/** Determines if the position is inside an inline template, templateUrl, or string in styleUrls. */
Expand All @@ -24,102 +29,94 @@ export function isInsideComponentDecorator(
if (document.languageId !== 'typescript') {
return true;
}
return isPropertyAssignmentToStringOrStringInArray(
document.getText(), document.offsetAt(position),
['template', 'templateUrl', 'styleUrls', 'styleUrl']);

const node = getNodeAtDocumentPosition(document, position);
if (!node) {
return false;
}
const assignment = getPropertyAssignmentFromValue(node, 'template') ??
getPropertyAssignmentFromValue(node, 'templateUrl') ??
// `node.parent` is used because the string is a child of an array element and we want to get
// the property name
getPropertyAssignmentFromValue(node.parent, 'styleUrls') ??
getPropertyAssignmentFromValue(node, 'styleUrl');
return assignment !== null;
}

/**
* Determines if the position is inside a string literal. Returns `true` if the document language is
* not TypeScript.
* Determines if the position is inside a string literal. Returns `true` if the document language
* is not TypeScript.
*/
export function isInsideStringLiteral(
document: vscode.TextDocument, position: vscode.Position): boolean {
if (document.languageId !== 'typescript') {
return true;
}
const offset = document.offsetAt(position);
const scanner = ts.createScanner(ts.ScriptTarget.ESNext, true /* skipTrivia */);
scanner.setText(document.getText());

let token: ts.SyntaxKind = scanner.scan();
while (token !== ts.SyntaxKind.EndOfFileToken && scanner.getStartPos() < offset) {
const isStringToken = token === ts.SyntaxKind.StringLiteral ||
token === ts.SyntaxKind.NoSubstitutionTemplateLiteral;
const isCursorInToken = scanner.getStartPos() <= offset &&
scanner.getStartPos() + scanner.getTokenText().length >= offset;
if (isCursorInToken && isStringToken) {
return true;
}
token = scanner.scan();
const node = getNodeAtDocumentPosition(document, position);

if (!node) {
return false;
}
return false;

return ts.isStringLiteralLike(node);
}

/**
* Basic scanner to determine if we're inside a string of a property with one of the given names.
*
* This scanner is not currently robust or perfect but provides us with an accurate answer _most_ of
* the time.
*
* False positives are OK here. Though this will give some false positives for determining if a
* position is within an Angular context, i.e. an object like `{template: ''}` that is not inside an
* `@Component` or `{styleUrls: [someFunction('stringL¦iteral')]}`, the @angular/language-service
* will always give us the correct answer. This helper gives us a quick win for optimizing the
* number of requests we send to the server.
*
* TODO(atscott): tagged templates don't work: #1872 /
* https://github.com/Microsoft/TypeScript/issues/20055
* Return the node that most tightly encompasses the specified `position`.
* @param node The starting node to start the top-down search.
* @param position The target position within the `node`.
*/
function isPropertyAssignmentToStringOrStringInArray(
documentText: string, offset: number, propertyAssignmentNames: string[]): boolean {
const scanner = ts.createScanner(ts.ScriptTarget.ESNext, true /* skipTrivia */);
scanner.setText(documentText);

let token: ts.SyntaxKind = scanner.scan();
let lastToken: ts.SyntaxKind|undefined;
let lastTokenText: string|undefined;
let unclosedBraces = 0;
let unclosedBrackets = 0;
let propertyAssignmentContext = false;
while (token !== ts.SyntaxKind.EndOfFileToken && scanner.getStartPos() < offset) {
if (lastToken === ts.SyntaxKind.Identifier && lastTokenText !== undefined &&
propertyAssignmentNames.includes(lastTokenText) && token === ts.SyntaxKind.ColonToken) {
propertyAssignmentContext = true;
token = scanner.scan();
continue;
}
if (unclosedBraces === 0 && unclosedBrackets === 0 && isPropertyAssignmentTerminator(token)) {
propertyAssignmentContext = false;
}

if (token === ts.SyntaxKind.OpenBracketToken) {
unclosedBrackets++;
} else if (token === ts.SyntaxKind.OpenBraceToken) {
unclosedBraces++;
} else if (token === ts.SyntaxKind.CloseBracketToken) {
unclosedBrackets--;
} else if (token === ts.SyntaxKind.CloseBraceToken) {
unclosedBraces--;
}

const isStringToken = token === ts.SyntaxKind.StringLiteral ||
token === ts.SyntaxKind.NoSubstitutionTemplateLiteral;
const isCursorInToken = scanner.getStartPos() <= offset &&
scanner.getStartPos() + scanner.getTokenText().length >= offset;
if (propertyAssignmentContext && isCursorInToken && isStringToken) {
return true;
}

lastTokenText = scanner.getTokenText();
lastToken = token;
token = scanner.scan();
function findTightestNodeAtPosition(node: ts.Node, position: number): ts.Node|undefined {
if (node.getStart() <= position && position < node.getEnd()) {
return node.forEachChild(c => findTightestNodeAtPosition(c, position)) ?? node;
}
return undefined;
}

return false;
/**
* Returns a property assignment from the assignment value if the property name
* matches the specified `key`, or `null` if there is no match.
*/
function getPropertyAssignmentFromValue(value: ts.Node, key: string): ts.PropertyAssignment|null {
const propAssignment = value.parent;
if (!propAssignment || !ts.isPropertyAssignment(propAssignment) ||
propAssignment.name.getText() !== key) {
return null;
}
return propAssignment;
}

function isPropertyAssignmentTerminator(token: ts.SyntaxKind) {
return token === ts.SyntaxKind.EndOfFileToken || token === ts.SyntaxKind.CommaToken ||
token === ts.SyntaxKind.SemicolonToken || token === ts.SyntaxKind.CloseBraceToken;
type NgLSClientSourceFile = ts.SourceFile&{[NgLSClientSourceFileVersion]: number};

/**
* The `TextDocument` is not extensible, so the `WeakMap` is used here.
*/
const ngLSClientSourceFileMap = new WeakMap<vscode.TextDocument, NgLSClientSourceFile>();
const NgLSClientSourceFileVersion = Symbol('NgLSClientSourceFileVersion');

/**
*
* Parse the document to `SourceFile` and return the node at the document position.
*/
function getNodeAtDocumentPosition(
document: vscode.TextDocument, position: vscode.Position): ts.Node|undefined {
const offset = document.offsetAt(position);

let sourceFile = ngLSClientSourceFileMap.get(document);
if (!sourceFile || sourceFile[NgLSClientSourceFileVersion] !== document.version) {
sourceFile =
ts.createSourceFile(
document.fileName, document.getText(), {
languageVersion: ts.ScriptTarget.ESNext,
jsDocParsingMode: ts.JSDocParsingMode.ParseNone,
},
/** setParentNodes */
true /** If not set, the `findTightestNodeAtPosition` will throw an error */) as
NgLSClientSourceFile;
sourceFile[NgLSClientSourceFileVersion] = document.version;

ngLSClientSourceFileMap.set(document, sourceFile);
}

return findTightestNodeAtPosition(sourceFile, offset);
}
47 changes: 32 additions & 15 deletions client/src/tests/embedded_support_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,42 @@ describe('embedded language support', () => {
});

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

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

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

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

it('at end of template', () => {
test(`template: '<div></div>¦'`, isInsideInlineTemplateRegion, true);
test(`const foo = {template: '<div></div>¦'}`, isInsideInlineTemplateRegion, true);
});

it('works for inline templates after a template string', () => {
test(
'const x = `${""}`;\n' +
'const foo = {template: `hello ¦world`}',
isInsideInlineTemplateRegion, true);
});

it('works for inline templates after a tagged template string inside tagged template string',
() => {
test(
'const x = `${`${""}`}`;\n' +
'const foo = {template: `hello ¦world`}',
isInsideInlineTemplateRegion, true);
});
});

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

it('just after template', () => {
test(`template: '<div></div>'¦`, isInsideComponentDecorator, false);
test(`const foo = {template: '<div></div>'¦}`, isInsideComponentDecorator, false);
});

it('inside template', () => {
test(`template: '<div>¦</div>'`, isInsideComponentDecorator, true);
test(`const foo = {template: '<div>¦</div>'}`, isInsideComponentDecorator, true);
});

it('just after templateUrl', () => {
test(`templateUrl: './abc.html'¦`, isInsideComponentDecorator, false);
test(`const foo = {templateUrl: './abc.html'¦}`, isInsideComponentDecorator, false);
});

it('inside templateUrl', () => {
test(`templateUrl: './abc¦.html'`, isInsideComponentDecorator, true);
test(`const foo = {templateUrl: './abc¦.html'}`, isInsideComponentDecorator, true);
});

it('just after styleUrls', () => {
test(`styleUrls: ['./abc.css']¦`, isInsideComponentDecorator, false);
test(`const foo = {styleUrls: ['./abc.css']¦}`, isInsideComponentDecorator, false);
});

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

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

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

it('inside non-string item of styleUrls', () => {
test(
`styleUrls: [getCss({strict: true¦, dirs: ['apple', 'banana']}), 'def.css']`,
`const foo = {styleUrls: [getCss({strict: true¦, dirs: ['apple', 'banana']}), 'def.css']}`,
isInsideComponentDecorator, false);
});
});
Expand All @@ -92,8 +107,10 @@ function test(
testFn: (doc: vscode.TextDocument, position: vscode.Position) => boolean,
expectation: boolean): void {
const {cursor, text} = extractCursorInfo(fileWithCursor);
const vdoc = TextDocument.create('test' as DocumentUri, 'typescript', 0, text) as {} as

const vdoc = TextDocument.create('test.ts' as DocumentUri, 'typescript', 0, text) as {} as
vscode.TextDocument;
(vdoc as any).fileName = 'test.ts';
const actual = testFn(vdoc, vdoc.positionAt(cursor));
expect(actual).toBe(expectation);
}
Expand Down