Skip to content

Commit bc2e0f8

Browse files
leaysgurleaysgur
committed
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.
1 parent bbec437 commit bc2e0f8

File tree

8 files changed

+56
-54
lines changed

8 files changed

+56
-54
lines changed

‎apps/oxfmt/src-js/bindings.d.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
* 3. `format_embedded_cb`: Callback to format embedded code in templates
1010
* 4. `format_file_cb`: Callback to format files
1111
*
12-
* Returns `true` if formatting succeeded without errors, `false` otherwise.
12+
* Returns a tuple of `[mode, exitCode]`:
13+
* - `mode`: "cli" | "lsp" | "init"
14+
* - `exitCode`: exit code (0, 1, 2) for "cli" mode, `undefined` for other modes
1315
*/
14-
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>
16+
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]>

‎apps/oxfmt/src-js/bindings.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -575,5 +575,5 @@ if (!nativeBinding) {
575575
throw new Error(`Failed to load native binding`)
576576
}
577577

578-
const { format } = nativeBinding
579-
export { format }
578+
const { runCli } = nativeBinding
579+
export { runCli }

‎apps/oxfmt/src-js/cli.ts‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,27 @@
1-
import { format } from "./bindings.js";
1+
import { runCli } from "./bindings.js";
22
import { setupConfig, formatEmbeddedCode, formatFile } from "./prettier-proxy.js";
33
import { runInit } from "./migration/init.js";
44

55
void (async () => {
66
const args = process.argv.slice(2);
77

8-
// Handle `--init` command in JS
9-
if (args.includes("--init")) {
10-
return await runInit();
11-
}
12-
13-
// Call the Rust formatter with our JS callback
14-
const success = await format(args, setupConfig, formatEmbeddedCode, formatFile);
8+
// Call the Rust CLI to parse args and determine mode
9+
const [mode, exitCode] = await runCli(args, setupConfig, formatEmbeddedCode, formatFile);
1510

16-
// NOTE: It's recommended to set `process.exitCode` instead of calling `process.exit()`.
17-
// `process.exit()` kills the process immediately and `stdout` may not be flushed before process dies.
18-
// https://nodejs.org/api/process.html#processexitcode
19-
if (!success) process.exitCode = 1;
11+
switch (mode) {
12+
// Handle `--init` command in JS
13+
case "init":
14+
await runInit();
15+
break;
16+
// LSP mode is handled by Rust, nothing to do here
17+
case "lsp":
18+
break;
19+
// CLI mode also handled by Rust, just need to set exit code
20+
case "cli":
21+
// NOTE: It's recommended to set `process.exitCode` instead of calling `process.exit()`.
22+
// `process.exit()` kills the process immediately and `stdout` may not be flushed before process dies.
23+
// https://nodejs.org/api/process.html#processexitcode
24+
if (exitCode) process.exitCode = exitCode;
25+
break;
26+
}
2027
})();

‎apps/oxfmt/src/cli/result.rs‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,18 @@ pub enum CliRunResult {
1313
FormatFailed,
1414
}
1515

