Address minor review comments

src/sarif/index.ts

@@ -71,13 +71,11 @@ export function combineSarifFiles(
 
 /**
  * Checks whether all the runs in the given SARIF files were produced by CodeQL.
- * @param sarifObjects The list of SARIF objects to check.
+ * @param sarifLogs The list of SARIF objects to check.
  */
-export function areAllRunsProducedByCodeQL(sarifObjects: sarif.Log[]): boolean {
-  return sarifObjects.every((sarifObject) => {
-    return sarifObject.runs?.every(
-      (run) => run.tool?.driver?.name === "CodeQL",
-    );
+export function areAllRunsProducedByCodeQL(sarifLogs: sarif.Log[]): boolean {
+  return sarifLogs.every((sarifLog: sarif.Log) => {
+    return sarifLog.runs?.every((run) => run.tool?.driver?.name === "CodeQL");
   });
 }
 
@@ -95,13 +93,13 @@ function createRunKey(run: sarif.Run): RunKey {
 /**
  * Checks whether all runs in the given SARIF files are unique (based on the
  * criteria used by Code Scanning to determine analysis categories).
- * @param sarifObjects The list of SARIF objects to check.
+ * @param sarifLogs The list of SARIF objects to check.
  */
-export function areAllRunsUnique(sarifObjects: sarif.Log[]): boolean {
+export function areAllRunsUnique(sarifLogs: sarif.Log[]): boolean {
   const keys = new Set<string>();
 
-  for (const sarifObject of sarifObjects) {
-    for (const run of sarifObject.runs) {
+  for (const sarifLog of sarifLogs) {
+    for (const run of sarifLog.runs) {
       const key = JSON.stringify(createRunKey(run));
 
       // If the key already exists, the runs are not unique.

src/upload-lib.test.ts

@@ -26,7 +26,7 @@ test("validateSarifFileSchema - valid", (t) => {
   const inputFile = `${__dirname}/../src/testdata/valid-sarif.sarif`;
   t.notThrows(() =>
     uploadLib.validateSarifFileSchema(
-      uploadLib.readSarifFile(inputFile),
+      uploadLib.readSarifFileOrThrow(inputFile),
       inputFile,
       getRunnerLogger(true),
     ),
@@ -37,7 +37,7 @@ test("validateSarifFileSchema - invalid", (t) => {
   const inputFile = `${__dirname}/../src/testdata/invalid-sarif.sarif`;
   t.throws(() =>
     uploadLib.validateSarifFileSchema(
-      uploadLib.readSarifFile(inputFile),
+      uploadLib.readSarifFileOrThrow(inputFile),
       inputFile,
       getRunnerLogger(true),
     ),
@@ -608,7 +608,7 @@ test("accept results with invalid artifactLocation.uri value", (t) => {
 
   const sarifFile = `${__dirname}/../src/testdata/with-invalid-uri.sarif`;
   uploadLib.validateSarifFileSchema(
-    uploadLib.readSarifFile(sarifFile),
+    uploadLib.readSarifFileOrThrow(sarifFile),
     sarifFile,
     mockLogger,
   );

src/upload-lib.ts

@@ -115,7 +115,7 @@ async function combineSarifFilesUsingCLI(
 ): Promise<sarif.Log> {
   logger.info("Combining SARIF files using the CodeQL CLI");
 
-  const sarifObjects = sarifFiles.map(util.readSarifFile);
+  const sarifObjects = sarifFiles.map(sarif.readSarifFile);
 
   const deprecationWarningMessage =
     gitHubVersion.type === GitHubVariant.GHES
@@ -197,7 +197,7 @@ async function combineSarifFilesUsingCLI(
     mergeRunsFromEqualCategory: true,
   });
 
-  return util.readSarifFile(outputFile);
+  return sarif.readSarifFile(outputFile);
 }
 
 // Populates the run.automationDetails.id field using the analysis_key and environment
@@ -447,7 +447,11 @@ function countResultsInSarif(sarifLog: string): number {
   return numResults;
 }
 
-export function readSarifFile(sarifFilePath: string): sarif.Log {
+/** A thin wrapper around `readSarifFile` which wraps exceptions in `InvalidSarifUploadError`.
+ *
+ * @throws InvalidSarifUploadError If parsing the SARIF file as JSON failed.
+ */
+export function readSarifFileOrThrow(sarifFilePath: string): sarif.Log {
   try {
     return sarif.readSarifFile(sarifFilePath);
   } catch (e) {
@@ -617,7 +621,7 @@ export async function postProcessSarifFiles(
   if (sarifPaths.length > 1) {
     // Validate that the files we were asked to upload are all valid SARIF files
     for (const sarifPath of sarifPaths) {
-      const parsedSarif = readSarifFile(sarifPath);
+      const parsedSarif = readSarifFileOrThrow(sarifPath);
       validateSarifFileSchema(parsedSarif, sarifPath, logger);
     }
 
@@ -629,7 +633,7 @@ export async function postProcessSarifFiles(
     );
   } else {
     const sarifPath = sarifPaths[0];
-    sarifLog = readSarifFile(sarifPath);
+    sarifLog = readSarifFileOrThrow(sarifPath);
     validateSarifFileSchema(sarifLog, sarifPath, logger);
 
     // Validate that there are no runs for the same category
@@ -755,7 +759,7 @@ export async function uploadPostProcessedFiles(
   logger.startGroup(`Uploading ${uploadTarget.name} results`);
 
   const sarifLog = postProcessingResults.sarif;
-  const toolNames = util.getToolNames(sarifLog);
+  const toolNames = sarif.getToolNames(sarifLog);
 
   logger.debug(`Validating that each SARIF run has a unique category`);
   validateUniqueCategory(sarifLog, uploadTarget.sentinelPrefix);

src/upload-sarif-action.ts

@@ -7,6 +7,7 @@ import { getGitHubVersion } from "./api-client";
 import { initFeatures } from "./feature-flags";
 import { Logger, getActionsLogger } from "./logging";
 import { getRepositoryNwo } from "./repository";
+import { InvalidSarifUploadError } from "./sarif";
 import {
   createStatusReportBase,
   sendStatusReport,
@@ -20,7 +21,6 @@ import * as upload_lib from "./upload-lib";
 import { postProcessAndUploadSarif } from "./upload-sarif";
 import {
   ConfigurationError,
-  InvalidSarifUploadError,
   checkActionVersion,
   checkDiskUsage,
   getErrorMessage,

src/util.ts

@@ -17,8 +17,6 @@ import { EnvVar } from "./environment";
 import { Language } from "./languages";
 import { Logger } from "./logging";
 
-export * from "./sarif";
-
 /**
  * The name of the file containing the base database OIDs, as stored in the
  * root of the database location.