Merge pull request #4184 from github/henrymercer/per-language-pr-check-failures

Use the combined bundle when queries may need other languages' library packs

Author: henrymercer

.github/workflows/__export-file-baseline-information.yml

@@ -11,6 +11,10 @@ installDotNet: true
 env:
   CODEQL_ACTION_SKIP_FILE_COVERAGE_ON_PRS: false
   CODEQL_ACTION_SUBLANGUAGE_FILE_COVERAGE: true
+  # To balance speed and coverage, we analyze only a single language (JavaScript), but use the
+  # combined bundle so we can test that baseline information is reported for each language in the
+  # multi-language source directory.
+  CODEQL_ACTION_PER_LANGUAGE_BUNDLES: false
 steps:
   - uses: ./../action/init
     id: init

lib/entry-points.js

@@ -21,19 +21,16 @@ inputs:
     required: false
   languages:
     description: >-
-      A comma-separated list of CodeQL languages that will be analyzed in subsequent
-      `github/codeql-action/init` and `github/codeql-action/analyze` invocations. If specified, the
-      Action may use this list to select a CodeQL CLI version that is best suited to analyzing those
-      languages, for example by preferring a version that has a cached overlay-base database for the
-      specified languages. This input is not remembered and must also be passed to
-      `github/codeql-action/init`.
+      A comma-separated list of CodeQL languages that the installed CodeQL CLI will be used to
+      analyze. If specified, the Action may use this list to select a CodeQL CLI version that is
+      best suited to analyzing those languages, for example by preferring a version that has a
+      cached overlay-base database for the specified languages.
     required: false
   analysis-kinds:
     description: >-
-      [Internal] A comma-separated list of analysis kinds that subsequent
-      `github/codeql-action/init` invocations will enable. If specified, the Action may use this
-      list to select a CodeQL CLI version that is best suited to those analysis kinds. This input is
-      not remembered and must also be passed to `github/codeql-action/init`.
+      [Internal] A comma-separated list of analysis kinds that the installed CodeQL CLI will be used
+      for. If specified, the Action may use this list to select a CodeQL CLI version that is best
+      suited to those analysis kinds.
 
       Available options are the same as for the `analysis-kinds` input on the `init` Action.
     default: 'code-scanning'

pr-checks/checks/export-file-baseline-information.yml

@@ -7,12 +7,12 @@ import * as sinon from "sinon";
 import { CodeQuality, CodeScanning, RiskAssessment } from "./analyses";
 import {
   runQueries,
-  defaultSuites,
   resolveQuerySuiteAlias,
   addSarifExtension,
   diffRangeExtensionPackContents,
 } from "./analyze";
 import { createStubCodeQL } from "./codeql";
+import { defaultSuites } from "./config/db-config";
 import { Feature } from "./feature-flags";
 import { BuiltInLanguage } from "./languages";
 import { getRunnerLogger } from "./logging";

setup-codeql/action.yml

@@ -9,6 +9,7 @@ import { getTemporaryDirectory } from "./actions-util";
 import * as analyses from "./analyses";
 import { setupCppAutobuild } from "./autobuild";
 import { type CodeQL } from "./codeql";
+import { defaultSuites } from "./config/db-config";
 import * as configUtils from "./config-utils";
 import {
   getCsharpTempDependencyDir,
@@ -357,15 +358,6 @@ dataExtensions:
   return diffRangeDir;
 }
 
