Remove codeQL.model.showTypeModels config

This removes the `codeQL.model.showTypeModels` setting. We will now
always show type models in the model editor. This also means that the
"auto-generation mode" needs to change to `models` to ensure that the
type models are actually shown to the user. This is done for Ruby, which
is currently the only language to show type models.
This commit is contained in:
Koen Vlaswinkel
2024-04-12 09:31:28 +02:00
parent c37bd9969c
commit d67cf4b9f7
11 changed files with 42 additions and 149 deletions

View File

@@ -725,7 +725,6 @@ export async function setAutogenerateQlPacks(choice: AutogenerateQLPacks) {
const MODEL_SETTING = new Setting("model", ROOT_SETTING);
const FLOW_GENERATION = new Setting("flowGeneration", MODEL_SETTING);
const LLM_GENERATION = new Setting("llmGeneration", MODEL_SETTING);
const SHOW_TYPE_MODELS = new Setting("showTypeModels", MODEL_SETTING);
const LLM_GENERATION_BATCH_SIZE = new Setting(
"llmGenerationBatchSize",
MODEL_SETTING,
@@ -749,7 +748,6 @@ export type ModelConfigPackVariables = {
export interface ModelConfig {
flowGeneration: boolean;
llmGeneration: boolean;
showTypeModels: boolean;
getPackLocation(
languageId: string,
variables: ModelConfigPackVariables,
@@ -771,10 +769,6 @@ export class ModelConfigListener extends ConfigListener implements ModelConfig {
return !!LLM_GENERATION.getValue<boolean>();
}
public get showTypeModels(): boolean {
return !!SHOW_TYPE_MODELS.getValue<boolean>();
}
/**
* Limits the number of candidates we send to the model in each request to avoid long requests.
* Note that the model may return fewer than this number of candidates.

View File

@@ -20,7 +20,7 @@ import type { AccessPathSuggestionRow } from "../suggestions";
// This is a subset of the model config that doesn't import the vscode module.
// It only includes settings that are actually used.
export type ModelConfig = {
showTypeModels: boolean;
flowGeneration: boolean;
};
/**
@@ -32,12 +32,12 @@ export type ModelConfig = {
*/
export function createModelConfig(modelConfig: ModelConfig): ModelConfig {
return {
showTypeModels: modelConfig.showTypeModels,
flowGeneration: modelConfig.flowGeneration,
};
}
export const defaultModelConfig: ModelConfig = {
showTypeModels: false,
flowGeneration: false,
};
type GenerateMethodDefinition<T> = (method: T) => DataTuple[];

View File

@@ -1,9 +1,6 @@
import type { BaseLogger } from "../../../common/logging";
import type { DecodedBqrs } from "../../../common/bqrs-cli-types";
import type {
GenerationContext,
ModelsAsDataLanguage,
} from "../models-as-data";
import type { ModelsAsDataLanguage } from "../models-as-data";
import type { ModeledMethod } from "../../modeled-method";
import type { DataTuple } from "../../model-extension-file";
@@ -12,21 +9,10 @@ export function parseGenerateModelResults(
bqrs: DecodedBqrs,
modelsAsDataLanguage: ModelsAsDataLanguage,
logger: BaseLogger,
{ config }: GenerationContext,
): ModeledMethod[] {
const modeledMethods: ModeledMethod[] = [];
for (const resultSetName in bqrs) {
if (
resultSetName ===
modelsAsDataLanguage.predicates.type?.extensiblePredicate &&
!config.showTypeModels
) {
// Don't load generated type results when type models are hidden. These are already
// automatically generated on start-up.
continue;
}
const definition = Object.values(modelsAsDataLanguage.predicates).find(
(definition) => definition.extensiblePredicate === resultSetName,
);

View File

@@ -170,7 +170,6 @@ export const ruby: ModelsAsDataLanguage = {
methodParameters: "",
};
},
isHidden: ({ config }) => !config.showTypeModels,
},
},
modelGeneration: {
@@ -210,7 +209,7 @@ export const ruby: ModelsAsDataLanguage = {
},
];
},
parseResults: (queryPath, bqrs, modelsAsDataLanguage, logger, context) => {
parseResults: (queryPath, bqrs, modelsAsDataLanguage, logger) => {
// Only parse type models when automatically generating models
const typePredicate = modelsAsDataLanguage.predicates.type;
if (!typePredicate) {
@@ -229,13 +228,12 @@ export const ruby: ModelsAsDataLanguage = {
},
modelsAsDataLanguage,
logger,
context,
);
},
// Only enabled for framework mode when type models are hidden
type: ({ mode, config }) =>
mode === Mode.Framework && !config.showTypeModels
? AutoModelGenerationType.SeparateFile
type: ({ mode }) =>
mode === Mode.Framework
? AutoModelGenerationType.Models
: AutoModelGenerationType.Disabled,
},
accessPathSuggestions: {

View File

@@ -278,7 +278,6 @@ export class ModelEditorView extends AbstractWebview<
modeledMethods,
mode,
this.cliServer,
this.modelConfig,
this.app.logger,
);
@@ -487,7 +486,6 @@ export class ModelEditorView extends AbstractWebview<
this.extensionPack,
this.language,
this.cliServer,
this.modelConfig,
this.app.logger,
);
this.modelingStore.setModeledMethods(this.databaseItem, modeledMethods);

View File

@@ -12,7 +12,6 @@ import { load as loadYaml } from "js-yaml";
import type { CodeQLCliServer } from "../codeql-cli/cli";
import { pathsEqual } from "../common/files";
import type { QueryLanguage } from "../common/query-language";
import type { ModelConfig } from "./languages";
export const GENERATED_MODELS_SUFFIX = ".model.generated.yml";
@@ -23,14 +22,12 @@ export async function saveModeledMethods(
modeledMethods: Readonly<Record<string, readonly ModeledMethod[]>>,
mode: Mode,
cliServer: CodeQLCliServer,
modelConfig: ModelConfig,
logger: NotificationLogger,
): Promise<void> {
const existingModeledMethods = await loadModeledMethodFiles(
extensionPack,
language,
cliServer,
modelConfig,
logger,
);
@@ -53,14 +50,9 @@ async function loadModeledMethodFiles(
extensionPack: ExtensionPack,
language: QueryLanguage,
cliServer: CodeQLCliServer,
modelConfig: ModelConfig,
logger: NotificationLogger,
): Promise<Record<string, Record<string, ModeledMethod[]>>> {
const modelFiles = await listModelFiles(
extensionPack.path,
cliServer,
modelConfig,
);
const modelFiles = await listModelFiles(extensionPack.path, cliServer);
const modeledMethodsByFile: Record<
string,
@@ -92,7 +84,6 @@ export async function loadModeledMethods(
extensionPack: ExtensionPack,
language: QueryLanguage,
cliServer: CodeQLCliServer,
modelConfig: ModelConfig,
logger: NotificationLogger,
): Promise<Record<string, ModeledMethod[]>> {
const existingModeledMethods: Record<string, ModeledMethod[]> = {};
@@ -101,7 +92,6 @@ export async function loadModeledMethods(
extensionPack,
language,
cliServer,
modelConfig,
logger,
);
for (const modeledMethods of Object.values(modeledMethodsByFile)) {
@@ -120,7 +110,6 @@ export async function loadModeledMethods(
export async function listModelFiles(
extensionPackPath: string,
cliServer: CodeQLCliServer,
modelConfig: ModelConfig,
): Promise<Set<string>> {
const result = await cliServer.resolveExtensions(
extensionPackPath,
@@ -131,11 +120,8 @@ export async function listModelFiles(
for (const [path, extensions] of Object.entries(result.data)) {
if (pathsEqual(path, extensionPackPath)) {
for (const extension of extensions) {
// We only load generated models when type models are shown
if (
!modelConfig.showTypeModels &&
extension.file.endsWith(GENERATED_MODELS_SUFFIX)
) {
// We never load generated models
if (extension.file.endsWith(GENERATED_MODELS_SUFFIX)) {
continue;
}

View File

@@ -36,33 +36,7 @@ describe(ModelTypeDropdown.name, () => {
);
});
it("allows changing the type to 'Type' for Ruby when type models are shown", async () => {
const method = createMethod();
const modeledMethod = createNoneModeledMethod();
render(
<ModelTypeDropdown
language={QueryLanguage.Ruby}
modeledMethod={modeledMethod}
modelPending={false}
onChange={onChange}
method={method}
modelConfig={{
...defaultModelConfig,
showTypeModels: true,
}}
/>,
);
await userEvent.selectOptions(screen.getByRole("combobox"), "type");
expect(onChange).toHaveBeenCalledWith(
expect.objectContaining({
type: "type",
}),
);
});
it("does not allow changing the type to 'Type' for Ruby when type models are not shown", async () => {
it("allows changing the type to 'Type' for Ruby", async () => {
const method = createMethod();
const modeledMethod = createNoneModeledMethod();
@@ -77,9 +51,12 @@ describe(ModelTypeDropdown.name, () => {
/>,
);
expect(
screen.queryByRole("option", { name: "Type" }),
).not.toBeInTheDocument();
await userEvent.selectOptions(screen.getByRole("combobox"), "type");
expect(onChange).toHaveBeenCalledWith(
expect.objectContaining({
type: "type",
}),
);
});
it("does not allow changing the type to 'Type' for Java", async () => {

View File

@@ -4,8 +4,6 @@ import { ruby } from "../../../../../src/model-editor/languages/ruby";
import { createMockLogger } from "../../../../__mocks__/loggerMock";
import type { ModeledMethod } from "../../../../../src/model-editor/modeled-method";
import { EndpointType } from "../../../../../src/model-editor/method";
import { Mode } from "../../../../../src/model-editor/shared/mode";
import { defaultModelConfig } from "../../../../../src/model-editor/languages";
describe("parseGenerateModelResults", () => {
it("should return the results", async () => {
@@ -78,13 +76,6 @@ describe("parseGenerateModelResults", () => {
bqrs,
ruby,
createMockLogger(),
{
mode: Mode.Framework,
config: {
...defaultModelConfig,
showTypeModels: true,
},
},
);
expect(result).toEqual([
{

View File

@@ -13,7 +13,6 @@ import { homedir, tmpdir } from "os";
import { mkdir, rm } from "fs-extra";
import { nanoid } from "nanoid";
import { QueryLanguage } from "../../../../src/common/query-language";
import { defaultModelConfig } from "../../../../src/model-editor/languages";
const dummyExtensionPackContents = `
name: dummy/pack
@@ -136,11 +135,7 @@ describe("modeled-method-fs", () => {
it("should return the empty set when the extension pack is empty", async () => {
const extensionPackPath = writeExtensionPackFiles("extension-pack", []);
const modelFiles = await listModelFiles(
extensionPackPath,
cli,
defaultModelConfig,
);
const modelFiles = await listModelFiles(extensionPackPath, cli);
expect(modelFiles).toEqual(new Set());
});
@@ -150,11 +145,7 @@ describe("modeled-method-fs", () => {
"library2.model.yml",
]);
const modelFiles = await listModelFiles(
extensionPackPath,
cli,
defaultModelConfig,
);
const modelFiles = await listModelFiles(extensionPackPath, cli);
expect(modelFiles).toEqual(
new Set([
join("models", "library1.model.yml"),
@@ -163,18 +154,14 @@ describe("modeled-method-fs", () => {
);
});
it("should ignore generated type models when type models are hidden", async () => {
it("should ignore generated models", async () => {
const extensionPackPath = writeExtensionPackFiles("extension-pack", [
"library1.model.yml",
"library2.model.yml",
"library.model.generated.yml",
]);
const modelFiles = await listModelFiles(
extensionPackPath,
cli,
defaultModelConfig,
);
const modelFiles = await listModelFiles(extensionPackPath, cli);
expect(modelFiles).toEqual(
new Set([
join("models", "library1.model.yml"),
@@ -183,37 +170,13 @@ describe("modeled-method-fs", () => {
);
});
it("should include generated type models when type models are shown", async () => {
const extensionPackPath = writeExtensionPackFiles("extension-pack", [
"library1.model.yml",
"library2.model.yml",
"library.model.generated.yml",
]);
const modelFiles = await listModelFiles(extensionPackPath, cli, {
...defaultModelConfig,
showTypeModels: true,
});
expect(modelFiles).toEqual(
new Set([
join("models", "library.model.generated.yml"),
join("models", "library1.model.yml"),
join("models", "library2.model.yml"),
]),
);
});
it("should ignore model files from other extension packs", async () => {
const extensionPackPath = writeExtensionPackFiles("extension-pack", [
"library1.model.yml",
]);
writeExtensionPackFiles("another-extension-pack", ["library2.model.yml"]);
const modelFiles = await listModelFiles(
extensionPackPath,
cli,
defaultModelConfig,
);
const modelFiles = await listModelFiles(extensionPackPath, cli);
expect(modelFiles).toEqual(
new Set([join("models", "library1.model.yml")]),
);
@@ -230,7 +193,6 @@ describe("modeled-method-fs", () => {
makeExtensionPack(extensionPackPath),
QueryLanguage.Java,
cli,
defaultModelConfig,
extLogger,
);

View File

@@ -138,10 +138,7 @@ describe("runGenerateQueries", () => {
createMockLogger(),
{
mode: Mode.Framework,
config: {
...defaultModelConfig,
showTypeModels: true,
},
config: defaultModelConfig,
},
),
);

View File

@@ -4,7 +4,10 @@ import { MethodModelingViewProvider } from "../../../../../src/model-editor/meth
import { createMockApp } from "../../../../__mocks__/appMock";
import { createMockModelingStore } from "../../../../__mocks__/model-editor/modelingStoreMock";
import { mockedObject } from "../../../../mocked-object";
import type { FromMethodModelingMessage } from "../../../../../src/common/interface-types";
import type {
FromMethodModelingMessage,
ToMethodModelingMessage,
} from "../../../../../src/common/interface-types";
import { DisposableObject } from "../../../../../src/common/disposable-object";
import { ModelingEvents } from "../../../../../src/model-editor/modeling-events";
import type {
@@ -17,6 +20,7 @@ import {
createMethod,
createUsage,
} from "../../../../factories/model-editor/method-factories";
import { QueryLanguage } from "../../../../../src/common/query-language";
describe("method modeling view provider", () => {
// Modeling store
@@ -45,7 +49,7 @@ describe("method modeling view provider", () => {
const modelingEvents = new ModelingEvents(app);
const modelConfigListener = mockedObject<ModelConfigListener>({
showTypeModels: true,
flowGeneration: true,
onDidChangeConfiguration: jest.fn(),
});
@@ -91,10 +95,10 @@ describe("method modeling view provider", () => {
viewState: {
language: undefined,
modelConfig: {
showTypeModels: true,
flowGeneration: true,
},
},
});
} satisfies ToMethodModelingMessage);
});
it("should load webview when active DB but no selected method", async () => {
@@ -116,10 +120,10 @@ describe("method modeling view provider", () => {
viewState: {
language: undefined,
modelConfig: {
showTypeModels: true,
flowGeneration: true,
},
},
});
} satisfies ToMethodModelingMessage);
expect(postMessage).toHaveBeenNthCalledWith(2, {
t: "setInModelingMode",
inModelingMode: true,
@@ -127,12 +131,12 @@ describe("method modeling view provider", () => {
expect(postMessage).toHaveBeenNthCalledWith(3, {
t: "setMethodModelingPanelViewState",
viewState: {
language: "java",
language: QueryLanguage.Java,
modelConfig: {
showTypeModels: true,
flowGeneration: true,
},
},
});
} satisfies ToMethodModelingMessage);
});
it("should load webview when active DB and a selected method", async () => {
@@ -165,10 +169,10 @@ describe("method modeling view provider", () => {
viewState: {
language: undefined,
modelConfig: {
showTypeModels: true,
flowGeneration: true,
},
},
});
} satisfies ToMethodModelingMessage);
expect(postMessage).toHaveBeenNthCalledWith(2, {
t: "setInModelingMode",
inModelingMode: true,
@@ -176,12 +180,12 @@ describe("method modeling view provider", () => {
expect(postMessage).toHaveBeenNthCalledWith(3, {
t: "setMethodModelingPanelViewState",
viewState: {
language: "java",
language: QueryLanguage.Java,
modelConfig: {
showTypeModels: true,
flowGeneration: true,
},
},
});
} satisfies ToMethodModelingMessage);
expect(postMessage).toHaveBeenNthCalledWith(4, {
t: "setSelectedMethod",
method: selectedMethodDetails.method,