Minimal Tests

Most tests start with a snippet of Rust code. 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.

It also makes sense to format snippets more compactly (for example, by placing enum definitions like enum E { Foo, Bar } on a single line), as long as they are still readable.

When using multiline fixtures, use unindented raw string literals:

    #[test]
    fn inline_field_shorthand() {
        check_assist(
            inline_local_variable,
            r#"
struct S { foo: i32}
fn main() {
    let $0foo = 92;
    S { foo }
}
"#,
            r#"
struct S { foo: i32}
fn main() {
    S { foo: 92 }
}
"#,
        );
    }

Rationale:

There are many benefits to this:

• less to read or to scroll past
• easier to understand what exactly is tested
• less stuff printed during printf-debugging
• less time to run test

Formatting ensures that you can use your editor’s “number of selected characters” feature to correlate offsets with test’s source code.

Tests

#[should_panic]

Do not use #[should_panic] tests. Instead, explicitly check for None, Err, etc.

Rationale: #[should_panic] is a tool for library authors to make sure that the API does not fail silently when misused. 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. Panic messages in the logs from the #[should_panic] tests are confusing.

#[ignore]

Do not #[ignore] tests. If the test currently does not work, assert the wrong behavior and add a fixme explaining why it is wrong.

Rationale: noticing when the behavior is fixed, making sure that even the wrong behavior is acceptable (ie, not a panic).

Function Preconditions

Express function preconditions in types and force the caller to provide them (rather than checking in callee):

// GOOD
fn frobnicate(walrus: Walrus) {
    ...
}

// BAD
fn frobnicate(walrus: Option<Walrus>) {
    let walrus = match walrus {
        Some(it) => it,
        None => return,
    };
    ...
}

Rationale: this makes control flow explicit at the call site. Call-site has more context, it often happens that the precondition falls out naturally or can be bubbled up higher in the stack.

Avoid splitting precondition check and precondition use across functions:

// GOOD
fn main() {
    let s: &str = ...;
    if let Some(contents) = string_literal_contents(s) {

    }
}

fn string_literal_contents(s: &str) -> Option<&str> {
    if s.starts_with('"') && s.ends_with('"') {
        Some(&s[1..s.len() - 1])
    } else {
        None
    }
}

// BAD
fn main() {
    let s: &str = ...;
    if is_string_literal(s) {
        let contents = &s[1..s.len() - 1];
    }
}

fn is_string_literal(s: &str) -> bool {
    s.starts_with('"') && s.ends_with('"')
}

In the “Bad” version, the precondition that 1 and s.len() - 1 are valid string literal boundaries is checked in is_string_literal but used in main. In the “Good” version, the precondition check and usage are checked in the same block, and then encoded in the types.

Rationale: non-local code properties degrade under change.

Control Flow

As a special case of the previous rule, do not hide control flow inside functions, push it to the caller:

// GOOD
if cond {
    f()
}

// BAD
fn f() {
    if !cond {
        return;
    }
    ...
}

See also this post

Assertions

Assert liberally. Prefer stdx::never! to standard assert!.

Rationale: See cross cutting concern: error handling.

Getters & Setters

If a field can have any value without breaking invariants, make the field public. Conversely, if there is an invariant, document it, enforce it in the “constructor” function, make the field private, provide a getter and a setter that keeps the invariant if needed.

Getters should return borrowed data:

struct Person {
    // Invariant: never empty
    first_name: String,
    middle_name: Option<String>
}

// GOOD
impl Person {
    fn first_name(&self) -> &str { self.first_name.as_str() }
    fn middle_name(&self) -> Option<&str> { self.middle_name.as_ref() }
}

// BAD
impl Person {
    fn first_name(&self) -> String { self.first_name.clone() }
    fn middle_name(&self) -> &Option<String> { &self.middle_name }
}

Rationale: we don’t provide public API, it’s cheaper to refactor than to pay getters rent. Non-local code properties degrade under change, privacy makes invariant local. Borrowed owned types (&String) disclose irrelevant details about internal representation. Irrelevant (neither right nor wrong) things obscure correctness.

Useless Types

More generally, always prefer types on the left

// GOOD      BAD
&[T]         &Vec<T>
&str         &String
Option<&T>   &Option<T>
&Path        &PathBuf

Rationale: types on the left are strictly more general. Even when generality is not required, consistency is important.

Constructors

Prefer Default to zero-argument new function.

// GOOD
#[derive(Default)]
struct Foo {
    bar: Option<Bar>
}

