feat(rules): add Rust language rules (rebased #660)#686
Conversation
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 <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
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 <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
Fixes from Copilot, Greptile, Cubic, and CodeRabbit reviews: - Add missing imports: use std::borrow::Cow, use anyhow::Context - Use anyhow::Result<T> 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 <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
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 <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR adds five new Rust language-specific rule files—coding style, testing, patterns, hooks, and security—that extend the common rule set, and updates catalog counts in documentation from 109→113 skills and 57→58 commands. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
affaan-m
force-pushed
the
community/pr-660-rust-rules
branch
from
ca5d166 to
db2ff64
Compare
|
1 similar comment
|
Analysis CompleteGenerated ECC bundle from 500 commits | Confidence: 100% View Pull Request #685Repository Profile
Detected Workflows (9)
Generated Instincts (17)
After merging, import with: Files
|
Analysis CompleteGenerated ECC bundle from 500 commits | Confidence: 100% View Pull Request #685Repository Profile
Detected Workflows (9)
Generated Instincts (17)
After merging, import with: Files
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR rebases community PR #660 onto the latest The new rule files are well-structured and technically sound — they follow the same conventions as the existing Go, Python, and TypeScript rule directories (YAML frontmatter with glob paths, extends-comment referencing the common counterpart, code examples, and skill references). Key observations:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User installs ECC plugin] --> B{Install Rust rules}
B --> C["Option 1: ./install.sh rust\n(⚠️ not yet registered in\ninstall-manifests.js)"]
B --> D["Option 2: Manual copy\ncp -r rules/rust ~/.claude/rules/rust"]
D --> E[Claude Code loads rule files]
E --> F{Rule file applies when\nmatching path glob}
F --> G["**/*.rs → coding-style.md\npatterns.md, security.md\ntesting.md, hooks.md"]
F --> H["**/Cargo.toml → hooks.md"]
G --> I[Claude follows Rust-specific\nguidelines during code generation]
H --> I
I --> J["cargo fmt / clippy / check\nhints via hooks.md"]
I --> K["thiserror/anyhow error patterns\nfrom coding-style.md"]
I --> L["Repository/Builder/Newtype\npatterns from patterns.md"]
I --> M["SQL injection, unsafe audits\nfrom security.md"]
I --> N["rstest / mockall / tokio::test\nfrom testing.md"]
Last reviewed commit: "fix: update catalog ..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } | ||
| ``` | ||
|
|
||
| ## 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 |
There was a problem hiding this comment.
mockall::mock! macro name conflict
The mock! block names the struct Repo, which generates MockRepo. However, the trait UserRepository is defined in the outer use super::* scope. mockall::mock! regenerates the trait's impl inside the macro — it doesn't auto-detect the outer trait. If UserRepository is an external/imported trait, this is fine. But the example could cause confusion because pub Repo {} body is empty (no inherent methods) yet the impl block expects the full method list.
More importantly, the mockall::mock! syntax is correct for mockall ≥ 0.11, but older versions require automock or a slightly different macro form. Consider adding a version note (e.g., # Cargo.toml: mockall = "0.13") to the code block header or a comment, so readers know which API revision is being demonstrated.
| } | |
| ``` | |
| ## 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 | |
| mockall::mock! { | |
| // Requires mockall ≥ 0.11 — add to Cargo.toml: mockall = "0.13" | |
| pub Repo {} | |
| impl UserRepository for Repo { | |
| fn find_by_id(&self, id: u64) -> Option<User>; | |
| } | |
| } |
| ```rust | ||
| pub trait OrderRepository: Send + Sync { | ||
| fn find_by_id(&self, id: u64) -> Result<Option<Order>, StorageError>; | ||
| fn find_all(&self) -> Result<Vec<Order>, StorageError>; | ||
| fn save(&self, order: &Order) -> Result<Order, StorageError>; | ||
| fn delete(&self, id: u64) -> Result<(), StorageError>; | ||
| } | ||
| ``` |
There was a problem hiding this comment.
The OrderRepository trait exposes only synchronous methods. In practice, nearly all Rust database crates (sqlx, sea-orm, diesel async) expose async fn interfaces. A developer following this pattern verbatim will hit a wall when they try to call .await — and working around sync traits with blocking executors in async contexts can introduce subtle deadlocks.
Consider adding either an async variant or an explicit note that this pattern is for synchronous storage only, and pointing readers to an async version (e.g., using async_trait or Rust 1.75+ async fn in trait):
// For async storage backends (sqlx, sea-orm, etc.) use async fn:
use async_trait::async_trait; // or native async fn in trait (Rust ≥ 1.75)
#[async_trait]
pub trait OrderRepository: Send + Sync {
async fn find_by_id(&self, id: u64) -> Result<Option<Order>, StorageError>;
async fn find_all(&self) -> Result<Vec<Order>, StorageError>;
async fn save(&self, order: &Order) -> Result<Order, StorageError>;
async fn delete(&self, id: u64) -> Result<(), StorageError>;
}| 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 |
There was a problem hiding this comment.
The Email::parse example uses hand-rolled length and @-position checks, yet immediately notes "For production use, prefer a validated email crate." The manual logic can produce false positives and false negatives (e.g., a@@b.c passes the @-position filter; quoted local-parts, IP-literal hosts, and IDNs are not handled).
Since the document's own recommendation is to use a crate, consider leading with the crate-based approach and relegating the manual example to a "what the crate is doing under the hood" aside, so the primary pattern shown is the safe one:
// GOOD — use a well-tested crate at the boundary
use email_address::EmailAddress;
pub struct Email(String);
impl Email {
pub fn parse(input: &str) -> Result<Self, ValidationError> {
let trimmed = input.trim();
if !EmailAddress::is_valid(trimmed) {
return Err(ValidationError::InvalidEmail(trimmed.to_string()));
}
Ok(Self(trimmed.to_string()))
}
}![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db2ff64978
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| --- | ||
| paths: | ||
| - "**/*.rs" | ||
| --- | ||
| # Rust Coding Style |
There was a problem hiding this comment.
These files add a Rust-specific ruleset, but the install surfaces still exclude it. I checked rules/README.md:17-21,33-60, which still documents only TypeScript/Python/Go/Swift/PHP copies, and install.sh --help from scripts/install-apply.js:19-45, which still lists only go, golang, java, javascript, kotlin, perl, php, python, swift, typescript; scripts/lib/install-manifests.js:39-58 has no rust legacy mapping either. Because rules are installed separately from the plugin, a user following the documented Step 2 flow cannot discover or explicitly request these new Rust rules.
Useful? React with 👍 / 👎.
2 participants
Summary
Original PR
Closes #660
Test plan
Summary by cubic
Adds Rust language rules extending the common set across coding style, hooks, patterns, testing, and security. Updates catalog counts in
AGENTS.mdandREADME.md(27 agents, 113 skills, 58 commands) and incorporates review fixes to examples, SQL placeholders, and hook paths.New Features
cargo fmt,clippy, ownership/borrowing, error handling (anyhow/thiserror), iterators, module layout.cargo fmt,cargo clippy,cargo check.#[test],#[tokio::test],rstest,proptest,mockall, coverage viacargo-llvm-cov.sqlx,unsafeaudit, dependency checks (cargo audit,cargo deny), safe error responses.Bug Fixes
anyhow::Resultconsistently, correct#[tokio::test].sqlxplaceholders to Postgres$1; excludeCargo.lockfrom hook paths.mockallname collision andcargo test --test api_testcommand.Written for commit db2ff64. Summary will update on new commits.
Summary by CodeRabbit