feat(rules): add Rust language rules#660
Conversation
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdded five Rust-specific documentation files (coding style, hooks, patterns, security, testing), updated test utilities and two test files for environment handling and lint suppression, and adjusted AGENTS/README catalog counts for skills and commands. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
Pull request overview
Adds a new Rust-specific ruleset under rules/rust/, mirroring the established per-language structure (e.g., Java/Kotlin) and extending the shared rules/common/* guidance with Rust conventions and examples.
Changes:
- Introduces Rust coding style rules (rustfmt/clippy, ownership/borrowing, error handling, module/visibility guidance).
- Adds Rust testing rules (unit/integration organization, rstest/mockall/tokio testing patterns, coverage commands).
- Adds Rust patterns, hooks, and security rules (trait-based repos/services, hooks for fmt/clippy/check, secrets/SQLi/input validation/unsafe/deps/error exposure).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| rules/rust/coding-style.md | Rust-specific coding standards and idioms extending common coding-style guidance. |
| rules/rust/testing.md | Rust testing organization/framework guidance extending common testing guidance. |
| rules/rust/patterns.md | Rust architectural/pattern recommendations extending common patterns guidance. |
| rules/rust/hooks.md | Rust-specific PostToolUse hook recommendations extending common hooks guidance. |
| rules/rust/security.md | Rust security practices extending common security guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn load_config(path: &str) -> anyhow::Result<Config> { | ||
| 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}")) | ||
| } |
There was a problem hiding this comment.
This snippet calls .with_context(...) but doesn’t bring the anyhow::Context trait into scope, so it won’t compile as-is. Add use anyhow::Context; (or fully qualify) in the example to make it runnable.
rules/rust/patterns.md
Outdated
| pub fn place_order(&self, request: CreateOrderRequest) -> Result<OrderSummary> { | ||
| let order = Order::from(request); | ||
| self.payment.charge(order.total())?; | ||
| let saved = self.repo.save(&order)?; | ||
| Ok(OrderSummary::from(saved)) | ||
| } |
There was a problem hiding this comment.
place_order is declared to return Result<OrderSummary> but Rust’s standard Result requires two type parameters. Consider using anyhow::Result<OrderSummary> for an application-style example, or Result<OrderSummary, ServiceError> for a library/service-style example.
rules/rust/patterns.md
Outdated
| struct UserId(u64); | ||
| struct OrderId(u64); | ||
|
|
||
| fn get_order(user: UserId, order: OrderId) -> Result<Order> { |
There was a problem hiding this comment.
get_order returns Result<Order> (missing the error type parameter), which won’t compile with the standard Result. Use anyhow::Result<Order> or an explicit Result<Order, E> to keep the example valid.
| fn get_order(user: UserId, order: OrderId) -> Result<Order> { | |
| fn get_order(user: UserId, order: OrderId) -> anyhow::Result<Order> { |
rules/rust/security.md
Outdated
| fn load_api_key() -> Result<String> { | ||
| std::env::var("PAYMENT_API_KEY") | ||
| .context("PAYMENT_API_KEY must be set") | ||
| } |
There was a problem hiding this comment.
load_api_key returns Result<String> (missing the error type parameter) and uses .context(...) which requires anyhow::Context in scope. Consider changing the signature to anyhow::Result<String> and adding/importing the Context trait in the example so it compiles.
rules/rust/testing.md
Outdated
| pub UserRepository {} | ||
| impl UserRepository for UserRepository { |
There was a problem hiding this comment.
In this mockall::mock! example, the macro defines a mock type named UserRepository, which conflicts with the already-defined pub trait UserRepository in the same scope. Use a different mock type name (or qualify the trait path in the impl ... for ... clause) so the example compiles and avoids the name collision.
| pub UserRepository {} | |
| impl UserRepository for UserRepository { | |
| pub MockUserRepository {} | |
| impl UserRepository for MockUserRepository { |
rules/rust/testing.md
Outdated
| - **rstest** for parameterized tests and fixtures | ||
| - **proptest** for property-based testing | ||
| - **mockall** for trait-based mocking | ||
| - **tokio::test** for async tests |
There was a problem hiding this comment.
In the framework list, tokio::test is presented as if it were a path/function, but the actual test macro is used as an attribute (#[tokio::test]). Consider updating the bullet to show the attribute form for consistency with the example below.
| - **tokio::test** for async tests | |
| - **`#[tokio::test]`** for async tests |
| fn normalize(input: &str) -> Cow<'_, str> { | ||
| if input.contains(' ') { | ||
| Cow::Owned(input.replace(' ', "_")) | ||
| } else { | ||
| Cow::Borrowed(input) | ||
| } | ||
| } |
There was a problem hiding this comment.
The example uses Cow<'_, str> and Cow::... without importing Cow (it’s not in the Rust prelude). Use std::borrow::Cow in the signature/variants or add an explicit use std::borrow::Cow; so the snippet compiles as written.
Greptile SummaryThis PR adds five Rust language rule files ( Key findings:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Developer edits *.rs file] --> B{PostToolUse Hook Fires}
B --> C[cargo fmt\nauto-format]
B --> D[cargo clippy\n-D warnings lint]
B --> E[cargo check\ncompilation verify]
C --> F{Code Passes?}
D --> F
E --> F
F -- Yes --> G[Commit Ready]
F -- No --> A
G --> H{Rule Files Activated\nvia paths frontmatter}
H --> H1[coding-style.md\nrustfmt · clippy · ownership]
H --> H2[testing.md\nunit · rstest · mockall · async]
H --> H3[patterns.md\nrepo · newtype · builder · FSM]
H --> H4[security.md\nsecrets · SQL · unsafe · audit]
H --> H5[hooks.md\nPostToolUse config]
H1 & H2 & H3 & H4 & H5 --> I[Deep Reference Skills]
I --> I1[rust-patterns skill]
I --> I2[rust-testing skill]
Last reviewed commit: "fix(tests): fix ESLi..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="rules/rust/testing.md">
<violation number="1" location="rules/rust/testing.md:146">
P2: Rust testing docs use an integration test target name that is inconsistent with the file layout example, making the command likely invalid when copied.</violation>
</file>
<file name="rules/rust/security.md">
<violation number="1" location="rules/rust/security.md:39">
P2: The sqlx "GOOD" example hardcodes `?` placeholder, which is invalid for Postgres despite claiming multi-backend guidance.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rules/rust/coding-style.md (1)
24-33: Make snippets copy-paste compilable by adding required imports.The examples use
Cowand.with_context(...)without showing required imports, so they fail when copied verbatim. Adduse std::borrow::Cow;anduse anyhow::Context;(or fully qualify in snippet).Suggested doc diff
```rust +use std::borrow::Cow; + // GOOD — immutable by default, new value returned fn normalize(input: &str) -> Cow<'_, str> { @@ // GOOD — application error with anyhow +use anyhow::Context; + fn load_config(path: &str) -> anyhow::Result<Config> { let content = std::fs::read_to_string(path) .with_context(|| format!("failed to read {path}"))?;Also applies to: 91-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rules/rust/coding-style.md` around lines 24 - 33, The snippets reference Cow and anyhow's Context without imports causing non-compilable examples; add the missing imports (e.g., add use std::borrow::Cow; for the normalize/Cow example and add use anyhow::Context; for load_config/.with_context) or fully qualify those types/traits in the snippets so functions like normalize and load_config and calls to .with_context compile when copy-pasted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rules/rust/security.md`:
- Around line 37-42: The "GOOD" example uses a backend-specific placeholder
(`?`) that breaks for Postgres; update the docs to either show explicit
per-backend examples using sqlx::query with Postgres placeholders (`$1`, `$2`),
MySQL (`?`), and SQLite (`?1`/`?`) or present a backend-agnostic pattern (e.g.,
show usage of sqlx::query with .bind and note the correct placeholder for each
supported backend). Reference the example's symbol sqlx::query and the
.bind(&name) call so readers can find and replace the placeholder token
appropriate for their DB.
---
Nitpick comments:
In `@rules/rust/coding-style.md`:
- Around line 24-33: The snippets reference Cow and anyhow's Context without
imports causing non-compilable examples; add the missing imports (e.g., add use
std::borrow::Cow; for the normalize/Cow example and add use anyhow::Context; for
load_config/.with_context) or fully qualify those types/traits in the snippets
so functions like normalize and load_config and calls to .with_context compile
when copy-pasted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bb6147c1-0bd9-431f-9d0d-177e06c06f2c
📒 Files selected for processing (5)
rules/rust/coding-style.mdrules/rust/hooks.mdrules/rust/patterns.mdrules/rust/security.mdrules/rust/testing.md
|
Analysis Failed
Troubleshooting
Retry: |
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="rules/rust/hooks.md">
<violation number="1" location="rules/rust/hooks.md:5">
P2: Removing `**/Cargo.lock` from Rust hook paths drops rule activation for lockfile-only dependency changes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Review feedback addressedThanks to all reviewers! All issues from the initial round have been fixed in commit cd4c557: Fixed (from Copilot, Greptile, Cubic, CodeRabbit):
Regarding the new Cubic comment on Cargo.lock removal: This was intentionally removed per Greptile's recommendation. The hooks file defines PostToolUse hooks for |
|
Thanks for addressing the review comments. CI tests are still failing across all platforms though — likely the component validation or catalog counts need updating to reflect the new rule files. Please check the failing test output and fix the validation errors. Once CI is green, this is ready to merge. |
chris-yyau
force-pushed
the
feat/rust-rules
branch
from
cd4c557 to
433985a
Compare
|
Analysis Failed
Troubleshooting
Retry: |
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>
chris-yyau
force-pushed
the
feat/rust-rules
branch
from
433985a to
44fdf79
Compare
|
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
|
- selective-install.test.js: suppress no-unused-vars for loadInstallManifests (imported for future use), remove unused result variable - cost-tracker.test.js: mirror HOME to USERPROFILE in env overrides so os.homedir() resolves to temp dir on Windows 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>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/hooks/cost-tracker.test.js (1)
54-65: Consider inlining the ternary for readability.The separated multiline ternary after the test call is functional but slightly unusual. An inline pattern or a simpler conditional block might be clearer.
♻️ Optional: Simplified pattern using if statement
- test('passes through input on stdout', () => { + if (test('passes through input on stdout', () => { const input = { model: 'claude-sonnet-4-20250514', usage: { input_tokens: 100, output_tokens: 50 } }; const inputStr = JSON.stringify(input); const result = runScript(input); assert.strictEqual(result.code, 0, `Expected exit code 0, got ${result.code}`); assert.strictEqual(result.stdout, inputStr, 'Expected stdout to match original input'); - }) - ? passed++ - : failed++; + })) { passed++; } else { failed++; }Alternatively, you could keep the current pattern if the team prefers it — it's functionally correct.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/hooks/cost-tracker.test.js` around lines 54 - 65, The multiline ternary after the test call is confusing; change the pattern to a simple conditional so it's clearer: after calling runScript (the variable result) and asserting, replace the separated " ? passed++ : failed++" with a straightforward if/else that increments passed or failed (or, if you prefer brevity, move the ternary inline on the same line) — locate the test block with the description 'passes through input on stdout', the variables inputStr and result from runScript, and update the increment logic to a single-line conditional or explicit if/else for readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/hooks/cost-tracker.test.js`:
- Around line 54-65: The multiline ternary after the test call is confusing;
change the pattern to a simple conditional so it's clearer: after calling
runScript (the variable result) and asserting, replace the separated " ?
passed++ : failed++" with a straightforward if/else that increments passed or
failed (or, if you prefer brevity, move the ternary inline on the same line) —
locate the test block with the description 'passes through input on stdout', the
variables inputStr and result from runScript, and update the increment logic to
a single-line conditional or explicit if/else for readability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7512f64b-7a71-481f-abd3-5cd65a2e05a7
📒 Files selected for processing (2)
tests/hooks/cost-tracker.test.jstests/lib/selective-install.test.js
✅ Files skipped from review due to trivial changes (1)
- tests/lib/selective-install.test.js
* 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 <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * 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 <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> * 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<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> * 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 <noreply@anthropic.com> Co-Authored-By: Happy <yesreply@happy.engineering> --------- Co-authored-by: Chris Yau <chris@diveanddev.com> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Happy <yesreply@happy.engineering>
Summary
rust-patternsandrust-testingskills for deep content, avoiding duplicationFiles added
rules/rust/coding-style.mdrules/rust/testing.mdmock!macro, tokio async tests, cargo-llvm-cov coveragerules/rust/patterns.mdrules/rust/hooks.mdrules/rust/security.mdDesign decisions
paths: ["**/*.rs"](plus**/Cargo.tomlfor hooks) matching the convention in other language rule directories.Test plan
mock!pattern, HTTP status codes)paths:frontmatter activates rules for.rsfiles🤖 Generated with Claude Code
Summary by cubic
Adds Rust language rules that extend our common standards for style, testing, patterns, hooks, and security. Also fixes test harness issues on Windows and an ESLint warning.
New Features
rules/rust/coding-style.md:rustfmt,clippy -D warnings, ownership/borrowing, errors (thiserror/anyhow), iterators, modules/visibility.rules/rust/testing.md: unit/integration layout,rstest,proptest,mockall, async via#[tokio::test], coverage withcargo-llvm-cov.rules/rust/patterns.md: trait repos, service layer, newtypes, enum state machines, builders, sealed traits, API response envelope.rules/rust/hooks.md:PostToolUseforcargo fmt,cargo clippy,cargo check.rules/rust/security.md: env-based secrets, parameterized SQL withsqlx(Postgres$1), parse-don’t-validate,unsafeaudit,cargo-audit/cargo-deny.Bug Fixes
anyhow::Resultwithanyhow::Context; corrected#[tokio::test]; wrappedmockallin#[cfg(test)]; switchedsqlxplaceholders to$1; fixedcargo test --test api_test; removedCargo.lockfrom hookspaths.HOMEtoUSERPROFILEincost-tracker.test.jsfor Windows; suppress ESLintno-unused-varsinselective-install.test.js.README.mdandAGENTS.md(skills 113, commands 58).Written for commit 2283e81. Summary will update on new commits.
Summary by CodeRabbit