po4yka/rust-skills/rust-code-style

Rust Code Style

This skill holds the layout and readability decisions that rustfmt cannot make. rustfmt owns line width, brace placement, wrapping, and the sort order inside each contiguous block of mod declarations or use imports. The rust-lints skill owns clippy.toml, rustfmt.toml, and the workspace lint tables. The rust-crate-architecture skill owns crate boundaries and dependency direction.

Most rules here are catalog defaults, not Rust facts. Each one carries its reason, so a team can override it on purpose. The flat_map and lines() rules are exceptions: they prevent lost errors and hangs, so they hold everywhere.

Checks

Use the tool where one exists. Review the rest by hand with the checklist.

No tool checks import groups, the rest of the item order, caller-before-callee order, the depth exception for mod.rs, or the early-return rule. A green run proves nothing about them.

Module layout

Use name.rs next to a name/ directory for every module that has children. This is the default because the module root then sits next to its children, and every editor tab has a distinct name. Use mod.rs only three levels deep or more. Do not mix the two patterns at one directory level: a reader then cannot predict where a module root is.

lib.rs declares the modules first, then re-exports the public API item by item, with no glob. A glob re-export makes every new pub item public API with no review. A pub mod prelude is the one module that a caller may glob-import. Keep it short, and build it from explicit pub use items. Avoid #[path].

Read references/module-layout.md when you create lib.rs or a prelude, when a downstream crate reports E0659, E0034, or "X is ambiguous" after a prelude change, when you enforce the layout with a lint, or when you include build.rs output.

Visibility

Start private. Widen one step only when a concrete caller in another module needs the item. Remove the widening when the caller goes away.

pub(super) follows the module tree, so check its callers again when you move the module. Do not write pub(in path) without a written reason: it names a module path, and a module move breaks the build.

unreachable_pub flags a pub item that no path from the crate root reaches, and suggests pub(crate). Fix the item; do not suppress the lint.

Do not widen visibility for a test. A child mod tests in the same file already reads the private items of its parent. If a test needs an item from another module, the item is part of the pub(crate) API, or the test belongs in the module that owns the item.

File structure order

Inside a .rs file, place the items in this order:

