diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index e8f9f21794..4d10a2eadc 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -927,22 +927,22 @@ pub(crate) fn handle_formatting( let captured_stderr = String::from_utf8(output.stderr).unwrap_or_default(); if !output.status.success() { - match output.status.code() { - Some(1) - if !captured_stderr.contains("not installed") - && !captured_stderr.contains("not available") => - { + let rustfmt_not_installed = + captured_stderr.contains("not installed") || captured_stderr.contains("not available"); + + return match output.status.code() { + Some(1) if !rustfmt_not_installed => { // While `rustfmt` doesn't have a specific exit code for parse errors this is the // likely cause exiting with 1. Most Language Servers swallow parse errors on // formatting because otherwise an error is surfaced to the user on top of the // syntax error diagnostics they're already receiving. This is especially jarring // if they have format on save enabled. log::info!("rustfmt exited with status 1, assuming parse error and ignoring"); - return Ok(None); + Ok(None) } _ => { // Something else happened - e.g. `rustfmt` is missing or caught a signal - return Err(LspError::new( + Err(LspError::new( -32900, format!( r#"rustfmt exited with: @@ -952,9 +952,9 @@ pub(crate) fn handle_formatting( output.status, captured_stdout, captured_stderr, ), ) - .into()); + .into()) } - } + }; } let (new_text, new_line_endings) = LineEndings::normalize(captured_stdout); diff --git a/docs/dev/style.md b/docs/dev/style.md index c594946be7..48ce4b92a6 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md @@ -842,7 +842,26 @@ Re-using originally single-purpose function often leads to bad coupling. ## Helper Variables -Introduce helper variables freely, especially for multiline conditions. +Introduce helper variables freely, especially for multiline conditions: + +```rust +// GOOD +let rustfmt_not_installed = + captured_stderr.contains("not installed") || captured_stderr.contains("not available"); + +match output.status.code() { + Some(1) if !rustfmt_not_installed => Ok(None), + _ => Err(format_err!("rustfmt failed:\n{}", captured_stderr)), +}; + +// BAD +match output.status.code() { + Some(1) + if !captured_stderr.contains("not installed") + && !captured_stderr.contains("not available") => Ok(None), + _ => Err(format_err!("rustfmt failed:\n{}", captured_stderr)), +}; +``` **Rationale:** like blocks, single-use variables are a cognitively cheap abstraction, as they have access to all the context. Extra variables help during debugging, they make it easy to print/view important intermediate results.