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,7 +75,7 @@ export class InvalidPythonPathInDebuggerService extends BaseDiagnosticsService
  public async diagnose(_resource: Resource): Promise<IDiagnostic[]> {
  return [];
  }
- public async validatePythonPath(pythonPath?: string, pythonPathSource?: 'settings'|'launch', resource?: Uri) {
+ public async validatePythonPath(pythonPath?: string, pythonPathSource?: PythonPathSource, resource?: Uri) {
  pythonPath = pythonPath ? this.resolveVariables(pythonPath, resource) : undefined;
  // tslint:disable-next-line:no-invalid-template-strings
  if (pythonPath === '${config:python.pythonPath}' || !pythonPath) {
@@ -84,7 +85,7 @@ export class InvalidPythonPathInDebuggerService extends BaseDiagnosticsService
  return true;
  }
  traceError(`Invalid Python Path '${pythonPath}'`);
- if (pythonPathSource === 'launch') {
+ 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 {
@@ -56,7 +57,7 @@ export interface IDiagnosticCommand {
 export const IInvalidPythonPathInDebuggerService = Symbol('IInvalidPythonPathInDebuggerService');
 
 export interface IInvalidPythonPathInDebuggerService extends IDiagnosticsService {
- validatePythonPath(pythonPath?: string, pythonPathSource?: 'settings'|'launch', 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,11 +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: 'settings'|'launch';
+ protected pythonPathSource: PythonPathSource;
  constructor(protected readonly workspaceService: IWorkspaceService,
  protected readonly documentManager: IDocumentManager,
  protected readonly configurationService: IConfigurationService) { }
@@ -55,9 +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 = 'settings';
+ this.pythonPathSource = PythonPathSource.settingsJson;
  } else{
- this.pythonPathSource = 'launch';
+ this.pythonPathSource = PythonPathSource.launchJson;
  }
  }
  protected debugOption(debugOptions: DebugOptions[], debugOption: DebugOptions) {

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

@@ -27,6 +27,7 @@ import { IWorkspaceService } from '../../../../client/common/application/types';
 import { IConfigurationService, IPythonSettings } from '../../../../client/common/types';
 import { IInterpreterHelper } from '../../../../client/interpreter/contracts';
 import { IServiceContainer } from '../../../../client/ioc/types';
+import { PythonPathSource } from '../../../../client/debugger/extension/types';
 
 suite('Application Diagnostics - Checks Python Path in debugger', () => {
  let diagnosticService: IInvalidPythonPathInDebuggerService;
@@ -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, 'settings', 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, 'launch');
+ 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, 'settings');
+ const valid = await diagnosticService.validatePythonPath(pythonPath, PythonPathSource.settingsJson);
 
  helper.verifyAll();
  expect(valid).to.be.equal(false, 'should be invalid');