rust.yaml

# Augment Code Review Guidelines for Rust
# Version: 1.0
# Last Updated: 2026-01-02

name: Rust Code Review Guidelines
description: Comprehensive code review rules for Rust covering safety, memory, performance, best practices, concurrency, and common pitfalls
globs:
  - "**/*.rs"

rules:
  # ============================================================================
  # SAFETY - Unsafe Code and Memory Safety
  # ============================================================================

  - id: rust-unsafe-block-justification
    name: Unsafe blocks must be justified with safety comments
    description: |
      Every `unsafe` block should have a `// SAFETY:` comment explaining why the unsafe code is sound.
      This helps reviewers verify the safety invariants and aids future maintenance.
      Bad: unsafe { ptr::write(dest, src); }
      Good: // SAFETY: dest is a valid, aligned pointer obtained from Box::into_raw(),
      and we have exclusive access to the memory location.
      unsafe { ptr::write(dest, src); }
    severity: high

  - id: rust-minimize-unsafe-scope
    name: Minimize unsafe block scope
    description: |
      Keep unsafe blocks as small as possible. Only wrap the specific operations that require unsafe,
      not entire functions or large code blocks. This makes it easier to audit and reason about safety.
    severity: high

  - id: rust-raw-pointer-validation
    name: Validate raw pointers before dereferencing
    description: |
      Raw pointers must be validated for null and alignment before dereferencing.
      Consider using NonNull<T> for pointers that should never be null.
      Bad: unsafe fn process(ptr: *const i32) -> i32 { *ptr }
      Good: Check ptr.is_null() || !ptr.is_aligned() before dereferencing.
    severity: high

  - id: rust-transmute-danger
    name: Avoid std::mem::transmute when safer alternatives exist
    description: |
      `transmute` is extremely dangerous and can easily cause undefined behavior.
      Prefer safer alternatives like `from_ne_bytes`, `to_ne_bytes`, pointer casts, or `bytemuck` crate.
      Bad: let bytes: [u8; 4] = unsafe { std::mem::transmute(value) };
      Good: let bytes: [u8; 4] = value.to_ne_bytes();
    severity: high

  - id: rust-ffi-safety
    name: FFI boundaries require careful safety handling
    description: |
      When interfacing with C code, ensure proper handling of null pointers, string encoding,
      ownership semantics, and exception safety. Use #[repr(C)] for FFI structs and document
      ownership transfer clearly.
    severity: high

  # ============================================================================
  # MEMORY - Lifetimes and Ownership
  # ============================================================================

  - id: rust-lifetime-annotations
    name: Use explicit lifetime annotations when unclear
    description: |
      While Rust can elide lifetimes in many cases, explicit annotations improve readability
      when the relationship between input and output lifetimes is non-obvious.
    severity: medium

  - id: rust-avoid-unnecessary-clone
    name: Avoid unnecessary clone() calls
    description: |
      Cloning can be expensive, especially for heap-allocated types. Consider borrowing,
      using references, or restructuring code to avoid clones. Use Cow<T> for conditional ownership.
      Bad: fn process(data: &String) { let owned = data.clone(); println!("{}", owned); }
      Good: fn process(data: &str) { println!("{}", data); }
    severity: medium

  - id: rust-arc-rc-usage
    name: Choose Arc vs Rc appropriately based on thread safety needs
    description: |
      Use Rc<T> for single-threaded reference counting (cheaper).
      Use Arc<T> when sharing across threads. Don't use Arc when Rc would suffice.
      Consider if ownership can be restructured to avoid reference counting entirely.
    severity: medium

  - id: rust-box-when-needed
    name: Use Box only when heap allocation is necessary
    description: |
      Don't box values unnecessarily. Box is appropriate for recursive types, large values
      that shouldn't be on the stack, or trait objects. Stack allocation is faster when possible.
    severity: low

  # ============================================================================
  # PERFORMANCE
  # ============================================================================

  - id: rust-avoid-allocations-hot-path
    name: Avoid allocations in hot paths
    description: |
      In performance-critical code, avoid heap allocations. Use stack-allocated arrays,
      ArrayVec, SmallVec, or pre-allocated buffers. Reuse allocations where possible.
      Bad: Creating new Vec in hot loop.
      Good: Use Vec::with_capacity and clear() to reuse allocation.
    severity: medium

  - id: rust-iterator-vs-loop
    name: Prefer iterators over manual loops for better optimization
    description: |
      Iterator chains often optimize better than manual loops due to LLVM's ability to
      vectorize and inline them. They're also more idiomatic and often clearer.
      Bad: for i in 0..v.len() { sum += v[i]; } with bounds checking on each access.
      Good: let sum: i32 = v.iter().sum(); with bounds check eliminated.
    severity: low

  - id: rust-collect-type-annotation
    name: Provide type annotation when using collect()
    description: |
      Always annotate the target type when using collect() to avoid ambiguity and help
      the compiler optimize. Using turbofish syntax or type annotation on the binding.
      Bad: let items = iter.collect();
      Good: let items: Vec<_> = iter.collect(); or iter.collect::<Vec<_>>();
    severity: low

  - id: rust-string-formatting
    name: Use write! macro for building strings in loops
    description: |
      When building strings incrementally, use write! with a String or use format_args!
      instead of repeated format! calls which allocate intermediate strings.
    severity: low

  - id: rust-zero-cost-abstractions
    name: Trust zero-cost abstractions but verify in hot paths
    description: |
      Rust's abstractions (iterators, Option, Result) are zero-cost, but in critical
      hot paths, verify with benchmarks that the compiler optimizes as expected.
    severity: low

  # ============================================================================
  # BEST PRACTICES - Error Handling and Idiomatic Rust
  # ============================================================================

  - id: rust-result-option-handling
    name: Handle Result and Option explicitly without silent failures
    description: |
      Don't silently ignore errors. Use ?, match, or combinators like map_err, ok_or,
      unwrap_or_else. If ignoring intentionally, use `let _ = ...` with a comment.
      Bad: let _ = file.write(data); without explanation.
      Good: file.write(data)?; or comment explaining intentional ignore.
    severity: high

  - id: rust-custom-error-types
    name: Use custom error types for library code
    description: |
      Library code should define custom error types using thiserror or manual Error impl.
      Avoid using String as error type. Use anyhow for applications, thiserror for libraries.
      Bad: fn parse(s: &str) -> Result<Data, String>
      Good: Use #[derive(Debug, thiserror::Error)] pub enum ParseError { ... }
    severity: medium

  - id: rust-derive-macros
    name: Use derive macros appropriately
    description: |
      Derive common traits (Debug, Clone, PartialEq, etc.) when appropriate. Don't derive
      Clone for types with expensive clone operations without considering the implications.
      Always derive Debug for public types.
    severity: low

  - id: rust-borrowing-patterns
    name: Prefer borrowing over ownership when possible
    description: |
      Functions should accept references unless they need ownership. Accept &str instead
      of String, &[T] instead of Vec<T> when only reading. This maximizes API flexibility.
      Bad: fn process(s: String) { println!("{}", s); }
      Good: fn process(s: &str) { println!("{}", s); }
    severity: medium

  - id: rust-impl-trait-return
    name: Use impl Trait for return types when appropriate
    description: |
      Return `impl Iterator<Item = T>` instead of Box<dyn Iterator> when the concrete type
      doesn't need to be named. This enables static dispatch and better optimization.
    severity: low

  # ============================================================================
  # CONCURRENCY
  # ============================================================================

  - id: rust-send-sync-bounds
    name: Understand Send and Sync trait bounds
    description: |
      When writing generic concurrent code, understand why Send and Sync bounds are needed.
      Send means safe to transfer to another thread, Sync means safe to share references.
      Missing bounds cause confusing errors; excessive bounds limit API usability.
    severity: medium

  - id: rust-mutex-vs-rwlock
    name: Choose Mutex vs RwLock based on access patterns
    description: |
      Use RwLock when reads significantly outnumber writes. Use Mutex for write-heavy
      workloads or when the critical section is short. RwLock has more overhead than Mutex.
      Bad: Write-heavy access using RwLock.
      Good: Use Mutex for write-heavy, RwLock for read-heavy access patterns.
    severity: medium

  - id: rust-async-runtime
    name: Be explicit about async runtime requirements
    description: |
      Document which async runtime (tokio, async-std, smol) your code requires.
      Avoid mixing runtimes. Use runtime-agnostic abstractions where possible.
    severity: medium

  - id: rust-spawn-blocking
    name: Use spawn_blocking for CPU-intensive work in async code
    description: |
      Don't block the async runtime with CPU-intensive operations. Use spawn_blocking
      or a dedicated thread pool for heavy computation.
      Bad: async fn process() { let result = expensive_computation(); }
      Good: Use tokio::task::spawn_blocking(|| expensive_computation()).await
    severity: high

  # ============================================================================
  # COMMON PITFALLS
  # ============================================================================

  - id: rust-deadlock-lock-ordering
    name: Maintain consistent lock ordering to prevent deadlocks
    description: |
      When acquiring multiple locks, always acquire them in a consistent global order.
      Document the expected lock ordering. Consider using parking_lot's deadlock detection
      in development builds. Both threads should always lock A before B.
    severity: high

  - id: rust-panic-in-drop
    name: Never panic in Drop implementations
    description: |
      Panicking in Drop can cause double-panic and abort. Handle errors gracefully in Drop,
      log them, or use a pattern like scopeguard that handles cleanup failures.
      Bad: self.cleanup().unwrap() in drop.
      Good: if let Err(e) = self.cleanup() { eprintln!("Cleanup failed: {}", e); }
    severity: high

  - id: rust-unwrap-expect-usage
    name: Avoid unwrap/expect in library code; use in tests and provably safe cases
    description: |
      Library code should propagate errors with ? or return Result/Option. Use unwrap/expect
      only when you can prove the value exists, and document why in the expect message.
      Bad: serde_json::from_str(s).unwrap() in public API.
      Good: Return Result<Data, ParseError> or use expect with explanation for provably safe cases.
    severity: medium

  - id: rust-integer-overflow
    name: Handle integer overflow explicitly
    description: |
      In release builds, integer overflow wraps silently. Use checked_*, saturating_*,
      or wrapping_* methods to make overflow behavior explicit. Consider using the
      overflow-checks profile setting.
    severity: medium

  - id: rust-forget-mem-forget
    name: Understand implications of mem::forget
    description: |
      mem::forget prevents Drop from running but is safe. However, it can leak resources.
      Usually ManuallyDrop is clearer for intentional leak scenarios.
    severity: medium