// BAD
struct Foo {
    bar: Option<Bar>
}

impl Foo {
    fn new() -> Foo {
        Foo { bar: None }
    }
}

Prefer Default even if it has to be implemented manually.

Rationale: less typing in the common case, uniformity.

Avoid using “dummy” states to implement a Default. If a type doesn’t have a sensible default, empty value, don’t hide it. Let the caller explicitly decide what the right initial state is.

Functions Over Objects

Avoid creating “doer” objects. That is, objects which are created only to execute a single action.

// GOOD
do_thing(arg1, arg2);

// BAD
ThingDoer::new(arg1, arg2).do();

Note that this concerns only outward API. When implementing do_thing, it might be very useful to create a context object.

pub fn do_thing(arg1: Arg1, arg2: Arg2) -> Res {
    let mut ctx = Ctx { arg1, arg2 };
    ctx.run()
}

struct Ctx {
    arg1: Arg1, arg2: Arg2
}

impl Ctx {
    fn run(self) -> Res {
        ...
    }
}

The difference is that Ctx is an impl detail here.

Sometimes a middle ground is acceptable if this can save some busywork:

ThingDoer::do(arg1, arg2);

pub struct ThingDoer {
    arg1: Arg1, arg2: Arg2,
}

impl ThingDoer {
    pub fn do(arg1: Arg1, arg2: Arg2) -> Res {
        ThingDoer { arg1, arg2 }.run()
    }
    fn run(self) -> Res {
        ...
    }
}

Rationale: not bothering the caller with irrelevant details, not mixing user API with implementor API.

Functions with many parameters

Avoid creating functions with many optional or boolean parameters. Introduce a Config struct instead.

// GOOD
pub struct AnnotationConfig {
    pub binary_target: bool,
    pub annotate_runnables: bool,
    pub annotate_impls: bool,
}

pub fn annotations(
    db: &RootDatabase,
    file_id: FileId,
    config: AnnotationConfig
) -> Vec<Annotation> {
    ...
}

// BAD
pub fn annotations(
    db: &RootDatabase,
    file_id: FileId,
    binary_target: bool,
    annotate_runnables: bool,
    annotate_impls: bool,
) -> Vec<Annotation> {
    ...
}

Rationale: reducing churn. If the function has many parameters, they most likely change frequently. By packing them into a struct we protect all intermediary functions from changes.

Do not implement Default for the Config struct, the caller has more context to determine better defaults. Do not store Config as a part of the state, pass it explicitly. This gives more flexibility for the caller.

If there is variation not only in the input parameters, but in the return type as well, consider introducing a Command type.

// MAYBE GOOD
pub struct Query {
    pub name: String,
    pub case_sensitive: bool,
}

impl Query {
    pub fn all(self) -> Vec<Item> { ... }
    pub fn first(self) -> Option<Item> { ... }
}

// MAYBE BAD
fn query_all(name: String, case_sensitive: bool) -> Vec<Item> { ... }
fn query_first(name: String, case_sensitive: bool) -> Option<Item> { ... }

Prefer Separate Functions Over Parameters

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.

// GOOD
fn caller_a() {
    foo()
}

fn caller_b() {
    foo_with_bar(Bar::new())
}

fn foo() { ... }
fn foo_with_bar(bar: Bar) { ... }

// BAD
fn caller_a() {
    foo(None)
}

fn caller_b() {
    foo(Some(Bar::new()))
}

fn foo(bar: Option<Bar>) { ... }

Rationale: more often than not, such functions display “false sharing” – they have additional if branching inside for two different cases. Splitting the two different control flows into two functions simplifies each path, and remove cross-dependencies between the two paths. If there’s common code between foo and foo_with_bar, extract that into a common helper.

Premature Pessimization

Avoid Allocations

Avoid writing code which is slower than it needs to be. Don’t allocate a Vec where an iterator would do, don’t allocate strings needlessly.

// GOOD
use itertools::Itertools;

let (first_word, second_word) = match text.split_ascii_whitespace().collect_tuple() {
    Some(it) => it,
    None => return,
}

// BAD
let words = text.split_ascii_whitespace().collect::<Vec<_>>();
if words.len() != 2 {
    return
}

Rationale: not allocating is almost always faster.

Push Allocations to the Call Site

If allocation is inevitable, let the caller allocate the resource:

// GOOD
fn frobnicate(s: String) {
    ...
}

