Merge pull request #1116 from github/aeisenberg/multi-lang-packs

Allow scans with packs for languages not being scanned

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [UNRELEASED]
 
+- CodeQL query packs listed in the `packs` configuration field will be skipped if their target language is not being analyzed in the current Actions job. Previously, this would throw an error. [#1116](https://github.com/github/codeql-action/pull/1116)
 - The combination of python2 and poetry is no longer supported. See https://github.com/actions/setup-python/issues/374 for more details. [#1124](https://github.com/github/codeql-action/pull/1124)
 
 ## 2.1.14 - 22 Jun 2022

src/config-utils.test.ts

@@ -10,7 +10,7 @@ import { getCachedCodeQL, setCodeQL } from "./codeql";
 import * as configUtils from "./config-utils";
 import { createFeatureFlags, FeatureFlag } from "./feature-flags";
 import { Language } from "./languages";
-import { getRunnerLogger } from "./logging";
+import { getRunnerLogger, Logger } from "./logging";
 import { setupTests } from "./testing-utils";
 import * as util from "./util";
 
@@ -1424,7 +1424,12 @@ const parsePacksMacro = test.macro({
     expected: Partial<Record<Language, string[]>>
   ) =>
     t.deepEqual(
-      configUtils.parsePacksFromConfig(packsByLanguage, languages, "/a/b"),
+      configUtils.parsePacksFromConfig(
+        packsByLanguage,
+        languages,
+        "/a/b",
+        mockLogger
+      ),
       expected
     ),
 
@@ -1446,7 +1451,8 @@ const parsePacksErrorMacro = test.macro({
         configUtils.parsePacksFromConfig(
           packsByLanguage as string[] | Record<string, string[]>,
           languages,
-          "/a/b"
+          "/a/b",
+          {} as Logger
         ),
       {
         message: expected,
@@ -1499,6 +1505,19 @@ test(
   }
 );
 
+test(
+  "two packs with unused language in config",
+  parsePacksMacro,
+  {
+    [Language.cpp]: ["a/b", "c/d@1.2.3"],
+    [Language.java]: ["d/e", "f/g@1.2.3"],
+  },
+  [Language.cpp, Language.csharp],
+  {
+    [Language.cpp]: ["a/b", "c/d@1.2.3"],
+  }
+);
+
 test(
   "packs with other valid names",
   parsePacksMacro,
@@ -1544,13 +1563,6 @@ test(
   [Language.java, Language.python],
   /The configuration file "\/a\/b" is invalid: property "packs" must split packages by language/
 );
-test(
-  "invalid language",
-  parsePacksErrorMacro,
-  { [Language.java]: ["c/d"] },
-  [Language.cpp],
-  /The configuration file "\/a\/b" is invalid: property "packs" has "java", but it is not one of the languages to analyze/
-);
 test(
   "not an array",
   parsePacksErrorMacro,
@@ -1583,13 +1595,25 @@ function parseInputAndConfigMacro(
   expected
 ) {
   t.deepEqual(
-    configUtils.parsePacks(packsFromConfig, packsFromInput, languages, "/a/b"),
+    configUtils.parsePacks(
+      packsFromConfig,
+      packsFromInput,
+      languages,
+      "/a/b",
+      mockLogger
+    ),
     expected
   );
 }
 parseInputAndConfigMacro.title = (providedTitle: string) =>
   `Parse Packs input and config: ${providedTitle}`;
 
+const mockLogger = {
+  info: (message: string) => {
+    console.log(message);
+  },
+} as Logger;
+
 function parseInputAndConfigErrorMacro(
   t: ExecutionContext<unknown>,
   packsFromConfig: string[] | Record<string, string[]>,
@@ -1603,7 +1627,8 @@ function parseInputAndConfigErrorMacro(
         packsFromConfig,
         packsFromInput,
         languages,
-        "/a/b"
+        "/a/b",
+        mockLogger
       );
     },
     {

src/config-utils.ts

@@ -629,14 +629,11 @@ export function getPathsInvalid(configFile: string): string {
   );
 }
 
-export function getPacksRequireLanguage(
-  lang: string,
-  configFile: string
-): string {
+function getPacksRequireLanguage(lang: string, configFile: string): string {
   return getConfigFilePropertyError(
     configFile,
     PACKS_PROPERTY,
-    `has "${lang}", but it is not one of the languages to analyze`
+    `has "${lang}", but it is not a valid language.`
   );
 }
 
@@ -1026,7 +1023,8 @@ async function loadConfig(
     parsedYAML[PACKS_PROPERTY] ?? {},
     packsInput,
     languages,
-    configFile
+    configFile,
+    logger
   );
 
   // If queries were provided using `with` in the action configuration,
@@ -1146,7 +1144,8 @@ const PACK_IDENTIFIER_PATTERN = (function () {
 export function parsePacksFromConfig(
   packsByLanguage: string[] | Record<string, string[]>,
   languages: Language[],
-  configFile: string
+  configFile: string,
+  logger: Logger
 ): Packs {
   const packs = {};
 
@@ -1168,7 +1167,16 @@ export function parsePacksFromConfig(
       throw new Error(getPacksInvalid(configFile));
     }
     if (!languages.includes(lang as Language)) {
-      throw new Error(getPacksRequireLanguage(lang, configFile));
+      // This particular language is not being analyzed in this run.
+      if (Language[lang as Language]) {
+        logger.info(
+          `Ignoring packs for ${lang} since this language is not being analyzed in this run.`
+        );
+        continue;
+      } else {
+        // This language is invalid, probably a misspelling
+        throw new Error(getPacksRequireLanguage(configFile, lang));
+      }
     }
     packs[lang] = [];
     for (const packStr of packsArr) {
@@ -1296,13 +1304,15 @@ export function parsePacks(
   rawPacksFromConfig: string[] | Record<string, string[]>,
   rawPacksInput: string | undefined,
   languages: Language[],
-  configFile: string
+  configFile: string,
+  logger: Logger
 ) {
   const packsFromInput = parsePacksFromInput(rawPacksInput, languages);
   const packsFomConfig = parsePacksFromConfig(
     rawPacksFromConfig,
     languages,
-    configFile
+    configFile,
+    logger
   );
 
   if (!packsFromInput) {

tests/multi-language-repo/.github/codeql/codeql-config-packaging.yml

@@ -6,6 +6,9 @@ packs:
     - dsp-testing/codeql-pack1@1.0.0
     - dsp-testing/codeql-pack2
     - dsp-testing/codeql-pack3:other-query.ql
+  ruby:
+    - dsp-testing/hucairz
+    - dsp-testing/i-dont-exist@1.0.0
 
 paths-ignore:
   - tests