-// A set of default query suite names that are understood by the CLI.
-export const defaultSuites: Set<string> = new Set([
-  "security-experimental",
-  "security-extended",
-  "security-and-quality",
-  "code-quality",
-  "code-scanning",
-]);
-
 /**
  * If `maybeSuite` is the name of a default query suite, it is resolved into the corresponding
  * query suite name for the given `language`. Otherwise, `maybeSuite` is returned as is.

src/analyze.test.ts

@@ -99,6 +99,7 @@ async function installIntoToolcache({
       ? { enabledVersions: [{ cliVersion, tagName }] }
       : SAMPLE_DEFAULT_CLI_VERSION,
     undefined, // rawLanguages
+    undefined, // otherLanguagePacksReason
     false, // useOverlayAwareDefaultCliVersion
     createFeatures([]),
     getRunnerLogger(true),
@@ -172,6 +173,7 @@ test.serial(
           util.GitHubVariant.DOTCOM,
           SAMPLE_DEFAULT_CLI_VERSION,
           undefined, // rawLanguages
+          undefined, // otherLanguagePacksReason
           false, // useOverlayAwareDefaultCliVersion
           features,
           getRunnerLogger(true),
@@ -207,6 +209,7 @@ test.serial(
         util.GitHubVariant.DOTCOM,
         SAMPLE_DEFAULT_CLI_VERSION,
         undefined, // rawLanguages
+        undefined, // otherLanguagePacksReason
         false, // useOverlayAwareDefaultCliVersion
         features,
         getRunnerLogger(true),
@@ -246,6 +249,7 @@ test.serial(
         util.GitHubVariant.DOTCOM,
         SAMPLE_DEFAULT_CLI_VERSION,
         undefined, // rawLanguages
+        undefined, // otherLanguagePacksReason
         false, // useOverlayAwareDefaultCliVersion
         features,
         getRunnerLogger(true),
@@ -355,6 +359,7 @@ for (const {
           util.GitHubVariant.DOTCOM,
           SAMPLE_DEFAULT_CLI_VERSION,
           undefined, // rawLanguages
+          undefined, // otherLanguagePacksReason
           false, // useOverlayAwareDefaultCliVersion
           features,
           getRunnerLogger(true),
@@ -397,6 +402,7 @@ for (const toolcacheVersion of [
           util.GitHubVariant.DOTCOM,
           SAMPLE_DEFAULT_CLI_VERSION,
           undefined, // rawLanguages
+          undefined, // otherLanguagePacksReason
           false, // useOverlayAwareDefaultCliVersion
           features,
           getRunnerLogger(true),
@@ -441,6 +447,7 @@ test.serial(
           ],
         },
         undefined, // rawLanguages
+        undefined, // otherLanguagePacksReason
         false, // useOverlayAwareDefaultCliVersion
         features,
         getRunnerLogger(true),
@@ -487,6 +494,7 @@ test.serial(
           ],
         },
         undefined, // rawLanguages
+        undefined, // otherLanguagePacksReason
         false, // useOverlayAwareDefaultCliVersion
         features,
         getRunnerLogger(true),
@@ -526,6 +534,7 @@ test.serial(
         util.GitHubVariant.DOTCOM,
         SAMPLE_DEFAULT_CLI_VERSION,
         undefined, // rawLanguages
+        undefined, // otherLanguagePacksReason
         false, // useOverlayAwareDefaultCliVersion
         features,
         getRunnerLogger(true),
@@ -567,6 +576,7 @@ test.serial(
         util.GitHubVariant.DOTCOM,
         SAMPLE_DEFAULT_CLI_VERSION,
         undefined, // rawLanguages
+        undefined, // otherLanguagePacksReason
         false, // useOverlayAwareDefaultCliVersion
         features,
         getRunnerLogger(true),

src/analyze.ts

@@ -302,6 +302,9 @@ export function isDiskConfigurationError(e: unknown): boolean {
  * @param variant
  * @param defaultCliVersion
  * @param rawLanguages Raw set of languages.
+ * @param otherLanguagePacksReason Why the CodeQL CLI may need packs for languages other than
+ *   `rawLanguages`, or `undefined` if it won't. If defined, the combined bundle is used. See
+ *   `PerLanguageBundleOptions.otherLanguagePacksReason`.
  * @param useOverlayAwareDefaultCliVersion Whether to select an overlay-aware default CLI version.
  * @param features Information about the features that are enabled.
  * @param logger
@@ -316,6 +319,7 @@ export async function setupCodeQL(
   variant: util.GitHubVariant,
   defaultCliVersion: CodeQLDefaultVersionInfo,
   rawLanguages: string[] | undefined,
+  otherLanguagePacksReason: string | undefined,
   useOverlayAwareDefaultCliVersion: boolean,
   features: FeatureEnablement,
   logger: Logger,
@@ -339,6 +343,7 @@ export async function setupCodeQL(
       variant,
       defaultCliVersion,
       rawLanguages,
+      otherLanguagePacksReason,
       useOverlayAwareDefaultCliVersion,
       features,
       logger,

src/codeql.test.ts

@@ -473,6 +473,12 @@ const simpleConfigFileContents = `
   queries:
     - uses: ./foo_file`;
 
+/** The configuration in `simpleConfigFileContents`. */
+const simpleConfigInput: UserConfig = {
+  name: "my config",
+  queries: [{ uses: "./foo_file" }],
+};
+
 /** A less minimal configuration file. */
 const otherConfigFileContents = `
   name: my config
@@ -558,16 +564,14 @@ test.serial(
         tempDir,
       );
 
-      const configInput = `
-      name: my config
-      queries:
-        - uses: ./foo
-      packs:
-        javascript:
-          - a/b@1.2.3
-        python:
-          - c/d@1.2.3
-    `;
+      const configInput: UserConfig = {
+        name: "my config",
+        queries: [{ uses: "./foo" }],
+        packs: {
+          javascript: ["a/b@1.2.3"],
+          python: ["c/d@1.2.3"],
+        },
+      };
 
       fs.mkdirSync(path.join(tempDir, "foo"));
 
@@ -598,7 +602,16 @@ test.serial(
         }),
       );
 
-      t.deepEqual(config.originalUserInput, yaml.load(configInput));
+      // Compare with a separate object rather than `configInput` itself, so that the test catches
+      // changes made to the input in place.
+      t.deepEqual(config.originalUserInput, {
+        name: "my config",
+        queries: [{ uses: "./foo" }],
+        packs: {
+          javascript: ["a/b@1.2.3"],
+          python: ["c/d@1.2.3"],
+        },
+      });
     });
   },
 );
@@ -2413,7 +2426,7 @@ test("determineUserConfig - loads config input", async (t) => {
     const expectedConfigPath = configUtils.userConfigFromActionPath(tmpDir);
 
     const inputs = createTestInitConfigInputs({
-      configInput: simpleConfigFileContents,
+      configInput: simpleConfigInput,
       configFile: undefined,
       workspacePath: tmpDir,
     });
@@ -2423,17 +2436,15 @@ test("determineUserConfig - loads config input", async (t) => {
 
     await target
       // The input source and path of the generated config file should have been logged.
-      .logs(
-        t,
-        "Using config from action input:",
-        `Using configuration file: ${expectedConfigPath}`,
-      )
-      // The message about no configuration input and
-      // the warning about both inputs should not have been logged.
+      .logs(t, `Using config from action input: ${expectedConfigPath}`)
+      // The message about no configuration input and the warning about both inputs should not have
+      // been logged. The generated config file isn't loaded, since the `config` input has already
+      // been parsed.
       .notLogs(
         t,
         "No configuration file was provided",
         "Both a config file and config input were provided. Ignoring config file.",
+        `Using configuration file: ${expectedConfigPath}`,
       )
       // The loaded configuration should match `simpleConfigFileContents`.
       .passes(t.deepEqual, {
@@ -2452,7 +2463,7 @@ test("determineUserConfig - ignores config file input when both specified", asyn
     const expectedConfigPath = configUtils.userConfigFromActionPath(tmpDir);
 
     const inputs = createTestInitConfigInputs({
-      configInput: simpleConfigFileContents,
+      configInput: simpleConfigInput,
       configFile: configFilePath,
       workspacePath: tmpDir,
     });
@@ -2466,10 +2477,14 @@ test("determineUserConfig - ignores config file input when both specified", asyn
       .logs(
         t,
         `Using config from action input: ${expectedConfigPath}`,
-        `Using configuration file: ${expectedConfigPath}`,
         "Both a config file and config input were provided. Ignoring config file.",
       )
-      .notLogs(t, "No configuration file was provided")
+      // The generated config file isn't loaded, since the `config` input has already been parsed.
+      .notLogs(
+        t,
+        "No configuration file was provided",
+        `Using configuration file: ${expectedConfigPath}`,
+      )
       // The loaded configuration should match `simpleConfigFileContents`.
       .passes(t.deepEqual, {
         name: "my config",
@@ -2482,11 +2497,14 @@ test("determineUserConfig - ignores config file input when both specified", asyn
 });
 
 /** A `config` input that we might get from Default Setup. */
-const defaultSetupConfigInput = `
-  threat-models: [local, remote]
-  default-setup:
-    org:
-      model-packs: [foo, bar]`;
+const defaultSetupConfigInput: UserConfig = {
+  "threat-models": ["local", "remote"],
+  "default-setup": {
+    org: {
+      "model-packs": ["foo", "bar"],
+    },
+  },
+};
 
 test("determineUserConfig - merges configs if FF is enabled in Default Setup", async (t) => {
   await withTmpDir(async (tmpDir) => {
@@ -2555,7 +2573,7 @@ test("determineUserConfig - ignores config file input in Default Setup if FF is
       .withArgs(
         tmpDir,
         createTestInitConfigInputs({
-          configInput: simpleConfigFileContents,
+          configInput: simpleConfigInput,
           configFile: configFilePath,
           workspacePath: tmpDir,
         }),
@@ -2565,10 +2583,14 @@ test("determineUserConfig - ignores config file input in Default Setup if FF is
       .logs(
         t,
         `Using config from action input: ${expectedConfigPath}`,
-        `Using configuration file: ${expectedConfigPath}`,
         "Both a config file and config input were provided. Ignoring config file.",
       )
-      .notLogs(t, "No configuration file was provided")
+      // The generated config file isn't loaded, since the `config` input has already been parsed.
+      .notLogs(
+        t,
+        "No configuration file was provided",
+        `Using configuration file: ${expectedConfigPath}`,
+      )
       .passes(t.deepEqual, {
         name: "my config",
         queries: [{ uses: "./foo_file" }],
@@ -2587,7 +2609,7 @@ test("determineUserConfig - ignores config file input outside Default Setup if F
       .withArgs(
         tmpDir,
         createTestInitConfigInputs({
-          configInput: simpleConfigFileContents,
+          configInput: simpleConfigInput,
           configFile: configFilePath,
           workspacePath: tmpDir,
         }),
@@ -2597,10 +2619,14 @@ test("determineUserConfig - ignores config file input outside Default Setup if F
       .logs(
         t,
         `Using config from action input: ${expectedConfigPath}`,
-        `Using configuration file: ${expectedConfigPath}`,
         "Both a config file and config input were provided. Ignoring config file.",
       )
-      .notLogs(t, "No configuration file was provided")
+      // The generated config file isn't loaded, since the `config` input has already been parsed.
+      .notLogs(
+        t,
+        "No configuration file was provided",
+        `Using configuration file: ${expectedConfigPath}`,
+      )
       .passes(t.deepEqual, {
         name: "my config",
         queries: [{ uses: "./foo_file" }],