diff options
author | Aleksey Kladov <[email protected]> | 2021-01-07 17:11:55 +0000 |
---|---|---|
committer | Aleksey Kladov <[email protected]> | 2021-01-07 17:11:55 +0000 |
commit | 5aed769afec96244c218ac094d46d22a1edb019c (patch) | |
tree | dbfc68738c1f17b1b7c1dd681227f47a18129668 /docs | |
parent | eb710a63ca94409dd410ebde57923bcaf48f2975 (diff) |
Styleguide readability
Diffstat (limited to 'docs')
-rw-r--r-- | docs/dev/style.md | 178 |
1 files changed, 115 insertions, 63 deletions
diff --git a/docs/dev/style.md b/docs/dev/style.md index be2c77847..f4748160b 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md | |||
@@ -53,6 +53,9 @@ We try to be very conservative with usage of crates.io dependencies. | |||
53 | Don't use small "helper" crates (exception: `itertools` is allowed). | 53 | Don't use small "helper" crates (exception: `itertools` is allowed). |
54 | If there's some general reusable bit of code you need, consider adding it to the `stdx` crate. | 54 | If there's some general reusable bit of code you need, consider adding it to the `stdx` crate. |
55 | 55 | ||
56 | **Rational:** keep compile times low, create ecosystem pressure for faster | ||
57 | compiles, reduce the number of things which might break. | ||
58 | |||
56 | ## Commit Style | 59 | ## Commit Style |
57 | 60 | ||
58 | We don't have specific rules around git history hygiene. | 61 | We don't have specific rules around git history hygiene. |
@@ -66,15 +69,18 @@ Such messages create a lot of duplicate notification traffic during rebases. | |||
66 | If possible, write commit messages from user's perspective: | 69 | If possible, write commit messages from user's perspective: |
67 | 70 | ||
68 | ``` | 71 | ``` |
69 | # Good | 72 | # GOOD |
70 | Goto definition works inside macros | 73 | Goto definition works inside macros |
71 | 74 | ||
72 | # Not as good | 75 | # BAD |
73 | Use original span for FileId | 76 | Use original span for FileId |
74 | ``` | 77 | ``` |
75 | 78 | ||
76 | This makes it easier to prepare a changelog. | 79 | This makes it easier to prepare a changelog. |
77 | 80 | ||
81 | **Rational:** clean history is potentially useful, but rarely used. | ||
82 | But many users read changelogs. | ||
83 | |||
78 | ## Clippy | 84 | ## Clippy |
79 | 85 | ||
80 | We don't enforce Clippy. | 86 | We don't enforce Clippy. |
@@ -82,21 +88,16 @@ A number of default lints have high false positive rate. | |||
82 | Selectively patching false-positives with `allow(clippy)` is considered worse than not using Clippy at all. | 88 | Selectively patching false-positives with `allow(clippy)` is considered worse than not using Clippy at all. |
83 | There's `cargo xtask lint` command which runs a subset of low-FPR lints. | 89 | There's `cargo xtask lint` command which runs a subset of low-FPR lints. |
84 | Careful tweaking of `xtask lint` is welcome. | 90 | Careful tweaking of `xtask lint` is welcome. |
85 | See also [rust-lang/clippy#5537](https://github.com/rust-lang/rust-clippy/issues/5537). | ||
86 | Of course, applying Clippy suggestions is welcome as long as they indeed improve the code. | 91 | Of course, applying Clippy suggestions is welcome as long as they indeed improve the code. |
87 | 92 | ||
93 | **Rational:** see [rust-lang/clippy#5537](https://github.com/rust-lang/rust-clippy/issues/5537). | ||
94 | |||
88 | # Code | 95 | # Code |
89 | 96 | ||
90 | ## Minimal Tests | 97 | ## Minimal Tests |
91 | 98 | ||
92 | Most tests in rust-analyzer start with a snippet of Rust code. | 99 | Most tests in rust-analyzer start with a snippet of Rust code. |
93 | This snippets should be minimal -- if you copy-paste a snippet of real code into the tests, make sure to remove everything which could be removed. | 100 | This snippets should be minimal -- if you copy-paste a snippet of real code into the tests, make sure to remove everything which could be removed. |
94 | There are many benefits to this: | ||
95 | |||
96 | * less to read or to scroll past | ||
97 | * easier to understand what exactly is tested | ||
98 | * less stuff printed during printf-debugging | ||
99 | * less time to run test | ||
100 | 101 | ||
101 | It also makes sense to format snippets more compactly (for example, by placing enum definitions like `enum E { Foo, Bar }` on a single line), | 102 | It also makes sense to format snippets more compactly (for example, by placing enum definitions like `enum E { Foo, Bar }` on a single line), |
102 | as long as they are still readable. | 103 | as long as they are still readable. |
@@ -125,19 +126,28 @@ fn main() { | |||
125 | } | 126 | } |
126 | ``` | 127 | ``` |
127 | 128 | ||
128 | That way, you can use your editor's "number of selected characters" feature to correlate offsets with test's source code. | 129 | **Rational:** |
130 | |||
131 | There are many benefits to this: | ||
132 | |||
133 | * less to read or to scroll past | ||
134 | * easier to understand what exactly is tested | ||
135 | * less stuff printed during printf-debugging | ||
136 | * less time to run test | ||
137 | |||
138 | Formatting ensures that you can use your editor's "number of selected characters" feature to correlate offsets with test's source code. | ||
129 | 139 | ||
130 | ## Preconditions | 140 | ## Function Preconditions |
131 | 141 | ||
132 | Express function preconditions in types and force the caller to provide them (rather than checking in callee): | 142 | Express function preconditions in types and force the caller to provide them (rather than checking in callee): |
133 | 143 | ||
134 | ```rust | 144 | ```rust |
135 | // Good | 145 | // GOOD |
136 | fn frbonicate(walrus: Walrus) { | 146 | fn frbonicate(walrus: Walrus) { |
137 | ... | 147 | ... |
138 | } | 148 | } |
139 | 149 | ||
140 | // Not as good | 150 | // BAD |
141 | fn frobnicate(walrus: Option<Walrus>) { | 151 | fn frobnicate(walrus: Option<Walrus>) { |
142 | let walrus = match walrus { | 152 | let walrus = match walrus { |
143 | Some(it) => it, | 153 | Some(it) => it, |
@@ -147,10 +157,13 @@ fn frobnicate(walrus: Option<Walrus>) { | |||
147 | } | 157 | } |
148 | ``` | 158 | ``` |
149 | 159 | ||
150 | Avoid preconditions that span across function boundaries: | 160 | **Rational:** this makes control flow explicit at the call site. |
161 | Call-site has more context, it often happens that the precondition falls out naturally or can be bubbled up higher in the stack. | ||
162 | |||
163 | Avoid splitting precondition check and precondition use across functions: | ||
151 | 164 | ||
152 | ```rust | 165 | ```rust |
153 | // Good | 166 | // GOOD |
154 | fn main() { | 167 | fn main() { |
155 | let s: &str = ...; | 168 | let s: &str = ...; |
156 | if let Some(contents) = string_literal_contents(s) { | 169 | if let Some(contents) = string_literal_contents(s) { |
@@ -166,7 +179,7 @@ fn string_literal_contents(s: &str) -> Option<&str> { | |||
166 | } | 179 | } |
167 | } | 180 | } |
168 | 181 | ||
169 | // Not as good | 182 | // BAD |
170 | fn main() { | 183 | fn main() { |
171 | let s: &str = ...; | 184 | let s: &str = ...; |
172 | if is_string_literal(s) { | 185 | if is_string_literal(s) { |
@@ -182,20 +195,24 @@ fn is_string_literal(s: &str) -> bool { | |||
182 | In the "Not as good" version, the precondition that `1` is a valid char boundary is checked in `is_string_literal` and used in `foo`. | 195 | In the "Not as good" version, the precondition that `1` is a valid char boundary is checked in `is_string_literal` and used in `foo`. |
183 | In the "Good" version, the precondition check and usage are checked in the same block, and then encoded in the types. | 196 | In the "Good" version, the precondition check and usage are checked in the same block, and then encoded in the types. |
184 | 197 | ||
198 | **Rational:** non-local code properties degrade under change. | ||
199 | |||
185 | When checking a boolean precondition, prefer `if !invariant` to `if negated_invariant`: | 200 | When checking a boolean precondition, prefer `if !invariant` to `if negated_invariant`: |
186 | 201 | ||
187 | ```rust | 202 | ```rust |
188 | // Good | 203 | // GOOD |
189 | if !(idx < len) { | 204 | if !(idx < len) { |
190 | return None; | 205 | return None; |
191 | } | 206 | } |
192 | 207 | ||
193 | // Not as good | 208 | // BAD |
194 | if idx >= len { | 209 | if idx >= len { |
195 | return None; | 210 | return None; |
196 | } | 211 | } |
197 | ``` | 212 | ``` |
198 | 213 | ||
214 | **Rational:** its useful to see the invariant relied upon by the rest of the function clearly spelled out. | ||
215 | |||
199 | ## Getters & Setters | 216 | ## Getters & Setters |
200 | 217 | ||
201 | If a field can have any value without breaking invariants, make the field public. | 218 | If a field can have any value without breaking invariants, make the field public. |
@@ -211,31 +228,36 @@ struct Person { | |||
211 | middle_name: Option<String> | 228 | middle_name: Option<String> |
212 | } | 229 | } |
213 | 230 | ||
214 | // Good | 231 | // GOOD |
215 | impl Person { | 232 | impl Person { |
216 | fn first_name(&self) -> &str { self.first_name.as_str() } | 233 | fn first_name(&self) -> &str { self.first_name.as_str() } |
217 | fn middle_name(&self) -> Option<&str> { self.middle_name.as_ref() } | 234 | fn middle_name(&self) -> Option<&str> { self.middle_name.as_ref() } |
218 | } | 235 | } |
219 | 236 | ||
220 | // Not as good | 237 | // BAD |
221 | impl Person { | 238 | impl Person { |
222 | fn first_name(&self) -> String { self.first_name.clone() } | 239 | fn first_name(&self) -> String { self.first_name.clone() } |
223 | fn middle_name(&self) -> &Option<String> { &self.middle_name } | 240 | fn middle_name(&self) -> &Option<String> { &self.middle_name } |
224 | } | 241 | } |
225 | ``` | 242 | ``` |
226 | 243 | ||
244 | **Rational:** we don't provide public API, it's cheaper to refactor than to pay getters rent. | ||
245 | Non-local code properties degrade under change, privacy makes invariant local. | ||
246 | Borrowed own data discloses irrelevant details about origin of data. | ||
247 | Irrelevant (neither right nor wrong) things obscure correctness. | ||
248 | |||
227 | ## Constructors | 249 | ## Constructors |
228 | 250 | ||
229 | Prefer `Default` to zero-argument `new` function | 251 | Prefer `Default` to zero-argument `new` function |
230 | 252 | ||
231 | ```rust | 253 | ```rust |
232 | // Good | 254 | // GOOD |
233 | #[derive(Default)] | 255 | #[derive(Default)] |
234 | struct Foo { | 256 | struct Foo { |
235 | bar: Option<Bar> | 257 | bar: Option<Bar> |
236 | } | 258 | } |
237 | 259 | ||
238 | // Not as good | 260 | // BAD |
239 | struct Foo { | 261 | struct Foo { |
240 | bar: Option<Bar> | 262 | bar: Option<Bar> |
241 | } | 263 | } |
@@ -249,16 +271,18 @@ impl Foo { | |||
249 | 271 | ||
250 | Prefer `Default` even it has to be implemented manually. | 272 | Prefer `Default` even it has to be implemented manually. |
251 | 273 | ||
274 | **Rational:** less typing in the common case, uniformity. | ||
275 | |||
252 | ## Functions Over Objects | 276 | ## Functions Over Objects |
253 | 277 | ||
254 | Avoid creating "doer" objects. | 278 | Avoid creating "doer" objects. |
255 | That is, objects which are created only to execute a single action. | 279 | That is, objects which are created only to execute a single action. |
256 | 280 | ||
257 | ```rust | 281 | ```rust |
258 | // Good | 282 | // GOOD |
259 | do_thing(arg1, arg2); | 283 | do_thing(arg1, arg2); |
260 | 284 | ||
261 | // Not as good | 285 | // BAD |
262 | ThingDoer::new(arg1, arg2).do(); | 286 | ThingDoer::new(arg1, arg2).do(); |
263 | ``` | 287 | ``` |
264 | 288 | ||
@@ -303,16 +327,14 @@ impl ThingDoer { | |||
303 | } | 327 | } |
304 | ``` | 328 | ``` |
305 | 329 | ||
330 | **Rational:** not bothering the caller with irrelevant details, not mixing user API with implementor API. | ||
331 | |||
306 | ## Avoid Monomorphization | 332 | ## Avoid Monomorphization |
307 | 333 | ||
308 | Rust uses monomorphization to compile generic code, meaning that for each instantiation of a generic functions with concrete types, the function is compiled afresh, *per crate*. | 334 | Avoid making a lot of code type parametric, *especially* on the boundaries between crates. |
309 | This allows for exceptionally good performance, but leads to increased compile times. | ||
310 | Runtime performance obeys 80%/20% rule -- only a small fraction of code is hot. | ||
311 | Compile time **does not** obey this rule -- all code has to be compiled. | ||
312 | For this reason, avoid making a lot of code type parametric, *especially* on the boundaries between crates. | ||
313 | 335 | ||
314 | ```rust | 336 | ```rust |
315 | // Good | 337 | // GOOD |
316 | fn frbonicate(f: impl FnMut()) { | 338 | fn frbonicate(f: impl FnMut()) { |
317 | frobnicate_impl(&mut f) | 339 | frobnicate_impl(&mut f) |
318 | } | 340 | } |
@@ -320,7 +342,7 @@ fn frobnicate_impl(f: &mut dyn FnMut()) { | |||
320 | // lots of code | 342 | // lots of code |
321 | } | 343 | } |
322 | 344 | ||
323 | // Not as good | 345 | // BAD |
324 | fn frbonicate(f: impl FnMut()) { | 346 | fn frbonicate(f: impl FnMut()) { |
325 | // lots of code | 347 | // lots of code |
326 | } | 348 | } |
@@ -329,15 +351,21 @@ fn frbonicate(f: impl FnMut()) { | |||
329 | Avoid `AsRef` polymorphism, it pays back only for widely used libraries: | 351 | Avoid `AsRef` polymorphism, it pays back only for widely used libraries: |
330 | 352 | ||
331 | ```rust | 353 | ```rust |
332 | // Good | 354 | // GOOD |
333 | fn frbonicate(f: &Path) { | 355 | fn frbonicate(f: &Path) { |
334 | } | 356 | } |
335 | 357 | ||
336 | // Not as good | 358 | // BAD |
337 | fn frbonicate(f: impl AsRef<Path>) { | 359 | fn frbonicate(f: impl AsRef<Path>) { |
338 | } | 360 | } |
339 | ``` | 361 | ``` |
340 | 362 | ||
363 | **Rational:** Rust uses monomorphization to compile generic code, meaning that for each instantiation of a generic functions with concrete types, the function is compiled afresh, *per crate*. | ||
364 | This allows for exceptionally good performance, but leads to increased compile times. | ||
365 | Runtime performance obeys 80%/20% rule -- only a small fraction of code is hot. | ||
366 | Compile time **does not** obey this rule -- all code has to be compiled. | ||
367 | |||
368 | |||
341 | # Premature Pessimization | 369 | # Premature Pessimization |
342 | 370 | ||
343 | ## Avoid Allocations | 371 | ## Avoid Allocations |
@@ -346,7 +374,7 @@ Avoid writing code which is slower than it needs to be. | |||
346 | Don't allocate a `Vec` where an iterator would do, don't allocate strings needlessly. | 374 | Don't allocate a `Vec` where an iterator would do, don't allocate strings needlessly. |
347 | 375 | ||
348 | ```rust | 376 | ```rust |
349 | // Good | 377 | // GOOD |
350 | use itertools::Itertools; | 378 | use itertools::Itertools; |
351 | 379 | ||
352 | let (first_word, second_word) = match text.split_ascii_whitespace().collect_tuple() { | 380 | let (first_word, second_word) = match text.split_ascii_whitespace().collect_tuple() { |
@@ -354,37 +382,40 @@ let (first_word, second_word) = match text.split_ascii_whitespace().collect_tupl | |||
354 | None => return, | 382 | None => return, |
355 | } | 383 | } |
356 | 384 | ||
357 | // Not as good | 385 | // BAD |
358 | let words = text.split_ascii_whitespace().collect::<Vec<_>>(); | 386 | let words = text.split_ascii_whitespace().collect::<Vec<_>>(); |
359 | if words.len() != 2 { | 387 | if words.len() != 2 { |
360 | return | 388 | return |
361 | } | 389 | } |
362 | ``` | 390 | ``` |
363 | 391 | ||
392 | **Rational:** not allocating is almost often faster. | ||
393 | |||
364 | ## Push Allocations to the Call Site | 394 | ## Push Allocations to the Call Site |
365 | 395 | ||
366 | If allocation is inevitable, let the caller allocate the resource: | 396 | If allocation is inevitable, let the caller allocate the resource: |
367 | 397 | ||
368 | ```rust | 398 | ```rust |
369 | // Good | 399 | // GOOD |
370 | fn frobnicate(s: String) { | 400 | fn frobnicate(s: String) { |
371 | ... | 401 | ... |
372 | } | 402 | } |
373 | 403 | ||
374 | // Not as good | 404 | // BAD |
375 | fn frobnicate(s: &str) { | 405 | fn frobnicate(s: &str) { |
376 | let s = s.to_string(); | 406 | let s = s.to_string(); |
377 | ... | 407 | ... |
378 | } | 408 | } |
379 | ``` | 409 | ``` |
380 | 410 | ||
381 | This is better because it reveals the costs. | 411 | **Rational:** reveals the costs. |
382 | It is also more efficient when the caller already owns the allocation. | 412 | It is also more efficient when the caller already owns the allocation. |
383 | 413 | ||
384 | ## Collection types | 414 | ## Collection types |
385 | 415 | ||
386 | Prefer `rustc_hash::FxHashMap` and `rustc_hash::FxHashSet` instead of the ones in `std::collections`. | 416 | Prefer `rustc_hash::FxHashMap` and `rustc_hash::FxHashSet` instead of the ones in `std::collections`. |
387 | They use a hasher that's slightly faster and using them consistently will reduce code size by some small amount. | 417 | |
418 | **Rational:** they use a hasher that's significantly faster and using them consistently will reduce code size by some small amount. | ||
388 | 419 | ||
389 | # Style | 420 | # Style |
390 | 421 | ||
@@ -393,6 +424,9 @@ They use a hasher that's slightly faster and using them consistently will reduce | |||
393 | Separate import groups with blank lines. | 424 | Separate import groups with blank lines. |
394 | Use one `use` per crate. | 425 | Use one `use` per crate. |
395 | 426 | ||
427 | Module declarations come before the imports. | ||
428 | Order them in "suggested reading order" for a person new to the code base. | ||
429 | |||
396 | ```rust | 430 | ```rust |
397 | mod x; | 431 | mod x; |
398 | mod y; | 432 | mod y; |
@@ -411,46 +445,45 @@ use crate::{} | |||
411 | use super::{} | 445 | use super::{} |
412 | ``` | 446 | ``` |
413 | 447 | ||
414 | Module declarations come before the imports. | 448 | **Rational:** consistency. |
415 | Order them in "suggested reading order" for a person new to the code base. | 449 | Reading order is important for new contributors. |
450 | Grouping by crate allows to spot unwanted dependencies easier. | ||
416 | 451 | ||
417 | ## Import Style | 452 | ## Import Style |
418 | 453 | ||
419 | Qualify items from `hir` and `ast`. | 454 | Qualify items from `hir` and `ast`. |
420 | 455 | ||
421 | ```rust | 456 | ```rust |
422 | // Good | 457 | // GOOD |
423 | use syntax::ast; | 458 | use syntax::ast; |
424 | 459 | ||
425 | fn frobnicate(func: hir::Function, strukt: ast::StructDef) {} | 460 | fn frobnicate(func: hir::Function, strukt: ast::Struct) {} |
426 | 461 | ||
427 | // Not as good | 462 | // BAD |
428 | use hir::Function; | 463 | use hir::Function; |
429 | use syntax::ast::StructDef; | 464 | use syntax::ast::Struct; |
430 | 465 | ||
431 | fn frobnicate(func: Function, strukt: StructDef) {} | 466 | fn frobnicate(func: Function, strukt: Struct) {} |
432 | ``` | 467 | ``` |
433 | 468 | ||
434 | Avoid local `use MyEnum::*` imports. | 469 | **Rational:** avoids name clashes, makes the layer clear at a glance. |
435 | |||
436 | Prefer `use crate::foo::bar` to `use super::bar`. | ||
437 | 470 | ||
438 | When implementing traits from `std::fmt` or `std::ops`, import the module: | 471 | When implementing traits from `std::fmt` or `std::ops`, import the module: |
439 | 472 | ||
440 | ```rust | 473 | ```rust |
441 | // Good | 474 | // GOOD |
442 | use std::fmt; | 475 | use std::fmt; |
443 | 476 | ||
444 | impl fmt::Display for RenameError { | 477 | impl fmt::Display for RenameError { |
445 | fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { .. } | 478 | fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { .. } |
446 | } | 479 | } |
447 | 480 | ||
448 | // Not as good | 481 | // BAD |
449 | impl std::fmt::Display for RenameError { | 482 | impl std::fmt::Display for RenameError { |
450 | fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { .. } | 483 | fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { .. } |
451 | } | 484 | } |
452 | 485 | ||
453 | // Not as good | 486 | // BAD |
454 | use std::ops::Deref; | 487 | use std::ops::Deref; |
455 | 488 | ||
456 | impl Deref for Widget { | 489 | impl Deref for Widget { |
@@ -459,6 +492,15 @@ impl Deref for Widget { | |||
459 | } | 492 | } |
460 | ``` | 493 | ``` |
461 | 494 | ||
495 | **Rational:** overall, less typing. | ||
496 | Makes it clear that a trait is implemented, rather than used. | ||
497 | |||
498 | Avoid local `use MyEnum::*` imports. | ||
499 | **Rational:** consistency. | ||
500 | |||
501 | Prefer `use crate::foo::bar` to `use super::bar` or `use self::bar::baz`. | ||
502 | **Rational:** consistency, this is the style which works in all cases. | ||
503 | |||
462 | ## Order of Items | 504 | ## Order of Items |
463 | 505 | ||
464 | Optimize for the reader who sees the file for the first time, and wants to get a general idea about what's going on. | 506 | Optimize for the reader who sees the file for the first time, and wants to get a general idea about what's going on. |
@@ -467,7 +509,7 @@ People read things from top to bottom, so place most important things first. | |||
467 | Specifically, if all items except one are private, always put the non-private item on top. | 509 | Specifically, if all items except one are private, always put the non-private item on top. |
468 | 510 | ||
469 | ```rust | 511 | ```rust |
470 | // Good | 512 | // GOOD |
471 | pub(crate) fn frobnicate() { | 513 | pub(crate) fn frobnicate() { |
472 | Helper::act() | 514 | Helper::act() |
473 | } | 515 | } |
@@ -481,7 +523,7 @@ impl Helper { | |||
481 | } | 523 | } |
482 | } | 524 | } |
483 | 525 | ||
484 | // Not as good | 526 | // BAD |
485 | #[derive(Default)] | 527 | #[derive(Default)] |
486 | struct Helper { stuff: i32 } | 528 | struct Helper { stuff: i32 } |
487 | 529 | ||
@@ -497,12 +539,11 @@ impl Helper { | |||
497 | ``` | 539 | ``` |
498 | 540 | ||
499 | If there's a mixture of private and public items, put public items first. | 541 | If there's a mixture of private and public items, put public items first. |
500 | If function bodies are folded in the editor, the source code should read as documentation for the public API. | ||
501 | 542 | ||
502 | Put `struct`s and `enum`s first, functions and impls last. Order types declarations in top-down manner. | 543 | Put `struct`s and `enum`s first, functions and impls last. Order type declarations in top-down manner. |
503 | 544 | ||
504 | ```rust | 545 | ```rust |
505 | // Good | 546 | // GOOD |
506 | struct Parent { | 547 | struct Parent { |
507 | children: Vec<Child> | 548 | children: Vec<Child> |
508 | } | 549 | } |
@@ -515,7 +556,7 @@ impl Parent { | |||
515 | impl Child { | 556 | impl Child { |
516 | } | 557 | } |
517 | 558 | ||
518 | // Not as good | 559 | // BAD |
519 | struct Child; | 560 | struct Child; |
520 | 561 | ||
521 | impl Child { | 562 | impl Child { |
@@ -529,6 +570,9 @@ impl Parent { | |||
529 | } | 570 | } |
530 | ``` | 571 | ``` |
531 | 572 | ||
573 | **Rational:** easier to get the sense of the API by visually scanning the file. | ||
574 | If function bodies are folded in the editor, the source code should read as documentation for the public API. | ||
575 | |||
532 | ## Variable Naming | 576 | ## Variable Naming |
533 | 577 | ||
534 | Use boring and long names for local variables ([yay code completion](https://github.com/rust-analyzer/rust-analyzer/pull/4162#discussion_r417130973)). | 578 | Use boring and long names for local variables ([yay code completion](https://github.com/rust-analyzer/rust-analyzer/pull/4162#discussion_r417130973)). |
@@ -556,12 +600,14 @@ enum -> enum_ | |||
556 | mod -> module | 600 | mod -> module |
557 | ``` | 601 | ``` |
558 | 602 | ||
603 | **Rationale:** consistency. | ||
604 | |||
559 | ## Early Returns | 605 | ## Early Returns |
560 | 606 | ||
561 | Do use early returns | 607 | Do use early returns |
562 | 608 | ||
563 | ```rust | 609 | ```rust |
564 | // Good | 610 | // GOOD |
565 | fn foo() -> Option<Bar> { | 611 | fn foo() -> Option<Bar> { |
566 | if !condition() { | 612 | if !condition() { |
567 | return None; | 613 | return None; |
@@ -570,7 +616,7 @@ fn foo() -> Option<Bar> { | |||
570 | Some(...) | 616 | Some(...) |
571 | } | 617 | } |
572 | 618 | ||
573 | // Not as good | 619 | // BAD |
574 | fn foo() -> Option<Bar> { | 620 | fn foo() -> Option<Bar> { |
575 | if condition() { | 621 | if condition() { |
576 | Some(...) | 622 | Some(...) |
@@ -580,20 +626,26 @@ fn foo() -> Option<Bar> { | |||
580 | } | 626 | } |
581 | ``` | 627 | ``` |
582 | 628 | ||
629 | **Rational:** reduce congnitive stack usage. | ||
630 | |||
583 | ## Comparisons | 631 | ## Comparisons |
584 | 632 | ||
585 | Use `<`/`<=`, avoid `>`/`>=`. | 633 | Use `<`/`<=`, avoid `>`/`>=`. |
586 | Less-then comparisons are more intuitive, they correspond spatially to [real line](https://en.wikipedia.org/wiki/Real_line) | ||
587 | 634 | ||
588 | ```rust | 635 | ```rust |
589 | // Good | 636 | // GOOD |
590 | assert!(lo <= x && x <= hi); | 637 | assert!(lo <= x && x <= hi); |
591 | 638 | ||
592 | // Not as good | 639 | // BAD |
593 | assert!(x >= lo && x <= hi>); | 640 | assert!(x >= lo && x <= hi>); |
594 | ``` | 641 | ``` |
595 | 642 | ||
643 | **Rational:** Less-then comparisons are more intuitive, they correspond spatially to [real line](https://en.wikipedia.org/wiki/Real_line). | ||
644 | |||
645 | |||
596 | ## Documentation | 646 | ## Documentation |
597 | 647 | ||
598 | For `.md` and `.adoc` files, prefer a sentence-per-line format, don't wrap lines. | 648 | For `.md` and `.adoc` files, prefer a sentence-per-line format, don't wrap lines. |
599 | If the line is too long, you want to split the sentence in two :-) | 649 | If the line is too long, you want to split the sentence in two :-) |
650 | |||
651 | **Rational:** much easier to edit the text and read the diff. | ||