diff options
Diffstat (limited to 'docs/dev/style.md')
-rw-r--r-- | docs/dev/style.md | 64 |
1 files changed, 61 insertions, 3 deletions
diff --git a/docs/dev/style.md b/docs/dev/style.md index e4a1672ca..48ce4b92a 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md | |||
@@ -55,9 +55,9 @@ https://www.tedinski.com/2018/02/06/system-boundaries.html | |||
55 | We try to be very conservative with usage of crates.io dependencies. | 55 | We try to be very conservative with usage of crates.io dependencies. |
56 | Don't use small "helper" crates (exception: `itertools` is allowed). | 56 | Don't use small "helper" crates (exception: `itertools` is allowed). |
57 | If there's some general reusable bit of code you need, consider adding it to the `stdx` crate. | 57 | If there's some general reusable bit of code you need, consider adding it to the `stdx` crate. |
58 | A useful exercise is to read Cargo.lock and see if some of the *transitive* dependencies do not make sense for rust-analyzer. | ||
58 | 59 | ||
59 | **Rationale:** keep compile times low, create ecosystem pressure for faster | 60 | **Rationale:** keep compile times low, create ecosystem pressure for faster compiles, reduce the number of things which might break. |
60 | compiles, reduce the number of things which might break. | ||
61 | 61 | ||
62 | ## Commit Style | 62 | ## Commit Style |
63 | 63 | ||
@@ -806,9 +806,67 @@ if let Some(expected_type) = ctx.expected_type.as_ref() { | |||
806 | } | 806 | } |
807 | ``` | 807 | ``` |
808 | 808 | ||
809 | **Rational:** `match` is almost always more compact. | 809 | **Rationale:** `match` is almost always more compact. |
810 | The `else` branch can get a more precise pattern: `None` or `Err(_)` instead of `_`. | 810 | The `else` branch can get a more precise pattern: `None` or `Err(_)` instead of `_`. |
811 | 811 | ||
812 | ## Helper Functions | ||
813 | |||
814 | Avoid creating singe-use helper functions: | ||
815 | |||
816 | ```rust | ||
817 | // GOOD | ||
818 | let buf = { | ||
819 | let mut buf = get_empty_buf(&mut arena); | ||
820 | buf.add_item(item); | ||
821 | buf | ||
822 | }; | ||
823 | |||
824 | // BAD | ||
825 | |||
826 | let buf = prepare_buf(&mut arena, item); | ||
827 | |||
828 | ... | ||
829 | |||
830 | fn prepare_buf(arena: &mut Arena, item: Item) -> ItemBuf { | ||
831 | let mut res = get_empty_buf(&mut arena); | ||
832 | res.add_item(item); | ||
833 | res | ||
834 | } | ||
835 | ``` | ||
836 | |||
837 | Exception: if you want to make use of `return` or `?`. | ||
838 | |||
839 | **Rationale:** single-use functions change frequently, adding or removing parameters adds churn. | ||
840 | A block serves just as well to delineate a bit of logic, but has access to all the context. | ||
841 | Re-using originally single-purpose function often leads to bad coupling. | ||
842 | |||
843 | ## Helper Variables | ||
844 | |||
845 | Introduce helper variables freely, especially for multiline conditions: | ||
846 | |||
847 | ```rust | ||
848 | // GOOD | ||
849 | let rustfmt_not_installed = | ||
850 | captured_stderr.contains("not installed") || captured_stderr.contains("not available"); | ||
851 | |||
852 | match 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 | ||
858 | match 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 | ``` | ||
865 | |||
866 | **Rationale:** like blocks, single-use variables are a cognitively cheap abstraction, as they have access to all the context. | ||
867 | Extra variables help during debugging, they make it easy to print/view important intermediate results. | ||
868 | Giving a name to a condition in `if` expression often improves clarity and leads to a nicer formatted code. | ||
869 | |||
812 | ## Token names | 870 | ## Token names |
813 | 871 | ||
814 | Use `T![foo]` instead of `SyntaxKind::FOO_KW`. | 872 | Use `T![foo]` instead of `SyntaxKind::FOO_KW`. |