diff --git a/skills/rust-expert-best-practices-code-review/SKILL.md b/skills/rust-expert-best-practices-code-review/SKILL.md new file mode 100644 index 0000000..d3c1fc0 --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/SKILL.md @@ -0,0 +1,83 @@ +--- +name: rust-expert-best-practices-code-review +description: Rust best practices and code quality guidelines for writing idiomatic, safe, and performant Rust code. This skill should be used when writing, reviewing, or refactoring Rust code. Triggers on tasks involving Rust programming, code review, error handling, type safety, or performance optimization. +license: MIT +metadata: + author: wispbit + version: "1.0.0" +--- + +# Rust Expert Best Practices + +Simple, pragmatic, opinionated. Only what matters for writing production-grade Rust code. + +## When to Apply + +Reference these guidelines when: +- Writing Rust code (structs, functions, enums, traits) +- Implementing error handling and Result types +- Reviewing Rust code for safety or performance issues +- Refactoring existing Rust codebases +- Designing APIs and public interfaces +- Optimizing Rust code for performance or clarity + +## Rule Categories by Priority + +| Priority | Category | Impact | Prefix | +|----------|----------|--------|--------| +| 1 | Type Safety | CRITICAL | `use-typesafe-`, `use-enum-` | +| 2 | Error Handling | CRITICAL-HIGH | `result-`, `avoid-panic` | +| 3 | API Design | HIGH | `use-borrowed-`, `prefer-builder-` | +| 4 | Code Quality | MEDIUM-HIGH | `use-iterator-`, `prefer-format` | +| 5 | Readability | MEDIUM | `use-named-`, `avoid-boolean-` | +| 6 | Performance | MEDIUM | `avoid-rc`, `avoid-box` | + +## Quick Reference + +- `use-borrowed-argument-types` - Use &str, &[T], &Path instead of &String, &Vec, &PathBuf +- `use-enum-deserialization` - Use exhaustive enum matching for safe deserialization +- `use-typesafe-index-wrappers` - Wrap index types to prevent mixing different indices +- `result-error-returns` - Use ? operator instead of unwrap/expect in Result functions +- `avoid-panic` - Use assert!, Result, or expect based on context instead of panic! +- `use-iterator-transforms` - Use iterator methods instead of explicit push loops +- `use-copied` - Use .copied() to avoid complex dereferencing patterns +- `prefer-format` - Use format! over manual string concatenation +- `prefer-builder-pattern-for-complex` - Use builder pattern for functions with 4+ parameters +- `use-named-placeholders` - Use named placeholders instead of bare _ in destructuring +- `decimal-comparison` - Use .is_sign_negative() instead of comparing to Decimal::ZERO +- `calculated-field-as-method` - Implement calculated fields as methods not struct fields +- `avoid-rc` - Avoid unnecessary Rc when simpler ownership patterns work +- `avoid-box` - Don't use Box for concrete types without legitimate reason +- `avoid-boolean-params` - Replace boolean parameters with enums or structs +- `match-statements-handle-all-cases` - Explicitly handle all enum variants without catch-all patterns + +## How to Use + +Read individual rule files for detailed explanations and code examples: + +``` +rules/use-borrowed-argument-types.md +rules/use-enum-deserialization.md +rules/use-typesafe-index-wrappers.md +rules/result-error-returns.md +rules/avoid-panic.md +rules/use-iterator-transforms.md +rules/use-copied.md +rules/prefer-format.md +rules/prefer-builder-pattern-for-complex.md +rules/use-named-placeholders.md +rules/decimal-comparison.md +rules/calculated-field-as-method.md +rules/avoid-rc.md +rules/avoid-box.md +rules/avoid-boolean-params.md +rules/match-statements-handle-all-cases.md +``` + +Each rule file contains: +- Brief explanation of why it matters +- When to use and when not to use the pattern +- Implementation requirements +- BAD code examples with explanation +- GOOD code examples with explanation +- Additional context and best practices diff --git a/skills/rust-expert-best-practices-code-review/rules/avoid-boolean-params.md b/skills/rust-expert-best-practices-code-review/rules/avoid-boolean-params.md new file mode 100644 index 0000000..b6f90e2 --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/avoid-boolean-params.md @@ -0,0 +1,110 @@ +--- +title: Avoid Boolean Parameters in Function Signatures +impact: MEDIUM-HIGH +impactDescription: Improves code readability and prevents boolean confusion at call sites +tags: api-design, function-parameters, enums, readability, rust +--- + +## Avoid Boolean Parameters in Function Signatures + +**Impact: MEDIUM-HIGH (Improves code readability and prevents boolean confusion at call sites)** + +Avoid boolean parameters in function signatures. Boolean parameters make code hard to read at the call site and are error-prone. + +Replace boolean parameters with enums or parameter structs for better readability and type safety. + +### BAD Examples + +```rust +// src/data_processor.rs +// Multiple boolean parameters are unclear at call site +fn process_data(data: &[u8], compress: bool, encrypt: bool, validate: bool) { + // Implementation details +} + +// What do these booleans mean? +process_data(&data, true, false, true); + +// src/email_service.rs +// Even single booleans can be unclear +fn send_email(recipient: &str, urgent: bool) { + // Implementation +} + +send_email("user@example.com", true); // What does true mean? + +// src/database.rs +// Constructor with boolean flags +impl DatabaseConnection { + fn new(host: &str, use_ssl: bool, auto_reconnect: bool) -> Self { + // Implementation + } +} +``` + +### GOOD Examples + +```rust +// src/good_processor.rs +// Use enums for boolean-like choices +enum Compression { + Enabled, + Disabled, +} + +enum Encryption { + Enabled, + Disabled, +} + +fn process_data(data: &[u8], compression: Compression, encryption: Encryption) { + // Implementation +} + +// Self-documenting call site +process_data(&data, Compression::Enabled, Encryption::Disabled); + +// src/good_options.rs +// For many options, use parameter structs +struct ProcessOptions { + compression: bool, + encryption: bool, + validation: bool, +} + +fn process_with_options(data: &[u8], options: ProcessOptions) { + // Implementation +} + +process_with_options(&data, ProcessOptions { + compression: true, + encryption: false, + validation: true, +}); + +// src/good_builder.rs +// Builder pattern for complex configurations +impl DatabaseConnection { + fn builder() -> DatabaseConnectionBuilder { + DatabaseConnectionBuilder::new() + } +} + +struct DatabaseConnectionBuilder { + host: String, + use_ssl: bool, + auto_reconnect: bool, +} + +impl DatabaseConnectionBuilder { + fn with_ssl(mut self) -> Self { + self.use_ssl = true; + self + } + + fn with_auto_reconnect(mut self) -> Self { + self.auto_reconnect = true; + self + } +} +``` \ No newline at end of file diff --git a/skills/rust-expert-best-practices-code-review/rules/avoid-box.md b/skills/rust-expert-best-practices-code-review/rules/avoid-box.md new file mode 100644 index 0000000..7596462 --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/avoid-box.md @@ -0,0 +1,80 @@ +--- +title: Avoid Unnecessary Box for Concrete Types +impact: MEDIUM +impactDescription: Reduces unnecessary heap allocations and improves performance +tags: box, heap-allocation, performance, memory-management, rust +--- + +## Avoid Unnecessary Box for Concrete Types + +**Impact: MEDIUM (Reduces unnecessary heap allocations and improves performance)** + +Do not use `Box` wrapper for concrete types unless there is a legitimate reason for heap allocation. `Box` should only be used in specific scenarios where it provides necessary functionality, not as a default choice. + +**Valid reasons to use `Box` with concrete types:** +- **Complex generic types** like `GenericWriter` where the compiler cannot determine size at compile time +- **Recursive types** or deeply nested generics that would cause stack overflow +- **Types where the exact size varies** based on runtime conditions +- **Enum variants** to prevent the enum from becoming too large + +**Note:** This rule only applies to concrete type identifiers. `Box` for trait objects is always acceptable and not checked by this rule. + +### BAD Examples + +```rust +// src/simple.rs +type Result = std::result::Result; + +pub fn get_number() -> Result> { + Ok(Box::new(42)) +} + +// src/data.rs +struct Record { + id: u32, + name: String, +} + +pub fn create_record(name: String) -> Box { + Box::new(Record { id: 1, name }) +} + +// src/collections.rs +use std::collections::HashMap; + +pub fn get_config() -> Box> { + Box::new(HashMap::new()) +} +``` + +### GOOD Examples + +```rust +// src/adapter.rs +use std::fmt::Debug; + +trait Processor { + fn process(&self, input: Input) -> Output; +} + +#[derive(Debug)] +pub struct ProcessorAdapter { + // Box is acceptable for trait objects + processor: Box>, +} + +// src/recursive.rs +// Recursive type requires Box for heap allocation +pub enum Node { + Leaf(i32), + Branch(Box, Box), +} + +// src/complex.rs +use std::io::Write; + +// Complex generic type where size is unknown at compile time +pub struct Writer { + inner: Box>>, +} +``` \ No newline at end of file diff --git a/skills/rust-expert-best-practices-code-review/rules/avoid-panic.md b/skills/rust-expert-best-practices-code-review/rules/avoid-panic.md new file mode 100644 index 0000000..f20eef5 --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/avoid-panic.md @@ -0,0 +1,193 @@ +--- +title: Avoid Panic - Use Appropriate Error Handling +impact: CRITICAL +impactDescription: Prevents unexpected crashes and enables graceful error handling +tags: error-handling, panic, assert, result, rust +--- + +## Avoid Panic - Use Appropriate Error Handling + +**Impact: CRITICAL (Prevents unexpected crashes and enables graceful error handling)** + +Use appropriate error handling methods instead of `panic!` based on the context and function signature. + +**Invariant Validation** +Use `assert!` for validation that checks program invariants and state consistency: +- Input parameter validation +- Business logic constraints +- Data integrity checks +- State validation and consistency checks + +**Functions Returning Result** +In functions that return `Result`, use proper error handling instead of `panic!`: +- Use `map_err` to convert errors into the expected error type +- Return errors using the `?` operator or explicit `Err()` returns +- Avoid `panic!`, `unwrap()`, or `unwrap_or_else(|| panic!(...))` patterns + +**Functions Not Returning Result** +Functions that use `panic!` for error handling should consider returning `Result` instead: +- Replace `panic!` with proper error types and `Err()` returns +- Update the function signature to return `Result` +- This allows callers to handle errors gracefully rather than crashing + +**Unrecoverable Program States** +Reserve `panic!` for situations where the program is in an invalid and unrecoverable state: +- Critical system initialization failures (including main function startup validation) +- Unreachable code paths in match statements +- Critical system component failures that cannot be recovered from + +**Debug-Only Validation** +Use `debug_assert!` for validation that should only run in debug builds: +- Internal consistency checks +- Performance-sensitive validation +- Developer-focused assertions + +**Error Messages with expect** +When using `unwrap()` followed by `panic!` with an error message, or `unwrap_or_else(|| panic!(...))` patterns, use `expect` instead: +- Replace `unwrap_or_else(|| panic!("message"))` with `expect("message")` +- Replace manual unwrap + panic patterns with chained `expect` calls + +### BAD Examples + +```rust +// src/validation.rs +// Input validation should use assert! +fn validate_items(items: &[Record]) { + if items.is_empty() { + panic!("collection must contain at least one item"); + } +} + +// Business logic validation should use assert! +fn validate_constraint(c: &Constraint) { + if c.max <= 0 { + panic!("max must be positive"); + } +} + +// src/calculator.rs +// Function should return Result instead of panicking +fn compute_ratio_bad(num: u32, denom: u32) -> f32 { + if denom == 0 { + panic!("denominator cannot be zero"); + } + + if num > denom { + panic!("numerator exceeds denominator"); + } + + (num as f32 / denom as f32) * 100.0 +} + +// src/parser.rs +// Result functions should use proper error handling +pub fn parse_int(value: &str, line: usize) -> Result { + let parsed = value.parse::().unwrap_or_else(|err| { + panic!( + "Failed to parse '{}' as i64 at line {}: {}", + value, + line + 1, + err + ) + }); + + if parsed < 0 { + panic!("Value must be non-negative: {} at line {}", parsed, line + 1); + } + + Ok(parsed) +} + +// src/data.rs +// Use expect instead of unwrap + panic pattern +fn get_field_typed(tbl: &DataTable, col: &str) -> &T { + if let Some(arr) = tbl.column(col) { + arr.as_any().downcast_ref::().unwrap() + } else { + panic!("Missing column '{col}' in table"); + } +} +``` + +### GOOD Examples + +```rust +// src/validation.rs +// Input validation with assert! +fn validate_items(items: &[Record]) { + assert!(!items.is_empty(), "collection must contain at least one item"); +} + +// Business logic validation with assert! +fn validate_constraint(c: &Constraint) { + assert!(c.max > 0, "max must be positive"); +} + +// src/calculator.rs +enum RatioError { + ZeroDenom, + OutOfRange { num: u32, denom: u32 }, +} + +// Function returning Result instead of panicking +fn compute_ratio(num: u32, denom: u32) -> Result { + if denom == 0 { + return Err(RatioError::ZeroDenom); + } + + if num > denom { + return Err(RatioError::OutOfRange { num, denom }); + } + + Ok((num as f32 / denom as f32) * 100.0) +} + +// src/app.rs +// Critical initialization failures (acceptable panic!) +async fn run(cfg: Config) -> Result { + if cfg.inputs.is_empty() { + panic!("No inputs configured"); + } + // Continue processing... +} + +// src/evaluator.rs +// Unreachable code paths (acceptable panic!) +fn eval_expr(expr: &Expr) -> Value { + match expr { + Expr::Int(n) => Value::Int(*n), + Expr::Str(s) => Value::Str(s.clone()), + _ => panic!("unreachable: invalid expr kind"), + } +} + +// src/parser.rs +// Proper error handling in Result functions +pub fn parse_int(value: &str, line: usize) -> Result { + let parsed = value.parse::().map_err(|err| { + std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!("Failed to parse '{}' as i64 at line {}: {}", value, line + 1, err), + ) + })?; + + if parsed < 0 { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!("Value must be non-negative: {} at line {}", parsed, line + 1), + )); + } + + Ok(parsed) +} + +// src/data.rs +// Use expect instead of unwrap + panic +fn get_field_typed(tbl: &DataTable, col: &str) -> &T { + tbl.column(col) + .expect(&format!("Missing column '{col}' in table")) + .as_any() + .downcast_ref::() + .expect(&format!("Column '{col}' is not the expected type")) +} +``` \ No newline at end of file diff --git a/skills/rust-expert-best-practices-code-review/rules/avoid-rc.md b/skills/rust-expert-best-practices-code-review/rules/avoid-rc.md new file mode 100644 index 0000000..fefdfbf --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/avoid-rc.md @@ -0,0 +1,111 @@ +--- +title: Avoid Unnecessary Rc Usage +impact: MEDIUM +impactDescription: Simplifies ownership patterns and reduces reference counting overhead +tags: ownership, rc, memory-management, performance, rust +--- + +## Avoid Unnecessary Rc Usage + +**Impact: MEDIUM (Simplifies ownership patterns and reduces reference counting overhead)** + +Avoid unnecessary use of `Rc` when simpler ownership patterns would suffice. + +**Function Parameters** + +Use borrowed references (`&T` or `&[T]`) instead of `Rc` for function parameters that only need to read data, unless the parameter type is part of a system architecture that intentionally uses `Rc` for shared ownership. + +**Struct Fields** + +Don't wrap struct fields in `Rc` unless multiple owners genuinely need to share the same data instance. + +**Valid Use Cases for Rc** + +- Multiple components need to share the same immutable data instance (shared configuration, metadata, reference data) +- System architecture intentionally uses `Rc` for shared ownership throughout the codebase (e.g., trait methods that require `Rc` parameters for event processing) + +### BAD Examples + +```rust +// src/module.rs +use std::rc::Rc; + +// Function parameter wrapped in Rc unnecessarily +fn compute_total(items: Rc>) -> i32 { + items.iter().sum() +} + +// src/models/mod.rs +use std::rc::Rc; + +// Struct fields wrapped in Rc without sharing need +struct Data { + label: Rc, + items: Rc>, +} + +// Using Rc just to avoid ownership thinking +fn build_data() -> Data { + Data { + label: Rc::new("x".to_string()), + items: Rc::new(vec![1, 2, 3]), + } +} +``` + +### GOOD Examples + +```rust +// src/module.rs +// Simple borrowed reference for read-only access +fn compute_total(items: &[i32]) -> i32 { + items.iter().sum() +} + +// src/models/mod.rs +// Owned data in struct when no sharing needed +struct Data { + label: String, + items: Vec, +} + +// src/services/mod.rs +use std::rc::Rc; + +struct Config { + value: String, +} + +struct Handler { + config: Rc, // Multiple handlers share same config +} + +// Rc used for legitimate shared ownership +fn build_handlers() -> (Handler, Handler) { + let config = Rc::new(Config { value: "x".into() }); + ( + Handler { config: config.clone() }, + Handler { config: config.clone() }, + ) +} + +// src/core/mod.rs +use std::rc::Rc; + +// System architecture using Rc for shared event data +trait HandlerTrait { + type In: Event; + fn process(&self, input: Rc, ctx: &dyn Context) -> Vec>; +} + +trait Event {} +trait Context {} + +struct EventData; +impl Event for EventData {} + +struct ProcessedData { + source: Rc, // Shared reference as part of architecture +} +impl Event for ProcessedData {} +``` \ No newline at end of file diff --git a/skills/rust-expert-best-practices-code-review/rules/calculated-field-as-method.md b/skills/rust-expert-best-practices-code-review/rules/calculated-field-as-method.md new file mode 100644 index 0000000..cbb48e9 --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/calculated-field-as-method.md @@ -0,0 +1,88 @@ +--- +title: Implement Calculated Fields as Methods +impact: MEDIUM-HIGH +impactDescription: Prevents data duplication and maintains single source of truth +tags: struct-design, methods, calculated-fields, data-modeling, rust +--- + +## Implement Calculated Fields as Methods + +**Impact: MEDIUM-HIGH (Prevents data duplication and maintains single source of truth)** + +Calculated fields that are derived from mathematical operations on other struct fields should be implemented as methods rather than stored as separate fields. + +**Mathematical Operations** +Target fields assigned using mathematical expressions (`+`, `-`, `*`, `/`, `%`) on other struct fields during construction or initialization. + +**Exceptions** +- Fields assigned from method calls or external computations +- Performance-critical code where recalculation is expensive +- Fields representing cached/memoized values with explicit cache invalidation +- Values computed once and never change (immutable computed fields) +- Computations involving external data sources like timestamps, database values, API responses, or data from other objects + +### BAD Examples + +```rust +// Mathematical computation between struct fields in constructor +impl Foo { + pub fn new(a: f64, b: f64) -> Self { + Self { + a, + b, + computed: a * b, // Should be method + derived: 2.0 * (a + b), // Should be method + } + } +} + +// Percentage calculation in struct initialization +impl Bar { + pub fn new(count: u32, total: u32) -> Self { + Self { + count, + total, + rate: (count as f64 / total as f64) * 100.0, // Should be method + } + } +} +``` + +### GOOD Examples + +```rust +// Calculated fields as methods +impl Foo { + pub fn new(a: f64, b: f64) -> Self { + Self { a, b } + } + + pub fn computed(&self) -> f64 { + self.a * self.b + } + + pub fn derived(&self) -> f64 { + 2.0 * (self.a + self.b) + } +} + +// External data computation (allowed) +impl Baz { + pub fn new(base: i64, offset: i64) -> Self { + Self { + value: base - offset, // External computation + kind: Kind::Default, + } + } +} + +// Method call assignment (allowed) +impl Qux { + pub fn new(x: f64, y: f64, processor: &Processor) -> Self { + Self { + x, + y, + output: processor.compute(x, y), // Method call + } + } +} \ No newline at end of file diff --git a/skills/rust-expert-best-practices-code-review/rules/decimal-comparison.md b/skills/rust-expert-best-practices-code-review/rules/decimal-comparison.md new file mode 100644 index 0000000..102ef0e --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/decimal-comparison.md @@ -0,0 +1,89 @@ +--- +title: Use is_sign_negative() for Decimal Comparisons +impact: MEDIUM +impactDescription: Improves code clarity and uses idiomatic Decimal methods +tags: decimal, comparison, rust-decimal, readability, rust +--- + +## Use is_sign_negative() for Decimal Comparisons + +**Impact: MEDIUM (Improves code clarity and uses idiomatic Decimal methods)** + +When checking if a `Decimal` value is negative, use the `is_sign_negative()` method instead of comparing to `Decimal::ZERO`. This is more explicit, readable, and uses the method provided by the Rust Decimal library specifically for checking the sign of a decimal value. + +**Patterns to Replace** +- `dec < Decimal::ZERO` → `dec.is_sign_negative()` +- `dec <= Decimal::ZERO` → `dec.is_sign_negative() || dec.is_zero()` +- `Decimal::ZERO > dec` → `dec.is_sign_negative()` +- `Decimal::ZERO >= dec` → `dec.is_sign_negative() || dec.is_zero()` + +**Valid Usage (not flagged)** +- `dec == Decimal::ZERO` — equality checks are acceptable +- `dec != Decimal::ZERO` — inequality checks are acceptable +- `dec >= Decimal::ZERO` — validation assertions are acceptable +- Variable initialization: `let dec = Decimal::ZERO` + +### BAD Examples + +```rust +// src/lib.rs +use rust_decimal::Decimal; + +fn check_decimals(dec: Decimal) -> bool { + // Direct comparison with ZERO + let is_negative = dec < Decimal::ZERO; + is_negative +} + +fn validate_value(value: Decimal) -> String { + // Reverse comparison + if Decimal::ZERO > value { + return "invalid".to_string(); + } + "valid".to_string() +} + +fn process_values(value1: Decimal, value2: Decimal) { + // Non-positive check + if value1 <= Decimal::ZERO { + println!("Non-positive value"); + } + + // Multiple comparisons + let first_negative = value1 < Decimal::ZERO; + let second_negative = value2 < Decimal::ZERO; +} +``` + +### GOOD Examples + +```rust +// src/lib.rs +use rust_decimal::Decimal; + +fn check_decimals(dec: Decimal) -> bool { + // Using is_sign_negative method + let is_negative = dec.is_sign_negative(); + is_negative +} + +fn validate_value(value: Decimal) -> String { + // Using is_sign_negative method + if value.is_sign_negative() { + return "invalid".to_string(); + } + "valid".to_string() +} + +fn process_values(value1: Decimal, value2: Decimal) { + // Non-positive check using both methods + if value1.is_sign_negative() || value1.is_zero() { + println!("Non-positive value"); + } + + // Equality check (acceptable) + if value2 == Decimal::ZERO { + println!("Zero value"); + } +} +``` \ No newline at end of file diff --git a/skills/rust-expert-best-practices-code-review/rules/match-statements-handle-all-cases.md b/skills/rust-expert-best-practices-code-review/rules/match-statements-handle-all-cases.md new file mode 100644 index 0000000..31519a6 --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/match-statements-handle-all-cases.md @@ -0,0 +1,106 @@ +--- +title: Match Statements Should Handle All Cases Explicitly +impact: HIGH +impactDescription: Prevents bugs when new enum variants are added through exhaustive matching +tags: pattern-matching, enums, exhaustive-match, type-safety, rust +--- + +## Match Statements Should Handle All Cases Explicitly + +**Impact: HIGH (Prevents bugs when new enum variants are added through exhaustive matching)** + +Match statements should explicitly handle all enum variants instead of using catch-all patterns (`_` or `..`). + +Catch-all patterns can hide bugs when new enum variants are added, as the compiler won't warn about unhandled cases. Instead, explicitly list all variants or group related variants using the `|` operator. + +### BAD Examples + +```rust +// Example enum (defined elsewhere, e.g. crate::types) +enum State { + A, + B, + C, + D, +} + +// Using catch-all pattern +fn process_state(state: State) -> String { + match state { + State::A => "processing a".to_string(), + State::B => "processing b".to_string(), + _ => "default handling".to_string(), // Hides unhandled variants + } +} + +// Using wildcard catch-all +fn handle_value(value: i32) -> String { + match value { + 1 => "one".to_string(), + 2 => "two".to_string(), + _ => "rest".to_string(), // Catch-all for remaining values + } +} + +// Example enum (defined elsewhere) +enum Kind { + X, + Y, + Z, +} + +// Multiple catch-all patterns +fn route(kind: Kind, level: i32) -> String { + match (kind, level) { + (Kind::X, 1) => "high priority".to_string(), + (Kind::Y, _) => "standard".to_string(), + _ => "fallback".to_string(), // Catch-all tuple pattern + } +} +``` + +### GOOD Examples + +```rust +// Example enum (defined elsewhere, e.g. crate::types) +enum State { + A, + B, + C, + D, +} + +// Explicitly handle all variants +fn process_state(state: State) -> String { + match state { + State::A => "processing a".to_string(), + State::B => "processing b".to_string(), + State::C => "processing c".to_string(), + State::D => "processing d".to_string(), + } +} + +// Example enum (defined elsewhere) +enum Kind { + X, + Y, + Z, + W, +} + +// Group related variants +fn route(kind: Kind) -> String { + match kind { + Kind::X | Kind::Y => "high priority".to_string(), + Kind::Z | Kind::W => "standard priority".to_string(), + } +} + +// std::result::Result — explicit handling with all possible outcomes +fn handle_result(result: Result) -> String { + match result { + Ok(value) => format!("success: {}", value), + Err(error) => format!("error: {}", error), + } +} +``` \ No newline at end of file diff --git a/skills/rust-expert-best-practices-code-review/rules/prefer-builder-pattern-for-complex.md b/skills/rust-expert-best-practices-code-review/rules/prefer-builder-pattern-for-complex.md new file mode 100644 index 0000000..b5a1122 --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/prefer-builder-pattern-for-complex.md @@ -0,0 +1,144 @@ +--- +title: Prefer Builder Pattern for Complex Constructors +impact: HIGH +impactDescription: Improves API ergonomics and prevents parameter confusion +tags: builder-pattern, api-design, constructors, ergonomics, rust +--- + +## Prefer Builder Pattern for Complex Constructors + +**Impact: HIGH (Improves API ergonomics and prevents parameter confusion)** + +Rust constructors with multiple parameters (especially with optional values) should use the builder pattern instead of having many parameters. + +**Constructors that should use builder pattern:** + +- Functions with 4 or more parameters +- Functions with `Option` parameters (regardless of parameter count) +- Functions where parameter order could be confusing + +**Exclude from builder pattern:** + +- Simple constructors with 1-3 required parameters only +- Structs/impls already ending with "Builder" +- Copy/Clone operations (`from_existing`, `from_other`, etc.) +- Internal utility constructors + +### BAD Examples + +```rust +// Too many parameters, hard to remember order +fn create_entity( + field_a: String, + field_b: String, + field_c: u8, + flag_a: bool, + flag_b: bool, + limit: u32, + optional_field: Option, +) -> Entity { + Entity { + field_a, + field_b, + field_c, + flag_a, + flag_b, + limit, + optional_field, + } +} + +// Constructor with optional parameters +impl ServiceConnection { + fn new( + endpoint: String, + port: u16, + user_id: String, + secret: Option, + use_tls: Option, + ) -> Self { + Self { + endpoint, + port, + user_id, + secret, + use_tls: use_tls.unwrap_or(false), + } + } +} + +// Constructor with exactly 4 parameters +impl CatalogRecord { + fn new( + record_id: RecordId, + label: String, + tags: Vec, + notes: Option, + ) -> Self { + Self { + record_id, + label, + tags, + notes, + } + } +} +``` + +### GOOD Examples + +```rust +// Simple constructor - no builder needed +impl Vector2 { + fn new(x: f64, y: f64) -> Self { + Self { x, y } + } +} + +// Already using builder pattern +impl EntityBuilder { + fn new() -> Self { + Self { + field_a: String::new(), + field_b: String::new(), + field_c: 0, + flag_a: false, + flag_b: false, + limit: 100, + optional_field: None, + } + } + + fn field_a(mut self, value: String) -> Self { + self.field_a = value; + self + } + + fn field_b(mut self, value: String) -> Self { + self.field_b = value; + self + } + + fn build(self) -> Result { + Ok(Entity { + field_a: self.field_a, + field_b: self.field_b, + field_c: self.field_c, + flag_a: self.flag_a, + flag_b: self.flag_b, + limit: self.limit, + optional_field: self.optional_field, + }) + } +} + +// Copy constructor - no builder needed +impl Settings { + fn from_existing(other: &Settings) -> Self { + Self { + config: other.config.clone(), + enabled: other.enabled, + } + } +} +``` \ No newline at end of file diff --git a/skills/rust-expert-best-practices-code-review/rules/prefer-format.md b/skills/rust-expert-best-practices-code-review/rules/prefer-format.md new file mode 100644 index 0000000..a85d8a6 --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/prefer-format.md @@ -0,0 +1,53 @@ +--- +title: Prefer format! Over Manual String Concatenation +impact: MEDIUM +impactDescription: Improves code clarity and reduces string building complexity +tags: strings, format-macro, concatenation, readability, rust +--- + +## Prefer format! Over Manual String Concatenation + +**Impact: MEDIUM (Improves code clarity and reduces string building complexity)** + +Prefer `format!` over manual string concatenation when building strings from multiple pieces (literals + variables). + +**Manual String Building** +Avoid using `push_str`, `push`, or `+` operator on a fresh `String` when a single `format!` call would be clearer. + +**Performance Note** +This rule prioritizes code clarity over micro-optimizations. Use `format!` unless performance profiling shows string building is a bottleneck. + +### BAD Examples + +```rust +// Multiple push operations +let mut s = "Hello ".to_owned(); +s.push_str(name); +s.push('!'); + +// String concatenation with + +let message = "Error: ".to_string() + &error_code + " - " + &description; + +// Mixed push operations +let mut path = base_dir.to_string(); +path.push('/'); +path.push_str(&filename); +path.push_str(".txt"); +``` + +### GOOD Examples + +```rust +// Single format! call +let s = format!("Hello {name}!"); + +// Format with multiple variables +let message = format!("Error: {error_code} - {description}"); + +// Format for path building +let path = format!("{base_dir}/{filename}.txt"); + +// Simple single operation (acceptable) +let mut s = base.to_string(); +s.push_str(suffix); +``` \ No newline at end of file diff --git a/skills/rust-expert-best-practices-code-review/rules/result-error-returns.md b/skills/rust-expert-best-practices-code-review/rules/result-error-returns.md new file mode 100644 index 0000000..84c998b --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/result-error-returns.md @@ -0,0 +1,112 @@ +--- +title: Proper Error Handling in Result-Returning Functions +impact: CRITICAL +impactDescription: Ensures proper error propagation and prevents panics in error paths +tags: error-handling, result, question-mark-operator, unwrap, rust +--- + +## Proper Error Handling in Result-Returning Functions + +**Impact: CRITICAL (Ensures proper error propagation and prevents panics in error paths)** + +Functions that return `Result` types must use proper error handling patterns and avoid redundant error handling constructs. + +**Avoid `.unwrap()` and `.expect()`** +Use the `?` operator instead of `.unwrap()` or `.expect()` methods to properly propagate errors. Exception: `.await.expect("message")?` is acceptable for handling task join results where you expect the task to succeed but want to propagate the inner Result. + +**Avoid Redundant Error Handling Patterns** +- Don't use `if let Err(e)` pattern followed by `return Err(e)` when you can use `?` operator +- Don't use `.map_err(|e| e)?` to forward errors unchanged +- Don't use `.and_then(|v| Ok(v))?` for identity transformations +- For async code, avoid `.await.unwrap()?` pattern which defeats error propagation + +### BAD Examples + +```rust +// src/handlers/user.rs +use std::error::Error; + +// Using unwrap() in Result-returning function +fn read_number() -> Result { + let s = "123"; + let n: i32 = s.parse::().unwrap(); // BAD: panic on invalid input + Ok(n) +} + +// Using expect() in Result-returning function +fn process_data() -> Result> { + let data = fetch_data().expect("Failed to fetch data"); + Ok(data.to_string()) +} + +// Redundant if let Err pattern +fn handle_result() -> Result { + if let Err(e) = some_operation() { + return Err(e); + } + let value = some_operation().unwrap(); + Ok(value) +} + +// Redundant map_err forwarding +fn forward_error() -> Result { + let value = some_result.map_err(|e| e)?; + Ok(value) +} + +// Async unwrap defeating error propagation +async fn async_handler() -> Result { + let value = handle.await.unwrap()?; // BAD: panics on task failure + Ok(value) +} +``` + +### GOOD Examples + +```rust +// src/handlers/user.rs +use std::error::Error; + +// Using ? operator for error propagation +fn read_number() -> Result { + let s = "123"; + let n: i32 = s.parse()?; // GOOD: returns Err on failure + Ok(n) +} + +// Using ? operator with different error types +fn process_data() -> Result> { + let data = fetch_data()?; + Ok(data.to_string()) +} + +// Direct use of ? operator +fn handle_result() -> Result { + let value = some_operation()?; + Ok(value) +} + +// Direct error propagation +fn forward_error() -> Result { + let value = some_result?; + Ok(value) +} + +// Proper async error handling +async fn async_handler() -> Result { + let value = handle.await?; // GOOD: handles async errors properly + Ok(value) +} + +// Acceptable: await.expect()? +async fn handle_task_join() -> Result { + let writer = task_handle.await.expect("Expected task to succeed")?; + Ok(writer) +} + +// Functions not returning Result can use unwrap (not covered by this rule) +fn main() { + let n = "123".parse::().unwrap(); // OK: main doesn't return Result + println!("{}", n); +} +``` \ No newline at end of file diff --git a/skills/rust-expert-best-practices-code-review/rules/use-borrowed-argument-types.md b/skills/rust-expert-best-practices-code-review/rules/use-borrowed-argument-types.md new file mode 100644 index 0000000..4423c67 --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/use-borrowed-argument-types.md @@ -0,0 +1,83 @@ +--- +title: Use Borrowed Argument Types Instead of Owned Types +impact: HIGH +impactDescription: Improves API flexibility and enables deref coercion for better ergonomics +tags: api-design, function-parameters, deref-coercion, ergonomics, rust +--- + +## Use Borrowed Argument Types Instead of Owned Types + +**Impact: HIGH (Improves API flexibility and enables deref coercion for better ergonomics)** + +Use borrowed argument types instead of references to owned types in function parameters. + +**Specific Replacements:** + +- `&String` → `&str` +- `&Vec` → `&[T]` +- `&PathBuf` → `&Path` +- `&Box` → `&T` + +This enables better API flexibility by accepting both owned and borrowed types through Rust's deref coercion. + +### BAD Examples + +```rust +// src/lib.rs +use std::path::PathBuf; + +// &String prevents passing string literals +fn three_vowels(word: &String) -> bool { + word.chars().filter(|c| "aeiou".contains(*c)).count() >= 3 +} + +// &Vec prevents passing arrays or slices +fn sum_numbers(nums: &Vec) -> i32 { + nums.iter().sum() +} + +// &PathBuf prevents passing &Path +fn read_config(path: &PathBuf) -> String { + std::fs::read_to_string(path).unwrap() +} + +// &Box adds unnecessary indirection +struct MyStruct { + value: i32, +} + +fn process_data(data: &Box) -> i32 { + data.value * 2 +} +``` + +### GOOD Examples + +```rust +// src/lib.rs +use std::path::Path; + +// &str accepts both &str and &String +fn three_vowels(word: &str) -> bool { + word.chars().filter(|c| "aeiou".contains(*c)).count() >= 3 +} + +// &[T] accepts Vec, arrays, and slices +fn sum_numbers(nums: &[i32]) -> i32 { + nums.iter().sum() +} + +// &Path accepts both PathBuf and &Path +fn read_config(path: &Path) -> String { + std::fs::read_to_string(path).unwrap() +} + +// &T removes unnecessary indirection +struct MyStruct { + value: i32, +} + +fn process_data(data: &MyStruct) -> i32 { + data.value * 2 +} +``` \ No newline at end of file diff --git a/skills/rust-expert-best-practices-code-review/rules/use-copied.md b/skills/rust-expert-best-practices-code-review/rules/use-copied.md new file mode 100644 index 0000000..fd0ae8b --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/use-copied.md @@ -0,0 +1,92 @@ +--- +title: Use .copied() for Copy Types in Iterators +impact: MEDIUM +impactDescription: Simplifies dereferencing patterns and improves code readability +tags: iterators, copy-types, dereferencing, readability, rust +--- + +## Use .copied() for Copy Types in Iterators + +**Impact: MEDIUM (Simplifies dereferencing patterns and improves code readability)** + +When iterating over collections containing Copy types (like `usize`, `i32`, `bool`, etc.), use `.copied()` to avoid complex dereferencing patterns with multiple reference layers. + +**Apply when:** +- Iterating over collections containing Copy types +- The iterator closure uses multiple dereference patterns (`&&&`, `&&`, etc.) +- The type being dereferenced implements the Copy trait + +### BAD Examples + +```rust +// src/lib.rs +fn filter_numbers(input: &[usize]) -> Vec { + input + .iter() + .filter(|&&&x| x > 5) + .collect() +} + +// src/lib.rs +fn filter_bools(input: &[bool]) -> Vec { + input + .iter() + .filter(|&&x| x) + .collect() +} + +// src/processing.rs +fn process_integers(input: &[i32]) -> Vec { + input + .iter() + .map(|&&&x| x * 2) + .filter(|&&&y| y > 10) + .collect() +} + +// src/math.rs +fn double_values(input: &[f64]) -> Vec { + input + .iter() + .map(|&&x| x * 2.0) + .collect() +} +``` + +### GOOD Examples + +```rust +// src/lib.rs +fn filter_numbers(input: &[usize]) -> Vec { + input + .iter() + .copied() + .filter(|&x| x > 5) + .collect() +} + +// src/lib.rs +fn filter_bools(input: &[bool]) -> Vec { + input + .iter() + .copied() + .filter(|x| *x) + .collect() +} + +// src/strings.rs +fn filter_strings(input: &[String]) -> Vec<&String> { + input + .iter() + .filter(|&s| s.len() > 5) + .collect() +} + +// src/sum.rs +fn sum_positive(input: &[i32]) -> i32 { + input + .iter() + .filter(|&x| *x > 0) + .sum() +} +``` \ No newline at end of file diff --git a/skills/rust-expert-best-practices-code-review/rules/use-enum-deserialization.md b/skills/rust-expert-best-practices-code-review/rules/use-enum-deserialization.md new file mode 100644 index 0000000..2273975 --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/use-enum-deserialization.md @@ -0,0 +1,121 @@ +--- +title: Use Exhaustive Enum Deserialization with Pattern Matching +impact: CRITICAL +impactDescription: Prevents invalid states and unsafe vector access through type-level safety +tags: deserialization, type-safety, pattern-matching, serde, enums, rust +--- + +## Use Exhaustive Enum Deserialization with Pattern Matching + +**Impact: CRITICAL (Prevents invalid states and unsafe vector access through type-level safety)** + +Use exhaustive enum deserialization with slice pattern matching for safe Vec access in Rust. + +**Exhaustive Match Statements** + +During deserialization, use explicit match statements on tuples of related fields to handle every meaningful combination. Any invalid combination must produce a descriptive error. + +**Vector Slice Pattern Matching** + +Use `match vec.as_slice()` with slice patterns (`[]`, `[one]`, `[first, ..]`) instead of `is_empty()` checks followed by indexing. + +**Custom Deserializers** + +Use `#[serde(deserialize_with = "...")]` to enforce business rules at deserialization boundaries with type-level safety. + +**Clear Error Messages** + +Errors must specify which fields are present/absent, why the combination is invalid, and what combinations are valid. + +### BAD Examples + +```rust +// src/models/item.rs +use serde::Deserialize; + +// Missing exhaustive matching - allows invalid states +#[derive(Deserialize)] +struct Item { + field_a: Option, + field_b: Option, + flag: bool, +} + +impl Item { + fn validate(&self) -> Result<(), String> { + // Generic error without context + if self.field_a.is_some() != self.field_b.is_some() { + return Err("Invalid state".to_string()); + } + Ok(()) + } +} + +// src/services/element.rs +// Unsafe vector access after is_empty check +fn get_first_element(elements: Vec) -> Option { + if !elements.is_empty() { + Some(elements[0].clone()) // Unsafe indexing + } else { + None + } +} +``` + +### GOOD Examples + +```rust +// src/models/config.rs +use serde::{Deserialize, Deserializer}; + +#[derive(Deserialize)] +struct ConfigHelper { + field_a: Option, + field_b: Option, + flag: bool, +} + +// Custom deserializer with explicit matching + type-level safety +#[derive(Deserialize)] +#[serde(deserialize_with = "deserialize_config")] +enum Config { + Active { a: String, b: String }, + Inactive, +} + +fn deserialize_config<'de, D>(deserializer: D) -> Result +where + D: Deserializer<'de>, +{ + let helper = ConfigHelper::deserialize(deserializer)?; + + match (helper.field_a, helper.field_b, helper.flag) { + (Some(a), Some(b), true) => Ok(Config::Active { a, b }), + (None, None, false) => Ok(Config::Inactive), + + (field_a, field_b, flag) => Err(serde::de::Error::custom(format!( + "Invalid config combination: field_a_present={}, field_b_present={}, flag={}. \ + Expected either (field_a & field_b present AND flag=true) or (field_a & field_b absent AND flag=false).", + field_a.is_some(), + field_b.is_some(), + flag + ))), + } +} + +// src/services/element.rs +#[derive(Debug)] +enum ElementError { + NotFound, + MultipleMatches, +} + +// Safe vector pattern matching (forces handling all cases) +fn pick_single_element(elements: Vec) -> Result { + match elements.as_slice() { + [] => Err(ElementError::NotFound), + [only] => Ok(only.clone()), + _ => Err(ElementError::MultipleMatches), + } +} +``` \ No newline at end of file diff --git a/skills/rust-expert-best-practices-code-review/rules/use-iterator-transforms.md b/skills/rust-expert-best-practices-code-review/rules/use-iterator-transforms.md new file mode 100644 index 0000000..0e3ca22 --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/use-iterator-transforms.md @@ -0,0 +1,91 @@ +--- +title: Use Iterator Transforms Instead of Push Loops +impact: MEDIUM-HIGH +impactDescription: Improves code clarity and functional programming style +tags: iterators, functional-programming, collections, idiomatic-rust, rust +--- + +## Use Iterator Transforms Instead of Push Loops + +**Impact: MEDIUM-HIGH (Improves code clarity and functional programming style)** + +Use iterator transforms instead of explicit push loops when building collections with simple transformations and filtering. + +Replace `for` loops that iterate over a collection and push elements to a mutable `Vec` when the loop only contains simple filtering with `if` conditions and has no early breaks, continues, or returns. + +**Iterator Methods to Use:** +- `.filter()` for conditional inclusion +- `.map()` for transformations +- `.filter_map()` for combined filtering and transformation +- `.collect()` to build the final collection + +### BAD Examples + +```rust +// Simple filtering and transformation +let mut names = Vec::new(); +for user in users { + if user.is_active { + names.push(user.name.to_lowercase()); + } +} + +// Just transformation +let mut ids = Vec::new(); +for item in items { + ids.push(item.id); +} + +// Multiple conditions with filtering +let mut valid_emails = Vec::new(); +for contact in contacts { + if contact.email.is_some() && contact.verified { + valid_emails.push(contact.email.unwrap()); + } +} + +// Simple filtering without transformation +let mut active_users = Vec::new(); +for user in users { + if user.status == Status::Active { + active_users.push(user); + } +} +``` + +### GOOD Examples + +```rust +// Iterator chain with filter and map +let names: Vec<_> = users + .into_iter() + .filter(|u| u.is_active) + .map(|u| u.name.to_lowercase()) + .collect(); + +// Simple map transformation +let ids: Vec<_> = items.into_iter().map(|item| item.id).collect(); + +// Combined filtering and transformation with filter_map +let valid_emails: Vec<_> = contacts + .into_iter() + .filter_map(|c| if c.verified { c.email } else { None }) + .collect(); + +// Simple filtering +let active_users: Vec<_> = users + .into_iter() + .filter(|u| u.status == Status::Active) + .collect(); + +// Complex logic that should remain as loop (has early return) +let mut results = Vec::new(); +for item in items { + if item.is_critical() { + return Err("Critical item found"); + } + if item.is_valid() { + results.push(item.process()); + } +} +``` \ No newline at end of file diff --git a/skills/rust-expert-best-practices-code-review/rules/use-named-placeholders.md b/skills/rust-expert-best-practices-code-review/rules/use-named-placeholders.md new file mode 100644 index 0000000..93d81b7 --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/use-named-placeholders.md @@ -0,0 +1,58 @@ +--- +title: Use Named Placeholders in Destructuring +impact: MEDIUM +impactDescription: Improves code readability and self-documentation +tags: pattern-matching, destructuring, readability, self-documenting, rust +--- + +## Use Named Placeholders in Destructuring + +**Impact: MEDIUM (Improves code readability and self-documentation)** + +Use named placeholders instead of bare `_` when destructuring structs in match patterns or let bindings. + +When destructuring structs with unused fields, provide descriptive names followed by `_` rather than using bare `_` placeholders. This makes the code more readable and self-documenting by clearly indicating what fields are being ignored. + +### BAD Examples + +```rust +// Bare underscore placeholders in match +match self { + Self::Rocket { _, _, .. } => { /* ... */ } +} + +// Multiple bare underscores in let binding +let User { _, _, name, .. } = user; + +// Function parameters with bare underscore +fn process_data(Data { _, value, .. }: Data) { + println!("{}", value); +} + +// Pattern matching with bare underscores +if let Config { _, enabled, .. } = config { + return enabled; +} +``` + +### GOOD Examples + +```rust +// Named placeholders showing what's being ignored +match self { + Self::Rocket { has_fuel: _, has_crew: _, .. } => { /* ... */ } +} + +// Clear field names in let binding +let User { id: _, email: _, name, .. } = user; + +// Self-documenting function parameters +fn process_data(Data { timestamp: _, value, .. }: Data) { + println!("{}", value); +} + +// Pattern matching with named placeholders +if let Config { debug_mode: _, enabled, .. } = config { + return enabled; +} +``` \ No newline at end of file diff --git a/skills/rust-expert-best-practices-code-review/rules/use-typesafe-index-wrappers.md b/skills/rust-expert-best-practices-code-review/rules/use-typesafe-index-wrappers.md new file mode 100644 index 0000000..af0f4a5 --- /dev/null +++ b/skills/rust-expert-best-practices-code-review/rules/use-typesafe-index-wrappers.md @@ -0,0 +1,110 @@ +--- +title: Use Type-Safe Index Wrappers +impact: CRITICAL +impactDescription: Prevents index mixing bugs at compile time through newtype pattern +tags: type-safety, newtype, indices, compile-time-safety, rust +--- + +## Use Type-Safe Index Wrappers + +**Impact: CRITICAL (Prevents index mixing bugs at compile time through newtype pattern)** + +Use type-safe index wrappers instead of raw primitive types (like `usize`) when working with index-based data structures to prevent mixing different index types. + +**Index Wrapper Requirements** + +Each index type should be a newtype wrapper with: +- **Derive traits**: `Debug`, `Clone`, `Copy`, `PartialEq`, `Eq` +- **Constructor method**: `new(index: primitive_type) -> Self` +- **Accessor method**: `get(self) -> primitive_type` +- **Hash trait**: Add `Hash` if the index will be used as a key in collections + +**Function Parameters** + +Functions that accept indices should use the specific index type rather than raw primitives to ensure type safety at compile time. + +### BAD Examples + +```rust +// src/container.rs +// Raw usize for different index types +pub struct Container { + items_a: Vec, + items_b: Vec, +} + +impl Container { + pub fn get_a(&self, index: usize) -> &Foo { + &self.items_a[index] + } + + pub fn get_b(&self, index: usize) -> &Bar { + &self.items_b[index] + } +} + +// src/processor.rs +// Risk of mixing indices in complex operations +pub struct Processor { + items_a: Vec, + items_b: Vec, +} + +impl Processor { + pub fn link(&mut self, a_idx: usize, b_idx: usize) { + // Could accidentally mix A and B indices + self.items_a[b_idx].attach(a_idx); + } +} +``` + +### GOOD Examples + +```rust +// src/indices.rs +// Type-safe index wrappers +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct IndexA(usize); + +impl IndexA { + pub fn new(index: usize) -> Self { + Self(index) + } + + pub fn get(self) -> usize { + self.0 + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct IndexB(usize); + +impl IndexB { + pub fn new(index: usize) -> Self { + Self(index) + } + + pub fn get(self) -> usize { + self.0 + } +} + +// src/container.rs +use crate::indices::{IndexA, IndexB}; + +// Using type-safe indices prevents mixing +pub struct Container { + items_a: Vec, + items_b: Vec, +} + +impl Container { + pub fn get_a(&self, index: IndexA) -> &Foo { + &self.items_a[index.get()] + } + + pub fn get_b(&self, index: IndexB) -> &Bar { + &self.items_b[index.get()] + } +} +``` \ No newline at end of file