Swallow expected `rustfmt` errors

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.

diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs
index 6373240d5f9f091c52c14eadcb30a78966e4678d..47222cd0ac209c5e3b380c4563a42748bc1886f6 100644 (file)
--- 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 {