Merge pull request #2831 from github/cklin/diff-informed-query-filtering

Respect `exclude-from-incremental` query tag for diff-informed analysis

src/analyze-action.ts

@@ -22,6 +22,7 @@ import { getCodeQL } from "./codeql";
 import { Config, getConfig } from "./config-utils";
 import { uploadDatabases } from "./database-upload";
 import { uploadDependencyCaches } from "./dependency-caching";
+import { getDiffInformedAnalysisBranches } from "./diff-informed-analysis-utils";
 import { EnvVar } from "./environment";
 import { Features } from "./feature-flags";
 import { Language } from "./languages";
@@ -269,11 +270,14 @@ async function run() {
       logger,
     );
 
-    const diffRangePackDir = await setupDiffInformedQueryRun(
+    const branches = await getDiffInformedAnalysisBranches(
       codeql,
-      logger,
       features,
+      logger,
     );
+    const diffRangePackDir = branches
+      ? await setupDiffInformedQueryRun(branches, logger)
+      : undefined;
 
     await warnIfGoInstalledAfterInit(config, logger);
     await runAutobuildIfLegacyGoWorkflow(config, logger);

src/analyze.ts

@@ -2,7 +2,6 @@ import * as fs from "fs";
 import * as path from "path";
 import { performance } from "perf_hooks";
 
-import * as github from "@actions/github";
 import * as io from "@actions/io";
 import del from "del";
 import * as yaml from "js-yaml";
@@ -16,8 +15,9 @@ import { getJavaTempDependencyDir } from "./dependency-caching";
 import { addDiagnostic, makeDiagnostic } from "./diagnostics";
 import {
   DiffThunkRange,
+  PullRequestBranches,
   writeDiffRangesJsonFile,
-} from "./diff-filtering-utils";
+} from "./diff-informed-analysis-utils";
 import { EnvVar } from "./environment";
 import { FeatureEnablement, Feature } from "./feature-flags";
 import { isScannedLanguage, Language } from "./languages";
@@ -255,38 +255,6 @@ async function finalizeDatabaseCreation(
   };
 }
 
