Skip to content

Commit d715cee

Browse files
committed
Introduce separate tree item types in the methods usage panel
This creates new tree item types for methods and usages such that these can contain references to their parent and children. This allows us to easily find the parent of a usage and to find the children of a method. This removes an expensive `find` call in `getParent`.
1 parent 39a9f4c commit d715cee

File tree

5 files changed

+93
-40
lines changed

5 files changed

+93
-40
lines changed

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

Lines changed: 53 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export class MethodsUsageDataProvider
2727
private methods: readonly Method[] = [];
2828
// sortedMethods is a separate field so we can check if the methods have changed
2929
// by reference, which is faster than checking if the methods have changed by value.
30-
private sortedMethods: readonly Method[] = [];
30+
private sortedTreeItems: readonly MethodTreeViewItem[] = [];
3131
private databaseItem: DatabaseItem | undefined = undefined;
3232
private sourceLocationPrefix: string | undefined = undefined;
3333
private hideModeledMethods: boolean = INITIAL_HIDE_MODELED_METHODS_VALUE;
@@ -72,7 +72,9 @@ export class MethodsUsageDataProvider
7272
this.modifiedMethodSignatures !== modifiedMethodSignatures
7373
) {
7474
this.methods = methods;
75-
this.sortedMethods = sortMethodsInGroups(methods, mode);
75+
this.sortedTreeItems = createTreeItems(
76+
sortMethodsInGroups(methods, mode),
77+
);
7678
this.databaseItem = databaseItem;
7779
this.sourceLocationPrefix =
7880
await this.databaseItem.getSourceLocationPrefix(this.cliServer);
@@ -86,15 +88,15 @@ export class MethodsUsageDataProvider
8688
}
8789

