with one click
heimdall-rust-api-discipline
// Rust API design discipline for heimdall — borrowed args, Fn callback lifetimes, HRTB workarounds, Drop design, and value-passing perf for large structs.
// Rust API design discipline for heimdall — borrowed args, Fn callback lifetimes, HRTB workarounds, Drop design, and value-passing perf for large structs.
[HINT] Download the complete skill directory including SKILL.md and all related files
| name | heimdall-rust-api-discipline |
| description | Rust API design discipline for heimdall — borrowed args, Fn callback lifetimes, HRTB workarounds, Drop design, and value-passing perf for large structs. |
Encode API-design discipline from production Rust pitfalls. Apply every rule to every changed public or pub(crate) function signature in a diff. Heimdall-specific file references are included where concrete risk exists.
Severity: WARNING
Flag &String, &Vec<T>, and &PathBuf in public or crate-public function signatures. The owned-reference shapes force callers to hold an allocation even when they have a slice.
// BAD
fn scan(path: &PathBuf) {}
fn log(msg: &String) {}
// GOOD
fn scan(path: &std::path::Path) {} // or: impl AsRef<Path>
fn log(msg: &str) {}
Risk areas in heimdall (check these paths):
src/scanner/ — file-path arguments throughoutsrc/server/ — request/response handler signaturessrc/oauth/ — token and URL string argsGrep: rg 'fn .+\(&String|fn .+\(&Vec<|fn .+\(&PathBuf' src/ --type rust -n
Reference: crabbook/borrowed_args.md
Fn callbacks with reference args in async contextsSeverity: CRITICAL for tokio::spawn closures
Heimdall uses tokio::spawn extensively in src/server/sse.rs and src/scanner/watcher.rs. Closures passed to tokio::spawn must be 'static + Send. Capturing &mut State inside such a closure is a compile error — the reference cannot be 'static.
Correct patterns:
// BAD: captures &mut state -- does not compile
tokio::spawn(async move { state.handle(event) });
// GOOD option 1: Arc<Mutex<State>> for shared mutable state
let state = Arc::new(Mutex::new(State::new()));
let state_clone = Arc::clone(&state);
tokio::spawn(async move { state_clone.lock().await.handle(event) });
// GOOD option 2: mpsc channel -- pass ownership of events to an owner task
let (tx, mut rx) = tokio::sync::mpsc::channel(32);
tokio::spawn(async move {
while let Some(event) = rx.recv().await { state.handle(event); }
});
Decision: Arc<Mutex<T>> is correct for low-contention config/status state. For high-frequency scanner events, prefer message-passing (the channel model above) to avoid lock contention.
Reference: crabbook/borrowing_in_generic_functions.md
Severity: WARNING
When a closure argument fails to type-check because its return type is a reference into the argument (e.g., |s: &str| -> &str), the compiler may infer a fixed lifetime instead of for<'a>. Workaround: name the function or use a helper shim:
fn force_hrtb<F: for<'a> Fn(&'a str) -> &'a str>(f: F) -> F { f }
// Then use: let cb = force_hrtb(|s| s);
In heimdall, this is most likely to arise in src/server/ route handlers and src/scanner/providers/ callback registration patterns.
Reference: crabbook/borrowing_in_generic_functions.md
impl Drop blocks partial movesSeverity: WARNING
Before adding impl Drop to any new struct, check whether any field in that struct needs to be moved out (consumed) — by Drop::drop itself or by external callers. impl Drop makes this impossible (only mem::take or unsafe ptr::read can extract values).
Checklist:
ManuallyDrop.impl Drop is fine — document field drop order (struct fields drop in declaration order).Pattern:
#[repr(transparent)]
struct Guard(std::mem::ManuallyDrop<Resource>);
impl Drop for Guard {
fn drop(&mut self) {
// SAFETY: only called once from Drop
let r = unsafe { std::mem::ManuallyDrop::take(&mut self.0) };
r.close();
}
}
Reference: crabbook/you_dont_want_drop.md
fn(T) -> T vs fn(&mut T) for large structsSeverity: WARNING on hot paths
Heimdall has several large structs that appear in hot-ish paths:
| File | Struct(s) | Approx size |
|---|---|---|
src/models.rs | session, turn, pricing models | 43 KB file |
src/pricing.rs | pricing calculation state | 39 KB file |
src/pricing_sync.rs | sync pricing state | 96 KB file |
src/config.rs | Config | 55 KB file |
For structs larger than 4 pointer-sized fields, fn(T) -> T forces a memcpy in and out — the compiler cannot optimize back to in-place mutation because panic semantics require the original to remain valid during the function. On a path that processes many turns or sessions, this silently multiplies allocation cost.
Rule:
fn(&mut T) for any processing of the large structs above on paths that run per-session, per-turn, or per-pricing-event.fn(T) -> T only for explicit ownership-transfer semantics (builder, state-machine transition on small structs).cargo flamegraph before adopting value-passing chains for large structs.Reference: crabbook/consume_and_borrowing.md
#[derive(Clone)] on resource-backed typesSeverity: WARNING
Deriving Clone on a struct containing Arc<T> does not duplicate the resource — it clones the handle, giving two owners pointing to the same resource. This is often the correct behavior, but must be documented. The hazard is when callers expect an isolated copy (for testing, per-connection isolation) but get aliasing instead.
In heimdall: Config, AppState, and database pool handles are passed with .clone() throughout src/server/ and src/scanner/. Verify that all such clones intentionally share the underlying resource and add a doc comment: /// Cloning shares the underlying pool/config.
Deref on non-pointer types causes method collisionSeverity: WARNING
Deref<Target = T> on a non-smart-pointer newtype exposes all of T's methods via auto-deref. If either Wrapper or T later adds a method with the same name, resolution changes silently — no compile error, different behavior. This also forecloses adding bounds in future versions without breaking callers.
Use AsRef<T>, From<T>/Into<T>, or explicit accessor methods instead of Deref on application newtypes. Check src/models.rs and src/scanner/ for any non-pointer Deref implementations.
async fn in traits not object-safe, no Send boundSeverity: WARNING
async fn in traits (stable Rust 1.75) is not dyn-safe and does not automatically add Send to the returned future. Teams replacing #[async_trait] with native syntax hit:
Box<dyn MyTrait> compile errors (not object-safe).tokio::spawn(obj.method()) failures (future not Send).Fix: use #[trait_variant::make(MyTraitSend: Send)] from the trait-variant crate, or keep #[async_trait] for traits that need dyn dispatch. Do not mass-replace #[async_trait] in heimdall without auditing every tokio::spawn and Box<dyn> use site in src/server/ and src/scanner/.
Apply to every changed public or pub(crate) signature in a diff:
&String, &Vec<T>, or &PathBuf parameter? → prefer &str, &[T], &Path, or impl AsRef<...>.&mut State captured in a tokio::spawn closure? → use Arc<Mutex<T>> or message-passing.force_hrtb shim.impl Drop on a struct with a field that callers need to consume? → use ManuallyDrop guard.fn(T) -> T consuming a large struct (> 4 pointer fields) in per-session/per-turn code? → use fn(&mut T).#[derive(Clone)] on a struct with Arc<T> fields where callers might expect an isolated copy? → add doc comment clarifying sharing semantics.Deref on a non-smart-pointer newtype? → use AsRef/From/accessors instead.async fn in a trait now used with dyn Trait or tokio::spawn? → add trait_variant or keep #[async_trait].