Skip to content

Commit 88ab735

Browse files
authored
Fix type safeness of executeCommand (#16383)
1 parent a960c8e commit 88ab735

37 files changed

Lines changed: 139 additions & 125 deletions

src/client/common/application/applicationEnvironment.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,11 @@ import { inject, injectable } from 'inversify';
77
import * as path from 'path';
88
import { parse } from 'semver';
99
import * as vscode from 'vscode';
10+
import { Channel } from '../constants';
1011
import { IPlatformService } from '../platform/types';
1112
import { ICurrentProcess, IPathUtils } from '../types';
1213
import { OSType } from '../utils/platform';
13-
import { Channel, IApplicationEnvironment } from './types';
14+
import { IApplicationEnvironment } from './types';
1415

1516
@injectable()
1617
export class ApplicationEnvironment implements IApplicationEnvironment {

src/client/common/application/commands.ts

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,11 @@
33

44
'use strict';
55

6-
import { CancellationToken, Disposable, Position, TextDocument, Uri } from 'vscode';
6+
import { CancellationToken, Position, TextDocument, Uri } from 'vscode';
77
import { Commands as LSCommands } from '../../activation/commands';
88
import { TensorBoardEntrypoint, TensorBoardEntrypointTrigger } from '../../tensorBoard/constants';
99
import { TestDataItem, TestFunction, TestsToRun, TestWorkspaceFolder } from '../../testing/common/types';
10-
import { Commands } from '../constants';
11-
import { Channel, CommandSource, ICommandManager } from './types';
10+
import { Channel, Commands, CommandSource } from '../constants';
1211

1312
export type CommandsWithoutArgs = keyof ICommandNameWithoutArgumentTypeMapping;
1413

@@ -48,10 +47,15 @@ interface ICommandNameWithoutArgumentTypeMapping {
4847
[Commands.Tests_Discovering]: [];
4948
[Commands.PickLocalProcess]: [];
5049
[Commands.OpenStartPage]: [];
50+
[Commands.ClearStorage]: [];
51+
[Commands.ReportIssue]: [];
52+
[Commands.RefreshTensorBoard]: [];
5153
[LSCommands.ClearAnalyisCache]: [];
5254
[LSCommands.RestartLS]: [];
5355
}
5456

57+
export type AllCommands = keyof ICommandNameArgumentTypeMapping;
58+
5559
/**
5660
* Mapping between commands and list of arguments.
5761
* Used to provide strong typing for command & args.
@@ -86,6 +90,7 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu
8690
['jupyter.opennotebook']: [undefined | Uri, undefined | CommandSource];
8791
['jupyter.runallcells']: [Uri];
8892
['extension.open']: [string];
93+
['workbench.action.openIssueReporter']: [{ extensionId: string; issueBody: string }];
8994
[Commands.GetSelectedInterpreterPath]: [{ workspaceFolder: string } | string[]];
9095
[Commands.Build_Workspace_Symbols]: [boolean, CancellationToken];
9196
[Commands.Sort_Imports]: [undefined, Uri];
@@ -130,17 +135,3 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu
130135
[Commands.navigateToTestSuite]: [Uri, TestDataItem, boolean];
131136
[Commands.LaunchTensorBoard]: [TensorBoardEntrypoint, TensorBoardEntrypointTrigger];
132137
}
133-
134-
//export const IPythonCommandManager = Symbol('IPythonCommandManager');
135-
export interface IPythonCommandManager extends ICommandManager {
136-
registerCommand<E extends keyof ICommandNameArgumentTypeMapping, U extends ICommandNameArgumentTypeMapping[E]>(
137-
command: E,
138-
callback: (...args: U) => any,
139-
thisArg?: any,
140-
): Disposable;
141-
142-
executeCommand<T, E extends keyof ICommandNameArgumentTypeMapping, U extends ICommandNameArgumentTypeMapping[E]>(
143-
command: E,
144-
...rest: U
145-
): Thenable<T | undefined>;
146-
}

src/client/common/application/commands/reportIssueCommand.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { EXTENSION_ROOT_DIR } from '../../../constants';
1212
import { IInterpreterService, IInterpreterVersionService } from '../../../interpreter/contracts';
1313
import { identifyEnvironment } from '../../../pythonEnvironments/common/environmentIdentifier';
1414
import { getPythonOutputChannelContent } from '../../../logging';
15+
import { Commands } from '../../constants';
1516

1617
/**
1718
* Allows the user to report an issue related to the Python extension using our template.
@@ -26,7 +27,7 @@ export class ReportIssueCommandHandler implements IExtensionSingleActivationServ
2627
) {}
2728

2829
public async activate(): Promise<void> {
29-
this.commandManager.registerCommand('python.reportIssue', this.openReportIssue, this);
30+
this.commandManager.registerCommand(Commands.ReportIssue, this.openReportIssue, this);
3031
}
3132

3233
private templatePath = path.join(EXTENSION_ROOT_DIR, 'resources', 'report_issue_template.md');

src/client/common/application/types.ts

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,9 @@ import {
6060
} from 'vscode';
6161
import type { NotebookConcatTextDocument, NotebookDocument } from 'vscode-proposed';
6262

63+
import { Channel } from '../constants';
6364
import { IAsyncDisposable, Resource } from '../types';
64-
65-
export enum CommandSource {
66-
auto = 'auto',
67-
ui = 'ui',
68-
codelens = 'codelens',
69-
commandPalette = 'commandpalette',
70-
testExplorer = 'testExplorer',
71-
}
65+
import { ICommandNameArgumentTypeMapping } from './commands';
7266

7367
export const IApplicationShell = Symbol('IApplicationShell');
7468
export interface IApplicationShell {
@@ -437,7 +431,11 @@ export interface ICommandManager {
437431
* @param thisArg The `this` context used when invoking the handler function.
438432
* @return Disposable which unregisters this command on disposal.
439433
*/
440-
registerCommand(command: string, callback: (...args: any[]) => any, thisArg?: any): Disposable;
434+
registerCommand<E extends keyof ICommandNameArgumentTypeMapping, U extends ICommandNameArgumentTypeMapping[E]>(
435+
command: E,
436+
callback: (...args: U) => any,
437+
thisArg?: any,
438+
): Disposable;
441439

442440
/**
443441
* Registers a text editor command that can be invoked via a keyboard shortcut,
@@ -473,7 +471,10 @@ export interface ICommandManager {
473471
* @return A thenable that resolves to the returned value of the given command. `undefined` when
474472
* the command handler function doesn't return anything.
475473
*/
476-
executeCommand<T>(command: string, ...rest: any[]): Thenable<T | undefined>;
474+
executeCommand<T, E extends keyof ICommandNameArgumentTypeMapping, U extends ICommandNameArgumentTypeMapping[E]>(
475+
command: E,
476+
...rest: U
477+
): Thenable<T | undefined>;
477478

478479
/**
479480
* Retrieve the list of all available commands. Commands starting an underscore are
@@ -1164,8 +1165,6 @@ export interface ILanguageService {
11641165
): Disposable;
11651166
}
11661167

1167-
export type Channel = 'stable' | 'insiders';
1168-
11691168
/**
11701169
* Wraps the `ActiveResourceService` API class. Created for injecting and mocking class methods in testing
11711170
*/

src/client/common/constants.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,16 @@ export const PYLANCE_EXTENSION_ID = 'ms-python.vscode-pylance';
1515
export const JUPYTER_EXTENSION_ID = 'ms-toolsai.jupyter';
1616
export const AppinsightsKey = 'AIF-d9b70cd4-b9f9-4d70-929b-a071c400b217';
1717

18+
export type Channel = 'stable' | 'insiders';
19+
20+
export enum CommandSource {
21+
auto = 'auto',
22+
ui = 'ui',
23+
codelens = 'codelens',
24+
commandPalette = 'commandpalette',
25+
testExplorer = 'testExplorer',
26+
}
27+
1828
export namespace Commands {
1929
export const Set_Interpreter = 'python.setInterpreter';
2030
export const Set_ShebangInterpreter = 'python.setShebangInterpreter';
@@ -69,6 +79,7 @@ export namespace Commands {
6979
export const OpenStartPage = 'python.startPage.open';
7080
export const LaunchTensorBoard = 'python.launchTensorBoard';
7181
export const RefreshTensorBoard = 'python.refreshTensorBoard';
82+
export const ReportIssue = 'python.reportIssue';
7283
}
7384
export namespace Octicons {
7485
export const Test_Pass = '$(check)';

src/client/common/serviceRegistry.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import { ApplicationEnvironment } from './application/applicationEnvironment';
1818
import { ApplicationShell } from './application/applicationShell';
1919
import { ClipboardService } from './application/clipboard';
2020
import { CommandManager } from './application/commandManager';
21-
import { IPythonCommandManager } from './application/commands';
2221
import { ReloadVSCodeCommandHandler } from './application/commands/reloadCommand';
2322
import { ReportIssueCommandHandler } from './application/commands/reportIssueCommand';
2423
import { DebugService } from './application/debugService';
@@ -141,7 +140,7 @@ export function registerTypes(serviceManager: IServiceManager) {
141140
IJupyterExtensionDependencyManager,
142141
JupyterExtensionDependencyManager,
143142
);
144-
serviceManager.addSingleton<IPythonCommandManager>(ICommandManager, CommandManager);
143+
serviceManager.addSingleton<ICommandManager>(ICommandManager, CommandManager);
145144
serviceManager.addSingleton<IConfigurationService>(IConfigurationService, ConfigurationService);
146145
serviceManager.addSingleton<IWorkspaceService>(IWorkspaceService, WorkspaceService);
147146
serviceManager.addSingleton<IProcessLogger>(IProcessLogger, ProcessLogger);

src/client/common/startPage/startPage.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,14 @@ import { IExtensionSingleActivationService } from '../../activation/types';
1010
import { EXTENSION_ROOT_DIR } from '../../constants';
1111
import { sendTelemetryEvent } from '../../telemetry';
1212
import {
13-
CommandSource,
1413
IApplicationEnvironment,
1514
IApplicationShell,
1615
ICommandManager,
1716
IDocumentManager,
1817
IWebviewPanelProvider,
1918
IWorkspaceService,
2019
} from '../application/types';
20+
import { CommandSource } from '../constants';
2121
import { IFileSystem } from '../platform/types';
2222
import { IConfigurationService, IExtensionContext, Resource } from '../types';
2323
import * as localize from '../utils/localize';

src/client/testing/codeLenses/testFiles.ts

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import {
1515
TextDocument,
1616
Uri,
1717
} from 'vscode';
18-
import { CommandSource } from '../../common/application/types';
1918
import { IWorkspaceService } from '../../../client/common/application/types';
2019
import { IFileSystem } from '../../../client/common/platform/types';
2120
import { IServiceContainer } from '../../../client/ioc/types';
@@ -167,12 +166,22 @@ export class TestFileCodeLensProvider implements CodeLensProvider {
167166
new CodeLens(range, {
168167
title: getTestStatusIcon(cls.status) + constants.Text.CodeLensRunUnitTest,
169168
command: constants.Commands.Tests_Run,
170-
arguments: [undefined, CommandSource.codelens, file, <TestsToRun>{ testSuite: [cls] }],
169+
arguments: [
170+
undefined,
171+
constants.CommandSource.codelens,
172+
file,
173+
<TestsToRun>{ testSuite: [cls] },
174+
],
171175
}),
172176
new CodeLens(range, {
173177
title: getTestStatusIcon(cls.status) + constants.Text.CodeLensDebugUnitTest,
174178
command: constants.Commands.Tests_Debug,
175-
arguments: [undefined, CommandSource.codelens, file, <TestsToRun>{ testSuite: [cls] }],
179+
arguments: [
180+
undefined,
181+
constants.CommandSource.codelens,
182+
file,
183+
<TestsToRun>{ testSuite: [cls] },
184+
],
176185
}),
177186
];
178187
}
@@ -251,12 +260,12 @@ function getFunctionCodeLens(
251260
new CodeLens(range, {
252261
title: getTestStatusIcon(fn.status) + constants.Text.CodeLensRunUnitTest,
253262
command: constants.Commands.Tests_Run,
254-
arguments: [undefined, CommandSource.codelens, file, <TestsToRun>{ testFunction: [fn] }],
263+
arguments: [undefined, constants.CommandSource.codelens, file, <TestsToRun>{ testFunction: [fn] }],
255264
}),
256265
new CodeLens(range, {
257266
title: getTestStatusIcon(fn.status) + constants.Text.CodeLensDebugUnitTest,
258267
command: constants.Commands.Tests_Debug,
259-
arguments: [undefined, CommandSource.codelens, file, <TestsToRun>{ testFunction: [fn] }],
268+
arguments: [undefined, constants.CommandSource.codelens, file, <TestsToRun>{ testFunction: [fn] }],
260269
}),
261270
];
262271
}
@@ -275,12 +284,12 @@ function getFunctionCodeLens(
275284
new CodeLens(range, {
276285
title: `${getTestStatusIcons(functions)} ${constants.Text.CodeLensRunUnitTest} (Multiple)`,
277286
command: constants.Commands.Tests_Picker_UI,
278-
arguments: [undefined, CommandSource.codelens, file, functions],
287+
arguments: [undefined, constants.CommandSource.codelens, file, functions],
279288
}),
280289
new CodeLens(range, {
281290
title: `${getTestStatusIcons(functions)} ${constants.Text.CodeLensDebugUnitTest} (Multiple)`,
282291
command: constants.Commands.Tests_Picker_UI_Debug,
283-
arguments: [undefined, CommandSource.codelens, file, functions],
292+
arguments: [undefined, constants.CommandSource.codelens, file, functions],
284293
}),
285294
];
286295
}

src/client/testing/common/managers/baseTestManager.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
OutputChannel,
1212
Uri,
1313
} from 'vscode';
14-
import { CommandSource, ICommandManager, IWorkspaceService } from '../../../common/application/types';
14+
import { ICommandManager, IWorkspaceService } from '../../../common/application/types';
1515
import '../../../common/extensions';
1616
import { isNotInstalledError } from '../../../common/helpers';
1717
import { traceError } from '../../../common/logger';
@@ -50,6 +50,7 @@ import {
5050
TestsToRun,
5151
WorkspaceTestStatus,
5252
} from '../types';
53+
import { CommandSource } from '../../../common/constants';
5354

5455
enum CancellationTokenType {
5556
testDiscovery,

src/client/testing/common/testUtils.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { inject, injectable, named } from 'inversify';
22
import * as path from 'path';
33
import { Uri, workspace } from 'vscode';
4-
import { CommandSource } from '../../common/application/types';
54
import { IApplicationShell, ICommandManager } from '../../common/application/types';
65
import * as constants from '../../common/constants';
76
import { Product } from '../../common/types';
@@ -230,7 +229,11 @@ export class TestsHelper implements ITestsHelper {
230229
public displayTestErrorMessage(message: string) {
231230
this.appShell.showErrorMessage(message, constants.Button_Text_Tests_View_Output).then((action) => {
232231
if (action === constants.Button_Text_Tests_View_Output) {
233-
this.commandManager.executeCommand(constants.Commands.Tests_ViewOutput, undefined, CommandSource.ui);
232+
this.commandManager.executeCommand(
233+
constants.Commands.Tests_ViewOutput,
234+
undefined,
235+
constants.CommandSource.ui,
236+
);
234237
}
235238
});
236239
}

0 commit comments

Comments
 (0)