// BAD
fn frobnicate(s: &str) {
    let s = s.to_string();
    ...
}

Rationale: reveals the costs. It is also more efficient when the caller already owns the allocation.

Avoid Intermediate Collections

When writing a recursive function to compute a sets of things, use an accumulator parameter instead of returning a fresh collection. Accumulator goes first in the list of arguments.

// GOOD
pub fn reachable_nodes(node: Node) -> FxHashSet<Node> {
    let mut res = FxHashSet::default();
    go(&mut res, node);
    res
}
fn go(acc: &mut FxHashSet<Node>, node: Node) {
    acc.insert(node);
    for n in node.neighbors() {
        go(acc, n);
    }
}

// BAD
pub fn reachable_nodes(node: Node) -> FxHashSet<Node> {
    let mut res = FxHashSet::default();
    res.insert(node);
    for n in node.neighbors() {
        res.extend(reachable_nodes(n));
    }
    res
}

Rationale: re-use allocations, accumulator style is more concise for complex cases.

Avoid Monomorphization

Avoid making a lot of code type parametric, especially on the boundaries between crates.

// GOOD
fn frobnicate(mut f: impl FnMut()) {
    frobnicate_impl(&mut f)
}
fn frobnicate_impl(f: &mut dyn FnMut()) {
    // lots of code
}

// BAD
fn frobnicate(mut f: impl FnMut()) {
    // lots of code
}

Avoid AsRef polymorphism, it pays back only for widely used libraries:

// GOOD
fn frobnicate(f: &Path) {
}

// BAD
fn frobnicate(f: impl AsRef<Path>) {
}

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. This allows for exceptionally good performance, but leads to increased compile times. Runtime performance obeys 80%/20% rule – only a small fraction of code is hot. Compile time does not obey this rule – all code has to be compiled.

Style

Order of Imports

Separate import groups with blank lines. Use one use per crate.

Module declarations come before the imports. Order them in “suggested reading order” for a person new to the code base.

mod x;
mod y;

// First std.
use std::{ ... }

// Second, external crates (both crates.io crates and other rust-analyzer crates).
use crate_foo::{ ... }
use crate_bar::{ ... }

// Then current crate.
use crate::{}

// Finally, parent and child modules, but prefer `use crate::`.
use super::{}

// Re-exports are treated as item definitions rather than imports, so they go
// after imports and modules. Use them sparingly.
pub use crate::x::Z;

Rationale: consistency. Reading order is important for new contributors. Grouping by crate allows spotting unwanted dependencies easier.

Import Style

When implementing traits from std::fmt or std::ops, import the module:

// GOOD
use std::fmt;

impl fmt::Display for RenameError {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { .. }
}

// BAD
impl std::fmt::Display for RenameError {
    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { .. }
}

// BAD
use std::ops::Deref;

impl Deref for Widget {
    type Target = str;
    fn deref(&self) -> &str { .. }
}

Rationale: overall, less typing. Makes it clear that a trait is implemented, rather than used.

Avoid local use MyEnum::* imports. Rationale: consistency.

Prefer use crate::foo::bar to use super::bar or use self::bar::baz. Rationale: consistency, this is the style which works in all cases.

By default, avoid re-exports. Rationale: for non-library code, re-exports introduce two ways to use something and allow for inconsistency.

Order of Items

Optimize for the reader who sees the file for the first time, and wants to get a general idea about what’s going on. People read things from top to bottom, so place most important things first.

Specifically, if all items except one are private, always put the non-private item on top.

// GOOD
pub(crate) fn frobnicate() {
    Helper::act()
}

#[derive(Default)]
struct Helper { stuff: i32 }

impl Helper {
    fn act(&self) {

    }
}

// BAD
#[derive(Default)]
struct Helper { stuff: i32 }

pub(crate) fn frobnicate() {
    Helper::act()
}

impl Helper {
    fn act(&self) {

    }
}

If there’s a mixture of private and public items, put public items first.

Put structs and enums first, functions and impls last. Order type declarations in top-down manner.

// GOOD
struct Parent {
    children: Vec<Child>
}

struct Child;

impl Parent {
}

impl Child {
}

// BAD
struct Child;

impl Child {
}

struct Parent {
    children: Vec<Child>
}

impl Parent {
}

Rationale: easier to get the sense of the API by visually scanning the file. If function bodies are folded in the editor, the source code should read as documentation for the public API.

Context Parameters