-interface PullRequestBranches {
-  base: string;
-  head: string;
-}
-
-function getPullRequestBranches(): PullRequestBranches | undefined {
-  const pullRequest = github.context.payload.pull_request;
-  if (pullRequest) {
-    return {
-      base: pullRequest.base.ref,
-      // We use the head label instead of the head ref here, because the head
-      // ref lacks owner information and by itself does not uniquely identify
-      // the head branch (which may be in a forked repository).
-      head: pullRequest.head.label,
-    };
-  }
-
-  // PR analysis under Default Setup does not have the pull_request context,
-  // but it should set CODE_SCANNING_REF and CODE_SCANNING_BASE_BRANCH.
-  const codeScanningRef = process.env.CODE_SCANNING_REF;
-  const codeScanningBaseBranch = process.env.CODE_SCANNING_BASE_BRANCH;
-  if (codeScanningRef && codeScanningBaseBranch) {
-    return {
-      base: codeScanningBaseBranch,
-      // PR analysis under Default Setup analyzes the PR head commit instead of
-      // the merge commit, so we can use the provided ref directly.
-      head: codeScanningRef,
-    };
-  }
-  return undefined;
-}
-
 /**
  * Set up the diff-informed analysis feature.
  *
@@ -294,23 +262,9 @@ function getPullRequestBranches(): PullRequestBranches | undefined {
  * the diff range information, or `undefined` if the feature is disabled.
  */
 export async function setupDiffInformedQueryRun(
-  codeql: CodeQL,
+  branches: PullRequestBranches,
   logger: Logger,
-  features: FeatureEnablement,
 ): Promise<string | undefined> {
-  if (!(await features.getValue(Feature.DiffInformedQueries, codeql))) {
-    return undefined;
-  }
-
-  const branches = getPullRequestBranches();
-  if (!branches) {
-    logger.info(
-      "Not performing diff-informed analysis " +
-        "because we are not analyzing a pull request.",
-    );
-    return undefined;
-  }
-
   return await withGroupAsync(
     "Generating diff range extension pack",
     async () => {

src/codeql.ts

@@ -1274,6 +1274,15 @@ async function generateCodeScanningConfig(
   if (Array.isArray(augmentedConfig.packs) && !augmentedConfig.packs.length) {
     delete augmentedConfig.packs;
   }
+
+  augmentedConfig["query-filters"] = [
+    ...(config.augmentationProperties.defaultQueryFilters || []),
+    ...(augmentedConfig["query-filters"] || []),
+  ];
+  if (augmentedConfig["query-filters"]?.length === 0) {
+    delete augmentedConfig["query-filters"];
+  }
+
   logger.info(
     `Writing augmented user configuration file to ${codeScanningConfigFile}`,
   );

src/config-utils.test.ts

@@ -809,11 +809,15 @@ const calculateAugmentationMacro = test.macro({
     languages: Language[],
     expectedAugmentationProperties: configUtils.AugmentationProperties,
   ) => {
-    const actualAugmentationProperties = configUtils.calculateAugmentation(
-      rawPacksInput,
-      rawQueriesInput,
-      languages,
-    );
+    const actualAugmentationProperties =
+      await configUtils.calculateAugmentation(
+        getCachedCodeQL(),
+        createFeatures([]),
+        rawPacksInput,
+        rawQueriesInput,
+        languages,
+        mockLogger,
+      );
     t.deepEqual(actualAugmentationProperties, expectedAugmentationProperties);
   },
   title: (_, title) => `Calculate Augmentation: ${title}`,
@@ -830,6 +834,7 @@ test(
     queriesInput: undefined,
     packsInputCombines: false,
     packsInput: undefined,
+    defaultQueryFilters: [],
   } as configUtils.AugmentationProperties,
 );
 
@@ -844,6 +849,7 @@ test(
     queriesInput: [{ uses: "a" }, { uses: "b" }, { uses: "c" }, { uses: "d" }],
     packsInputCombines: false,
     packsInput: undefined,
+    defaultQueryFilters: [],
   } as configUtils.AugmentationProperties,
 );
 
@@ -858,6 +864,7 @@ test(
     queriesInput: [{ uses: "a" }, { uses: "b" }, { uses: "c" }, { uses: "d" }],
     packsInputCombines: false,
     packsInput: undefined,
+    defaultQueryFilters: [],
   } as configUtils.AugmentationProperties,
 );
 
@@ -872,6 +879,7 @@ test(
     queriesInput: undefined,
     packsInputCombines: false,
     packsInput: ["codeql/a", "codeql/b", "codeql/c", "codeql/d"],
+    defaultQueryFilters: [],
   } as configUtils.AugmentationProperties,
 );
 
@@ -886,6 +894,7 @@ test(
     queriesInput: undefined,
     packsInputCombines: true,
     packsInput: ["codeql/a", "codeql/b", "codeql/c", "codeql/d"],
+    defaultQueryFilters: [],
   } as configUtils.AugmentationProperties,
 );
 
