diff options
Diffstat (limited to 'docs/dev/style.md')
-rw-r--r-- | docs/dev/style.md | 72 |
1 files changed, 65 insertions, 7 deletions
diff --git a/docs/dev/style.md b/docs/dev/style.md index e4a1672ca..468dedff2 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md | |||
@@ -53,11 +53,11 @@ https://www.tedinski.com/2018/02/06/system-boundaries.html | |||
53 | ## Crates.io Dependencies | 53 | ## Crates.io Dependencies |
54 | 54 | ||
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` and `either` are 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 *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 | ||
@@ -330,7 +330,7 @@ When implementing `do_thing`, it might be very useful to create a context object | |||
330 | 330 | ||
331 | ```rust | 331 | ```rust |
332 | pub fn do_thing(arg1: Arg1, arg2: Arg2) -> Res { | 332 | pub fn do_thing(arg1: Arg1, arg2: Arg2) -> Res { |
333 | let mut ctx = Ctx { arg1, arg2 } | 333 | let mut ctx = Ctx { arg1, arg2 }; |
334 | ctx.run() | 334 | ctx.run() |
335 | } | 335 | } |
336 | 336 | ||
@@ -586,7 +586,7 @@ use super::{} | |||
586 | 586 | ||
587 | **Rationale:** consistency. | 587 | **Rationale:** consistency. |
588 | Reading order is important for new contributors. | 588 | Reading order is important for new contributors. |
589 | Grouping by crate allows to spot unwanted dependencies easier. | 589 | Grouping by crate allows spotting unwanted dependencies easier. |
590 | 590 | ||
591 | ## Import Style | 591 | ## Import Style |
592 | 592 | ||
@@ -779,7 +779,7 @@ assert!(x < y); | |||
779 | assert!(x > 0); | 779 | assert!(x > 0); |
780 | 780 | ||
781 | // BAD | 781 | // BAD |
782 | assert!(x >= lo && x <= hi>); | 782 | assert!(x >= lo && x <= hi); |
783 | assert!(r1 < l2 || l1 > r2); | 783 | assert!(r1 < l2 || l1 > r2); |
784 | assert!(y > x); | 784 | assert!(y > x); |
785 | assert!(0 > x); | 785 | assert!(0 > x); |
@@ -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`. |