Packaging: Address review comments

1. Better malformed data guard for PackDownloadOutput
2. Fix Packs type
3. Remove TODO in init-action

src/analysis-paths.test.ts

@@ -3,7 +3,6 @@ import * as path from "path";
 import test from "ava";
 
 import * as analysisPaths from "./analysis-paths";
-import { Packs } from "./config-utils";
 import { setupTests } from "./testing-utils";
 import * as util from "./util";
 
@@ -22,7 +21,7 @@ test("emptyPaths", async (t) => {
       codeQLCmd: "",
       gitHubVersion: { type: util.GitHubVariant.DOTCOM } as util.GitHubVersion,
       dbLocation: path.resolve(tmpDir, "codeql_databases"),
-      packs: {} as Packs,
+      packs: {},
     };
     analysisPaths.includeAndExcludeAnalysisPaths(config);
     t.is(process.env["LGTM_INDEX_INCLUDE"], undefined);
@@ -44,7 +43,7 @@ test("nonEmptyPaths", async (t) => {
       codeQLCmd: "",
       gitHubVersion: { type: util.GitHubVariant.DOTCOM } as util.GitHubVersion,
       dbLocation: path.resolve(tmpDir, "codeql_databases"),
-      packs: {} as Packs,
+      packs: {},
     };
     analysisPaths.includeAndExcludeAnalysisPaths(config);
     t.is(process.env["LGTM_INDEX_INCLUDE"], "path1\npath2");
@@ -70,7 +69,7 @@ test("exclude temp dir", async (t) => {
       codeQLCmd: "",
       gitHubVersion: { type: util.GitHubVariant.DOTCOM } as util.GitHubVersion,
       dbLocation: path.resolve(tempDir, "codeql_databases"),
-      packs: {} as Packs,
+      packs: {},
     };
     analysisPaths.includeAndExcludeAnalysisPaths(config);
     t.is(process.env["LGTM_INDEX_INCLUDE"], undefined);

src/analyze.test.ts

@@ -8,7 +8,7 @@ import sinon from "sinon";
 
 import { runQueries } from "./analyze";
 import { setCodeQL } from "./codeql";
-import { Config, Packs } from "./config-utils";
+import { Config } from "./config-utils";
 import { getIdPrefix } from "./count-loc";
 import * as count from "./count-loc";
 import { Language } from "./languages";
@@ -39,19 +39,20 @@ test("status report fields and search path setting", async (t) => {
       [Language.cpp]: [
         {
           packName: "a/b",
-          version: clean("1.0.0"),
+          version: clean("1.0.0")!,
         },
       ],
       [Language.java]: [
         {
           packName: "c/d",
-          version: clean("2.0.0"),
+          version: clean("2.0.0")!,
         },
       ],
-    } as Packs;
+    };
 
     for (const language of Object.values(Language)) {
       setCodeQL({
+        packDownload: async () => ({ packs: [] }),
         databaseAnalyze: async (
           _,
           sarifFile: string,

src/codeql.ts

@@ -796,10 +796,18 @@ function getCodeQLForCmd(cmd: string): CodeQL {
       }).exec();
 
       try {
-        return JSON.parse(output) as PackDownloadOutput;
+        const parsedOutput: PackDownloadOutput = JSON.parse(output);
+        if (
+          Array.isArray(parsedOutput.packs) &&
+          parsedOutput.packs.every((p) => p.name && p.version)
+        ) {
+          return parsedOutput;
+        } else {
+          throw new Error("Unexpected output from pack download");
+        }
       } catch (e) {
         throw new Error(
-          `Attempted to download specified packs but got an error:${"\n"}${output}.`
+          `Attempted to download specified packs but got an error:\n${output}\n${e}`
         );
       }
     },

src/config-utils.ts

@@ -127,7 +127,7 @@ export interface Config {
   packs: Packs;
 }
 
-export type Packs = Record<Partial<Language>, PackWithVersion[]>;
+export type Packs = Partial<Record<Language, PackWithVersion[]>>;
 
 export interface PackWithVersion {
   /** qualified name of a package reference */
@@ -1032,8 +1032,8 @@ export function parsePacks(
   packsByLanguage: string[] | Record<string, string[]> | undefined,
   languages: Language[],
   configFile: string
-) {
-  const packs = {} as Packs;
+): Packs {
+  const packs = {};
 
   if (!packsByLanguage) {
     return packs;
@@ -1071,6 +1071,7 @@ function toPackWithVersion(packStr, configFile: string): PackWithVersion {
   if (typeof packStr !== "string") {
     throw new Error(getPacksStrInvalid(packStr, configFile));
   }
+
   const nameWithVersion = packStr.split("@");
   let version: string | undefined;
   if (
@@ -1158,7 +1159,7 @@ export async function initConfig(
   for (const language of config.languages) {
     const hasBuiltinQueries = config.queries[language]?.builtin.length > 0;
     const hasCustomQueries = config.queries[language]?.custom.length > 0;
-    const hasPacks = config.packs[language]?.length > 0;
+    const hasPacks = (config.packs[language]?.length || 0) > 0;
     if (!hasPacks && !hasBuiltinQueries && !hasCustomQueries) {
       throw new Error(
         `Did not detect any queries to run for ${language}. ` +

src/init-action.ts

@@ -1,9 +1,4 @@
-import * as fs from "fs";
-import * as os from "os";
-import * as path from "path";
-
 import * as core from "@actions/core";
-import * as yaml from "js-yaml";
 
 import {
   createStatusReportBase,
@@ -182,28 +177,6 @@ async function run() {
         );
       }
     }
-
-    ////////////////////////////////
-    // TODO This should not happen in the action, we should be able to
-    // generate the default qlconfig from the CLI
-    // DO NOT COMMIT THIS
-    const defaultQlConfig = {
-      registryKind: "docker",
-      registries: [
-        {
-          url: "https://ghcr.io/v2/",
-          packages: "*",
-        },
-      ],
-    };
-
-    fs.mkdirSync(path.join(os.homedir(), ".codeql"));
-    fs.writeFileSync(
-      path.join(os.homedir(), ".codeql", "qlconfig.yml"),
-      yaml.safeDump(defaultQlConfig),
-      "utf8"
-    );
-    ////////////////////////////////
   } catch (e) {
     core.setFailed(e.message);
     console.log(e);