From 9a478ad6760577cc62cceb303c57c6349283d407 Mon Sep 17 00:00:00 2001 From: Affaan Mustafa Date: Fri, 20 Mar 2026 01:19:42 -0700 Subject: [PATCH] feat(rules): add Rust language rules (rebased #660) (#686) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(rules): add Rust coding style, hooks, and patterns rules Add language-specific rules for Rust extending the common rule set: - coding-style.md: rustfmt, clippy, ownership idioms, error handling, iterator patterns, module organization, visibility - hooks.md: PostToolUse hooks for rustfmt, clippy, cargo check - patterns.md: trait-based repository, newtype, enum state machines, builder, sealed traits, API response envelope Rules reference existing rust-patterns skill for deep content. Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Co-Authored-By: Happy * feat(rules): add Rust testing and security rules Add remaining Rust language-specific rules: - testing.md: cargo test, rstest parameterized tests, mockall mocking with mock! macro, tokio async tests, cargo-llvm-cov coverage - security.md: secrets via env vars, parameterized SQL with sqlx, parse-don't-validate input validation, unsafe code audit requirements, cargo-audit dependency scanning, proper HTTP error status codes Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Co-Authored-By: Happy * fix(rules): address review feedback on Rust rules Fixes from Copilot, Greptile, Cubic, and CodeRabbit reviews: - Add missing imports: use std::borrow::Cow, use anyhow::Context - Use anyhow::Result consistently (patterns.md, security.md) - Change sqlx placeholder from ? to $1 (Postgres is most common) - Remove Cargo.lock from hooks.md paths (auto-generated file) - Fix tokio::test to show attribute form #[tokio::test] - Fix mockall mock! name collision, wrap in #[cfg(test)] mod tests - Fix --test target to match file layout (api_test, not integration) Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Co-Authored-By: Happy * fix: update catalog counts in README.md and AGENTS.md Update documented counts to match actual repository state after rebase: - Skills: 109 → 113 (new skills merged to main) - Commands: 57 → 58 (new command merged to main) Generated with [Claude Code](https://claude.ai/code) via [Happy](https://happy.engineering) Co-Authored-By: Claude Co-Authored-By: Happy --------- Co-authored-by: Chris Yau Co-authored-by: Claude Co-authored-by: Happy --- rules/rust/coding-style.md | 151 +++++++++++++++++++++++++++++++++ rules/rust/hooks.md | 16 ++++ rules/rust/patterns.md | 168 +++++++++++++++++++++++++++++++++++++ rules/rust/security.md | 141 +++++++++++++++++++++++++++++++ rules/rust/testing.md | 154 ++++++++++++++++++++++++++++++++++ 5 files changed, 630 insertions(+) create mode 100644 rules/rust/coding-style.md create mode 100644 rules/rust/hooks.md create mode 100644 rules/rust/patterns.md create mode 100644 rules/rust/security.md create mode 100644 rules/rust/testing.md diff --git a/rules/rust/coding-style.md b/rules/rust/coding-style.md new file mode 100644 index 00000000..cda67b54 --- /dev/null +++ b/rules/rust/coding-style.md @@ -0,0 +1,151 @@ +--- +paths: + - "**/*.rs" +--- +# Rust Coding Style + +> This file extends [common/coding-style.md](../common/coding-style.md) with Rust-specific content. + +## Formatting + +- **rustfmt** for enforcement — always run `cargo fmt` before committing +- **clippy** for lints — `cargo clippy -- -D warnings` (treat warnings as errors) +- 4-space indent (rustfmt default) +- Max line width: 100 characters (rustfmt default) + +## Immutability + +Rust variables are immutable by default — embrace this: + +- Use `let` by default; only use `let mut` when mutation is required +- Prefer returning new values over mutating in place +- Use `Cow<'_, T>` when a function may or may not need to allocate + +```rust +use std::borrow::Cow; + +// GOOD — immutable by default, new value returned +fn normalize(input: &str) -> Cow<'_, str> { + if input.contains(' ') { + Cow::Owned(input.replace(' ', "_")) + } else { + Cow::Borrowed(input) + } +} + +// BAD — unnecessary mutation +fn normalize_bad(input: &mut String) { + *input = input.replace(' ', "_"); +} +``` + +## Naming + +Follow standard Rust conventions: +- `snake_case` for functions, methods, variables, modules, crates +- `PascalCase` (UpperCamelCase) for types, traits, enums, type parameters +- `SCREAMING_SNAKE_CASE` for constants and statics +- Lifetimes: short lowercase (`'a`, `'de`) — descriptive names for complex cases (`'input`) + +## Ownership and Borrowing + +- Borrow (`&T`) by default; take ownership only when you need to store or consume +- Never clone to satisfy the borrow checker without understanding the root cause +- Accept `&str` over `String`, `&[T]` over `Vec` in function parameters +- Use `impl Into` for constructors that need to own a `String` + +```rust +// GOOD — borrows when ownership isn't needed +fn word_count(text: &str) -> usize { + text.split_whitespace().count() +} + +// GOOD — takes ownership in constructor via Into +fn new(name: impl Into) -> Self { + Self { name: name.into() } +} + +// BAD — takes String when &str suffices +fn word_count_bad(text: String) -> usize { + text.split_whitespace().count() +} +``` + +## Error Handling + +- Use `Result` and `?` for propagation — never `unwrap()` in production code +- **Libraries**: define typed errors with `thiserror` +- **Applications**: use `anyhow` for flexible error context +- Add context with `.with_context(|| format!("failed to ..."))?` +- Reserve `unwrap()` / `expect()` for tests and truly unreachable states + +```rust +// GOOD — library error with thiserror +#[derive(Debug, thiserror::Error)] +pub enum ConfigError { + #[error("failed to read config: {0}")] + Io(#[from] std::io::Error), + #[error("invalid config format: {0}")] + Parse(String), +} + +// GOOD — application error with anyhow +use anyhow::Context; + +fn load_config(path: &str) -> anyhow::Result { + let content = std::fs::read_to_string(path) + .with_context(|| format!("failed to read {path}"))?; + toml::from_str(&content) + .with_context(|| format!("failed to parse {path}")) +} +``` + +## Iterators Over Loops + +Prefer iterator chains for transformations; use loops for complex control flow: + +```rust +// GOOD — declarative and composable +let active_emails: Vec<&str> = users.iter() + .filter(|u| u.is_active) + .map(|u| u.email.as_str()) + .collect(); + +// GOOD — loop for complex logic with early returns +for user in &users { + if let Some(verified) = verify_email(&user.email)? { + send_welcome(&verified)?; + } +} +``` + +## Module Organization + +Organize by domain, not by type: + +```text +src/ +├── main.rs +├── lib.rs +├── auth/ # Domain module +│ ├── mod.rs +│ ├── token.rs +│ └── middleware.rs +├── orders/ # Domain module +│ ├── mod.rs +│ ├── model.rs +│ └── service.rs +└── db/ # Infrastructure + ├── mod.rs + └── pool.rs +``` + +## Visibility + +- Default to private; use `pub(crate)` for internal sharing +- Only mark `pub` what is part of the crate's public API +- Re-export public API from `lib.rs` + +## References + +See skill: `rust-patterns` for comprehensive Rust idioms and patterns. diff --git a/rules/rust/hooks.md b/rules/rust/hooks.md new file mode 100644 index 00000000..4511f1c2 --- /dev/null +++ b/rules/rust/hooks.md @@ -0,0 +1,16 @@ +--- +paths: + - "**/*.rs" + - "**/Cargo.toml" +--- +# Rust Hooks + +> This file extends [common/hooks.md](../common/hooks.md) with Rust-specific content. + +## PostToolUse Hooks + +Configure in `~/.claude/settings.json`: + +- **cargo fmt**: Auto-format `.rs` files after edit +- **cargo clippy**: Run lint checks after editing Rust files +- **cargo check**: Verify compilation after changes (faster than `cargo build`) diff --git a/rules/rust/patterns.md b/rules/rust/patterns.md new file mode 100644 index 00000000..3d807e7d --- /dev/null +++ b/rules/rust/patterns.md @@ -0,0 +1,168 @@ +--- +paths: + - "**/*.rs" +--- +# Rust Patterns + +> This file extends [common/patterns.md](../common/patterns.md) with Rust-specific content. + +## Repository Pattern with Traits + +Encapsulate data access behind a trait: + +```rust +pub trait OrderRepository: Send + Sync { + fn find_by_id(&self, id: u64) -> Result, StorageError>; + fn find_all(&self) -> Result, StorageError>; + fn save(&self, order: &Order) -> Result; + fn delete(&self, id: u64) -> Result<(), StorageError>; +} +``` + +Concrete implementations handle storage details (Postgres, SQLite, in-memory for tests). + +## Service Layer + +Business logic in service structs; inject dependencies via constructor: + +```rust +pub struct OrderService { + repo: Box, + payment: Box, +} + +impl OrderService { + pub fn new(repo: Box, payment: Box) -> Self { + Self { repo, payment } + } + + pub fn place_order(&self, request: CreateOrderRequest) -> anyhow::Result { + let order = Order::from(request); + self.payment.charge(order.total())?; + let saved = self.repo.save(&order)?; + Ok(OrderSummary::from(saved)) + } +} +``` + +## Newtype Pattern for Type Safety + +Prevent argument mix-ups with distinct wrapper types: + +```rust +struct UserId(u64); +struct OrderId(u64); + +fn get_order(user: UserId, order: OrderId) -> anyhow::Result { + // Can't accidentally swap user and order IDs at call sites + todo!() +} +``` + +## Enum State Machines + +Model states as enums — make illegal states unrepresentable: + +```rust +enum ConnectionState { + Disconnected, + Connecting { attempt: u32 }, + Connected { session_id: String }, + Failed { reason: String, retries: u32 }, +} + +fn handle(state: &ConnectionState) { + match state { + ConnectionState::Disconnected => connect(), + ConnectionState::Connecting { attempt } if *attempt > 3 => abort(), + ConnectionState::Connecting { .. } => wait(), + ConnectionState::Connected { session_id } => use_session(session_id), + ConnectionState::Failed { retries, .. } if *retries < 5 => retry(), + ConnectionState::Failed { reason, .. } => log_failure(reason), + } +} +``` + +Always match exhaustively — no wildcard `_` for business-critical enums. + +## Builder Pattern + +Use for structs with many optional parameters: + +```rust +pub struct ServerConfig { + host: String, + port: u16, + max_connections: usize, +} + +impl ServerConfig { + pub fn builder(host: impl Into, port: u16) -> ServerConfigBuilder { + ServerConfigBuilder { + host: host.into(), + port, + max_connections: 100, + } + } +} + +pub struct ServerConfigBuilder { + host: String, + port: u16, + max_connections: usize, +} + +impl ServerConfigBuilder { + pub fn max_connections(mut self, n: usize) -> Self { + self.max_connections = n; + self + } + + pub fn build(self) -> ServerConfig { + ServerConfig { + host: self.host, + port: self.port, + max_connections: self.max_connections, + } + } +} +``` + +## Sealed Traits for Extensibility Control + +Use a private module to seal a trait, preventing external implementations: + +```rust +mod private { + pub trait Sealed {} +} + +pub trait Format: private::Sealed { + fn encode(&self, data: &[u8]) -> Vec; +} + +pub struct Json; +impl private::Sealed for Json {} +impl Format for Json { + fn encode(&self, data: &[u8]) -> Vec { todo!() } +} +``` + +## API Response Envelope + +Consistent API responses using a generic enum: + +```rust +#[derive(Debug, serde::Serialize)] +#[serde(tag = "status")] +pub enum ApiResponse { + #[serde(rename = "ok")] + Ok { data: T }, + #[serde(rename = "error")] + Error { message: String }, +} +``` + +## References + +See skill: `rust-patterns` for comprehensive patterns including ownership, traits, generics, concurrency, and async. diff --git a/rules/rust/security.md b/rules/rust/security.md new file mode 100644 index 00000000..83c0e077 --- /dev/null +++ b/rules/rust/security.md @@ -0,0 +1,141 @@ +--- +paths: + - "**/*.rs" +--- +# Rust Security + +> This file extends [common/security.md](../common/security.md) with Rust-specific content. + +## Secrets Management + +- Never hardcode API keys, tokens, or credentials in source code +- Use environment variables: `std::env::var("API_KEY")` +- Fail fast if required secrets are missing at startup +- Keep `.env` files in `.gitignore` + +```rust +// BAD +const API_KEY: &str = "sk-abc123..."; + +// GOOD — environment variable with early validation +fn load_api_key() -> anyhow::Result { + std::env::var("PAYMENT_API_KEY") + .context("PAYMENT_API_KEY must be set") +} +``` + +## SQL Injection Prevention + +- Always use parameterized queries — never format user input into SQL strings +- Use query builder or ORM (sqlx, diesel, sea-orm) with bind parameters + +```rust +// BAD — SQL injection via format string +let query = format!("SELECT * FROM users WHERE name = '{name}'"); +sqlx::query(&query).fetch_one(&pool).await?; + +// GOOD — parameterized query with sqlx +// Placeholder syntax varies by backend: Postgres: $1 | MySQL: ? | SQLite: $1 +sqlx::query("SELECT * FROM users WHERE name = $1") + .bind(&name) + .fetch_one(&pool) + .await?; +``` + +## Input Validation + +- Validate all user input at system boundaries before processing +- Use the type system to enforce invariants (newtype pattern) +- Parse, don't validate — convert unstructured data to typed structs at the boundary +- Reject invalid input with clear error messages + +```rust +// Parse, don't validate — invalid states are unrepresentable +pub struct Email(String); + +impl Email { + pub fn parse(input: &str) -> Result { + let trimmed = input.trim(); + let at_pos = trimmed.find('@') + .filter(|&p| p > 0 && p < trimmed.len() - 1) + .ok_or_else(|| ValidationError::InvalidEmail(input.to_string()))?; + let domain = &trimmed[at_pos + 1..]; + if trimmed.len() > 254 || !domain.contains('.') { + return Err(ValidationError::InvalidEmail(input.to_string())); + } + // For production use, prefer a validated email crate (e.g., `email_address`) + Ok(Self(trimmed.to_string())) + } + + pub fn as_str(&self) -> &str { + &self.0 + } +} +``` + +## Unsafe Code + +- Minimize `unsafe` blocks — prefer safe abstractions +- Every `unsafe` block must have a `// SAFETY:` comment explaining the invariant +- Never use `unsafe` to bypass the borrow checker for convenience +- Audit all `unsafe` code during review — it is a red flag without justification +- Prefer `safe` FFI wrappers around C libraries + +```rust +// GOOD — safety comment documents ALL required invariants +let widget: &Widget = { + // SAFETY: `ptr` is non-null, aligned, points to an initialized Widget, + // and no mutable references or mutations exist for its lifetime. + unsafe { &*ptr } +}; + +// BAD — no safety justification +unsafe { &*ptr } +``` + +## Dependency Security + +- Run `cargo audit` to scan for known CVEs in dependencies +- Run `cargo deny check` for license and advisory compliance +- Use `cargo tree` to audit transitive dependencies +- Keep dependencies updated — set up Dependabot or Renovate +- Minimize dependency count — evaluate before adding new crates + +```bash +# Security audit +cargo audit + +# Deny advisories, duplicate versions, and restricted licenses +cargo deny check + +# Inspect dependency tree +cargo tree +cargo tree -d # Show duplicates only +``` + +## Error Messages + +- Never expose internal paths, stack traces, or database errors in API responses +- Log detailed errors server-side; return generic messages to clients +- Use `tracing` or `log` for structured server-side logging + +```rust +// Map errors to appropriate status codes and generic messages +// (Example uses axum; adapt the response type to your framework) +match order_service.find_by_id(id) { + Ok(order) => Ok((StatusCode::OK, Json(order))), + Err(ServiceError::NotFound(_)) => { + tracing::info!(order_id = id, "order not found"); + Err((StatusCode::NOT_FOUND, "Resource not found")) + } + Err(e) => { + tracing::error!(order_id = id, error = %e, "unexpected error"); + Err((StatusCode::INTERNAL_SERVER_ERROR, "Internal server error")) + } +} +``` + +## References + +See skill: `rust-patterns` for unsafe code guidelines and ownership patterns. +See skill: `security-review` for general security checklists. diff --git a/rules/rust/testing.md b/rules/rust/testing.md new file mode 100644 index 00000000..dae4b675 --- /dev/null +++ b/rules/rust/testing.md @@ -0,0 +1,154 @@ +--- +paths: + - "**/*.rs" +--- +# Rust Testing + +> This file extends [common/testing.md](../common/testing.md) with Rust-specific content. + +## Test Framework + +- **`#[test]`** with `#[cfg(test)]` modules for unit tests +- **rstest** for parameterized tests and fixtures +- **proptest** for property-based testing +- **mockall** for trait-based mocking +- **`#[tokio::test]`** for async tests + +## Test Organization + +```text +my_crate/ +├── src/ +│ ├── lib.rs # Unit tests in #[cfg(test)] modules +│ ├── auth/ +│ │ └── mod.rs # #[cfg(test)] mod tests { ... } +│ └── orders/ +│ └── service.rs # #[cfg(test)] mod tests { ... } +├── tests/ # Integration tests (each file = separate binary) +│ ├── api_test.rs +│ ├── db_test.rs +│ └── common/ # Shared test utilities +│ └── mod.rs +└── benches/ # Criterion benchmarks + └── benchmark.rs +``` + +Unit tests go inside `#[cfg(test)]` modules in the same file. Integration tests go in `tests/`. + +## Unit Test Pattern + +```rust +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn creates_user_with_valid_email() { + let user = User::new("Alice", "alice@example.com").unwrap(); + assert_eq!(user.name, "Alice"); + } + + #[test] + fn rejects_invalid_email() { + let result = User::new("Bob", "not-an-email"); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("invalid email")); + } +} +``` + +## Parameterized Tests + +```rust +use rstest::rstest; + +#[rstest] +#[case("hello", 5)] +#[case("", 0)] +#[case("rust", 4)] +fn test_string_length(#[case] input: &str, #[case] expected: usize) { + assert_eq!(input.len(), expected); +} +``` + +## Async Tests + +```rust +#[tokio::test] +async fn fetches_data_successfully() { + let client = TestClient::new().await; + let result = client.get("/data").await; + assert!(result.is_ok()); +} +``` + +## Mocking with mockall + +Define traits in production code; generate mocks in test modules: + +```rust +// Production trait — pub so integration tests can import it +pub trait UserRepository { + fn find_by_id(&self, id: u64) -> Option; +} + +#[cfg(test)] +mod tests { + use super::*; + use mockall::predicate::eq; + + mockall::mock! { + pub Repo {} + impl UserRepository for Repo { + fn find_by_id(&self, id: u64) -> Option; + } + } + + #[test] + fn service_returns_user_when_found() { + let mut mock = MockRepo::new(); + mock.expect_find_by_id() + .with(eq(42)) + .times(1) + .returning(|_| Some(User { id: 42, name: "Alice".into() })); + + let service = UserService::new(Box::new(mock)); + let user = service.get_user(42).unwrap(); + assert_eq!(user.name, "Alice"); + } +} +``` + +## Test Naming + +Use descriptive names that explain the scenario: +- `creates_user_with_valid_email()` +- `rejects_order_when_insufficient_stock()` +- `returns_none_when_not_found()` + +## Coverage + +- Target 80%+ line coverage +- Use **cargo-llvm-cov** for coverage reporting +- Focus on business logic — exclude generated code and FFI bindings + +```bash +cargo llvm-cov # Summary +cargo llvm-cov --html # HTML report +cargo llvm-cov --fail-under-lines 80 # Fail if below threshold +``` + +## Testing Commands + +```bash +cargo test # Run all tests +cargo test -- --nocapture # Show println output +cargo test test_name # Run tests matching pattern +cargo test --lib # Unit tests only +cargo test --test api_test # Specific integration test (tests/api_test.rs) +cargo test --doc # Doc tests only +``` + +## References + +See skill: `rust-testing` for comprehensive testing patterns including property-based testing, fixtures, and benchmarking with Criterion.