Some parameters are threaded unchanged through many function calls. They determine the “context” of the operation. Pass such parameters first, not last. If there are several context parameters, consider packing them into a struct Ctx and passing it as &self.

// GOOD
fn dfs(graph: &Graph, v: Vertex) -> usize {
    let mut visited = FxHashSet::default();
    return go(graph, &mut visited, v);

    fn go(graph: &Graph, visited: &mut FxHashSet<Vertex>, v: usize) -> usize {
        ...
    }
}

// BAD
fn dfs(v: Vertex, graph: &Graph) -> usize {
    fn go(v: usize, graph: &Graph, visited: &mut FxHashSet<Vertex>) -> usize {
        ...
    }

    let mut visited = FxHashSet::default();
    go(v, graph, &mut visited)
}

Rationale: consistency. Context-first works better when non-context parameter is a lambda.

Variable Naming

Use boring and long names for local variables. The default name is a lowercased name of the type: global_state: GlobalState. Avoid ad-hoc acronyms and contractions, but use the ones that exist consistently (db, ctx, acc). Prefer American spelling (color, behavior).

Default names:

• res – “result of the function” local variable
• it – I don’t really care about the name
• foo_count – number of foos
• foo_idx – index of foo

Many names in rust-analyzer conflict with keywords. We use mangled names instead of r#ident syntax:

crate  -> krate
enum   -> enum_
fn     -> func
impl   -> imp
macro  -> mac
mod    -> module
struct -> strukt
trait  -> trait_
type   -> ty

Rationale: consistency.

Error Handling Trivia

Use anyhow::Result rather than just Result.

Rationale: makes it immediately clear what result that is.

Use anyhow::format_err! rather than anyhow::anyhow.

Rationale: consistent, boring, avoids stuttering.

There’s no specific guidance on the formatting of error messages, see anyhow/#209. Do not end error and context messages with . though.

Early Returns

Do use early returns

// GOOD
fn foo() -> Option<Bar> {
    if !condition() {
        return None;
    }

    Some(...)
}

// BAD
fn foo() -> Option<Bar> {
    if condition() {
        Some(...)
    } else {
        None
    }
}

Rationale: reduce cognitive stack usage.

Use return Err(err) to throw an error:

// GOOD
fn f() -> Result<(), ()> {
    if condition {
        return Err(());
    }
    Ok(())
}

// BAD
fn f() -> Result<(), ()> {
    if condition {
        Err(())?;
    }
    Ok(())
}

Rationale: return has type !, which allows the compiler to flag dead code (Err(...)? is of unconstrained generic type T).

Comparisons

When doing multiple comparisons use </<=, avoid >/>=.

// GOOD
assert!(lo <= x && x <= hi);
assert!(r1 < l2 || r2 < l1);
assert!(x < y);
assert!(0 < x);

// BAD
assert!(x >= lo && x <= hi);
assert!(r1 < l2 || l1 > r2);
assert!(y > x);
assert!(x > 0);

Rationale: Less-then comparisons are more intuitive, they correspond spatially to real line.

If-let

Avoid if let ... { } else { } construct, use match instead.

// GOOD
match ctx.expected_type.as_ref() {
    Some(expected_type) => completion_ty == expected_type && !expected_type.is_unit(),
    None => false,
}

// BAD
if let Some(expected_type) = ctx.expected_type.as_ref() {
    completion_ty == expected_type && !expected_type.is_unit()
} else {
    false
}

Rationale: match is almost always more compact. The else branch can get a more precise pattern: None or Err(_) instead of _.

Match Ergonomics

Don’t use the ref keyword.

Rationale: consistency & simplicity. ref was required before match ergonomics. Today, it is redundant. Between ref and match ergonomics, the latter is more ergonomic in most cases, and is simpler (does not require a keyword).

Empty Match Arms

Use => (), when a match arm is intentionally empty:

// GOOD
match result {
    Ok(_) => (),
    Err(err) => error!("{}", err),
}

// BAD
match result {
    Ok(_) => {}
    Err(err) => error!("{}", err),
}

Rationale: consistency.

Functional Combinators

Use high order monadic combinators like map, then when they are a natural choice; don’t bend the code to fit into some combinator. If writing a chain of combinators creates friction, replace them with control flow constructs: for, if, match. Mostly avoid bool::then and Option::filter.

// GOOD
if !x.cond() {
    return None;
}
Some(x)

// BAD
Some(x).filter(|it| it.cond())

