Fixing validation of 'Improvements to message displayed when python path is invalid (in launch.json)' (microsoft#4136)

* added pythonPathSource information * Code reviews

Author: Kartik Raj

src/client/application/diagnostics/checks/invalidPythonPathInDebugger.ts

@@ -13,6 +13,7 @@ import { traceError } from '../../../common/logger';
 import { IConfigurationService, Resource } from '../../../common/types';
 import { Diagnostics } from '../../../common/utils/localize';
 import { SystemVariables } from '../../../common/variables/systemVariables';
+import { PythonPathSource } from '../../../debugger/extension/types';
 import { IInterpreterHelper } from '../../../interpreter/contracts';
 import { IServiceContainer } from '../../../ioc/types';
 import { BaseDiagnostic, BaseDiagnosticsService } from '../base';
@@ -74,19 +75,17 @@ export class InvalidPythonPathInDebuggerService extends BaseDiagnosticsService
  public async diagnose(_resource: Resource): Promise<IDiagnostic[]> {
  return [];
  }
- public async validatePythonPath(pythonPath?: string, resource?: Uri) {
+ public async validatePythonPath(pythonPath?: string, pythonPathSource?: PythonPathSource, resource?: Uri) {
  pythonPath = pythonPath ? this.resolveVariables(pythonPath, resource) : undefined;
- let pathInLaunchJson = true;
  // tslint:disable-next-line:no-invalid-template-strings
  if (pythonPath === '${config:python.pythonPath}' || !pythonPath) {
- pathInLaunchJson = false;
  pythonPath = this.configService.getSettings(resource).pythonPath;
  }
  if (await this.interpreterHelper.getInterpreterInformation(pythonPath).catch(() => undefined)) {
  return true;
  }
  traceError(`Invalid Python Path '${pythonPath}'`);
- if (pathInLaunchJson) {
+ if (pythonPathSource === PythonPathSource.launchJson) {
  this.handle([new InvalidPythonPathInDebuggerDiagnostic(DiagnosticCodes.InvalidPythonPathInDebuggerLaunchDiagnostic, resource)])
  .catch(ex => traceError('Failed to handle invalid python path in launch.json debugger', ex))
  .ignoreErrors();

src/client/application/diagnostics/types.ts

@@ -5,6 +5,7 @@
 
 import { DiagnosticSeverity, Uri } from 'vscode';
 import { Resource } from '../../common/types';
+import { PythonPathSource } from '../../debugger/extension/types';
 import { DiagnosticCodes } from './constants';
 
 export enum DiagnosticScope {
@@ -35,10 +36,6 @@ export interface IDiagnosticsService {
  handle(diagnostics: IDiagnostic[]): Promise<void>;
 }
 
-export interface IInvalidPythonPathInDebuggerService extends IDiagnosticsService {
- validatePythonPath(pythonPath?: string, resource?: Uri): Promise<boolean>;
-}
-
 export const IDiagnosticFilterService = Symbol('IDiagnosticFilterService');
 
 export interface IDiagnosticFilterService {
@@ -60,7 +57,7 @@ export interface IDiagnosticCommand {
 export const IInvalidPythonPathInDebuggerService = Symbol('IInvalidPythonPathInDebuggerService');
 
 export interface IInvalidPythonPathInDebuggerService extends IDiagnosticsService {
- validatePythonPath(pythonPath?: string, resource?: Uri): Promise<boolean>;
+ validatePythonPath(pythonPath?: string, pythonPathSource?: PythonPathSource, resource?: Uri): Promise<boolean>;
 }
 export const ISourceMapSupportService = Symbol('ISourceMapSupportService');
 export interface ISourceMapSupportService {

src/client/debugger/extension/configuration/resolvers/base.ts

@@ -15,10 +15,12 @@ import { sendTelemetryEvent } from '../../../../telemetry';
 import { EventName } from '../../../../telemetry/constants';
 import { DebuggerTelemetry } from '../../../../telemetry/types';
 import { AttachRequestArguments, DebugOptions, LaunchRequestArguments } from '../../../types';
+import { PythonPathSource } from '../../types';
 import { IDebugConfigurationResolver } from '../types';
 
 @injectable()
 export abstract class BaseConfigurationResolver<T extends DebugConfiguration> implements IDebugConfigurationResolver<T> {
+ protected pythonPathSource: PythonPathSource;
  constructor(protected readonly workspaceService: IWorkspaceService,
  protected readonly documentManager: IDocumentManager,
  protected readonly configurationService: IConfigurationService) { }
@@ -54,6 +56,9 @@ export abstract class BaseConfigurationResolver<T extends DebugConfiguration> im
  if (debugConfiguration.pythonPath === '${config:python.pythonPath}' || !debugConfiguration.pythonPath) {
  const pythonPath = this.configurationService.getSettings(workspaceFolder).pythonPath;
  debugConfiguration.pythonPath = pythonPath;
+ this.pythonPathSource = PythonPathSource.settingsJson;
+ } else{
+ this.pythonPathSource = PythonPathSource.launchJson;
  }
  }
  protected debugOption(debugOptions: DebugOptions[], debugOption: DebugOptions) {

src/client/debugger/extension/configuration/resolvers/launch.ts

@@ -124,6 +124,6 @@ export class LaunchConfigurationResolver extends BaseConfigurationResolver<Launc
 
  protected async validateLaunchConfiguration(folder: WorkspaceFolder | undefined, debugConfiguration: LaunchRequestArguments): Promise<boolean> {
  const diagnosticService = this.invalidPythonPathInDebuggerService;
- return diagnosticService.validatePythonPath(debugConfiguration.pythonPath, folder ? folder.uri : undefined);
+ return diagnosticService.validatePythonPath(debugConfiguration.pythonPath, this.pythonPathSource, folder ? folder.uri : undefined);
  }
 }

src/client/debugger/extension/types.ts

@@ -29,3 +29,8 @@ export enum DebugConfigurationType {
  launchModule = 'launchModule',
  launchPyramid = 'launchPyramid'
 }
+
+export enum PythonPathSource {
+ launchJson = 'launch.json',
+ settingsJson = 'settings.json'
+}

src/test/application/diagnostics/checks/invalidPythonPathInDebugger.unit.test.ts

@@ -25,6 +25,7 @@ import {
 } from '../../../../client/application/diagnostics/types';
 import { IWorkspaceService } from '../../../../client/common/application/types';
 import { IConfigurationService, IPythonSettings } from '../../../../client/common/types';
+import { PythonPathSource } from '../../../../client/debugger/extension/types';
 import { IInterpreterHelper } from '../../../../client/interpreter/contracts';
 import { IServiceContainer } from '../../../../client/ioc/types';
 
@@ -271,7 +272,7 @@ suite('Application Diagnostics - Checks Python Path in debugger', () => {
  .returns(() => Promise.resolve({}))
  .verifiable(typemoq.Times.once());
 
- const valid = await diagnosticService.validatePythonPath(pythonPath, Uri.parse('something'));
+ const valid = await diagnosticService.validatePythonPath(pythonPath, PythonPathSource.settingsJson, Uri.parse('something'));
 
  configService.verifyAll();
  helper.verifyAll();
@@ -362,7 +363,7 @@ suite('Application Diagnostics - Checks Python Path in debugger', () => {
  .returns(() => Promise.resolve(undefined))
  .verifiable(typemoq.Times.once());
 
- const valid = await diagnosticService.validatePythonPath(pythonPath);
+ const valid = await diagnosticService.validatePythonPath(pythonPath, PythonPathSource.launchJson);
 
  helper.verifyAll();
  expect(valid).to.be.equal(false, 'should be invalid');
@@ -394,7 +395,7 @@ suite('Application Diagnostics - Checks Python Path in debugger', () => {
  .returns(() => Promise.resolve(undefined))
  .verifiable(typemoq.Times.once());
 
- const valid = await diagnosticService.validatePythonPath(pythonPath);
+ const valid = await diagnosticService.validatePythonPath(pythonPath, PythonPathSource.settingsJson);
 
  helper.verifyAll();
  expect(valid).to.be.equal(false, 'should be invalid');

src/test/debugger/extension/configuration/resolvers/launch.unit.test.ts

@@ -60,7 +60,7 @@ suite('Debugging - Config Resolver Launch', () => {
  factory.setup(f => f.create(TypeMoq.It.isAny())).returns(() => Promise.resolve(pythonExecutionService.object));
  helper.setup(h => h.getInterpreterInformation(TypeMoq.It.isAny())).returns(() => Promise.resolve({}));
  diagnosticsService
- .setup(h => h.validatePythonPath(TypeMoq.It.isAny(), TypeMoq.It.isAny()))
+ .setup(h => h.validatePythonPath(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
  .returns(() => Promise.resolve(true));
 
  const configProviderUtils = new ConfigurationProviderUtils(factory.object, fileSystem.object, appShell.object);
@@ -425,7 +425,7 @@ suite('Debugging - Config Resolver Launch', () => {
 
  diagnosticsService.reset();
  diagnosticsService
- .setup(h => h.validatePythonPath(TypeMoq.It.isValue(pythonPath), TypeMoq.It.isAny()))
+ .setup(h => h.validatePythonPath(TypeMoq.It.isValue(pythonPath), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
  .returns(() => Promise.resolve(false))
  .verifiable(TypeMoq.Times.once());
 
@@ -443,7 +443,7 @@ suite('Debugging - Config Resolver Launch', () => {
 
  diagnosticsService.reset();
  diagnosticsService
- .setup(h => h.validatePythonPath(TypeMoq.It.isValue(pythonPath), TypeMoq.It.isAny()))
+ .setup(h => h.validatePythonPath(TypeMoq.It.isValue(pythonPath), TypeMoq.It.isAny(), TypeMoq.It.isAny()))
  .returns(() => Promise.resolve(true))
  .verifiable(TypeMoq.Times.once());