Add --check option to format command (#1962)

* add: --check option to format command

* add: return exit code when error occured during formatting

* add: test for --check option of format command

* fix: output error to stderr not stdout

* add: error message when error occured during formatting

* refactor: separate test for format

Author: AobaIwaki123

cli/console.ts

@@ -234,7 +234,6 @@ export function printExecutedAction(
   const jobIdSuffix = jobIds.length > 0 ? ` (jobId: ${jobIds.join(", ")})` : "";
   switch (executedAction.status) {
     case dataform.ActionResult.ExecutionStatus.SUCCESSFUL: {
-
       switch (executionAction.type) {
         case "table": {
           writeStdOut(
@@ -250,15 +249,21 @@ export function printExecutedAction(
           writeStdOut(
             `${successOutput(
               `Assertion ${dryRun ? "dry run success" : "passed"}: `
-            )} ${assertionString(executionAction.target, executionAction.tasks.length === 0)}${jobIdSuffix}`
+            )} ${assertionString(
+              executionAction.target,
+              executionAction.tasks.length === 0
+            )}${jobIdSuffix}`
           );
           return;
         }
         case "operation": {
           writeStdOut(
             `${successOutput(
               `Operation ${dryRun ? "dry run success" : "completed successfully"}: `
-            )} ${operationString(executionAction.target, executionAction.tasks.length === 0)}${jobIdSuffix}`
+            )} ${operationString(
+              executionAction.target,
+              executionAction.tasks.length === 0
+            )}${jobIdSuffix}`
           );
           return;
         }
@@ -371,15 +376,24 @@ export function printFormatFilesResult(
   formatResults: Array<{
     filename: string;
     err?: Error;
+    needsFormatting?: boolean;
   }>
 ) {
   const sorted = formatResults.sort((a, b) => a.filename.localeCompare(b.filename));
-  const successfulFormatResults = sorted.filter(result => !result.err);
+  const successfulFormatResults = sorted.filter(result => !result.err && !result.needsFormatting);
+  const needsFormattingResults = sorted.filter(result => !result.err && result.needsFormatting);
   const failedFormatResults = sorted.filter(result => !!result.err);
+
   if (successfulFormatResults.length > 0) {
     printSuccess("Successfully formatted:");
     successfulFormatResults.forEach(result => writeStdOut(result.filename, 1));
   }
+
+  if (needsFormattingResults.length > 0) {
+    printError("Files that need formatting:");
+    needsFormattingResults.forEach(result => writeStdErr(result.filename, 1));
+  }
+
   if (failedFormatResults.length > 0) {
     printError("Errors encountered during formatting:");
     failedFormatResults.forEach(result =>

cli/index.ts

@@ -171,6 +171,7 @@ const watchOptionName = "watch";
 
 const dryRunOptionName = "dry-run";
 const runTestsOptionName = "run-tests";
+const checkOptionName = "check";
 
 const actionRetryLimitName = "action-retry-limit";
 
@@ -586,7 +587,17 @@ export function runCli() {
         format: `format [${projectDirMustExistOption.name}]`,
         description: "Format the dataform project's files.",
         positionalOptions: [projectDirMustExistOption],
-        options: [actionsOption],
+        options: [
+          actionsOption,
+          {
+            name: checkOptionName,
+            option: {
+              describe: "Check if files are formatted correctly without modifying them.",
+              type: "boolean",
+              default: false
+            }
+          }
+        ],
         processFn: async argv => {
           let actions = ["{definitions,includes}/**/*.{js,sqlx}"];
           if (actionsOption.name in argv && argv[actionsOption.name].length > 0) {
@@ -597,15 +608,35 @@ export function runCli() {
               glob.sync(action, { cwd: argv[projectDirMustExistOption.name] })
             )
             .flat();
-          const results: Array<{ filename: string; err?: Error }> = await Promise.all(
+
+          const isCheckMode = argv[checkOptionName];
+          const results: Array<{
+            filename: string;
+            err?: Error;
+            needsFormatting?: boolean;
+          }> = await Promise.all(
             filenames.map(async (filename: string) => {
               try {
-                await formatFile(path.resolve(argv[projectDirMustExistOption.name], filename), {
-                  overwriteFile: true
-                });
-                return {
-                  filename
-                };
+                const filePath = path.resolve(argv[projectDirMustExistOption.name], filename);
+                if (isCheckMode) {
+                  // In check mode, we don't modify files, just check if they need formatting
+                  const fileContent = fs.readFileSync(filePath).toString();
+                  const formattedContent = await formatFile(filePath, {
+                    overwriteFile: false
+                  });
+                  return {
+                    filename,
+                    needsFormatting: fileContent !== formattedContent
+                  };
+                } else {
+                  // Normal formatting mode
+                  await formatFile(filePath, {
+                    overwriteFile: true
+                  });
+                  return {
+                    filename
+                  };
+                }
               } catch (e) {
                 return {
                   filename,
@@ -614,7 +645,28 @@ export function runCli() {
               }
             })
           );
+
           printFormatFilesResult(results);
+
+          // Return error code if there are any formatting errors
+          const failedFormatResults = results.filter(result => !!result.err);
+          if (failedFormatResults.length > 0) {
+            printError(`${failedFormatResults.length} file(s) failed to format.`);
+            return 1;
+          }
+
+          // In check mode, return an error code if any files need formatting
+          if (isCheckMode) {
+            const filesNeedingFormatting = results.filter(result => result.needsFormatting);
+            if (filesNeedingFormatting.length > 0) {
+              printError(
+                `${filesNeedingFormatting.length} file(s) would be reformatted. Run the format command without --check to update.`
+              );
+              return 1;
+            }
+            printSuccess("All files are formatted correctly!");
+          }
+
           return 0;
         }
       }

cli/index_test.ts

@@ -304,4 +304,92 @@ select 1 as \${dataform.projectConfig.vars.testVar2}
       warehouseState: {}
     });
   });
+
+  test("test for format command", async () => {
+    const projectDir = tmpDirFixture.createNewTmpDir();
+    const npmCacheDir = tmpDirFixture.createNewTmpDir();
+    const workflowSettingsPath = path.join(projectDir, "workflow_settings.yaml");
+    const packageJsonPath = path.join(projectDir, "package.json");
+
+    // Initialize a project using the CLI, don't install packages.
+    await getProcessResult(
+      execFile(nodePath, [cliEntryPointPath, "init", projectDir, "dataform-open-source", "US"])
+    );
+
+    // Install packages manually to get around bazel read-only sandbox issues.
+    const workflowSettings = dataform.WorkflowSettings.create(
+      loadYaml(fs.readFileSync(workflowSettingsPath, "utf8"))
+    );
+    delete workflowSettings.dataformCoreVersion;
+    fs.writeFileSync(workflowSettingsPath, dumpYaml(workflowSettings));
+    fs.writeFileSync(
+      packageJsonPath,
+      `{
+  "dependencies":{
+    "@dataform/core": "${version}"
+  }
+}`
+    );
+    await getProcessResult(
+      execFile(npmPath, [
+        "install",
+        "--prefix",
+        projectDir,
+        "--cache",
+        npmCacheDir,
+        corePackageTarPath
+      ])
+    );
+
+    // Create a correctly formatted file
+    const formattedFilePath = path.join(projectDir, "definitions", "formatted.sqlx");
+    fs.ensureFileSync(formattedFilePath);
+    fs.writeFileSync(
+      formattedFilePath,
+      `
+config {
+  type: "table"
+}
+
+SELECT
+  1 AS test
+`
+    );
+
+    // Create a file that needs formatting (extra spaces, inconsistent indentation)
+    const unformattedFilePath = path.join(projectDir, "definitions", "unformatted.sqlx");
+    fs.ensureFileSync(unformattedFilePath);
+    fs.writeFileSync(
+      unformattedFilePath,
+      `
+config {   type:  "table"   }
+SELECT  1  as   test
+`
+    );
+
+    // Test with --check flag on a project with files needing formatting
+    const beforeFormatCheckResult = await getProcessResult(
+      execFile(nodePath, [cliEntryPointPath, "format", projectDir, "--check"])
+    );
+
+    // Should exit with code 1 when files need formatting
+    expect(beforeFormatCheckResult.exitCode).equals(1);
+    expect(beforeFormatCheckResult.stderr).contains("Files that need formatting");
+    expect(beforeFormatCheckResult.stderr).contains("unformatted.sqlx");
+
+    // Format the files (without check flag)
+    const formatCheckResult = await getProcessResult(
+      execFile(nodePath, [cliEntryPointPath, "format", projectDir])
+    );
+    expect(formatCheckResult.exitCode).equals(0);
+
+    // Test with --check flag after formatting
+    const afterFormatCheckResult = await getProcessResult(
+      execFile(nodePath, [cliEntryPointPath, "format", projectDir, "--check"])
+    );
+
+    // Should exit with code 0 when all files are properly formatted
+    expect(afterFormatCheckResult.exitCode).equals(0);
+    expect(afterFormatCheckResult.stdout).contains("All files are formatted correctly");
+  });
 });