This rule is more “soft” then others, and boils down mostly to taste. The guiding principle behind this rule is that code should be dense in computation, and sparse in the number of expressions per line. The second example contains less computation – the filter function is an indirection for if, it doesn’t do any useful work by itself. At the same time, it is more crowded – it takes more time to visually scan it.

Rationale: consistency, playing to language’s strengths. 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).

Turbofish

Prefer type ascription over the turbofish. When ascribing types, avoid _

// GOOD
let mutable: Vec<T> = old.into_iter().map(|it| builder.make_mut(it)).collect();

// BAD
let mutable: Vec<_> = old.into_iter().map(|it| builder.make_mut(it)).collect();

// BAD
let mutable = old.into_iter().map(|it| builder.make_mut(it)).collect::<Vec<_>>();

Rationale: consistency, readability. If compiler struggles to infer the type, the human would as well. Having the result type specified up-front helps with understanding what the chain of iterator methods is doing.

Helper Functions

Avoid creating single-use helper functions:

// GOOD
let buf = {
    let mut buf = get_empty_buf(&mut arena);
    buf.add_item(item);
    buf
};

// BAD
let buf = prepare_buf(&mut arena, item);

...

fn prepare_buf(arena: &mut Arena, item: Item) -> ItemBuf {
    let mut res = get_empty_buf(&mut arena);
    res.add_item(item);
    res
}

Exception: if you want to make use of return or ?.

Rationale: single-use functions change frequently, adding or removing parameters adds churn. A block serves just as well to delineate a bit of logic, but has access to all the context. Re-using originally single-purpose function often leads to bad coupling.

Local Helper Functions

Put nested helper functions at the end of the enclosing functions (this requires using return statement). Don’t nest more than one level deep.

// GOOD
fn dfs(graph: &Graph, v: Vertex) -> usize {
    let mut visited = FxHashSet::default();
    return go(graph, &mut visited, v);

    fn go(graph: &Graph, visited: &mut FxHashSet<Vertex>, v: usize) -> usize {
        ...
    }
}

// BAD
fn dfs(graph: &Graph, v: Vertex) -> usize {
    fn go(graph: &Graph, visited: &mut FxHashSet<Vertex>, v: usize) -> usize {
        ...
    }

    let mut visited = FxHashSet::default();
    go(graph, &mut visited, v)
}

Rationale: consistency, improved top-down readability.

Helper Variables

Introduce helper variables freely, especially for multiline conditions:

// GOOD
let rustfmt_not_installed =
    captured_stderr.contains("not installed") || captured_stderr.contains("not available");

match output.status.code() {
    Some(1) if !rustfmt_not_installed => Ok(None),
    _ => Err(format_err!("rustfmt failed:\n{}", captured_stderr)),
};

// BAD
match output.status.code() {
    Some(1)
        if !captured_stderr.contains("not installed")
           && !captured_stderr.contains("not available") => Ok(None),
    _ => Err(format_err!("rustfmt failed:\n{}", captured_stderr)),
};

Rationale: Like blocks, single-use variables are a cognitively cheap abstraction, as they have access to all the context. Extra variables help during debugging, they make it easy to print/view important intermediate results. Giving a name to a condition inside an if expression often improves clarity and leads to nicely formatted code.

Token names

Use T![foo] instead of SyntaxKind::FOO_KW.

// GOOD
match p.current() {
    T![true] | T![false] => true,
    _ => false,
}

// BAD

match p.current() {
    SyntaxKind::TRUE_KW | SyntaxKind::FALSE_KW => true,
    _ => false,
}

Rationale: The macro uses the familiar Rust syntax, avoiding ambiguities like “is this a brace or bracket?”.

Documentation

Style inline code comments as proper sentences. Start with a capital letter, end with a dot.

// GOOD

// Only simple single segment paths are allowed.
MergeBehavior::Last => {
    tree.use_tree_list().is_none() && tree.path().map(path_len) <= Some(1)
}

// BAD

// only simple single segment paths are allowed
MergeBehavior::Last => {
    tree.use_tree_list().is_none() && tree.path().map(path_len) <= Some(1)
}

Rationale: writing a sentence (or maybe even a paragraph) rather just “a comment” creates a more appropriate frame of mind. It tricks you into writing down more of the context you keep in your head while coding.

For .md files prefer a sentence-per-line format, don’t wrap lines. If the line is too long, you want to split the sentence in two :-)

Rationale: much easier to edit the text and read the diff, see this link.

Logging

Always use tracing for logging, never use println! or eprintln!.