1. Crate and module attributes (#![forbid(unsafe_code)],

#![doc = include_str!("../README.md")]).

1. mod declarations: private first, then pub mod, with a blank line between the two groups.

The #[cfg(test)] mod tests declaration goes at the bottom (step 10).

1. use imports (see Import style).
2. pub use re-exports.
3. Constants and statics.
4. Type definitions: structs, enums, type aliases.
5. Trait definitions.
6. impl blocks: inherent first, then trait impls.
7. Free functions: public first, then private helpers.
8. #[cfg(test)] mod tests.

Put a blank line between steps 3 and 4 too. rustfmt (reorder_modules and reorder_imports, stable and on by default) sorts each contiguous block of mod or use lines by name and ignores visibility. Without the blank lines, cargo fmt mixes mod with pub mod, and moves pub use crate::... above use std::... (rustfmt 1.9.0, Rust 1.98.1).

Steps 2 to 4 set their own order. Inside each of steps 5 to 9, put public items before private items. The file then reads as a table of contents: the API surface comes before the internals. Put a type before its impl blocks, because the reader needs the data shape before the methods. Put inherent impls before trait impls, because the inherent methods are the core API.

Import style

Separate the imports into four groups, with one blank line between the groups:

use std::collections::HashMap;
use std::sync::{Arc, Mutex};
 
use serde::{Deserialize, Serialize};
use tokio::sync::mpsc;
 
use app_protocol::FrameKind;
use app_telemetry::SpanId;
 
use crate::types::SharedState;
use crate::util::format_duration;

The order is: std (and core, alloc), then external crates, then workspace crates, then crate::, self::, and super::.

Stable rustfmt sorts the imports inside a group. It does not move an import between groups, because group_imports is unstable. Keep the groups by hand, and keep the pub use block apart from them with a blank line (see File structure order).

The sort order inside a group depends on the style edition. Style edition 2024 uses version sorting and puts u8 before u16; style editions 2015 to 2021 put them the other way round (rustfmt 1.9.0, Rust 1.98.1). A direct rustfmt call, for example from a pre-commit hook, can therefore disagree with cargo fmt, and cargo fmt --check then fails in CI. The rust-lints skill owns the edition and style_edition keys in rustfmt.toml. The cargo-workflows skill says when to change them during an edition migration.

Use one braced import for two or more items of one module: use std::sync::{Arc, Mutex};. Stable rustfmt does not merge imports, because imports_granularity is unstable.

Import the items and traits that the file uses often. Write the full path at the call site for an item that the file uses once, such as std::time::Instant::now(). This default keeps the import block short and reduces merge conflicts.

Function rules

Caller before callee

Place a calling function before the functions it calls. The reader then follows the code from top to bottom.

Iterator chains and collectors

Keep every closure in a .map(), .filter(), or .collect() chain pure. Write a side effect as a for loop.

Ban Iterator::for_each through disallowed-methods in clippy.toml. This is a catalog default, not a Rust rule: a for loop covers every side-effecting use and shows the effect in plain sight. clippy::needless_for_each (pedantic) is a lighter built-in check, but it does not fire on a chain such as .filter(..).for_each(..). The rust-lints skill holds the clippy.toml entry.

Never flat_map or flatten over a Result. Err yields zero items, so the failure disappears, and no lint or type error reports it. Use collect into a Result to stop at the first error, or partition to keep both halves. Use filter_map with .ok() only when the drop is the intent.

Never skip the errors of BufRead::lines() with filter_map, flat_map, or flatten: the read can hang. A reader that fails on every call, such as a directory opened as a file on Unix, makes lines() yield Err forever, so the first next() never returns. Collect into io::Result<Vec<String>> to propagate the error. Use map_while(Result::ok) only when a silent stop at the first error is the intent.

Read references/iterator-style.md when you judge whether an expression or closure hides a side effect, choose a collector for fallible items, or see a benchmark that argues for for_each.

Business paths use if/else and match

Keep a guard return (return, let ... else) for bookkeeping: a null check, a handle check, a permission guard. This rule does not limit ?, which propagates an error. Write mutually exclusive business paths as if/else or match. This is a catalog default: one match shows every alternative in one place, while a chain of early returns makes the reader hold every earlier condition to know when the last line runs.

Error handling

A library returns a typed error, so the caller can match on the variant:

#[derive(Debug, thiserror::Error)]
pub enum SessionError {
    #[error("the request timed out after {0:?}")]
    Timeout(std::time::Duration),
 
    #[error("the handle is not valid")]
    InvalidHandle,
 
    #[error("transport failure")]
    Transport(#[from] std::io::Error),
}

Do not put anyhow::Error in the public signature of a library crate. It erases the variants, so the caller can only print the message.

A binary reports the error to a person. Add context at each layer:

use anyhow::Context as _;
 
fn load(path: &std::path::Path) -> anyhow::Result<String> {
    let text = std::fs::read_to_string(path)
        .with_context(|| format!("failed to read the config file {}", path.display()))?;
    anyhow::ensure!(!text.trim().is_empty(), "the config file {} is empty", path.display());
    Ok(text)
}

Propagate with ? and let the caller decide. In non-test code, .expect("<invariant>") is acceptable only for a documented, unconditional invariant. The rust-discipline skill owns that rule and the lock-poisoning policy. The rust-panic-safety skill owns panic policy at a boundary. The ffi-error-progress-cancel skill owns error mapping across an FFI boundary.

Rustdoc contract

A doc comment on a public item is part of the API. Three sections carry information that the signature cannot. cargo doc renders each one as a heading, so a reader finds it in the same place every time.

use std::path::Path;
 
pub enum ConfigError { NotFound, PermissionDenied, InvalidUtf8 }