Merge #1449

1449: Swallow expected `rustfmt` errors r=matklad a=etaoins

My workflow in Visual Studio Code + Rust Analyzer has become:

1. Make a change to Rust source code using all the analysis magic

2. Save the file to trigger `cargo watch`. I have format on save enabled for all file types so this also runs `rustfmt`

3. Fix any diagnostics that `cargo watch` finds

Unfortunately if the Rust source has any syntax errors the act of saving will pop up a scary "command has failed" message and will switch to the "Output" tab to show the `rustfmt` error and exit code.

I did a quick survey of what other Language Servers do in this case. Both the JSON and TypeScript servers will swallow the error and return success. This is consistent with how I remember my workflow in those languages. The syntax error will show up as a diagnostic so it should be clear why the file isn't formatting.

I checked the `rustfmt` source code and while it does distinguish "parse errors" from "operational errors" internally they both result in exit status of 1. However, more catastrophic errors (missing `rustfmt`, SIGSEGV, etc) will return 127+ error codes which we can distinguish from a normal failure.

This changes our handler to log an info message and feign success if `rustfmt` exits with status 1.

Another option I considered was only swallowing the error if the formatting request came from format-on-save. However, the Language Server Protocol doesn't seem to distinguish those cases.

Co-authored-by: Ryan Cumming <[email protected]>

1 files changed, 25 insertions, 10 deletions

diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs
index 6373240d5..47222cd0a 100644
--- a/crates/ra_lsp_server/src/main_loop/handlers.rs
+++ b/crates/ra_lsp_server/src/main_loop/handlers.rs
@@ -621,17 +621,32 @@ pub fn handle_formatting(
 
     let output = rustfmt.wait_with_output()?;
     let captured_stdout = String::from_utf8(output.stdout)?;
+
     if !output.status.success() {
-        return Err(LspError::new(
-            -32900,
-            format!(
-                r#"rustfmt exited with:
-            Status: {}
-            stdout: {}"#,
-                output.status, captured_stdout,
-            ),
-        )
-        .into());
+        match output.status.code() {
+            Some(1) => {
+                // 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);
+            }
+            _ => {
+                // Something else happened - e.g. `rustfmt` is missing or caught a signal
+                return Err(LspError::new(
+                    -32900,
+                    format!(
+                        r#"rustfmt exited with:
+                           Status: {}
+                           stdout: {}"#,
+                        output.status, captured_stdout,
+                    ),
+                )
+                .into());
+            }
+        }
     }
 
     Ok(Some(vec![TextEdit {