aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAleksey Kladov <[email protected]>2021-04-05 11:02:47 +0100
committerAleksey Kladov <[email protected]>2021-04-05 11:02:47 +0100
commita01fd1af19314a73e5436a912f1cac2341ea11cd (patch)
tree19d03d2f7516f29f65f1e1de8ee1ab2254f2e98c
parent58924cfae1be231e01736407fa666f31898a699e (diff)
internal: explain "extract if condition" refactoring
-rw-r--r--crates/rust-analyzer/src/handlers.rs18
-rw-r--r--docs/dev/style.md21
2 files changed, 29 insertions, 10 deletions
diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs
index e8f9f2179..4d10a2ead 100644
--- a/crates/rust-analyzer/src/handlers.rs
+++ b/crates/rust-analyzer/src/handlers.rs
@@ -927,22 +927,22 @@ pub(crate) fn handle_formatting(
927 let captured_stderr = String::from_utf8(output.stderr).unwrap_or_default(); 927 let captured_stderr = String::from_utf8(output.stderr).unwrap_or_default();
928 928
929 if !output.status.success() { 929 if !output.status.success() {
930 match output.status.code() { 930 let rustfmt_not_installed =
931 Some(1) 931 captured_stderr.contains("not installed") || captured_stderr.contains("not available");
932 if !captured_stderr.contains("not installed") 932
933 && !captured_stderr.contains("not available") => 933 return match output.status.code() {
934 { 934 Some(1) if !rustfmt_not_installed => {
935 // While `rustfmt` doesn't have a specific exit code for parse errors this is the 935 // While `rustfmt` doesn't have a specific exit code for parse errors this is the
936 // likely cause exiting with 1. Most Language Servers swallow parse errors on 936 // likely cause exiting with 1. Most Language Servers swallow parse errors on
937 // formatting because otherwise an error is surfaced to the user on top of the 937 // formatting because otherwise an error is surfaced to the user on top of the
938 // syntax error diagnostics they're already receiving. This is especially jarring 938 // syntax error diagnostics they're already receiving. This is especially jarring
939 // if they have format on save enabled. 939 // if they have format on save enabled.
940 log::info!("rustfmt exited with status 1, assuming parse error and ignoring"); 940 log::info!("rustfmt exited with status 1, assuming parse error and ignoring");
941 return Ok(None); 941 Ok(None)
942 } 942 }
943 _ => { 943 _ => {
944 // Something else happened - e.g. `rustfmt` is missing or caught a signal 944 // Something else happened - e.g. `rustfmt` is missing or caught a signal
945 return Err(LspError::new( 945 Err(LspError::new(
946 -32900, 946 -32900,
947 format!( 947 format!(
948 r#"rustfmt exited with: 948 r#"rustfmt exited with:
@@ -952,9 +952,9 @@ pub(crate) fn handle_formatting(
952 output.status, captured_stdout, captured_stderr, 952 output.status, captured_stdout, captured_stderr,
953 ), 953 ),
954 ) 954 )
955 .into()); 955 .into())
956 } 956 }
957 } 957 };
958 } 958 }
959 959
960 let (new_text, new_line_endings) = LineEndings::normalize(captured_stdout); 960 let (new_text, new_line_endings) = LineEndings::normalize(captured_stdout);
diff --git a/docs/dev/style.md b/docs/dev/style.md
index c594946be..48ce4b92a 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.
842 842
843## Helper Variables 843## Helper Variables
844 844
845Introduce helper variables freely, especially for multiline conditions. 845Introduce helper variables freely, especially for multiline conditions:
846
847```rust
848// GOOD
849let rustfmt_not_installed =
850 captured_stderr.contains("not installed") || captured_stderr.contains("not available");
851
852match output.status.code() {
853 Some(1) if !rustfmt_not_installed => Ok(None),
854 _ => Err(format_err!("rustfmt failed:\n{}", captured_stderr)),
855};
856
857// BAD
858match output.status.code() {
859 Some(1)
860 if !captured_stderr.contains("not installed")
861 && !captured_stderr.contains("not available") => Ok(None),
862 _ => Err(format_err!("rustfmt failed:\n{}", captured_stderr)),
863};
864```
846 865
847**Rationale:** like blocks, single-use variables are a cognitively cheap abstraction, as they have access to all the context. 866**Rationale:** like blocks, single-use variables are a cognitively cheap abstraction, as they have access to all the context.
848Extra variables help during debugging, they make it easy to print/view important intermediate results. 867Extra variables help during debugging, they make it easy to print/view important intermediate results.