diff --git a/apps/oxlint/src/js_plugins/external_linter.rs b/apps/oxlint/src/js_plugins/external_linter.rs index ede3cf478723d..8a9b9493a7397 100644 --- a/apps/oxlint/src/js_plugins/external_linter.rs +++ b/apps/oxlint/src/js_plugins/external_linter.rs @@ -1,7 +1,4 @@ -use std::{ - error::Error, - sync::{atomic::Ordering, mpsc::channel}, -}; +use std::sync::{atomic::Ordering, mpsc::channel}; use napi::{ Status, @@ -45,17 +42,23 @@ fn wrap_load_plugin(cb: JsLoadPluginCb) -> ExternalLinterLoadPluginCb { let cb = &cb; let res = tokio::task::block_in_place(|| { tokio::runtime::Handle::current().block_on(async move { - let result = cb - .call_async(FnArgs::from((plugin_url, package_name))) - .await? - .into_future() - .await?; - let plugin_load_result: PluginLoadResult = serde_json::from_str(&result)?; - Ok(plugin_load_result) + cb.call_async(FnArgs::from((plugin_url, package_name))).await?.into_future().await }) }); - res.map_err(|err: Box| format!("Error in `loadPlugin` callback: {err:?}")) + match res { + // `loadPlugin` returns JSON string if plugin loaded successfully, or an error occurred + Ok(json) => match serde_json::from_str::(&json) { + // Plugin loaded successfully, or error occurred on JS side + Ok(result) => Ok(result), + // Invalid JSON - should be impossible, because we control serialization on JS side + Err(err) => { + Err(format!("Failed to deserialize JSON returned by `loadPlugin`: {err}")) + } + }, + // `loadPlugin` threw an error - should be impossible because `loadPlugin` is wrapped in try-catch + Err(err) => Err(format!("`loadPlugin` threw an error: {err}")), + } }) } @@ -80,19 +83,19 @@ fn wrap_setup_configs(cb: JsSetupConfigsCb) -> ExternalLinterSetupConfigsCb { options_json, ThreadsafeFunctionCallMode::NonBlocking, move |result, _env| { - let _ = match &result { - Ok(()) => tx.send(Ok(())), - Err(e) => tx.send(Err(e.to_string())), - }; - result + let _ = tx.send(result); + Ok(()) }, ); if status == Status::Ok { match rx.recv() { + // Setup succeeded Ok(Ok(())) => Ok(()), - Ok(Err(err)) => Err(format!("`setupConfigs` callback reported error: {err}")), - Err(err) => Err(format!("`setupConfigs` callback did not respond: {err}")), + // `setupConfigs` threw an error - should be impossible because it should be infallible + Ok(Err(err)) => Err(format!("`setupConfigs` threw an error: {err}")), + // Failure calling JS function + Err(err) => Err(format!("`setupConfigs` did not respond: {err}")), } } else { Err(format!("Failed to schedule `setupConfigs` callback: {status:?}")) @@ -132,29 +135,32 @@ fn wrap_lint_file(cb: JsLintFileCb) -> ExternalLinterLintFileCb { FnArgs::from((file_path, buffer_id, buffer, rule_ids, options_ids, settings_json)), ThreadsafeFunctionCallMode::NonBlocking, move |result, _env| { - let _ = match &result { - Ok(None) => tx.send(Ok(LintFileReturnValue::Success(Vec::new()))), - Ok(Some(r)) => match serde_json::from_str::(r) { - Ok(v) => tx.send(Ok(v)), - Err(_e) => { - tx.send(Err("Failed to deserialize lint result".to_string())) - } - }, - Err(e) => tx.send(Err(e.to_string())), - }; - - result.map(|_| ()) + let _ = tx.send(result); + Ok(()) }, ); if status == Status::Ok { match rx.recv() { - Ok(Ok(x)) => match x { - LintFileReturnValue::Success(diagnostics) => Ok(diagnostics), - LintFileReturnValue::Failure(err) => Err(err), - }, - Ok(Err(err)) => Err(format!("`lintFile` callback reported error: {err}")), - Err(err) => Err(format!("`lintFile` callback did not respond: {err}")), + // `lintFile` returns `null` if no diagnostics reported, and no error occurred + Ok(Ok(None)) => Ok(Vec::new()), + // `lintFile` returns JSON string if diagnostics reported, or an error occurred + Ok(Ok(Some(json))) => { + match serde_json::from_str(&json) { + // Diagnostics reported + Ok(LintFileReturnValue::Success(diagnostics)) => Ok(diagnostics), + // Error occurred on JS side + Ok(LintFileReturnValue::Failure(err)) => Err(err), + // Invalid JSON - should be impossible, because we control serialization on JS side + Err(err) => Err(format!( + "Failed to deserialize JSON returned by `lintFile`: {err}" + )), + } + } + // `lintFile` threw an error - should be impossible because `lintFile` is wrapped in try-catch + Ok(Err(err)) => Err(format!("`lintFile` threw an error: {err}")), + // Failure calling JS function + Err(err) => Err(format!("`lintFile` did not respond: {err}")), } } else { Err(format!("Failed to schedule `lintFile` callback: {status:?}"))