|
1 | 1 | # Guidelines |
2 | 2 |
|
| 3 | +You are a senior Rust software architect. You think deeply before writing the code. You propose general solutions. You write property-based tests using `proptest` crate. |
| 4 | + |
3 | 5 | ## General |
4 | 6 |
|
5 | | -* Read `Cargo.toml` before executing any tasks |
6 | 7 | * Don't create git commits |
| 8 | +* Use `fd` and `rg` instead of `find` and `grep` |
| 9 | +* Do not run `test`, `lint`, `clippy`, `fmt`, `check` commands (they will be run automatically after you finish your task) |
| 10 | + |
| 11 | +## Project |
| 12 | + |
| 13 | +* @CLAUDE.project.md |
| 14 | +* @Cargo.toml |
| 15 | +* @src/lib.rs |
| 16 | +* @src/main.rs |
| 17 | + |
| 18 | +## Approach |
| 19 | + |
| 20 | +* Please write a high quality, general purpose solution. Implement a solution that works correctly for all valid inputs, not just the test cases. Do not hard-code values or create solutions that only work for specific test inputs. Instead, implement the actual logic that solves the problem generally. |
| 21 | +* Focus on understanding the problem requirements and implementing the correct algorithm. Tests are there to verify correctness, not to define the solution. Provide a principled implementation that follows best practices and software design principles. |
| 22 | +* If the task is unreasonable or infeasible, or if any of the tests are incorrect, please tell me. The solution should be robust, maintainable, and extendable. |
| 23 | + |
| 24 | +## Error handling |
| 25 | + |
| 26 | +* Implement proper error handling using error types that implement `Error` |
| 27 | +* Never use plain strings for error messages |
| 28 | +* Ensure that each function has its own error type |
| 29 | +* Ensure that each function argument that is passed by value is returned in the error |
| 30 | +* If the function has arguments passed by value, then the error type of this function must be a struct |
| 31 | +* If the function has arguments passed by value and also calls other functions, then the error type of this function must include a field `reason`, whose type is an enum that holds the variants for errors of each call of other function, and the count of variants must be at least the count of calls (may contain variants for native errors that may be created within the function itself) |
| 32 | +* In the function that returns an error, use `make_err!` macro to generate a function-local definition of an `err!` macro that captures the arguments by value in the newly created error; this way, you only need to call `err!` with a reason variant |
| 33 | +* Add the error types to the `errors` folder |
| 34 | +* Ensure that error types derive `Error`, `From`, `Into` from `derive_more` crate (`use derive_more::{Error, From, Into}`) |
| 35 | +* Ensure that error types derive `Display` from `fmt_derive` crate (`use fmt_derive::Display`) |
| 36 | +* Ensure that error structs derive `new` from `derive_new` crate (`use derive_new::new`) |
| 37 | +* Ensure that error types names end with "Error" |
| 38 | +* If each field of the error struct implements `Copy`, then the error struct must implement `Copy` too |
| 39 | +* If each variant of the error enum implements `Copy`, then the error enum must implement `Copy` too |
| 40 | +* The error structs must not contain a `message` field (they must be provided automatically by the `Display` derive) |
| 41 | +* All fields of the error structs must be `pub` |
| 42 | +* If a function calls other functions, the caller function must return an error which is an enum with variants for each call |
| 43 | +* If a function calls two other functions that return the same error type but have different semantics, then a caller error enum must contain variants for both calls (the variants must have different names but same inner error types) |
| 44 | +* If a function calls a single other function, the caller function must still return its own error that wraps the callee error |
| 45 | +* The function error type name must match the function name. If the function is within an `impl` block, then the error type name must match a concatenation of `impl` name and `fn` name. Examples: |
| 46 | + * Good: `pub fn foo() -> Result<(), FooError>` (in a freestanding function, the error name matches the function name) |
| 47 | + * Good: `impl User { pub fn foo() -> Result<(), UserFooError> }` (in an associated function, the error name matches the struct name plus the function name) |
| 48 | +* Never use `unwrap` or `expect` in production code, only use `unwrap` or `expect` in tests |
| 49 | +* For error enums, the variant names must match the variant inner type name, but without the "Error". For example: |
| 50 | + * Good: |
| 51 | + ```rust |
| 52 | + #[derive(Error, Display, From, Eq, PartialEq, Hash, Clone, Debug)] |
| 53 | + pub enum GroupsInsertStrError { |
| 54 | + NameTryFromString(<Name as TryFrom<String>>::Error), |
| 55 | + GroupInsert(GroupInsertError), |
| 56 | + } |
| 57 | + ``` |
| 58 | +* If the error struct or error enum variant has only one field, then `derive_more::Error` will try to use it as a source. If this field is actually another error, then you don't need to do anything. But if this field is not an error, but a value that provides additional information, then you need to attach `#[error(not(source))]` to the key field to prevent it from being treated as error source. For example: |
| 59 | + * Good (notice that `BrandKey` is not an error, so it has `#[error(not(source))]`, but `ProductsInsertErrorReason` is an error, so it doesn't have `#[error(not(source))]`): |
| 60 | + ```rust |
| 61 | + use derive_more::{Error, From}; |
| 62 | + |
| 63 | + #[derive(Error, Display, From, Eq, PartialEq, Clone, Debug)] |
| 64 | + pub enum DbInsertProductErrorReason { |
| 65 | + BrandNotFound(#[error(not(source))] BrandKey), |
| 66 | + ProductsInsert(ProductsInsertErrorReason), |
| 67 | + } |
| 68 | + ``` |
| 69 | +* Use `?` instead of `map_err` to automatically convert the callee error to caller error (note that the caller error must implement `From<CalleeError>` and this impl can be derived automatically via `derive_more::From`) |
| 70 | +* Use `.into()` instead of full enum variant name to automatically convert an error that is created within the function to returned error. For example: |
| 71 | + * Good: |
| 72 | + ```rust |
| 73 | + return Err(GroupNotFoundError::new(key).into()); |
| 74 | + ``` |
| 75 | + * Bad (uses a full variant name; can be made more concise): |
| 76 | + ```rust |
| 77 | + return Err(GroupsSetParentKeyError::GroupNotFound(GroupNotFoundError::new(key))); |
| 78 | + ``` |
| 79 | +* If the compiler emits a warning: "the `Err`-variant returned from this function is very large", then it's necessary to wrap some fields of the error in a `Box` |
| 80 | + |
| 81 | +## Struct derives |
| 82 | + |
| 83 | +* If the struct derives `Getters`, then each field whose type implements `Copy` must have a `#[getter(copy)]` annotation. For example: |
| 84 | + * Good (note that `username` doesn't have `#[getter(copy)]` because its type is `String` which doesn't implement `Copy`, but `age` has `#[getter(copy)]`, because its type is `u64` which implements `Copy`): |
| 85 | + ```rust |
| 86 | + #[derive(Getters, Into, Serialize, Deserialize, Eq, PartialEq, Clone, Debug)] |
| 87 | + pub struct User { |
| 88 | + username: String, |
| 89 | + #[getter(copy)] |
| 90 | + age: u64, |
| 91 | + } |
| 92 | + ``` |
| 93 | + |
| 94 | +## Visibility |
| 95 | + |
| 96 | +* If a struct has a `new` method that returns a `Result`, then this is a private struct |
| 97 | +* Every field of a private struct must be private (not `pub`) to enforce validation |
| 98 | +* A private struct must always implement `TryFrom` instead of `From` (must never implement `From`) to enforce validation |
| 99 | +* A private struct that has `#[derive(Deserialize)]` must always use `#[serde(try_from = ...)]` to enforce validation during deserialization |
| 100 | +* A private struct should not implement `Default` in most cases (very rarely it may implement `Default` only if the default value is a valid value) |
| 101 | +* The code must always call the `new` method to enforce validation |
7 | 102 |
|
8 | | -## Commands |
| 103 | +## Newtypes |
9 | 104 |
|
10 | | -Always use `mise run ...` commands to run the tests / lints. |
| 105 | +* The macro calls that begin with `subtype` (for example, `subtype!` and `subtype_string!`) expand to newtypes |
11 | 106 |
|
12 | | -* Run tests: `mise run test` (use this instead of `cargo test`) |
13 | | -* Run specific test: `mise run test <test_file_path>` (use this instead of `cargo test`) |
14 | | -* Format code: `mise run fmt` (use this instead of `cargo fmt`) |
15 | | -* Lint code: `mise run lint` (use this instead of `cargo clippy`) |
16 | | -* Check types: `mise run check` (use this instead of `cargo check`) |
| 107 | +## Code style |
17 | 108 |
|
18 | | -Always execute `mise run fmt` after completing your task. |
| 109 | +* The file names must match the names of the primary item in this file (for example: a file with `struct User` must be in `user.rs`) |
| 110 | +* Don't use `mod.rs`, use module files with submodules in the folder with the same name (for example: `user.rs` with submodules in `user` folder) |
| 111 | +* Put the trait implementations in the same file as the target struct (for example: put `impl TryFrom<...> for User` in the same file as `struct User`, which is `user.rs`) |
| 112 | +* Use destructuring assignment for tuple arguments, for example: `fn try_from((name, parent_key): (&str, GroupKey)) -> ...` |
| 113 | +* Add a local `use` statement for enums to minimize the code size. For example: |
| 114 | + * Good: |
| 115 | + ```rust |
| 116 | + pub fn apply(op: GroupsOp) { |
| 117 | + use GroupsOp::*; |
| 118 | + match op { |
| 119 | + InsertOne(_) => {} |
| 120 | + UpdateOne(_, _) => {} |
| 121 | + DeleteOne(_) => {} |
| 122 | + } |
| 123 | + } |
| 124 | + ``` |
| 125 | + * Bad: |
| 126 | + ```rust |
| 127 | + pub fn apply(op: GroupsOp) { |
| 128 | + match op { |
| 129 | + GroupsOp::InsertOne(_) => {} |
| 130 | + GroupsOp::UpdateOne(_, _) => {} |
| 131 | + GroupsOp::DeleteOne(_) => {} |
| 132 | + } |
| 133 | + } |
| 134 | + ``` |
| 135 | +* Simplify the callsite code by accepting `impl Into`. For example: |
| 136 | + * Good: |
| 137 | + ```rust |
| 138 | + pub fn foo(input: impl Into<String>) { |
| 139 | + let input = input.into(); |
| 140 | + // do something |
| 141 | + } |
| 142 | + ``` |
| 143 | + * Bad: |
| 144 | + ```rust |
| 145 | + /// This is bad because the callsite may have to call .into() when passing the input argument |
| 146 | + pub fn foo(input: String) {} |
| 147 | + ``` |
| 148 | +* Provide additional flexibility for callsite by accepting `&impl AsRef` or `&mut impl AsMut` (e.g. both `PathBuf` and `Config` may implement `AsRef<Path>`). For example: |
| 149 | + * Good: |
| 150 | + ```rust |
| 151 | + pub fn bar(input: &mut impl AsMut<String>) { |
| 152 | + let input = input.as_mut(); |
| 153 | + // do something |
| 154 | + } |
| 155 | + |
| 156 | + pub fn baz(input: &impl AsRef<str>) { |
| 157 | + let input = input.as_ref(); |
| 158 | + // do something |
| 159 | + } |
| 160 | + ``` |
| 161 | + * Bad: |
| 162 | + ```rust |
| 163 | + /// This is bad because the callsite may have to call .as_mut() when passing the input argument |
| 164 | + pub fn bar(input: &mut String) {} |
| 165 | + |
| 166 | + /// This is bad because the callsite may have to call .as_ref() when passing the input argument |
| 167 | + pub fn baz(input: &str) {} |
| 168 | + ``` |
| 169 | +* Generalize fn signatures by accepting `impl IntoIterator` instead of slice or `Vec`. For example: |
| 170 | + * Good: |
| 171 | + ```rust |
| 172 | + pub fn foo<'a>(inputs: impl IntoIterator<Item = &'a str>) { |
| 173 | + // do something |
| 174 | + } |
| 175 | + |
| 176 | + pub fn bar(inputs: impl IntoIterator<Item = String>) { |
| 177 | + // do something |
| 178 | + } |
| 179 | + ``` |
| 180 | + * Bad: |
| 181 | + ```rust |
| 182 | + /// This is bad because it is not general enough |
| 183 | + pub fn foo(inputs: &[str]) {} |
| 184 | + |
| 185 | + /// This is bad because it is not general enough and also forces the caller to collect the strings into a vec, which is bad for performance |
| 186 | + pub fn bar(inputs: impl IntoIterator<Item = String>) {} |
| 187 | + ``` |
| 188 | +* Write `macro_rules!` macros to reduce boilerplate |
| 189 | +* If you see similar code in different places, write a macro and replace the similar code with a macro call |
0 commit comments