diff --git a/news/2 Fixes/2478.md b/news/2 Fixes/2478.md new file mode 100644 index 000000000000..2a1e623bf56b --- /dev/null +++ b/news/2 Fixes/2478.md @@ -0,0 +1 @@ +Upgrade `pythonExperimental` to `python` in `launch.json`. diff --git a/src/client/application/diagnostics/applicationDiagnostics.ts b/src/client/application/diagnostics/applicationDiagnostics.ts index 5956cf5d6e5e..65c5be3a10cc 100644 --- a/src/client/application/diagnostics/applicationDiagnostics.ts +++ b/src/client/application/diagnostics/applicationDiagnostics.ts @@ -9,19 +9,20 @@ import { STANDARD_OUTPUT_CHANNEL } from '../../common/constants'; import { ILogger, IOutputChannel } from '../../common/types'; import { IServiceContainer } from '../../ioc/types'; import { IApplicationDiagnostics } from '../types'; -import { EnvironmentPathVariableDiagnosticsServiceId } from './checks/envPathVariable'; import { IDiagnostic, IDiagnosticsService } from './types'; @injectable() export class ApplicationDiagnostics implements IApplicationDiagnostics { constructor(@inject(IServiceContainer) private readonly serviceContainer: IServiceContainer) { } public async performPreStartupHealthCheck(): Promise { - const envHealthCheck = this.serviceContainer.get(IDiagnosticsService, EnvironmentPathVariableDiagnosticsServiceId); - const diagnostics = await envHealthCheck.diagnose(); - this.log(diagnostics); - if (diagnostics.length > 0) { - await envHealthCheck.handle(diagnostics); - } + const diagnosticsServices = this.serviceContainer.getAll(IDiagnosticsService); + await Promise.all(diagnosticsServices.map(async diagnosticsService => { + const diagnostics = await diagnosticsService.diagnose(); + this.log(diagnostics); + if (diagnostics.length > 0) { + await diagnosticsService.handle(diagnostics); + } + })); } private log(diagnostics: IDiagnostic[]): void { const logger = this.serviceContainer.get(ILogger); diff --git a/src/client/application/diagnostics/checks/invalidDebuggerType.ts b/src/client/application/diagnostics/checks/invalidDebuggerType.ts new file mode 100644 index 000000000000..a57677f26590 --- /dev/null +++ b/src/client/application/diagnostics/checks/invalidDebuggerType.ts @@ -0,0 +1,115 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { inject, injectable } from 'inversify'; +import * as path from 'path'; +import { DiagnosticSeverity, WorkspaceFolder } from 'vscode'; +import { ICommandManager, IWorkspaceService } from '../../../common/application/types'; +import '../../../common/extensions'; +import { IFileSystem } from '../../../common/platform/types'; +import { IServiceContainer } from '../../../ioc/types'; +import { BaseDiagnostic, BaseDiagnosticsService } from '../base'; +import { IDiagnosticsCommandFactory } from '../commands/types'; +import { DiagnosticCodes } from '../constants'; +import { DiagnosticCommandPromptHandlerServiceId, MessageCommandPrompt } from '../promptHandler'; +import { DiagnosticScope, IDiagnostic, IDiagnosticHandlerService } from '../types'; + +const InvalidDebuggerTypeMessage = 'Your launch.json file needs to be updated to change the "pythonExperimental" debug ' + + 'configurations to use the "python" debugger type, otherwise Python debugging may ' + + 'not work. Would you like to automatically update your launch.json file now?'; + +export class InvalidDebuggerTypeDiagnostic extends BaseDiagnostic { + constructor(message) { + super(DiagnosticCodes.InvalidDebuggerTypeDiagnostic, + message, DiagnosticSeverity.Error, DiagnosticScope.WorkspaceFolder); + } +} + +export const InvalidDebuggerTypeDiagnosticsServiceId = 'InvalidDebuggerTypeDiagnosticsServiceId'; + +const CommandName = 'python.debugger.replaceExperimental'; + +@injectable() +export class InvalidDebuggerTypeDiagnosticsService extends BaseDiagnosticsService { + protected readonly messageService: IDiagnosticHandlerService; + protected readonly fs: IFileSystem; + constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) { + super([DiagnosticCodes.InvalidEnvironmentPathVariableDiagnostic], serviceContainer); + this.messageService = serviceContainer.get>(IDiagnosticHandlerService, DiagnosticCommandPromptHandlerServiceId); + const cmdManager = serviceContainer.get(ICommandManager); + this.fs = this.serviceContainer.get(IFileSystem); + cmdManager.registerCommand(CommandName, this.fixLaunchJson, this); + } + public async diagnose(): Promise { + if (await this.isExperimentalDebuggerUsed()) { + return [new InvalidDebuggerTypeDiagnostic(InvalidDebuggerTypeMessage)]; + } else { + return []; + } + } + public async handle(diagnostics: IDiagnostic[]): Promise { + // This class can only handle one type of diagnostic, hence just use first item in list. + if (diagnostics.length === 0 || !this.canHandle(diagnostics[0])) { + return; + } + const diagnostic = diagnostics[0]; + const commandFactory = this.serviceContainer.get(IDiagnosticsCommandFactory); + const options = [ + { + prompt: 'Yes, update launch.json', + command: commandFactory.createCommand(diagnostic, { type: 'executeVSCCommand', options: 'python.debugger.replaceExperimental' }) + }, + { + prompt: 'No, I will do it later' + } + ]; + + await this.messageService.handle(diagnostic, { commandPrompts: options }); + } + private async isExperimentalDebuggerUsed() { + const workspaceService = this.serviceContainer.get(IWorkspaceService); + if (!workspaceService.hasWorkspaceFolders) { + return false; + } + + const results = await Promise.all(workspaceService.workspaceFolders!.map(workspaceFolder => this.isExperimentalDebuggerUsedInWorkspace(workspaceFolder))); + return results.filter(used => used === true).length > 0; + } + private getLaunchJsonFile(workspaceFolder: WorkspaceFolder) { + return path.join(workspaceFolder.uri.fsPath, '.vscode', 'launch.json'); + } + private async isExperimentalDebuggerUsedInWorkspace(workspaceFolder: WorkspaceFolder) { + const launchJson = this.getLaunchJsonFile(workspaceFolder); + if (!await this.fs.fileExists(launchJson)) { + return false; + } + + const fileContents = await this.fs.readFile(launchJson); + return fileContents.indexOf('"pythonExperimental"') > 0; + } + private async fixLaunchJson() { + const workspaceService = this.serviceContainer.get(IWorkspaceService); + if (!workspaceService.hasWorkspaceFolders) { + return false; + } + + await Promise.all(workspaceService.workspaceFolders!.map(workspaceFolder => this.fixLaunchJsonInWorkspace(workspaceFolder))); + } + private async fixLaunchJsonInWorkspace(workspaceFolder: WorkspaceFolder) { + if (!await this.isExperimentalDebuggerUsedInWorkspace(workspaceFolder)) { + return; + } + + const launchJson = this.getLaunchJsonFile(workspaceFolder); + let fileContents = await this.fs.readFile(launchJson); + const debuggerType = new RegExp('"pythonExperimental"', 'g'); + const debuggerLabel = new RegExp('"Python Experimental:', 'g'); + + fileContents = fileContents.replace(debuggerType, '"python"'); + fileContents = fileContents.replace(debuggerLabel, '"Python:'); + + await this.fs.writeFile(launchJson, fileContents); + } +} diff --git a/src/client/application/diagnostics/commands/execVSCCommand.ts b/src/client/application/diagnostics/commands/execVSCCommand.ts new file mode 100644 index 000000000000..83a2edaa3dbd --- /dev/null +++ b/src/client/application/diagnostics/commands/execVSCCommand.ts @@ -0,0 +1,19 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +'use strict'; + +import { ICommandManager } from '../../../common/application/types'; +import { IServiceContainer } from '../../../ioc/types'; +import { IDiagnostic } from '../types'; +import { BaseDiagnosticCommand } from './base'; + +export class ExecuteVSCCommand extends BaseDiagnosticCommand { + constructor(diagnostic: IDiagnostic, private serviceContainer: IServiceContainer, private commandName: string) { + super(diagnostic); + } + public async invoke(): Promise { + const cmdManager = this.serviceContainer.get(ICommandManager); + return cmdManager.executeCommand(this.commandName).then(() => undefined); + } +} diff --git a/src/client/application/diagnostics/commands/factory.ts b/src/client/application/diagnostics/commands/factory.ts index 47d2e63aa0d8..d4cc51fa84fe 100644 --- a/src/client/application/diagnostics/commands/factory.ts +++ b/src/client/application/diagnostics/commands/factory.ts @@ -6,6 +6,7 @@ import { inject, injectable } from 'inversify'; import { IServiceContainer } from '../../../ioc/types'; import { IDiagnostic, IDiagnosticCommand } from '../types'; +import { ExecuteVSCCommand } from './execVSCCommand'; import { IgnoreDiagnosticCommand } from './ignore'; import { LaunchBrowserCommand } from './launchBrowser'; import { CommandOptions, IDiagnosticsCommandFactory } from './types'; @@ -22,6 +23,9 @@ export class DiagnosticsCommandFactory implements IDiagnosticsCommandFactory { case 'launch': { return new LaunchBrowserCommand(diagnostic, this.serviceContainer, options.options); } + case 'executeVSCCommand': { + return new ExecuteVSCCommand(diagnostic, this.serviceContainer, options.options); + } default: { throw new Error(`Unknown Diagnostic command commandType '${commandType}'`); } diff --git a/src/client/application/diagnostics/commands/types.ts b/src/client/application/diagnostics/commands/types.ts index b6f95e2bc769..e22fbb0a2d9d 100644 --- a/src/client/application/diagnostics/commands/types.ts +++ b/src/client/application/diagnostics/commands/types.ts @@ -8,7 +8,8 @@ import { DiagnosticScope, IDiagnostic, IDiagnosticCommand } from '../types'; export type CommandOption = { type: Type; options: Option }; export type LaunchBrowserOption = CommandOption<'launch', string>; export type IgnoreDiagnostOption = CommandOption<'ignore', DiagnosticScope>; -export type CommandOptions = LaunchBrowserOption | IgnoreDiagnostOption; +export type ExecuteVSCCommandOption = CommandOption<'executeVSCCommand', string>; +export type CommandOptions = LaunchBrowserOption | IgnoreDiagnostOption | ExecuteVSCCommandOption; export const IDiagnosticsCommandFactory = Symbol('IDiagnosticsCommandFactory'); diff --git a/src/client/application/diagnostics/constants.ts b/src/client/application/diagnostics/constants.ts index 88431d4a855b..e3848dba1bac 100644 --- a/src/client/application/diagnostics/constants.ts +++ b/src/client/application/diagnostics/constants.ts @@ -4,5 +4,6 @@ 'use strict'; export enum DiagnosticCodes { - InvalidEnvironmentPathVariableDiagnostic = 'InvalidEnvironmentPathVariableDiagnostic' + InvalidEnvironmentPathVariableDiagnostic = 'InvalidEnvironmentPathVariableDiagnostic', + InvalidDebuggerTypeDiagnostic = 'InvalidDebuggerTypeDiagnostic' } diff --git a/src/client/application/diagnostics/serviceRegistry.ts b/src/client/application/diagnostics/serviceRegistry.ts index 6554a6ddf37b..a76c7aced8d6 100644 --- a/src/client/application/diagnostics/serviceRegistry.ts +++ b/src/client/application/diagnostics/serviceRegistry.ts @@ -5,6 +5,7 @@ import { IServiceManager } from '../../ioc/types'; import { EnvironmentPathVariableDiagnosticsService, EnvironmentPathVariableDiagnosticsServiceId } from './checks/envPathVariable'; +import { InvalidDebuggerTypeDiagnosticsService, InvalidDebuggerTypeDiagnosticsServiceId } from './checks/invalidDebuggerType'; import { DiagnosticsCommandFactory } from './commands/factory'; import { IDiagnosticsCommandFactory } from './commands/types'; import { DiagnosticFilterService } from './filter'; @@ -15,5 +16,6 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IDiagnosticFilterService, DiagnosticFilterService); serviceManager.addSingleton>(IDiagnosticHandlerService, DiagnosticCommandPromptHandlerService, DiagnosticCommandPromptHandlerServiceId); serviceManager.addSingleton(IDiagnosticsService, EnvironmentPathVariableDiagnosticsService, EnvironmentPathVariableDiagnosticsServiceId); + serviceManager.addSingleton(IDiagnosticsService, InvalidDebuggerTypeDiagnosticsService, InvalidDebuggerTypeDiagnosticsServiceId); serviceManager.addSingleton(IDiagnosticsCommandFactory, DiagnosticsCommandFactory); } diff --git a/src/client/common/platform/fileSystem.ts b/src/client/common/platform/fileSystem.ts index 2ec6abfdc497..80cd7fba227a 100644 --- a/src/client/common/platform/fileSystem.ts +++ b/src/client/common/platform/fileSystem.ts @@ -168,4 +168,8 @@ export class FileSystem implements IFileSystem { }); }); } + + public writeFile(filePath: string, data: {}): Promise { + return fs.writeFile(filePath, data); + } } diff --git a/src/client/common/platform/types.ts b/src/client/common/platform/types.ts index 50c35bd74629..064e8dec303f 100644 --- a/src/client/common/platform/types.ts +++ b/src/client/common/platform/types.ts @@ -78,6 +78,7 @@ export interface IFileSystem { getSubDirectories(rootDir: string): Promise; arePathsSame(path1: string, path2: string): boolean; readFile(filePath: string): Promise; + writeFile(filePath: string, data: {}): Promise; appendFileSync(filename: string, data: {}, encoding: string): void; appendFileSync(filename: string, data: {}, options?: { encoding?: string; mode?: number; flag?: string }): void; // tslint:disable-next-line:unified-signatures diff --git a/src/test/application/diagnostics/applicationDiagnostics.unit.test.ts b/src/test/application/diagnostics/applicationDiagnostics.unit.test.ts index 1fad39ac5453..541f98a45f6c 100644 --- a/src/test/application/diagnostics/applicationDiagnostics.unit.test.ts +++ b/src/test/application/diagnostics/applicationDiagnostics.unit.test.ts @@ -9,15 +9,18 @@ import * as typemoq from 'typemoq'; import { DiagnosticSeverity } from 'vscode'; import { ApplicationDiagnostics } from '../../../client/application/diagnostics/applicationDiagnostics'; import { EnvironmentPathVariableDiagnosticsServiceId } from '../../../client/application/diagnostics/checks/envPathVariable'; +import { InvalidDebuggerTypeDiagnosticsServiceId } from '../../../client/application/diagnostics/checks/invalidDebuggerType'; import { DiagnosticScope, IDiagnostic, IDiagnosticsService } from '../../../client/application/diagnostics/types'; import { IApplicationDiagnostics } from '../../../client/application/types'; import { STANDARD_OUTPUT_CHANNEL } from '../../../client/common/constants'; import { ILogger, IOutputChannel } from '../../../client/common/types'; import { IServiceContainer } from '../../../client/ioc/types'; +// tslint:disable-next-line:max-func-body-length suite('Application Diagnostics - ApplicationDiagnostics', () => { let serviceContainer: typemoq.IMock; let envHealthCheck: typemoq.IMock; + let debuggerTypeCheck: typemoq.IMock; let outputChannel: typemoq.IMock; let logger: typemoq.IMock; let appDiagnostics: IApplicationDiagnostics; @@ -25,11 +28,16 @@ suite('Application Diagnostics - ApplicationDiagnostics', () => { setup(() => { serviceContainer = typemoq.Mock.ofType(); envHealthCheck = typemoq.Mock.ofType(); + debuggerTypeCheck = typemoq.Mock.ofType(); outputChannel = typemoq.Mock.ofType(); logger = typemoq.Mock.ofType(); serviceContainer.setup(d => d.get(typemoq.It.isValue(IDiagnosticsService), typemoq.It.isValue(EnvironmentPathVariableDiagnosticsServiceId))) .returns(() => envHealthCheck.object); + serviceContainer.setup(d => d.get(typemoq.It.isValue(IDiagnosticsService), typemoq.It.isValue(InvalidDebuggerTypeDiagnosticsServiceId))) + .returns(() => debuggerTypeCheck.object); + serviceContainer.setup(d => d.getAll(typemoq.It.isValue(IDiagnosticsService))) + .returns(() => [envHealthCheck.object, debuggerTypeCheck.object]); serviceContainer.setup(d => d.get(typemoq.It.isValue(IOutputChannel), typemoq.It.isValue(STANDARD_OUTPUT_CHANNEL))) .returns(() => outputChannel.object); serviceContainer.setup(d => d.get(typemoq.It.isValue(ILogger))) @@ -38,14 +46,18 @@ suite('Application Diagnostics - ApplicationDiagnostics', () => { appDiagnostics = new ApplicationDiagnostics(serviceContainer.object); }); - test('Performing Pre Startup Health Check must check Path environment variable', async () => { + test('Performing Pre Startup Health Check must check Path environment variable and Debugger Type', async () => { envHealthCheck.setup(e => e.diagnose()) .returns(() => Promise.resolve([])) .verifiable(typemoq.Times.once()); + debuggerTypeCheck.setup(e => e.diagnose()) + .returns(() => Promise.resolve([])) + .verifiable(typemoq.Times.once()); await appDiagnostics.performPreStartupHealthCheck(); envHealthCheck.verifyAll(); + debuggerTypeCheck.verifyAll(); }); test('Diagnostics Returned by Per Startup Health Checks must be logged', async () => { @@ -84,16 +96,16 @@ suite('Application Diagnostics - ApplicationDiagnostics', () => { case DiagnosticSeverity.Error: { logger.setup(l => l.logError(message)) .verifiable(typemoq.Times.once()); - outputChannel.setup(o => o.appendLine(message)) + outputChannel.setup(o => o.appendLine(message)) .verifiable(typemoq.Times.once()); - break; + break; } case DiagnosticSeverity.Warning: { logger.setup(l => l.logWarning(message)) .verifiable(typemoq.Times.once()); - outputChannel.setup(o => o.appendLine(message)) + outputChannel.setup(o => o.appendLine(message)) .verifiable(typemoq.Times.once()); - break; + break; } default: { logger.setup(l => l.logInformation(message)) @@ -106,10 +118,14 @@ suite('Application Diagnostics - ApplicationDiagnostics', () => { envHealthCheck.setup(e => e.diagnose()) .returns(() => Promise.resolve(diagnostics)) .verifiable(typemoq.Times.once()); + debuggerTypeCheck.setup(e => e.diagnose()) + .returns(() => Promise.resolve([])) + .verifiable(typemoq.Times.once()); await appDiagnostics.performPreStartupHealthCheck(); envHealthCheck.verifyAll(); + debuggerTypeCheck.verifyAll(); outputChannel.verifyAll(); logger.verifyAll(); });