mrva/vscode-codeql
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Merge pull request #3398 from github/robertbrignull/sort-in-modeling-store

Browse Source 
Move all sorting to the modeling store, and then all views preserve the sorting
This commit is contained in:
Robert
2024-02-26 11:05:59 +00:00
committed by GitHub
parent 1da465f7df 9a5654648d
commit 6e61ddb0f2
9 changed files with 52 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -16,7 +16,7 @@ import { INITIAL_HIDE_MODELED_METHODS_VALUE } from "../shared/hide-modeled-metho
import { getModelingStatus } from "../shared/modeling-status";
import { assertNever } from "../../common/helpers-pure";
import type { ModeledMethod } from "../modeled-method";
import { groupMethods, sortGroupNames, sortMethods } from "../shared/sorting";
import { groupMethods, sortGroupNames } from "../shared/sorting";
import type { Mode } from "../shared/mode";
import { INITIAL_MODE } from "../shared/mode";
import type { UrlValueResolvable } from "../../common/raw-result-types";
@@ -63,7 +63,6 @@ export class MethodsUsageDataProvider
mode: Mode,
modeledMethods: Readonly<Record<string, readonly ModeledMethod[]>>,
modifiedMethodSignatures: ReadonlySet<string>,
processedByAutoModelMethods: ReadonlySet<string>,
): Promise<void> {
if (
this.methods !== methods ||
@@ -74,15 +73,7 @@ export class MethodsUsageDataProvider
this.modifiedMethodSignatures !== modifiedMethodSignatures
) {
this.methods = methods;
this.sortedTreeItems = createTreeItems(
sortMethodsInGroups(
methods,
modeledMethods,
mode,
modifiedMethodSignatures,
processedByAutoModelMethods,
),
);
this.sortedTreeItems = createTreeItems(createGroups(methods, mode));
this.databaseItem = databaseItem;
this.sourceLocationPrefix =
await this.databaseItem.getSourceLocationPrefix(this.cliServer);
@@ -253,27 +244,9 @@ function urlValueResolvablesAreEqual(
return false;
}
function sortMethodsInGroups(
methods: readonly Method[],
modeledMethods: Readonly<Record<string, readonly ModeledMethod[]>>,
mode: Mode,
modifiedMethodSignatures: ReadonlySet<string>,
processedByAutoModelMethods: ReadonlySet<string>,
): Method[] {
function createGroups(methods: readonly Method[], mode: Mode): Method[] {
const grouped = groupMethods(methods, mode);
const sortedGroupNames = sortGroupNames(grouped);
return sortedGroupNames.flatMap((groupName) => {
const group = grouped[groupName];
return sortMethods(
group,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
});
return sortGroupNames(grouped).flatMap((groupName) => grouped[groupName]);
}
function createTreeItems(methods: readonly Method[]): MethodTreeViewItem[] {

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

@@ -39,7 +39,6 @@ export class MethodsUsagePanel extends DisposableObject {
mode: Mode,
modeledMethods: Readonly<Record<string, readonly ModeledMethod[]>>,
modifiedMethodSignatures: ReadonlySet<string>,
processedByAutoModelMethods: ReadonlySet<string>,
): Promise<void> {
await this.dataProvider.setState(
methods,
@@ -48,7 +47,6 @@ export class MethodsUsagePanel extends DisposableObject {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
const numOfApis = hideModeledMethods
? methods.filter((api) => !api.supported).length
@@ -122,7 +120,6 @@ export class MethodsUsagePanel extends DisposableObject {
activeState.mode,
activeState.modeledMethods,
activeState.modifiedMethodSignatures,
activeState.processedByAutoModelMethods,
);
}
}

2
extensions/ql-vscode/src/model-editor/model-editor-view.ts
View File

@@ -288,6 +288,8 @@ export class ModelEditorView extends AbstractWebview<
Object.keys(modeledMethods),
);
this.modelingStore.updateMethodSorting(this.databaseItem);
void telemetryListener?.sendUIInteraction(
"model-editor-save-modeled-methods",
);

46
extensions/ql-vscode/src/model-editor/modeling-store.ts
View File

@@ -6,6 +6,7 @@ import type { ModeledMethod } from "./modeled-method";
import type { ModelingEvents } from "./modeling-events";
import { INITIAL_HIDE_MODELED_METHODS_VALUE } from "./shared/hide-modeled-methods";
import type { Mode } from "./shared/mode";
import { sortMethods } from "./shared/sorting";
interface InternalDbModelingState {
databaseItem: DatabaseItem;
@@ -155,17 +156,25 @@ export class ModelingStore extends DisposableObject {
}
public setMethods(dbItem: DatabaseItem, methods: Method[]) {
const dbState = this.getState(dbItem);
const dbUri = dbItem.databaseUri.toString();
this.changeMethods(dbItem, (state) => {
state.methods = sortMethods(
methods,
state.modeledMethods,
state.modifiedMethodSignatures,
state.processedByAutoModelMethods,
);
});
}
dbState.methods = [...methods];
this.modelingEvents.fireMethodsChangedEvent(
methods,
dbUri,
dbItem,
dbUri === this.activeDb,
);
public updateMethodSorting(dbItem: DatabaseItem) {
this.changeMethods(dbItem, (state) => {
state.methods = sortMethods(
state.methods,
state.modeledMethods,
state.modifiedMethodSignatures,
state.processedByAutoModelMethods,
);
});
}
public setHideModeledMethods(
@@ -374,6 +383,7 @@ export class ModelingStore extends DisposableObject {
...processedByAutoModelMethods,
]);
});
this.updateMethodSorting(dbItem);
}
public updateModelEvaluationRun(
@@ -421,6 +431,22 @@ export class ModelingStore extends DisposableObject {
return this.state.get(databaseItem.databaseUri.toString())!;
}
private changeMethods(
dbItem: DatabaseItem,
updateState: (state: InternalDbModelingState) => void,
) {
const state = this.getState(dbItem);
updateState(state);
this.modelingEvents.fireMethodsChangedEvent(
state.methods,
dbItem.databaseUri.toString(),
dbItem,
dbItem.databaseUri.toString() === this.activeDb,
);
}
private changeModifiedMethods(
dbItem: DatabaseItem,
updateState: (state: InternalDbModelingState) => void,

9
extensions/ql-vscode/src/model-editor/shared/auto-model-candidates.ts
View File

@@ -1,7 +1,7 @@
import type { Method, MethodSignature } from "../method";
import type { ModeledMethod } from "../modeled-method";
import type { Mode } from "./mode";
import { groupMethods, sortGroupNames, sortMethods } from "./sorting";
import { groupMethods, sortGroupNames } from "./sorting";
/**
* Return the candidates that the model should be run on. This includes limiting the number of
@@ -44,10 +44,5 @@ export function getCandidates(
// Sort the same way as the UI so we send the first ones listed in the UI first
const grouped = groupMethods(candidateMethods, mode);
const sortedGroupNames = sortGroupNames(grouped);
return sortedGroupNames.flatMap((name) =>
// We can safely pass empty sets for `modifiedSignatures` and `processedByAutoModelMethods`
// because we've filtered out all methods that are already modeled or have already been processed by auto-model.
sortMethods(grouped[name], modeledMethodsBySignature, new Set(), new Set()),
);
return sortGroupNames(grouped).flatMap((name) => grouped[name]);
}

4
extensions/ql-vscode/src/model-editor/shared/sorting.ts
View File

@@ -4,6 +4,10 @@ import type { ModeledMethod } from "../modeled-method";
import { Mode } from "./mode";
import { calculateModeledPercentage } from "./modeled-percentage";
/**
* Groups methods by library or package name.
* Does not change the order of methods within a group.
*/
export function groupMethods(
methods: readonly Method[],
mode: Mode,

16
extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx
View File

@@ -3,7 +3,6 @@ import type { Method } from "../../model-editor/method";
import { canMethodBeModeled } from "../../model-editor/method";
import type { ModeledMethod } from "../../model-editor/modeled-method";
import { useMemo } from "react";
import { sortMethods } from "../../model-editor/shared/sorting";
import { HiddenMethodsRow } from "./HiddenMethodsRow";
import type { ModelEditorViewState } from "../../model-editor/shared/view-state";
import { ScreenReaderOnly } from "../common/ScreenReaderOnly";
@@ -48,12 +47,7 @@ export const ModeledMethodDataGrid = ({
] = useMemo(() => {
const methodsWithModelability = [];
let numHiddenMethods = 0;
for (const method of sortMethods(
methods,
modeledMethodsMap,
modifiedSignatures,
processedByAutoModelMethods,
)) {
for (const method of methods) {
const modeledMethods = modeledMethodsMap[method.signature] ?? [];
const methodIsUnsaved = modifiedSignatures.has(method.signature);
const methodCanBeModeled = canMethodBeModeled(
@@ -69,13 +63,7 @@ export const ModeledMethodDataGrid = ({
}
}
return [methodsWithModelability, numHiddenMethods];
}, [
hideModeledMethods,
methods,
modeledMethodsMap,
modifiedSignatures,
processedByAutoModelMethods,
]);
}, [hideModeledMethods, methods, modeledMethodsMap, modifiedSignatures]);
const someMethodsAreVisible = methodsWithModelability.length > 0;

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

@@ -27,7 +27,6 @@ describe("MethodsUsageDataProvider", () => {
const methods: Method[] = [];
const modeledMethods: Record<string, ModeledMethod[]> = {};
const modifiedMethodSignatures: Set<string> = new Set();
const processedByAutoModelMethods: Set<string> = new Set();
const dbItem = mockedObject<DatabaseItem>({
getSourceLocationPrefix: () => "test",
});
@@ -40,7 +39,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
const onDidChangeTreeDataListener = jest.fn();
@@ -53,7 +51,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
expect(onDidChangeTreeDataListener).not.toHaveBeenCalled();
@@ -69,7 +66,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
const onDidChangeTreeDataListener = jest.fn();
@@ -82,7 +78,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
expect(onDidChangeTreeDataListener).toHaveBeenCalledTimes(1);
@@ -100,7 +95,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
const onDidChangeTreeDataListener = jest.fn();
@@ -113,7 +107,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
expect(onDidChangeTreeDataListener).toHaveBeenCalledTimes(1);
@@ -127,7 +120,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
const onDidChangeTreeDataListener = jest.fn();
@@ -140,7 +132,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
expect(onDidChangeTreeDataListener).toHaveBeenCalledTimes(1);
@@ -156,7 +147,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
const onDidChangeTreeDataListener = jest.fn();
@@ -169,7 +159,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods2,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
expect(onDidChangeTreeDataListener).toHaveBeenCalledTimes(1);
@@ -185,7 +174,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
const onDidChangeTreeDataListener = jest.fn();
@@ -198,7 +186,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures2,
processedByAutoModelMethods,
);
expect(onDidChangeTreeDataListener).toHaveBeenCalledTimes(1);
@@ -217,7 +204,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
const onDidChangeTreeDataListener = jest.fn();
@@ -230,7 +216,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
expect(onDidChangeTreeDataListener).toHaveBeenCalledTimes(1);
@@ -270,7 +255,6 @@ describe("MethodsUsageDataProvider", () => {
createSinkModeledMethod(),
];
const modifiedMethodSignatures: Set<string> = new Set();
const processedByAutoModelMethods: Set<string> = new Set();
const dbItem = mockedObject<DatabaseItem>({
getSourceLocationPrefix: () => "test",
@@ -307,7 +291,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
expect(dataProvider.getChildren().length).toEqual(4);
});
@@ -321,7 +304,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
expect(dataProvider.getChildren().length).toEqual(3);
});
@@ -410,7 +392,7 @@ describe("MethodsUsageDataProvider", () => {
}),
];
it("should sort methods", async () => {
it("should not change sort methods of methods", async () => {
await dataProvider.setState(
methods,
dbItem,
@@ -418,7 +400,6 @@ describe("MethodsUsageDataProvider", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
expect(
dataProvider
@@ -426,11 +407,7 @@ describe("MethodsUsageDataProvider", () => {
.map(
(item) => (item as MethodsUsageTreeViewItem).method.signature,
),
).toEqual(["b.a.C.d()", "b.a.C.b()", "b.a.C.a()", "a.b.C.d()"]);
// reasoning for sort order:
// b.a.C.d() has more usages than b.a.C.b()
// b.a.C.b() is supported, b.a.C.a() is not
// b.a.C.a() is in a more unsupported library, a.b.C.d() is in a more supported library
).toEqual(["b.a.C.a()", "b.a.C.b()", "b.a.C.d()", "a.b.C.d()"]);
});
});
});

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

@@ -28,7 +28,6 @@ describe("MethodsUsagePanel", () => {
const methods: Method[] = [createMethod()];
const modeledMethods: Record<string, ModeledMethod[]> = {};
const modifiedMethodSignatures: Set<string> = new Set();
const processedByAutoModelMethods: Set<string> = new Set();
it("should update the tree view with the correct batch number", async () => {
const mockTreeView = {
@@ -51,7 +50,6 @@ describe("MethodsUsagePanel", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
expect(mockTreeView.badge?.value).toBe(1);
@@ -67,7 +65,6 @@ describe("MethodsUsagePanel", () => {
const mode = Mode.Application;
const modeledMethods: Record<string, ModeledMethod[]> = {};
const modifiedMethodSignatures: Set<string> = new Set();
const processedByAutoModelMethods: Set<string> = new Set();
const usage = createUsage();
beforeEach(() => {
@@ -98,7 +95,6 @@ describe("MethodsUsagePanel", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
await panel.revealItem(method.signature, usage);
@@ -126,7 +122,6 @@ describe("MethodsUsagePanel", () => {
mode,
modeledMethods,
modifiedMethodSignatures,
processedByAutoModelMethods,
);
await panel.revealItem(method.signature, usage);