Fix2 for issues identified in peer review

Improves error handling during initial environment setup for the CDS
extractor. Ensures that error codes are used when exiting due to an
error in the configuration of the CDS extractor.

Minor refactoring of index-files.ts script in order to migrate
environment setup tasks to dedicated functions with associated unit
tests.

Migrates response-file checking and related logic to a dedicated
'getCdsFilePathsToProcess` function, which also helps to prepare for
dropping the "index-files" approach in favor of an "autobuild" based
approach.

Author: data-douser

extractors/cds/tools/index-files.ts

@@ -1,20 +1,14 @@
 import { addCompilationDiagnostic, compileCdsToJson, determineCdsCommand } from './src/cdsCompiler';
-import { runJavaScriptExtractor, validateRequirements } from './src/codeql';
-import {
-  configureLgtmIndexFilters,
-  getAutobuildScriptPath,
-  getCodeQLExePath,
-  getJavaScriptExtractorRoot,
-  getPlatformInfo,
-  setupJavaScriptExtractorEnv,
-} from './src/environment';
-import { dirExists, readResponseFile } from './src/filesystem';
+import { runJavaScriptExtractor } from './src/codeql';
+import { configureLgtmIndexFilters, setupAndValidateEnvironment } from './src/environment';
+import { getCdsFilePathsToProcess } from './src/filesystem';
 import { findPackageJsonDirs, installDependencies } from './src/packageManager';
 import { validateArguments } from './src/utils';
 
 // Validate arguments to this script.
 if (!validateArguments(process.argv, 4)) {
-  process.exit(0);
+  // Exit with an error code on invalid use of this script.
+  process.exit(1);
 }
 
 // Get command-line (CLI) arguments and store them in named variables for clarity.
@@ -26,73 +20,37 @@ const sourceRoot: string = process.argv[3];
 process.chdir(sourceRoot);
 
 console.log(`Indexing CDS files in project source directory: ${sourceRoot}`);
-const { platform: osPlatform, arch: osPlatformArch } = getPlatformInfo();
-console.log(`Detected OS platform=${osPlatform} : arch=${osPlatformArch}`);
 
