Skip to content

Commit a852f16

Browse files
committed
Keep reference to original items in usage data provider
This changes the usage data provider tree items to keep a reference to the method and usage instead of only including their properties in the tree item. This makes it easier to find the original method and usage when revealing an item in the tree. It also removes the `getParent` call in `getTreeItem`. The main reason for this fix is to ensure `codeQLModelEditor.jumpToMethod` gets the correct `usage` argument. It received the tree item before, but now we can actually pass the usage that was clicked on.
1 parent a7f8019 commit a852f16

3 files changed

Lines changed: 33 additions & 23 deletions

File tree

extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-data-provider.ts

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -89,26 +89,26 @@ export class MethodsUsageDataProvider
8989

9090
getTreeItem(item: MethodsUsageTreeViewItem): TreeItem {
9191
if (isMethodTreeViewItem(item)) {
92+
const { method } = item;
93+
9294
return {
93-
label: `${item.packageName}.${item.typeName}.${item.methodName}${item.methodParameters}`,
95+
label: `${method.packageName}.${method.typeName}.${method.methodName}${method.methodParameters}`,
9496
collapsibleState: TreeItemCollapsibleState.Collapsed,
95-
iconPath: this.getModelingStatusIcon(item),
97+
iconPath: this.getModelingStatusIcon(method),
9698
};
9799
} else {
98-
const method = this.getParent(item);
99-
if (!method || !isMethodTreeViewItem(method)) {
100-
throw new Error("Parent not found for tree item");
101-
}
100+
const { method, usage } = item;
101+
102102
return {
103-
label: item.label,
104-
description: `${this.relativePathWithinDatabase(item.url.uri)} [${
105-
item.url.startLine
106-
}, ${item.url.endLine}]`,
103+
label: usage.label,
104+
description: `${this.relativePathWithinDatabase(usage.url.uri)} [${
105+
usage.url.startLine
106+
}, ${usage.url.endLine}]`,
107107
collapsibleState: TreeItemCollapsibleState.None,
108108
command: {
109109
title: "Show usage",
110110
command: "codeQLModelEditor.jumpToMethod",
111-
arguments: [method, item, this.databaseItem],
111+
arguments: [method, usage, this.databaseItem],
112112
},
113113
};
114114
}
@@ -146,7 +146,7 @@ export class MethodsUsageDataProvider
146146
getChildren(item?: MethodsUsageTreeViewItem): MethodsUsageTreeViewItem[] {
147147
if (item === undefined) {
148148
if (this.hideModeledMethods) {
149-
return this.sortedTreeItems.filter((api) => !api.supported);
149+
return this.sortedTreeItems.filter((api) => !api.method.supported);
150150
} else {
151151
return [...this.sortedTreeItems];
152152
}
@@ -172,21 +172,24 @@ export class MethodsUsageDataProvider
172172
usage: Usage,
173173
): UsageTreeViewItem | undefined {
174174
const method = this.sortedTreeItems.find(
175-
(m) => m.signature === methodSignature,
175+
(m) => m.method.signature === methodSignature,
176176
);
177177
if (!method) {
178178
return undefined;
179179
}
180180

181-
return method.children.find((u) => usagesAreEqual(u, usage));
181+
return method.children.find((u) => usagesAreEqual(u.usage, usage));
182182
}
183183
}
184184

185-
type MethodTreeViewItem = Method & {
185+
type MethodTreeViewItem = {
186+
method: Method;
186187
children: UsageTreeViewItem[];
187188
};
188189

189-
type UsageTreeViewItem = Usage & {
190+
type UsageTreeViewItem = {
191+
method: Method;
192+
usage: Usage;
190193
parent: MethodTreeViewItem;
191194
};
192195

@@ -195,7 +198,7 @@ export type MethodsUsageTreeViewItem = MethodTreeViewItem | UsageTreeViewItem;
195198
function isMethodTreeViewItem(
196199
item: MethodsUsageTreeViewItem,
197200
): item is MethodTreeViewItem {
198-
return "children" in item && "usages" in item;
201+
return "children" in item && "method" in item;
199202
}
200203

201204
function usagesAreEqual(u1: Usage, u2: Usage): boolean {
@@ -225,12 +228,13 @@ function sortMethodsInGroups(methods: readonly Method[], mode: Mode): Method[] {
225228
function createTreeItems(methods: readonly Method[]): MethodTreeViewItem[] {
226229
return methods.map((method) => {
227230
const newMethod: MethodTreeViewItem = {
228-
...method,
231+
method,
229232
children: [],
230233
};
231234

232235
newMethod.children = method.usages.map((usage) => ({
233-
...usage,
236+
method,
237+
usage,
234238
// This needs to be a reference to the parent method, not a copy of it.
235239
parent: newMethod,
236240
}));

extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/methods-usage/methods-usage-data-provider.test.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -246,12 +246,13 @@ describe("MethodsUsageDataProvider", () => {
246246
const usage = createUsage({});
247247

248248
const methodTreeItem: MethodsUsageTreeViewItem = {
249-
...supportedMethod,
249+
method: supportedMethod,
250250
children: [],
251251
};
252252

253253
const usageTreeItem: MethodsUsageTreeViewItem = {
254-
...usage,
254+
method: supportedMethod,
255+
usage,
255256
parent: methodTreeItem,
256257
};
257258
methodTreeItem.children = [usageTreeItem];
@@ -383,7 +384,9 @@ describe("MethodsUsageDataProvider", () => {
383384
expect(
384385
dataProvider
385386
.getChildren()
386-
.map((item) => (item as Method).signature),
387+
.map(
388+
(item) => (item as MethodsUsageTreeViewItem).method.signature,
389+
),
387390
).toEqual(["b.a.C.d()", "b.a.C.b()", "b.a.C.a()", "a.b.C.d()"]);
388391
// reasoning for sort order:
389392
// b.a.C.d() has more usages than b.a.C.b()

extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/methods-usage/methods-usage-panel.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,10 @@ describe("MethodsUsagePanel", () => {
8686
await panel.revealItem(method.signature, usage);
8787

8888
expect(mockTreeView.reveal).toHaveBeenCalledWith(
89-
expect.objectContaining(usage),
89+
expect.objectContaining({
90+
method,
91+
usage,
92+
}),
9093
);
9194
});
9295

0 commit comments

Comments
 (0)