8890
getTreeItem(item: MethodsUsageTreeViewItem): TreeItem {
89-
if (isExternalApiUsage(item)) {
91+
if (isMethodTreeViewItem(item)) {
9092
return {
9193
label: `${item.packageName}.${item.typeName}.${item.methodName}${item.methodParameters}`,
9294
collapsibleState: TreeItemCollapsibleState.Collapsed,
9395
iconPath: this.getModelingStatusIcon(item),
9496
};
9597
} else {
9698
const method = this.getParent(item);
97-
if (!method || !isExternalApiUsage(method)) {
99+
if (!method || !isMethodTreeViewItem(method)) {
98100
throw new Error("Parent not found for tree item");
99101
}
100102
return {
@@ -144,12 +146,12 @@ export class MethodsUsageDataProvider
144146
getChildren(item?: MethodsUsageTreeViewItem): MethodsUsageTreeViewItem[] {
145147
if (item === undefined) {
146148
if (this.hideModeledMethods) {
147-
return this.sortedMethods.filter((api) => !api.supported);
149+
return this.sortedTreeItems.filter((api) => !api.supported);
148150
} else {
149-
return [...this.sortedMethods];
151+
return [...this.sortedTreeItems];
150152
}
151-
} else if (isExternalApiUsage(item)) {
152-
return [...item.usages];
153+
} else if (isMethodTreeViewItem(item)) {
154+
return item.children;
153155
} else {
154156
return [];
155157
}
@@ -158,29 +160,42 @@ export class MethodsUsageDataProvider
158160
getParent(
159161
item: MethodsUsageTreeViewItem,
160162
): MethodsUsageTreeViewItem | undefined {
161-
if (isExternalApiUsage(item)) {
163+
if (isMethodTreeViewItem(item)) {
162164
return undefined;
163165
} else {
164-
return this.methods.find((e) => e.usages.includes(item));
166+
return item.parent;
165167
}
166168
}
167169

168-
public resolveCanonicalUsage(usage: Usage): Usage | undefined {
169-
for (const method of this.methods) {
170-
for (const u of method.usages) {
171-
if (usagesAreEqual(u, usage)) {
172-
return u;
173-
}
174-
}
170+
public resolveUsageTreeViewItem(
171+
methodSignature: string,
172+
usage: Usage,
173+
): UsageTreeViewItem | undefined {
174+
const method = this.sortedTreeItems.find(
175+
(m) => m.signature === methodSignature,
176+
);
177+
if (!method) {
178+
return undefined;
175179
}
176-
return undefined;
180+
181+
return method.children.find((u) => usagesAreEqual(u, usage));
177182
}
178183
}
179184

180-
export type MethodsUsageTreeViewItem = Method | Usage;
185+
type MethodTreeViewItem = Method & {
186+
children: UsageTreeViewItem[];
187+
};
188+
189+
type UsageTreeViewItem = Usage & {
190+
parent: MethodTreeViewItem;
191+
};
192+
193+
export type MethodsUsageTreeViewItem = MethodTreeViewItem | UsageTreeViewItem;
181194

182-
function isExternalApiUsage(item: MethodsUsageTreeViewItem): item is Method {
183-
return (item as any).usages !== undefined;
195+
function isMethodTreeViewItem(
196+
item: MethodsUsageTreeViewItem,
197+
): item is MethodTreeViewItem {
198+
return "children" in item && "usages" in item;
184199
}
185200

186201
function usagesAreEqual(u1: Usage, u2: Usage): boolean {
@@ -206,3 +221,20 @@ function sortMethodsInGroups(methods: readonly Method[], mode: Mode): Method[] {
206221
return sortMethods(group);
207222
});
208223
}
224+
225+
function createTreeItems(methods: readonly Method[]): MethodTreeViewItem[] {
226+
return methods.map((method) => {
227+
const newMethod: MethodTreeViewItem = {
228+
...method,
229+
children: [],
230+
};
231+
232+
newMethod.children = method.usages.map((usage) => ({
233+
...usage,
234+
// This needs to be a reference to the parent method, not a copy of it.
235+
parent: newMethod,
236+
}));
237+
238+
return newMethod;
239+
});
240+
}

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,16 @@ export class MethodsUsagePanel extends DisposableObject {
5656
};
5757
}
5858

59-
public async revealItem(usage: Usage): Promise<void> {
60-
const canonicalUsage = this.dataProvider.resolveCanonicalUsage(usage);
61-
if (canonicalUsage !== undefined) {
62-
await this.treeView.reveal(canonicalUsage);
59+
public async revealItem(
60+
methodSignature: string,
61+
usage: Usage,
62+
): Promise<void> {
63+
const usageTreeViewItem = this.dataProvider.resolveUsageTreeViewItem(
64+
methodSignature,
65+
usage,
66+
);
67+
if (usageTreeViewItem !== undefined) {
68+
await this.treeView.reveal(usageTreeViewItem);
6369
}
6470
}
6571

extensions/ql-vscode/src/model-editor/model-editor-module.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ export class ModelEditorModule extends DisposableObject {
102102
method: Method,
103103
usage: Usage,
104104
): Promise<void> {
105-
await this.methodsUsagePanel.revealItem(usage);
105+
await this.methodsUsagePanel.revealItem(method.signature, usage);
106106
await this.methodModelingPanel.setMethod(databaseItem, method);
107107
await showResolvableLocation(usage.url, databaseItem, this.app.logger);
108108
}

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

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ import {
33
CallClassification,
44
Method,
55
} from "../../../../../src/model-editor/method";
6-
import { MethodsUsageDataProvider } from "../../../../../src/model-editor/methods-usage/methods-usage-data-provider";
6+
import {
7+
MethodsUsageDataProvider,
8+
MethodsUsageTreeViewItem,
9+
} from "../../../../../src/model-editor/methods-usage/methods-usage-data-provider";
710
import { DatabaseItem } from "../../../../../src/databases/local-databases";
811
import {
912
createMethod,
@@ -242,13 +245,23 @@ describe("MethodsUsageDataProvider", () => {
242245

243246
const usage = createUsage({});
244247

248+
const methodTreeItem: MethodsUsageTreeViewItem = {
249+
...supportedMethod,
250+
children: [],
251+
};
252+
253+
const usageTreeItem: MethodsUsageTreeViewItem = {
254+
...usage,
255+
parent: methodTreeItem,
256+
};
257+
methodTreeItem.children = [usageTreeItem];
258+
245259
it("should return [] if item is a usage", async () => {
246-
expect(dataProvider.getChildren(usage)).toEqual([]);
260+
expect(dataProvider.getChildren(usageTreeItem)).toEqual([]);
247261
});
248262

249-
it("should return usages if item is external api usage", async () => {
250-
const method = createMethod({ usages: [usage] });
251-
expect(dataProvider.getChildren(method)).toEqual([usage]);
263+
it("should return usages if item is method", async () => {
264+
expect(dataProvider.getChildren(methodTreeItem)).toEqual([usageTreeItem]);
252265
});
253266

254267
it("should show all methods if hideModeledMethods is false and looking at the root", async () => {

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

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,10 @@ describe("MethodsUsagePanel", () => {
6868
});
6969

7070
it("should reveal the correct item in the tree view", async () => {
71-
const methods = [
72-
createMethod({
73-
usages: [usage],
74-
}),
75-
];
71+
const method = createMethod({
72+
usages: [usage],
73+
});
74+
const methods = [method];
7675

7776
const panel = new MethodsUsagePanel(modelingStore, mockCliServer);
7877
await panel.setState(
@@ -84,13 +83,16 @@ describe("MethodsUsagePanel", () => {
8483
modifiedMethodSignatures,
8584
);
8685

87-
await panel.revealItem(usage);
86+
await panel.revealItem(method.signature, usage);
8887

89-
expect(mockTreeView.reveal).toHaveBeenCalledWith(usage);
88+
expect(mockTreeView.reveal).toHaveBeenCalledWith(
89+
expect.objectContaining(usage),
90+
);
9091
});
9192

9293
it("should do nothing if usage cannot be found", async () => {
93-
const methods = [createMethod({})];
94+
const method = createMethod({});
95+
const methods = [method];
9496
const panel = new MethodsUsagePanel(modelingStore, mockCliServer);
9597
await panel.setState(
9698
methods,
@@ -101,7 +103,7 @@ describe("MethodsUsagePanel", () => {
101103
modifiedMethodSignatures,
102104
);
103105

104-
await panel.revealItem(usage);
106+
await panel.revealItem(method.signature, usage);
105107

106108
expect(mockTreeView.reveal).not.toHaveBeenCalled();
107109
});

0 commit comments

Comments
 (0)