-// Get the `codeql` (CLI) executable path as this is needed to lookup information
-// about the JavaScript extractor (root directory), and to run CodeQL's JavaScript
-// extractor via its autobuild script.
-const codeqlExePath = getCodeQLExePath();
-
-// Validate that source (code) root directory exists.
-if (!dirExists(sourceRoot)) {
-  const codeqlExe = osPlatform === 'win32' ? 'codeql.exe' : 'codeql';
-  console.warn(
-    `'${codeqlExe} database index-files --language cds' terminated early due to internal error: could not find project root directory '${sourceRoot}'.`,
-  );
-  process.exit(0);
-}
-
-// Setup JavaScript extractor environment.
-const jsExtractorRoot = getJavaScriptExtractorRoot(codeqlExePath);
-if (!jsExtractorRoot) {
-  const codeqlExe = osPlatform === 'win32' ? 'codeql.exe' : 'codeql';
+// Setup the environment and validate all requirements.
+const {
+  success: envSetupSuccess,
+  errorMessages,
+  codeqlExePath,
+  autobuildScriptPath,
+  platformInfo,
+} = setupAndValidateEnvironment(sourceRoot);
+
+if (!envSetupSuccess) {
+  const codeqlExe = platformInfo.isWindows ? 'codeql.exe' : 'codeql';
   console.warn(
-    `'${codeqlExe} database index-files --language cds' terminated early as CODEQL_EXTRACTOR_JAVASCRIPT_ROOT environment variable is not set.`,
+    `'${codeqlExe} database index-files --language cds' terminated early due to: ${errorMessages.join(
+      ', ',
+    )}.`,
   );
-  process.exit(0);
+  // Exit with an error code when environment setup fails.
+  process.exit(1);
 }
 
-// Set environment variables for JavaScript extractor.
-process.env.CODEQL_EXTRACTOR_JAVASCRIPT_ROOT = jsExtractorRoot;
-setupJavaScriptExtractorEnv();
-
-// Get autobuild script path.
-const autobuildScriptPath = getAutobuildScriptPath(jsExtractorRoot);
-
-// Validate all required components.
-if (
-  !validateRequirements(
-    sourceRoot,
-    codeqlExePath,
-    responseFile,
-    autobuildScriptPath,
-    jsExtractorRoot,
-  )
-) {
-  process.exit(0);
+// Validate response file and get the fully paths of CDS files to process.
+const filePathsResult = getCdsFilePathsToProcess(responseFile, platformInfo);
+if (!filePathsResult.success) {
+  console.warn(filePathsResult.errorMessage);
+  // Exit with an error if unable to get a list of `.cds` file paths to process.
+  process.exit(1);
 }
 
-// Read and validate response files.
-let cdsFilePathsToProcess: string[] = [];
-try {
-  cdsFilePathsToProcess = readResponseFile(responseFile);
-  if (!cdsFilePathsToProcess.length) {
-    const codeqlExe = osPlatform === 'win32' ? 'codeql.exe' : 'codeql';
-    console.warn(
-      `'${codeqlExe} database index-files --language cds' terminated early as response file '${responseFile}' is empty. This is because no CDS files were selected or found.`,
-    );
-    process.exit(0);
-  }
-} catch (err) {
-  const codeqlExe = osPlatform === 'win32' ? 'codeql.exe' : 'codeql';
-  console.warn(
-    `'${codeqlExe} database index-files --language cds' terminated early as response file '${responseFile}' could not be read due to an error: ${String(
-      err,
-    )}`,
-  );
-  process.exit(0);
-}
+// Get the validated list of CDS files to process
+const cdsFilePathsToProcess = filePathsResult.cdsFilePaths;
 
 // Find all package.json directories that have a `@sap/cds` node dependency.
 const packageJsonDirs = findPackageJsonDirs(cdsFilePathsToProcess);
@@ -106,7 +64,7 @@ const cdsCommand = determineCdsCommand();
 
 console.log('Processing CDS files to JSON ...');
 
-// Compile each CDS file to JSON
+// Compile each `.cds` file to create a `.cds.json` file.
 for (const rawCdsFilePath of cdsFilePathsToProcess) {
   try {
     // Use resolved path directly instead of passing through getArg

extractors/cds/tools/src/environment.ts

@@ -2,6 +2,8 @@ import { execFileSync } from 'child_process';
 import { arch, platform } from 'os';
 import { join, resolve } from 'path';
 
+import { dirExists } from './filesystem';
+
 /**
  * Interface for platform information
  */
@@ -12,6 +14,18 @@ export interface PlatformInfo {
   exeExtension: string;
 }
 
+/**
+ * Interface for environment validation results
+ */
+export interface EnvironmentSetupResult {
+  success: boolean;
+  errorMessages: string[];
+  codeqlExePath: string;
+  jsExtractorRoot: string;
+  autobuildScriptPath: string;
+  platformInfo: PlatformInfo;
+}
+
 /**
  * Get platform information
  * @returns Platform information including OS platform, architecture, and whether it's Windows
@@ -130,3 +144,45 @@ export function configureLgtmIndexFilters(): void {
   // Configure to copy over the .cds files as well, by pretending they are JSON.
   process.env.LGTM_INDEX_FILETYPES = '.cds:JSON';
 }
+
+/**
+ * Sets up the environment and validates key components for CDS extractor
+ * @param sourceRoot The source root directory
+ * @returns The environment setup result
+ */
+export function setupAndValidateEnvironment(sourceRoot: string): EnvironmentSetupResult {
+  const errorMessages: string[] = [];
+  const platformInfo = getPlatformInfo();
+
+  // Get the CodeQL executable path
+  const codeqlExePath = getCodeQLExePath();
+
+  // Validate that the required source root directory exists
+  if (!dirExists(sourceRoot)) {
+    errorMessages.push(`project root directory '${sourceRoot}' does not exist`);
+  }
+
+  // Setup JavaScript extractor environment
+  const jsExtractorRoot = getJavaScriptExtractorRoot(codeqlExePath);
+  if (!jsExtractorRoot) {
+    errorMessages.push(`CODEQL_EXTRACTOR_JAVASCRIPT_ROOT environment variable is not set`);
+  }
+
+  // Set environment variables for JavaScript extractor
+  if (jsExtractorRoot) {
+    process.env.CODEQL_EXTRACTOR_JAVASCRIPT_ROOT = jsExtractorRoot;
+    setupJavaScriptExtractorEnv();
+  }
+
+  // Get autobuild script path
+  const autobuildScriptPath = jsExtractorRoot ? getAutobuildScriptPath(jsExtractorRoot) : '';
+
+  return {
+    success: errorMessages.length === 0,
+    errorMessages,
+    codeqlExePath,
+    jsExtractorRoot,
+    autobuildScriptPath,
+    platformInfo,
+  };
+}

extractors/cds/tools/src/filesystem.ts

@@ -1,6 +1,15 @@
 import { existsSync, readdirSync, readFileSync, renameSync, statSync } from 'fs';
 import { format, join, parse } from 'path';
 
+/**
+ * Check if a directory exists
+ * @param dirPath Path to the directory to check
+ * @returns True if the directory exists, false otherwise
+ */
+export function dirExists(dirPath: string): boolean {
+  return existsSync(dirPath) && statSync(dirPath).isDirectory();
+}
+
 /**
  * Check if a file exists and can be read
  * @param filePath Path to the file to check
@@ -11,12 +20,59 @@ export function fileExists(filePath: string): boolean {
 }
 
 /**
- * Check if a directory exists
- * @param dirPath Path to the directory to check
- * @returns True if the directory exists, false otherwise
+ * Read and validate a response file to get the list of CDS files to process
+ * @param responseFile Path to the response file
+ * @param platformInfo Platform information object with isWindows property
+ * @returns Object containing success status, CDS file paths to process, and error message if any
  */
-export function dirExists(dirPath: string): boolean {
-  return existsSync(dirPath) && statSync(dirPath).isDirectory();
+export function getCdsFilePathsToProcess(
+  responseFile: string,
+  platformInfo: { isWindows: boolean },
+): {
+  success: boolean;
+  cdsFilePaths: string[];
+  errorMessage?: string;
+} {
+  // First validate the response file exists
+  const responseFileValidation = validateResponseFile(responseFile);
+  if (!responseFileValidation.success) {
+    return {
+      success: false,
+      cdsFilePaths: [],
+      errorMessage: `'${
+        platformInfo.isWindows ? 'codeql.exe' : 'codeql'
+      } database index-files --language cds' terminated early as ${responseFileValidation.errorMessage}`,
+    };
+  }
+
+  // Now read the file paths from the response file
+  try {
+    const cdsFilePathsToProcess = readResponseFile(responseFile);
+
+    // Check if there are any file paths to process
+    if (!cdsFilePathsToProcess.length) {
+      return {
+        success: false,
+        cdsFilePaths: [],
+        errorMessage: `'${
+          platformInfo.isWindows ? 'codeql.exe' : 'codeql'
+        } database index-files --language cds' terminated early as response file '${responseFile}' is empty. This is because no CDS files were selected or found.`,
+      };
+    }
+
+    return {
+      success: true,
+      cdsFilePaths: cdsFilePathsToProcess,
+    };
+  } catch (err) {
+    return {
+      success: false,
+      cdsFilePaths: [],
+      errorMessage: `'${
+        platformInfo.isWindows ? 'codeql.exe' : 'codeql'
+      } database index-files --language cds' terminated early as response file '${responseFile}' could not be read due to an error: ${String(err)}`,
+    };
+  }
 }
 
 /**
@@ -72,3 +128,21 @@ export function recursivelyRenameJsonFiles(dirPath: string): void {
     }
   }
 }
+
+/**
+ * Validate a response file exists and can be read
+ * @param responseFile Path to the response file
+ * @returns Object containing success status and error message if any
+ */
+export function validateResponseFile(responseFile: string): {
+  success: boolean;
+  errorMessage?: string;
+} {
+  if (!fileExists(responseFile)) {
+    return {
+      success: false,
+      errorMessage: `response file '${responseFile}' does not exist. This is because no CDS files were selected or found`,
+    };
+  }
+  return { success: true };
+}