1 Our approach to "clean code" is two-fold:
3 * We generally don't block PRs on style changes.
4 * At the same time, all code in rust-analyzer is constantly refactored.
6 It is explicitly OK for a reviewer to flag only some nits in the PR, and then send a follow-up cleanup PR for things which are easier to explain by example, cc-ing the original author.
7 Sending small cleanup PRs (like renaming a single local variable) is encouraged.
9 When reviewing pull requests prefer extending this document to leaving
10 non-reusable comments on the pull request itself.
16 Everyone knows that it's better to send small & focused pull requests.
17 The problem is, sometimes you *have* to, eg, rewrite the whole compiler, and that just doesn't fit into a set of isolated PRs.
19 The main things to keep an eye on are the boundaries between various components.
20 There are three kinds of changes:
22 1. Internals of a single component are changed.
23 Specifically, you don't change any `pub` items.
24 A good example here would be an addition of a new assist.
26 2. API of a component is expanded.
27 Specifically, you add a new `pub` function which wasn't there before.
28 A good example here would be expansion of assist API, for example, to implement lazy assists or assists groups.
30 3. A new dependency between components is introduced.
31 Specifically, you add a `pub use` reexport from another crate or you add a new line to the `[dependencies]` section of `Cargo.toml`.
32 A good example here would be adding reference search capability to the assists crates.
34 For the first group, the change is generally merged as long as:
36 * it works for the happy case,
38 * it doesn't panic for the unhappy case.
40 For the second group, the change would be subjected to quite a bit of scrutiny and iteration.
41 The new API needs to be right (or at least easy to change later).
42 The actual implementation doesn't matter that much.
43 It's very important to minimize the amount of changed lines of code for changes of the second kind.
44 Often, you start doing a change of the first kind, only to realize that you need to elevate to a change of the second kind.
45 In this case, we'll probably ask you to split API changes into a separate PR.
47 Changes of the third group should be pretty rare, so we don't specify any specific process for them.
48 That said, adding an innocent-looking `pub use` is a very simple way to break encapsulation, keep an eye on it!
50 Note: if you enjoyed this abstract hand-waving about boundaries, you might appreciate
51 https://www.tedinski.com/2018/02/06/system-boundaries.html
53 ## 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` and `either` are allowed).
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.
60 **Rationale:** keep compile times low, create ecosystem pressure for faster compiles, reduce the number of things which might break.
64 We don't have specific rules around git history hygiene.
65 Maintaining clean git history is strongly encouraged, but not enforced.
66 Use rebase workflow, it's OK to rewrite history during PR review process.
67 After you are happy with the state of the code, please use [interactive rebase](https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History) to squash fixup commits.
69 Avoid @mentioning people in commit messages and pull request descriptions(they are added to commit message by bors).
70 Such messages create a lot of duplicate notification traffic during rebases.
72 If possible, write Pull Request titles and descriptions from the user's perspective:
76 Make goto definition work inside macros
79 Use original span for FileId
82 This makes it easier to prepare a changelog.
84 If the change adds a new user-visible functionality, consider recording a GIF with [peek](https://github.com/phw/peek) and pasting it into the PR description.
86 To make writing the release notes easier, you can mark a pull request as a feature, fix, internal change, or minor.
87 Minor changes are excluded from the release notes, while the other types are distributed in their corresponding sections.
88 There are two ways to mark this:
90 * use a `feat: `, `feature: `, `fix: `, `internal: ` or `minor: ` prefix in the PR title
91 * write `changelog [feature|fix|internal|skip] [description]` in a comment or in the PR description; the description is optional, and will replace the title if included.
93 These comments don't have to be added by the PR author.
94 Editing a comment or the PR description or title is also fine, as long as it happens before the release.
96 **Rationale:** clean history is potentially useful, but rarely used.
97 But many users read changelogs.
98 Including a description and GIF suitable for the changelog means less work for the maintainers on the release day.
102 We don't enforce Clippy.
103 A number of default lints have high false positive rate.
104 Selectively patching false-positives with `allow(clippy)` is considered worse than not using Clippy at all.
105 There's a `cargo lint` command which runs a subset of low-FPR lints.
106 Careful tweaking of `lint` is welcome.
107 Of course, applying Clippy suggestions is welcome as long as they indeed improve the code.
109 **Rationale:** see [rust-lang/clippy#5537](https://github.com/rust-lang/rust-clippy/issues/5537).
115 Most tests in rust-analyzer start with a snippet of Rust code.
116 These 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.
118 It also makes sense to format snippets more compactly (for example, by placing enum definitions like `enum E { Foo, Bar }` on a single line),
119 as long as they are still readable.
121 When using multiline fixtures, use unindented raw string literals:
125 fn inline_field_shorthand() {
127 inline_local_variable,
147 There are many benefits to this:
149 * less to read or to scroll past
150 * easier to understand what exactly is tested
151 * less stuff printed during printf-debugging
152 * less time to run test
154 Formatting ensures that you can use your editor's "number of selected characters" feature to correlate offsets with test's source code.
159 [`cov_mark::hit! / cov_mark::check!`](https://github.com/matklad/cov-mark)
160 when testing specific conditions.
161 Do not place several marks into a single test or condition.
162 Do not reuse marks between several tests.
164 **Rationale:** marks provide an easy way to find the canonical test for each bit of code.
165 This makes it much easier to understand.
166 More than one mark per test / code branch doesn't add significantly to understanding.
170 Do not use `#[should_panic]` tests.
171 Instead, explicitly check for `None`, `Err`, etc.
173 **Rationale:** `#[should_panic]` is a tool for library authors to make sure that the API does not fail silently when misused.
174 `rust-analyzer` is not a library, we don't need to test for API misuse, and we have to handle any user input without panics.
175 Panic messages in the logs from the `#[should_panic]` tests are confusing.
179 Do not `#[ignore]` tests.
180 If the test currently does not work, assert the wrong behavior and add a fixme explaining why it is wrong.
182 **Rationale:** noticing when the behavior is fixed, making sure that even the wrong behavior is acceptable (ie, not a panic).
184 ## Function Preconditions
186 Express function preconditions in types and force the caller to provide them (rather than checking in callee):
190 fn frobnicate(walrus: Walrus) {
195 fn frobnicate(walrus: Option<Walrus>) {
196 let walrus = match walrus {
204 **Rationale:** this makes control flow explicit at the call site.
205 Call-site has more context, it often happens that the precondition falls out naturally or can be bubbled up higher in the stack.
207 Avoid splitting precondition check and precondition use across functions:
213 if let Some(contents) = string_literal_contents(s) {
218 fn string_literal_contents(s: &str) -> Option<&str> {
219 if s.starts_with('"') && s.ends_with('"') {
220 Some(&s[1..s.len() - 1])
229 if is_string_literal(s) {
230 let contents = &s[1..s.len() - 1];
234 fn is_string_literal(s: &str) -> bool {
235 s.starts_with('"') && s.ends_with('"')
239 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`.
240 In the "Good" version, the precondition check and usage are checked in the same block, and then encoded in the types.
242 **Rationale:** non-local code properties degrade under change.
244 When checking a boolean precondition, prefer `if !invariant` to `if negated_invariant`:
258 **Rationale:** it's useful to see the invariant relied upon by the rest of the function clearly spelled out.
262 As a special case of the previous rule, do not hide control flow inside functions, push it to the caller:
282 Prefer [`stdx::never!`](https://docs.rs/always-assert/0.1.2/always_assert/macro.never.html) to standard `assert!`.
284 **Rationale:** See [cross cutting concern: error handling](https://github.com/rust-lang/rust-analyzer/blob/master/docs/dev/architecture.md#error-handling).
288 If a field can have any value without breaking invariants, make the field public.
289 Conversely, if there is an invariant, document it, enforce it in the "constructor" function, make the field private, and provide a getter.
290 Never provide setters.
292 Getters should return borrowed data:
296 // Invariant: never empty
298 middle_name: Option<String>
303 fn first_name(&self) -> &str { self.first_name.as_str() }
304 fn middle_name(&self) -> Option<&str> { self.middle_name.as_ref() }
309 fn first_name(&self) -> String { self.first_name.clone() }
310 fn middle_name(&self) -> &Option<String> { &self.middle_name }
314 **Rationale:** we don't provide public API, it's cheaper to refactor than to pay getters rent.
315 Non-local code properties degrade under change, privacy makes invariant local.
316 Borrowed owned types (`&String`) disclose irrelevant details about internal representation.
317 Irrelevant (neither right nor wrong) things obscure correctness.
321 More generally, always prefer types on the left
327 Option<&T> &Option<T>
331 **Rationale:** types on the left are strictly more general.
332 Even when generality is not required, consistency is important.
336 Prefer `Default` to zero-argument `new` function.
357 Prefer `Default` even if it has to be implemented manually.
359 **Rationale:** less typing in the common case, uniformity.
361 Use `Vec::new` rather than `vec![]`.
363 **Rationale:** uniformity, strength reduction.
365 Avoid using "dummy" states to implement a `Default`.
366 If a type doesn't have a sensible default, empty value, don't hide it.
367 Let the caller explicitly decide what the right initial state is.
369 ## Functions Over Objects
371 Avoid creating "doer" objects.
372 That is, objects which are created only to execute a single action.
376 do_thing(arg1, arg2);
379 ThingDoer::new(arg1, arg2).do();
382 Note that this concerns only outward API.
383 When implementing `do_thing`, it might be very useful to create a context object.
386 pub fn do_thing(arg1: Arg1, arg2: Arg2) -> Res {
387 let mut ctx = Ctx { arg1, arg2 };
392 arg1: Arg1, arg2: Arg2
396 fn run(self) -> Res {
402 The difference is that `Ctx` is an impl detail here.
404 Sometimes a middle ground is acceptable if this can save some busywork:
407 ThingDoer::do(arg1, arg2);
409 pub struct ThingDoer {
410 arg1: Arg1, arg2: Arg2,
414 pub fn do(arg1: Arg1, arg2: Arg2) -> Res {
415 ThingDoer { arg1, arg2 }.run()
417 fn run(self) -> Res {
423 **Rationale:** not bothering the caller with irrelevant details, not mixing user API with implementor API.
425 ## Functions with many parameters
427 Avoid creating functions with many optional or boolean parameters.
428 Introduce a `Config` struct instead.
432 pub struct AnnotationConfig {
433 pub binary_target: bool,
434 pub annotate_runnables: bool,
435 pub annotate_impls: bool,
441 config: AnnotationConfig
442 ) -> Vec<Annotation> {
451 annotate_runnables: bool,
452 annotate_impls: bool,
453 ) -> Vec<Annotation> {
458 **Rationale:** reducing churn.
459 If the function has many parameters, they most likely change frequently.
460 By packing them into a struct we protect all intermediary functions from changes.
462 Do not implement `Default` for the `Config` struct, the caller has more context to determine better defaults.
463 Do not store `Config` as a part of the `state`, pass it explicitly.
464 This gives more flexibility for the caller.
466 If there is variation not only in the input parameters, but in the return type as well, consider introducing a `Command` type.
472 pub case_sensitive: bool,
476 pub fn all(self) -> Vec<Item> { ... }
477 pub fn first(self) -> Option<Item> { ... }
481 fn query_all(name: String, case_sensitive: bool) -> Vec<Item> { ... }
482 fn query_first(name: String, case_sensitive: bool) -> Option<Item> { ... }
485 ## Prefer Separate Functions Over Parameters
487 If a function has a `bool` or an `Option` parameter, and it is always called with `true`, `false`, `Some` and `None` literals, split the function in two.
496 foo_with_bar(Bar::new())
500 fn foo_with_bar(bar: Bar) { ... }
508 foo(Some(Bar::new()))
511 fn foo(bar: Option<Bar>) { ... }
514 **Rationale:** more often than not, such functions display "`false sharing`" -- they have additional `if` branching inside for two different cases.
515 Splitting the two different control flows into two functions simplifies each path, and remove cross-dependencies between the two paths.
516 If there's common code between `foo` and `foo_with_bar`, extract *that* into a common helper.
518 ## Appropriate String Types
520 When interfacing with OS APIs, use `OsString`, even if the original source of data is utf-8 encoded.
521 **Rationale:** cleanly delineates the boundary when the data goes into the OS-land.
523 Use `AbsPathBuf` and `AbsPath` over `std::Path`.
524 **Rationale:** rust-analyzer is a long-lived process which handles several projects at the same time.
525 It is important not to leak cwd by accident.
527 # Premature Pessimization
531 Avoid writing code which is slower than it needs to be.
532 Don't allocate a `Vec` where an iterator would do, don't allocate strings needlessly.
536 use itertools::Itertools;
538 let (first_word, second_word) = match text.split_ascii_whitespace().collect_tuple() {
544 let words = text.split_ascii_whitespace().collect::<Vec<_>>();
545 if words.len() != 2 {
550 **Rationale:** not allocating is almost always faster.
552 ## Push Allocations to the Call Site
554 If allocation is inevitable, let the caller allocate the resource:
558 fn frobnicate(s: String) {
563 fn frobnicate(s: &str) {
564 let s = s.to_string();
569 **Rationale:** reveals the costs.
570 It is also more efficient when the caller already owns the allocation.
574 Prefer `rustc_hash::FxHashMap` and `rustc_hash::FxHashSet` instead of the ones in `std::collections`.
576 **Rationale:** they use a hasher that's significantly faster and using them consistently will reduce code size by some small amount.
578 ## Avoid Intermediate Collections
580 When writing a recursive function to compute a sets of things, use an accumulator parameter instead of returning a fresh collection.
581 Accumulator goes first in the list of arguments.
585 pub fn reachable_nodes(node: Node) -> FxHashSet<Node> {
586 let mut res = FxHashSet::default();
590 fn go(acc: &mut FxHashSet<Node>, node: Node) {
592 for n in node.neighbors() {
598 pub fn reachable_nodes(node: Node) -> FxHashSet<Node> {
599 let mut res = FxHashSet::default();
601 for n in node.neighbors() {
602 res.extend(reachable_nodes(n));
608 **Rationale:** re-use allocations, accumulator style is more concise for complex cases.
610 ## Avoid Monomorphization
612 Avoid making a lot of code type parametric, *especially* on the boundaries between crates.
616 fn frobnicate(f: impl FnMut()) {
617 frobnicate_impl(&mut f)
619 fn frobnicate_impl(f: &mut dyn FnMut()) {
624 fn frobnicate(f: impl FnMut()) {
629 Avoid `AsRef` polymorphism, it pays back only for widely used libraries:
633 fn frobnicate(f: &Path) {
637 fn frobnicate(f: impl AsRef<Path>) {
641 **Rationale:** 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*.
642 This allows for exceptionally good performance, but leads to increased compile times.
643 Runtime performance obeys 80%/20% rule -- only a small fraction of code is hot.
644 Compile time **does not** obey this rule -- all code has to be compiled.
650 Separate import groups with blank lines.
651 Use one `use` per crate.
653 Module declarations come before the imports.
654 Order them in "suggested reading order" for a person new to the code base.
663 // Second, external crates (both crates.io crates and other rust-analyzer crates).
664 use crate_foo::{ ... }
665 use crate_bar::{ ... }
667 // Then current crate.
670 // Finally, parent and child modules, but prefer `use crate::`.
673 // Re-exports are treated as item definitions rather than imports, so they go
674 // after imports and modules. Use them sparingly.
678 **Rationale:** consistency.
679 Reading order is important for new contributors.
680 Grouping by crate allows spotting unwanted dependencies easier.
684 Qualify items from `hir` and `ast`.
690 fn frobnicate(func: hir::Function, strukt: ast::Struct) {}
694 use syntax::ast::Struct;
696 fn frobnicate(func: Function, strukt: Struct) {}
699 **Rationale:** avoids name clashes, makes the layer clear at a glance.
701 When implementing traits from `std::fmt` or `std::ops`, import the module:
707 impl fmt::Display for RenameError {
708 fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { .. }
712 impl std::fmt::Display for RenameError {
713 fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { .. }
719 impl Deref for Widget {
721 fn deref(&self) -> &str { .. }
725 **Rationale:** overall, less typing.
726 Makes it clear that a trait is implemented, rather than used.
728 Avoid local `use MyEnum::*` imports.
729 **Rationale:** consistency.
731 Prefer `use crate::foo::bar` to `use super::bar` or `use self::bar::baz`.
732 **Rationale:** consistency, this is the style which works in all cases.
734 By default, avoid re-exports.
735 **Rationale:** for non-library code, re-exports introduce two ways to use something and allow for inconsistency.
739 Optimize for the reader who sees the file for the first time, and wants to get a general idea about what's going on.
740 People read things from top to bottom, so place most important things first.
742 Specifically, if all items except one are private, always put the non-private item on top.
746 pub(crate) fn frobnicate() {
751 struct Helper { stuff: i32 }
761 struct Helper { stuff: i32 }
763 pub(crate) fn frobnicate() {
774 If there's a mixture of private and public items, put public items first.
776 Put `struct`s and `enum`s first, functions and impls last. Order type declarations in top-down manner.
806 **Rationale:** easier to get the sense of the API by visually scanning the file.
807 If function bodies are folded in the editor, the source code should read as documentation for the public API.
809 ## Context Parameters
811 Some parameters are threaded unchanged through many function calls.
812 They determine the "context" of the operation.
813 Pass such parameters first, not last.
814 If there are several context parameters, consider packing them into a `struct Ctx` and passing it as `&self`.
818 fn dfs(graph: &Graph, v: Vertex) -> usize {
819 let mut visited = FxHashSet::default();
820 return go(graph, &mut visited, v);
822 fn go(graph: &Graph, visited: &mut FxHashSet<Vertex>, v: usize) -> usize {
828 fn dfs(v: Vertex, graph: &Graph) -> usize {
829 fn go(v: usize, graph: &Graph, visited: &mut FxHashSet<Vertex>) -> usize {
833 let mut visited = FxHashSet::default();
834 go(v, graph, &mut visited)
838 **Rationale:** consistency.
839 Context-first works better when non-context parameter is a lambda.
843 Use boring and long names for local variables ([yay code completion](https://github.com/rust-lang/rust-analyzer/pull/4162#discussion_r417130973)).
844 The default name is a lowercased name of the type: `global_state: GlobalState`.
845 Avoid ad-hoc acronyms and contractions, but use the ones that exist consistently (`db`, `ctx`, `acc`).
846 Prefer American spelling (color, behavior).
850 * `res` -- "result of the function" local variable
851 * `it` -- I don't really care about the name
852 * `n_foos` -- number of foos (prefer this to `foo_count`)
853 * `foo_idx` -- index of `foo`
855 Many names in rust-analyzer conflict with keywords.
856 We use mangled names instead of `r#ident` syntax:
870 **Rationale:** consistency.
878 fn foo() -> Option<Bar> {
887 fn foo() -> Option<Bar> {
896 **Rationale:** reduce cognitive stack usage.
898 Use `return Err(err)` to throw an error:
902 fn f() -> Result<(), ()> {
910 fn f() -> Result<(), ()> {
918 **Rationale:** `return` has type `!`, which allows the compiler to flag dead
919 code (`Err(...)?` is of unconstrained generic type `T`).
923 When doing multiple comparisons use `<`/`<=`, avoid `>`/`>=`.
927 assert!(lo <= x && x <= hi);
928 assert!(r1 < l2 || r2 < l1);
933 assert!(x >= lo && x <= hi);
934 assert!(r1 < l2 || l1 > r2);
939 **Rationale:** Less-then comparisons are more intuitive, they correspond spatially to [real line](https://en.wikipedia.org/wiki/Real_line).
943 Avoid `if let ... { } else { }` construct, use `match` instead.
947 match ctx.expected_type.as_ref() {
948 Some(expected_type) => completion_ty == expected_type && !expected_type.is_unit(),
953 if let Some(expected_type) = ctx.expected_type.as_ref() {
954 completion_ty == expected_type && !expected_type.is_unit()
960 **Rationale:** `match` is almost always more compact.
961 The `else` branch can get a more precise pattern: `None` or `Err(_)` instead of `_`.
965 Don't use the `ref` keyword.
967 **Rationale:** consistency & simplicity.
968 `ref` was required before [match ergonomics](https://github.com/rust-lang/rfcs/blob/master/text/2005-match-ergonomics.md).
969 Today, it is redundant.
970 Between `ref` and mach ergonomics, the latter is more ergonomic in most cases, and is simpler (does not require a keyword).
974 Use `=> (),` when a match arm is intentionally empty:
980 Err(err) => error!("{}", err),
986 Err(err) => error!("{}", err),
990 **Rationale:** consistency.
992 ## Functional Combinators
994 Use high order monadic combinators like `map`, `then` when they are a natural choice; don't bend the code to fit into some combinator.
995 If writing a chain of combinators creates friction, replace them with control flow constructs: `for`, `if`, `match`.
996 Mostly avoid `bool::then` and `Option::filter`.
1006 Some(x).filter(|it| it.cond())
1009 This rule is more "soft" then others, and boils down mostly to taste.
1010 The guiding principle behind this rule is that code should be dense in computation, and sparse in the number of expressions per line.
1011 The second example contains *less* computation -- the `filter` function is an indirection for `if`, it doesn't do any useful work by itself.
1012 At the same time, it is more crowded -- it takes more time to visually scan it.
1014 **Rationale:** consistency, playing to language's strengths.
1015 Rust has first-class support for imperative control flow constructs like `for` and `if`, while functions are less first-class due to lack of universal function type, currying, and non-first-class effects (`?`, `.await`).
1019 Prefer type ascription over the turbofish.
1020 When ascribing types, avoid `_`
1024 let mutable: Vec<T> = old.into_iter().map(|it| builder.make_mut(it)).collect();
1027 let mutable: Vec<_> = old.into_iter().map(|it| builder.make_mut(it)).collect();
1030 let mutable = old.into_iter().map(|it| builder.make_mut(it)).collect::<Vec<_>>();
1033 **Rationale:** consistency, readability.
1034 If compiler struggles to infer the type, the human would as well.
1035 Having the result type specified up-front helps with understanding what the chain of iterator methods is doing.
1039 Avoid creating single-use helper functions:
1044 let mut buf = get_empty_buf(&mut arena);
1050 let buf = prepare_buf(&mut arena, item);
1054 fn prepare_buf(arena: &mut Arena, item: Item) -> ItemBuf {
1055 let mut res = get_empty_buf(&mut arena);
1061 Exception: if you want to make use of `return` or `?`.
1063 **Rationale:** single-use functions change frequently, adding or removing parameters adds churn.
1064 A block serves just as well to delineate a bit of logic, but has access to all the context.
1065 Re-using originally single-purpose function often leads to bad coupling.
1067 ## Local Helper Functions
1069 Put nested helper functions at the end of the enclosing functions
1070 (this requires using return statement).
1071 Don't nest more than one level deep.
1075 fn dfs(graph: &Graph, v: Vertex) -> usize {
1076 let mut visited = FxHashSet::default();
1077 return go(graph, &mut visited, v);
1079 fn go(graph: &Graph, visited: &mut FxHashSet<Vertex>, v: usize) -> usize {
1085 fn dfs(graph: &Graph, v: Vertex) -> usize {
1086 fn go(graph: &Graph, visited: &mut FxHashSet<Vertex>, v: usize) -> usize {
1090 let mut visited = FxHashSet::default();
1091 go(graph, &mut visited, v)
1095 **Rationale:** consistency, improved top-down readability.
1099 Introduce helper variables freely, especially for multiline conditions:
1103 let rustfmt_not_installed =
1104 captured_stderr.contains("not installed") || captured_stderr.contains("not available");
1106 match output.status.code() {
1107 Some(1) if !rustfmt_not_installed => Ok(None),
1108 _ => Err(format_err!("rustfmt failed:\n{}", captured_stderr)),
1112 match output.status.code() {
1114 if !captured_stderr.contains("not installed")
1115 && !captured_stderr.contains("not available") => Ok(None),
1116 _ => Err(format_err!("rustfmt failed:\n{}", captured_stderr)),
1120 **Rationale:** Like blocks, single-use variables are a cognitively cheap abstraction, as they have access to all the context.
1121 Extra variables help during debugging, they make it easy to print/view important intermediate results.
1122 Giving a name to a condition inside an `if` expression often improves clarity and leads to nicely formatted code.
1126 Use `T![foo]` instead of `SyntaxKind::FOO_KW`.
1131 T![true] | T![false] => true,
1138 SyntaxKind::TRUE_KW | SyntaxKind::FALSE_KW => true,
1143 **Rationale:** The macro uses the familiar Rust syntax, avoiding ambiguities like "is this a brace or bracket?".
1147 Style inline code comments as proper sentences.
1148 Start with a capital letter, end with a dot.
1153 // Only simple single segment paths are allowed.
1154 MergeBehavior::Last => {
1155 tree.use_tree_list().is_none() && tree.path().map(path_len) <= Some(1)
1160 // only simple single segment paths are allowed
1161 MergeBehavior::Last => {
1162 tree.use_tree_list().is_none() && tree.path().map(path_len) <= Some(1)
1166 **Rationale:** writing a sentence (or maybe even a paragraph) rather just "a comment" creates a more appropriate frame of mind.
1167 It tricks you into writing down more of the context you keep in your head while coding.
1169 For `.md` and `.adoc` files, prefer a sentence-per-line format, don't wrap lines.
1170 If the line is too long, you want to split the sentence in two :-)
1172 **Rationale:** much easier to edit the text and read the diff, see [this link](https://asciidoctor.org/docs/asciidoc-recommended-practices/#one-sentence-per-line).