Skip to content

Commit a671524

Browse files
authored
Avoid use of instanceof (#1625)
* Avoid use of instanceof instanceof causes issues in nightly because VSIX version and test version of the PackageNode class are different Also allow passing folder context to package commands as are some random focus issues * Fix review comment
1 parent 7574471 commit a671524

File tree

6 files changed

+56
-24
lines changed

6 files changed

+56
-24
lines changed

src/commands.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ export function register(ctx: WorkspaceContext): vscode.Disposable[] {
148148
),
149149
vscode.commands.registerCommand(
150150
Commands.RESET_PACKAGE,
151-
async () => await resetPackage(ctx)
151+
async folder => await resetPackage(ctx, folder)
152152
),
153153
vscode.commands.registerCommand("swift.runScript", async () => await runSwiftScript(ctx)),
154154
vscode.commands.registerCommand("swift.openPackage", async () => {
@@ -182,27 +182,27 @@ export function register(ctx: WorkspaceContext): vscode.Disposable[] {
182182
async () => await insertFunctionComment(ctx)
183183
),
184184
vscode.commands.registerCommand(Commands.USE_LOCAL_DEPENDENCY, async (item, dep) => {
185-
if (item instanceof PackageNode) {
185+
if (PackageNode.isPackageNode(item)) {
186186
return await useLocalDependency(item.name, ctx, dep);
187187
}
188188
}),
189-
vscode.commands.registerCommand("swift.editDependency", async item => {
190-
if (item instanceof PackageNode) {
191-
return await editDependency(item.name, ctx);
189+
vscode.commands.registerCommand("swift.editDependency", async (item, folder) => {
190+
if (PackageNode.isPackageNode(item)) {
191+
return await editDependency(item.name, ctx, folder);
192192
}
193193
}),
194-
vscode.commands.registerCommand(Commands.UNEDIT_DEPENDENCY, async item => {
195-
if (item instanceof PackageNode) {
196-
return await uneditDependency(item.name, ctx);
194+
vscode.commands.registerCommand(Commands.UNEDIT_DEPENDENCY, async (item, folder) => {
195+
if (PackageNode.isPackageNode(item)) {
196+
return await uneditDependency(item.name, ctx, folder);
197197
}
198198
}),
199199
vscode.commands.registerCommand("swift.openInWorkspace", async item => {
200-
if (item instanceof PackageNode) {
200+
if (PackageNode.isPackageNode(item)) {
201201
return await openInWorkspace(item);
202202
}
203203
}),
204204
vscode.commands.registerCommand("swift.openExternal", item => {
205-
if (item instanceof PackageNode) {
205+
if (PackageNode.isPackageNode(item)) {
206206
return openInExternalEditor(item);
207207
}
208208
}),

src/commands/dependencies/edit.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,19 @@ import { createSwiftTask } from "../../tasks/SwiftTaskProvider";
1717
import { FolderOperation, WorkspaceContext } from "../../WorkspaceContext";
1818
import { executeTaskWithUI } from "../utilities";
1919
import { packageName } from "../../utilities/tasks";
20+
import { FolderContext } from "../../FolderContext";
2021

2122
/**
2223
* Setup package dependency to be edited
2324
* @param identifier Identifier of dependency we want to edit
2425
* @param ctx workspace context
2526
*/
26-
export async function editDependency(identifier: string, ctx: WorkspaceContext) {
27-
const currentFolder = ctx.currentFolder;
27+
export async function editDependency(
28+
identifier: string,
29+
ctx: WorkspaceContext,
30+
folder: FolderContext | undefined
31+
) {
32+
const currentFolder = folder ?? ctx.currentFolder;
2833
if (!currentFolder) {
2934
return;
3035
}

src/commands/dependencies/unedit.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,12 @@ import { FolderContext } from "../../FolderContext";
2323
* @param identifier Identifier of dependency
2424
* @param ctx workspace context
2525
*/
26-
export async function uneditDependency(identifier: string, ctx: WorkspaceContext) {
27-
const currentFolder = ctx.currentFolder;
26+
export async function uneditDependency(
27+
identifier: string,
28+
ctx: WorkspaceContext,
29+
folder: FolderContext | undefined
30+
) {
31+
const currentFolder = folder ?? ctx.currentFolder;
2832
if (!currentFolder) {
2933
ctx.outputChannel.log("currentFolder is not set.");
3034
return false;

src/commands/resetPackage.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@ import { packageName } from "../utilities/tasks";
2222
/**
2323
* Executes a {@link vscode.Task task} to reset the complete cache/build directory.
2424
*/
25-
export async function resetPackage(ctx: WorkspaceContext) {
26-
const current = ctx.currentFolder;
25+
export async function resetPackage(ctx: WorkspaceContext, folder: FolderContext | undefined) {
26+
const current = folder ?? ctx.currentFolder;
2727
if (!current) {
2828
return;
2929
}

src/ui/ProjectPanelProvider.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,24 @@ async function getChildren(directoryPath: string, parentId?: string): Promise<Fi
6969
export class PackageNode {
7070
private id: string;
7171

72+
/**
73+
* "instanceof" has a bad effect in our nightly tests when the VSIX
74+
* bundled source is used. For example:
75+
*
76+
* ```
77+
* vscode.commands.registerCommand(Commands.UNEDIT_DEPENDENCY, async (item, folder) => {
78+
* if (item instanceof PackageNode) {
79+
* return await uneditDependency(item.name, ctx, folder);
80+
* }
81+
* }),
82+
* ```
83+
*
84+
* So instead we'll check for this set boolean property. Even if the implementation of the
85+
* {@link PackageNode} class changes, this property should not need to change
86+
*/
87+
static isPackageNode = (item: { __isPackageNode?: boolean }) => item.__isPackageNode ?? false;
88+
__isPackageNode = true;
89+
7290
constructor(
7391
private dependency: ResolvedDependency,
7492
private childDependencies: (dependency: Dependency) => ResolvedDependency[],

test/integration-tests/commands/dependency.test.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ import { FolderContext } from "../../../src/FolderContext";
2020
import { WorkspaceContext } from "../../../src/WorkspaceContext";
2121
import { Commands } from "../../../src/commands";
2222
import { activateExtensionForSuite, findWorkspaceFolder } from "../utilities/testutilities";
23-
import { executeTaskAndWaitForResult, waitForNoRunningTasks } from "../../utilities/tasks";
24-
import { createBuildAllTask } from "../../../src/tasks/SwiftTaskProvider";
23+
import { waitForNoRunningTasks } from "../../utilities/tasks";
2524

2625
suite("Dependency Commmands Test Suite @slow", function () {
2726
// full workflow's interaction with spm is longer than the default timeout
@@ -59,7 +58,6 @@ suite("Dependency Commmands Test Suite @slow", function () {
5958
setup(async () => {
6059
await waitForNoRunningTasks();
6160
treeProvider = new ProjectPanelProvider(workspaceContext);
62-
await executeTaskAndWaitForResult(await createBuildAllTask(depsContext));
6361
});
6462

6563
teardown(() => {
@@ -69,7 +67,9 @@ suite("Dependency Commmands Test Suite @slow", function () {
6967
async function getDependency() {
7068
const headers = await treeProvider.getChildren();
7169
const header = headers.find(n => n.name === "Dependencies") as PackageNode;
72-
expect(header).to.not.be.undefined;
70+
if (!header) {
71+
return;
72+
}
7373
const children = await header.getChildren();
7474
return children.find(
7575
n => n.name.toLocaleLowerCase() === "swift-markdown"
@@ -83,7 +83,7 @@ suite("Dependency Commmands Test Suite @slow", function () {
8383
async function getDependencyInState(state: "remote" | "editing") {
8484
for (let i = 0; i < 10; i++) {
8585
const dep = await getDependency();
86-
if (dep.type === state) {
86+
if (dep?.type === state) {
8787
return dep;
8888
}
8989
await new Promise(resolve => setTimeout(resolve, 1000));
@@ -98,7 +98,8 @@ suite("Dependency Commmands Test Suite @slow", function () {
9898
const result = await vscode.commands.executeCommand(
9999
Commands.USE_LOCAL_DEPENDENCY,
100100
item,
101-
localDep
101+
localDep,
102+
depsContext
102103
);
103104
expect(result).to.be.true;
104105

@@ -112,7 +113,10 @@ suite("Dependency Commmands Test Suite @slow", function () {
112113
await useLocalDependencyTest();
113114

114115
// spm reset
115-
const result = await vscode.commands.executeCommand(Commands.RESET_PACKAGE);
116+
const result = await vscode.commands.executeCommand(
117+
Commands.RESET_PACKAGE,
118+
depsContext
119+
);
116120
expect(result).to.be.true;
117121

118122
const dep = await getDependencyInState("remote");
@@ -125,7 +129,8 @@ suite("Dependency Commmands Test Suite @slow", function () {
125129

126130
const result = await vscode.commands.executeCommand(
127131
Commands.UNEDIT_DEPENDENCY,
128-
await getDependency()
132+
await getDependencyInState("editing"),
133+
depsContext
129134
);
130135
expect(result).to.be.true;
131136

0 commit comments

Comments
 (0)