aboutsummaryrefslogtreecommitdiff
path: root/docs/dev/style.md
diff options
context:
space:
mode:
Diffstat (limited to 'docs/dev/style.md')
-rw-r--r--docs/dev/style.md72
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
55We try to be very conservative with usage of crates.io dependencies. 55We try to be very conservative with usage of crates.io dependencies.
56Don't use small "helper" crates (exception: `itertools` is allowed). 56Don't use small "helper" crates (exception: `itertools` and `either` are allowed).
57If there's some general reusable bit of code you need, consider adding it to the `stdx` crate. 57If there's some general reusable bit of code you need, consider adding it to the `stdx` crate.
58A 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.
60compiles, 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
332pub fn do_thing(arg1: Arg1, arg2: Arg2) -> Res { 332pub 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.
588Reading order is important for new contributors. 588Reading order is important for new contributors.
589Grouping by crate allows to spot unwanted dependencies easier. 589Grouping by crate allows spotting unwanted dependencies easier.
590 590
591## Import Style 591## Import Style
592 592
@@ -779,7 +779,7 @@ assert!(x < y);
779assert!(x > 0); 779assert!(x > 0);
780 780
781// BAD 781// BAD
782assert!(x >= lo && x <= hi>); 782assert!(x >= lo && x <= hi);
783assert!(r1 < l2 || l1 > r2); 783assert!(r1 < l2 || l1 > r2);
784assert!(y > x); 784assert!(y > x);
785assert!(0 > x); 785assert!(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.
810The `else` branch can get a more precise pattern: `None` or `Err(_)` instead of `_`. 810The `else` branch can get a more precise pattern: `None` or `Err(_)` instead of `_`.
811 811
812## Helper Functions
813
814Avoid creating singe-use helper functions:
815
816```rust
817// GOOD
818let buf = {
819 let mut buf = get_empty_buf(&mut arena);
820 buf.add_item(item);
821 buf
822};
823
824// BAD
825
826let buf = prepare_buf(&mut arena, item);
827
828...
829
830fn 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
837Exception: if you want to make use of `return` or `?`.
838
839**Rationale:** single-use functions change frequently, adding or removing parameters adds churn.
840A block serves just as well to delineate a bit of logic, but has access to all the context.
841Re-using originally single-purpose function often leads to bad coupling.
842
843## Helper Variables
844
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```
865
866**Rationale:** like blocks, single-use variables are a cognitively cheap abstraction, as they have access to all the context.
867Extra variables help during debugging, they make it easy to print/view important intermediate results.
868Giving 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
814Use `T![foo]` instead of `SyntaxKind::FOO_KW`. 872Use `T![foo]` instead of `SyntaxKind::FOO_KW`.