Merge pull request #3401 from github/koesie10/hide-generated-type-models

Generate separate file for generated type models in Ruby by default
2024-02-28 11:47:58 +01:00
parent ab36153511 0599adccc0
commit 359ee76d52
10 changed files with 242 additions and 72 deletions
--- a/extensions/ql-vscode/src/model-editor/generate.ts
+++ b/extensions/ql-vscode/src/model-editor/generate.ts
@@ -5,19 +5,15 @@ import type { QueryRunner } from "../query-server";
 import type { CodeQLCliServer } from "../codeql-cli/cli";
 import type { ProgressCallback } from "../common/vscode/progress";
 import { getOnDiskWorkspaceFolders } from "../common/vscode/workspace-folders";
-import type { ModeledMethod } from "./modeled-method";
 import { runQuery } from "../local-queries/run-query";
 import type { QueryConstraints } from "../local-queries";
 import { resolveQueries } from "../local-queries";
 import type { DecodedBqrs } from "../common/bqrs-cli-types";
+
 type GenerateQueriesOptions = {
  queryConstraints: QueryConstraints;
  filterQueries?: (queryPath: string) => boolean;
-  parseResults: (
-    queryPath: string,
-    results: DecodedBqrs,
-  ) => ModeledMethod[] | Promise<ModeledMethod[]>;
-  onResults: (results: ModeledMethod[]) => void | Promise<void>;
+  onResults: (queryPath: string, results: DecodedBqrs) => void | Promise<void>;

  cliServer: CodeQLCliServer;
  queryRunner: QueryRunner;
@@ -28,7 +24,7 @@ type GenerateQueriesOptions = {
 };

 export async function runGenerateQueries(options: GenerateQueriesOptions) {
-  const { queryConstraints, filterQueries, parseResults, onResults } = options;
+  const { queryConstraints, filterQueries, onResults } = options;

  options.progress({
    message: "Resolving queries",
@@ -55,7 +51,7 @@ export async function runGenerateQueries(options: GenerateQueriesOptions) {

    const bqrs = await runSingleGenerateQuery(queryPath, i, maxStep, options);
    if (bqrs) {
-      await onResults(await parseResults(queryPath, bqrs));
+      await onResults(queryPath, bqrs);
    }
  }
 }
--- a/extensions/ql-vscode/src/model-editor/languages/models-as-data.ts
+++ b/extensions/ql-vscode/src/model-editor/languages/models-as-data.ts
@@ -7,7 +7,7 @@ import type {
  SummaryModeledMethod,
  TypeModeledMethod,
 } from "../modeled-method";
-import type { DataTuple } from "../model-extension-file";
+import type { DataTuple, ModelExtension } from "../model-extension-file";
 import type { Mode } from "../shared/mode";
 import type { QueryConstraints } from "../../local-queries/query-constraints";
 import type {
@@ -68,6 +68,11 @@ export type ModelsAsDataLanguagePredicate<T> = {
  isHidden?: (context: IsHiddenContext) => boolean;
 };

+export type GenerationContext = {
+  mode: Mode;
+  config: ModelConfig;
+};
+
 type ParseGenerationResults = (
  // The path to the query that generated the results.
  queryPath: string,
@@ -78,24 +83,37 @@ type ParseGenerationResults = (
  modelsAsDataLanguage: ModelsAsDataLanguage,
  // The logger to use for logging.
  logger: BaseLogger,
+  // Context about this invocation of the generation.
+  context: GenerationContext,
 ) => ModeledMethod[];

 type ModelsAsDataLanguageModelGeneration = {
  queryConstraints: (mode: Mode) => QueryConstraints;
  filterQueries?: (queryPath: string) => boolean;
  parseResults: ParseGenerationResults;
+};
+
+type ParseResultsToYaml = (
+  // The path to the query that generated the results.
+  queryPath: string,
+  // The results of the query.
+  bqrs: DecodedBqrs,
+  // The language-specific predicate that was used to generate the results. This is passed to allow
+  // sharing of code between different languages.
+  modelsAsDataLanguage: ModelsAsDataLanguage,
+  // The logger to use for logging.
+  logger: BaseLogger,
+) => ModelExtension[];
+
+type ModelsAsDataLanguageAutoModelGeneration = {
+  queryConstraints: (mode: Mode) => QueryConstraints;
+  filterQueries?: (queryPath: string) => boolean;
+  parseResultsToYaml: ParseResultsToYaml;
  /**
-   * If autoRun is not undefined, the query will be run automatically when the user starts the
-   * model editor.
-   *
-   * This only applies to framework mode. Application mode will never run the query automatically.
+   * By default, auto model generation is enabled for all modes. This function can be used to
+   * override that behavior.
   */
-  autoRun?: {
-    /**
-     * If defined, will use a custom parsing function when the query is run automatically.
-     */
-    parseResults?: ParseGenerationResults;
-  };
+  enabled?: (context: GenerationContext) => boolean;
 };

 type ModelsAsDataLanguageAccessPathSuggestions = {
@@ -145,6 +163,7 @@ export type ModelsAsDataLanguage = {
  ) => EndpointType | undefined;
  predicates: ModelsAsDataLanguagePredicates;
  modelGeneration?: ModelsAsDataLanguageModelGeneration;
+  autoModelGeneration?: ModelsAsDataLanguageAutoModelGeneration;
  accessPathSuggestions?: ModelsAsDataLanguageAccessPathSuggestions;
  /**
   * Returns the list of valid arguments that can be selected for the given method.
--- a/extensions/ql-vscode/src/model-editor/languages/ruby/generate.ts
+++ b/extensions/ql-vscode/src/model-editor/languages/ruby/generate.ts
@@ -1,6 +1,9 @@
 import type { BaseLogger } from "../../../common/logging";
 import type { DecodedBqrs } from "../../../common/bqrs-cli-types";
-import type { ModelsAsDataLanguage } from "../models-as-data";
+import type {
+  GenerationContext,
+  ModelsAsDataLanguage,
+} from "../models-as-data";
 import type { ModeledMethod } from "../../modeled-method";
 import type { DataTuple } from "../../model-extension-file";

@@ -9,10 +12,21 @@ 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,
    );
--- a/extensions/ql-vscode/src/model-editor/languages/ruby/index.ts
+++ b/extensions/ql-vscode/src/model-editor/languages/ruby/index.ts
@@ -178,28 +178,40 @@ export const ruby: ModelsAsDataLanguage = {
      "tags contain all": ["modeleditor", "generate-model", modeTag(mode)],
    }),
    parseResults: parseGenerateModelResults,
-    autoRun: {
-      parseResults: (queryPath, bqrs, modelsAsDataLanguage, logger) => {
-        // Only type models are generated automatically
-        const typePredicate = modelsAsDataLanguage.predicates.type;
-        if (!typePredicate) {
-          throw new Error("Type predicate not found");
-        }
+  },
+  autoModelGeneration: {
+    queryConstraints: (mode) => ({
+      kind: "table",
+      "tags contain all": ["modeleditor", "generate-model", modeTag(mode)],
+    }),
+    parseResultsToYaml: (_queryPath, bqrs, modelsAsDataLanguage) => {
+      const typePredicate = modelsAsDataLanguage.predicates.type;
+      if (!typePredicate) {
+        throw new Error("Type predicate not found");
+      }

-        const filteredBqrs = Object.fromEntries(
-          Object.entries(bqrs).filter(
-            ([key]) => key === typePredicate.extensiblePredicate,
-          ),
-        );
+      const typeTuples = bqrs[typePredicate.extensiblePredicate];
+      if (!typeTuples) {
+        return [];
+      }

-        return parseGenerateModelResults(
-          queryPath,
-          filteredBqrs,
-          modelsAsDataLanguage,
-          logger,
-        );
-      },
+      return [
+        {
+          addsTo: {
+            pack: "codeql/ruby-all",
+            extensible: typePredicate.extensiblePredicate,
+          },
+          data: typeTuples.tuples.filter((tuple): tuple is string[] => {
+            return (
+              tuple.filter((x) => typeof x === "string").length === tuple.length
+            );
+          }),
+        },
+      ];
    },
+    // Only enabled for framework mode when type models are hidden
+    enabled: ({ mode, config }) =>
+      mode === Mode.Framework && !config.showTypeModels,
  },
  accessPathSuggestions: {
    queryConstraints: (mode) => ({
--- a/extensions/ql-vscode/src/model-editor/model-editor-view.ts
+++ b/extensions/ql-vscode/src/model-editor/model-editor-view.ts
@@ -41,7 +41,11 @@ import type { ModeledMethod } from "./modeled-method";
 import type { ExtensionPack } from "./shared/extension-pack";
 import type { ModelConfigListener } from "../config";
 import { Mode } from "./shared/mode";
-import { loadModeledMethods, saveModeledMethods } from "./modeled-method-fs";
+import {
+  GENERATED_MODELS_SUFFIX,
+  loadModeledMethods,
+  saveModeledMethods,
+} from "./modeled-method-fs";
 import { pickExtensionPack } from "./extension-pack-picker";
 import type { QueryLanguage } from "../common/query-language";
 import { getLanguageDisplayName } from "../common/query-language";
@@ -60,6 +64,10 @@ import { parseAccessPathSuggestionRowsToOptions } from "./suggestions-bqrs";
 import { ModelEvaluator } from "./model-evaluator";
 import type { ModelEvaluationRunState } from "./shared/model-evaluation-run-state";
 import type { VariantAnalysisManager } from "../variant-analysis/variant-analysis-manager";
+import type { ModelExtensionFile } from "./model-extension-file";
+import { modelExtensionFileToYaml } from "./yaml";
+import { outputFile } from "fs-extra";
+import { join } from "path";

 export class ModelEditorView extends AbstractWebview<
  ToModelEditorMessage,
@@ -264,6 +272,7 @@ export class ModelEditorView extends AbstractWebview<
                modeledMethods,
                mode,
                this.cliServer,
+                this.modelConfig,
                this.app.logger,
              );

@@ -483,6 +492,7 @@ export class ModelEditorView extends AbstractWebview<
        this.extensionPack,
        this.language,
        this.cliServer,
+        this.modelConfig,
        this.app.logger,
      );
      this.modelingStore.setModeledMethods(this.databaseItem, modeledMethods);
@@ -648,14 +658,18 @@ export class ModelEditorView extends AbstractWebview<
          await runGenerateQueries({
            queryConstraints: modelGeneration.queryConstraints(mode),
            filterQueries: modelGeneration.filterQueries,
-            parseResults: (queryPath, results) =>
-              modelGeneration.parseResults(
+            onResults: async (queryPath, results) => {
+              const modeledMethods = modelGeneration.parseResults(
                queryPath,
                results,
                modelsAsDataLanguage,
                this.app.logger,
-              ),
-            onResults: async (modeledMethods) => {
+                {
+                  mode,
+                  config: this.modelConfig,
+                },
+              );
+
              this.addModeledMethodsFromArray(modeledMethods);
            },
            cliServer: this.cliServer,
@@ -681,15 +695,17 @@ export class ModelEditorView extends AbstractWebview<

  protected async generateModeledMethodsOnStartup(): Promise<void> {
    const mode = this.modelingStore.getMode(this.databaseItem);
-    if (mode !== Mode.Framework) {
+    const modelsAsDataLanguage = getModelsAsDataLanguage(this.language);
+    const autoModelGeneration = modelsAsDataLanguage.autoModelGeneration;
+
+    if (autoModelGeneration === undefined) {
      return;
    }

-    const modelsAsDataLanguage = getModelsAsDataLanguage(this.language);
-    const modelGeneration = modelsAsDataLanguage.modelGeneration;
-    const autoRun = modelGeneration?.autoRun;
-
-    if (modelGeneration === undefined || autoRun === undefined) {
+    if (
+      autoModelGeneration.enabled &&
+      !autoModelGeneration.enabled({ mode, config: this.modelConfig })
+    ) {
      return;
    }

@@ -701,22 +717,23 @@ export class ModelEditorView extends AbstractWebview<
          message: "Generating models",
        });

-        const parseResults =
-          autoRun.parseResults ?? modelGeneration.parseResults;
+        const extensionFile: ModelExtensionFile = {
+          extensions: [],
+        };

        try {
          await runGenerateQueries({
-            queryConstraints: modelGeneration.queryConstraints(mode),
-            filterQueries: modelGeneration.filterQueries,
-            parseResults: (queryPath, results) =>
-              parseResults(
+            queryConstraints: autoModelGeneration.queryConstraints(mode),
+            filterQueries: autoModelGeneration.filterQueries,
+            onResults: (queryPath, results) => {
+              const extensions = autoModelGeneration.parseResultsToYaml(
                queryPath,
                results,
                modelsAsDataLanguage,
                this.app.logger,
-              ),
-            onResults: async (modeledMethods) => {
-              this.addModeledMethodsFromArray(modeledMethods);
+              );
+
+              extensionFile.extensions.push(...extensions);
            },
            cliServer: this.cliServer,
            queryRunner: this.queryRunner,
@@ -733,7 +750,25 @@ export class ModelEditorView extends AbstractWebview<
              asError(e),
            )`Failed to auto-run generating models: ${getErrorMessage(e)}`,
          );
+          return;
        }
+
+        progress({
+          step: 4000,
+          maxStep: 4000,
+          message: "Saving generated models",
+        });
+
+        const fileContents = `# This file was automatically generated from ${this.databaseItem.name}. Manual changes will not persist.\n\n${modelExtensionFileToYaml(extensionFile)}`;
+        const filePath = join(
+          this.extensionPack.path,
+          "models",
+          `${this.language}${GENERATED_MODELS_SUFFIX}`,
+        );
+
+        await outputFile(filePath, fileContents);
+
+        void this.app.logger.log(`Saved generated model file to ${filePath}`);
      },
      {
        cancellable: false,
--- a/extensions/ql-vscode/src/model-editor/modeled-method-fs.ts
+++ b/extensions/ql-vscode/src/model-editor/modeled-method-fs.ts
@@ -12,6 +12,9 @@ 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";

 export async function saveModeledMethods(
  extensionPack: ExtensionPack,
@@ -20,12 +23,14 @@ 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,
  );

@@ -48,9 +53,14 @@ 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);
+  const modelFiles = await listModelFiles(
+    extensionPack.path,
+    cliServer,
+    modelConfig,
+  );

  const modeledMethodsByFile: Record<
    string,
@@ -82,6 +92,7 @@ export async function loadModeledMethods(
  extensionPack: ExtensionPack,
  language: QueryLanguage,
  cliServer: CodeQLCliServer,
+  modelConfig: ModelConfig,
  logger: NotificationLogger,
 ): Promise<Record<string, ModeledMethod[]>> {
  const existingModeledMethods: Record<string, ModeledMethod[]> = {};
@@ -90,6 +101,7 @@ export async function loadModeledMethods(
    extensionPack,
    language,
    cliServer,
+    modelConfig,
    logger,
  );
  for (const modeledMethods of Object.values(modeledMethodsByFile)) {
@@ -108,6 +120,7 @@ export async function loadModeledMethods(
 export async function listModelFiles(
  extensionPackPath: string,
  cliServer: CodeQLCliServer,
+  modelConfig: ModelConfig,
 ): Promise<Set<string>> {
  const result = await cliServer.resolveExtensions(
    extensionPackPath,
@@ -118,6 +131,14 @@ 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)
+        ) {
+          continue;
+        }
+
        modelFiles.add(relative(extensionPackPath, extension.file));
      }
    }
--- a/extensions/ql-vscode/src/model-editor/yaml.ts
+++ b/extensions/ql-vscode/src/model-editor/yaml.ts
@@ -337,7 +337,7 @@ function validateModelExtensionFile(data: unknown): data is ModelExtensionFile {
 *
 * @param data The data extension file
 */
-function modelExtensionFileToYaml(data: ModelExtensionFile) {
+export function modelExtensionFileToYaml(data: ModelExtensionFile) {
  const extensions = data.extensions
    .map((extension) => {
      const data =
--- a/extensions/ql-vscode/test/unit-tests/model-editor/languages/ruby/generate.test.ts
+++ b/extensions/ql-vscode/test/unit-tests/model-editor/languages/ruby/generate.test.ts
@@ -4,6 +4,8 @@ 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 () => {
@@ -76,6 +78,13 @@ describe("parseGenerateModelResults", () => {
      bqrs,
      ruby,
      createMockLogger(),
+      {
+        mode: Mode.Framework,
+        config: {
+          ...defaultModelConfig,
+          showTypeModels: true,
+        },
+      },
    );
    expect(result).toEqual([
      {
--- a/extensions/ql-vscode/test/vscode-tests/cli-integration/model-editor/modeled-method-fs.test.ts
+++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/model-editor/modeled-method-fs.test.ts
@@ -13,6 +13,7 @@ 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
@@ -135,7 +136,11 @@ 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);
+      const modelFiles = await listModelFiles(
+        extensionPackPath,
+        cli,
+        defaultModelConfig,
+      );
      expect(modelFiles).toEqual(new Set());
    });

@@ -145,7 +150,11 @@ describe("modeled-method-fs", () => {
        "library2.model.yml",
      ]);

-      const modelFiles = await listModelFiles(extensionPackPath, cli);
+      const modelFiles = await listModelFiles(
+        extensionPackPath,
+        cli,
+        defaultModelConfig,
+      );
      expect(modelFiles).toEqual(
        new Set([
          join("models", "library1.model.yml"),
@@ -154,13 +163,57 @@ describe("modeled-method-fs", () => {
      );
    });

+    it("should ignore generated type models when type models are hidden", async () => {
+      const extensionPackPath = writeExtensionPackFiles("extension-pack", [
+        "library1.model.yml",
+        "library2.model.yml",
+        "library.model.generated.yml",
+      ]);
+
+      const modelFiles = await listModelFiles(
+        extensionPackPath,
+        cli,
+        defaultModelConfig,
+      );
+      expect(modelFiles).toEqual(
+        new Set([
+          join("models", "library1.model.yml"),
+          join("models", "library2.model.yml"),
+        ]),
+      );
+    });
+
+    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);
+      const modelFiles = await listModelFiles(
+        extensionPackPath,
+        cli,
+        defaultModelConfig,
+      );
      expect(modelFiles).toEqual(
        new Set([join("models", "library1.model.yml")]),
      );
@@ -177,6 +230,7 @@ describe("modeled-method-fs", () => {
        makeExtensionPack(extensionPackPath),
        QueryLanguage.Java,
        cli,
+        defaultModelConfig,
        extLogger,
      );

--- a/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/generate.test.ts
+++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/model-editor/generate.test.ts
@@ -14,6 +14,7 @@ import { ruby } from "../../../../src/model-editor/languages/ruby";
 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("runGenerateQueries", () => {
  const modelsAsDataLanguage = ruby;
@@ -128,14 +129,23 @@ describe("runGenerateQueries", () => {
    await runGenerateQueries({
      queryConstraints: modelGeneration.queryConstraints(Mode.Framework),
      filterQueries: modelGeneration.filterQueries,
-      parseResults: (queryPath, results) =>
-        modelGeneration.parseResults(
-          queryPath,
-          results,
-          modelsAsDataLanguage,
-          createMockLogger(),
-        ),
-      onResults,
+      onResults: (queryPath, results) => {
+        onResults(
+          modelGeneration.parseResults(
+            queryPath,
+            results,
+            modelsAsDataLanguage,
+            createMockLogger(),
+            {
+              mode: Mode.Framework,
+              config: {
+                ...defaultModelConfig,
+                showTypeModels: true,
+              },
+            },
+          ),
+        );
+      },
      ...options,
    });
    expect(onResults).toHaveBeenCalledWith([