From e75eccb4168798e258fcb1c3931f2459b5be56be Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 4 Oct 2023 12:34:40 +0100 Subject: [PATCH 01/24] Change MethodRow to accept ModelEditorViewState object instead of just Mode --- .../ql-vscode/src/view/model-editor/LibraryRow.tsx | 2 +- .../ql-vscode/src/view/model-editor/MethodRow.tsx | 11 ++++++----- .../src/view/model-editor/ModeledMethodDataGrid.tsx | 8 ++++---- .../view/model-editor/__tests__/MethodRow.spec.tsx | 12 +++++++++++- .../__tests__/ModeledMethodDataGrid.spec.tsx | 12 +++++++++++- 5 files changed, 33 insertions(+), 12 deletions(-) diff --git a/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx b/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx index 2c7871a69..74c2e3fbc 100644 --- a/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx @@ -235,7 +235,7 @@ export const LibraryRow = ({ modeledMethods={modeledMethods} modifiedSignatures={modifiedSignatures} inProgressMethods={inProgressMethods} - mode={viewState.mode} + viewState={viewState} hideModeledMethods={hideModeledMethods} revealedMethodSignature={revealedMethodSignature} onChange={onChange} diff --git a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx index b1880355c..21fde2801 100644 --- a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx @@ -21,6 +21,7 @@ import { MethodName } from "./MethodName"; import { ModelTypeDropdown } from "./ModelTypeDropdown"; import { ModelInputDropdown } from "./ModelInputDropdown"; import { ModelOutputDropdown } from "./ModelOutputDropdown"; +import { ModelEditorViewState } from "../../model-editor/shared/view-state"; const ApiOrMethodCell = styled(VSCodeDataGridCell)` display: flex; @@ -58,7 +59,7 @@ export type MethodRowProps = { modeledMethod: ModeledMethod | undefined; methodIsUnsaved: boolean; modelingInProgress: boolean; - mode: Mode; + viewState: ModelEditorViewState; revealedMethodSignature: string | null; onChange: (modeledMethod: ModeledMethod) => void; }; @@ -90,7 +91,7 @@ const ModelableMethodRow = forwardRef( method, modeledMethod, methodIsUnsaved, - mode, + viewState, revealedMethodSignature, onChange, } = props; @@ -112,7 +113,7 @@ const ModelableMethodRow = forwardRef( - {mode === Mode.Application && ( + {viewState.mode === Mode.Application && ( {method.usages.length} @@ -178,7 +179,7 @@ const UnmodelableMethodRow = forwardRef< HTMLElement | undefined, MethodRowProps >((props, ref) => { - const { method, mode, revealedMethodSignature } = props; + const { method, viewState, revealedMethodSignature } = props; const jumpToUsage = useCallback( () => sendJumpToUsageMessage(method), @@ -194,7 +195,7 @@ const UnmodelableMethodRow = forwardRef< - {mode === Mode.Application && ( + {viewState.mode === Mode.Application && ( {method.usages.length} diff --git a/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx b/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx index 11044c5ff..9ff27ad21 100644 --- a/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx +++ b/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx @@ -8,10 +8,10 @@ import { MethodRow } from "./MethodRow"; import { Method } from "../../model-editor/method"; import { ModeledMethod } from "../../model-editor/modeled-method"; import { useMemo } from "react"; -import { Mode } from "../../model-editor/shared/mode"; import { sortMethods } from "../../model-editor/shared/sorting"; import { InProgressMethods } from "../../model-editor/shared/in-progress-methods"; import { HiddenMethodsRow } from "./HiddenMethodsRow"; +import { ModelEditorViewState } from "../../model-editor/shared/view-state"; export const GRID_TEMPLATE_COLUMNS = "0.5fr 0.125fr 0.125fr 0.125fr 0.125fr"; @@ -21,7 +21,7 @@ export type ModeledMethodDataGridProps = { modeledMethods: Record; modifiedSignatures: Set; inProgressMethods: InProgressMethods; - mode: Mode; + viewState: ModelEditorViewState; hideModeledMethods: boolean; revealedMethodSignature: string | null; onChange: (modeledMethod: ModeledMethod) => void; @@ -33,7 +33,7 @@ export const ModeledMethodDataGrid = ({ modeledMethods, modifiedSignatures, inProgressMethods, - mode, + viewState, hideModeledMethods, revealedMethodSignature, onChange, @@ -95,7 +95,7 @@ export const ModeledMethodDataGrid = ({ packageName, method.signature, )} - mode={mode} + viewState={viewState} revealedMethodSignature={revealedMethodSignature} onChange={onChange} /> diff --git a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx index 2c22455c0..64874f60c 100644 --- a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx +++ b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx @@ -9,6 +9,8 @@ import { Mode } from "../../../model-editor/shared/mode"; import { MethodRow, MethodRowProps } from "../MethodRow"; import { ModeledMethod } from "../../../model-editor/modeled-method"; import userEvent from "@testing-library/user-event"; +import { ModelEditorViewState } from "../../../model-editor/shared/view-state"; +import { createMockExtensionPack } from "../../../../test/factories/model-editor/extension-pack"; describe(MethodRow.name, () => { const method = createMethod({ @@ -31,6 +33,14 @@ describe(MethodRow.name, () => { }; const onChange = jest.fn(); + const viewState: ModelEditorViewState = { + mode: Mode.Application, + showFlowGeneration: false, + showLlmButton: false, + showMultipleModels: false, + extensionPack: createMockExtensionPack(), + }; + const render = (props: Partial = {}) => reactRender( { methodIsUnsaved={false} modelingInProgress={false} revealedMethodSignature={null} - mode={Mode.Application} + viewState={viewState} onChange={onChange} {...props} />, diff --git a/extensions/ql-vscode/src/view/model-editor/__tests__/ModeledMethodDataGrid.spec.tsx b/extensions/ql-vscode/src/view/model-editor/__tests__/ModeledMethodDataGrid.spec.tsx index 25cb0268c..a9ae87ba6 100644 --- a/extensions/ql-vscode/src/view/model-editor/__tests__/ModeledMethodDataGrid.spec.tsx +++ b/extensions/ql-vscode/src/view/model-editor/__tests__/ModeledMethodDataGrid.spec.tsx @@ -7,6 +7,8 @@ import { ModeledMethodDataGrid, ModeledMethodDataGridProps, } from "../ModeledMethodDataGrid"; +import { ModelEditorViewState } from "../../../model-editor/shared/view-state"; +import { createMockExtensionPack } from "../../../../test/factories/model-editor/extension-pack"; describe(ModeledMethodDataGrid.name, () => { const method1 = createMethod({ @@ -41,6 +43,14 @@ describe(ModeledMethodDataGrid.name, () => { }); const onChange = jest.fn(); + const viewState: ModelEditorViewState = { + mode: Mode.Application, + showFlowGeneration: false, + showLlmButton: false, + showMultipleModels: false, + extensionPack: createMockExtensionPack(), + }; + const render = (props: Partial = {}) => reactRender( { }} modifiedSignatures={new Set([method1.signature])} inProgressMethods={new InProgressMethods()} - mode={Mode.Application} + viewState={viewState} hideModeledMethods={false} revealedMethodSignature={null} onChange={onChange} From a704cd7baed8c4179c8142c4cea3c16469613224 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 4 Oct 2023 14:27:59 +0100 Subject: [PATCH 02/24] Convert getModelingStatus to take ModeledMethod[] --- .../methods-usage/methods-usage-data-provider.ts | 5 ++++- .../ql-vscode/src/model-editor/shared/modeling-status.ts | 6 +++--- .../src/view/method-modeling/MethodModelingView.tsx | 3 ++- extensions/ql-vscode/src/view/model-editor/MethodRow.tsx | 5 ++++- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-data-provider.ts b/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-data-provider.ts index fe8ee875d..444276579 100644 --- a/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-data-provider.ts +++ b/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-data-provider.ts @@ -102,7 +102,10 @@ export class MethodsUsageDataProvider const modeledMethod = this.modeledMethods[method.signature]; const modifiedMethod = this.modifiedMethodSignatures.has(method.signature); - const status = getModelingStatus(modeledMethod, modifiedMethod); + const status = getModelingStatus( + modeledMethod ? [modeledMethod] : [], + modifiedMethod, + ); switch (status) { case "unmodeled": return new ThemeIcon("error", new ThemeColor("errorForeground")); diff --git a/extensions/ql-vscode/src/model-editor/shared/modeling-status.ts b/extensions/ql-vscode/src/model-editor/shared/modeling-status.ts index 496b4c0e7..f0737ec32 100644 --- a/extensions/ql-vscode/src/model-editor/shared/modeling-status.ts +++ b/extensions/ql-vscode/src/model-editor/shared/modeling-status.ts @@ -3,13 +3,13 @@ import { ModeledMethod } from "../modeled-method"; export type ModelingStatus = "unmodeled" | "unsaved" | "saved"; export function getModelingStatus( - modeledMethod: ModeledMethod | undefined, + modeledMethods: ModeledMethod[], methodIsUnsaved: boolean, ): ModelingStatus { - if (modeledMethod) { + if (modeledMethods.length > 0) { if (methodIsUnsaved) { return "unsaved"; - } else if (modeledMethod.type !== "none") { + } else if (modeledMethods.some((m) => m.type !== "none")) { return "saved"; } } diff --git a/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx b/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx index 390d2fde9..9125fbe6a 100644 --- a/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx +++ b/extensions/ql-vscode/src/view/method-modeling/MethodModelingView.tsx @@ -18,7 +18,8 @@ export function MethodModelingView(): JSX.Element { const [isMethodModified, setIsMethodModified] = useState(false); const modelingStatus = useMemo( - () => getModelingStatus(modeledMethod, isMethodModified), + () => + getModelingStatus(modeledMethod ? [modeledMethod] : [], isMethodModified), [modeledMethod, isMethodModified], ); diff --git a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx index 21fde2801..182366448 100644 --- a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx @@ -101,7 +101,10 @@ const ModelableMethodRow = forwardRef( [method], ); - const modelingStatus = getModelingStatus(modeledMethod, methodIsUnsaved); + const modelingStatus = getModelingStatus( + modeledMethod ? [modeledMethod] : [], + methodIsUnsaved, + ); return ( Date: Wed, 4 Oct 2023 15:41:08 +0100 Subject: [PATCH 03/24] Convert ApiOrMethodCell to ApiOrMethodRow --- .../src/view/model-editor/MethodRow.tsx | 53 ++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx index 182366448..7d7e0c66d 100644 --- a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx @@ -23,7 +23,8 @@ import { ModelInputDropdown } from "./ModelInputDropdown"; import { ModelOutputDropdown } from "./ModelOutputDropdown"; import { ModelEditorViewState } from "../../model-editor/shared/view-state"; -const ApiOrMethodCell = styled(VSCodeDataGridCell)` +const ApiOrMethodRow = styled.div` + min-height: calc(var(--input-height) * 1px); display: flex; flex-direction: row; align-items: center; @@ -112,18 +113,20 @@ const ModelableMethodRow = forwardRef( ref={ref} focused={revealedMethodSignature === method.signature} > - - - - - {viewState.mode === Mode.Application && ( - - {method.usages.length} - - )} - View - {props.modelingInProgress && } - + + + + + + {viewState.mode === Mode.Application && ( + + {method.usages.length} + + )} + View + {props.modelingInProgress && } + + {props.modelingInProgress && ( <> @@ -195,17 +198,19 @@ const UnmodelableMethodRow = forwardRef< ref={ref} focused={revealedMethodSignature === method.signature} > - - - - {viewState.mode === Mode.Application && ( - - {method.usages.length} - - )} - View - - + + + + + {viewState.mode === Mode.Application && ( + + {method.usages.length} + + )} + View + + + Method already modeled From 252c7a20c4560ef029a6090394bbd86c3544acf6 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 4 Oct 2023 15:42:13 +0100 Subject: [PATCH 04/24] Convert MethodRow to display multiple modelings --- .../model-editor/MethodRow.stories.tsx | 32 ++++-- .../src/view/model-editor/MethodRow.tsx | 98 ++++++++++++------- .../model-editor/ModeledMethodDataGrid.tsx | 35 ++++--- .../model-editor/__tests__/MethodRow.spec.tsx | 6 +- 4 files changed, 111 insertions(+), 60 deletions(-) diff --git a/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx b/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx index 9c2118914..2e15420a1 100644 --- a/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx +++ b/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx @@ -7,6 +7,9 @@ import { CallClassification, Method } from "../../model-editor/method"; import { ModeledMethod } from "../../model-editor/modeled-method"; import { VSCodeDataGrid } from "@vscode/webview-ui-toolkit/react"; import { GRID_TEMPLATE_COLUMNS } from "../../view/model-editor/ModeledMethodDataGrid"; +import { ModelEditorViewState } from "../../model-editor/shared/view-state"; +import { createMockExtensionPack } from "../../../test/factories/model-editor/extension-pack"; +import { Mode } from "../../model-editor/shared/mode"; export default { title: "CodeQL Model Editor/Method Row", @@ -66,51 +69,66 @@ const modeledMethod: ModeledMethod = { methodParameters: "()", }; +const viewState: ModelEditorViewState = { + extensionPack: createMockExtensionPack(), + showFlowGeneration: true, + showLlmButton: true, + showMultipleModels: true, + mode: Mode.Application, +}; + export const Unmodeled = Template.bind({}); Unmodeled.args = { method, - modeledMethod: undefined, + modeledMethods: [], methodCanBeModeled: true, + viewState, }; export const Source = Template.bind({}); Source.args = { method, - modeledMethod: { ...modeledMethod, type: "source" }, + modeledMethods: [{ ...modeledMethod, type: "source" }], methodCanBeModeled: true, + viewState, }; export const Sink = Template.bind({}); Sink.args = { method, - modeledMethod: { ...modeledMethod, type: "sink" }, + modeledMethods: [{ ...modeledMethod, type: "sink" }], methodCanBeModeled: true, + viewState, }; export const Summary = Template.bind({}); Summary.args = { method, - modeledMethod: { ...modeledMethod, type: "summary" }, + modeledMethods: [{ ...modeledMethod, type: "summary" }], methodCanBeModeled: true, + viewState, }; export const Neutral = Template.bind({}); Neutral.args = { method, - modeledMethod: { ...modeledMethod, type: "neutral" }, + modeledMethods: [{ ...modeledMethod, type: "neutral" }], methodCanBeModeled: true, + viewState, }; export const AlreadyModeled = Template.bind({}); AlreadyModeled.args = { method: { ...method, supported: true }, - modeledMethod: undefined, + modeledMethods: [], + viewState, }; export const ModelingInProgress = Template.bind({}); ModelingInProgress.args = { method, - modeledMethod, + modeledMethods: [modeledMethod], modelingInProgress: true, methodCanBeModeled: true, + viewState, }; diff --git a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx index 7d7e0c66d..32be92cda 100644 --- a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx @@ -23,6 +23,12 @@ import { ModelInputDropdown } from "./ModelInputDropdown"; import { ModelOutputDropdown } from "./ModelOutputDropdown"; import { ModelEditorViewState } from "../../model-editor/shared/view-state"; +const MultiModelColumn = styled(VSCodeDataGridCell)` + display: flex; + flex-direction: column; + gap: 0.5em; +`; + const ApiOrMethodRow = styled.div` min-height: calc(var(--input-height) * 1px); display: flex; @@ -57,7 +63,7 @@ const DataGridRow = styled(VSCodeDataGridRow)<{ focused?: boolean }>` export type MethodRowProps = { method: Method; methodCanBeModeled: boolean; - modeledMethod: ModeledMethod | undefined; + modeledMethods: ModeledMethod[]; methodIsUnsaved: boolean; modelingInProgress: boolean; viewState: ModelEditorViewState; @@ -90,22 +96,23 @@ const ModelableMethodRow = forwardRef( (props, ref) => { const { method, - modeledMethod, + modeledMethods: modeledMethodsArg, methodIsUnsaved, viewState, revealedMethodSignature, onChange, } = props; + const modeledMethods = viewState.showMultipleModels + ? modeledMethodsArg + : modeledMethodsArg.slice(0, 1); + const jumpToUsage = useCallback( () => sendJumpToUsageMessage(method), [method], ); - const modelingStatus = getModelingStatus( - modeledMethod ? [modeledMethod] : [], - methodIsUnsaved, - ); + const modelingStatus = getModelingStatus(modeledMethods, methodIsUnsaved); return ( ( )} {!props.modelingInProgress && ( <> - - - - - - - - - - - - + + {forEachModeledMethod(modeledMethods, (modeledMethod) => ( + + ))} + + + {forEachModeledMethod(modeledMethods, (modeledMethod) => ( + + ))} + + + {forEachModeledMethod(modeledMethods, (modeledMethod) => ( + + ))} + + + {forEachModeledMethod(modeledMethods, (modeledMethod) => ( + + ))} + )} @@ -227,3 +246,14 @@ function sendJumpToUsageMessage(method: Method) { usage: method.usages[0], }); } + +function forEachModeledMethod( + modeledMethods: ModeledMethod[], + renderer: (modeledMethod: ModeledMethod | undefined) => JSX.Element, +): JSX.Element | JSX.Element[] { + if (modeledMethods.length === 0) { + return renderer(undefined); + } else { + return modeledMethods.map(renderer); + } +} diff --git a/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx b/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx index 9ff27ad21..bf53b479c 100644 --- a/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx +++ b/extensions/ql-vscode/src/view/model-editor/ModeledMethodDataGrid.tsx @@ -84,22 +84,25 @@ export const ModeledMethodDataGrid = ({ Kind - {methodsWithModelability.map(({ method, methodCanBeModeled }) => ( - - ))} + {methodsWithModelability.map(({ method, methodCanBeModeled }) => { + const modeledMethod = modeledMethods[method.signature]; + return ( + + ); + })} )} { { it("shows the modeling status indicator when unmodeled", () => { render({ - modeledMethod: undefined, + modeledMethods: [], }); expect(screen.getByLabelText("Method not modeled")).toBeInTheDocument(); @@ -137,7 +137,7 @@ describe(MethodRow.name, () => { it("renders an unmodelable method", () => { render({ methodCanBeModeled: false, - modeledMethod: undefined, + modeledMethods: [], }); expect(screen.queryByRole("combobox")).not.toBeInTheDocument(); From 8eef4eb4f25b4c65f02c6e0794cd545c8c3466f1 Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 4 Oct 2023 15:42:35 +0100 Subject: [PATCH 05/24] Add story with multiple modelings --- .../src/stories/model-editor/MethodRow.stories.tsx | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx b/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx index 2e15420a1..9ea43028a 100644 --- a/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx +++ b/extensions/ql-vscode/src/stories/model-editor/MethodRow.stories.tsx @@ -132,3 +132,15 @@ ModelingInProgress.args = { methodCanBeModeled: true, viewState, }; + +export const MultipleModelings = Template.bind({}); +MultipleModelings.args = { + method, + modeledMethods: [ + { ...modeledMethod, type: "source" }, + { ...modeledMethod, type: "sink" }, + { ...modeledMethod }, + ], + methodCanBeModeled: true, + viewState, +}; From f87b1c46de5d6580626a152cd32ea0db2d90ddbc Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 4 Oct 2023 18:26:52 +0100 Subject: [PATCH 06/24] Add some simple tests of rendering multiple models --- .../model-editor/__tests__/MethodRow.spec.tsx | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx index 11d435021..fb1b99601 100644 --- a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx +++ b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx @@ -134,6 +134,44 @@ describe(MethodRow.name, () => { expect(screen.getByLabelText("Loading")).toBeInTheDocument(); }); + it("can render multiple models", () => { + render({ + modeledMethods: [ + { ...modeledMethod, type: "source" }, + { ...modeledMethod, type: "sink" }, + { ...modeledMethod, type: "summary" }, + ], + viewState: { + ...viewState, + showMultipleModels: true, + }, + }); + + const kindInputs = screen.getAllByRole("combobox", { name: "Model type" }); + expect(kindInputs).toHaveLength(3); + expect(kindInputs[0]).toHaveValue("source"); + expect(kindInputs[1]).toHaveValue("sink"); + expect(kindInputs[2]).toHaveValue("summary"); + }); + + it("renders only first model when showMultipleModels feature flag is disabled", () => { + render({ + modeledMethods: [ + { ...modeledMethod, type: "source" }, + { ...modeledMethod, type: "sink" }, + { ...modeledMethod, type: "summary" }, + ], + viewState: { + ...viewState, + showMultipleModels: false, + }, + }); + + const kindInputs = screen.getAllByRole("combobox", { name: "Model type" }); + expect(kindInputs.length).toBe(1); + expect(kindInputs[0]).toHaveValue("source"); + }); + it("renders an unmodelable method", () => { render({ methodCanBeModeled: false, From 5ba64a1db3553c3844c9428afa255676f5983ddc Mon Sep 17 00:00:00 2001 From: Robert Date: Wed, 4 Oct 2023 18:45:56 +0100 Subject: [PATCH 07/24] Add test for when there's no modeled method --- .../src/view/model-editor/__tests__/MethodRow.spec.tsx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx index fb1b99601..4f34ab1e7 100644 --- a/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx +++ b/extensions/ql-vscode/src/view/model-editor/__tests__/MethodRow.spec.tsx @@ -64,6 +64,14 @@ describe(MethodRow.name, () => { expect(screen.queryByLabelText("Loading")).not.toBeInTheDocument(); }); + it("renders when there is no modeled method", () => { + render({ modeledMethods: [] }); + + expect(screen.queryAllByRole("combobox")).toHaveLength(4); + expect(screen.getByLabelText("Method not modeled")).toBeInTheDocument(); + expect(screen.queryByLabelText("Loading")).not.toBeInTheDocument(); + }); + it("can change the kind", async () => { render(); From 705a7975c576afd3877948991efe198dc33b024b Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Fri, 6 Oct 2023 18:03:10 -0400 Subject: [PATCH 08/24] Add `additionalArgs` option to `launch.json` --- .../src/debugger/debug-configuration.ts | 2 ++ .../ql-vscode/src/debugger/debug-protocol.ts | 2 ++ .../ql-vscode/src/debugger/debug-session.ts | 1 + .../contextual/query-resolver.ts | 1 + .../src/local-queries/local-queries.ts | 1 + .../ql-vscode/src/local-queries/run-query.ts | 1 + .../legacy/legacy-query-runner.ts | 1 + .../src/query-server/new-query-runner.ts | 2 ++ .../src/query-server/query-runner.ts | 3 +++ .../ql-vscode/src/query-server/run-queries.ts | 3 +++ .../cli-integration/debugger/debugger.test.ts | 23 +++++++++++++++++++ 11 files changed, 40 insertions(+) diff --git a/extensions/ql-vscode/src/debugger/debug-configuration.ts b/extensions/ql-vscode/src/debugger/debug-configuration.ts index f12f13149..b67d96893 100644 --- a/extensions/ql-vscode/src/debugger/debug-configuration.ts +++ b/extensions/ql-vscode/src/debugger/debug-configuration.ts @@ -22,6 +22,7 @@ export interface QLDebugArgs { extensionPacks?: string[] | string; quickEval?: boolean; noDebug?: boolean; + additionalArgs?: Record; } /** @@ -120,6 +121,7 @@ export class QLDebugConfigurationProvider extensionPacks, quickEvalContext, noDebug: qlConfiguration.noDebug ?? false, + additionalArgs: qlConfiguration.additionalArgs ?? {}, }; return resultConfiguration; diff --git a/extensions/ql-vscode/src/debugger/debug-protocol.ts b/extensions/ql-vscode/src/debugger/debug-protocol.ts index 1cfaf6cc0..c79cd75f1 100644 --- a/extensions/ql-vscode/src/debugger/debug-protocol.ts +++ b/extensions/ql-vscode/src/debugger/debug-protocol.ts @@ -70,6 +70,8 @@ export interface LaunchConfig { quickEvalContext: QuickEvalContext | undefined; /** Run the query without debugging it. */ noDebug: boolean; + /** Undocumented: Additional arguments to be passed to the `runQuery` API on the query server. */ + additionalArgs: Record; } export interface LaunchRequest extends Request, DebugProtocol.LaunchRequest { diff --git a/extensions/ql-vscode/src/debugger/debug-session.ts b/extensions/ql-vscode/src/debugger/debug-session.ts index 0d971414b..89a585f94 100644 --- a/extensions/ql-vscode/src/debugger/debug-session.ts +++ b/extensions/ql-vscode/src/debugger/debug-session.ts @@ -161,6 +161,7 @@ class RunningQuery extends DisposableObject { true, config.additionalPacks, config.extensionPacks, + config.additionalArgs, queryStorageDir, undefined, undefined, diff --git a/extensions/ql-vscode/src/language-support/contextual/query-resolver.ts b/extensions/ql-vscode/src/language-support/contextual/query-resolver.ts index 1d08d6caa..ddb3a3d07 100644 --- a/extensions/ql-vscode/src/language-support/contextual/query-resolver.ts +++ b/extensions/ql-vscode/src/language-support/contextual/query-resolver.ts @@ -44,6 +44,7 @@ export async function runContextualQuery( false, getOnDiskWorkspaceFolders(), undefined, + {}, queryStorageDir, undefined, templates, diff --git a/extensions/ql-vscode/src/local-queries/local-queries.ts b/extensions/ql-vscode/src/local-queries/local-queries.ts index 735ff93ab..a1117d242 100644 --- a/extensions/ql-vscode/src/local-queries/local-queries.ts +++ b/extensions/ql-vscode/src/local-queries/local-queries.ts @@ -456,6 +456,7 @@ export class LocalQueries extends DisposableObject { true, additionalPacks, extensionPacks, + {}, this.queryStorageDir, undefined, templates, diff --git a/extensions/ql-vscode/src/local-queries/run-query.ts b/extensions/ql-vscode/src/local-queries/run-query.ts index 0f94dbd41..c2bdd66cd 100644 --- a/extensions/ql-vscode/src/local-queries/run-query.ts +++ b/extensions/ql-vscode/src/local-queries/run-query.ts @@ -41,6 +41,7 @@ export async function runQuery({ false, additionalPacks, extensionPacks, + {}, queryStorageDir, undefined, undefined, diff --git a/extensions/ql-vscode/src/query-server/legacy/legacy-query-runner.ts b/extensions/ql-vscode/src/query-server/legacy/legacy-query-runner.ts index e2a2b2818..d98fd6ee9 100644 --- a/extensions/ql-vscode/src/query-server/legacy/legacy-query-runner.ts +++ b/extensions/ql-vscode/src/query-server/legacy/legacy-query-runner.ts @@ -65,6 +65,7 @@ export class LegacyQueryRunner extends QueryRunner { query: CoreQueryTarget, additionalPacks: string[], extensionPacks: string[] | undefined, + _additionalRunQueryArgs: Record, // Ignored in legacy query server generateEvalLog: boolean, outputDir: QueryOutputDir, progress: ProgressCallback, diff --git a/extensions/ql-vscode/src/query-server/new-query-runner.ts b/extensions/ql-vscode/src/query-server/new-query-runner.ts index a38cbf111..82364eeba 100644 --- a/extensions/ql-vscode/src/query-server/new-query-runner.ts +++ b/extensions/ql-vscode/src/query-server/new-query-runner.ts @@ -75,6 +75,7 @@ export class NewQueryRunner extends QueryRunner { query: CoreQueryTarget, additionalPacks: string[], extensionPacks: string[] | undefined, + additionalRunQueryArgs: Record, generateEvalLog: boolean, outputDir: QueryOutputDir, progress: ProgressCallback, @@ -89,6 +90,7 @@ export class NewQueryRunner extends QueryRunner { generateEvalLog, additionalPacks, extensionPacks, + additionalRunQueryArgs, outputDir, progress, token, diff --git a/extensions/ql-vscode/src/query-server/query-runner.ts b/extensions/ql-vscode/src/query-server/query-runner.ts index bbba546be..6d5c5e896 100644 --- a/extensions/ql-vscode/src/query-server/query-runner.ts +++ b/extensions/ql-vscode/src/query-server/query-runner.ts @@ -75,6 +75,7 @@ export abstract class QueryRunner { query: CoreQueryTarget, additionalPacks: string[], extensionPacks: string[] | undefined, + additionalRunQueryArgs: Record, generateEvalLog: boolean, outputDir: QueryOutputDir, progress: ProgressCallback, @@ -107,6 +108,7 @@ export abstract class QueryRunner { generateEvalLog: boolean, additionalPacks: string[], extensionPacks: string[] | undefined, + additionalRunQueryArgs: Record, queryStorageDir: string, id = `${basename(query.queryPath)}-${nanoid()}`, templates: Record | undefined, @@ -133,6 +135,7 @@ export abstract class QueryRunner { query, additionalPacks, extensionPacks, + additionalRunQueryArgs, generateEvalLog, outputDir, progress, diff --git a/extensions/ql-vscode/src/query-server/run-queries.ts b/extensions/ql-vscode/src/query-server/run-queries.ts index 35983065d..5e6d6bdc6 100644 --- a/extensions/ql-vscode/src/query-server/run-queries.ts +++ b/extensions/ql-vscode/src/query-server/run-queries.ts @@ -27,6 +27,7 @@ export async function compileAndRunQueryAgainstDatabaseCore( generateEvalLog: boolean, additionalPacks: string[], extensionPacks: string[] | undefined, + additionalRunQueryArgs: Record, outputDir: QueryOutputDir, progress: ProgressCallback, token: CancellationToken, @@ -55,6 +56,8 @@ export async function compileAndRunQueryAgainstDatabaseCore( logPath: evalLogPath, target, extensionPacks, + // Add any additional arguments without interpretation. + ...additionalRunQueryArgs, }; // Update the active query logger every time there is a new request to compile. diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/debugger/debugger.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/debugger/debugger.test.ts index 17943b807..f27f6000b 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/debugger/debugger.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/debugger/debugger.test.ts @@ -177,4 +177,27 @@ describeWithCodeQL()("Debugger", () => { expect(editor.document.isDirty).toBe(false); }); }); + + it("should pass additionalArgs through to query server", async () => { + await withDebugController(appCommands, async (controller) => { + await controller.startDebugging( + { + query: quickEvalQueryPath, + additionalArgs: { + // Overrides the value passed to the query server + queryPath: simpleQueryPath, + }, + }, + true, + ); + await controller.expectLaunched(); + const result = await controller.expectSucceeded(); + await controller.expectExited(); + await controller.expectTerminated(); + await controller.expectSessionClosed(); + + // Expect the number of results to be the same as if we had run the simple query, not the quick eval query. + expect(await getResultCount(result.results.outputDir, cli)).toBe(2); + }); + }); }); From 9e26c29ddb38c1adde05f24725378e62f8599456 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Fri, 6 Oct 2023 18:35:20 -0400 Subject: [PATCH 09/24] Fix tests for older CLIs --- .../vscode-tests/cli-integration/debugger/debugger.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/debugger/debugger.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/debugger/debugger.test.ts index f27f6000b..bc4bfd773 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/debugger/debugger.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/debugger/debugger.test.ts @@ -179,6 +179,10 @@ describeWithCodeQL()("Debugger", () => { }); it("should pass additionalArgs through to query server", async () => { + if (!(await cli.cliConstraints.supportsNewQueryServerForTests())) { + // Only works with the new query server. + return; + } await withDebugController(appCommands, async (controller) => { await controller.startDebugging( { From b76369330dc5075d466c20edcc615bc2eb85b9c0 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Wed, 4 Oct 2023 14:22:57 +0200 Subject: [PATCH 10/24] Convert remaining extension host code to handle multiple models This converts all remaining extension host code to handle multiple models per method. The only place where we're using the legacy format is in the webview and in the boundary between the webview and the extension host. --- .../ql-vscode/src/model-editor/auto-model.ts | 11 ++-- .../src/model-editor/auto-modeler.ts | 39 +++++++------- .../method-modeling-view-provider.ts | 21 +++++--- .../methods-usage-data-provider.ts | 13 +++-- .../methods-usage/methods-usage-panel.ts | 2 +- .../src/model-editor/model-editor-view.ts | 39 +++++++++----- .../model-editor/modeled-methods-legacy.ts | 52 ++++++++++++++++++- .../src/model-editor/modeling-store.ts | 27 ++++++---- .../model-editor/shared/modeling-status.ts | 21 ++++++++ .../model-editor/auto-model.test.ts | 28 +++++----- .../methods-usage-data-provider.test.ts | 6 +-- .../methods-usage/methods-usage-panel.test.ts | 4 +- 12 files changed, 179 insertions(+), 84 deletions(-) diff --git a/extensions/ql-vscode/src/model-editor/auto-model.ts b/extensions/ql-vscode/src/model-editor/auto-model.ts index 0fcf1c155..e529e0b84 100644 --- a/extensions/ql-vscode/src/model-editor/auto-model.ts +++ b/extensions/ql-vscode/src/model-editor/auto-model.ts @@ -14,13 +14,13 @@ import { groupMethods, sortGroupNames, sortMethods } from "./shared/sorting"; * the order in the UI. * @param mode Whether it is application or framework mode. * @param methods all methods. - * @param modeledMethods the currently modeled methods. + * @param modeledMethodsBySignature the currently modeled methods. * @returns list of modeled methods that are candidates for modeling. */ export function getCandidates( mode: Mode, methods: Method[], - modeledMethods: Record, + modeledMethodsBySignature: Record, ): MethodSignature[] { // Sort the same way as the UI so we send the first ones listed in the UI first const grouped = groupMethods(methods, mode); @@ -32,12 +32,11 @@ export function getCandidates( const candidates: MethodSignature[] = []; for (const method of sortedMethods) { - const modeledMethod: ModeledMethod = modeledMethods[method.signature] ?? { - type: "none", - }; + const modeledMethods: ModeledMethod[] = + modeledMethodsBySignature[method.signature] ?? []; // Anything that is modeled is not a candidate - if (modeledMethod.type !== "none") { + if (modeledMethods.some((m) => m.type !== "none")) { continue; } diff --git a/extensions/ql-vscode/src/model-editor/auto-modeler.ts b/extensions/ql-vscode/src/model-editor/auto-modeler.ts index f691aeeb8..6308f442b 100644 --- a/extensions/ql-vscode/src/model-editor/auto-modeler.ts +++ b/extensions/ql-vscode/src/model-editor/auto-modeler.ts @@ -16,7 +16,6 @@ import { QueryRunner } from "../query-server"; import { DatabaseItem } from "../databases/local-databases"; import { Mode } from "./shared/mode"; import { CancellationTokenSource } from "vscode"; -import { convertToLegacyModeledMethods } from "./modeled-methods-legacy"; // Limit the number of candidates we send to the model in each request // to avoid long requests. @@ -43,7 +42,7 @@ export class AutoModeler { inProgressMethods: string[], ) => Promise, private readonly addModeledMethods: ( - modeledMethods: Record, + modeledMethods: Record, ) => Promise, ) { this.jobs = new Map(); @@ -60,7 +59,7 @@ export class AutoModeler { public async startModeling( packageName: string, methods: Method[], - modeledMethods: Record, + modeledMethods: Record, mode: Mode, ): Promise { if (this.jobs.has(packageName)) { @@ -107,7 +106,7 @@ export class AutoModeler { private async modelPackage( packageName: string, methods: Method[], - modeledMethods: Record, + modeledMethods: Record, mode: Mode, cancellationTokenSource: CancellationTokenSource, ): Promise { @@ -193,31 +192,31 @@ export class AutoModeler { filename: "auto-model.yml", }); - const rawLoadedMethods = loadDataExtensionYaml(models); - if (!rawLoadedMethods) { + const loadedMethods = loadDataExtensionYaml(models); + if (!loadedMethods) { return; } - const loadedMethods = convertToLegacyModeledMethods(rawLoadedMethods); - // Any candidate that was part of the response is a negative result // meaning that the canidate is not a sink for the kinds that the LLM is checking for. // For now we model this as a sink neutral method, however this is subject // to discussion. for (const candidate of candidateMethods) { if (!(candidate.signature in loadedMethods)) { - loadedMethods[candidate.signature] = { - type: "neutral", - kind: "sink", - input: "", - output: "", - provenance: "ai-generated", - signature: candidate.signature, - packageName: candidate.packageName, - typeName: candidate.typeName, - methodName: candidate.methodName, - methodParameters: candidate.methodParameters, - }; + loadedMethods[candidate.signature] = [ + { + type: "neutral", + kind: "sink", + input: "", + output: "", + provenance: "ai-generated", + signature: candidate.signature, + packageName: candidate.packageName, + typeName: candidate.typeName, + methodName: candidate.methodName, + methodParameters: candidate.methodParameters, + }, + ]; } } diff --git a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts index e80c5528a..0be5d98ce 100644 --- a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts +++ b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts @@ -13,6 +13,10 @@ import { AbstractWebviewViewProvider } from "../../common/vscode/abstract-webvie import { assertNever } from "../../common/helpers-pure"; import { ModelEditorViewTracker } from "../model-editor-view-tracker"; import { showMultipleModels } from "../../config"; +import { + convertFromLegacyModeledMethod, + convertToLegacyModeledMethod, +} from "../modeled-methods-legacy"; export class MethodModelingViewProvider extends AbstractWebviewViewProvider< ToMethodModelingMessage, @@ -62,7 +66,9 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< await this.postMessage({ t: "setSelectedMethod", method: selectedMethod.method, - modeledMethod: selectedMethod.modeledMethod, + modeledMethod: convertToLegacyModeledMethod( + selectedMethod.modeledMethods, + ), isModified: selectedMethod.isModified, }); } @@ -94,9 +100,10 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< case "setModeledMethod": { const activeState = this.ensureActiveState(); - this.modelingStore.updateModeledMethod( + this.modelingStore.updateModeledMethods( activeState.databaseItem, - msg.method, + msg.method.signature, + convertFromLegacyModeledMethod(msg.method), ); break; } @@ -141,11 +148,11 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< this.push( this.modelingStore.onModeledMethodsChanged(async (e) => { if (this.webviewView && e.isActiveDb) { - const modeledMethod = e.modeledMethods[this.method?.signature ?? ""]; - if (modeledMethod) { + const modeledMethods = e.modeledMethods[this.method?.signature ?? ""]; + if (modeledMethods) { await this.postMessage({ t: "setModeledMethod", - method: modeledMethod, + method: convertToLegacyModeledMethod(modeledMethods), }); } } @@ -171,7 +178,7 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< await this.postMessage({ t: "setSelectedMethod", method: e.method, - modeledMethod: e.modeledMethod, + modeledMethod: convertToLegacyModeledMethod(e.modeledMethods), isModified: e.isModified, }); } diff --git a/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-data-provider.ts b/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-data-provider.ts index fe8ee875d..046e38fcc 100644 --- a/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-data-provider.ts +++ b/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-data-provider.ts @@ -14,7 +14,7 @@ import { DatabaseItem } from "../../databases/local-databases"; import { relative } from "path"; import { CodeQLCliServer } from "../../codeql-cli/cli"; import { INITIAL_HIDE_MODELED_METHODS_VALUE } from "../shared/hide-modeled-methods"; -import { getModelingStatus } from "../shared/modeling-status"; +import { getModelingStatusForModeledMethods } from "../shared/modeling-status"; import { assertNever } from "../../common/helpers-pure"; import { ModeledMethod } from "../modeled-method"; @@ -26,7 +26,7 @@ export class MethodsUsageDataProvider private databaseItem: DatabaseItem | undefined = undefined; private sourceLocationPrefix: string | undefined = undefined; private hideModeledMethods: boolean = INITIAL_HIDE_MODELED_METHODS_VALUE; - private modeledMethods: Record = {}; + private modeledMethods: Record = {}; private modifiedMethodSignatures: Set = new Set(); private readonly onDidChangeTreeDataEmitter = this.push( @@ -52,7 +52,7 @@ export class MethodsUsageDataProvider methods: Method[], databaseItem: DatabaseItem, hideModeledMethods: boolean, - modeledMethods: Record, + modeledMethods: Record, modifiedMethodSignatures: Set, ): Promise { if ( @@ -99,10 +99,13 @@ export class MethodsUsageDataProvider } private getModelingStatusIcon(method: Method): ThemeIcon { - const modeledMethod = this.modeledMethods[method.signature]; + const modeledMethods = this.modeledMethods[method.signature]; const modifiedMethod = this.modifiedMethodSignatures.has(method.signature); - const status = getModelingStatus(modeledMethod, modifiedMethod); + const status = getModelingStatusForModeledMethods( + modeledMethods, + modifiedMethod, + ); switch (status) { case "unmodeled": return new ThemeIcon("error", new ThemeColor("errorForeground")); diff --git a/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-panel.ts b/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-panel.ts index 9c0c2fb58..cbc80ac73 100644 --- a/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-panel.ts +++ b/extensions/ql-vscode/src/model-editor/methods-usage/methods-usage-panel.ts @@ -34,7 +34,7 @@ export class MethodsUsagePanel extends DisposableObject { methods: Method[], databaseItem: DatabaseItem, hideModeledMethods: boolean, - modeledMethods: Record, + modeledMethods: Record, modifiedMethodSignatures: Set, ): Promise { await this.dataProvider.setState( diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view.ts b/extensions/ql-vscode/src/model-editor/model-editor-view.ts index fb931a99c..ba0ca2d5c 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts @@ -48,6 +48,7 @@ import { telemetryListener } from "../common/vscode/telemetry"; import { ModelingStore } from "./modeling-store"; import { ModelEditorViewTracker } from "./model-editor-view-tracker"; import { + convertFromLegacyModeledMethod, convertFromLegacyModeledMethods, convertToLegacyModeledMethods, } from "./modeled-methods-legacy"; @@ -259,7 +260,7 @@ export class ModelEditorView extends AbstractWebview< await this.generateModeledMethodsFromLlm( msg.packageName, msg.methods, - msg.modeledMethods, + convertFromLegacyModeledMethods(msg.modeledMethods), ); void telemetryListener?.sendUIInteraction( "model-editor-generate-methods-from-llm", @@ -303,7 +304,10 @@ export class ModelEditorView extends AbstractWebview< ); break; case "setModeledMethod": { - this.setModeledMethod(msg.method); + this.setModeledMethods( + msg.method.signature, + convertFromLegacyModeledMethod(msg.method), + ); break; } default: @@ -363,10 +367,7 @@ export class ModelEditorView extends AbstractWebview< this.cliServer, this.app.logger, ); - this.modelingStore.setModeledMethods( - this.databaseItem, - convertToLegacyModeledMethods(modeledMethods), - ); + this.modelingStore.setModeledMethods(this.databaseItem, modeledMethods); } catch (e: unknown) { void showAndLogErrorMessage( this.app.logger, @@ -438,10 +439,16 @@ export class ModelEditorView extends AbstractWebview< queryStorageDir: this.queryStorageDir, databaseItem: addedDatabase ?? this.databaseItem, onResults: async (modeledMethods) => { - const modeledMethodsByName: Record = {}; + const modeledMethodsByName: Record = {}; for (const modeledMethod of modeledMethods) { - modeledMethodsByName[modeledMethod.signature] = modeledMethod; + if (!(modeledMethod.signature in modeledMethodsByName)) { + modeledMethodsByName[modeledMethod.signature] = []; + } + + modeledMethodsByName[modeledMethod.signature].push( + modeledMethod, + ); } this.addModeledMethods(modeledMethodsByName); @@ -466,7 +473,7 @@ export class ModelEditorView extends AbstractWebview< private async generateModeledMethodsFromLlm( packageName: string, methods: Method[], - modeledMethods: Record, + modeledMethods: Record, ): Promise { await this.autoModeler.startModeling( packageName, @@ -603,7 +610,7 @@ export class ModelEditorView extends AbstractWebview< if (event.dbUri === this.databaseItem.databaseUri.toString()) { await this.postMessage({ t: "setModeledMethods", - methods: event.modeledMethods, + methods: convertToLegacyModeledMethods(event.modeledMethods), }); } }), @@ -621,7 +628,7 @@ export class ModelEditorView extends AbstractWebview< ); } - private addModeledMethods(modeledMethods: Record) { + private addModeledMethods(modeledMethods: Record) { this.modelingStore.addModeledMethods(this.databaseItem, modeledMethods); this.modelingStore.addModifiedMethods( @@ -630,13 +637,17 @@ export class ModelEditorView extends AbstractWebview< ); } - private setModeledMethod(method: ModeledMethod) { + private setModeledMethods(signature: string, methods: ModeledMethod[]) { const state = this.modelingStore.getStateForActiveDb(); if (!state) { throw new Error("Attempting to set modeled method without active db"); } - this.modelingStore.updateModeledMethod(state.databaseItem, method); - this.modelingStore.addModifiedMethod(state.databaseItem, method.signature); + this.modelingStore.updateModeledMethods( + state.databaseItem, + signature, + methods, + ); + this.modelingStore.addModifiedMethod(state.databaseItem, signature); } } diff --git a/extensions/ql-vscode/src/model-editor/modeled-methods-legacy.ts b/extensions/ql-vscode/src/model-editor/modeled-methods-legacy.ts index 2e6f3af1c..3ca32c7c2 100644 --- a/extensions/ql-vscode/src/model-editor/modeled-methods-legacy.ts +++ b/extensions/ql-vscode/src/model-editor/modeled-methods-legacy.ts @@ -1,23 +1,71 @@ import { ModeledMethod } from "./modeled-method"; +/** + * Converts a record of ModeledMethod[] indexed by signature to a record of a single ModeledMethod indexed by signature + * for legacy usage. This function should always be used instead of the trivial conversion to track usages of this + * conversion. + * + * This method should only be called inside a `onMessage` function (or its equivalent). If it's used anywhere else, + * consider whether the boundary is correct: the boundary should as close as possible to the webview -> extension host + * boundary. + * + * @param modeledMethods The record of ModeledMethod[] indexed by signature + */ export function convertFromLegacyModeledMethods( modeledMethods: Record, ): Record { // Convert a single ModeledMethod to an array of ModeledMethods return Object.fromEntries( Object.entries(modeledMethods).map(([signature, modeledMethod]) => { - return [signature, [modeledMethod]]; + return [signature, convertFromLegacyModeledMethod(modeledMethod)]; }), ); } +/** + * Converts a record of a single ModeledMethod indexed by signature to a record of ModeledMethod[] indexed by signature + * for legacy usage. This function should always be used instead of the trivial conversion to track usages of this + * conversion. + * + * This method should only be called inside a `postMessage` call. If it's used anywhere else, consider whether the + * boundary is correct: the boundary should as close as possible to the extension host -> webview boundary. + * + * @param modeledMethods The record of a single ModeledMethod indexed by signature + */ export function convertToLegacyModeledMethods( modeledMethods: Record, ): Record { // Always take the first modeled method in the array return Object.fromEntries( Object.entries(modeledMethods).map(([signature, modeledMethods]) => { - return [signature, modeledMethods[0]]; + return [signature, convertToLegacyModeledMethod(modeledMethods)]; }), ); } + +/** + * Converts a single ModeledMethod to a ModeledMethod[] for legacy usage. This function should always be used instead + * of the trivial conversion to track usages of this conversion. + * + * This method should only be called inside a `onMessage` function (or its equivalent). If it's used anywhere else, + * consider whether the boundary is correct: the boundary should as close as possible to the webview -> extension host + * boundary. + * + * @param modeledMethod The single ModeledMethod + */ +export function convertFromLegacyModeledMethod(modeledMethod: ModeledMethod) { + return [modeledMethod]; +} + +/** + * Converts a ModeledMethod[] to a single ModeledMethod for legacy usage. This function should always be used instead + * of the trivial conversion to track usages of this conversion. + * + * This method should only be called inside a `postMessage` call. If it's used anywhere else, consider whether the + * boundary is correct: the boundary should as close as possible to the extension host -> webview boundary. + * + * @param modeledMethods The ModeledMethod[] + */ +export function convertToLegacyModeledMethod(modeledMethods: ModeledMethod[]) { + return modeledMethods[0]; +} diff --git a/extensions/ql-vscode/src/model-editor/modeling-store.ts b/extensions/ql-vscode/src/model-editor/modeling-store.ts index c6e27986d..dca3477ee 100644 --- a/extensions/ql-vscode/src/model-editor/modeling-store.ts +++ b/extensions/ql-vscode/src/model-editor/modeling-store.ts @@ -10,7 +10,7 @@ export interface DbModelingState { databaseItem: DatabaseItem; methods: Method[]; hideModeledMethods: boolean; - modeledMethods: Record; + modeledMethods: Record; modifiedMethodSignatures: Set; selectedMethod: Method | undefined; selectedUsage: Usage | undefined; @@ -28,7 +28,7 @@ interface HideModeledMethodsChangedEvent { } interface ModeledMethodsChangedEvent { - modeledMethods: Record; + modeledMethods: Record; dbUri: string; isActiveDb: boolean; } @@ -43,7 +43,7 @@ interface SelectedMethodChangedEvent { databaseItem: DatabaseItem; method: Method; usage: Usage; - modeledMethod: ModeledMethod | undefined; + modeledMethods: ModeledMethod[]; isModified: boolean; } @@ -199,14 +199,15 @@ export class ModelingStore extends DisposableObject { public addModeledMethods( dbItem: DatabaseItem, - methods: Record, + methods: Record, ) { this.changeModeledMethods(dbItem, (state) => { const newModeledMethods = { ...methods, + // Keep all methods that are already modeled in some form in the state ...Object.fromEntries( - Object.entries(state.modeledMethods).filter( - ([_, value]) => value.type !== "none", + Object.entries(state.modeledMethods).filter(([_, value]) => + value.some((m) => m.type !== "none"), ), ), }; @@ -216,17 +217,21 @@ export class ModelingStore extends DisposableObject { public setModeledMethods( dbItem: DatabaseItem, - methods: Record, + methods: Record, ) { this.changeModeledMethods(dbItem, (state) => { state.modeledMethods = { ...methods }; }); } - public updateModeledMethod(dbItem: DatabaseItem, method: ModeledMethod) { + public updateModeledMethods( + dbItem: DatabaseItem, + signature: string, + modeledMethods: ModeledMethod[], + ) { this.changeModeledMethods(dbItem, (state) => { const newModeledMethods = { ...state.modeledMethods }; - newModeledMethods[method.signature] = method; + newModeledMethods[signature] = modeledMethods; state.modeledMethods = newModeledMethods; }); } @@ -280,7 +285,7 @@ export class ModelingStore extends DisposableObject { databaseItem: dbItem, method, usage, - modeledMethod: dbState.modeledMethods[method.signature], + modeledMethods: dbState.modeledMethods[method.signature], isModified: dbState.modifiedMethodSignatures.has(method.signature), }); } @@ -299,7 +304,7 @@ export class ModelingStore extends DisposableObject { return { method: selectedMethod, usage: dbState.selectedUsage, - modeledMethod: dbState.modeledMethods[selectedMethod.signature], + modeledMethods: dbState.modeledMethods[selectedMethod.signature], isModified: dbState.modifiedMethodSignatures.has( selectedMethod.signature, ), diff --git a/extensions/ql-vscode/src/model-editor/shared/modeling-status.ts b/extensions/ql-vscode/src/model-editor/shared/modeling-status.ts index 496b4c0e7..f1cc8792e 100644 --- a/extensions/ql-vscode/src/model-editor/shared/modeling-status.ts +++ b/extensions/ql-vscode/src/model-editor/shared/modeling-status.ts @@ -15,3 +15,24 @@ export function getModelingStatus( } return "unmodeled"; } + +export function getModelingStatusForModeledMethods( + modeledMethods: ModeledMethod[], + methodIsUnsaved: boolean, +): ModelingStatus { + if (modeledMethods.length === 0) { + return "unmodeled"; + } + + if (methodIsUnsaved) { + return "unsaved"; + } + + for (const modeledMethod of modeledMethods) { + if (modeledMethod.type !== "none") { + return "saved"; + } + } + + return "unmodeled"; +} diff --git a/extensions/ql-vscode/test/unit-tests/model-editor/auto-model.test.ts b/extensions/ql-vscode/test/unit-tests/model-editor/auto-model.test.ts index ded1adc12..7389ff8f6 100644 --- a/extensions/ql-vscode/test/unit-tests/model-editor/auto-model.test.ts +++ b/extensions/ql-vscode/test/unit-tests/model-editor/auto-model.test.ts @@ -99,19 +99,21 @@ describe("getCandidates", () => { usages: [], }, ]; - const modeledMethods: Record = { - "org.my.A#x()": { - type: "neutral", - kind: "", - input: "", - output: "", - provenance: "manual", - signature: "org.my.A#x()", - packageName: "org.my", - typeName: "A", - methodName: "x", - methodParameters: "()", - }, + const modeledMethods: Record = { + "org.my.A#x()": [ + { + type: "neutral", + kind: "", + input: "", + output: "", + provenance: "manual", + signature: "org.my.A#x()", + packageName: "org.my", + typeName: "A", + methodName: "x", + methodParameters: "()", + }, + ], }; const candidates = getCandidates(Mode.Application, methods, modeledMethods); expect(candidates.length).toEqual(0); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/methods-usage/methods-usage-data-provider.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/methods-usage/methods-usage-data-provider.test.ts index 4092ad492..f8b7c77fc 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/methods-usage/methods-usage-data-provider.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/methods-usage/methods-usage-data-provider.test.ts @@ -20,7 +20,7 @@ describe("MethodsUsageDataProvider", () => { describe("setState", () => { const hideModeledMethods = false; const methods: Method[] = []; - const modeledMethods: Record = {}; + const modeledMethods: Record = {}; const modifiedMethodSignatures: Set = new Set(); const dbItem = mockedObject({ getSourceLocationPrefix: () => "test", @@ -125,7 +125,7 @@ describe("MethodsUsageDataProvider", () => { }); it("should emit onDidChangeTreeData event when modeled methods has changed", async () => { - const modeledMethods2: Record = {}; + const modeledMethods2: Record = {}; await dataProvider.setState( methods, @@ -213,7 +213,7 @@ describe("MethodsUsageDataProvider", () => { }); const methods: Method[] = [supportedMethod, unsupportedMethod]; - const modeledMethods: Record = {}; + const modeledMethods: Record = {}; const modifiedMethodSignatures: Set = new Set(); const dbItem = mockedObject({ diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/methods-usage/methods-usage-panel.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/methods-usage/methods-usage-panel.test.ts index 6a51877c4..30456a04d 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/methods-usage/methods-usage-panel.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/methods-usage/methods-usage-panel.test.ts @@ -21,7 +21,7 @@ describe("MethodsUsagePanel", () => { describe("setState", () => { const hideModeledMethods = false; const methods: Method[] = [createMethod()]; - const modeledMethods: Record = {}; + const modeledMethods: Record = {}; const modifiedMethodSignatures: Set = new Set(); it("should update the tree view with the correct batch number", async () => { @@ -50,7 +50,7 @@ describe("MethodsUsagePanel", () => { let modelingStore: ModelingStore; const hideModeledMethods: boolean = false; - const modeledMethods: Record = {}; + const modeledMethods: Record = {}; const modifiedMethodSignatures: Set = new Set(); const usage = createUsage(); From 8b0825ab3c98dfc15c5b2cfe4c8c7784989de5ec Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 9 Oct 2023 14:32:56 +0100 Subject: [PATCH 11/24] Use better name for variable --- extensions/ql-vscode/src/view/model-editor/MethodRow.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx index 32be92cda..710367f1f 100644 --- a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx @@ -96,7 +96,7 @@ const ModelableMethodRow = forwardRef( (props, ref) => { const { method, - modeledMethods: modeledMethodsArg, + modeledMethods: modeledMethodsProp, methodIsUnsaved, viewState, revealedMethodSignature, @@ -104,8 +104,8 @@ const ModelableMethodRow = forwardRef( } = props; const modeledMethods = viewState.showMultipleModels - ? modeledMethodsArg - : modeledMethodsArg.slice(0, 1); + ? modeledMethodsProp + : modeledMethodsProp.slice(0, 1); const jumpToUsage = useCallback( () => sendJumpToUsageMessage(method), From 0265353370e08d8628287e7db2b8150cee306464 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 9 Oct 2023 14:44:43 +0100 Subject: [PATCH 12/24] Use index as react key --- .../src/view/model-editor/MethodRow.tsx | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx index 710367f1f..dc8581c7d 100644 --- a/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/MethodRow.tsx @@ -153,9 +153,9 @@ const ModelableMethodRow = forwardRef( {!props.modelingInProgress && ( <> - {forEachModeledMethod(modeledMethods, (modeledMethod) => ( + {forEachModeledMethod(modeledMethods, (modeledMethod, index) => ( ( ))} - {forEachModeledMethod(modeledMethods, (modeledMethod) => ( + {forEachModeledMethod(modeledMethods, (modeledMethod, index) => ( ( ))} - {forEachModeledMethod(modeledMethods, (modeledMethod) => ( + {forEachModeledMethod(modeledMethods, (modeledMethod, index) => ( ( ))} - {forEachModeledMethod(modeledMethods, (modeledMethod) => ( + {forEachModeledMethod(modeledMethods, (modeledMethod, index) => ( JSX.Element, + renderer: ( + modeledMethod: ModeledMethod | undefined, + index: number, + ) => JSX.Element, ): JSX.Element | JSX.Element[] { if (modeledMethods.length === 0) { - return renderer(undefined); + return renderer(undefined, 0); } else { return modeledMethods.map(renderer); } From d4cbfbb70eb5b6ddf037351b1ba4a8fdb381dc83 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Mon, 9 Oct 2023 11:04:22 -0400 Subject: [PATCH 13/24] Rename to `additionalRunQueryArgs` --- extensions/ql-vscode/package.json | 4 ++++ extensions/ql-vscode/src/debugger/debug-configuration.ts | 4 ++-- extensions/ql-vscode/src/debugger/debug-protocol.ts | 2 +- extensions/ql-vscode/src/debugger/debug-session.ts | 2 +- .../vscode-tests/cli-integration/debugger/debugger.test.ts | 2 +- 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index abe9d4355..18e3c3746 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -110,6 +110,10 @@ "string" ], "description": "Names of extension packs to include in the evaluation. These are resolved from the locations specified in `additionalPacks`." + }, + "additionalRunQueryArgs": { + "type": "object", + "description": "**Internal use only**. Additional arguments to pass to the `runQuery` command of the query server, without validation." } } } diff --git a/extensions/ql-vscode/src/debugger/debug-configuration.ts b/extensions/ql-vscode/src/debugger/debug-configuration.ts index b67d96893..195eadc2d 100644 --- a/extensions/ql-vscode/src/debugger/debug-configuration.ts +++ b/extensions/ql-vscode/src/debugger/debug-configuration.ts @@ -22,7 +22,7 @@ export interface QLDebugArgs { extensionPacks?: string[] | string; quickEval?: boolean; noDebug?: boolean; - additionalArgs?: Record; + additionalRunQueryArgs?: Record; } /** @@ -121,7 +121,7 @@ export class QLDebugConfigurationProvider extensionPacks, quickEvalContext, noDebug: qlConfiguration.noDebug ?? false, - additionalArgs: qlConfiguration.additionalArgs ?? {}, + additionalRunQueryArgs: qlConfiguration.additionalRunQueryArgs ?? {}, }; return resultConfiguration; diff --git a/extensions/ql-vscode/src/debugger/debug-protocol.ts b/extensions/ql-vscode/src/debugger/debug-protocol.ts index c79cd75f1..959490edc 100644 --- a/extensions/ql-vscode/src/debugger/debug-protocol.ts +++ b/extensions/ql-vscode/src/debugger/debug-protocol.ts @@ -71,7 +71,7 @@ export interface LaunchConfig { /** Run the query without debugging it. */ noDebug: boolean; /** Undocumented: Additional arguments to be passed to the `runQuery` API on the query server. */ - additionalArgs: Record; + additionalRunQueryArgs: Record; } export interface LaunchRequest extends Request, DebugProtocol.LaunchRequest { diff --git a/extensions/ql-vscode/src/debugger/debug-session.ts b/extensions/ql-vscode/src/debugger/debug-session.ts index 89a585f94..5e3a0f8ee 100644 --- a/extensions/ql-vscode/src/debugger/debug-session.ts +++ b/extensions/ql-vscode/src/debugger/debug-session.ts @@ -161,7 +161,7 @@ class RunningQuery extends DisposableObject { true, config.additionalPacks, config.extensionPacks, - config.additionalArgs, + config.additionalRunQueryArgs, queryStorageDir, undefined, undefined, diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/debugger/debugger.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/debugger/debugger.test.ts index bc4bfd773..d525469e5 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/debugger/debugger.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/debugger/debugger.test.ts @@ -187,7 +187,7 @@ describeWithCodeQL()("Debugger", () => { await controller.startDebugging( { query: quickEvalQueryPath, - additionalArgs: { + additionalRunQueryArgs: { // Overrides the value passed to the query server queryPath: simpleQueryPath, }, From feebf7c3fd7238efd65207681b4fb00bfede2b75 Mon Sep 17 00:00:00 2001 From: Robert Date: Mon, 9 Oct 2023 16:17:46 +0100 Subject: [PATCH 14/24] Only include method signatures in generateMethodsFromLlm --- extensions/ql-vscode/src/common/interface-types.ts | 3 +-- .../src/model-editor/model-editor-view.ts | 14 ++++++++++---- .../ql-vscode/src/view/model-editor/LibraryRow.tsx | 10 ++++++---- .../src/view/model-editor/ModelEditor.tsx | 9 ++------- .../src/view/model-editor/ModeledMethodsList.tsx | 3 +-- 5 files changed, 20 insertions(+), 19 deletions(-) diff --git a/extensions/ql-vscode/src/common/interface-types.ts b/extensions/ql-vscode/src/common/interface-types.ts index 58c96d3b2..58b334d83 100644 --- a/extensions/ql-vscode/src/common/interface-types.ts +++ b/extensions/ql-vscode/src/common/interface-types.ts @@ -556,8 +556,7 @@ interface GenerateMethodMessage { interface GenerateMethodsFromLlmMessage { t: "generateMethodsFromLlm"; packageName: string; - methods: Method[]; - modeledMethods: Record; + methodSignatures: string[]; } interface StopGeneratingMethodsFromLlmMessage { diff --git a/extensions/ql-vscode/src/model-editor/model-editor-view.ts b/extensions/ql-vscode/src/model-editor/model-editor-view.ts index a768168e6..8a34b0a6c 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts @@ -267,8 +267,7 @@ export class ModelEditorView extends AbstractWebview< case "generateMethodsFromLlm": await this.generateModeledMethodsFromLlm( msg.packageName, - msg.methods, - msg.modeledMethods, + msg.methodSignatures, ); void telemetryListener?.sendUIInteraction( "model-editor-generate-methods-from-llm", @@ -474,9 +473,16 @@ export class ModelEditorView extends AbstractWebview< private async generateModeledMethodsFromLlm( packageName: string, - methods: Method[], - modeledMethods: Record, + methodSignatures: string[], ): Promise { + const methods = this.modelingStore.getMethods( + this.databaseItem, + methodSignatures, + ); + const modeledMethods = this.modelingStore.getModeledMethods( + this.databaseItem, + methodSignatures, + ); await this.autoModeler.startModeling( packageName, methods, diff --git a/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx b/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx index b5b9d275c..324597a59 100644 --- a/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx +++ b/extensions/ql-vscode/src/view/model-editor/LibraryRow.tsx @@ -81,8 +81,7 @@ export type LibraryRowProps = { onSaveModelClick: (methodSignatures: string[]) => void; onGenerateFromLlmClick: ( dependencyName: string, - methods: Method[], - modeledMethods: Record, + methodSignatures: string[], ) => void; onStopGenerateFromLlmClick: (dependencyName: string) => void; onGenerateFromSourceClick: () => void; @@ -126,11 +125,14 @@ export const LibraryRow = ({ const handleModelWithAI = useCallback( async (e: React.MouseEvent) => { - onGenerateFromLlmClick(title, methods, modeledMethods); + onGenerateFromLlmClick( + title, + methods.map((m) => m.signature), + ); e.stopPropagation(); e.preventDefault(); }, - [title, methods, modeledMethods, onGenerateFromLlmClick], + [title, methods, onGenerateFromLlmClick], ); const handleStopModelWithAI = useCallback( diff --git a/extensions/ql-vscode/src/view/model-editor/ModelEditor.tsx b/extensions/ql-vscode/src/view/model-editor/ModelEditor.tsx index 19dbf53ea..0591b13c4 100644 --- a/extensions/ql-vscode/src/view/model-editor/ModelEditor.tsx +++ b/extensions/ql-vscode/src/view/model-editor/ModelEditor.tsx @@ -219,16 +219,11 @@ export function ModelEditor({ }, []); const onGenerateFromLlmClick = useCallback( - ( - packageName: string, - methods: Method[], - modeledMethods: Record, - ) => { + (packageName: string, methodSignatures: string[]) => { vscode.postMessage({ t: "generateMethodsFromLlm", packageName, - methods, - modeledMethods, + methodSignatures, }); }, [], diff --git a/extensions/ql-vscode/src/view/model-editor/ModeledMethodsList.tsx b/extensions/ql-vscode/src/view/model-editor/ModeledMethodsList.tsx index d59657ecc..6fd5b8820 100644 --- a/extensions/ql-vscode/src/view/model-editor/ModeledMethodsList.tsx +++ b/extensions/ql-vscode/src/view/model-editor/ModeledMethodsList.tsx @@ -23,8 +23,7 @@ export type ModeledMethodsListProps = { onSaveModelClick: (methodSignatures: string[]) => void; onGenerateFromLlmClick: ( packageName: string, - methods: Method[], - modeledMethods: Record, + methodSignatures: string[], ) => void; onStopGenerateFromLlmClick: (packageName: string) => void; onGenerateFromSourceClick: () => void; From 64df792eda9121ed9f9343a1fb45583ba6340b1f Mon Sep 17 00:00:00 2001 From: Philip Ginsbach Date: Mon, 4 Sep 2023 14:27:30 +0100 Subject: [PATCH 15/24] TextMate grammar: imports can have instantiation arguments --- .../ql-vscode/syntaxes/ql.tmLanguage.yml | 22 +++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/syntaxes/ql.tmLanguage.yml b/extensions/ql-vscode/syntaxes/ql.tmLanguage.yml index d7f8ac071..f0d73d3d9 100644 --- a/extensions/ql-vscode/syntaxes/ql.tmLanguage.yml +++ b/extensions/ql-vscode/syntaxes/ql.tmLanguage.yml @@ -170,6 +170,14 @@ repository: match: '\]' name: punctuation.squarebracket.close.ql + open-angle: + match: '<' + name: punctuation.anglebracket.open.ql + + close-angle: + match: '>' + name: punctuation.anglebracket.close.ql + operator-or-punctuation: patterns: - include: '#relational-operator' @@ -186,6 +194,8 @@ repository: - include: '#close-brace' - include: '#open-bracket' - include: '#close-bracket' + - include: '#open-angle' + - include: '#close-angle' # Keywords dont-care: @@ -651,6 +661,15 @@ repository: - include: '#non-context-sensitive' - include: '#annotation' + instantiation-arguments: + beginPattern: '#open-angle' + end: '#close-angle' + patterns: + - include: '#potential-instantiation' + + potential-instantiation: + matches: '(?#simple-id) (?#instantiation-arguments)?' + # An `import` directive. Note that we parse the optional `as` clause as a separate top-level # directive, because otherwise it's too hard to figure out where the `import` directive ends. import-directive: @@ -664,7 +683,7 @@ repository: name: meta.block.import-directive.ql patterns: - include: '#non-context-sensitive' - - match: '(?#simple-id)' + - match: '(?#potential-instantiation)' name: entity.name.type.namespace.ql # The end pattern for an `as` clause, whether on an `import` directive, in an aggregate, or on a @@ -703,7 +722,6 @@ repository: - match: '(?#simple-id)|(?#at-lower-id)' name: entity.name.type.ql - # A `module` declaration, whether a module definition or an alias declaration. module-declaration: # Starts with the `module` keyword. From 599d31e5ac52b699c4b162fe40af5e60de5d2f22 Mon Sep 17 00:00:00 2001 From: Philip Ginsbach Date: Mon, 4 Sep 2023 13:49:32 +0100 Subject: [PATCH 16/24] add changelog entry for TextMate instantiation argument syntax --- extensions/ql-vscode/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index bdc2e476c..ae3226240 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -7,6 +7,7 @@ - Increase the required version of VS Code to 1.82.0. [#2877](https://github.com/github/vscode-codeql/pull/2877) - Fix a bug where the query server was restarted twice after configuration changes. [#2884](https://github.com/github/vscode-codeql/pull/2884). - Add support for the `telemetry.telemetryLevel` setting. For more information, see the [telemetry documentation](https://codeql.github.com/docs/codeql-for-visual-studio-code/about-telemetry-in-codeql-for-visual-studio-code). [#2824](https://github.com/github/vscode-codeql/pull/2824). +- Fixed syntax highlighting directly after import statements with instantiation arguments. ## 1.9.1 - 29 September 2023 From 353e22d6e8e5400f08b87ad6125f8553eb471ed9 Mon Sep 17 00:00:00 2001 From: Philip Ginsbach Date: Fri, 8 Sep 2023 09:50:28 +0100 Subject: [PATCH 17/24] link to the PR from the changelog entry --- extensions/ql-vscode/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index ae3226240..56c1e2a9c 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -7,7 +7,7 @@ - Increase the required version of VS Code to 1.82.0. [#2877](https://github.com/github/vscode-codeql/pull/2877) - Fix a bug where the query server was restarted twice after configuration changes. [#2884](https://github.com/github/vscode-codeql/pull/2884). - Add support for the `telemetry.telemetryLevel` setting. For more information, see the [telemetry documentation](https://codeql.github.com/docs/codeql-for-visual-studio-code/about-telemetry-in-codeql-for-visual-studio-code). [#2824](https://github.com/github/vscode-codeql/pull/2824). -- Fixed syntax highlighting directly after import statements with instantiation arguments. +- Fix syntax highlighting directly after import statements with instantiation arguments. [#2792](https://github.com/github/vscode-codeql/pull/2792) ## 1.9.1 - 29 September 2023 From c78f01758a635dfebc2b30e537b2778786697d8d Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Mon, 25 Sep 2023 18:31:59 -0400 Subject: [PATCH 18/24] Alternate fix for import highlighting with instantiations --- .../ql-vscode/syntaxes/ql.tmLanguage.yml | 35 ++++++++++++------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/extensions/ql-vscode/syntaxes/ql.tmLanguage.yml b/extensions/ql-vscode/syntaxes/ql.tmLanguage.yml index f0d73d3d9..fe10a86d0 100644 --- a/extensions/ql-vscode/syntaxes/ql.tmLanguage.yml +++ b/extensions/ql-vscode/syntaxes/ql.tmLanguage.yml @@ -661,29 +661,38 @@ repository: - include: '#non-context-sensitive' - include: '#annotation' - instantiation-arguments: + # The argument list of an instantiation, enclosed in angle brackets. + instantiation-args: beginPattern: '#open-angle' - end: '#close-angle' + endPattern: '#close-angle' + name: meta.type.parameters.ql patterns: - - include: '#potential-instantiation' - - potential-instantiation: - matches: '(?#simple-id) (?#instantiation-arguments)?' + # Include `#instantiation-args` first so that `#open-angle` and `#close-angle` take precedence + # over `#relational-operator`. + - include: '#instantiation-args' + - include: '#non-context-sensitive' + - match: '(?#simple-id)' + name: entity.name.type.namespace.ql # An `import` directive. Note that we parse the optional `as` clause as a separate top-level # directive, because otherwise it's too hard to figure out where the `import` directive ends. import-directive: beginPattern: '#import' - # Ends with a simple-id that is not followed by a `.` or a `::`. This does not handle comments or - # line breaks between the simple-id and the `.` or `::`. - end: '(?#simple-id) (?!\s*(\.|\:\:))' - endCaptures: - '0': - name: entity.name.type.namespace.ql + # TextMate makes it tricky to tell whether an identifier that we encounter is part of the + # `import` directive or whether it's the first token of the next module-level declaration. + # To find the end of the import directive, we'll look for a zero-width match where the previous + # token is either an identifier (other than `import`) or a `>`, and the next token is not a `.`, + # `<`, `,`, or `::`. This works for nearly all real-world `import` directives, but it will end the + # `import` directive too early if there is a comment or line break between two components of the + # module expression. + end: '(?)|[A-Za-z0-9_]) (?!\s*(\.|\:\:|\,|(?#open-angle)))' name: meta.block.import-directive.ql patterns: + # Include `#instantiation-args` first so that `#open-angle` and `#close-angle` take precedence + # over `#relational-operator`. + - include: '#instantiation-args' - include: '#non-context-sensitive' - - match: '(?#potential-instantiation)' + - match: '(?#simple-id)' name: entity.name.type.namespace.ql # The end pattern for an `as` clause, whether on an `import` directive, in an aggregate, or on a From 47fa163cb9fffce379458962eb391c9ef9082dcb Mon Sep 17 00:00:00 2001 From: Philip Ginsbach Date: Fri, 8 Sep 2023 09:43:39 +0100 Subject: [PATCH 19/24] Update compiled grammar --- syntaxes/ql.tmLanguage.json | 63 ++++++++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 8 deletions(-) diff --git a/syntaxes/ql.tmLanguage.json b/syntaxes/ql.tmLanguage.json index 66e1cc8ec..a8b5c3909 100644 --- a/syntaxes/ql.tmLanguage.json +++ b/syntaxes/ql.tmLanguage.json @@ -108,6 +108,14 @@ "match": "(?x)\\]", "name": "punctuation.squarebracket.close.ql" }, + "open-angle": { + "match": "(?x)<", + "name": "punctuation.anglebracket.open.ql" + }, + "close-angle": { + "match": "(?x)>", + "name": "punctuation.anglebracket.close.ql" + }, "operator-or-punctuation": { "patterns": [ { @@ -151,6 +159,12 @@ }, { "include": "#close-bracket" + }, + { + "include": "#open-angle" + }, + { + "include": "#close-angle" } ] }, @@ -661,9 +675,9 @@ "begin": "(?x)(?<=/\\*\\*)([^*]|\\*(?!/))*$", "while": "(?x)(^|\\G)\\s*([^*]|\\*(?!/))(?=([^*]|[*](?!/))*$)", "patterns": [ - - - + + + { "match": "(?x)\\G\\s* (@\\S+)", "name": "keyword.tag.ql" @@ -723,15 +737,48 @@ } ] }, - "import-directive": { - "end": "(?x)(?:\\b [A-Za-z][0-9A-Za-z_]* (?:(?!(?:[0-9A-Za-z_])))) (?!\\s*(\\.|\\:\\:))", - "endCaptures": { - "0": { + "instantiation-args": { + "name": "meta.type.parameters.ql", + "patterns": [ + { + "include": "#instantiation-args" + }, + { + "include": "#non-context-sensitive" + }, + { + "match": "(?x)(?:\\b [A-Za-z][0-9A-Za-z_]* (?:(?!(?:[0-9A-Za-z_]))))", "name": "entity.name.type.namespace.ql" } + ], + "begin": "(?x)((?:<))", + "beginCaptures": { + "1": { + "patterns": [ + { + "include": "#open-angle" + } + ] + } }, + "end": "(?x)((?:>))", + "endCaptures": { + "1": { + "patterns": [ + { + "include": "#close-angle" + } + ] + } + } + }, + "import-directive": { + "end": "(?x)(?)|[A-Za-z0-9_]) (?!\\s*(\\.|\\:\\:|\\,|(?:<)))", "name": "meta.block.import-directive.ql", "patterns": [ + { + "include": "#instantiation-args" + }, { "include": "#non-context-sensitive" }, @@ -1493,4 +1540,4 @@ "name": "constant.character.escape.ql" } } -} \ No newline at end of file +} From df02fecf3cea5fc32dd331756223557a505849eb Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Mon, 9 Oct 2023 14:44:33 -0400 Subject: [PATCH 20/24] Fix test expectations --- .../no-workspace/model-editor/auto-model-codeml-queries.test.ts | 1 + .../no-workspace/model-editor/external-api-usage-query.test.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/auto-model-codeml-queries.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/auto-model-codeml-queries.test.ts index 161c88210..b2bccaa90 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/auto-model-codeml-queries.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/auto-model-codeml-queries.test.ts @@ -159,6 +159,7 @@ describe("runAutoModelQueries", () => { false, expect.arrayContaining([expect.stringContaining("tmp")]), ["/a/b/c/my-extension-pack"], + {}, "/tmp/queries", undefined, undefined, diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/external-api-usage-query.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/external-api-usage-query.test.ts index d207fa887..5641474f2 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/external-api-usage-query.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/external-api-usage-query.test.ts @@ -165,6 +165,7 @@ describe("external api usage query", () => { false, [], ["my/extensions"], + {}, "/tmp/queries", undefined, undefined, From 54e1b29940b806d121ab37bb51e4a2823cf8535f Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Tue, 10 Oct 2023 09:26:27 +0200 Subject: [PATCH 21/24] Add explicit return type to convertToLegacyModeledMethod --- .../method-modeling-view-provider.ts | 11 +++++++---- .../src/model-editor/modeled-methods-legacy.ts | 16 ++++++++++++---- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts index 0be5d98ce..93db7c61e 100644 --- a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts +++ b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts @@ -150,10 +150,13 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< if (this.webviewView && e.isActiveDb) { const modeledMethods = e.modeledMethods[this.method?.signature ?? ""]; if (modeledMethods) { - await this.postMessage({ - t: "setModeledMethod", - method: convertToLegacyModeledMethod(modeledMethods), - }); + const modeledMethod = convertToLegacyModeledMethod(modeledMethods); + if (modeledMethod) { + await this.postMessage({ + t: "setModeledMethod", + method: modeledMethod, + }); + } } } }), diff --git a/extensions/ql-vscode/src/model-editor/modeled-methods-legacy.ts b/extensions/ql-vscode/src/model-editor/modeled-methods-legacy.ts index 3ca32c7c2..deff3f83f 100644 --- a/extensions/ql-vscode/src/model-editor/modeled-methods-legacy.ts +++ b/extensions/ql-vscode/src/model-editor/modeled-methods-legacy.ts @@ -37,9 +37,15 @@ export function convertToLegacyModeledMethods( ): Record { // Always take the first modeled method in the array return Object.fromEntries( - Object.entries(modeledMethods).map(([signature, modeledMethods]) => { - return [signature, convertToLegacyModeledMethod(modeledMethods)]; - }), + Object.entries(modeledMethods) + .map(([signature, modeledMethods]) => { + const modeledMethod = convertToLegacyModeledMethod(modeledMethods); + if (!modeledMethod) { + return null; + } + return [signature, modeledMethod]; + }) + .filter((entry): entry is [string, ModeledMethod] => entry !== null), ); } @@ -66,6 +72,8 @@ export function convertFromLegacyModeledMethod(modeledMethod: ModeledMethod) { * * @param modeledMethods The ModeledMethod[] */ -export function convertToLegacyModeledMethod(modeledMethods: ModeledMethod[]) { +export function convertToLegacyModeledMethod( + modeledMethods: ModeledMethod[], +): ModeledMethod | undefined { return modeledMethods[0]; } From e98611fd21d88147ad05691b295c98db270b70bc Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Tue, 10 Oct 2023 08:57:36 +0100 Subject: [PATCH 22/24] Ensure modified methods updated after changing a method from the modeling panel (#2929) --- .../method-modeling/method-modeling-view-provider.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts index 207807836..5e31fab1d 100644 --- a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts +++ b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts @@ -100,6 +100,10 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< activeState.databaseItem, msg.method, ); + this.modelingStore.addModifiedMethod( + activeState.databaseItem, + msg.method.signature, + ); break; } case "revealInModelEditor": From f794a19d96b447b11d1c94a39c166ed8e703bcc9 Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Tue, 10 Oct 2023 09:51:50 +0100 Subject: [PATCH 23/24] Track db related to modeled method and react when it closes (#2930) --- .../ql-vscode/src/common/interface-types.ts | 2 +- .../method-modeling/method-modeling-panel.ts | 8 ++++++-- .../method-modeling-view-provider.ts | 16 ++++++++++++++-- .../src/model-editor/model-editor-module.ts | 2 +- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/extensions/ql-vscode/src/common/interface-types.ts b/extensions/ql-vscode/src/common/interface-types.ts index 58c96d3b2..52bdfb093 100644 --- a/extensions/ql-vscode/src/common/interface-types.ts +++ b/extensions/ql-vscode/src/common/interface-types.ts @@ -634,7 +634,7 @@ interface SetMethodModelingPanelViewStateMessage { interface SetMethodMessage { t: "setMethod"; - method: Method; + method: Method | undefined; } interface SetMethodModifiedMessage { diff --git a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-panel.ts b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-panel.ts index e4e54694b..0fa3bed7d 100644 --- a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-panel.ts +++ b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-panel.ts @@ -6,6 +6,7 @@ import { Method } from "../method"; import { ModelingStore } from "../modeling-store"; import { ModelEditorViewTracker } from "../model-editor-view-tracker"; import { ModelConfigListener } from "../../config"; +import { DatabaseItem } from "../../databases/local-databases"; export class MethodModelingPanel extends DisposableObject { private readonly provider: MethodModelingViewProvider; @@ -36,7 +37,10 @@ export class MethodModelingPanel extends DisposableObject { ); } - public async setMethod(method: Method): Promise { - await this.provider.setMethod(method); + public async setMethod( + databaseItem: DatabaseItem, + method: Method, + ): Promise { + await this.provider.setMethod(databaseItem, method); } } diff --git a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts index 5e31fab1d..fec1e091a 100644 --- a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts +++ b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts @@ -13,6 +13,7 @@ import { AbstractWebviewViewProvider } from "../../common/vscode/abstract-webvie import { assertNever } from "../../common/helpers-pure"; import { ModelEditorViewTracker } from "../model-editor-view-tracker"; import { ModelConfigListener } from "../../config"; +import { DatabaseItem } from "../../databases/local-databases"; export class MethodModelingViewProvider extends AbstractWebviewViewProvider< ToMethodModelingMessage, @@ -21,6 +22,7 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< public static readonly viewType = "codeQLMethodModeling"; private method: Method | undefined = undefined; + private databaseItem: DatabaseItem | undefined = undefined; constructor( app: App, @@ -46,8 +48,12 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< }); } - public async setMethod(method: Method): Promise { + public async setMethod( + databaseItem: DatabaseItem | undefined, + method: Method | undefined, + ): Promise { this.method = method; + this.databaseItem = databaseItem; if (this.isShowingView) { await this.postMessage({ @@ -174,6 +180,8 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< this.modelingStore.onSelectedMethodChanged(async (e) => { if (this.webviewView) { this.method = e.method; + this.databaseItem = e.databaseItem; + await this.postMessage({ t: "setSelectedMethod", method: e.method, @@ -194,13 +202,17 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< ); this.push( - this.modelingStore.onDbClosed(async () => { + this.modelingStore.onDbClosed(async (dbUri) => { if (!this.modelingStore.anyDbsBeingModeled()) { await this.postMessage({ t: "setInModelingMode", inModelingMode: false, }); } + + if (dbUri === this.databaseItem?.databaseUri.toString()) { + await this.setMethod(undefined, undefined); + } }), ); } diff --git a/extensions/ql-vscode/src/model-editor/model-editor-module.ts b/extensions/ql-vscode/src/model-editor/model-editor-module.ts index 1eb7c0f17..7ad81d0b2 100644 --- a/extensions/ql-vscode/src/model-editor/model-editor-module.ts +++ b/extensions/ql-vscode/src/model-editor/model-editor-module.ts @@ -105,7 +105,7 @@ export class ModelEditorModule extends DisposableObject { usage: Usage, ): Promise { await this.methodsUsagePanel.revealItem(usage); - await this.methodModelingPanel.setMethod(method); + await this.methodModelingPanel.setMethod(databaseItem, method); await showResolvableLocation(usage.url, databaseItem, this.app.logger); } From 7eab7e4e48cbfa2164ec1b4b328ac9f1a1b3b967 Mon Sep 17 00:00:00 2001 From: Charis Kyriakou Date: Tue, 10 Oct 2023 10:12:01 +0100 Subject: [PATCH 24/24] Set modeling mode when initialising method modeling panel (#2932) --- .../method-modeling/method-modeling-view-provider.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts index fec1e091a..c57c9e51e 100644 --- a/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts +++ b/extensions/ql-vscode/src/model-editor/method-modeling/method-modeling-view-provider.ts @@ -74,6 +74,11 @@ export class MethodModelingViewProvider extends AbstractWebviewViewProvider< isModified: selectedMethod.isModified, }); } + + await this.postMessage({ + t: "setInModelingMode", + inModelingMode: true, + }); } }