Merge pull request #3287 from github/robertbrignull/move_language_pack

robertbrignull · web-flow · commit 0391f972ad9f · 2024-01-30T15:22:06.000Z
Move language and pack to top level of variant analysis object
diff --git a/extensions/ql-vscode/src/query-history/history-item-label-provider.ts b/extensions/ql-vscode/src/query-history/history-item-label-provider.ts
@@ -101,7 +101,7 @@ export class HistoryItemLabelProvider {
       t: new Date(item.variantAnalysis.executionStartTime).toLocaleString(
         env.language,
       ),
-      q: `${item.variantAnalysis.query.name} (${item.variantAnalysis.query.language})`,
+      q: `${item.variantAnalysis.query.name} (${item.variantAnalysis.language})`,
       d: buildRepoLabel(item),
       r: resultCount,
       s: humanizeQueryStatus(item.status),
diff --git a/extensions/ql-vscode/src/query-history/query-history-info.ts b/extensions/ql-vscode/src/query-history/query-history-info.ts
@@ -55,7 +55,7 @@ export function getLanguage(item: QueryHistoryInfo): QueryLanguage | undefined {
     case "local":
       return item.initialInfo.databaseInfo.language;
     case "variant-analysis":
-      return item.variantAnalysis.query.language;
+      return item.variantAnalysis.language;
     default:
       assertNever(item);
   }
diff --git a/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-domain-mapper.ts b/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-domain-mapper.ts
@@ -56,7 +56,7 @@ function mapVariantAnalysisDtoToDto(
     query: {
       name: variantAnalysis.query.name,
       filePath: variantAnalysis.query.filePath,
-      language: mapQueryLanguageToDto(variantAnalysis.query.language),
+      language: mapQueryLanguageToDto(variantAnalysis.language),
       text: variantAnalysis.query.text,
       kind: variantAnalysis.query.kind,
     },
diff --git a/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-dto-mapper.ts b/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-dto-mapper.ts
@@ -53,10 +53,10 @@ function mapVariantAnalysisToDomainModel(
       fullName: variantAnalysis.controllerRepo.fullName,
       private: variantAnalysis.controllerRepo.private,
     },
+    language: mapQueryLanguageToDomainModel(variantAnalysis.query.language),
     query: {
       name: variantAnalysis.query.name,
       filePath: variantAnalysis.query.filePath,
-      language: mapQueryLanguageToDomainModel(variantAnalysis.query.language),
       text: variantAnalysis.query.text,
       kind: variantAnalysis.query.kind,
     },
diff --git a/extensions/ql-vscode/src/variant-analysis/export-results.ts b/extensions/ql-vscode/src/variant-analysis/export-results.ts
@@ -324,7 +324,7 @@ const buildVariantAnalysisGistDescription = (
   const repositoryLabel = summaries.length
     ? `(${pluralize(summaries.length, "repository", "repositories")})`
     : "";
-  return `${variantAnalysis.query.name} (${variantAnalysis.query.language}) ${resultLabel} ${repositoryLabel}`;
+  return `${variantAnalysis.query.name} (${variantAnalysis.language}) ${resultLabel} ${repositoryLabel}`;
 };
 
 /**
diff --git a/extensions/ql-vscode/src/variant-analysis/gh-api/gh-api-client.ts b/extensions/ql-vscode/src/variant-analysis/gh-api/gh-api-client.ts
@@ -14,13 +14,13 @@ export async function submitVariantAnalysis(
 ): Promise<VariantAnalysis> {
   const octokit = await credentials.getOctokit();
 
-  const { actionRepoRef, query, databases, controllerRepoId } =
+  const { actionRepoRef, language, pack, databases, controllerRepoId } =
     submissionDetails;
 
   const data: VariantAnalysisSubmissionRequest = {
     action_repo_ref: actionRepoRef,
-    language: query.language,
-    query_pack: query.pack,
+    language,
+    query_pack: pack,
     repositories: databases.repositories,
     repository_lists: databases.repositoryLists,
     repository_owners: databases.repositoryOwners,
diff --git a/extensions/ql-vscode/src/variant-analysis/markdown-generation.ts b/extensions/ql-vscode/src/variant-analysis/markdown-generation.ts
@@ -42,7 +42,7 @@ interface VariantAnalysisMarkdown {
  * Generates markdown files with variant analysis results.
  */
 export async function generateVariantAnalysisMarkdown(
-  variantAnalysis: Pick<VariantAnalysis, "query">,
+  variantAnalysis: Pick<VariantAnalysis, "language" | "query">,
   results: AsyncIterable<
     [VariantAnalysisScannedRepository, VariantAnalysisScannedRepositoryResult]
   >,
@@ -77,7 +77,7 @@ export async function generateVariantAnalysisMarkdown(
       for (const interpretedResult of result.interpretedResults) {
         const individualResult = generateMarkdownForInterpretedResult(
           interpretedResult,
-          variantAnalysis.query.language,
+          variantAnalysis.language,
         );
         resultsFileContent.push(...individualResult);
       }
diff --git a/extensions/ql-vscode/src/variant-analysis/shared/variant-analysis.ts b/extensions/ql-vscode/src/variant-analysis/shared/variant-analysis.ts
@@ -5,10 +5,10 @@ import { QueryLanguage } from "../../common/query-language";
 export interface VariantAnalysis {
   id: number;
   controllerRepo: Repository;
+  language: QueryLanguage;
   query: {
     name: string;
     filePath: string;
-    language: QueryLanguage;
     text: string;
     kind?: string;
   };
@@ -135,15 +135,14 @@ export interface VariantAnalysisSubmission {
   startTime: number;
   controllerRepoId: number;
   actionRepoRef: string;
+  language: QueryLanguage;
+  /** Base64 encoded query pack. */
+  pack: string;
   query: {
     name: string;
     filePath: string;
-    language: QueryLanguage;
     text: string;
     kind?: string;
-
-    // Base64 encoded query pack.
-    pack: string;
   };
   queries?: VariantAnalysisQueries;
   databases: {
diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts
@@ -412,11 +412,11 @@ export class VariantAnalysisManager
       startTime: queryStartTime,
       actionRepoRef: actionBranch,
       controllerRepoId: controllerRepo.id,
+      language: variantAnalysisLanguage,
+      pack: base64Pack,
       query: {
         name: queryName,
         filePath: firstQueryFile,
-        pack: base64Pack,
-        language: variantAnalysisLanguage,
         text: queryText,
         kind: queryMetadata?.kind,
       },
diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-mapper.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-mapper.ts
@@ -29,10 +29,10 @@ export function mapVariantAnalysis(
 ): VariantAnalysis {
   return mapUpdatedVariantAnalysis(
     {
+      language: submission.language,
       query: {
         name: submission.query.name,
         filePath: submission.query.filePath,
-        language: submission.query.language,
         text: submission.query.text,
         kind: submission.query.kind,
       },
@@ -47,7 +47,7 @@ export function mapVariantAnalysis(
 export function mapUpdatedVariantAnalysis(
   previousVariantAnalysis: Pick<
     VariantAnalysis,
-    "query" | "queries" | "databases" | "executionStartTime"
+    "language" | "query" | "queries" | "databases" | "executionStartTime"
   >,
   response: ApiVariantAnalysis,
 ): VariantAnalysis {
@@ -73,6 +73,7 @@ export function mapUpdatedVariantAnalysis(
       fullName: response.controller_repo.full_name,
       private: response.controller_repo.private,
     },
+    language: previousVariantAnalysis.language,
     query: previousVariantAnalysis.query,
     queries: previousVariantAnalysis.queries,
     databases: previousVariantAnalysis.databases,
diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts
@@ -66,7 +66,7 @@ export class VariantAnalysisMonitor extends DisposableObject {
     variantAnalysis: VariantAnalysis,
   ): Promise<void> {
     const variantAnalysisLabel = `${variantAnalysis.query.name} (${
-      variantAnalysis.query.language
+      variantAnalysis.language
     }) [${new Date(variantAnalysis.executionStartTime).toLocaleString(
       env.language,
     )}]`;
diff --git a/extensions/ql-vscode/test/factories/variant-analysis/shared/variant-analysis-submission.ts b/extensions/ql-vscode/test/factories/variant-analysis/shared/variant-analysis-submission.ts
@@ -7,13 +7,13 @@ export function createMockSubmission(): VariantAnalysisSubmission {
     startTime: faker.number.int(),
     controllerRepoId: faker.number.int(),
     actionRepoRef: "repo-ref",
+    language: QueryLanguage.Javascript,
+    pack: "base64-encoded-string",
     query: {
       name: "query-name",
       filePath: "query-file-path",
-      language: QueryLanguage.Javascript,
       text: "query-text",
       kind: "table",
-      pack: "base64-encoded-string",
     },
     databases: {
       repositories: ["1", "2", "3"],
diff --git a/extensions/ql-vscode/test/factories/variant-analysis/shared/variant-analysis.ts b/extensions/ql-vscode/test/factories/variant-analysis/shared/variant-analysis.ts
@@ -31,10 +31,10 @@ export function createMockVariantAnalysis({
         prefix: "",
       })}`,
     },
+    language,
     query: {
       name: "a-query-name",
       filePath: "a-query-file-path",
-      language,
       text: "a-query-text",
     },
     databases: {
diff --git a/extensions/ql-vscode/test/unit-tests/variant-analysis/markdown-generation.test.ts b/extensions/ql-vscode/test/unit-tests/variant-analysis/markdown-generation.test.ts
@@ -25,12 +25,12 @@ describe(generateVariantAnalysisMarkdown.name, () => {
     it("should generate markdown file for each repo with results", async () => {
       const actualFiles = await generateVariantAnalysisMarkdown(
         {
+          language: QueryLanguage.Javascript,
           query: {
             name: "Shell command built from environment values",
             filePath:
               "c:\\git-repo\\vscode-codeql-starter\\ql\\javascript\\ql\\src\\Security\\CWE-078\\ShellCommandInjectionFromEnvironment.ql",
             text: '/**\n * @name Shell command built from environment values\n * @description Building a shell command string with values from the enclosing\n *              environment may cause subtle bugs or vulnerabilities.\n * @kind path-problem\n * @problem.severity warning\n * @security-severity 6.3\n * @precision high\n * @id js/shell-command-injection-from-environment\n * @tags correctness\n *       security\n *       external/cwe/cwe-078\n *       external/cwe/cwe-088\n */\n\nimport javascript\nimport DataFlow::PathGraph\nimport semmle.javascript.security.dataflow.ShellCommandInjectionFromEnvironmentQuery\n\nfrom\n  Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, DataFlow::Node highlight,\n  Source sourceNode\nwhere\n  sourceNode = source.getNode() and\n  cfg.hasFlowPath(source, sink) and\n  if cfg.isSinkWithHighlight(sink.getNode(), _)\n  then cfg.isSinkWithHighlight(sink.getNode(), highlight)\n  else highlight = sink.getNode()\nselect highlight, source, sink, "This shell command depends on an uncontrolled $@.", sourceNode,\n  sourceNode.getSourceType()\n',
-            language: QueryLanguage.Javascript,
           },
         },
         getResults(pathProblemAnalysesResults),
@@ -49,12 +49,12 @@ describe(generateVariantAnalysisMarkdown.name, () => {
     it("should generate markdown file for each repo with results", async () => {
       const actualFiles = await generateVariantAnalysisMarkdown(
         {
+          language: QueryLanguage.Javascript,
           query: {
             name: "Inefficient regular expression",
             filePath:
               "c:\\git-repo\\vscode-codeql-starter\\ql\\javascript\\ql\\src\\Performance\\ReDoS.ql",
             text: '/**\n * @name Inefficient regular expression\n * @description A regular expression that requires exponential time to match certain inputs\n *              can be a performance bottleneck, and may be vulnerable to denial-of-service\n *              attacks.\n * @kind problem\n * @problem.severity error\n * @security-severity 7.5\n * @precision high\n * @id js/redos\n * @tags security\n *       external/cwe/cwe-1333\n *       external/cwe/cwe-730\n *       external/cwe/cwe-400\n */\n\nimport javascript\nimport semmle.javascript.security.performance.ReDoSUtil\nimport semmle.javascript.security.performance.ExponentialBackTracking\n\nfrom RegExpTerm t, string pump, State s, string prefixMsg\nwhere hasReDoSResult(t, pump, s, prefixMsg)\nselect t,\n  "This part of the regular expression may cause exponential backtracking on strings " + prefixMsg +\n    "containing many repetitions of \'" + pump + "\'."\n',
-            language: QueryLanguage.Javascript,
           },
         },
         getResults(problemAnalysesResults),
@@ -73,11 +73,11 @@ describe(generateVariantAnalysisMarkdown.name, () => {
     it("should generate markdown file for each repo with results", async () => {
       const actualFiles = await generateVariantAnalysisMarkdown(
         {
+          language: QueryLanguage.Javascript,
           query: {
             name: "Contradictory guard nodes",
             filePath: "c:\\Users\\foo\\bar\\quick-query.ql",
             text: '/**\n * @name Contradictory guard nodes\n * \n * @description Snippet from "UselessComparisonTest.ql"\n */\n\nimport javascript\n\n/**\n * Holds if there are any contradictory guard nodes in `container`.\n *\n * We use this to restrict reachability analysis to a small set of containers.\n */\npredicate hasContradictoryGuardNodes(StmtContainer container) {\n  exists(ConditionGuardNode guard |\n    RangeAnalysis::isContradictoryGuardNode(guard) and\n    container = guard.getContainer()\n  )\n}\n\nfrom StmtContainer c\nwhere hasContradictoryGuardNodes(c)\nselect c, c.getNumLines()',
-            language: QueryLanguage.Javascript,
           },
         },
         getResults(rawResultsAnalysesResults),
diff --git a/extensions/ql-vscode/test/unit-tests/variant-analysis/variant-analysis-mapper.test.ts b/extensions/ql-vscode/test/unit-tests/variant-analysis/variant-analysis-mapper.test.ts
@@ -44,9 +44,9 @@ describe(mapVariantAnalysis.name, () => {
         fullName: mockApiResponse.controller_repo.full_name,
         private: mockApiResponse.controller_repo.private,
       },
+      language: QueryLanguage.Javascript,
       query: {
         filePath: "query-file-path",
-        language: QueryLanguage.Javascript,
         name: "query-name",
         text: mockSubmission.query.text,
         kind: "table",
diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts
@@ -392,7 +392,7 @@ describe("Variant Analysis Manager", () => {
       const request: VariantAnalysisSubmission =
         mockSubmitVariantAnalysis.mock.calls[0][1];
 
-      const packFS = await readBundledPack(request.query.pack);
+      const packFS = await readBundledPack(request.pack);
       filesThatExist.forEach((file) => {
         expect(file).toExistInCodeQLPack(packFS);
       });

Original file line number	Diff line number	Diff line change
`@@ -55,7 +55,7 @@ export function getLanguage(item: QueryHistoryInfo): QueryLanguage \| undefined {`
`55`	`55`	`case "local":`
`56`	`56`	`return item.initialInfo.databaseInfo.language;`
`57`	`57`	`case "variant-analysis":`
`58`		`- return item.variantAnalysis.query.language;`
	`58`	`+ return item.variantAnalysis.language;`
`59`	`59`	`default:`
`60`	`60`	`assertNever(item);`
`61`	`61`	`}`