From 4e82666060aa956122688da62ea9f44a8c851a51 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 17 Apr 2019 12:02:37 -0700 Subject: [PATCH 1/5] Use a document selector pattern only in multi-root --- news/2 Fixes/5333.md | 1 + .../languageServer/analysisOptions.ts | 28 ++++++++----- .../analysisOptions.unit.test.ts | 40 ++++++++++++++++++- 3 files changed, 56 insertions(+), 13 deletions(-) create mode 100644 news/2 Fixes/5333.md diff --git a/news/2 Fixes/5333.md b/news/2 Fixes/5333.md new file mode 100644 index 000000000000..30f632405b25 --- /dev/null +++ b/news/2 Fixes/5333.md @@ -0,0 +1 @@ +Restrict files from being processed by `Language Server` only when in a mult-root workspace. \ No newline at end of file diff --git a/src/client/activation/languageServer/analysisOptions.ts b/src/client/activation/languageServer/analysisOptions.ts index caa9172c7e45..3ca6ded97fbf 100644 --- a/src/client/activation/languageServer/analysisOptions.ts +++ b/src/client/activation/languageServer/analysisOptions.ts @@ -6,7 +6,7 @@ import { inject, injectable, named } from 'inversify'; import * as path from 'path'; import { CancellationToken, CompletionContext, ConfigurationChangeEvent, Disposable, Event, EventEmitter, OutputChannel, Position, TextDocument } from 'vscode'; -import { LanguageClientOptions, ProvideCompletionItemsSignature, RevealOutputChannelOn } from 'vscode-languageclient'; +import { DocumentFilter, DocumentSelector, LanguageClientOptions, ProvideCompletionItemsSignature, RevealOutputChannelOn } from 'vscode-languageclient'; import { IWorkspaceService } from '../../common/application/types'; import { isTestExecution, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from '../../common/constants'; import { traceDecorators, traceError } from '../../common/logger'; @@ -103,17 +103,10 @@ export class LanguageServerAnalysisOptions implements ILanguageServerAnalysisOpt this.excludedFiles = this.getExcludedFiles(); this.typeshedPaths = this.getTypeshedPaths(); const workspaceFolder = this.workspace.getWorkspaceFolder(this.resource); - const documentSelector = [ - { scheme: 'file', language: PYTHON_LANGUAGE }, - { scheme: 'untitled', language: PYTHON_LANGUAGE } - ]; - if (workspaceFolder) { - // tslint:disable-next-line:no-any - (documentSelector[0] as any).pattern = `${workspaceFolder.uri.fsPath}/**/*`; - } - // Options to control the language client + const documentSelector = this.getDocumentSelector(this.resource); + // Options to control the language client. return { - // Register the server for Python documents + // Register the server for Python documents. documentSelector, workspaceFolder, synchronize: { @@ -148,6 +141,19 @@ export class LanguageServerAnalysisOptions implements ILanguageServerAnalysisOpt } }; } + protected getDocumentSelector(resource: Resource): DocumentSelector { + const documentSelector: DocumentFilter[] = [ + { scheme: 'file', language: PYTHON_LANGUAGE }, + { scheme: 'untitled', language: PYTHON_LANGUAGE } + ]; + // Set the document selector only when in a multi-root workspace scenario. + const workspaceFolder = this.workspace.getWorkspaceFolder(resource); + if (workspaceFolder && Array.isArray(this.workspace.workspaceFolders) && this.workspace.workspaceFolders!.length > 1) { + // tslint:disable-next-line:no-any + documentSelector[0].pattern = `${workspaceFolder.uri.fsPath}/**/*`; + } + return documentSelector; + } protected getExcludedFiles(): string[] { const list: string[] = ['**/Lib/**', '**/site-packages/**']; this.getVsCodeExcludeSection('search.exclude', list); diff --git a/src/test/activation/languageServer/analysisOptions.unit.test.ts b/src/test/activation/languageServer/analysisOptions.unit.test.ts index 9d452a048d7a..ac5a50e920ab 100644 --- a/src/test/activation/languageServer/analysisOptions.unit.test.ts +++ b/src/test/activation/languageServer/analysisOptions.unit.test.ts @@ -7,14 +7,16 @@ import { expect } from 'chai'; import { instance, mock, verify, when } from 'ts-mockito'; import * as typemoq from 'typemoq'; import { ConfigurationChangeEvent, Uri } from 'vscode'; +import { DocumentSelector } from 'vscode-languageclient'; import { LanguageServerAnalysisOptions } from '../../../client/activation/languageServer/analysisOptions'; import { LanguageServerFolderService } from '../../../client/activation/languageServer/languageServerFolderService'; import { ILanguageServerFolderService } from '../../../client/activation/types'; import { IWorkspaceService } from '../../../client/common/application/types'; import { WorkspaceService } from '../../../client/common/application/workspace'; import { ConfigurationService } from '../../../client/common/configuration/service'; +import { PYTHON_LANGUAGE } from '../../../client/common/constants'; import { PathUtils } from '../../../client/common/platform/pathUtils'; -import { IConfigurationService, IDisposable, IExtensionContext, IOutputChannel, IPathUtils, IPythonExtensionBanner } from '../../../client/common/types'; +import { IConfigurationService, IDisposable, IExtensionContext, IOutputChannel, IPathUtils, IPythonExtensionBanner, Resource } from '../../../client/common/types'; import { EnvironmentVariablesProvider } from '../../../client/common/variables/environmentVariablesProvider'; import { IEnvironmentVariablesProvider } from '../../../client/common/variables/types'; import { IInterpreterService } from '../../../client/interpreter/contracts'; @@ -24,8 +26,11 @@ import { sleep } from '../../core'; // tslint:disable:no-unnecessary-override no-any chai-vague-errors no-unused-expression max-func-body-length -suite('Language Server - Analysis Options', () => { +suite('xLanguage Server - Analysis Options', () => { class TestClass extends LanguageServerAnalysisOptions { + public getDocumentSelector(resource: Resource): DocumentSelector { + return super.getDocumentSelector(resource); + } public getExcludedFiles(): string[] { return super.getExcludedFiles(); } @@ -199,4 +204,35 @@ suite('Language Server - Analysis Options', () => { expect(settingsChangedInvokedCount).to.be.equal(1); }); + test('Ensure search pattern is only provided in multi-root workspaces', () => { + const resource = Uri.file(__filename); + const workspaceFolder = { name: '', index: 0, uri: Uri.file(__dirname) }; + when(workspace.getWorkspaceFolder(resource)).thenReturn(workspaceFolder); + when(workspace.workspaceFolders).thenReturn([workspaceFolder]); + + const expectedSelector = [ + { scheme: 'file', language: PYTHON_LANGUAGE }, + { scheme: 'untitled', language: PYTHON_LANGUAGE } + ]; + + const selector = analysisOptions.getDocumentSelector(resource); + + expect(selector).to.deep.equal(expectedSelector); + }); + test('Ensure search pattern is provided in a multi-root workspace', () => { + const resource = Uri.file(__filename); + const workspaceFolder1 = { name: '1', index: 0, uri: Uri.file(__dirname) }; + const workspaceFolder2 = { name: '2', index: 1, uri: Uri.file(__dirname) }; + when(workspace.getWorkspaceFolder(resource)).thenReturn(workspaceFolder1); + when(workspace.workspaceFolders).thenReturn([workspaceFolder1, workspaceFolder2]); + + const expectedSelector = [ + { scheme: 'file', language: PYTHON_LANGUAGE, pattern: `${workspaceFolder1.uri.fsPath}/**/*` }, + { scheme: 'untitled', language: PYTHON_LANGUAGE } + ]; + + const selector = analysisOptions.getDocumentSelector(resource); + + expect(selector).to.deep.equal(expectedSelector); + }); }); From a62d4c4104c889d686911bd65fdfb99186704158 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 17 Apr 2019 12:03:42 -0700 Subject: [PATCH 2/5] Typo --- src/test/activation/languageServer/analysisOptions.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/activation/languageServer/analysisOptions.unit.test.ts b/src/test/activation/languageServer/analysisOptions.unit.test.ts index ac5a50e920ab..e1e0177650e0 100644 --- a/src/test/activation/languageServer/analysisOptions.unit.test.ts +++ b/src/test/activation/languageServer/analysisOptions.unit.test.ts @@ -26,7 +26,7 @@ import { sleep } from '../../core'; // tslint:disable:no-unnecessary-override no-any chai-vague-errors no-unused-expression max-func-body-length -suite('xLanguage Server - Analysis Options', () => { +suite('Language Server - Analysis Options', () => { class TestClass extends LanguageServerAnalysisOptions { public getDocumentSelector(resource: Resource): DocumentSelector { return super.getDocumentSelector(resource); From c305e4452da3064dfbe6ccd32dadbcab7db1d012 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 17 Apr 2019 13:11:17 -0700 Subject: [PATCH 3/5] Address code review comments --- .../languageServer/analysisOptions.ts | 7 ++--- .../analysisOptions.unit.test.ts | 28 ++++++++++++------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/client/activation/languageServer/analysisOptions.ts b/src/client/activation/languageServer/analysisOptions.ts index 3ca6ded97fbf..b6bbb4f8cab5 100644 --- a/src/client/activation/languageServer/analysisOptions.ts +++ b/src/client/activation/languageServer/analysisOptions.ts @@ -5,7 +5,7 @@ import { inject, injectable, named } from 'inversify'; import * as path from 'path'; -import { CancellationToken, CompletionContext, ConfigurationChangeEvent, Disposable, Event, EventEmitter, OutputChannel, Position, TextDocument } from 'vscode'; +import { CancellationToken, CompletionContext, ConfigurationChangeEvent, Disposable, Event, EventEmitter, OutputChannel, Position, TextDocument, WorkspaceFolder } from 'vscode'; import { DocumentFilter, DocumentSelector, LanguageClientOptions, ProvideCompletionItemsSignature, RevealOutputChannelOn } from 'vscode-languageclient'; import { IWorkspaceService } from '../../common/application/types'; import { isTestExecution, PYTHON_LANGUAGE, STANDARD_OUTPUT_CHANNEL } from '../../common/constants'; @@ -103,7 +103,7 @@ export class LanguageServerAnalysisOptions implements ILanguageServerAnalysisOpt this.excludedFiles = this.getExcludedFiles(); this.typeshedPaths = this.getTypeshedPaths(); const workspaceFolder = this.workspace.getWorkspaceFolder(this.resource); - const documentSelector = this.getDocumentSelector(this.resource); + const documentSelector = this.getDocumentSelector(workspaceFolder); // Options to control the language client. return { // Register the server for Python documents. @@ -141,13 +141,12 @@ export class LanguageServerAnalysisOptions implements ILanguageServerAnalysisOpt } }; } - protected getDocumentSelector(resource: Resource): DocumentSelector { + protected getDocumentSelector(workspaceFolder?: WorkspaceFolder): DocumentSelector { const documentSelector: DocumentFilter[] = [ { scheme: 'file', language: PYTHON_LANGUAGE }, { scheme: 'untitled', language: PYTHON_LANGUAGE } ]; // Set the document selector only when in a multi-root workspace scenario. - const workspaceFolder = this.workspace.getWorkspaceFolder(resource); if (workspaceFolder && Array.isArray(this.workspace.workspaceFolders) && this.workspace.workspaceFolders!.length > 1) { // tslint:disable-next-line:no-any documentSelector[0].pattern = `${workspaceFolder.uri.fsPath}/**/*`; diff --git a/src/test/activation/languageServer/analysisOptions.unit.test.ts b/src/test/activation/languageServer/analysisOptions.unit.test.ts index e1e0177650e0..12288379e3a9 100644 --- a/src/test/activation/languageServer/analysisOptions.unit.test.ts +++ b/src/test/activation/languageServer/analysisOptions.unit.test.ts @@ -6,7 +6,7 @@ import { expect } from 'chai'; import { instance, mock, verify, when } from 'ts-mockito'; import * as typemoq from 'typemoq'; -import { ConfigurationChangeEvent, Uri } from 'vscode'; +import { ConfigurationChangeEvent, Uri, WorkspaceFolder } from 'vscode'; import { DocumentSelector } from 'vscode-languageclient'; import { LanguageServerAnalysisOptions } from '../../../client/activation/languageServer/analysisOptions'; import { LanguageServerFolderService } from '../../../client/activation/languageServer/languageServerFolderService'; @@ -28,8 +28,8 @@ import { sleep } from '../../core'; suite('Language Server - Analysis Options', () => { class TestClass extends LanguageServerAnalysisOptions { - public getDocumentSelector(resource: Resource): DocumentSelector { - return super.getDocumentSelector(resource); + public getDocumentSelector(workspaceFolder?: WorkspaceFolder): DocumentSelector { + return super.getDocumentSelector(workspaceFolder); } public getExcludedFiles(): string[] { return super.getExcludedFiles(); @@ -204,10 +204,20 @@ suite('Language Server - Analysis Options', () => { expect(settingsChangedInvokedCount).to.be.equal(1); }); + test('Ensure search pattern is provided when there are no workspace folders', () => { + when(workspace.workspaceFolders).thenReturn([]); + + const expectedSelector = [ + { scheme: 'file', language: PYTHON_LANGUAGE }, + { scheme: 'untitled', language: PYTHON_LANGUAGE } + ]; + + const selector = analysisOptions.getDocumentSelector(); + + expect(selector).to.deep.equal(expectedSelector); + }); test('Ensure search pattern is only provided in multi-root workspaces', () => { - const resource = Uri.file(__filename); - const workspaceFolder = { name: '', index: 0, uri: Uri.file(__dirname) }; - when(workspace.getWorkspaceFolder(resource)).thenReturn(workspaceFolder); + const workspaceFolder: WorkspaceFolder = { name: '', index: 0, uri: Uri.file(__dirname) }; when(workspace.workspaceFolders).thenReturn([workspaceFolder]); const expectedSelector = [ @@ -215,15 +225,13 @@ suite('Language Server - Analysis Options', () => { { scheme: 'untitled', language: PYTHON_LANGUAGE } ]; - const selector = analysisOptions.getDocumentSelector(resource); + const selector = analysisOptions.getDocumentSelector(workspaceFolder); expect(selector).to.deep.equal(expectedSelector); }); test('Ensure search pattern is provided in a multi-root workspace', () => { - const resource = Uri.file(__filename); const workspaceFolder1 = { name: '1', index: 0, uri: Uri.file(__dirname) }; const workspaceFolder2 = { name: '2', index: 1, uri: Uri.file(__dirname) }; - when(workspace.getWorkspaceFolder(resource)).thenReturn(workspaceFolder1); when(workspace.workspaceFolders).thenReturn([workspaceFolder1, workspaceFolder2]); const expectedSelector = [ @@ -231,7 +239,7 @@ suite('Language Server - Analysis Options', () => { { scheme: 'untitled', language: PYTHON_LANGUAGE } ]; - const selector = analysisOptions.getDocumentSelector(resource); + const selector = analysisOptions.getDocumentSelector(workspaceFolder1); expect(selector).to.deep.equal(expectedSelector); }); From 609545c1ce6768e503a28ea29f289d98457f24d2 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 17 Apr 2019 13:13:52 -0700 Subject: [PATCH 4/5] Code review comments --- .../activation/languageServer/analysisOptions.unit.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/activation/languageServer/analysisOptions.unit.test.ts b/src/test/activation/languageServer/analysisOptions.unit.test.ts index 12288379e3a9..181d4e48b726 100644 --- a/src/test/activation/languageServer/analysisOptions.unit.test.ts +++ b/src/test/activation/languageServer/analysisOptions.unit.test.ts @@ -204,7 +204,7 @@ suite('Language Server - Analysis Options', () => { expect(settingsChangedInvokedCount).to.be.equal(1); }); - test('Ensure search pattern is provided when there are no workspace folders', () => { + test('Ensure search pattern is not provided when there are no workspaces', () => { when(workspace.workspaceFolders).thenReturn([]); const expectedSelector = [ @@ -216,7 +216,7 @@ suite('Language Server - Analysis Options', () => { expect(selector).to.deep.equal(expectedSelector); }); - test('Ensure search pattern is only provided in multi-root workspaces', () => { + test('Ensure search pattern is not provided in single-root workspaces', () => { const workspaceFolder: WorkspaceFolder = { name: '', index: 0, uri: Uri.file(__dirname) }; when(workspace.workspaceFolders).thenReturn([workspaceFolder]); From c7ebb0e204d75a9a022bc07703c0899d318c19d8 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 17 Apr 2019 13:20:30 -0700 Subject: [PATCH 5/5] Fix linter --- src/test/activation/languageServer/analysisOptions.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/activation/languageServer/analysisOptions.unit.test.ts b/src/test/activation/languageServer/analysisOptions.unit.test.ts index 181d4e48b726..5a6fb078f30c 100644 --- a/src/test/activation/languageServer/analysisOptions.unit.test.ts +++ b/src/test/activation/languageServer/analysisOptions.unit.test.ts @@ -16,7 +16,7 @@ import { WorkspaceService } from '../../../client/common/application/workspace'; import { ConfigurationService } from '../../../client/common/configuration/service'; import { PYTHON_LANGUAGE } from '../../../client/common/constants'; import { PathUtils } from '../../../client/common/platform/pathUtils'; -import { IConfigurationService, IDisposable, IExtensionContext, IOutputChannel, IPathUtils, IPythonExtensionBanner, Resource } from '../../../client/common/types'; +import { IConfigurationService, IDisposable, IExtensionContext, IOutputChannel, IPathUtils, IPythonExtensionBanner } from '../../../client/common/types'; import { EnvironmentVariablesProvider } from '../../../client/common/variables/environmentVariablesProvider'; import { IEnvironmentVariablesProvider } from '../../../client/common/variables/types'; import { IInterpreterService } from '../../../client/interpreter/contracts';