Ensure configuration settings are retrieved using the ConfigurationService (microsoft#3780)

* Store python path in the `settings.json` file of the workspace folder * Do not use getInstance in config settings * Fix tests * Fixed tests * Fix lintger

Author: DonJayamanne

src/client/common/configSettingMonitor.ts

@@ -3,9 +3,8 @@
 
 import { inject, injectable } from 'inversify';
 import { Disposable, Event, EventEmitter, FileSystemWatcher, Uri, workspace } from 'vscode';
-import { PythonSettings } from '../configSettings';
-import { NON_WINDOWS_PATH_VARIABLE_NAME, WINDOWS_PATH_VARIABLE_NAME } from '../platform/constants';
-import { ICurrentProcess, IDisposableRegistry, IsWindows } from '../types';
+import { IPlatformService } from '../platform/types';
+import { IConfigurationService, ICurrentProcess, IDisposableRegistry } from '../types';
 import { EnvironmentVariables, IEnvironmentVariablesProvider, IEnvironmentVariablesService } from './types';
 
 @injectable()
@@ -15,7 +14,9 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
  private disposables: Disposable[] = [];
  private changeEventEmitter: EventEmitter<Uri | undefined>;
  constructor(@inject(IEnvironmentVariablesService) private envVarsService: IEnvironmentVariablesService,
- @inject(IDisposableRegistry) disposableRegistry: Disposable[], @inject(IsWindows) private isWidows: boolean,
+ @inject(IDisposableRegistry) disposableRegistry: Disposable[],
+ @inject(IPlatformService) private platformService: IPlatformService,
+ @inject(IConfigurationService) private readonly configurationService: IConfigurationService,
  @inject(ICurrentProcess) private process: ICurrentProcess) {
  disposableRegistry.push(this);
  this.changeEventEmitter = new EventEmitter();
@@ -32,7 +33,7 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
  });
  }
  public async getEnvironmentVariables(resource?: Uri): Promise<EnvironmentVariables> {
- const settings = PythonSettings.getInstance(resource);
+ const settings = this.configurationService.getSettings(resource);
  if (!this.cache.has(settings.envFile)) {
  const workspaceFolderUri = this.getWorkspaceFolderUri(resource);
  this.createFileWatcher(settings.envFile, workspaceFolderUri);
@@ -41,7 +42,7 @@ export class EnvironmentVariablesProvider implements IEnvironmentVariablesProvid
  mergedVars = {};
  }
  this.envVarsService.mergeVariables(this.process.env, mergedVars!);
- const pathVariable = this.isWidows ? WINDOWS_PATH_VARIABLE_NAME : NON_WINDOWS_PATH_VARIABLE_NAME;
+ const pathVariable = this.platformService.pathVariableName;
  const pathValue = this.process.env[pathVariable];
  if (pathValue) {
  this.envVarsService.appendPath(mergedVars!, pathValue);

src/client/common/variables/environmentVariablesProvider.ts

@@ -1,9 +1,8 @@
 import { inject, injectable } from 'inversify';
 import { ConfigurationTarget, Disposable, QuickPickItem, QuickPickOptions, Uri } from 'vscode';
 import { IApplicationShell, ICommandManager, IDocumentManager, IWorkspaceService } from '../../common/application/types';
-import * as settings from '../../common/configSettings';
 import { Commands } from '../../common/constants';
-import { IPathUtils } from '../../common/types';
+import { IConfigurationService, IPathUtils } from '../../common/types';
 import { IInterpreterService, IShebangCodeLensProvider, PythonInterpreter, WorkspacePythonPath } from '../contracts';
 import { IInterpreterComparer, IInterpreterSelector, IPythonPathUpdaterServiceManager } from './types';
 
@@ -23,6 +22,7 @@ export class InterpreterSelector implements IInterpreterSelector {
  @inject(IInterpreterComparer) private readonly interpreterComparer: IInterpreterComparer,
  @inject(IPythonPathUpdaterServiceManager) private readonly pythonPathUpdaterService: IPythonPathUpdaterServiceManager,
  @inject(IShebangCodeLensProvider) private readonly shebangCodeLensProvider: IShebangCodeLensProvider,
+ @inject(IConfigurationService) private readonly configurationService: IConfigurationService,
  @inject(ICommandManager) private readonly commandManager: ICommandManager) {
  }
  public dispose() {
@@ -64,7 +64,7 @@ export class InterpreterSelector implements IInterpreterSelector {
  }
 
  const suggestions = await this.getSuggestions(wkspace);
- const currentPythonPath = this.pathUtils.getDisplayName(settings.PythonSettings.getInstance().pythonPath, wkspace ? wkspace.fsPath : undefined);
+ const currentPythonPath = this.pathUtils.getDisplayName(this.configurationService.getSettings(wkspace).pythonPath, wkspace ? wkspace.fsPath : undefined);
  const quickPickOptions: QuickPickOptions = {
  matchOnDetail: true,
  matchOnDescription: true,

src/client/interpreter/configuration/interpreterSelector.ts

@@ -1,18 +1,21 @@
 import { inject, injectable } from 'inversify';
-import { CancellationToken, CodeLens, Command, Event, Position, Range, TextDocument, Uri, workspace } from 'vscode';
-import * as settings from '../../common/configSettings';
-import { IS_WINDOWS } from '../../common/platform/constants';
+import { CancellationToken, CodeLens, Command, Event, Position, Range, TextDocument, Uri } from 'vscode';
+import { IWorkspaceService } from '../../common/application/types';
+import { IPlatformService } from '../../common/platform/types';
 import { IProcessServiceFactory } from '../../common/process/types';
-import { IServiceContainer } from '../../ioc/types';
+import { IConfigurationService } from '../../common/types';
 import { IShebangCodeLensProvider } from '../contracts';
 
 @injectable()
 export class ShebangCodeLensProvider implements IShebangCodeLensProvider {
- // tslint:disable-next-line:no-any
- public onDidChangeCodeLenses: Event<void> = workspace.onDidChangeConfiguration as any as Event<void>;
- private readonly processServiceFactory: IProcessServiceFactory;
- constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
- this.processServiceFactory = serviceContainer.get<IProcessServiceFactory>(IProcessServiceFactory);
+ public readonly onDidChangeCodeLenses: Event<void>;
+ constructor(@inject(IProcessServiceFactory) private readonly processServiceFactory: IProcessServiceFactory,
+ @inject(IConfigurationService) private readonly configurationService: IConfigurationService,
+ @inject(IPlatformService) private readonly platformService: IPlatformService,
+ @inject(IWorkspaceService) workspaceService: IWorkspaceService) {
+ // tslint:disable-next-line:no-any
+ this.onDidChangeCodeLenses = workspaceService.onDidChangeConfiguration as any as Event<void>;
+
  }
  public async detectShebang(document: TextDocument): Promise<string | undefined> {
  const firstLine = document.lineAt(0);
@@ -28,14 +31,13 @@ export class ShebangCodeLensProvider implements IShebangCodeLensProvider {
  const pythonPath = await this.getFullyQualifiedPathToInterpreter(shebang, document.uri);
  return typeof pythonPath === 'string' && pythonPath.length > 0 ? pythonPath : undefined;
  }
- public async provideCodeLenses(document: TextDocument, token: CancellationToken): Promise<CodeLens[]> {
- const codeLenses = await this.createShebangCodeLens(document);
- return Promise.resolve(codeLenses);
+ public async provideCodeLenses(document: TextDocument, _token?: CancellationToken): Promise<CodeLens[]> {
+ return this.createShebangCodeLens(document);
  }
  private async getFullyQualifiedPathToInterpreter(pythonPath: string, resource: Uri) {
  let cmdFile = pythonPath;
  let args = ['-c', 'import sys;print(sys.executable)'];
- if (pythonPath.indexOf('bin/env ') >= 0 && !IS_WINDOWS) {
+ if (pythonPath.indexOf('bin/env ') >= 0 && !this.platformService.isWindows) {
  // In case we have pythonPath as '/usr/bin/env python'.
  const parts = pythonPath.split(' ').map(part => part.trim()).filter(part => part.length > 0);
  cmdFile = parts.shift()!;
@@ -48,12 +50,10 @@ export class ShebangCodeLensProvider implements IShebangCodeLensProvider {
  }
  private async createShebangCodeLens(document: TextDocument) {
  const shebang = await this.detectShebang(document);
- const pythonPath = settings.PythonSettings.getInstance(document.uri).pythonPath;
- const resolvedPythonPath = await this.getFullyQualifiedPathToInterpreter(pythonPath, document.uri);
- if (!shebang || shebang === resolvedPythonPath) {
+ const pythonPath = this.configurationService.getSettings(document.uri).pythonPath;
+ if (!shebang || shebang === pythonPath) {
  return [];
  }
-
  const firstLine = document.lineAt(0);
  const startOfShebang = new Position(0, 0);
  const endOfShebang = new Position(0, firstLine.text.length - 1);

src/client/interpreter/display/shebangCodeLensProvider.ts

@@ -8,7 +8,7 @@ import * as proxy from './jediProxy';
 
 export class PythonDefinitionProvider implements vscode.DefinitionProvider {
  public constructor(private jediFactory: JediFactory) { }
- private static parseData(data: proxy.IDefinitionResult, possibleWord: string): vscode.Definition {
+ private static parseData(data: proxy.IDefinitionResult, possibleWord: string): vscode.Definition | undefined {
  if (data && Array.isArray(data.definitions) && data.definitions.length > 0) {
  const definitions = data.definitions.filter(d => d.text === possibleWord);
  const definition = definitions.length > 0 ? definitions[0] : data.definitions[data.definitions.length - 1];
@@ -18,19 +18,21 @@ export class PythonDefinitionProvider implements vscode.DefinitionProvider {
  definition.range.endLine, definition.range.endColumn);
  return new vscode.Location(definitionResource, range);
  }
- return null;
  }
  @captureTelemetry(DEFINITION)
- public provideDefinition(document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken): Thenable<vscode.Definition> {
+ public async provideDefinition(document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken): Promise<vscode.Definition | undefined> {
  const filename = document.fileName;
  if (document.lineAt(position.line).text.match(/^\s*\/\//)) {
- return Promise.resolve(null);
+ return;
  }
  if (position.character <= 0) {
- return Promise.resolve(null);
+ return;
  }
 
  const range = document.getWordRangeAtPosition(position);
+ if (!range) {
+ return;
+ }
  const columnIndex = range.isEmpty ? position.character : range.end.character;
  const cmd: proxy.ICommand<proxy.IDefinitionResult> = {
  command: proxy.CommandType.Definitions,
@@ -42,8 +44,7 @@ export class PythonDefinitionProvider implements vscode.DefinitionProvider {
  cmd.source = document.getText();
  }
  const possibleWord = document.getText(range);
- return this.jediFactory.getJediProxyHandler<proxy.IDefinitionResult>(document.uri).sendCommand(cmd, token).then(data => {
- return PythonDefinitionProvider.parseData(data, possibleWord);
- });
+ const data = await this.jediFactory.getJediProxyHandler<proxy.IDefinitionResult>(document.uri).sendCommand(cmd, token);
+ return data ? PythonDefinitionProvider.parseData(data, possibleWord) : undefined;
  }
 }

src/client/providers/definitionProvider.ts

@@ -10,16 +10,16 @@ import {
  CancellationToken, CancellationTokenSource, CompletionItemKind,
  Disposable, SymbolKind, Uri
 } from 'vscode';
-import { PythonSettings } from '../common/configSettings';
 import { isTestExecution } from '../common/constants';
 import '../common/extensions';
 import { IS_WINDOWS } from '../common/platform/constants';
 import { IPythonExecutionFactory } from '../common/process/types';
-import { BANNER_NAME_PROPOSE_LS, ILogger, IPythonExtensionBanner } from '../common/types';
+import { BANNER_NAME_PROPOSE_LS, IConfigurationService, ILogger, IPythonExtensionBanner, IPythonSettings } from '../common/types';
 import { createDeferred, Deferred } from '../common/utils/async';
 import { debounce, swallowExceptions } from '../common/utils/decorators';
 import { StopWatch } from '../common/utils/stopWatch';
 import { IEnvironmentVariablesProvider } from '../common/variables/types';
+import { IInterpreterService } from '../interpreter/contracts';
 import { IServiceContainer } from '../ioc/types';
 import { Logger } from './../common/logger';
 
@@ -134,7 +134,7 @@ commandNames.set(CommandType.Symbols, 'names');
 
 export class JediProxy implements Disposable {
  private proc?: ChildProcess;
- private pythonSettings: PythonSettings;
+ private pythonSettings: IPythonSettings;
  private cmdId: number = 0;
  private lastKnownPythonInterpreter: string;
  private previousData = '';
@@ -152,13 +152,17 @@ export class JediProxy implements Disposable {
  private lastCmdIdProcessed?: number;
  private lastCmdIdProcessedForPidUsage?: number;
  private proposeNewLanguageServerPopup: IPythonExtensionBanner;
+ private readonly disposables: Disposable[] = [];
 
  public constructor(private extensionRootDir: string, workspacePath: string, private serviceContainer: IServiceContainer) {
  this.workspacePath = workspacePath;
- this.pythonSettings = PythonSettings.getInstance(Uri.file(workspacePath));
+ const configurationService = serviceContainer.get<IConfigurationService>(IConfigurationService);
+ this.pythonSettings = configurationService.getSettings(Uri.file(workspacePath));
  this.lastKnownPythonInterpreter = this.pythonSettings.pythonPath;
  this.logger = serviceContainer.get<ILogger>(ILogger);
- this.pythonSettings.on('change', () => this.pythonSettingsChangeHandler());
+ const interpreterService = serviceContainer.get<IInterpreterService>(IInterpreterService);
+ const disposable = interpreterService.onDidChangeInterpreter(this.onDidChangeInterpreter.bind(this));
+ this.disposables.push(disposable);
  this.initialized = createDeferred<void>();
  this.startLanguageServer().then(() => this.initialized.resolve()).ignoreErrors();
 
@@ -172,6 +176,12 @@ export class JediProxy implements Disposable {
  }
 
  public dispose() {
+ while (this.disposables.length > 0) {
+ const disposable = this.disposables.pop();
+ if (disposable) {
+ disposable.dispose();
+ }
+ }
  this.killProcess();
  }
 
@@ -277,7 +287,7 @@ export class JediProxy implements Disposable {
  }
 
  @swallowExceptions('JediProxy')
- private async pythonSettingsChangeHandler() {
+ private async onDidChangeInterpreter() {
  if (this.lastKnownPythonInterpreter === this.pythonSettings.pythonPath) {
  return;
  }