Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/2 Fixes/2478.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Upgrade `pythonExperimental` to `python` in `launch.json`.
15 changes: 8 additions & 7 deletions src/client/application/diagnostics/applicationDiagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
const envHealthCheck = this.serviceContainer.get<IDiagnosticsService>(IDiagnosticsService, EnvironmentPathVariableDiagnosticsServiceId);
const diagnostics = await envHealthCheck.diagnose();
this.log(diagnostics);
if (diagnostics.length > 0) {
await envHealthCheck.handle(diagnostics);
}
const diagnosticsServices = this.serviceContainer.getAll<IDiagnosticsService>(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>(ILogger);
Expand Down
115 changes: 115 additions & 0 deletions src/client/application/diagnostics/checks/invalidDebuggerType.ts
Original file line number Diff line number Diff line change
@@ -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<MessageCommandPrompt>;
protected readonly fs: IFileSystem;
constructor(@inject(IServiceContainer) serviceContainer: IServiceContainer) {
super([DiagnosticCodes.InvalidEnvironmentPathVariableDiagnostic], serviceContainer);
this.messageService = serviceContainer.get<IDiagnosticHandlerService<MessageCommandPrompt>>(IDiagnosticHandlerService, DiagnosticCommandPromptHandlerServiceId);
const cmdManager = serviceContainer.get<ICommandManager>(ICommandManager);
this.fs = this.serviceContainer.get<IFileSystem>(IFileSystem);
cmdManager.registerCommand(CommandName, this.fixLaunchJson, this);
}
public async diagnose(): Promise<IDiagnostic[]> {
if (await this.isExperimentalDebuggerUsed()) {
return [new InvalidDebuggerTypeDiagnostic(InvalidDebuggerTypeMessage)];
} else {
return [];
}
}
public async handle(diagnostics: IDiagnostic[]): Promise<void> {
// 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>(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>(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>(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);
}
}
19 changes: 19 additions & 0 deletions src/client/application/diagnostics/commands/execVSCCommand.ts
Original file line number Diff line number Diff line change
@@ -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<void> {
const cmdManager = this.serviceContainer.get<ICommandManager>(ICommandManager);
return cmdManager.executeCommand(this.commandName).then(() => undefined);
}
}
4 changes: 4 additions & 0 deletions src/client/application/diagnostics/commands/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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}'`);
}
Expand Down
3 changes: 2 additions & 1 deletion src/client/application/diagnostics/commands/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import { DiagnosticScope, IDiagnostic, IDiagnosticCommand } from '../types';
export type CommandOption<Type, Option> = { 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');

Expand Down
3 changes: 2 additions & 1 deletion src/client/application/diagnostics/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
'use strict';

export enum DiagnosticCodes {
InvalidEnvironmentPathVariableDiagnostic = 'InvalidEnvironmentPathVariableDiagnostic'
InvalidEnvironmentPathVariableDiagnostic = 'InvalidEnvironmentPathVariableDiagnostic',
InvalidDebuggerTypeDiagnostic = 'InvalidDebuggerTypeDiagnostic'
}
2 changes: 2 additions & 0 deletions src/client/application/diagnostics/serviceRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -15,5 +16,6 @@ export function registerTypes(serviceManager: IServiceManager) {
serviceManager.addSingleton<IDiagnosticFilterService>(IDiagnosticFilterService, DiagnosticFilterService);
serviceManager.addSingleton<IDiagnosticHandlerService<MessageCommandPrompt>>(IDiagnosticHandlerService, DiagnosticCommandPromptHandlerService, DiagnosticCommandPromptHandlerServiceId);
serviceManager.addSingleton<IDiagnosticsService>(IDiagnosticsService, EnvironmentPathVariableDiagnosticsService, EnvironmentPathVariableDiagnosticsServiceId);
serviceManager.addSingleton<IDiagnosticsService>(IDiagnosticsService, InvalidDebuggerTypeDiagnosticsService, InvalidDebuggerTypeDiagnosticsServiceId);
serviceManager.addSingleton<IDiagnosticsCommandFactory>(IDiagnosticsCommandFactory, DiagnosticsCommandFactory);
}
4 changes: 4 additions & 0 deletions src/client/common/platform/fileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,8 @@ export class FileSystem implements IFileSystem {
});
});
}

public writeFile(filePath: string, data: {}): Promise<void> {
return fs.writeFile(filePath, data);
}
}
1 change: 1 addition & 0 deletions src/client/common/platform/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export interface IFileSystem {
getSubDirectories(rootDir: string): Promise<string[]>;
arePathsSame(path1: string, path2: string): boolean;
readFile(filePath: string): Promise<string>;
writeFile(filePath: string, data: {}): Promise<void>;
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,35 @@ 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<IServiceContainer>;
let envHealthCheck: typemoq.IMock<IDiagnosticsService>;
let debuggerTypeCheck: typemoq.IMock<IDiagnosticsService>;
let outputChannel: typemoq.IMock<IOutputChannel>;
let logger: typemoq.IMock<ILogger>;
let appDiagnostics: IApplicationDiagnostics;

setup(() => {
serviceContainer = typemoq.Mock.ofType<IServiceContainer>();
envHealthCheck = typemoq.Mock.ofType<IDiagnosticsService>();
debuggerTypeCheck = typemoq.Mock.ofType<IDiagnosticsService>();
outputChannel = typemoq.Mock.ofType<IOutputChannel>();
logger = typemoq.Mock.ofType<ILogger>();

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)))
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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))
Expand All @@ -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();
});
Expand Down