@@ -898,12 +907,15 @@ const calculateAugmentationErrorMacro = test.macro({
     languages: Language[],
     expectedError: RegExp | string,
   ) => {
-    t.throws(
+    await t.throwsAsync(
       () =>
         configUtils.calculateAugmentation(
+          getCachedCodeQL(),
+          createFeatures([]),
           rawPacksInput,
           rawQueriesInput,
           languages,
+          mockLogger,
         ),
       { message: expectedError },
     );

src/config-utils.ts

@@ -8,6 +8,7 @@ import * as semver from "semver";
 import * as api from "./api-client";
 import { CachingKind, getCachingKind } from "./caching-utils";
 import { CodeQL } from "./codeql";
+import { shouldPerformDiffInformedAnalysis } from "./diff-informed-analysis-utils";
 import { Feature, FeatureEnablement } from "./feature-flags";
 import { Language, parseLanguage } from "./languages";
 import { Logger } from "./logging";
@@ -173,10 +174,16 @@ export interface AugmentationProperties {
    * Whether or not the packs input combines with the packs in the config.
    */
   packsInputCombines: boolean;
+
   /**
    * The packs input from the `with` block of the action declaration
    */
   packsInput?: string[];
+
+  /**
+   * Default query filters to apply to the queries in the config.
+   */
+  defaultQueryFilters?: QueryFilter[];
 }
 
 /**
@@ -188,6 +195,7 @@ export const defaultAugmentationProperties: AugmentationProperties = {
   packsInputCombines: false,
   packsInput: undefined,
   queriesInput: undefined,
+  defaultQueryFilters: [],
 };
 export type Packs = Partial<Record<Language, string[]>>;
 
@@ -461,10 +469,13 @@ export async function getDefaultConfig({
     logger,
   );
 
-  const augmentationProperties = calculateAugmentation(
+  const augmentationProperties = await calculateAugmentation(
+    codeql,
+    features,
     packsInput,
     queriesInput,
     languages,
+    logger,
   );
 
   const { trapCaches, trapCacheDownloadTime } = await downloadCacheWithTime(
@@ -567,10 +578,13 @@ async function loadConfig({
     logger,
   );
 
-  const augmentationProperties = calculateAugmentation(
+  const augmentationProperties = await calculateAugmentation(
+    codeql,
+    features,
     packsInput,
     queriesInput,
     languages,
+    logger,
   );
 
   const { trapCaches, trapCacheDownloadTime } = await downloadCacheWithTime(
@@ -605,11 +619,14 @@ async function loadConfig({
  * and the CLI does not know about these inputs so we need to inject them into
  * the config file sent to the CLI.
  *
+ * @param codeql The CodeQL object.
+ * @param features The feature enablement object.
  * @param rawPacksInput The packs input from the action configuration.
  * @param rawQueriesInput The queries input from the action configuration.
  * @param languages The languages that the config file is for. If the packs input
  *    is non-empty, then there must be exactly one language. Otherwise, an
  *    error is thrown.
+ * @param logger The logger to use for logging.
  *
  * @returns The properties that need to be augmented in the config file.
  *
@@ -617,11 +634,14 @@ async function loadConfig({
  *     not have exactly one language.
  */
 // exported for testing.
-export function calculateAugmentation(
+export async function calculateAugmentation(
+  codeql: CodeQL,
+  features: FeatureEnablement,
   rawPacksInput: string | undefined,
   rawQueriesInput: string | undefined,
   languages: Language[],
-): AugmentationProperties {
+  logger: Logger,
+): Promise<AugmentationProperties> {
   const packsInputCombines = shouldCombine(rawPacksInput);
   const packsInput = parsePacksFromInput(
     rawPacksInput,
@@ -634,11 +654,17 @@ export function calculateAugmentation(
     queriesInputCombines,
   );
 
+  const defaultQueryFilters: QueryFilter[] = [];
+  if (await shouldPerformDiffInformedAnalysis(codeql, features, logger)) {
+    defaultQueryFilters.push({ exclude: { tags: "exclude-from-incremental" } });
+  }
+
   return {
     packsInputCombines,
     packsInput: packsInput?.[languages[0]],
     queriesInput,
     queriesInputCombines,
+    defaultQueryFilters,
   };
 }
 

src/diff-filtering-utils.ts

@@ -1,42 +0,0 @@
-import * as fs from "fs";
-import * as path from "path";
-
-import * as actionsUtil from "./actions-util";
-import { Logger } from "./logging";
-
-export interface DiffThunkRange {
-  path: string;
-  startLine: number;
-  endLine: number;
-}
-
-function getDiffRangesJsonFilePath(): string {
-  return path.join(actionsUtil.getTemporaryDirectory(), "pr-diff-range.json");
-}
-
-export function writeDiffRangesJsonFile(
-  logger: Logger,
-  ranges: DiffThunkRange[],
-): void {
-  const jsonContents = JSON.stringify(ranges, null, 2);
-  const jsonFilePath = getDiffRangesJsonFilePath();
-  fs.writeFileSync(jsonFilePath, jsonContents);
-  logger.debug(
-    `Wrote pr-diff-range JSON file to ${jsonFilePath}:\n${jsonContents}`,
-  );
-}
-
-export function readDiffRangesJsonFile(
-  logger: Logger,
-): DiffThunkRange[] | undefined {
-  const jsonFilePath = getDiffRangesJsonFilePath();
-  if (!fs.existsSync(jsonFilePath)) {
-    logger.debug(`Diff ranges JSON file does not exist at ${jsonFilePath}`);
-    return undefined;
-  }
-  const jsonContents = fs.readFileSync(jsonFilePath, "utf8");
-  logger.debug(
-    `Read pr-diff-range JSON file from ${jsonFilePath}:\n${jsonContents}`,
-  );
-  return JSON.parse(jsonContents) as DiffThunkRange[];
-}

src/diff-informed-analysis-utils.ts

@@ -0,0 +1,118 @@
+import * as fs from "fs";
+import * as path from "path";
+
+import * as github from "@actions/github";
+
+import * as actionsUtil from "./actions-util";
+import type { CodeQL } from "./codeql";
+import { Feature, FeatureEnablement } from "./feature-flags";
+import { Logger } from "./logging";
+
+export interface PullRequestBranches {
+  base: string;
+  head: string;
+}
+
+function getPullRequestBranches(): PullRequestBranches | undefined {
+  const pullRequest = github.context.payload.pull_request;
+  if (pullRequest) {
+    return {
+      base: pullRequest.base.ref,
+      // We use the head label instead of the head ref here, because the head
+      // ref lacks owner information and by itself does not uniquely identify
+      // the head branch (which may be in a forked repository).
+      head: pullRequest.head.label,
+    };
+  }
+
+  // PR analysis under Default Setup does not have the pull_request context,
+  // but it should set CODE_SCANNING_REF and CODE_SCANNING_BASE_BRANCH.
+  const codeScanningRef = process.env.CODE_SCANNING_REF;
+  const codeScanningBaseBranch = process.env.CODE_SCANNING_BASE_BRANCH;
+  if (codeScanningRef && codeScanningBaseBranch) {
+    return {
+      base: codeScanningBaseBranch,
+      // PR analysis under Default Setup analyzes the PR head commit instead of
+      // the merge commit, so we can use the provided ref directly.
+      head: codeScanningRef,
+    };
+  }
+  return undefined;
+}
+
+/**
+ * Check if the action should perform diff-informed analysis.
+ */
+export async function shouldPerformDiffInformedAnalysis(
+  codeql: CodeQL,
+  features: FeatureEnablement,
+  logger: Logger,
+): Promise<boolean> {
+  return (
+    (await getDiffInformedAnalysisBranches(codeql, features, logger)) !==
+    undefined
+  );
+}
+
+/**
+ * Get the branches to use for diff-informed analysis.
+ *
+ * @returns If the action should perform diff-informed analysis, return
+ * the base and head branches that should be used to compute the diff ranges.
+ * Otherwise return `undefined`.
+ */
+export async function getDiffInformedAnalysisBranches(
+  codeql: CodeQL,
+  features: FeatureEnablement,
+  logger: Logger,
+): Promise<PullRequestBranches | undefined> {
+  if (!(await features.getValue(Feature.DiffInformedQueries, codeql))) {
+    return undefined;
+  }
+
+  const branches = getPullRequestBranches();
+  if (!branches) {
+    logger.info(
+      "Not performing diff-informed analysis " +
+        "because we are not analyzing a pull request.",
+    );
+  }
+  return branches;
+}
+
+export interface DiffThunkRange {
+  path: string;
+  startLine: number;
+  endLine: number;
+}
+
+function getDiffRangesJsonFilePath(): string {
+  return path.join(actionsUtil.getTemporaryDirectory(), "pr-diff-range.json");
+}
+
+export function writeDiffRangesJsonFile(
+  logger: Logger,
+  ranges: DiffThunkRange[],
+): void {
+  const jsonContents = JSON.stringify(ranges, null, 2);
+  const jsonFilePath = getDiffRangesJsonFilePath();
+  fs.writeFileSync(jsonFilePath, jsonContents);
+  logger.debug(
+    `Wrote pr-diff-range JSON file to ${jsonFilePath}:\n${jsonContents}`,
+  );
+}
+
+export function readDiffRangesJsonFile(
+  logger: Logger,
+): DiffThunkRange[] | undefined {
+  const jsonFilePath = getDiffRangesJsonFilePath();
+  if (!fs.existsSync(jsonFilePath)) {
+    logger.debug(`Diff ranges JSON file does not exist at ${jsonFilePath}`);
+    return undefined;
+  }
+  const jsonContents = fs.readFileSync(jsonFilePath, "utf8");
+  logger.debug(
+    `Read pr-diff-range JSON file from ${jsonFilePath}:\n${jsonContents}`,
+  );
+  return JSON.parse(jsonContents) as DiffThunkRange[];
+}

src/upload-lib.ts

@@ -14,7 +14,7 @@ import * as api from "./api-client";
 import { getGitHubVersion, wrapApiConfigurationError } from "./api-client";
 import { CodeQL, getCodeQL } from "./codeql";
 import { getConfig } from "./config-utils";
-import { readDiffRangesJsonFile } from "./diff-filtering-utils";
+import { readDiffRangesJsonFile } from "./diff-informed-analysis-utils";
 import { EnvVar } from "./environment";
 import { FeatureEnablement } from "./feature-flags";
 import * as fingerprints from "./fingerprints";