Skip to content

Commit 1b281d5

Browse files
DonJayamanneDonJayamanne
authored
Ensure Outline view does not overload the symbol provider (#1858)
* Ensure `Outline` view does not overload the symbol provider * Add more tests
1 parent 2608259 commit 1b281d5

File tree

5 files changed

+198
-25
lines changed

5 files changed

+198
-25
lines changed

‎news/3 Code Health/1856.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Ensure `Outline` view doesn't overload the language server with too many requets, while user is editing text in the editor.

‎package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1899,6 +1899,7 @@
18991899
},
19001900
"devDependencies": {
19011901
"@types/chai": "^4.1.2",
1902+
"@types/chai-arrays": "^1.0.2",
19021903
"@types/chai-as-promised": "^7.1.0",
19031904
"@types/del": "^3.0.0",
19041905
"@types/event-stream": "^3.3.33",
@@ -1921,6 +1922,7 @@
19211922
"JSONStream": "^1.3.2",
19221923
"azure-storage": "^2.8.1",
19231924
"chai": "^4.1.2",
1925+
"chai-arrays": "^2.0.0",
19241926
"chai-as-promised": "^7.1.1",
19251927
"codecov": "^3.0.0",
19261928
"colors": "^1.2.1",

‎src/client/providers/symbolProvider.ts

Lines changed: 56 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,80 @@
11
'use strict';
22

3-
import * as vscode from 'vscode';
3+
import { CancellationToken, DocumentSymbolProvider, Location, Range, SymbolInformation, TextDocument, Uri } from 'vscode';
4+
import { createDeferred, Deferred } from '../common/helpers';
45
import { JediFactory } from '../languageServices/jediProxyFactory';
56
import { captureTelemetry } from '../telemetry';
67
import { SYMBOL } from '../telemetry/constants';
78
import * as proxy from './jediProxy';
89

9-
export class PythonSymbolProvider implements vscode.DocumentSymbolProvider {
10-
public constructor(private jediFactory: JediFactory) { }
11-
private static parseData(document: vscode.TextDocument, data: proxy.ISymbolResult): vscode.SymbolInformation[] {
10+
export class PythonSymbolProvider implements DocumentSymbolProvider {
11+
private debounceRequest: Map<string, { timer: NodeJS.Timer; deferred: Deferred<SymbolInformation[]> }>;
12+
public constructor(private jediFactory: JediFactory, private readonly debounceTimeoutMs = 500) {
13+
this.debounceRequest = new Map<string, { timer: NodeJS.Timer; deferred: Deferred<SymbolInformation[]> }>();
14+
}
15+
private static parseData(document: TextDocument, data?: proxy.ISymbolResult): SymbolInformation[] {
1216
if (data) {
1317
const symbols = data.definitions.filter(sym => sym.fileName === document.fileName);
1418
return symbols.map(sym => {
1519
const symbol = sym.kind;
16-
const range = new vscode.Range(
20+
const range = new Range(
1721
sym.range.startLine, sym.range.startColumn,
1822
sym.range.endLine, sym.range.endColumn);
19-
const uri = vscode.Uri.file(sym.fileName);
20-
const location = new vscode.Location(uri, range);
21-
return new vscode.SymbolInformation(sym.text, symbol, sym.container, location);
23+
const uri = Uri.file(sym.fileName);
24+
const location = new Location(uri, range);
25+
return new SymbolInformation(sym.text, symbol, sym.container, location);
2226
});
2327
}
2428
return [];
2529
}
2630
@captureTelemetry(SYMBOL)
27-
public provideDocumentSymbols(document: vscode.TextDocument, token: vscode.CancellationToken): Thenable<vscode.SymbolInformation[]> {
28-
const filename = document.fileName;
31+
public provideDocumentSymbols(document: TextDocument, token: CancellationToken): Thenable<SymbolInformation[]> {
32+
const key = `${document.uri.fsPath}`;
33+
if (this.debounceRequest.has(key)) {
34+
const item = this.debounceRequest.get(key)!;
35+
clearTimeout(item.timer);
36+
item.deferred.resolve([]);
37+
}
2938

30-
const cmd: proxy.ICommand<proxy.ISymbolResult> = {
31-
command: proxy.CommandType.Symbols,
32-
fileName: filename,
33-
columnIndex: 0,
34-
lineIndex: 0
35-
};
39+
const deferred = createDeferred<SymbolInformation[]>();
40+
const timer = setTimeout(() => {
41+
if (token.isCancellationRequested) {
42+
return deferred.resolve([]);
43+
}
3644

37-
if (document.isDirty) {
38-
cmd.source = document.getText();
39-
}
45+
const filename = document.fileName;
46+
const cmd: proxy.ICommand<proxy.ISymbolResult> = {
47+
command: proxy.CommandType.Symbols,
48+
fileName: filename,
49+
columnIndex: 0,
50+
lineIndex: 0
51+
};
52+
53+
if (document.isDirty) {
54+
cmd.source = document.getText();
55+
}
56+
57+
this.jediFactory.getJediProxyHandler<proxy.ISymbolResult>(document.uri).sendCommand(cmd, token)
58+
.then(data => PythonSymbolProvider.parseData(document, data))
59+
.then(items => deferred.resolve(items))
60+
.catch(ex => deferred.reject(ex));
61+
62+
}, this.debounceTimeoutMs);
4063

41-
return this.jediFactory.getJediProxyHandler<proxy.ISymbolResult>(document.uri).sendCommand(cmd, token).then(data => {
42-
return PythonSymbolProvider.parseData(document, data);
64+
token.onCancellationRequested(() => {
65+
clearTimeout(timer);
66+
deferred.resolve([]);
67+
this.debounceRequest.delete(key);
4368
});
69+
70+
// When a document is not saved on FS, we cannot uniquely identify it, so lets not debounce, but delay the symbol provider.
71+
if (!document.isUntitled) {
72+
this.debounceRequest.set(key, { timer, deferred });
73+
}
74+
75+
return deferred.promise;
4476
}
45-
public provideDocumentSymbolsForInternalUse(document: vscode.TextDocument, token: vscode.CancellationToken): Thenable<vscode.SymbolInformation[]> {
77+
public provideDocumentSymbolsForInternalUse(document: TextDocument, token: CancellationToken): Thenable<SymbolInformation[]> {
4678
const filename = document.fileName;
4779

4880
const cmd: proxy.ICommand<proxy.ISymbolResult> = {
@@ -56,8 +88,7 @@ export class PythonSymbolProvider implements vscode.DocumentSymbolProvider {
5688
cmd.source = document.getText();
5789
}
5890

59-
return this.jediFactory.getJediProxyHandler<proxy.ISymbolResult>(document.uri).sendCommandNonCancellableCommand(cmd, token).then(data => {
60-
return PythonSymbolProvider.parseData(document, data);
61-
});
91+
return this.jediFactory.getJediProxyHandler<proxy.ISymbolResult>(document.uri).sendCommandNonCancellableCommand(cmd, token)
92+
.then(data => PythonSymbolProvider.parseData(document, data));
6293
}
6394
}

‎src/test/providers/symbolProvider.test.ts

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
'use strict';
5+
6+
// tslint:disable:max-func-body-length no-any no-require-imports no-var-requires
7+
8+
import { expect, use } from 'chai';
9+
import * as TypeMoq from 'typemoq';
10+
import { CancellationToken, CancellationTokenSource, CompletionItemKind, DocumentSymbolProvider, SymbolKind, TextDocument, Uri } from 'vscode';
11+
import { JediFactory } from '../../client/languageServices/jediProxyFactory';
12+
import { IDefinition, ISymbolResult, JediProxyHandler } from '../../client/providers/jediProxy';
13+
import { PythonSymbolProvider } from '../../client/providers/symbolProvider';
14+
const assertArrays = require('chai-arrays');
15+
use(assertArrays);
16+
17+
suite('Symbol Provider', () => {
18+
let symbolProvider: DocumentSymbolProvider;
19+
let jediHandler: TypeMoq.IMock<JediProxyHandler<ISymbolResult>>;
20+
let jediFactory: TypeMoq.IMock<JediFactory>;
21+
setup(() => {
22+
jediFactory = TypeMoq.Mock.ofType(JediFactory);
23+
jediHandler = TypeMoq.Mock.ofType<JediProxyHandler<ISymbolResult>>();
24+
25+
jediFactory.setup(j => j.getJediProxyHandler(TypeMoq.It.isAny()))
26+
.returns(() => jediHandler.object);
27+
});
28+
29+
async function testDocumentation(requestId: number, fileName: string, expectedSize: number, token?: CancellationToken, isUntitled = false) {
30+
const doc = TypeMoq.Mock.ofType<TextDocument>();
31+
token = token ? token : new CancellationTokenSource().token;
32+
const symbolResult = TypeMoq.Mock.ofType<ISymbolResult>();
33+
34+
const definitions: IDefinition[] = [
35+
{
36+
container: '', fileName: fileName, kind: SymbolKind.Array,
37+
range: { endColumn: 0, endLine: 0, startColumn: 0, startLine: 0 },
38+
rawType: '', text: '', type: CompletionItemKind.Class
39+
}
40+
];
41+
42+
doc.setup(d => d.fileName).returns(() => fileName);
43+
doc.setup(d => d.isUntitled).returns(() => isUntitled);
44+
doc.setup(d => d.uri).returns(() => Uri.file(fileName));
45+
doc.setup(d => d.getText(TypeMoq.It.isAny())).returns(() => '');
46+
symbolResult.setup(c => c.requestId).returns(() => requestId);
47+
symbolResult.setup(c => c.definitions).returns(() => definitions);
48+
symbolResult.setup((c: any) => c.then).returns(() => undefined);
49+
jediHandler.setup(j => j.sendCommand(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
50+
.returns(() => Promise.resolve(symbolResult.object));
51+
52+
const items = await symbolProvider.provideDocumentSymbols(doc.object, token);
53+
expect(items).to.be.array();
54+
expect(items).to.be.ofSize(expectedSize);
55+
}
56+
57+
test('Ensure symbols are returned', async () => {
58+
symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
59+
await testDocumentation(1, __filename, 1);
60+
});
61+
test('Ensure symbols are returned (for untitled documents)', async () => {
62+
symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
63+
await testDocumentation(1, __filename, 1, undefined, true);
64+
});
65+
test('Ensure symbols are returned with a debounce of 100ms', async () => {
66+
symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
67+
await testDocumentation(1, __filename, 1);
68+
});
69+
test('Ensure symbols are returned with a debounce of 100ms (for untitled documents)', async () => {
70+
symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
71+
await testDocumentation(1, __filename, 1, undefined, true);
72+
});
73+
test('Ensure symbols are not returned when cancelled', async () => {
74+
symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
75+
const tokenSource = new CancellationTokenSource();
76+
tokenSource.cancel();
77+
await testDocumentation(1, __filename, 0, tokenSource.token);
78+
});
79+
test('Ensure symbols are not returned when cancelled (for untitled documents)', async () => {
80+
symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
81+
const tokenSource = new CancellationTokenSource();
82+
tokenSource.cancel();
83+
await testDocumentation(1, __filename, 0, tokenSource.token, true);
84+
});
85+
test('Ensure symbols are returned only for the last request', async () => {
86+
symbolProvider = new PythonSymbolProvider(jediFactory.object, 100);
87+
await Promise.all([
88+
testDocumentation(1, __filename, 0),
89+
testDocumentation(2, __filename, 0),
90+
testDocumentation(3, __filename, 1)
91+
]);
92+
});
93+
test('Ensure symbols are returned for all the requests when the doc is untitled', async () => {
94+
symbolProvider = new PythonSymbolProvider(jediFactory.object, 100);
95+
await Promise.all([
96+
testDocumentation(1, __filename, 1, undefined, true),
97+
testDocumentation(2, __filename, 1, undefined, true),
98+
testDocumentation(3, __filename, 1, undefined, true)
99+
]);
100+
});
101+
test('Ensure symbols are returned for multiple documents', async () => {
102+
symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
103+
await Promise.all([
104+
testDocumentation(1, 'file1', 1),
105+
testDocumentation(2, 'file2', 1)
106+
]);
107+
});
108+
test('Ensure symbols are returned for multiple untitled documents ', async () => {
109+
symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
110+
await Promise.all([
111+
testDocumentation(1, 'file1', 1, undefined, true),
112+
testDocumentation(2, 'file2', 1, undefined, true)
113+
]);
114+
});
115+
test('Ensure symbols are returned for multiple documents with a debounce of 100ms', async () => {
116+
symbolProvider = new PythonSymbolProvider(jediFactory.object, 100);
117+
await Promise.all([
118+
testDocumentation(1, 'file1', 1),
119+
testDocumentation(2, 'file2', 1)
120+
]);
121+
});
122+
test('Ensure symbols are returned for multiple untitled documents with a debounce of 100ms', async () => {
123+
symbolProvider = new PythonSymbolProvider(jediFactory.object, 100);
124+
await Promise.all([
125+
testDocumentation(1, 'file1', 1, undefined, true),
126+
testDocumentation(2, 'file2', 1, undefined, true)
127+
]);
128+
});
129+
});

‎yarn.lock

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,12 @@
2525
dependencies:
2626
samsam "1.3.0"
2727

28+
"@types/chai-arrays@^1.0.2":
29+
version "1.0.2"
30+
resolved "https://registry.yarnpkg.com/@types/chai-arrays/-/chai-arrays-1.0.2.tgz#1f89c183c960334c47d9f24105195c4326db0cc7"
31+
dependencies:
32+
"@types/chai" "*"
33+
2834
"@types/chai-as-promised@^7.1.0":
2935
version "7.1.0"
3036
resolved "https://registry.yarnpkg.com/@types/chai-as-promised/-/chai-as-promised-7.1.0.tgz#010b04cde78eacfb6e72bfddb3e58fe23c2e78b9"
@@ -608,6 +614,10 @@ center-align@^0.1.1:
608614
align-text "^0.1.3"
609615
lazy-cache "^1.0.3"
610616

617+
chai-arrays@^2.0.0:
618+
version "2.0.0"
619+
resolved "https://registry.yarnpkg.com/chai-arrays/-/chai-arrays-2.0.0.tgz#d95820d1b39dc2e4abaa01f984b7f9123986a7cc"
620+
611621
chai-as-promised@^7.1.1:
612622
version "7.1.1"
613623
resolved "https://registry.yarnpkg.com/chai-as-promised/-/chai-as-promised-7.1.1.tgz#08645d825deb8696ee61725dbf590c012eb00ca0"

0 commit comments

Comments
 (0)