16-
impl Termination for CliRunResult {
17-
fn report(self) -> ExitCode {
16+
impl CliRunResult {
17+
pub fn exit_code(&self) -> u8 {
1818
match self {
19-
Self::None | Self::FormatSucceeded => ExitCode::from(0),
20-
Self::InvalidOptionConfig | Self::FormatMismatch => ExitCode::from(1),
21-
Self::NoFilesFound | Self::FormatFailed => ExitCode::from(2),
19+
Self::None | Self::FormatSucceeded => 0,
20+
Self::InvalidOptionConfig | Self::FormatMismatch => 1,
21+
Self::NoFilesFound | Self::FormatFailed => 2,
2222
}
2323
}
2424
}
25+
26+
impl Termination for CliRunResult {
27+
fn report(self) -> ExitCode {
28+
ExitCode::from(self.exit_code())
29+
}
30+
}

‎apps/oxfmt/src/main.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use oxfmt::cli::{
44

55
// Pure Rust CLI entry point.
66
// This CLI only supports the basic `Cli` mode.
7-
// For full featured JS CLI entry point, see `format()` exported by `main_napi.rs`.
7+
// For full featured JS CLI entry point, see `run_cli()` exported by `main_napi.rs`.
88

99
#[tokio::main]
1010
async fn main() -> CliRunResult {

‎apps/oxfmt/src/main_napi.rs‎

Lines changed: 16 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,9 @@
1-
use std::{
2-
ffi::OsString,
3-
process::{ExitCode, Termination},
4-
};
1+
use std::ffi::OsString;
52

63
use napi_derive::napi;
74

85
use crate::{
9-
cli::{
10-
CliRunResult, FormatRunner, Mode, format_command, init_miette, init_rayon, init_tracing,
11-
},
6+
cli::{FormatRunner, Mode, format_command, init_miette, init_rayon, init_tracing},
127
core::{ExternalFormatter, JsFormatEmbeddedCb, JsFormatFileCb, JsSetupConfigCb},
138
lsp::run_lsp,
149
};
@@ -24,11 +19,13 @@ use crate::{
2419
/// 3. `format_embedded_cb`: Callback to format embedded code in templates
2520
/// 4. `format_file_cb`: Callback to format files
2621
///
27-
/// Returns `true` if formatting succeeded without errors, `false` otherwise.
22+
/// Returns a tuple of `[mode, exitCode]`:
23+
/// - `mode`: "cli" | "lsp" | "init"
24+
/// - `exitCode`: exit code (0, 1, 2) for "cli" mode, `undefined` for other modes
2825
#[expect(clippy::allow_attributes)]
2926
#[allow(clippy::trailing_empty_array, clippy::unused_async)] // https://github.com/napi-rs/napi-rs/issues/2758
3027
#[napi]
31-
pub async fn format(
28+
pub async fn run_cli(
3229
args: Vec<String>,
3330
#[napi(ts_arg_type = "(configJSON: string, numThreads: number) => Promise<string[]>")]
3431
setup_config_cb: JsSetupConfigCb,
@@ -38,18 +35,7 @@ pub async fn format(
3835
ts_arg_type = "(parserName: string, fileName: string, code: string) => Promise<string>"
3936
)]
4037
format_file_cb: JsFormatFileCb,
41-
) -> bool {
42-
format_impl(args, setup_config_cb, format_embedded_cb, format_file_cb).await.report()
43-
== ExitCode::SUCCESS
44-
}
45-
46-
/// Run the formatter.
47-
async fn format_impl(
48-
args: Vec<String>,
49-
setup_config_cb: JsSetupConfigCb,
50-
format_embedded_cb: JsFormatEmbeddedCb,
51-
format_file_cb: JsFormatFileCb,
52-
) -> CliRunResult {
38+
) -> (String, Option<u8>) {
5339
// Convert String args to OsString for compatibility with bpaf
5440
let args: Vec<OsString> = args.into_iter().map(OsString::from).collect();
5541

@@ -58,19 +44,17 @@ async fn format_impl(
5844
Ok(cmd) => cmd,
5945
Err(e) => {
6046
e.print_message(100);
61-
return if e.exit_code() == 0 {
62-
CliRunResult::None
63-
} else {
64-
CliRunResult::InvalidOptionConfig
65-
};
47+
// `bpaf` returns exit_code 0 for --help/--version, non-0 for parse errors
48+
let exit_code = u8::from(e.exit_code() != 0);
49+
return ("cli".to_string(), Some(exit_code));
6650
}
6751
};
6852

6953
match command.mode {
70-
Mode::Init => unreachable!("`--init` should be handled by JS side"),
54+
Mode::Init => ("init".to_string(), None),
7155
Mode::Lsp => {
7256
run_lsp().await;
73-
CliRunResult::None
57+
("lsp".to_string(), None)
7458
}
7559
Mode::Cli(_) => {
7660
init_tracing();
@@ -81,7 +65,10 @@ async fn format_impl(
8165
let external_formatter =
8266
ExternalFormatter::new(setup_config_cb, format_embedded_cb, format_file_cb);
8367

84-
FormatRunner::new(command).with_external_formatter(Some(external_formatter)).run()
68+
let result =
69+
FormatRunner::new(command).with_external_formatter(Some(external_formatter)).run();
70+
71+
("cli".to_string(), Some(result.exit_code()))
8572
}
8673
}
8774
}

‎apps/oxfmt/test/__snapshots__/ignore_and_override.test.ts.snap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ Finished in <variable>ms on 1 files using 1 threads.
1818
--------------------
1919
arguments: --check --ignore-path ignore1
2020
working directory: fixtures/ignore_and_override
21-
exit code: 1
21+
exit code: 2
2222
--- STDOUT ---------
2323
Checking formatting...
2424
@@ -28,7 +28,7 @@ Expected at least one target file
2828
--------------------
2929
arguments: --check --ignore-path ignore1 should_format/ok.js
3030
working directory: fixtures/ignore_and_override
31-
exit code: 1
31+
exit code: 2
3232
--- STDOUT ---------
3333
3434
--- STDERR ---------

‎apps/oxfmt/test/__snapshots__/no_error_on_unmatched_pattern.test.ts.snap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ No files found matching the given patterns.
1515
--------------------
1616
arguments: --check __non__existent__file.js
1717
working directory: fixtures
18-
exit code: 1
18+
exit code: 2
1919
--- STDOUT ---------
2020
Checking formatting...
2121

0 commit comments

Comments
 (0)