From bc2e0f8a123e8aea2ed046dca97b6058faaa10c3 Mon Sep 17 00:00:00 2001
From: leaysgur <6259812+leaysgur@users.noreply.github.com>
Date: Fri, 12 Dec 2025 05:56:10 +0000
Subject: [PATCH] fix(oxfmt): Report `exitCode` correctly (#16770)

Previously,

- `oxfmt(napi) not-exists.js` returns `exitCode: 1`
- `oxfmt(rust) not-exists.js` returns `exitCode: 2` = Expected

To fix this, I updated napi callback return value from `bool`.

Additionally, I realized that by returning the processing `Mode` simultaneously, Rust can handle the CLI arguments in better way. Like `--init --help`, which now shows help.

And finally changed napi binding name from `format()` to `runCli()` for the future Node API.
---
 apps/oxfmt/src-js/bindings.d.ts               |  6 ++-
 apps/oxfmt/src-js/bindings.js                 |  4 +-
 apps/oxfmt/src-js/cli.ts                      | 31 ++++++++-----
 apps/oxfmt/src/cli/result.rs                  | 16 ++++---
 apps/oxfmt/src/main.rs                        |  2 +-
 apps/oxfmt/src/main_napi.rs                   | 45 +++++++------------
 .../ignore_and_override.test.ts.snap          |  4 +-
 ...no_error_on_unmatched_pattern.test.ts.snap |  2 +-
 8 files changed, 56 insertions(+), 54 deletions(-)

diff --git a/apps/oxfmt/src-js/bindings.d.ts b/apps/oxfmt/src-js/bindings.d.ts
index 52eee40f94132..5a5213f3093a7 100644
--- a/apps/oxfmt/src-js/bindings.d.ts
+++ b/apps/oxfmt/src-js/bindings.d.ts
@@ -9,6 +9,8 @@
  * 3. `format_embedded_cb`: Callback to format embedded code in templates
  * 4. `format_file_cb`: Callback to format files
  *
- * Returns `true` if formatting succeeded without errors, `false` otherwise.
+ * Returns a tuple of `[mode, exitCode]`:
+ * - `mode`: "cli" | "lsp" | "init"
+ * - `exitCode`: exit code (0, 1, 2) for "cli" mode, `undefined` for other modes
  */
-export declare function format(args: Array<string>, setupConfigCb: (configJSON: string, numThreads: number) => Promise<string[]>, formatEmbeddedCb: (tagName: string, code: string) => Promise<string>, formatFileCb: (parserName: string, fileName: string, code: string) => Promise<string>): Promise<boolean>
+export declare function runCli(args: Array<string>, setupConfigCb: (configJSON: string, numThreads: number) => Promise<string[]>, formatEmbeddedCb: (tagName: string, code: string) => Promise<string>, formatFileCb: (parserName: string, fileName: string, code: string) => Promise<string>): Promise<[string, number | undefined | null]>
diff --git a/apps/oxfmt/src-js/bindings.js b/apps/oxfmt/src-js/bindings.js
index 19d9210aa778c..24116188198c4 100644
--- a/apps/oxfmt/src-js/bindings.js
+++ b/apps/oxfmt/src-js/bindings.js
@@ -575,5 +575,5 @@ if (!nativeBinding) {
   throw new Error(`Failed to load native binding`)
 }
 
-const { format } = nativeBinding
-export { format }
+const { runCli } = nativeBinding
+export { runCli }
diff --git a/apps/oxfmt/src-js/cli.ts b/apps/oxfmt/src-js/cli.ts
index 8b0bb3e973a0f..50a7c818c5979 100644
--- a/apps/oxfmt/src-js/cli.ts
+++ b/apps/oxfmt/src-js/cli.ts
@@ -1,20 +1,27 @@
-import { format } from "./bindings.js";
+import { runCli } from "./bindings.js";
 import { setupConfig, formatEmbeddedCode, formatFile } from "./prettier-proxy.js";
 import { runInit } from "./migration/init.js";
 
 void (async () => {
   const args = process.argv.slice(2);
 
-  // Handle `--init` command in JS
-  if (args.includes("--init")) {
-    return await runInit();
-  }
-
-  // Call the Rust formatter with our JS callback
-  const success = await format(args, setupConfig, formatEmbeddedCode, formatFile);
+  // Call the Rust CLI to parse args and determine mode
+  const [mode, exitCode] = await runCli(args, setupConfig, formatEmbeddedCode, formatFile);
 
-  // NOTE: It's recommended to set `process.exitCode` instead of calling `process.exit()`.
-  // `process.exit()` kills the process immediately and `stdout` may not be flushed before process dies.
-  // https://nodejs.org/api/process.html#processexitcode
-  if (!success) process.exitCode = 1;
+  switch (mode) {
+    // Handle `--init` command in JS
+    case "init":
+      await runInit();
+      break;
+    // LSP mode is handled by Rust, nothing to do here
+    case "lsp":
+      break;
+    // CLI mode also handled by Rust, just need to set exit code
+    case "cli":
+      // NOTE: It's recommended to set `process.exitCode` instead of calling `process.exit()`.
+      // `process.exit()` kills the process immediately and `stdout` may not be flushed before process dies.
+      // https://nodejs.org/api/process.html#processexitcode
+      if (exitCode) process.exitCode = exitCode;
+      break;
+  }
 })();
diff --git a/apps/oxfmt/src/cli/result.rs b/apps/oxfmt/src/cli/result.rs
index 402223f5136d4..b4df953be941c 100644
--- a/apps/oxfmt/src/cli/result.rs
+++ b/apps/oxfmt/src/cli/result.rs
@@ -13,12 +13,18 @@ pub enum CliRunResult {
     FormatFailed,
 }
 
-impl Termination for CliRunResult {
-    fn report(self) -> ExitCode {
+impl CliRunResult {
+    pub fn exit_code(&self) -> u8 {
         match self {
-            Self::None | Self::FormatSucceeded => ExitCode::from(0),
-            Self::InvalidOptionConfig | Self::FormatMismatch => ExitCode::from(1),
-            Self::NoFilesFound | Self::FormatFailed => ExitCode::from(2),
+            Self::None | Self::FormatSucceeded => 0,
+            Self::InvalidOptionConfig | Self::FormatMismatch => 1,
+            Self::NoFilesFound | Self::FormatFailed => 2,
         }
     }
 }
+
+impl Termination for CliRunResult {
+    fn report(self) -> ExitCode {
+        ExitCode::from(self.exit_code())
+    }
+}
diff --git a/apps/oxfmt/src/main.rs b/apps/oxfmt/src/main.rs
index 9f449e2a4c47a..f9fecaf442bc7 100644
--- a/apps/oxfmt/src/main.rs
+++ b/apps/oxfmt/src/main.rs
@@ -4,7 +4,7 @@ use oxfmt::cli::{
 
 // Pure Rust CLI entry point.
 // This CLI only supports the basic `Cli` mode.
-// For full featured JS CLI entry point, see `format()` exported by `main_napi.rs`.
+// For full featured JS CLI entry point, see `run_cli()` exported by `main_napi.rs`.
 
 #[tokio::main]
 async fn main() -> CliRunResult {
diff --git a/apps/oxfmt/src/main_napi.rs b/apps/oxfmt/src/main_napi.rs
index 03048af485f48..4098c3f9d993d 100644
--- a/apps/oxfmt/src/main_napi.rs
+++ b/apps/oxfmt/src/main_napi.rs
@@ -1,14 +1,9 @@
-use std::{
-    ffi::OsString,
-    process::{ExitCode, Termination},
-};
+use std::ffi::OsString;
 
 use napi_derive::napi;
 
 use crate::{
-    cli::{
-        CliRunResult, FormatRunner, Mode, format_command, init_miette, init_rayon, init_tracing,
-    },
+    cli::{FormatRunner, Mode, format_command, init_miette, init_rayon, init_tracing},
     core::{ExternalFormatter, JsFormatEmbeddedCb, JsFormatFileCb, JsSetupConfigCb},
     lsp::run_lsp,
 };
@@ -24,11 +19,13 @@ use crate::{
 /// 3. `format_embedded_cb`: Callback to format embedded code in templates
 /// 4. `format_file_cb`: Callback to format files
 ///
-/// Returns `true` if formatting succeeded without errors, `false` otherwise.
+/// Returns a tuple of `[mode, exitCode]`:
+/// - `mode`: "cli" | "lsp" | "init"
+/// - `exitCode`: exit code (0, 1, 2) for "cli" mode, `undefined` for other modes
 #[expect(clippy::allow_attributes)]
 #[allow(clippy::trailing_empty_array, clippy::unused_async)] // https://github.com/napi-rs/napi-rs/issues/2758
 #[napi]
-pub async fn format(
+pub async fn run_cli(
     args: Vec<String>,
     #[napi(ts_arg_type = "(configJSON: string, numThreads: number) => Promise<string[]>")]
     setup_config_cb: JsSetupConfigCb,
@@ -38,18 +35,7 @@ pub async fn format(
         ts_arg_type = "(parserName: string, fileName: string, code: string) => Promise<string>"
     )]
     format_file_cb: JsFormatFileCb,
-) -> bool {
-    format_impl(args, setup_config_cb, format_embedded_cb, format_file_cb).await.report()
-        == ExitCode::SUCCESS
-}
-
-/// Run the formatter.
-async fn format_impl(
-    args: Vec<String>,
-    setup_config_cb: JsSetupConfigCb,
-    format_embedded_cb: JsFormatEmbeddedCb,
-    format_file_cb: JsFormatFileCb,
-) -> CliRunResult {
+) -> (String, Option<u8>) {
     // Convert String args to OsString for compatibility with bpaf
     let args: Vec<OsString> = args.into_iter().map(OsString::from).collect();
 
@@ -58,19 +44,17 @@ async fn format_impl(
         Ok(cmd) => cmd,
         Err(e) => {
             e.print_message(100);
-            return if e.exit_code() == 0 {
-                CliRunResult::None
-            } else {
-                CliRunResult::InvalidOptionConfig
-            };
+            // `bpaf` returns exit_code 0 for --help/--version, non-0 for parse errors
+            let exit_code = u8::from(e.exit_code() != 0);
+            return ("cli".to_string(), Some(exit_code));
         }
     };
 
     match command.mode {
-        Mode::Init => unreachable!("`--init` should be handled by JS side"),
+        Mode::Init => ("init".to_string(), None),
         Mode::Lsp => {
             run_lsp().await;
-            CliRunResult::None
+            ("lsp".to_string(), None)
         }
         Mode::Cli(_) => {
             init_tracing();
@@ -81,7 +65,10 @@ async fn format_impl(
             let external_formatter =
                 ExternalFormatter::new(setup_config_cb, format_embedded_cb, format_file_cb);
 
-            FormatRunner::new(command).with_external_formatter(Some(external_formatter)).run()
+            let result =
+                FormatRunner::new(command).with_external_formatter(Some(external_formatter)).run();
+
+            ("cli".to_string(), Some(result.exit_code()))
         }
     }
 }
diff --git a/apps/oxfmt/test/__snapshots__/ignore_and_override.test.ts.snap b/apps/oxfmt/test/__snapshots__/ignore_and_override.test.ts.snap
index ee3512e251679..fb23f8602dad3 100644
--- a/apps/oxfmt/test/__snapshots__/ignore_and_override.test.ts.snap
+++ b/apps/oxfmt/test/__snapshots__/ignore_and_override.test.ts.snap
@@ -18,7 +18,7 @@ Finished in <variable>ms on 1 files using 1 threads.
 --------------------
 arguments: --check --ignore-path ignore1
 working directory: fixtures/ignore_and_override
-exit code: 1
+exit code: 2
 --- STDOUT ---------
 Checking formatting...
 
@@ -28,7 +28,7 @@ Expected at least one target file
 --------------------
 arguments: --check --ignore-path ignore1 should_format/ok.js
 working directory: fixtures/ignore_and_override
-exit code: 1
+exit code: 2
 --- STDOUT ---------
 
 --- STDERR ---------
diff --git a/apps/oxfmt/test/__snapshots__/no_error_on_unmatched_pattern.test.ts.snap b/apps/oxfmt/test/__snapshots__/no_error_on_unmatched_pattern.test.ts.snap
index 2f24fadac8b42..33203a98a0fe1 100644
--- a/apps/oxfmt/test/__snapshots__/no_error_on_unmatched_pattern.test.ts.snap
+++ b/apps/oxfmt/test/__snapshots__/no_error_on_unmatched_pattern.test.ts.snap
@@ -15,7 +15,7 @@ No files found matching the given patterns.
 --------------------
 arguments: --check __non__existent__file.js
 working directory: fixtures
-exit code: 1
+exit code: 2
 --- STDOUT ---------
 Checking formatting...