Ensure Outline view does not overload the symbol provider (#1858)

* Ensure `Outline` view does not overload the symbol provider

* Add more tests

news/3 Code Health/1856.md

@@ -0,0 +1 @@
+Ensure `Outline` view doesn't overload the language server with too many requets, while user is editing text in the editor.

package.json

@@ -1899,6 +1899,7 @@
  },
  "devDependencies": {
  "@types/chai": "^4.1.2",
+ "@types/chai-arrays": "^1.0.2",
  "@types/chai-as-promised": "^7.1.0",
  "@types/del": "^3.0.0",
  "@types/event-stream": "^3.3.33",
@@ -1921,6 +1922,7 @@
  "JSONStream": "^1.3.2",
  "azure-storage": "^2.8.1",
  "chai": "^4.1.2",
+ "chai-arrays": "^2.0.0",
  "chai-as-promised": "^7.1.1",
  "codecov": "^3.0.0",
  "colors": "^1.2.1",

src/client/providers/symbolProvider.ts

@@ -1,48 +1,80 @@
 'use strict';
 
-import * as vscode from 'vscode';
+import { CancellationToken, DocumentSymbolProvider, Location, Range, SymbolInformation, TextDocument, Uri } from 'vscode';
+import { createDeferred, Deferred } from '../common/helpers';
 import { JediFactory } from '../languageServices/jediProxyFactory';
 import { captureTelemetry } from '../telemetry';
 import { SYMBOL } from '../telemetry/constants';
 import * as proxy from './jediProxy';
 
-export class PythonSymbolProvider implements vscode.DocumentSymbolProvider {
- public constructor(private jediFactory: JediFactory) { }
- private static parseData(document: vscode.TextDocument, data: proxy.ISymbolResult): vscode.SymbolInformation[] {
+export class PythonSymbolProvider implements DocumentSymbolProvider {
+ private debounceRequest: Map<string, { timer: NodeJS.Timer; deferred: Deferred<SymbolInformation[]> }>;
+ public constructor(private jediFactory: JediFactory, private readonly debounceTimeoutMs = 500) {
+ this.debounceRequest = new Map<string, { timer: NodeJS.Timer; deferred: Deferred<SymbolInformation[]> }>();
+ }
+ private static parseData(document: TextDocument, data?: proxy.ISymbolResult): SymbolInformation[] {
  if (data) {
  const symbols = data.definitions.filter(sym => sym.fileName === document.fileName);
  return symbols.map(sym => {
  const symbol = sym.kind;
- const range = new vscode.Range(
+ const range = new Range(
  sym.range.startLine, sym.range.startColumn,
  sym.range.endLine, sym.range.endColumn);
- const uri = vscode.Uri.file(sym.fileName);
- const location = new vscode.Location(uri, range);
- return new vscode.SymbolInformation(sym.text, symbol, sym.container, location);
+ const uri = Uri.file(sym.fileName);
+ const location = new Location(uri, range);
+ return new SymbolInformation(sym.text, symbol, sym.container, location);
  });
  }
  return [];
  }
  @captureTelemetry(SYMBOL)
- public provideDocumentSymbols(document: vscode.TextDocument, token: vscode.CancellationToken): Thenable<vscode.SymbolInformation[]> {
- const filename = document.fileName;
+ public provideDocumentSymbols(document: TextDocument, token: CancellationToken): Thenable<SymbolInformation[]> {
+ const key = `${document.uri.fsPath}`;
+ if (this.debounceRequest.has(key)) {
+ const item = this.debounceRequest.get(key)!;
+ clearTimeout(item.timer);
+ item.deferred.resolve([]);
+ }
 
- const cmd: proxy.ICommand<proxy.ISymbolResult> = {
- command: proxy.CommandType.Symbols,
- fileName: filename,
- columnIndex: 0,
- lineIndex: 0
- };
+ const deferred = createDeferred<SymbolInformation[]>();
+ const timer = setTimeout(() => {
+ if (token.isCancellationRequested) {
+ return deferred.resolve([]);
+ }
 
- if (document.isDirty) {
- cmd.source = document.getText();
- }
+ const filename = document.fileName;
+ const cmd: proxy.ICommand<proxy.ISymbolResult> = {
+ command: proxy.CommandType.Symbols,
+ fileName: filename,
+ columnIndex: 0,
+ lineIndex: 0
+ };
+
+ if (document.isDirty) {
+ cmd.source = document.getText();
+ }
+
+ this.jediFactory.getJediProxyHandler<proxy.ISymbolResult>(document.uri).sendCommand(cmd, token)
+ .then(data => PythonSymbolProvider.parseData(document, data))
+ .then(items => deferred.resolve(items))
+ .catch(ex => deferred.reject(ex));
+
+ }, this.debounceTimeoutMs);
 
- return this.jediFactory.getJediProxyHandler<proxy.ISymbolResult>(document.uri).sendCommand(cmd, token).then(data => {
- return PythonSymbolProvider.parseData(document, data);
+ token.onCancellationRequested(() => {
+ clearTimeout(timer);
+ deferred.resolve([]);
+ this.debounceRequest.delete(key);
  });
+
+ // When a document is not saved on FS, we cannot uniquely identify it, so lets not debounce, but delay the symbol provider.
+ if (!document.isUntitled) {
+ this.debounceRequest.set(key, { timer, deferred });
+ }
+
+ return deferred.promise;
  }
- public provideDocumentSymbolsForInternalUse(document: vscode.TextDocument, token: vscode.CancellationToken): Thenable<vscode.SymbolInformation[]> {
+ public provideDocumentSymbolsForInternalUse(document: TextDocument, token: CancellationToken): Thenable<SymbolInformation[]> {
  const filename = document.fileName;
 
  const cmd: proxy.ICommand<proxy.ISymbolResult> = {
@@ -56,8 +88,7 @@ export class PythonSymbolProvider implements vscode.DocumentSymbolProvider {
  cmd.source = document.getText();
  }
 
- return this.jediFactory.getJediProxyHandler<proxy.ISymbolResult>(document.uri).sendCommandNonCancellableCommand(cmd, token).then(data => {
- return PythonSymbolProvider.parseData(document, data);
- });
+ return this.jediFactory.getJediProxyHandler<proxy.ISymbolResult>(document.uri).sendCommandNonCancellableCommand(cmd, token)
+ .then(data => PythonSymbolProvider.parseData(document, data));
  }
 }

src/test/providers/symbolProvider.test.ts

@@ -0,0 +1,129 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+'use strict';
+
+// tslint:disable:max-func-body-length no-any no-require-imports no-var-requires
+
+import { expect, use } from 'chai';
+import * as TypeMoq from 'typemoq';
+import { CancellationToken, CancellationTokenSource, CompletionItemKind, DocumentSymbolProvider, SymbolKind, TextDocument, Uri } from 'vscode';
+import { JediFactory } from '../../client/languageServices/jediProxyFactory';
+import { IDefinition, ISymbolResult, JediProxyHandler } from '../../client/providers/jediProxy';
+import { PythonSymbolProvider } from '../../client/providers/symbolProvider';
+const assertArrays = require('chai-arrays');
+use(assertArrays);
+
+suite('Symbol Provider', () => {
+ let symbolProvider: DocumentSymbolProvider;
+ let jediHandler: TypeMoq.IMock<JediProxyHandler<ISymbolResult>>;
+ let jediFactory: TypeMoq.IMock<JediFactory>;
+ setup(() => {
+ jediFactory = TypeMoq.Mock.ofType(JediFactory);
+ jediHandler = TypeMoq.Mock.ofType<JediProxyHandler<ISymbolResult>>();
+
+ jediFactory.setup(j => j.getJediProxyHandler(TypeMoq.It.isAny()))
+ .returns(() => jediHandler.object);
+ });
+
+ async function testDocumentation(requestId: number, fileName: string, expectedSize: number, token?: CancellationToken, isUntitled = false) {
+ const doc = TypeMoq.Mock.ofType<TextDocument>();
+ token = token ? token : new CancellationTokenSource().token;
+ const symbolResult = TypeMoq.Mock.ofType<ISymbolResult>();
+
+ const definitions: IDefinition[] = [
+ {
+ container: '', fileName: fileName, kind: SymbolKind.Array,
+ range: { endColumn: 0, endLine: 0, startColumn: 0, startLine: 0 },
+ rawType: '', text: '', type: CompletionItemKind.Class
+ }
+ ];
+
+ doc.setup(d => d.fileName).returns(() => fileName);
+ doc.setup(d => d.isUntitled).returns(() => isUntitled);
+ doc.setup(d => d.uri).returns(() => Uri.file(fileName));
+ doc.setup(d => d.getText(TypeMoq.It.isAny())).returns(() => '');
+ symbolResult.setup(c => c.requestId).returns(() => requestId);
+ symbolResult.setup(c => c.definitions).returns(() => definitions);
+ symbolResult.setup((c: any) => c.then).returns(() => undefined);
+ jediHandler.setup(j => j.sendCommand(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
+ .returns(() => Promise.resolve(symbolResult.object));
+
+ const items = await symbolProvider.provideDocumentSymbols(doc.object, token);
+ expect(items).to.be.array();
+ expect(items).to.be.ofSize(expectedSize);
+ }
+
+ test('Ensure symbols are returned', async () => {
+ symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
+ await testDocumentation(1, __filename, 1);
+ });
+ test('Ensure symbols are returned (for untitled documents)', async () => {
+ symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
+ await testDocumentation(1, __filename, 1, undefined, true);
+ });
+ test('Ensure symbols are returned with a debounce of 100ms', async () => {
+ symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
+ await testDocumentation(1, __filename, 1);
+ });
+ test('Ensure symbols are returned with a debounce of 100ms (for untitled documents)', async () => {
+ symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
+ await testDocumentation(1, __filename, 1, undefined, true);
+ });
+ test('Ensure symbols are not returned when cancelled', async () => {
+ symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
+ const tokenSource = new CancellationTokenSource();
+ tokenSource.cancel();
+ await testDocumentation(1, __filename, 0, tokenSource.token);
+ });
+ test('Ensure symbols are not returned when cancelled (for untitled documents)', async () => {
+ symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
+ const tokenSource = new CancellationTokenSource();
+ tokenSource.cancel();
+ await testDocumentation(1, __filename, 0, tokenSource.token, true);
+ });
+ test('Ensure symbols are returned only for the last request', async () => {
+ symbolProvider = new PythonSymbolProvider(jediFactory.object, 100);
+ await Promise.all([
+ testDocumentation(1, __filename, 0),
+ testDocumentation(2, __filename, 0),
+ testDocumentation(3, __filename, 1)
+ ]);
+ });
+ test('Ensure symbols are returned for all the requests when the doc is untitled', async () => {
+ symbolProvider = new PythonSymbolProvider(jediFactory.object, 100);
+ await Promise.all([
+ testDocumentation(1, __filename, 1, undefined, true),
+ testDocumentation(2, __filename, 1, undefined, true),
+ testDocumentation(3, __filename, 1, undefined, true)
+ ]);
+ });
+ test('Ensure symbols are returned for multiple documents', async () => {
+ symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
+ await Promise.all([
+ testDocumentation(1, 'file1', 1),
+ testDocumentation(2, 'file2', 1)
+ ]);
+ });
+ test('Ensure symbols are returned for multiple untitled documents ', async () => {
+ symbolProvider = new PythonSymbolProvider(jediFactory.object, 0);
+ await Promise.all([
+ testDocumentation(1, 'file1', 1, undefined, true),
+ testDocumentation(2, 'file2', 1, undefined, true)
+ ]);
+ });
+ test('Ensure symbols are returned for multiple documents with a debounce of 100ms', async () => {
+ symbolProvider = new PythonSymbolProvider(jediFactory.object, 100);
+ await Promise.all([
+ testDocumentation(1, 'file1', 1),
+ testDocumentation(2, 'file2', 1)
+ ]);
+ });
+ test('Ensure symbols are returned for multiple untitled documents with a debounce of 100ms', async () => {
+ symbolProvider = new PythonSymbolProvider(jediFactory.object, 100);
+ await Promise.all([
+ testDocumentation(1, 'file1', 1, undefined, true),
+ testDocumentation(2, 'file2', 1, undefined, true)
+ ]);
+ });
+});

yarn.lock

@@ -25,6 +25,12 @@
  dependencies:
  samsam "1.3.0"
 
+"@types/chai-arrays@^1.0.2":
+ version "1.0.2"
+ resolved "https://registry.yarnpkg.com/@types/chai-arrays/-/chai-arrays-1.0.2.tgz#1f89c183c960334c47d9f24105195c4326db0cc7"
+ dependencies:
+ "@types/chai" "*"
+
 "@types/chai-as-promised@^7.1.0":
  version "7.1.0"
  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:
  align-text "^0.1.3"
  lazy-cache "^1.0.3"
 
+chai-arrays@^2.0.0:
+ version "2.0.0"
+ resolved "https://registry.yarnpkg.com/chai-arrays/-/chai-arrays-2.0.0.tgz#d95820d1b39dc2e4abaa01f984b7f9123986a7cc"
+
 chai-as-promised@^7.1.1:
  version "7.1.1"
  resolved "https://registry.yarnpkg.com/chai-as-promised/-/chai-as-promised-7.1.1.tgz#08645d825deb8696ee61725dbf590c012eb00ca0"