diff --git a/.agents/skills/rust-best-practices/SKILL.md b/.agents/skills/rust-best-practices/SKILL.md new file mode 100644 index 00000000..f527c7f4 --- /dev/null +++ b/.agents/skills/rust-best-practices/SKILL.md @@ -0,0 +1,94 @@ +--- +name: rust-best-practices +description: > + Guide for writing idiomatic Rust code based on Apollo GraphQL's best practices handbook. Use this skill when: + (1) writing new Rust code or functions, + (2) reviewing or refactoring existing Rust code, + (3) deciding between borrowing vs cloning or ownership patterns, + (4) implementing error handling with Result types, + (5) optimizing Rust code for performance, + (6) writing tests or documentation for Rust projects. +license: MIT +compatibility: Rust 1.70+, Cargo +metadata: + author: apollographql + version: "1.1.0" +allowed-tools: Bash(cargo:*) Bash(rustc:*) Bash(rustfmt:*) Bash(clippy:*) Read Write Edit Glob Grep +--- + +# Rust Best Practices + +Apply these guidelines when writing or reviewing Rust code. Based on Apollo GraphQL's [Rust Best Practices Handbook](https://github.com/apollographql/rust-best-practices). + +## Best Practices Reference + +Before reviewing, familiarize yourself with Apollo's Rust best practices. Read ALL relevant chapters in the same turn in parallel. Reference these files when providing feedback: + +- [Chapter 1 - Coding Styles and Idioms](references/chapter_01.md): Borrowing vs cloning, Copy trait, Option/Result handling, iterators, comments +- [Chapter 2 - Clippy and Linting](references/chapter_02.md): Clippy configuration, important lints, workspace lint setup +- [Chapter 3 - Performance Mindset](references/chapter_03.md): Profiling, avoiding redundant clones, stack vs heap, zero-cost abstractions +- [Chapter 4 - Error Handling](references/chapter_04.md): Result vs panic, thiserror vs anyhow, error hierarchies +- [Chapter 5 - Automated Testing](references/chapter_05.md): Test naming, one assertion per test, snapshot testing +- [Chapter 6 - Generics and Dispatch](references/chapter_06.md): Static vs dynamic dispatch, trait objects +- [Chapter 7 - Type State Pattern](references/chapter_07.md): Compile-time state safety, when to use it +- [Chapter 8 - Comments vs Documentation](references/chapter_08.md): When to comment, doc comments, rustdoc +- [Chapter 9 - Understanding Pointers](references/chapter_09.md): Thread safety, Send/Sync, pointer types + +## Quick Reference + +### Borrowing & Ownership +- Prefer `&T` over `.clone()` unless ownership transfer is required +- Use `&str` over `String`, `&[T]` over `Vec` in function parameters +- Small `Copy` types (≀24 bytes) can be passed by value +- Use `Cow<'_, T>` when ownership is ambiguous + +### Error Handling +- Return `Result` for fallible operations; avoid `panic!` in production +- Never use `unwrap()`/`expect()` outside tests +- Use `thiserror` for library errors, `anyhow` for binaries only +- Prefer `?` operator over match chains for error propagation + +### Performance +- Always benchmark with `--release` flag +- Run `cargo clippy -- -D clippy::perf` for performance hints +- Avoid cloning in loops; use `.iter()` instead of `.into_iter()` for Copy types +- Prefer iterators over manual loops; avoid intermediate `.collect()` calls + +### Linting +Run regularly: `cargo clippy --all-targets --all-features --locked -- -D warnings` + +Key lints to watch: +- `redundant_clone` - unnecessary cloning +- `large_enum_variant` - oversized variants (consider boxing) +- `needless_collect` - premature collection + +Use `#[expect(clippy::lint)]` over `#[allow(...)]` with justification comment. + +### Testing +- Name tests descriptively: `process_should_return_error_when_input_empty()` +- One assertion per test when possible +- Use doc tests (`///`) for public API examples +- Consider `cargo insta` for snapshot testing generated output + +### Generics & Dispatch +- Prefer generics (static dispatch) for performance-critical code +- Use `dyn Trait` only when heterogeneous collections are needed +- Box at API boundaries, not internally + +### Type State Pattern +Encode valid states in the type system to catch invalid operations at compile time: +```rust +struct Connection { /* ... */ _state: PhantomData } +struct Disconnected; +struct Connected; + +impl Connection { + fn send(&self, data: &[u8]) { /* only connected can send */ } +} +``` + +### Documentation +- `//` comments explain *why* (safety, workarounds, design rationale) +- `///` doc comments explain *what* and *how* for public APIs +- Every `TODO` needs a linked issue: `// TODO(#42): ...` +- Enable `#![deny(missing_docs)]` for libraries diff --git a/.agents/skills/rust-best-practices/references/chapter_01.md b/.agents/skills/rust-best-practices/references/chapter_01.md new file mode 100644 index 00000000..d41bfd09 --- /dev/null +++ b/.agents/skills/rust-best-practices/references/chapter_01.md @@ -0,0 +1,552 @@ +# Chapter 1 - Coding Styles and Idioms + +## 1.1 Borrowing Over Cloning + +Rust's ownership system encourages **borrow** (`&T`) instead of **cloning** (`T.clone()`). +> ❗ Performance recommendation + +### βœ… When to `Clone`: + +* You need to change the object AND preserve the original object (immutable snapshots). +* When you have `Arc` or `Rc` pointers. +* When data is shared across threads, usually `Arc`. +* Avoid massive refactoring of non performance critical code. +* When caching results (dummy example below): +```rust +fn get_config(&self) -> Config { + self.cached_config.clone() +} +``` +* When the underlying API expects Owned Data. + +### 🚨 `Clone` traps to avoid: + +* Auto-cloning inside loops `.map(|x| x.clone)`, prefer to call `.cloned()` or `.copied()` at the end of the iterator. +* Cloning large data structures like `Vec` or `HashMap`. +* Clone because of bad API design instead of adjusting lifetimes. +* Prefer `&[T]` instead of `Vec` or `&Vec`. +* Prefer `&str` or `&String` instead of `String`. +* Prefer `&T` instead of `T`. +* Clone a reference argument, if you need ownership, make it explicit in the arguments for the caller. Example: +```rust +fn take_a_borrow(thing: &Thing) { + let thing_cloned = thing.clone(); // the caller should have passed ownership instead +} +``` + +### βœ… Prefer borrowing: +```rust +fn process(name: &str) { + println!("Hello {name}"); +} + +let user = String::from("foo"); +process(&user); +``` + +### ❌ Avoid redundant cloning: +```rust +fn process_string(name: String) { + println!("Hello {name}"); +} + +let user = String::from("foo"); +process(user.clone()); // Unnecessary clone +``` + +## 1.2 When to pass by value? (Copy trait) + +Not all types should be passed by reference (`&T`). If a type is **small** and it is **cheap to copy**, it is often better to **pass it by value**. Rust makes it explicit via the `Copy` trait. + +### βœ… When to pass by value, `Copy`: +* The type **implements** `Copy` (`u32`, `bool`, `f32`, small structs). +* The cost of moving the value is negligible. + +```rust +fn increment(x: u32) -> u32 { + x + 1 +} + +let num = 1; +let new_num = increment(num); // `num` still usable after this point +``` + +### ❓ Which structs should be `Copy`? +* When to consider declaring `Copy` on your own types: +* All fields are `Copy` themselves. +* The struct is `small`, up to 2 (maybe 3) words of memory or 24 bytes (each word is 64 bits/8bytes). +* The struct **represents a "plain data object"**, without resourcing to ownership (no heap allocations. Example: `Vec` and `Strings`). + +❗**Rust Arrays are stack allocated.** Which means they can be copied if their underlying type is `Copy`, but this will be allocated in the program stack which can easily become a stack overflow. More on [Chapter 3 - Stack vs Heap](./chapter_03.md#33-stack-vs-heap-be-size-smart) + +For reference, each primitive type size in bytes: + +#### Integers: + +| Type | Size | +|------------- |---------- | +| i8 u8 | 1 byte | +| i16 u16 | 2 bytes | +| i32 u32 | 4 bytes | +| i64 u64 | 8 bytes | +| isize usize | Arch | +| i128 u128 | 16 bytes | + +#### Floating Point: + +| Type | Size | +|---------- |---------- | +| f32 | 4 bytes | +| f64 | 8 bytes | + + +#### Other: + +| Type | Size | +|---------- |---------- | +| bool | 1 byte | +| char | 4 bytes | + + +### βœ… Good struct to derive `Copy`: +```rust +#[derive(Debug, Copy, Clone)] +struct Point { + x: f32, + y: f32, + z: f32 +} +``` + +### ❌ Bad struct to derive `Copy`: +```rust +#[derive(Debug, Clone)] +struct BadIdea { + age: i32, + name: String, // String is not `Copy` +} +``` + +### ❓Which Enums should be `Copy`? +* If your enum acts like tags and atoms. +* The enum payloads are all `Copy`. +* **❗Enums size are based on their largest element.** + +### βœ… Good Enum to derive +```rust +#[derive(Debug, Copy, Clone)] +enum Direction { + North, + South, + East, + West, +} +``` + +## 1.3 Handling `Option` and `Result` +Rust 1.65 introduced a better way to safely unpack Option and Result types with the `let Some(x) = … else { … }` or `let Ok(x) = … else { … }` when you have a default `return` value, `continue` or `break` default else case. It allows early returns when the missing case is **expected and normal**, not exceptional. + +### βœ… Cases to use each pattern matching for Option and Return +* Use `match` when you want to pattern match against the inner types `T` and `E` +```rust +match self { + Ok(Direction::South) => { … }, + Ok(Direction::North) => { … }, + Ok(Direction::East) => { … }, + Ok(Direction::West) => { … }, + Err(E::One) => { … }, + Err(E::Two) => { … }, +} + +match self { + Some(3|5) => { … } + Some(x) if x > 10 => { … } + Some(x) => { … } + None => { … } +} +``` + +* Use `match` when your type is transformed into something more complex Like `Result` becoming `Result, E>`. +```rust +match self { + Ok(t) => Ok(Some(t)), + Err(E::Empty) => Ok(None), + Err(err) => Err(err), +} +``` + +* Use `let PATTERN = EXPRESSION else { DIVERGING_CODE; }` when the divergent code doesn't need to know about the failed pattern matches or doesn't need extra computation: +```rust +let Some(&Direction::North) = self.direction.as_ref() else { + return Err(DirectionNotAvailable(self.direction)); +} +``` + +* Use `let PATTERN = EXPRESSION else { DIVERGING_CODE; }` when you want to break or continue a pattern match +```rust +for x in self { + let Some(x) = x else { + continue; + } +} +``` + +* Use `if let PATTERN = EXPRESSION else { DIVERGING_CODE; }` when `DIVERGING_CODE` needs extra computation: +```rust +if let Some(x) = self.next() { + // computation +} else { + // computation when `None/Err` or not matched +} +``` + +❗**If you don't care about the value of the `Err` case, please use `?` to propagate the `Err` to the caller.** + +### ❌ Bad Option/Return pattern matching: + +* Conversion between Result and Option (prefer `.ok()`,`.ok_or()`, and `ok_or_else()`) +```rust +match self { + Ok(t) => Some(t), + Err(_) => None +} +``` + +* `if let PATTERN = EXPRESSION else { DIVERGING_CODE; }` when divergent code is a default or pre-computed value (prefer `let PATTERN = EXPRESSION else { DIVERGING_CODE; }`): +```rust +if let Some(values) = self.next() { + // computation + (Some(..), values) +} else { + (None, Vec::new()) +} +``` + +* Using `unwrap` or `expect` outside tests: +```rust +let port = config.port.unwrap(); +``` + +## 1.4 Prevent Early Allocation + +When dealing with functions like `or`, `map_or`, `unwrap_or`, `ok_or`, consider that they have special cases for when memory allocation is required, like creating a new string, creating a collection or even calling functions that manage some state, so they can be replaced with their `_else` counter-part: + +### βœ… Good cases + +```rust +let x = None; +assert_eq!(x.ok_or(ParseError::ValueAbsent), Err(ParseError::ValueAbsent)); + +let x = None; +assert_eq!(x.ok_or_else(|| ParseError::ValueAbsent(format!("this is a value {x}"))), Err(ParseError::ValueAbsent)); + + +let x: Result<_, &str> = Ok("foo"); +assert_eq!(x.map_or(42, |v| v.len()), 3); + + +let x : Result<_, String> = Ok("foo"); +assert_eq!(x.map_or_else(|e|format!("Error: {e}"), |v| v.len()), 3); + +let x = "1,2,3,4"; +assert_eq!(x.parse_to_option_vec.unwrap_or_else(Vec::new), Ok(vec![1, 2, 3, 4])); +``` + +### ❌ Bad cases + +```rust +let x : Result<_, String> = Ok("foo"); +assert_eq!(x.map_or(format!("Error with uninformed content"), |v| v.len()), 3); + +let x = "1,2,3,4"; +assert_eq!(x.parse_to_option_vec.unwrap_or(Vec::new()), Ok(vec![1, 2, 3, 4])); // could be replaced with `.unwrap_or_default` + +let x = None; +assert_eq!(x.ok_or(ParseError::ValueAbsent(format!("this is a value {x}"))), Err(ParseError::ValueAbsent)); +``` + +### Mapping Err + +When dealing with Result::Err, sometimes is necessary to log and transform the Err into a more abstract or more detailed error, this can be done with `inspect_err` and `map_err`: + +```rust +let x = Err(ParseError::InvalidContent(...)); + +x + .inspect_err(|err| tracing::error!("function_name: {err}")) + .map_err(|err| GeneralError::from(("function_name", err)))?; +``` + +## 1.5 Iterator, `.iter` vs `for` + +First we need to understand a basic loop with each one of them. Let's consider the following problem, we need to sum all even numbers between 0 and 10 incremented by 1: + +* `for`: +```rust +let mut sum = 0; +for x in 0..=10 { + if x % 2 == 0 { + sum += x + 1; + } +} +``` + +* `iter`: +```rust +let sum: i32 = (0..=10) + .filter(|x| x % 2 == 0) + .map(|x| x + 1) + .sum(); +``` + +> Both versions do the same thing and are correct and idiomatic, but each shines in different contexts. + +### When to prefer `for` loops +* When you need **early exits** (`break`, `continue`, `return`). +* **Simple iteration** with side-effects (e.g., logging, IO) + * logging can be done correctly in `Iterators` using `inspect` and `inspect_err` functions. +* When readability matters more than simplicity or chaining. + +#### Example: +```rust +for value in &mut value { + if *value == 0 { + break; + } + *value += fancy_equation(); +} +``` + +### When to prefer `iterators` loops (`.iter()` and `.into_iter()`) +* When you are `transforming collections` or `Option/Results`. +* You can **compose multiple steps** elegantly. +* No need for early exits. +* You need support for indexed values with `.enumerate`. +```rust +let values: Vec<_> = vec.into_iter() + .enumerate() + .filter(|(_index, value)| value % 2 == 0) + .map(|(index, value)| value % index) + .collect() +``` +* You need to use collections functions like `.windows` or `chunks`. +* You need to combine data from multiple sources and don't want to allocate multiple collections. +* Iterators can be combined with `for` loops: +```rust +for value in vec.iter().enumerate() + .filter(|(index, value)| value % index == 0) { + // ... +} +``` + +> #### ❗REMEMBER: Iterators are Lazy +> +> * `.iter`, `.map`, `.filter` don't do anything until you call its consumer, e.g. `.collect`, `.sum`, `.for_each`. +> * **Lazy Evaluation** means that iterator chains are fused into one loop at compile time. + +### 🚨 Anti-patterns to AVOID + +* Don't chain without formatting. Prefer each chained function on its own line with the correct indentation (`rustfmt` should take care of this). +* Don't chain if it makes the code unreadable. +* Avoid needlessly collect/allocate of a collection (e.g. vector) just to throw it away later by some larger operation or by another iteration. +* Prefer `iter` over `into_iter` unless you don't need the ownership of the collection. +* Prefer `iter` over `into_iter` for collections that inner type implements `Copy`, e.g. `Vec`. +* For summing numbers prefer `.sum` over `.fold`. `.sum` is specialized for summing values, so the compiler knows it can make optimizations on that front, while fold has a blackbox closure that needs to be applied at every step. If you need to sum by an initial value, just added in the expression `let my_sum = [1, 2, 3].sum() + 3`. + +## 1.6 Comments: Context, not Clutter + +> "Context are for why, not what or how" + +Well-written Rust code, with expressive types and good naming, often speaks for itself. Many high-quality codebases thrive on **few or no comments**. And that's a good thing. + +Still, there are **moments where code alone isn't enough** - when there are performance quirks, external constraints, or non-obvious tradeoffs that require a nudge to the reader. In those cases, a concise comment can prevent hours of head-scratching or searching git history. + +### βœ… Good comments + +* Safety concerns: +```rust +// SAFETY: We have checked that the pointer is valid and non-null. @Function xyz. +unsafe { std::ptr::copy_nonoverlapping(src, dst, len); } +``` + +* Performance quirks: +```rust +// This algorithm is a fast square root approximation +const THREE_HALVES: f32 = 1.5; +fn q_rsqrt(number: f32 ) -> f32 { + let mut i: i32 = number.to_bits() as i32; + i = 0x5F375A86_i32.wrapping_sub(i >> 1); + let y = f32::from_bits(i as u32); + y * (THREE_HALVES - (number * 0.5 * y * y)) +} +``` + +* Clear code beats comments. However, when the why isn't obvious, say it plainly - or link to where: +```rust +// PERF: Generating the root store per subgraph caused high TLS startup latency on MacOS +// This works as a caching alternative. See: [ADR-123](link/to/adr-123) +let subgraph_tls_root_store: RootCertStore = configuration + .tls + .subgraph + .all + .create_certificate_store() + .transpose()? + .unwrap_or_else(crate::services::http::HttpClientService::native_roots_store); +``` + +### ❌ Bad comments + +* Wall-of-text explanations: long comments and multiline comments +```rust +// Lorem Ipsum is simply dummy text of the printing and typesetting industry. +// Lorem Ipsum has been the industry's standard dummy text ever since the 1500s, +// when an unknown printer took a galley +fn do_something_odd() { + … +} +``` +> Prefer `/// doc` comment if it's describing the function. + +* Comments that could be better represented as functions or are plain obvious +```rust +fn computation() { + // increment i by 1 + i += 1; +} +``` + +### βœ… Breaking up long functions over commenting them + +If you find yourself writing a long comment explaining "what", "how" or "each step" in a function, it might be time to split it. So the suggestion is to refactor. This can be beneficial not only for readability, but testability: + +#### ❌ Instead of: +```rust +fn process_request(request: T) { + // We first need to validate request, because of corner case x, y, z + // As the payload can only be decoded when they are valid + // Then we can perform authorization on the payload + // lastly with the authorized payload we can dispatch to handler +} +``` + +#### βœ… Prefer +```rust +fn process_request(request: T) -> Result<(), Error> { + validate_request_headers(&request)?; + let payload = decode_payload(&request); + authorize(&payload)?; + dispatch_to_handler(payload) +} + +#[cfg(test)] +mod tests { + #[test] + fn validate_request_happy_path() { ... } + + #[test] + fn validate_request_fails_on_x() { ... } + + #[test] + fn validate_request_fails_on_y() { ... } + + #[test] + fn decode_validated_request() { ... } + + #[test] + fn authorize_payload_xyz() { ... } +} +``` + +Let **structure** and **naming** replace commentary, and enhance its documentation with **tests as living documentation**. + +### πŸ“ TODOs are not comments - track them properly + +Avoid leaving lingering `// TODO: Lorem Ipsum` comments in the code. Instead: +* Turn them into Jira or Github Issues. +* If needed, to avoid future confusion, reference the issue in the code and the code in the issue. + +```rust +// See issue #123: support hyper 2.0 +``` + +This helps keeping the code clean and making sure tasks are not forgotten. + +### Comments as Living Documentation + +There are a few gotchas when calling comments "living documentation": +* Code evolves. +* Context changes. +* Comments get stale. +* Many large comments make people avoid reading them. +* Team becomes fearful of delete irrelevant comments. + +If you find a comment, **don't trust it blindly**. Read it in context. If it's wrong or outdated, fix or remove it. A misleading comment is worse than no comments at all. + +> Comments should bother you - they demand re-verification, just like stale tests. + +When deeper justification is needed, prefer to: +* **Link to a Design Doc or an ADR**, business logic lives well in design docs while performance tradeoffs live well in ADRs. +* Move runtime example and usage docs into Rust Docs, `/// doc comment`, where they can be tested and kept up-to-date by tools like `cargo doc`. + +> Doc-comments and Doc-testing, `///` and `//!` in [Chapter 8 - Comments vs Documentation](./chapter_08.md) + +## 1.7 Use Declarations - "imports" + +Different languages have different ways of sorting their imports, in the Rust ecosystem the [standard way](https://github.com/rust-lang/rustfmt/issues/4107) is: + +- `std` (`core`, `alloc` would also fit here). +- External crates (what is in your Cargo.toml `[dependencies]`). +- Workspace crates (workspace member crates). +- This module `super::`. +- This module `crate::`. + +```rust +// std +use std::sync::Arc; + +// external crates +use chrono::Utc; +use juniper::{FieldError, FieldResult}; +use uuid::Uuid; + +// crate code lives in workspace +use broker::database::PooledConnection; + +// super:: / crate:: +use super::schema::{Context, Payload}; +use super::update::convert_publish_payload; +use crate::models::Event; +``` + +Some enterprise solutions opt to include their core packages after `std`, so all external packages that start with enterprise name are located before the others: + +```rust +// std +use std::sync::Arc; + +// enterprise external crates +use enterprise_crate_name::some_module::SomeThing; + +// external crates +use chrono::Utc; +use juniper::{FieldError, FieldResult}; +use uuid::Uuid; + +// crate code lives in workspace +use broker::database::PooledConnection; + +// super:: / crate:: +use super::schema::{Context, Payload}; +use super::update::convert_publish_payload; +use crate::models::Event; +``` + +One way of not having to manually control this is using the following arguments in your `rustfmt.toml`: + +```toml +reorder_imports = true +imports_granularity = "Crate" +group_imports = "StdExternalCrate" +``` + +> As of Rust version 1.88, it is necessary to execute rustfmt in nightly to correctly reorder code `cargo +nightly fmt`. diff --git a/.agents/skills/rust-best-practices/references/chapter_02.md b/.agents/skills/rust-best-practices/references/chapter_02.md new file mode 100644 index 00000000..306b5835 --- /dev/null +++ b/.agents/skills/rust-best-practices/references/chapter_02.md @@ -0,0 +1,117 @@ +# Chapter 2 - Clippy and Linting Discipline + +Be sure to have `cargo clippy` installed with your rust compiler, run `cargo clippy -V` in your terminal for a rust project and you should get something like this `clippy 0.1.86 (05f9846f89 2025-03-31)`. If terminal fails to show a clippy version, please run the following code `rustup update && rustup component add clippy`. + +Clippy documentation can be found [here](https://doc.rust-lang.org/clippy/usage.html). + +## 2.1 Why care about linting? + +Rust compiler is a powerful tool that catches many mistakes. However, some more in-depth analysis require extra tools, that is where `cargo clippy` clippy comes into to play. Clippy checks for: +* Performance pitfalls. +* Style issues. +* Redundant code. +* Potential bugs. +* Non-idiomatic Rust. + +## 2.2 Always run `cargo clippy` + +Add the following to your daily workflow: + +```shell +$ cargo clippy --all-targets --all-features --locked -- -D warnings +``` + +* `--all-targets`: checks library, tests, benches and examples. +* `--all-features`: checks code for all features enabled, auto solves conflicting features. +* `--locked`: Requires `Cargo.lock` to be up-to-date, can be solved with `$ cargo update`. +* `-D warnings`: treats warnings as errors + +Potential additions elements to add: + +* `-- -W clippy::pedantic`: lints which are rather strict or have occasional false positives. +* `-- -W clippy::nursery`: Optionally can be added to check for new lints that are still under development. +* ❗ Add this to your Makefile, Justfile, xtask or CI Pipeline. + +> Example at ApolloGraphQL +> +> In the `Router` project there is a `xtask` configured for linting that can be executed with `cargo xtask lint`. + +## 2.3 Important Clippy Lints to Respect + +| Lint Name | Why | Link | +| --------- | ----| -----| +| `redundant_clone` | Detects unnecessary `clones`, has performance impact | [link (nursery + perf)](https://rust-lang.github.io/rust-clippy/master/#redundant_clone) | +| `needless_borrow` group | Removes redundant `&` borrowing | [link (style)](https://rust-lang.github.io/rust-clippy/master/#needless_borrow) | +| `map_unwrap_or` / `map_or` | Simplifies nested `Option/Result` handling | [`map_unwrap_or`](https://rust-lang.github.io/rust-clippy/master/#map_unwrap_or) [`unnecessary_map_or`](https://rust-lang.github.io/rust-clippy/master/#unnecessary_map_or) [`unnecessary_result_map_or_else`](https://rust-lang.github.io/rust-clippy/master/#unnecessary_result_map_or_else) | +| `manual_ok_or` | Suggest using `.ok_or_else` instead of `match` | [link (style)](https://rust-lang.github.io/rust-clippy/master/#manual_ok_or) | +| `large_enum_variant` | Warns if an enum has very large variant which is bad for memory. Suggests `Boxing` it | [link (perf)](https://rust-lang.github.io/rust-clippy/master/#large_enum_variant) | +| `unnecessary_wraps` | If your function always returns `Some` or `Ok`, you don't need `Option`/`Result` | [link (pedantic)](https://rust-lang.github.io/rust-clippy/master/#unnecessary_wraps) | +| `clone_on_copy` | Catches accidental `.clone()` on `Copy` types like `u32` and `bool` | [link (complexity)](https://rust-lang.github.io/rust-clippy/master/#clone_on_copy) | +| `needless_collect` | Prevents collecting and allocating an iterator, when allocation is not needed | [link (nursery)](https://rust-lang.github.io/rust-clippy/master/#needless_collect) | + +## 2.4 Fix warnings, don't silence them! + +**NEVER** just `#[allow(clippy::lint_something)]` unless: + +* You **truly understand** why the warning happens and you have a reason why it is better that way. +* You **document** why it is being ignored. +* ❗ Don't use `allow`, but `expect`, it will give a warning in case the lint is not true anymore, `#[expect(clippy::lint_something)]`. + +### Example: + +```rust +// Faster matching is preferred over size efficiency +#[expect(clippy::large_enum_variant)] +enum Message { + Code(u8), + Content([u8; 1024]), +} +``` + +> The fix would be: +> +> ```rust +> // Faster matching is preferred over size efficiency +> #[expect(clippy::large_enum_variant)] +> enum Message { +> Code(u8), +> Content(Box<[u8; 1024]>), +> } +> ``` + +### Handling false positives + +Sometimes Clippy complains even when your code is correct, in those cases there are two solutions: +1. Try to refactor the code, so it improves the warning. +2. **Locally** override the lint with `#[expect(clippy::lint_name)]` and a comment with the reason. +3. Avoid global overrides, unless it is core crate issue, a good example of this is the Bevy Engine that has a set of lints that should be allowed by default. + +## 2.5 Configure workspace/package lints + +In your `Cargo.toml` file it is possible to determine which lints and their priorities over each other. In case of 2 or more conflicting lints, the higher priority one will be chosen. Example configuration for a package: + +```toml +[lints.rust] +future-incompatible = "warn" +nonstandard_style = "deny" + +[lints.clippy] +all = { level = "deny", priority = 10 } +redundant_clone = { level = "deny", priority = 9 } +manual_while_let_some = { level = "deny", priority = 4 } +pedantic = { level = "warn", priority = 3 } +``` + +And for a workspace: + +```toml +[workspace.lints.rust] +future-incompatible = "warn" +nonstandard_style = "deny" + +[workspace.lints.clippy] +all = { level = "deny", priority = 10 } +redundant_clone = { level = "deny", priority = 9 } +manual_while_let_some = { level = "deny", priority = 4 } +pedantic = { level = "warn", priority = 3 } +``` diff --git a/.agents/skills/rust-best-practices/references/chapter_03.md b/.agents/skills/rust-best-practices/references/chapter_03.md new file mode 100644 index 00000000..074295e2 --- /dev/null +++ b/.agents/skills/rust-best-practices/references/chapter_03.md @@ -0,0 +1,209 @@ +# Chapter 3 - Performance Mindset + +The **golden rule** of performance work: + +> Don't guess, measure. + +Rust code is often already pretty fast - don't "optimize" without evidence. Optimize only after finding bottlenecks. + +### A good first steps +* Use `--release` flag on you builds (might sound dummy, but it is quite common to hear people complaining that their Rust code is slower than their X language code, and 99% of the time is because they didn't use the `--release` flag). +* `$ cargo clippy -- -D clippy::perf` gives you important tips on best practices for performance. +* [`cargo bench`](https://doc.rust-lang.org/cargo/commands/cargo-bench.html) is a cargo tool to create micro-benchmarks and test different code solutions. Write a test scenario and bench you solution against the original code, if your improvement is larger than 5%, might be a good performance improvement. +* [`cargo flamegraph`](https://github.com/flamegraph-rs/flamegraph) a powerful profiler for Rust code. For MacOS, [samply](https://github.com/mstange/samply) might be a better DX option. + +> #### Further reading on Benchmarking: +> - [How to build a Custom Benchmarking Harness in Rust](https://bencher.dev/learn/benchmarking/rust/custom-harness/) + + +## 3.1 Flamegraph + +Flamegraph helps you visualize how much time CPU spent on each task. + +```shell +# Installing flamegraph +cargo install flamegraph + +# cargo support provided through the cargo-flamegraph binary! +# defaults to profiling cargo run --release +cargo flamegraph + +# by default, `--release` profile is used, +# but you can override this: +cargo flamegraph --dev + +# if you'd like to profile a specific binary: +cargo flamegraph --bin=stress2 + +# Profile unit tests. +# Note that a separating `--` is necessary if `--unit-test` is the last flag. +cargo flamegraph --unit-test -- test::in::package::with::single::crate +cargo flamegraph --unit-test crate_name -- test::in::package::with::multiple:crate + +# Profile integration tests. +cargo flamegraph --test test_name + +# Run criterion benchmark +# Note that the last --bench is required for `criterion 0.3` to run in benchmark mode, instead of test mode. +cargo flamegraph --bench some_benchmark --features some_features -- --bench + +# Run workspace example +cargo flamegraph --example some_example --features some_features +``` + +> ❗ Always run your profiles with `--release` enabled, the `--dev` flag isn't realistic as it doesn't have optimizations enabled. + +The result will look like a flame graph where: + +* The `y-axis` shows the **stack depth number**. When looking at a flamegraph, the main function of your program will be closer to the bottom, and the called functions will be stacked on top, with the functions that they call stacked on top of them. + +* The `width of each box` shows the **total time that that function** is on the CPU or is part of the call stack. If a function's box is wider than others, that means that it consumes more CPU per execution than other functions, or that it is called more than other functions. + +> ❗ The **color of each box** isn't significant, and **is chosen at random**. + +### 🚨 Remember +* Thick stacks: heavy CPU usage +* Thin stacks: low intensity (cheap) + +## 3.2 Avoid Redundant Cloning + +> Cloning is cheap... **until it isn't** + +In sections [Borrowing over Cloning](./chapter_01.md#11-borrowing-over-cloning) and [Important Clippy lints to respect](./chapter_02.md#23-important-clippy-lints-to-respect) we mentioned the impacts of cloning and the relevant clippy lint [`redundant_clone`](https://rust-lang.github.io/rust-clippy/master/#redundant_clone), so in this section we will explore a bit "when to pass ownership". + +* 🚨 If you really need to clone, leave it to the last moment. + +### When to pass ownership? + +* Only `.clone()` if you truly need a new owned copy. A few examples: + * Crate API Design requires owned data. + * Have overloaded `std::ops` but still need ownership to the old data: + ```rust + use std::ops::Add; + + #[derive(Debug, Copy, Clone, PartialEq)] + struct Point { + x: i32, + y: i32, + } + + impl Add for Point { + type Output = Self; + + fn add(self, other: Self) -> Self { + Self { + x: self.x + other.x, + y: self.y + other.y, + } + } + } + + assert_eq!(Point { x: 1, y: 0 } + Point { x: 2, y: 3 }, + Point { x: 3, y: 3 }); + ``` + * Need to do comparison snapshots or due to API you need multiple owned instances of the data. + ```rust + fn snapshot(a: &MyValue, b:&MyValue) -> MyValueDiff { + a - b + } + + impl Sub for MyValue { + type Output = MyValueDiff; + + fn sub(self, other: Self) -> MyValue { + ... + } + } + + fn main() { + let mut a = MyValue::default(); + let b = a.clone(); + + a.magical_update(); + println!("{:?}", snapshot(&a, &b)); + } + ``` +* You have reference counted pointers (`Arc, Rc`). +* You have small structs that are to big to `Copy` but as costly as `std::collections`. An example is HTTP client like `hyper_util::client::legacy::Client` that cloning allows you to share the connection pool. +* You have a chained struct modifier that needs owned mutation, some **builders** require owned mutation, but most custom builders can be done with `pub fn with_xyz(&mut self, value: Xyz) -> &mut Self`. +```rust +// Inline `HashMap` insertion extension + +fn insert_owned(mut self, key: K, value: V) -> Self { + self.insert(key, value); + self +} +``` +* Ownership can also be a good way to model business logic / state. For example: +```rust +let not_validated: String = ...;// some user source +let validated = Validate::try_from(not_validated)?; +// Technically that `try_from` maybe didn't need ownership, but taking it lets us model intent +``` + +### When **NOT** to pass ownership? + +* Prefer API designs that take reference (`fn process(values: &[T])`), instead of ownership (`fn process(values: Vec)`). +* If you only need read access to elements, prefer `.iter` or slices: +```rust +for item in &some_vec { + ... +} +``` +* You need to mutate data that is owned by another thread, use `&mut MyStruct`. + +### Use `Cow` for `Maybe Owned` data + +Sometimes you don't actually need owned data, but that is not clear from the API perspective, so using [`std::borrow::Cow`](https://doc.rust-lang.org/std/borrow/enum.Cow.html) is a way to efficiently address this case: + +```rust +use std::borrow::Cow; + +fn hello_greet(name: Cow<'_, str>) { + println!("Hello {name}"); +} + +hello_greet(Cow::Borrowed("Julia")); +hello_greet(Cow::Owned("Naomi".to_string())); +``` + +## 3.3 Stack vs Heap: Be size-smart! + +### βœ… Good Practices + +* Keep small types (`impl Copy`, `usize`, `bool`, etc) **on the stack**. +* Avoid passing huge types (`> 512 bytes`) by value or transferring ownership. Prefer pass by reference (e.g. `&T` and `&mut T`). +* Heap allocate recursive data structures: +```rust +enum OctreeNode { + Node(T), + Children(Box<[Node; 8]>), +} +``` +* Return small types by value, types that implement `Copy` or a cheaply Cloned are efficient to return by value (e.g. `struct Vector2 {x: f32, y: f32}`). + +### ❗ Be Mindful + +* Only use `#[inline]` when benchmark proves beneficial, Rust is already pretty good at inlining **without** hints. +* Avoid massive stack allocations, box them. Example `let buffer: Box<[u8; 65536]> = Box::new(..)` would first allocate `[u8; 65536]` on the stack then box it, a non-const solution to this would be `let buffer: Box<[u8]> = vec![0; 65536].into_boxed_slice()`. +* For large `const` arrays, considering using [crate smallvec](https://docs.rs/smallvec/latest/smallvec/) as it behaves like an array, but is smart enough to allocate large arrays on the heap. + +## 3.4 Iterators and Zero-Cost Abstractions + +Rust iterators are lazy, but eventually compiled away into very efficient tight loops that are only called when consumed. Chaining `.filter()`, `.map()`, `.rev()`, `.skip()`, `.take()`, `.collect()` usually doesn't cost extra and the compiler can reason well enough how to optimize them. +* Prefer `iterators` over manual `for` loops when working with collections, the compiler can optimize them better than manually doing it. +* Calling `.iter()` only creates a **reference** to the original collection, this allows you to hold multiple iterators of the same collection. + +#### ❗ Avoid creating intermediate collections unless it is really needed: + +* Consider that `process` accepts an `iterator`. +* ❌ BAD - useless intermediate collection: +```rust +let doubled: Vec<_> = items.iter().map(|x| x * 2).collect(); +process(doubled); +``` +* βœ… GOOD - pass the iterator (`fn process(arg: impl Iterator)`): +```rust +let doubled_iter = items.iter().map(|x| x * 2); +process(doubled_iter); +``` diff --git a/.agents/skills/rust-best-practices/references/chapter_04.md b/.agents/skills/rust-best-practices/references/chapter_04.md new file mode 100644 index 00000000..ebca7119 --- /dev/null +++ b/.agents/skills/rust-best-practices/references/chapter_04.md @@ -0,0 +1,180 @@ +# Chapter 4 - Errors Handling + +Rust enforces a strict error handling approach, but *how* you handle them defines where your code feels ergonomic, consistent and safe - as opposing cryptic and painful. This chapter dives into best practices for modeling and managing fallible operations across libraries and binaries. + +> Even if you decide to crash you application with `unwrap` or `expect`, Rust forces you to declare that intentionally. + +## 4.1 Prefer `Result`, avoid panic 🫨 + +Rust has a powerful type that wraps fallible data, [`Result`](https://doc.rust-lang.org/std/result/), this allows us to handle Error cases according to our needs and manage the state of the application based on that. + +* If your function can fail, prefer to return a `Result`: +```rust +fn divide(x: f64, y: f64) -> Result { + if y == 0.0 { + Err(DivisionError::DividedByZero) + } else { + Ok(x / y) + } +} +``` + +* Use `panic!` only in unrecoverable conditions - typically tests, assertions, bugs or a need to crash the application for some explicit reason. +* There are 3 relevant macros that can replace `panic!` in appropriate conditions: + * `todo!`, similar to panic, but alerts the compiler that you are aware that there is code missing. + * `unreachable!`, you have reasoned about the code block and are sure that condition `xyz` is not possible and if ever becomes possible you want to be alerted. + * `unimplemented!`, specially useful for alerting that a block is not yet implement with a reason. + +## 4.2 Avoid `unwrap`/`expect` in Production + +Although `expect` is preferred to `unwrap`, as it can have context, they should be avoided in production code as there are smarter alternatives to them. Considering that, they should be used in the following scenarios: +- In tests, assertions or test helper functions. +- When failure is impossible. +- When the smarter options can't handle the specific case. + +### 🚨 Alternative ways of handling `unwrap`/`expect`: + +* If your `Result` (or `Option`) can have a predefined early return value in case of `Result::Err`, that doesn't need to know the `Err` value, use `let Ok(..) = else { return ... }` pattern, as it helps with flatten functions: +```rust +let Ok(json) = serde_json::from_str(&input) else { + return Err(MyError::InvalidJson); +} +``` +* If your `Result` (or `Option`) needs error recovery in case of `Result::Err`, that doesn't need to know the `Err` value, use `if let Ok(..) else { ... }` pattern: +```rust +if let Ok(json) = serde_json::from_str(&input) else { + ... +} else { + Err(do_something_with_input(&input)) +} +``` +* Functions that can have to handle `Option::None` values are recommended to return `Result`, where `E` is a crate or module level error, like the examples above. +* Lastly `unwrap_or`, `unwrap_or_else` or `unwrap_or_default`, these functions help you create alternative exits to unwrap that manage the uninitialized values. + +## 4.3 `thiserror` for Crate level errors + +Deriving Error manually is verbose and error prone, the rust ecosystem has a really good crate to help with this, `thiserror`. It allows you to create error types that easily implement `From` trait as well as easy error message (`Display`), improving developer experience while working seamlessly with `?` and integrating with `std::error::Error`: + +```rust +#[derive(Debug, thiserror::Error)] +pub enum MyError { + #[error("Network Timeout")] + Timeout, + #[error("Invalid data: {0}")] + InvalidData(String), + #[error(transparent)] + Serialization(#[from] serde_json::Error), + #[error("Invalid request information. Header: {headers}, Metadata: {metadata}")] + InvalidRequest { + headers: Headers, + metadata: Metadata + } +} +``` + +### Error Hierarchies and Wrapping + +For layered systems the best practice is to use nested `enum/struct` errors with `#[from]`: + +```rust +use crate::database::DbError; +use crate::external_services::ExternalHttpError; + +#[derive(Debug, thiserror::Error)] +pub enum ServiceError { + #[error("Database handler error: {0}")] + Db(#[from] DbError), + #[error("External services error: {0}")] + ExternalServices(#[from] ExternalHttpError) +} +``` + +## 4.4 Reserve `anyhow` for Binaries + +`anyhow` is an amazing crate, and quite useful for projects that are beginning and need accelerated speed. However, there is a turning point where it just painfully propagates through your code, considering this, `anyhow` is recommended only for **binaries**, where ergonomic error handling is needed and there is no need for precise error types: + +```rust +use anyhow::{Context, Result, anyhow}; + +fn main() -> Result<()> { + let content = std::fs::read_to_string("config.json") + .context("Failed to read config file")?; + Config::from_str(&content) + .map_err(|err| anyhow!("Config parsing error: {err}")) +} +``` + +### 🚨 `Anyhow` Gotchas + +* Keeping the `context` and `anyhow` strings up-to-date in all code base is harder than keeping `thiserror` messages as you don't have a single point of entry. +* `anyhow::Result` erases context that a caller might need, so avoid using it in a library. +* test helper functions can use `anyhow` with little to no issues. + +## 4.5 Use `?` to Bubble Errors + +Prefer using `?` over verbose alternatives like `match` chains: +```rust +fn handle_request(req: &Request) -> Result { + validate_headers(req)?; + validate_body_format(req)?; + validate_credentials(req)?; + let body = Body::try_from(req)?; + + Ok(ValidatedRequest::try_from((req, body))?) +} +``` + +> In case error recovery is needed, use `or_else`, `map_err`, `if let Ok(..) else`. To **inspect or log your error**, use `inspect_err`. + +## 4.6 Unit Test should exercise errors + +While many errors don't implement PartialEq and Eq, making it hard to do direct assertions between them, it is possible to check the error messages with `format!` or `to_string()`, making the errors meaningful and test validated: + +```rust +#[test] +fn error_does_not_implement_partial_eq() { + let err = divide(10., 0.0).unwrap_err(); + assert_eq!(err.to_string(), "division by zero"); +} + +#[test] +fn error_implements_partial_eq() { + let err = process(my_value).unwrap_err(); + + assert_eq!( + err, + MyError { + .. + } + ) +} +``` + +## 4.7 Important Topics + +### Custom Error Structs + +Sometimes you don't need an enum to handle your errors, as there is only one type of error that your module can have. This can be solved with `struct Errors`: + +```rust +#[derive(Debug, thiserror::Error, PartialEq)] +#[error("Request failed with code `{code}`: {message}")] +struct HttpError { + code: u16, + message: String +} +``` + +### Async Errors + +When using async runtimes, like Tokio, make sure that your errors implement `Send + Sync + 'static` where needed, specially in tasks or across `.await` boundaries: + +```rust +#[tokio::main] +async fn main() -> Result<(), Box> { + ... + Ok(()) +} +``` + +> Avoid `Box` in libraries unless it is really needed diff --git a/.agents/skills/rust-best-practices/references/chapter_05.md b/.agents/skills/rust-best-practices/references/chapter_05.md new file mode 100644 index 00000000..397fef5d --- /dev/null +++ b/.agents/skills/rust-best-practices/references/chapter_05.md @@ -0,0 +1,381 @@ +# Chapter 5 - Automated Testing + +> Tests are not just for correctness. They are the first place people look to understand how your code works. + +* Tests in rust are declared with the attribute macro `#[test]`. Most code editors can compile and run the functions declared under the macro individually or blocks of them. +* Test can have special compilation flags with `#[cfg(test)]`. Also executable in code editors if it contained `#[test]`, it is a good way to mock complicated functions or override traits. + +## 5.1 Tests as Living Documentation + +In Rust, as in many other languages, tests often show how the functions are meant to be used. If a test is clear and targeted, it's often more helpful than reading the function body, when combined with other tests, they serve as living documentation. + +### Use descriptive names + +> In the unit test name we should see the following: +> * `unit_of_work`: which *function* we are calling. The **action** that will be executed. This is often be the name of the the test `mod` where the function is being tested. +```rust +#[cfg(test)] +mod test { + mod function_name { + #[test] + fn returns_y_when_x() { ... } + } +} +``` +> * `expected_behavior`: the set of **assertions** that we need to verify that the test works. +> * `state_that_the_test_will_check`: the general **arrangement**, or setup, of the specific test case. + +#### ❌ Don't use a generic name for a test +```rust +#[test] +fn test_add_happy_path() { + assert_eq!(add(2, 2), 4); +} +``` +#### βœ… Use a name which reads like a sentence, describing the desired behavior +> Alternatively, if you function has too many tests, you can blob them together in a `mod`, it makes it easier to read and navigate. + +```rust +// OPTION 1 +#[test] +fn process_should_return_blob_when_larger_than_b() { + let a = setup_a_to_be_xyz(); + let b = Some(2); + let expected = MyExpectedStruct { ... }; + + let result = process(a, b).unwrap(); + + assert_eq!(result, expected); +} + +// OPTION 2 +mod process { + #[test] + fn should_return_blob_when_larger_than_b() { + let a = setup_a_to_be_xyz(); + let b = Some(2); + let expected = MyExpectedStruct { ... }; + + let result = process(a, b).unwrap(); + + assert_eq!(result, expected); + } +} +``` + +> When executing `cargo test` the test output for each option will look like: +> Option 1: `process_should_return_blob_when_larger_than_b`. +> Option 2: `process::should_return_blob_when_larger_than_b`. + +### Use modules for organization + +Most IDEs can run a single module of tests all together. +The test name in the output will also contain the name of the module. +Together, that means you can use the module name to group related tests together: + +```rust +#[cfg(test)] +mod test { // IDEs will provide a ▢️ button here + + mod process { + #[test] // IDEs will provide a ▢️ button here + fn returns_error_xyz_when_b_is_negative() { + let a = setup_a_to_be_xyz(); + let b = Some(-5); + let expected = MyError::Xyz; + + let result = process(a, b).unwrap_err(); + + assert_eq!(result, expected); + } + + #[test] // IDEs will provide a ▢️ button here + fn returns_invalid_input_error_when_a_and_b_not_present() { + let a = None; + let b = None; + let expected = MyError::InvalidInput; + + let result = process(a, b).unwrap_err(); + + assert_eq!(result, expected); + } + } +} +``` + +### Only test one behavior per function + +To keep tests clear, they should describe _one_ thing that the unit does. +This makes it easier to understand why a test is failing. + +#### ❌ Don't test multiple things in the same test +```rust +fn test_thing_parser(...) { + assert!(Thing::parse("abcd").is_ok()); + assert!(Thing::parse("ABCD").is_err()); +} +``` + +#### βœ… Test one thing per test +```rust +#[cfg(test)] +mod test_thing_parser { + #[test] + fn lowercase_letters_are_valid() { + assert!( + Thing::parse("abcd").is_ok(), + // Works like `eprintln`, `format` and `println` macros + "Thing parse error: {:?}", + Thing::parse("abcd").unwrap_err() + ); + } + + #[test] + fn capital_letters_are_invalid() { + assert!(Thing::parse("ABCD").is_err()); + } +} +``` + +> `Ok` scenarios should have an `eprintln` of the `Err` case. + +### Use very few, ideally one, assertion per test + +When there are multiple assertions per test, it's both harder to understand the intended behavior and +often requires many iterations to fix a broken test, as you work through assertions one by one. + +❌ Don't include many assertions in one test: + +```rust +#[test] +fn test_valid_inputs() { + assert!(the_function("a").is_ok()); + assert!(the_function("ab").is_ok()); + assert!(the_function("ba").is_ok()); + assert!(the_function("bab").is_ok()); +} +``` + +If you are testing separate behaviors, make multiple tests each with descriptive names. +To avoid boilerplate, either use a shared setup function or [rstest](https://crates.io/crates/rstest) cases *with descriptive test names*: +```rust +#[rstest] +#[case::single("a")] +#[case::first_letter("ab")] +#[case::last_letter("ba")] +#[case::in_the_middle("bab")] +fn the_function_accepts_all_strings_with_a(#[case] input: &str) { + assert!(the_function(input).is_ok()); +} +``` + +> Considerations when using `rstest` +> +> * It's harder for both IDEs and humans to run/locate specific tests. +> * Expectation vs condition naming is now visually inverted (expectation first). + +## 5.2 Add Test Examples to your Docs + +We will deep dive into docs at a later stage, so in this section we will just briefly go over how to add tests to you docs. Rustdoc can turn examples into executable tests using `///` with a few advantages: + +* These tests run with `cargo test` **BUT NOT** `cargo nextest run`. If using `nextest`, make sure to run `cargo t --doc` separately. +* They serve both as documentation and correctness checks, and are kept up to date by changes, due to the fact that the compiler checks them. +* No extra testing boilerplate. You can easily hide test sections by prefixing the line with `#`. +* ❗ There is no issue if you have test duplication between doc-tests and other non-public facing tests. + +```rust +/// Helper function that adds any two numeric values together. +/// This function reasons about which would be the correct type to parse based on the type +/// and the size of the numeric value. +/// +/// # Examples +/// +/// ```rust +/// # use crate_name::generic_add; +/// use num::numeric; +/// +/// # assert_eq!( +/// generic_add(5.2, 4) // => 9.2 +/// # , 9.2) +/// +/// # assert_eq!( +/// generic_add(2, 2.0) // => 4 +/// # , 4) +/// ``` +``` + +This documentation code would look like: +```rust +use num::numeric; + +generic_add(5.2, 4) // => 9.2 +generic_add(2, 2.0) // => 4 +``` + +## 5.3 Unit Test vs Integration Tests vs Doc tests + +As a general rule, without delving into *test pyramid naming*, rust has 3 sets of tests: + +### Unit Test + +Tests that go in the **same module** as the **tested unit** was declared, this allows the test runner to have visibility over private functions and parent `use` declarations. They can also consume `pub(crate)` functions from other modules if needed. Unit tests can be more focused on **implementation and edge-cases checks**. + +* They should be as simple as possible, testing one state and one behavior of the unit. KISS. +* They should test for errors and edge cases. +* Different tests of the same unit can be combined under a single `#[cfg(test)] mod test_unit_of_work {...}`, allowing multiple submodules for different `units_of_work`. +* Try to keep external states/side effects to your API to minimum and focus those tests on the `mod.rs` files. +* Tests that are not yet fully implemented can be ignored with the `#[ignore = "optional message"]` attribute. +* Tests that intentionally panic should be annotated with the attribute `#[should_panic]`. + +```rust +#[cfg(test)] +mod unit_of_work_tests { + use super::*; + + #[test] + fn unit_state_behavior() { + let expected = ...; + let result = ...; + assert_eq!(result, expected, "Failed because {}", result - expected); + } +} +``` + +### Integration Tests + +Tests that go under the `tests/` directory, they are entirely external to your library and use the same code as any other code would use, not have access to private and crate level functions, which means they can **only test** functions on your **public API**. + +> Their purpose is to test whether many parts of the code work together correctly, units of code that work correctly on their own could have problems when integrated. + +* Test for happy paths and common use cases. +* Allow external states and side effects, [testcontainers](https://rust.testcontainers.org/) might help. +* if testing binaries, try to break **executable** and **functions** into `src/main.rs` and `src/lib.rs`, respectively. + +``` +β”œβ”€β”€ Cargo.lock +β”œβ”€β”€ Cargo.toml +β”œβ”€β”€ src +β”‚ └── lib.rs +└── tests + β”œβ”€β”€ mod.rs + β”œβ”€β”€ common + β”‚ └── mod.rs + └── integration_test.rs +``` + +### Doc Testing + +As mentioned in section [5.2](#52-add-test-examples-to-your-docs), doc tests should have happy paths, general public API usage and more powerful attributes that improve documentation, like custom CSS for the code blocks. + +### Attributes: + +* `ignore`: tells rust to ignore the code, usually not recommended, if you want just a code formatted text, use `text`. +* `should_panic`: tells the rust compiler that this example block will panic. +* `no_run`: compiles but doesn't execute the code, similar to `cargo check`. Very useful when dealing with side-effects for documentation. +* `compile_fail`: Test rustdoc that this block should cause a compilation fail, important when you want to demonstrate wrong use cases. + +## 5.4 How to `assert!` + +Rust comes with 2 macros to make assertions: +* `assert!` for asserting boolean values like `assert!(value.is_ok(), "'value' is not Ok: {value:?}")` +* `assert_eq!` for checking equality between two different values, `assert_eq!(result, expected, "'result' differs from 'expected': {}", result.diff(expected))`. + +### 🚨 `assert!` reminders +* Rust asserts support formatted strings, like the previous examples, those strings will be printed in case of failure, so it is a good practice to add what the actual state was and how it differs from the expected. +* If you don't care about the exact pattern matching value, using `matches!` combined with `assert!` might be a good alternative. +```rust +assert!(matches!(error, MyError::BadInput(_), "Expected `BadInput`, found {error}")); +``` +* Use `#[should_panic]` wisely. It should only be used when panic is the desired behavior, prefer result instead of panic. +* There are some other that can enhance your testing experience like: + * [`rstest`](https://crates.io/crates/rstest): fixture based test framework with procedural macros. + * [`pretty_assertions`](https://crates.io/crates/pretty_assertions): overrides `assert_eq` and `assert_ne`, and creates colorful diffs between them. + +## 5.5 Snapshot Testing with `cargo insta` + +> When correctness is visual or structural, snapshots tell the story better than asserts. + +1. Add to your dependencies: +```toml +insta = { version = "1.42.2", features = ["yaml"] } +``` +> For most real world applications the recommendation is to use YAML snapshots of serializable values. This is because they look best under version control and the diff viewer and support redaction. To use this enable the yaml feature of insta. + +2. For a better review experience, add the CLI `cargo install cargo-insta`. + +3. Writing a simple test: +```rust +fn split_words(s: &str) -> Vec<&str> { + s.split_whitespace().collect() +} + +#[test] +fn test_split_words() { + let words = split_words("hello from the other side"); + insta::assert_yaml_snapshot!(words); +} +``` + +4. Run `cargo insta test` to execute, and `cargo insta review` to review conflicts. + +To learn more about `cargo insta` check out its [documentation](https://insta.rs/docs/quickstart/) as it is a very complete and well documented tool. + +### What is snapshot testing? + +Snapshot testing compares your output (text, Json, HTML, YAML, etc) against a saved "golden" version. On future runs, the test fails if the output changes, unless humanly approved. It is perfect for: +* Generate code. +* Serializing complex data. +* Rendered HTML. +* CLI output. + +#### ❌ What not to test with snapshot +* Very stable, numeric-only or small structured data associated logic (prefer `assert_eq!`). +* Critical path logic (prefer precise unit tests). +* Flaky tests, randomly generated output, unless redacted. +* Snapshots of external resources, use mocks and stubs. + +## 5.6 βœ… Snapshot Best Practices + +* Named snapshots, it gives meaningful snapshot files names, e.g. `snapshots/this_is_a_named_snapshot.snap` +```rust +assert_snapshot!("this_is_a_named_snapshot", output); +``` + +* Keep snapshots small and clear. +```rust +// βœ… Best case: +assert_snapshot!("app_config/http", whole_app_config.http); + +// ❌ Worst case: +assert_snapshot!("app_config", whole_app_config); // Huge object +``` + +> #### 🚨 Avoid snapshotting huge objects +> Huge objects become hard to review and reason about. + +* Avoid snapshotting simple types (primitives, flat enums, small structs): +```rust +// βœ… Better: +assert_eq!(meaning_of_life, 42); + +// ❌ OVERKILL: +assert_snapshot!("the_meaning_of_life", meaning_of_life); // meaning_of_life == 42 +``` + +* Use [redactions](https://insta.rs/docs/redactions/) for unstable fields (randomly generated, timestamps, uuid, etc): +```rust +use insta::assert_json_snapshot; + +#[test] +fn endpoint_get_user_data() { + let data = http::client.get_user_data(); + assert_json_snapshot!( + "endpoints/subroute/get_user_data", + data, + ".created_at" => "[timestamp]", + ".id" => "[uuid]" + ); +} +``` +* Commit your snapshots into git. They will be stored in `snapshots/` alongside your tests. +* Review changes carefully before accepting. diff --git a/.agents/skills/rust-best-practices/references/chapter_06.md b/.agents/skills/rust-best-practices/references/chapter_06.md new file mode 100644 index 00000000..2467e51c --- /dev/null +++ b/.agents/skills/rust-best-practices/references/chapter_06.md @@ -0,0 +1,161 @@ +# Chapter 6 - Generics, Dynamic Dispatch and Static Dispatch + +> Static where you can, dynamic where you must + +Rust allows you to handle polymorphic code in two ways: +* **Generics / Static Dispatch**: compile-time, monomorphized per use. +* **Trait Objects / Dynamic Dispatch**: runtime vtable, single implementation. + +Understanding the trade-offs lets you write faster, smaller and more flexible code. + +## 6.1 [Generics](https://doc.rust-lang.org/book/ch10-00-generics.html) + +Every programming language has tools for effectively handling the duplication of concepts. In Rust, one such tool is generics: abstract stand-ins for concrete types or other properties. We can express the behavior of generics or how they relate to other generics without knowing what will be in their place when compiling and running the code. + +We use generics to create definitions for items like function signatures or structs, which we can then use with many different concrete data types. Let's first look at how to define functions, structs, enums, and methods using generics. Generics can also be used to implement Type State Pattern and constrain a struct functionality to certain expected types, more on type state on [Chapter 7](./chapter_07.md). + +[Generics by Examples](https://doc.rust-lang.org/rust-by-example/generics.html). + +### Generics Performance + +You might be wondering whether there is a runtime cost when using generic type parameters. The good news is that using generic types won't make your program run any slower than it would with concrete types. Rust accomplishes this by performing monomorphization of the code using generics at compile time. Monomorphization is the process of turning generic code into specific code by filling in the concrete types that are used when compiled. The compiler checks for all occurrences of the generic parameter and generates code for the concrete types the generic code is called with. + +## 6.2 Static Dispatch: `impl Trait` or `` + +A static dispatch is basically a constrained version of a generics, a trait bounded generic, at compile-time it is able to check if your generic satisfies the declared traits. + +### βœ… Best when: +* You want **zero runtime cost**, by paying the compile time cost. +* You need **tight loops or performance**. +* Your types are **known at compile time**. +* Your are working with **single-use implementations** (monomorphized). + +### 🏎️ Example: High-performance function with generic +```rust +fn specialized_sum>(iter: U) -> T { + iter.map(|x| x.random_mapping()).sum() +} + +// or, equivalent, more modern +fn specialized_sum(iter: impl Iterator) -> T { + iter.map(|x| x.random_mapping()).sum() +} +``` + +This is compiled into **specialized machine code** for each usage, fast and inlined. + +## 6.3 Dynamic Dispatch: `dyn Trait` + +Usually dynamic dispatch is used with some kind of pointer or a reference, like `Box`, `Arc` or `&dyn trait`. + +### βœ… Best when: +* You absolutely need runtime polymorphism. +* You need to **store different implementations** in one collection. +* You want to **abstract internals behind a stable interface**. +* You are writing a **plugin-style architecture**. + +> ❗ Closer to what you would get in an object oriented language and can have some heavy costs associated to it. Can avoid generic entirely and let you mix types that implement the same traits. + +### 🚚 Example: Heterogeneous collection + +```rust +trait Animal { + fn greet(&self) -> String; +} + +struct Dog; +impl Animal for Dog { + fn greet(&self) -> String { + "woof".to_string() + } +} + +struct Cat; +impl Animal for Cat { + fn greet(&self) -> String { + "meow".to_string() + } +} + +fn all_animals_greeting(animals: Vec>) { + for animal in animals { + println!("{}", animal.greet()) + } +} +``` + +## 6.4 Trade-off summary + +| | Static Dispatch (impl Trait) | Dynamic Dispatch (dyn Trait) | +|------------------- |------------------------------ |---------------------------------- | +| Performance | βœ… Faster, inlined | ❌ Slower: vtable indirection | +| Compile time | ❌ Slower: monomorphization | βœ… Faster: shared code | +| Binary size | ❌ Larger: per-type codegen | βœ… Smaller | +| Flexibility | ❌ Rigid, one type at a time | βœ… Can mix types in collections | +| Use in trait fn() | ❌ Traits must be object-safe | βœ… Works with trait objects | +| Errors | βœ… Clearer | ❌ Erased types can confuse errors | + +* Prefer generics/static dispatch when you control the call site and want performance. +* Use dynamic dispatch when you need abstraction, plugins or mixed types. 🚨 Runtime cost. +* If you are not sure, start with generics, trait bound them - then use `Box` when flexibility outweighs speed. + +> Favor static dispatch until your trait needs to live behind a pointer. + +## 6.5 Best Practices for Dynamic Dispatch + +Dynamic dispatch `Ptr` is a powerful tool, but it also has significant performance trade-offs. You should only reach for it when **type erasure or runtime polymorphism** are essential. It is important to know when you need Trait Objects: + +### βœ… Use Dynamic Dispatch When: + +* You need heterogeneous types in a collection: +```rust +fn all_animals_greeting(animals: Vec>) { + for animal in animals { + println!("{}", animal.greet()) + } +} +``` + +* You want runtime plugins or hot-swappable components. +* You want to abstract internals from the caller (library design). + + +### ❌ Avoid Dynamic Dispatch When: + +* You control the concrete types. +* You are writing code in performance critical paths. +* You can express the same logic in other ways while keeping simplicity, e.g. generics. + +## 6.6 🚨 Trait Objects Ergonomics + +* Prefer `&dyn Trait` over `Box` when you don't need ownership. +* Use `Arc` for shared access across threads. +* Don't use `dyn Trait` if the trait has methods that return `Self`. +* **Avoid boxing too early**. Don't box inside structs unless you are sure it'll be beneficial or is required (recursive). +```rust +// βœ… Use generics when possible +struct Renderer { + backend: B +} + +// ❌ Premature Boxing +struct Renderer { + backend: Box // Boxing too early +} +``` +* If you must expose a `dyn trait` in a public API, `Box` at the boundary, not internally. +* **Object Safety**: You can only create `dyn Traits` from object-safe traits: + * It has **no generic methods**. + * It doesn't require `Self: Sized`. + * All method signatures use `&self`, `&mut self` or `self`. + ```rust + // βœ… Object Safe + trait Runnable { + fn run(&self); + } + + // ❌ Not Object Safe + trait Factory { + fn create() -> T; // generic methods are not allowed + } + ``` diff --git a/.agents/skills/rust-best-practices/references/chapter_07.md b/.agents/skills/rust-best-practices/references/chapter_07.md new file mode 100644 index 00000000..f7eab2c4 --- /dev/null +++ b/.agents/skills/rust-best-practices/references/chapter_07.md @@ -0,0 +1,226 @@ +# Chapter 7 - Type State Pattern + +Models state at compile time, preventing bugs by making illegal states unrepresentable. It takes advantage of the Rust generics and type system to create sub-types that can only be reached if a certain condition is achieved, making some operations illegal at compile time. + +> Recently it became the standard design pattern of Rust programming. However, it is not exclusive to Rust, as it is achievable and has inspired other languages to do the same [swift](https://swiftology.io/articles/typestate/) and [typescript](https://catchts.com/type-state). + +## 7.1 What is Type State Pattern? + +**Type State Pattern** is a design pattern where you encode different **states** of the system as **types**, not as runtime flags or enums. This allows the compiler to enforce state transitions and prevent illegal actions at compile time. It also improves the developer experience, as developers only have access to certain functions based on the state of the type. + +> Invalid states become compile errors instead of runtime bugs. + +## 7.2 Why use it? + +* Avoids runtime checks for state validity. If you reach certain states, you can make certain assumptions of the data you have. +* Models state transitions as type transitions. This is similar to a state machine, but in compile time. +* Prevents data misuse, e.g. using uninitialized objects. +* Improves API safety and correctness. +* The phantom data field is removed after compilation so no extra memory is allocated. + +## 7.3 Simple Example: File State + +[Github Example](https://github.com/apollographql/rust-best-practices/tree/main/examples/simple-type-state) +```rust +use std::{io, path::{Path, PathBuf}}; + +struct FileNotOpened; +struct FileOpened; + +#[derive(Debug)] +struct File { + /// Path to the opened file + path: PathBuf, + /// Open `File` handler + handle: Option, + /// Type state manager + _state: std::marker::PhantomData +} + +impl File { + /// `open` is the only entry point for this struct. + /// * When called with a valid path, it will return a `File` with a valid `handler` and `path` + /// * `open` serves as an alternative to `new` and `defaults` methods (usable when your struct needs valid data to exist). + fn open(path: &Path) -> io::Result> { + // If file is invalid, it will return `std::io::Error` + let file = std::fs::File::open(path)?; + Ok( + File { + path: path.to_path_buf(), + // Always valid + handle: Some(file), + _state: std::marker::PhantomData:: + } + ) + } +} + +impl File { + /// Reads the content of the `File` as a `String`. + /// `read` can only be called by state `File` + fn read(&mut self) -> io::Result { + use io::Read; + + let mut content = String::new(); + let Some(handle)= self.handle.as_mut() else { + unreachable!("Safe to unwrap as state can only be reached when file is open"); + }; + handle.read_to_string(&mut content)?; + Ok(content) + } + + /// Returns the valid path buffer. + fn path(&self) -> &PathBuf { + &self.path + } +} +``` + +## 7.4 Real-World Examples + +### Builder Pattern with Compile-Time Guarantees + +> Forces the user to **set required fields** before calling `.build()`. + +[Github Example](https://github.com/apollographql/rust-best-practices/tree/main/examples/type-state-builder) + +A type-state pattern can have more than one associated states: + +```rust +use std::marker::PhantomData; + +struct MissingName; +struct NameSet; +struct MissingAge; +struct AgeSet; + +#[derive(Debug)] +struct Person { + name: String, + age: u8, + email: Option, +} + +struct Builder { + name: Option, + age: u8, + email: Option, + _name_marker: PhantomData, + _age_marker: PhantomData, +} + +impl Builder { + fn new() -> Self { + Builder { name: None, age: 0, _name_marker: PhantomData, _age_marker: PhantomData, email: None } + } + + fn name(self, name: String) -> Builder { + Builder { name: Some(name), _name_marker: PhantomData::, age: self.age, _age_marker: PhantomData, email: None } + } + + fn age(self, age: u8) -> Builder { + Builder { age, _age_marker: PhantomData::, name: None, _name_marker: PhantomData, email: None } + } +} + +impl Builder { + fn age(self, age: u8) -> Builder { + Builder { age, _age_marker: PhantomData::, name: self.name, _name_marker: PhantomData::, email: None } + } +} + +impl Builder { + fn email(self, email: String) -> Self { + Self { name: self.name , age: self.age , email: Some(email) , _name_marker: self._name_marker , _age_marker: self._age_marker } + } + + fn name(self, name: String) -> Builder { + Builder { name: Some(name), _name_marker: PhantomData::, age: self.age, _age_marker: PhantomData::, email: self.email } + } +} + +impl Builder { + fn build(self) -> Person { + Person { + name: self.name.unwrap_or_else(|| unreachable!("Name is guarantee to be set")), + age: self.age, + email: self.email, + } + } +} +``` + +Although a bit more verbose than a usual builder, this guarantees that all necessary fields are present (note that e-mail is optional field only present in the final builder). + +#### Usage: +```rust +// βœ… Valid cases +let person: Person = Builder::new().name("name".to_string()).age(30).build(); +let person: Person = Builder::new().age(30).name("name".to_string()).build(); +let person: Person = Builder::new().age(30).name("name".to_string()).email("myself@email.com".to_string()).build(); + +// ❌ Invalid cases +let person: Person = Builder::new().name("name".to_string()).build(); // ❌ Compile error: Age required to `build` +let person: Person = Builder::new().age(30).build(); // ❌ Compile error: Name required to `build` +let person: Person = Builder::new().age(30).email("myself@email.com".to_string()).build(); // ❌ Compile error: Name required to `build` +let person: Person = Builder::new().build();// ❌ Compile error: Name and Age required to `build` +``` + +### Network Protocol State Machine + +Illegal transitions like sending a message before connecting **simply don't compile**: + +```rust +// Mock example +struct Disconnected; +struct Connected; + +struct Client { + stream: Option, + _state: std::marker::PhantomData +} + +impl Client { + fn connect(addr: &str) -> std::io::Result> { + let stream = std::net::TcpStream::connect(addr)?; + Ok(Client { + stream: Some(stream), + _state: std::marker::PhantomData:: + }) + } +} + +impl Client { + fn send(&mut self, msg: &str) { + use std::io::Write; + let Some(stream) = self.stream.as_mut() else { + unreachable!("Stream is guarantee to be set"); + }; + stream.write_all(msg.as_bytes()) + } +} +``` + +## 7.5 Pros and Cons + +### βœ… Use Type-State Pattern When: +* Your want **compile-time state safety**. +* You need to enforce **API constraints**. +* You are writing a library/crate that is heavy dependent on variants. +* Your want to replace runtime booleans or enums with **type-safe code paths**. +* You need compile time correctness. + +### ❌ Avoid it when: +* Writing trivial states like enums. +* Don't need type-safety. +* When it leads to overcomplicated generics. +* When runtime flexibility is required. + +### 🚨 Downsides and Cautions +* Can lead to more **verbose solutions**. +* Can lead to **complex type signatures**. +* May require **unsafe** to return **variant outputs** based on different states. +* May required a bunch of duplication (e.g. same struct field reused). +* PhantomData is not intuitive for beginners and can feel a bit hacky. + +> Use this pattern when it **saves bugs, increases safety or simplifies logic**, not just for cleverness. diff --git a/.agents/skills/rust-best-practices/references/chapter_08.md b/.agents/skills/rust-best-practices/references/chapter_08.md new file mode 100644 index 00000000..19153401 --- /dev/null +++ b/.agents/skills/rust-best-practices/references/chapter_08.md @@ -0,0 +1,254 @@ +# Chapter 8 - Comments vs Documentation + +> Clear code beats clear comments. However, when the why isn't obvious, comment it plainly - or link to where you can read more context. + +## 8.1 Comments vs Documentation: Know the Difference + +| Purpose | Use `// comment` | Use `/// doc` or `//! crate doc` | +|-------------- |------------------------------------------- |---------------------------------------------------------------- | +| Describe Why | βœ… Yes - explains tricky reasoning | ❌ Not for documentation | +| Describe API | ❌ Not useful | βœ… Yes - public interfaces, usage, details, errors, panics | +| Maintainable | 🚨 Often becomes obsolete and hard to reason | βœ… Tied to code, appears in generated docs and can run test cases | +| Visibility | Local development only | Exported to users and tools like `cargo doc` | + +## 8.2 When to use comments + +Use `//` comments (double slashed) when something can't be expressed clearly in code, like: +* **Safety Guarantees**, some of which can be better expressed with code conditionals. +* Workarounds or **Optimizations**. +* Legacy or **platform-specific** behaviors. Some of them can be expressed with `#[cfg(..)]`. +* Links to **Design Docs** or **ADRs**. +* Assumptions or **gotchas** that aren't obvious. + +> Name your comments! For example, a comment regarding a safety guarantee should start with `// SAFETY: ...`. + +### βœ… Good comment: +```rust +// SAFETY: `ptr` is guaranteed to be non-null and aligned by caller +unsafe { std::ptr::copy_nonoverlapping(src, dst, len); } +``` + +### βœ… Design context comment: +```rust +// CONTEXT: Reuse root cert store across subgraphs to avoid duplicate OS calls: +// [ADR-12](link/to/adr-12): TLS Performance on MacOS +``` + +## 8.3 When comments get in the way + +Avoid comments that: +* Restate obvious things (`// increment i by 1 for the next loop`). +* Can grow stale over time. +* `TODO`s without actions (links to some versioned issue). +* Could be replaced by better naming or smaller functions. + +### ❌ Bad comment: +```rust +fn compute(counter: &mut usize) { + // increment by 1 + *counter += 1; +} +``` + +### ❌ Too long or outdated +```rust +// Originally written in 2028 for some now-defunct platform +``` + +## 8.4 Don't Write Living Documentation (living comments) + +Comments as a "living documentation" is a **dangerous myth**, as comments are **not free**: +* They **rot** - nobody compiles comments. +* They **mislead** - readers usually assume they are true with no critique, e.g. "the other developer knows this code better than I do". +* They **go stale** - unless maintained with the code, they become irrelevant. +* They are **noisy** - comments can clutter your code with multiple unnecessary lines. + +If something deserves to live beyond a PR, put it in: +* An **ADR** (Architectural Design Record). +* A Design Document. +* Document it **in code** by using types, doc comments, examples, renaming code blocks into cleaner functions. +* Add tests to cover and explain the change. + +> ### 🚨 If you find a comment, **read it in context**. Does it still make sense? If not, remove or update it, or ask for help. Comments should bother you. + +## 8.5 Replace Comments with Code + +Instead of long commented blocks, break logic into named helper functions: + +#### ❌ Commented code block: +```rust +fn save_user(&self) -> Result<(), MyError> { + // check if the user is authenticated + if self.is_authenticated() { + // serialize user data + let data = serde_json::to_string(self)?; + // write to file + std::fs::write(self.path(), data)?; + } +} +``` +**βœ… Extract for clarity**: + +```rust +fn save_auth_user(&self) -> Result { + if self.is_authenticated() { + let path = self.path(); + let serialized_user = serde_json::to_string(self)?; + std::fs::write(path, serialized_user)?; + Ok(path) + } else { + Err(MyError::UserNotAuthenticated) + } +} +``` + +## 8.6 `TODO` should become issues + +Don't leave `// TODO:` scattered around the codebase with no owner. Instead: +1. File Github Issue or Jira Ticket. (Prefer github issues on public repositories). +2. Reference the issue in the code: + +```rust +// TODO(issue #42): Remove workaround after bugfix +``` + +This makes `TODO`s trackable, actionable and visible to everyone. + +## 8.7 When to use doc comments + +Use `///` doc comments to document: +* All **public functions, structs, traits, enums**. +* Their purpose, their usage and their behaviors. +* Anything developers need to understand how to use it correctly. +* Add context that related to `Errors` and `Panics`. +* Plenty of examples. + +### βœ… Good doc comment: + +```rust +/// Loads [`User`] profile from disk +/// +/// # Error +/// - Returns [`MyError`] if the file is missing [`MyError::FileNotFound`]. +/// - Returns [`MyError`] if the content is an invalid Json, [`MyError::InvalidJson`]. +fn load_user(path: &Path) -> Result {...} +``` + +**Doc comments can also include examples, links and even tests:** + +```rust +/// Returns the square of the integer part of any number. +/// Square is limited to `u128`. +/// +/// # Examples +/// +/// ```rust +/// assert_eq!(square(4.3), 16) +/// ``` +fn square(x: impl ToInt) -> u128 { ... } +``` + +## 8.8 Documentation in Rust: How, When and Why + +Rust provides **first-class documentation tooling** via rustdoc, which makes documenting your code a key part of writing idiomatic and maintainable rust. There are doc specific lints to help with documentation, like: + +| Lint | Description | +|-------------- |------------------------------------------- | +| [missing_docs](https://doc.rust-lang.org/rustdoc/lints.html#missing_docs) | Warns that a public functions, struct, const, enum has missing documentation | +| [broken_intra_doc_links](https://doc.rust-lang.org/rustdoc/lints.html#broken_intra_doc_links) | Detects if an internal documentation link is broken. Specially useful when things are renamed. | +| [empty_docs](https://rust-lang.github.io/rust-clippy/master/#empty_docs) | Disallow empty docs - preventing bypass of `missing_docs` | +| [missing_panics_doc](https://rust-lang.github.io/rust-clippy/master/#missing_panics_doc) | Warns that documentation should have a `# Panics` section if function can panic | +| [missing_errors_doc](https://rust-lang.github.io/rust-clippy/master/#missing_errors_doc) | Warns that documentation should have a `# Errors` section if function returns a `Result` explaining `Err` conditions | +| [missing_safety_doc](https://rust-lang.github.io/rust-clippy/master/#missing_safety_doc) | Warns that documentation should have a `# Safety` section if public facing functions have visible unsafe blocks | + + +### Difference between `///` and `//!` + +| Style | Used for | Scope |Example | +|---------- |------------------------------ |------------------------------------------- |---------------------------------------------------------------- | +| `///` | Line doc comment | Public items like struct, fn, enum, consts | Documenting, giving context and usage to `fn`, `struct`, `enum`, etc | +| `//!` | Module level doc comment | Modules or entire crates | Explaining crate/module purpose with common use cases and quickstart | + +### `///` Item level documentation + +Use `///` for functions, structs, traits, enums, const, etc: + +```rust +/// Adds two numbers together. +/// +/// # Examples +/// +/// ``` +/// let result = my_crate::add(2, 3); +/// assert_eq!(result, 5); +/// ``` +pub fn add(a: i32, b: i32) -> i32 { + a + b +} +``` +* βœ… Write clear and descriptive **What it does** and **how to use it**. +* βœ… Use `# Examples` section to better explain **how to use it**. +* βœ… Prefer writing examples that can be tested via `cargo test`, even if you have to hide their output with starting `#`: +```rust +/// ``` +/// let result = my_crate::add(2, 3); +/// # assert_eq!(result, 5); +/// ``` +``` +* βœ… Use `# Panics`, `# Errors` and `# Safety` sections when relevant. +* Add relevant context to the type. + +### `//!` Module/Crate level Documentation + +Use `//!` when you want to document the **purpose of a module or a crate**. It is places at the top of a `lib.rs` or `mod.rs` file, for example `engine/mod.rs`: +```rust +//! This module implements a custom chess engine. +//! +//! It handles board state, move generation and check detection. +//! +//! # Example +//! ``` +//! let board = chess::engine::Board::default(); +//! assert!(board.is_valid()); +//! ``` +``` + +## 8.9 Checklist for Documentation coverage + +πŸ“¦ Crate-Level (lib.rs) +- [ ] `//!` doc at top explains **what the crate does**, and **what problems it solves**. +- [ ] Includes crate-level `# Examples` or pointers to modules. + +πŸ“ Modules (mod.rs or inline) +- [ ] `//!` doc explains **what this module is for**, its **exports**, and **invariants**. +- [ ] Avoid repeating doc comments on re-exported items unless clarification is needed. + +🧱 Structs, Enums, Traits +- `///` doc explains: + - [ ] The role this type plays. + - [ ] Invariants or expectations. + - [ ] Example construction or usage. +- [ ] Consider using [`#[non_exhaustive]`](https://doc.rust-lang.org/reference/attributes/type_system.html#the-non_exhaustive-attribute) if external users may match on it. + +πŸ”§ Functions and Methods +- `///` doc covers: + - [ ] What it does. + - [ ] Parameters and their meaning. + - [ ] Return value behavior. + - [ ] Edge cases (`# Panics`, `# Errors`). + - [ ] Usage example, `# Examples`. + +πŸ“‘ Traits +- [ ] Explain the **purpose** of the trait (marker? dynamic dispatch?). +- [ ] Doc for each method β€” include **when/why** to implement it. +- [ ] Document clearly default implemented methods and when to override. + +πŸ“¦ Public Constants +- [ ] Document what they configure and when you'd want to use them. + +### πŸ“Œ Best Practices +* βœ… Use examples generously β€” they double as test cases. +* βœ… Prefer clarity over formality β€” it's for humans, not machines. +* βœ… Prefer doc comments to explain usage, and leave implementation details to code comments if needed. +* βœ… Use `cargo doc --open` to check your output often. +* βœ… Add `#![deny(missing_docs)]` and other relevant doc lints in top-level modules if you want to enforce full doc coverage. diff --git a/.agents/skills/rust-best-practices/references/chapter_09.md b/.agents/skills/rust-best-practices/references/chapter_09.md new file mode 100644 index 00000000..bbef404c --- /dev/null +++ b/.agents/skills/rust-best-practices/references/chapter_09.md @@ -0,0 +1,256 @@ +# Chapter 9 - Understanding Pointers + +Many higher level languages hide memory management, typically **passing by value** (copy data) or **passing by reference** (reference to shared data) without worrying about allocation, heap, stack, ownership and lifetimes, it is all delegated to the garbage collector or VM. Here is a comparison on this topic between a few languages: + +### πŸ“Œ Language Comparison + +| Language | Value Types | Reference/Pointer Types | Async Model & Types | Manual Memory | +|------------ |------------------------------------- |----------------------------------------------------------- |---------------------------------------------------------------------------- |------------------------------ | +| Python | None | Everything is a reference | async def, await, Task, coroutines and asyncio.Future | ❌ Not Allowed | +| Javascript | Primitives | Objects | `async/await`, `Promise`, `setTimeout`. single threaded event loop | ❌ Not Allowed | +| Java | Primitives | Objects | `Future`, threads, Loom (green threads) | ❌ Almost none & not recommended | +| Go | Values are copied unless using `&T` | Pointers (`*T`, `&T`), escape analysis | goroutines, `channels`, `sync.Mutex`, `context.Context` | ⚠️ Limited | +| C | Primitives and structs supported | Raw pointers `T*` and `*void` | Threads, event loops (`libuv`, `libevent`) | βœ… Fully | +| C++ | Primitives and references | Raw `T*` and smart pointers `shared_ptr` and `unique_ptr` | threads, `std::future`, `std::async`, (since c++ 20 `co_await/coroutines`) | βœ… Mostly | +| Rust | Primitives, Arrays, `impl Copy` | `&T`, `&mut T`, `Box`, `Arc` | `async/await`, `tokio`, `Future`, `JoinHandle`, `Send + Sync` | βœ…πŸ”’ Safe and Explicit | + +## 9.1 Thread Safety + +Rust tracks pointers using `Send` and `Sync` traits: +- `Send` means data can move across threads. +- `Sync` means data can be referenced from multiple threads. + +> A pointer is thread-safe only if the data behind it is. + +| Pointer Type | Short Description | Send + Sync? | Main Use | +|---------------- |--------------------------------------------------------------------------- |-------------------------------------- |------------ | +| `&T` | Shared reference | Yes | Shared access | +| `&mut T` | Exclusive mutable reference | No, not Send | Exclusive mutation | +| `Box` | Heap-allocated owning pointer | Yes, if T: Send + Sync | Heap allocation | +| `Rc` | Single-threaded ref counted pointer | No, neither | Multiple owners (single-thread) | +| `Arc` | Atomic ref counter pointer | Yes | Multiple owners (multi-thread) | +| `Cell` | Interior mutability for copy types | No, not Sync | Shared mutable, non-threaded | +| `RefCell` | Interior mutability (dynamic borrow checker) | No, not Sync | Shared mutable, non-threaded | +| `Mutex` | Thread-safe interior mutability with exclusive access | Yes | Shared mutable, threaded | +| `RwLock` | Thread-safe shared readonly access OR exclusive mutable access | Yes | Shared mutable, threaded | +| `OnceCell` | Single-thread one-time initialization container (interior mutability ONCE) | No, not Sync | Simple lazy value initialization | +| `LazyCell` | A lazy version of `OnceCell` that calls function closure to initialize | No, not Sync | Complex lazy value initialization | +| `OnceLock` | Thread-safe version of `OnceCell` | Yes | Multi-thread single init | +| `LazyLock` | Thread-safe version of `LazyCell` | Yes | Multi-thread complex init | +| `*const T/*mut T` | Raw Pointers | No, user must ensure safety manually | Raw memory / FFI | + +## 9.2 When to use pointers: + +### `&T` - Shared Borrow: + +Probably the most common type in a Rust code base, it is **Safe, with no mutation** and allows **multiple readers**. + +```rust +let data: String = String::from_str("this a string").unwrap(); + +print_len(&data); +print_capacity(&data); +print_bytes(&data); + +fn print_len(s: &str) { + println!("{}", s.len()) +} + +fn print_capacity(s: &String) { + println!("{}", s.capacity()) +} + +fn print_bytes(s: &String) { + println!("{:?}", s.as_bytes()) +} +``` +### `&mut T` - Exclusive Borrow: + +Probably the most common *mutable* type in a Rust code base, it is **Safe, but only allows one mutable borrow at a time**. + +```rust +let mut data: String = String::from_str("this a string").unwrap(); +mark_update(&mut data); + +fn mark_update(s: &mut String) { + s.push_str("_update"); +} +``` + +### [`Box`](https://doc.rust-lang.org/std/boxed/struct.Box.html) - Heap Allocated + +Single-owner heap-allocated data, great for recursive types and large structs. + +```rust +pub enum MySubBoxedEnum { + Single(T), + Double(Box>, Box>), + Multi(Vec), // Note that Vec is already a boxed value +} +``` + +### [`Rc`](https://doc.rust-lang.org/std/rc/struct.Rc.html) - Reference Counter (single-thread) + +You need multiple references to data in a single thread. Most common example is linked-list implementation. + +### [`Arc`](https://doc.rust-lang.org/std/sync/struct.Arc.html) - Atomic Reference Counter (multi-thread) + +You need multiple references to data in multiple threads. Most common use cases is sharing readonly Vec across thread with `Arc<[T]>` and wrapping a `Mutex` so it can be easily shared across threads, `Arc>`. + +### [`RefCell`](https://doc.rust-lang.org/std/cell/struct.RefCell.html) - Runtime checked interior mutability + +Used when you need shared access and the ability to mutate date, borrow rules are enforced at runtime. **It may panic!**. + +```rust +use std::cell::RefCell; +let x = RefCell::new(42); +*x.borrow_mut() += 1; + +assert_eq!(&*x.borrow(), 42, "Not meaning of life"); +``` + +Panic example: +```rust +use std::cell::RefCell; +let x = RefCell::new(42); + +let borrow = x.borrow(); + +let mutable = x.borrow_mut(); +``` + +### [`Cell`](https://doc.rust-lang.org/std/cell/struct.Cell.html) - Copy-only interior mutability + +Somewhat the fast and safe version of `RefCell`, but it is limited to types that implement the `Copy` trait: + +```rust +use std::cell::Cell; + +struct SomeStruct { + regular_field: u8, + special_field: Cell, +} + +let my_struct = SomeStruct { + regular_field: 0, + special_field: Cell::new(1), +}; + +let new_value = 100; + +// ERROR: `my_struct` is immutable +// my_struct.regular_field = new_value; + +// WORKS: although `my_struct` is immutable, `special_field` is a `Cell`, +// which can always be mutated with copy values +my_struct.special_field.set(new_value); +assert_eq!(my_struct.special_field.get(), new_value); +``` + +### [`Mutex`](https://doc.rust-lang.org/std/sync/struct.Mutex.html) - Thread-safe mutability + +An exclusive access pointer that allows a thread to read/write the data contained inside. It is usually wrapped in an `Arc` to allow shared access to the Mutex. + +### [`RwLock`](https://doc.rust-lang.org/std/sync/struct.RwLock.html) - Thread-safe mutability + +Similar to a `Mutex`, but it allows multiple threads to read it OR a single thread to write. It is usually wrapped in an `Arc` to allow shared access to the RwLock. + + +### [`*const T/*mut T`](https://doc.rust-lang.org/std/primitive.pointer.html) - Raw pointers + +Inherently **unsafe** and necessary for FFI. Rust makes their usage explicit to avoid accidental misuse and unwilling manual memory management. + +```rust +let x = 5; +let ptr = &x as *const i32; +unsafe { + println!("PTR is {}", *ptr) +} +``` + +### [`OnceCell`](https://doc.rust-lang.org/std/cell/struct.OnceCell.html) - Single-thread single initialization container + +Most useful when you need to share a configuration between multiple data structures. + +```rust +use std::{cell::OnceCell, rc::Rc}; + +#[derive(Debug, Default)] +struct MyStruct { + distance: usize, + root: Option>>, +} + +fn main() { + let root = MyStruct::default(); + let root_cell = Rc::new(OnceCell::new()); + if let Err(previous) = root_cell.set(root) { + eprintln!("Previous Root {previous:?}"); + } + let child_1 = MyStruct{ + distance: 1, + root: Some(root_cell.clone()) + }; + + let child_2 = MyStruct{ + distance: 2, + root: Some(root_cell.clone()) + }; + + + println!("Child 1: {child_1:?}"); + println!("Child 2: {child_2:?}"); +} +``` + +### [`LazyCell`](https://doc.rust-lang.org/std/cell/struct.LazyCell.html) - Lazy initialization of `OnceCell` + +Useful when the initialized data can be delayed to when it is actually being called. + +### [`OnceLock`](https://doc.rust-lang.org/std/sync/struct.OnceLock.html) - thread-safe `OnceCell` + +Useful when you need a `static` value. + +```rust +use std::sync::OnceLock; + +static CELL: OnceLock = OnceLock::new(); + +// `OnceLock` has not been written to yet. +assert!(CELL.get().is_none()); + +// Spawn a thread and write to `OnceLock`. +std::thread::spawn(|| { + let value = CELL.get_or_init(|| 12345); + assert_eq!(value, &12345); +}) +.join() +.unwrap(); + +// `OnceLock` now contains the value. +assert_eq!( + CELL.get(), + Some(&12345), +); +``` + +### [`LazyLock`](https://doc.rust-lang.org/std/sync/struct.LazyLock.html) - thread-safe `LazyCell` + +Similar to `OnceLock`, but the static value is a bit more complex to initialize. + +```rust +use std::sync::LazyLock; + +static CONFIG: LazyLock> = LazyLock::new(|| { + let data = read_config(); + let mut config: HashMap<&str, T> = data.into(); + config.insert("special_case", T::default()); + config +}); + +let _ = &*CONFIG; +``` + +## References +- [Mara Bos - Rust Atomics and Locks](https://marabos.nl/atomics/) +- [Semicolon video on pointers](https://www.youtube.com/watch?v=Ag_6Q44PBNs) diff --git a/.agents/skills/svelte-code-writer/SKILL.md b/.agents/skills/svelte-code-writer/SKILL.md new file mode 100644 index 00000000..b50ccf90 --- /dev/null +++ b/.agents/skills/svelte-code-writer/SKILL.md @@ -0,0 +1,66 @@ +--- +name: svelte-code-writer +description: CLI tools for Svelte 5 documentation lookup and code analysis. MUST be used whenever creating, editing or analyzing any Svelte component (.svelte) or Svelte module (.svelte.ts/.svelte.js). If possible, this skill should be executed within the svelte-file-editor agent for optimal results. +--- + +# Svelte 5 Code Writer + +## CLI Tools + +You have access to `@sveltejs/mcp` CLI for Svelte-specific assistance. Use these commands via `npx`: + +### List Documentation Sections + +```bash +npx @sveltejs/mcp list-sections +``` + +Lists all available Svelte 5 and SvelteKit documentation sections with titles and paths. + +### Get Documentation + +```bash +npx @sveltejs/mcp get-documentation ",,..." +``` + +Retrieves full documentation for specified sections. Use after `list-sections` to fetch relevant docs. + +**Example:** + +```bash +npx @sveltejs/mcp get-documentation "$state,$derived,$effect" +``` + +### Svelte Autofixer + +```bash +npx @sveltejs/mcp svelte-autofixer "" [options] +``` + +Analyzes Svelte code and suggests fixes for common issues. + +**Options:** + +- `--async` - Enable async Svelte mode (default: false) +- `--svelte-version` - Target version: 4 or 5 (default: 5) + +**Examples:** + +```bash +# Analyze inline code (escape $ as \$) +npx @sveltejs/mcp svelte-autofixer '' + +# Analyze a file +npx @sveltejs/mcp svelte-autofixer ./src/lib/Component.svelte + +# Target Svelte 4 +npx @sveltejs/mcp svelte-autofixer ./Component.svelte --svelte-version 4 +``` + +**Important:** When passing code with runes (`$state`, `$derived`, etc.) via the terminal, escape the `$` character as `\$` to prevent shell variable substitution. + +## Workflow + +1. **Uncertain about syntax?** Run `list-sections` then `get-documentation` for relevant topics +2. **Reviewing/debugging?** Run `svelte-autofixer` on the code to detect issues +3. **Always validate** - Run `svelte-autofixer` before finalizing any Svelte component diff --git a/.agents/skills/svelte-core-bestpractices/SKILL.md b/.agents/skills/svelte-core-bestpractices/SKILL.md new file mode 100644 index 00000000..ffa73ee7 --- /dev/null +++ b/.agents/skills/svelte-core-bestpractices/SKILL.md @@ -0,0 +1,176 @@ +--- +name: svelte-core-bestpractices +description: Guidance on writing fast, robust, modern Svelte code. Load this skill whenever in a Svelte project and asked to write/edit or analyze a Svelte component or module. Covers reactivity, event handling, styling, integration with libraries and more. +--- + +## `$state` + +Only use the `$state` rune for variables that should be _reactive_ β€” in other words, variables that cause an `$effect`, `$derived` or template expression to update. Everything else can be a normal variable. + +Objects and arrays (`$state({...})` or `$state([...])`) are made deeply reactive, meaning mutation will trigger updates. This has a trade-off: in exchange for fine-grained reactivity, the objects must be proxied, which has performance overhead. In cases where you're dealing with large objects that are only ever reassigned (rather than mutated), use `$state.raw` instead. This is often the case with API responses, for example. + +## `$derived` + +To compute something from state, use `$derived` rather than `$effect`: + +```js +// do this +let square = $derived(num * num); + +// don't do this +let square; + +$effect(() => { + square = num * num; +}); +``` + +> [!NOTE] `$derived` is given an expression, _not_ a function. If you need to use a function (because the expression is complex, for example) use `$derived.by`. + +Deriveds are writable β€” you can assign to them, just like `$state`, except that they will re-evaluate when their expression changes. + +If the derived expression is an object or array, it will be returned as-is β€” it is _not_ made deeply reactive. You can, however, use `$state` inside `$derived.by` in the rare cases that you need this. + +## `$effect` + +Effects are an escape hatch and should mostly be avoided. In particular, avoid updating state inside effects. + +- If you need to sync state to an external library such as D3, it is often neater to use [`{@attach ...}`](references/@attach.md) +- If you need to run some code in response to user interaction, put the code directly in an event handler or use a [function binding](references/bind.md) as appropriate +- If you need to log values for debugging purposes, use [`$inspect`](references/$inspect.md) +- If you need to observe something external to Svelte, use [`createSubscriber`](references/svelte-reactivity.md) + +Never wrap the contents of an effect in `if (browser) {...}` or similar β€” effects do not run on the server. + +## `$props` + +Treat props as though they will change. For example, values that depend on props should usually use `$derived`: + +```js +// @errors: 2451 +let { type } = $props(); + +// do this +let color = $derived(type === 'danger' ? 'red' : 'green'); + +// don't do this β€” `color` will not update if `type` changes +let color = type === 'danger' ? 'red' : 'green'; +``` + +## `$inspect.trace` + +`$inspect.trace` is a debugging tool for reactivity. If something is not updating properly or running more than it should you can add `$inspect.trace(label)` as the first line of an `$effect` or `$derived.by` (or any function they call) to trace their dependencies and discover which one triggered an update. + +## Events + +Any element attribute starting with `on` is treated as an event listener: + +```svelte + + + + + + + +``` + +If you need to attach listeners to `window` or `document` you can use `` and ``: + +```svelte + + +``` + +Avoid using `onMount` or `$effect` for this. + +## Snippets + +[Snippets](references/snippet.md) are a way to define reusable chunks of markup that can be instantiated with the [`{@render ...}`](references/@render.md) tag, or passed to components as props. They must be declared within the template. + +```svelte +{#snippet greeting(name)} +

hello {name}!

+{/snippet} + +{@render greeting('world')} +``` + +> [!NOTE] Snippets declared at the top level of a component (i.e. not inside elements or blocks) can be referenced inside ` + + + +``` + +On updates, a stack trace will be printed, making it easy to find the origin of a state change (unless you're in the playground, due to technical limitations). + +## $inspect(...).with + +`$inspect` returns a property `with`, which you can invoke with a callback, which will then be invoked instead of `console.log`. The first argument to the callback is either `"init"` or `"update"`; subsequent arguments are the values passed to `$inspect` (demo: + +```svelte + + + +``` + +## $inspect.trace(...) + +This rune, added in 5.14, causes the surrounding function to be _traced_ in development. Any time the function re-runs as part of an [effect]($effect) or a [derived]($derived), information will be printed to the console about which pieces of reactive state caused the effect to fire. + +```svelte + +``` + +`$inspect.trace` takes an optional first argument which will be used as the label. diff --git a/.agents/skills/svelte-core-bestpractices/references/@attach.md b/.agents/skills/svelte-core-bestpractices/references/@attach.md new file mode 100644 index 00000000..5c113b16 --- /dev/null +++ b/.agents/skills/svelte-core-bestpractices/references/@attach.md @@ -0,0 +1,166 @@ +Attachments are functions that run in an [effect]($effect) when an element is mounted to the DOM or when [state]($state) read inside the function updates. + +Optionally, they can return a function that is called before the attachment re-runs, or after the element is later removed from the DOM. + +> [!NOTE] +> Attachments are available in Svelte 5.29 and newer. + +```svelte + + + +
...
+``` + +An element can have any number of attachments. + +## Attachment factories + +A useful pattern is for a function, such as `tooltip` in this example, to _return_ an attachment (demo: + +```svelte + + + + + + +``` + +Since the `tooltip(content)` expression runs inside an [effect]($effect), the attachment will be destroyed and recreated whenever `content` changes. The same thing would happen for any state read _inside_ the attachment function when it first runs. (If this isn't what you want, see [Controlling when attachments re-run](#Controlling-when-attachments-re-run).) + +## Inline attachments + +Attachments can also be created inline (demo: + +```svelte + + { + const context = canvas.getContext('2d'); + + $effect(() => { + context.fillStyle = color; + context.fillRect(0, 0, canvas.width, canvas.height); + }); + }} +> +``` + +> [!NOTE] +> The nested effect runs whenever `color` changes, while the outer effect (where `canvas.getContext(...)` is called) only runs once, since it doesn't read any reactive state. + +## Conditional attachments + +Falsy values like `false` or `undefined` are treated as no attachment, enabling conditional usage: + +```svelte +
...
+``` + +## Passing attachments to components + +When used on a component, `{@attach ...}` will create a prop whose key is a [`Symbol`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol). If the component then [spreads](/tutorial/svelte/spread-props) props onto an element, the element will receive those attachments. + +This allows you to create _wrapper components_ that augment elements (demo: + +```svelte + + + + + +``` + +```svelte + + + + + + +``` + +## Controlling when attachments re-run + +Attachments, unlike [actions](use), are fully reactive: `{@attach foo(bar)}` will re-run on changes to `foo` _or_ `bar` (or any state read inside `foo`): + +```js +// @errors: 7006 2304 2552 +function foo(bar) { + return (node) => { + veryExpensiveSetupWork(node); + update(node, bar); + }; +} +``` + +In the rare case that this is a problem (for example, if `foo` does expensive and unavoidable setup work) consider passing the data inside a function and reading it in a child effect: + +```js +// @errors: 7006 2304 2552 +function foo(+++getBar+++) { + return (node) => { + veryExpensiveSetupWork(node); + ++++ $effect(() => { + update(node, getBar()); + });+++ + } +} +``` + +## Creating attachments programmatically + +To add attachments to an object that will be spread onto a component or element, use [`createAttachmentKey`](svelte-attachments#createAttachmentKey). + +## Converting actions to attachments + +If you're using a library that only provides actions, you can convert them to attachments with [`fromAction`](svelte-attachments#fromAction), allowing you to (for example) use them with components. diff --git a/.agents/skills/svelte-core-bestpractices/references/@render.md b/.agents/skills/svelte-core-bestpractices/references/@render.md new file mode 100644 index 00000000..2e606855 --- /dev/null +++ b/.agents/skills/svelte-core-bestpractices/references/@render.md @@ -0,0 +1,35 @@ +To render a [snippet](snippet), use a `{@render ...}` tag. + +```svelte +{#snippet sum(a, b)} +

{a} + {b} = {a + b}

+{/snippet} + +{@render sum(1, 2)} +{@render sum(3, 4)} +{@render sum(5, 6)} +``` + +The expression can be an identifier like `sum`, or an arbitrary JavaScript expression: + +```svelte +{@render (cool ? coolSnippet : lameSnippet)()} +``` + +## Optional snippets + +If the snippet is potentially undefined β€” for example, because it's an incoming prop β€” then you can use optional chaining to only render it when it _is_ defined: + +```svelte +{@render children?.()} +``` + +Alternatively, use an [`{#if ...}`](if) block with an `:else` clause to render fallback content: + +```svelte +{#if children} + {@render children()} +{:else} +

fallback content

+{/if} +``` diff --git a/.agents/skills/svelte-core-bestpractices/references/await-expressions.md b/.agents/skills/svelte-core-bestpractices/references/await-expressions.md new file mode 100644 index 00000000..18c22316 --- /dev/null +++ b/.agents/skills/svelte-core-bestpractices/references/await-expressions.md @@ -0,0 +1,180 @@ +As of Svelte 5.36, you can use the `await` keyword inside your components in three places where it was previously unavailable: + +- at the top level of your component's ` + + + + +

{a} + {b} = {await add(a, b)}

+``` + +...if you increment `a`, the contents of the `

` will _not_ immediately update to read this β€” + +```html +

2 + 2 = 3

+``` + +β€” instead, the text will update to `2 + 2 = 4` when `add(a, b)` resolves. + +Updates can overlap β€” a fast update will be reflected in the UI while an earlier slow update is still ongoing. + +## Concurrency + +Svelte will do as much asynchronous work as it can in parallel. For example if you have two `await` expressions in your markup... + +```svelte +

{await one()}

{await two()}

+``` + +...both functions will run at the same time, as they are independent expressions, even though they are _visually_ sequential. + +This does not apply to sequential `await` expressions inside your ` + + + +{#if open} + + (open = false)} /> +{/if} +``` + +## Caveats + +As an experimental feature, the details of how `await` is handled (and related APIs like `$effect.pending()`) are subject to breaking changes outside of a semver major release, though we intend to keep such changes to a bare minimum. + +## Breaking changes + +Effects run in a slightly different order when the `experimental.async` option is `true`. Specifically, _block_ effects like `{#if ...}` and `{#each ...}` now run before an `$effect.pre` or `beforeUpdate` in the same component, which means that in very rare situations. diff --git a/.agents/skills/svelte-core-bestpractices/references/bind.md b/.agents/skills/svelte-core-bestpractices/references/bind.md new file mode 100644 index 00000000..80b2f4c4 --- /dev/null +++ b/.agents/skills/svelte-core-bestpractices/references/bind.md @@ -0,0 +1,16 @@ +## Function bindings + +You can also use `bind:property={get, set}`, where `get` and `set` are functions, allowing you to perform validation and transformation: + +```svelte + value, (v) => (value = v.toLowerCase())} /> +``` + +In the case of readonly bindings like [dimension bindings](#Dimensions), the `get` value should be `null`: + +```svelte +
...
+``` + +> [!NOTE] +> Function bindings are available in Svelte 5.9.0 and newer. diff --git a/.agents/skills/svelte-core-bestpractices/references/each.md b/.agents/skills/svelte-core-bestpractices/references/each.md new file mode 100644 index 00000000..283b754f --- /dev/null +++ b/.agents/skills/svelte-core-bestpractices/references/each.md @@ -0,0 +1,42 @@ +## Keyed each blocks + +```svelte + +{#each expression as name (key)}...{/each} +``` + +```svelte + +{#each expression as name, index (key)}...{/each} +``` + +If a _key_ expression is provided β€” which must uniquely identify each list item β€” Svelte will use it to intelligently update the list when data changes by inserting, moving and deleting items, rather than adding or removing items at the end and updating the state in the middle. + +The key can be any object, but strings and numbers are recommended since they allow identity to persist when the objects themselves change. + +```svelte +{#each items as item (item.id)} +
  • {item.name} x {item.qty}
  • +{/each} + + +{#each items as item, i (item.id)} +
  • {i + 1}: {item.name} x {item.qty}
  • +{/each} +``` + +You can freely use destructuring and rest patterns in each blocks. + +```svelte +{#each items as { id, name, qty }, i (id)} +
  • {i + 1}: {name} x {qty}
  • +{/each} + +{#each objects as { id, ...rest }} +
  • {id}
  • +{/each} + +{#each items as [id, ...rest]} +
  • {id}
  • +{/each} +``` diff --git a/.agents/skills/svelte-core-bestpractices/references/hydratable.md b/.agents/skills/svelte-core-bestpractices/references/hydratable.md new file mode 100644 index 00000000..a7baf74d --- /dev/null +++ b/.agents/skills/svelte-core-bestpractices/references/hydratable.md @@ -0,0 +1,100 @@ +In Svelte, when you want to render asynchronous content data on the server, you can simply `await` it. This is great! However, it comes with a pitfall: when hydrating that content on the client, Svelte has to redo the asynchronous work, which blocks hydration for however long it takes: + +```svelte + + +

    {user.name}

    +``` + +That's silly, though. If we've already done the hard work of getting the data on the server, we don't want to get it again during hydration on the client. `hydratable` is a low-level API built to solve this problem. You probably won't need this very often β€” it will be used behind the scenes by whatever datafetching library you use. For example, it powers [remote functions in SvelteKit](/docs/kit/remote-functions). + +To fix the example above: + +```svelte + + +

    {user.name}

    +``` + +This API can also be used to provide access to random or time-based values that are stable between server rendering and hydration. For example, to get a random number that doesn't update on hydration: + +```ts +import { hydratable } from 'svelte'; +const rand = hydratable('random', () => Math.random()); +``` + +If you're a library author, be sure to prefix the keys of your `hydratable` values with the name of your library so that your keys don't conflict with other libraries. + +## Serialization + +All data returned from a `hydratable` function must be serializable. But this doesn't mean you're limited to JSON β€” Svelte uses [`devalue`](https://npmjs.com/package/devalue), which can serialize all sorts of things including `Map`, `Set`, `URL`, and `BigInt`. Check the documentation page for a full list. In addition to these, thanks to some Svelte magic, you can also fearlessly use promises: + +```svelte + + +{await promises.one} +{await promises.two} +``` + +## CSP + +`hydratable` adds an inline ` + +{#snippet hello(name)} +

    hello {name}! {message}!

    +{/snippet} + +{@render hello('alice')} +{@render hello('bob')} +``` + +...and they are 'visible' to everything in the same lexical scope (i.e. siblings, and children of those siblings): + +```svelte +
    + {#snippet x()} + {#snippet y()}...{/snippet} + + + {@render y()} + {/snippet} + + + {@render y()} +
    + + +{@render x()} +``` + +Snippets can reference themselves and each other (demo: + +```svelte +{#snippet blastoff()} + πŸš€ +{/snippet} + +{#snippet countdown(n)} + {#if n > 0} + {n}... + {@render countdown(n - 1)} + {:else} + {@render blastoff()} + {/if} +{/snippet} + +{@render countdown(10)} +``` + +## Passing snippets to components + +### Explicit props + +Within the template, snippets are values just like any other. As such, they can be passed to components as props (demo: + +```svelte + + +{#snippet header()} + fruit + qty + price + total +{/snippet} + +{#snippet row(d)} + {d.name} + {d.qty} + {d.price} + {d.qty * d.price} +{/snippet} + + +``` + +Think about it like passing content instead of data to a component. The concept is similar to slots in web components. + +### Implicit props + +As an authoring convenience, snippets declared directly _inside_ a component implicitly become props _on_ the component (demo: + +```svelte + +
    + {#snippet header()} + + + + + {/snippet} + + {#snippet row(d)} + + + + + {/snippet} +
    fruitqtypricetotal{d.name}{d.qty}{d.price}{d.qty * d.price}
    +``` + +### Implicit `children` snippet + +Any content inside the component tags that is _not_ a snippet declaration implicitly becomes part of the `children` snippet (demo: + +```svelte + + +``` + +```svelte + + + + + +``` + +> [!NOTE] Note that you cannot have a prop called `children` if you also have content inside the component β€” for this reason, you should avoid having props with that name + +### Optional snippet props + +You can declare snippet props as being optional. You can either use optional chaining to not render anything if the snippet isn't set... + +```svelte + + +{@render children?.()} +``` + +...or use an `#if` block to render fallback content: + +```svelte + + +{#if children} + {@render children()} +{:else} + fallback content +{/if} +``` + +## Typing snippets + +Snippets implement the `Snippet` interface imported from `'svelte'`: + +```svelte + +``` + +With this change, red squigglies will appear if you try and use the component without providing a `data` prop and a `row` snippet. Notice that the type argument provided to `Snippet` is a tuple, since snippets can have multiple parameters. + +We can tighten things up further by declaring a generic, so that `data` and `row` refer to the same type: + +```svelte + +``` + +## Exporting snippets + +Snippets declared at the top level of a `.svelte` file can be exported from a ` + +{#snippet add(a, b)} + {a} + {b} = {a + b} +{/snippet} +``` + +> [!NOTE] +> This requires Svelte 5.5.0 or newer + +## Programmatic snippets + +Snippets can be created programmatically with the [`createRawSnippet`](svelte#createRawSnippet) API. This is intended for advanced use cases. + +## Snippets and slots + +In Svelte 4, content can be passed to components using [slots](legacy-slots). Snippets are more powerful and flexible, and so slots have been deprecated in Svelte 5. diff --git a/.agents/skills/svelte-core-bestpractices/references/svelte-reactivity.md b/.agents/skills/svelte-core-bestpractices/references/svelte-reactivity.md new file mode 100644 index 00000000..262e3617 --- /dev/null +++ b/.agents/skills/svelte-core-bestpractices/references/svelte-reactivity.md @@ -0,0 +1,61 @@ +## createSubscriber + +
    + +Available since 5.7.0 + +
    + +Returns a `subscribe` function that integrates external event-based systems with Svelte's reactivity. +It's particularly useful for integrating with web APIs like `MediaQuery`, `IntersectionObserver`, or `WebSocket`. + +If `subscribe` is called inside an effect (including indirectly, for example inside a getter), +the `start` callback will be called with an `update` function. Whenever `update` is called, the effect re-runs. + +If `start` returns a cleanup function, it will be called when the effect is destroyed. + +If `subscribe` is called in multiple effects, `start` will only be called once as long as the effects +are active, and the returned teardown function will only be called when all effects are destroyed. + +It's best understood with an example. Here's an implementation of [`MediaQuery`](/docs/svelte/svelte-reactivity#MediaQuery): + +```js +// @errors: 7031 +import { createSubscriber } from 'svelte/reactivity'; +import { on } from 'svelte/events'; + +export class MediaQuery { + #query; + #subscribe; + + constructor(query) { + this.#query = window.matchMedia(`(${query})`); + + this.#subscribe = createSubscriber((update) => { + // when the `change` event occurs, re-run any effects that read `this.current` + const off = on(this.#query, 'change', update); + + // stop listening when all the effects are destroyed + return () => off(); + }); + } + + get current() { + // This makes the getter reactive, if read in an effect + this.#subscribe(); + + // Return the current state of the query, whether or not we're in an effect + return this.#query.matches; + } +} +``` + +
    + +```dts +function createSubscriber( + start: (update: () => void) => (() => void) | void +): () => void; +``` + +
    diff --git a/.agents/skills/tauri-v2/SKILL.md b/.agents/skills/tauri-v2/SKILL.md new file mode 100644 index 00000000..58a35950 --- /dev/null +++ b/.agents/skills/tauri-v2/SKILL.md @@ -0,0 +1,368 @@ +--- +name: tauri-v2 +description: "Tauri v2 cross-platform app development with Rust backend. Use when configuring tauri.conf.json, implementing Rust commands (#[tauri::command]), setting up IPC patterns (invoke, emit, channels), configuring permissions/capabilities, troubleshooting build issues, or deploying desktop/mobile apps. Triggers on Tauri, src-tauri, invoke, emit, capabilities.json." +--- + +# Tauri v2 Development Skill + +> Build cross-platform desktop and mobile apps with web frontends and Rust backends. + +## Before You Start + +**This skill prevents 8+ common errors and saves ~60% tokens.** + +| Metric | Without Skill | With Skill | +|--------|--------------|------------| +| Setup Time | ~2 hours | ~30 min | +| Common Errors | 8+ | 0 | +| Token Usage | High (exploration) | Low (direct patterns) | + +### Known Issues This Skill Prevents + +1. Permission denied errors from missing capabilities +2. IPC failures from unregistered commands in `generate_handler!` +3. State management panics from type mismatches +4. Mobile build failures from missing Rust targets +5. White screen issues from misconfigured dev URLs + +## Quick Start + +### Step 1: Create a Tauri Command + +```rust +// src-tauri/src/lib.rs +#[tauri::command] +fn greet(name: String) -> String { + format!("Hello, {}!", name) +} + +pub fn run() { + tauri::Builder::default() + .invoke_handler(tauri::generate_handler![greet]) + .run(tauri::generate_context!()) + .expect("error while running tauri application"); +} +``` + +**Why this matters:** Commands not in `generate_handler![]` silently fail when invoked from frontend. + +### Step 2: Call from Frontend + +```typescript +import { invoke } from '@tauri-apps/api/core'; + +const greeting = await invoke('greet', { name: 'World' }); +console.log(greeting); // "Hello, World!" +``` + +**Why this matters:** Use `@tauri-apps/api/core` (not `@tauri-apps/api/tauri` - that's v1 API). + +### Step 3: Add Required Permissions + +```json +// src-tauri/capabilities/default.json +{ + "$schema": "../gen/schemas/desktop-schema.json", + "identifier": "default", + "windows": ["main"], + "permissions": ["core:default"] +} +``` + +**Why this matters:** Tauri v2 denies everything by default - explicit permissions required for all operations. + +## Critical Rules + +### Always Do + +- Register every command in `tauri::generate_handler![cmd1, cmd2, ...]` +- Return `Result` from commands for proper error handling +- Use `Mutex` for shared state accessed from multiple commands +- Add capabilities before using any plugin features +- Use `lib.rs` for shared code (required for mobile builds) + +### Never Do + +- Never use borrowed types (`&str`) in async commands - use owned types +- Never block the main thread - use async for I/O operations +- Never hardcode paths - use Tauri path APIs (`app.path()`) +- Never skip capability setup - even "safe" operations need permissions + +### Common Mistakes + +**Wrong - Borrowed type in async:** +```rust +#[tauri::command] +async fn bad(name: &str) -> String { // Compile error! + name.to_string() +} +``` + +**Correct - Owned type:** +```rust +#[tauri::command] +async fn good(name: String) -> String { + name +} +``` + +**Why:** Async commands cannot borrow data across await points; Tauri requires owned types for async command parameters. + +## Known Issues Prevention + +| Issue | Root Cause | Solution | +|-------|-----------|----------| +| "Command not found" | Missing from `generate_handler!` | Add command to handler macro | +| "Permission denied" | Missing capability | Add to `capabilities/default.json` | +| State panic on access | Type mismatch in `State` | Use exact type from `.manage()` | +| White screen on launch | Frontend not building | Check `beforeDevCommand` in config | +| IPC timeout | Blocking async command | Remove blocking code or use spawn | +| Mobile build fails | Missing Rust targets | Run `rustup target add ` | + +## Configuration Reference + +### tauri.conf.json + +```json +{ + "$schema": "./gen/schemas/desktop-schema.json", + "productName": "my-app", + "version": "1.0.0", + "identifier": "com.example.myapp", + "build": { + "devUrl": "http://localhost:5173", + "frontendDist": "../dist", + "beforeDevCommand": "npm run dev", + "beforeBuildCommand": "npm run build" + }, + "app": { + "windows": [{ + "label": "main", + "title": "My App", + "width": 800, + "height": 600 + }], + "security": { + "csp": "default-src 'self'; img-src 'self' data:", + "capabilities": ["default"] + } + }, + "bundle": { + "active": true, + "targets": "all", + "icon": ["icons/icon.icns", "icons/icon.ico", "icons/icon.png"] + } +} +``` + +**Key settings:** +- `build.devUrl`: Must match your frontend dev server port +- `app.security.capabilities`: Array of capability file identifiers + +### Cargo.toml + +```toml +[package] +name = "app" +version = "0.1.0" +edition = "2021" + +[lib] +name = "app_lib" +crate-type = ["staticlib", "cdylib", "rlib"] + +[build-dependencies] +tauri-build = { version = "2", features = [] } + +[dependencies] +tauri = { version = "2", features = [] } +serde = { version = "1", features = ["derive"] } +serde_json = "1" +``` + +**Key settings:** +- `[lib]` section: Required for mobile builds +- `crate-type`: Must include all three types for cross-platform + +## Common Patterns + +### Error Handling Pattern + +```rust +use thiserror::Error; + +#[derive(Debug, Error)] +enum AppError { + #[error("IO error: {0}")] + Io(#[from] std::io::Error), + #[error("Not found: {0}")] + NotFound(String), +} + +impl serde::Serialize for AppError { + fn serialize(&self, serializer: S) -> Result + where S: serde::ser::Serializer { + serializer.serialize_str(self.to_string().as_ref()) + } +} + +#[tauri::command] +fn risky_operation() -> Result { + Ok("success".into()) +} +``` + +### State Management Pattern + +```rust +use std::sync::Mutex; +use tauri::State; + +struct AppState { + counter: u32, +} + +#[tauri::command] +fn increment(state: State<'_, Mutex>) -> u32 { + let mut s = state.lock().unwrap(); + s.counter += 1; + s.counter +} + +// In builder: +tauri::Builder::default() + .manage(Mutex::new(AppState { counter: 0 })) +``` + +### Event Emission Pattern + +```rust +use tauri::Emitter; + +#[tauri::command] +fn start_task(app: tauri::AppHandle) { + std::thread::spawn(move || { + app.emit("task-progress", 50).unwrap(); + app.emit("task-complete", "done").unwrap(); + }); +} +``` + +```typescript +import { listen } from '@tauri-apps/api/event'; + +const unlisten = await listen('task-progress', (e) => { + console.log('Progress:', e.payload); +}); +// Call unlisten() when done +``` + +### Channel Streaming Pattern + +```rust +use tauri::ipc::Channel; + +#[derive(Clone, serde::Serialize)] +#[serde(tag = "event", content = "data")] +enum DownloadEvent { + Progress { percent: u32 }, + Complete { path: String }, +} + +#[tauri::command] +async fn download(url: String, on_event: Channel) { + for i in 0..=100 { + on_event.send(DownloadEvent::Progress { percent: i }).unwrap(); + } + on_event.send(DownloadEvent::Complete { path: "/downloads/file".into() }).unwrap(); +} +``` + +```typescript +import { invoke, Channel } from '@tauri-apps/api/core'; + +const channel = new Channel(); +channel.onmessage = (msg) => console.log(msg.event, msg.data); +await invoke('download', { url: 'https://...', onEvent: channel }); +``` + +## Bundled Resources + +### References + +Located in `references/`: +- [`capabilities-reference.md`](references/capabilities-reference.md) - Permission patterns and examples +- [`ipc-patterns.md`](references/ipc-patterns.md) - Complete IPC examples + +> **Note:** For deep dives on specific topics, see the reference files above. + +## Dependencies + +### Required + +| Package | Version | Purpose | +|---------|---------|---------| +| `@tauri-apps/cli` | ^2.0.0 | CLI tooling | +| `@tauri-apps/api` | ^2.0.0 | Frontend APIs | +| `tauri` | ^2.0.0 | Rust core | +| `tauri-build` | ^2.0.0 | Build scripts | + +### Optional (Plugins) + +| Package | Version | Purpose | +|---------|---------|---------| +| `tauri-plugin-fs` | ^2.0.0 | File system access | +| `tauri-plugin-dialog` | ^2.0.0 | Native dialogs | +| `tauri-plugin-shell` | ^2.0.0 | Shell commands, open URLs | +| `tauri-plugin-http` | ^2.0.0 | HTTP client | +| `tauri-plugin-store` | ^2.0.0 | Key-value storage | + +## Official Documentation + +- [Tauri v2 Documentation](https://v2.tauri.app/) +- [Commands Reference](https://v2.tauri.app/develop/calling-rust/) +- [Capabilities & Permissions](https://v2.tauri.app/security/capabilities/) +- [Configuration Reference](https://v2.tauri.app/reference/config/) + +## Troubleshooting + +### White Screen on Launch + +**Symptoms:** App launches but shows blank white screen + +**Solution:** +1. Verify `devUrl` matches your frontend dev server port +2. Check `beforeDevCommand` runs your dev server +3. Open DevTools (Cmd+Option+I / Ctrl+Shift+I) to check for errors + +### Command Returns Undefined + +**Symptoms:** `invoke()` returns undefined instead of expected value + +**Solution:** +1. Verify command is in `generate_handler![]` +2. Check Rust command actually returns a value +3. Ensure argument names match (camelCase in JS, snake_case in Rust by default) + +### Mobile Build Failures + +**Symptoms:** Android/iOS build fails with missing target + +**Solution:** +```bash +# Android targets +rustup target add aarch64-linux-android armv7-linux-androideabi i686-linux-android x86_64-linux-android + +# iOS targets (macOS only) +rustup target add aarch64-apple-ios x86_64-apple-ios aarch64-apple-ios-sim +``` + +## Setup Checklist + +Before using this skill, verify: + +- [ ] `npx tauri info` shows correct Tauri v2 versions +- [ ] `src-tauri/capabilities/default.json` exists with at least `core:default` +- [ ] All commands registered in `generate_handler![]` +- [ ] `lib.rs` contains shared code (for mobile support) +- [ ] Required Rust targets installed for target platforms diff --git a/.agents/skills/tauri-v2/references/README.md b/.agents/skills/tauri-v2/references/README.md new file mode 100644 index 00000000..a07cf7b7 --- /dev/null +++ b/.agents/skills/tauri-v2/references/README.md @@ -0,0 +1,10 @@ +# Tauri v2 References + +This directory contains detailed reference documentation for Tauri v2 development. + +## Contents + +- **capabilities-reference.md** - Security permissions and capability configuration +- **ipc-patterns.md** - Complete IPC examples (commands, events, channels) + +These references are loaded by Claude when deeper context is needed. diff --git a/.agents/skills/tauri-v2/references/capabilities-reference.md b/.agents/skills/tauri-v2/references/capabilities-reference.md new file mode 100644 index 00000000..cee11dc2 --- /dev/null +++ b/.agents/skills/tauri-v2/references/capabilities-reference.md @@ -0,0 +1,333 @@ +# Tauri v2 Capabilities & Permissions Reference + +## Overview + +Tauri v2 uses a capabilities-based security model. By default, **nothing is allowed** - you must explicitly grant permissions through capability files. + +## Capability File Structure + +Location: `src-tauri/capabilities/` + +```json +{ + "$schema": "../gen/schemas/desktop-schema.json", + "identifier": "capability-name", + "description": "What this capability allows", + "windows": ["main", "settings"], + "webviews": [], + "permissions": [ + "core:default", + "plugin-name:permission-name" + ] +} +``` + +## Core Permissions + +### Essential (Almost Always Needed) + +```json +{ + "permissions": [ + "core:default", + "core:window:default", + "core:event:default" + ] +} +``` + +### Window Permissions + +| Permission | Description | +|------------|-------------| +| `core:window:default` | Basic window operations | +| `core:window:allow-close` | Allow closing windows | +| `core:window:allow-set-title` | Allow changing window title | +| `core:window:allow-minimize` | Allow minimizing | +| `core:window:allow-maximize` | Allow maximizing | +| `core:window:allow-set-size` | Allow resizing | +| `core:window:allow-set-position` | Allow repositioning | +| `core:window:allow-set-fullscreen` | Allow fullscreen toggle | + +### Event Permissions + +| Permission | Description | +|------------|-------------| +| `core:event:default` | Basic event listening | +| `core:event:allow-emit` | Allow emitting events | +| `core:event:allow-listen` | Allow listening to events | + +## Plugin Permissions + +### File System (`tauri-plugin-fs`) + +```json +{ + "permissions": [ + "fs:default", + "fs:allow-read-dir", + "fs:allow-read-file", + "fs:allow-write-file", + "fs:allow-create-dir", + "fs:allow-remove-file", + "fs:allow-rename" + ] +} +``` + +**With Scopes:** +```json +{ + "permissions": [ + { + "identifier": "fs:allow-read-file", + "allow": [ + { "path": "$APPDATA/*" }, + { "path": "$HOME/Documents/*" } + ] + } + ] +} +``` + +### Dialog (`tauri-plugin-dialog`) + +```json +{ + "permissions": [ + "dialog:default", + "dialog:allow-open", + "dialog:allow-save", + "dialog:allow-message", + "dialog:allow-ask", + "dialog:allow-confirm" + ] +} +``` + +### Shell (`tauri-plugin-shell`) + +```json +{ + "permissions": [ + "shell:default", + "shell:allow-open", + "shell:allow-execute" + ] +} +``` + +**Scoped Execute:** +```json +{ + "permissions": [ + { + "identifier": "shell:allow-execute", + "allow": [ + { "name": "git", "args": true }, + { "name": "npm", "args": ["install", "run"] } + ] + } + ] +} +``` + +### HTTP (`tauri-plugin-http`) + +```json +{ + "permissions": [ + "http:default" + ] +} +``` + +**With URL Scopes:** +```json +{ + "permissions": [ + { + "identifier": "http:default", + "allow": [ + { "url": "https://api.example.com/*" }, + { "url": "https://*.myapp.com/*" } + ] + } + ] +} +``` + +### Store (`tauri-plugin-store`) + +```json +{ + "permissions": [ + "store:default", + "store:allow-get", + "store:allow-set", + "store:allow-delete", + "store:allow-keys", + "store:allow-clear" + ] +} +``` + +### Clipboard (`tauri-plugin-clipboard-manager`) + +```json +{ + "permissions": [ + "clipboard-manager:default", + "clipboard-manager:allow-read", + "clipboard-manager:allow-write" + ] +} +``` + +### Notification (`tauri-plugin-notification`) + +```json +{ + "permissions": [ + "notification:default", + "notification:allow-send", + "notification:allow-request-permission" + ] +} +``` + +### Global Shortcut (`tauri-plugin-global-shortcut`) + +```json +{ + "permissions": [ + "global-shortcut:default", + "global-shortcut:allow-register", + "global-shortcut:allow-unregister" + ] +} +``` + +## Platform-Specific Capabilities + +```json +{ + "identifier": "desktop-only", + "platforms": ["linux", "macos", "windows"], + "permissions": ["global-shortcut:default"] +} +``` + +```json +{ + "identifier": "mobile-only", + "platforms": ["iOS", "android"], + "permissions": ["biometric:default", "haptics:default"] +} +``` + +## Remote URL Access + +Allow Tauri commands from remote URLs: + +```json +{ + "identifier": "remote-access", + "remote": { + "urls": ["https://*.myapp.com"] + }, + "permissions": ["http:default"] +} +``` + +## Custom Permission Files + +Create custom permissions in `src-tauri/permissions/`: + +**`custom.toml`:** +```toml +[[permission]] +identifier = "allow-home-documents" +description = "Allow access to home documents" +commands.allow = ["read_file", "write_file"] + +[[scope.allow]] +path = "$HOME/Documents/**" +``` + +Reference in capability: +```json +{ + "permissions": ["custom:allow-home-documents"] +} +``` + +## Capability Best Practices + +1. **Principle of Least Privilege**: Only grant what's needed +2. **Use Scopes**: Limit file/URL access to specific paths +3. **Separate Capabilities**: Create focused capability files for different features +4. **Platform-Specific**: Use platform filtering for platform-specific features +5. **Document**: Add descriptions to explain why permissions are needed + +## Common Capability Patterns + +### Minimal App + +```json +{ + "identifier": "minimal", + "windows": ["main"], + "permissions": ["core:default"] +} +``` + +### File Manager + +```json +{ + "identifier": "file-manager", + "windows": ["main"], + "permissions": [ + "core:default", + "fs:default", + "dialog:allow-open", + "dialog:allow-save" + ] +} +``` + +### Web-Connected App + +```json +{ + "identifier": "web-app", + "windows": ["main"], + "permissions": [ + "core:default", + "http:default", + "shell:allow-open" + ] +} +``` + +### Full Desktop App + +```json +{ + "identifier": "full-desktop", + "windows": ["main"], + "permissions": [ + "core:default", + "core:window:default", + "core:event:default", + "fs:default", + "dialog:default", + "shell:default", + "clipboard-manager:default", + "notification:default", + "global-shortcut:default", + "store:default" + ] +} +``` diff --git a/.agents/skills/tauri-v2/references/ipc-patterns.md b/.agents/skills/tauri-v2/references/ipc-patterns.md new file mode 100644 index 00000000..1d2543f2 --- /dev/null +++ b/.agents/skills/tauri-v2/references/ipc-patterns.md @@ -0,0 +1,459 @@ +# Tauri v2 IPC Patterns Reference + +## Overview + +Tauri v2 provides three IPC primitives: +1. **Commands**: Request-response (most common) +2. **Events**: Fire-and-forget notifications +3. **Channels**: High-frequency streaming + +## Commands (invoke) + +### Basic Command + +**Rust:** +```rust +#[tauri::command] +fn greet(name: String) -> String { + format!("Hello, {}!", name) +} + +// Register in builder +tauri::Builder::default() + .invoke_handler(tauri::generate_handler![greet]) +``` + +**Frontend:** +```typescript +import { invoke } from '@tauri-apps/api/core'; + +const result = await invoke('greet', { name: 'World' }); +``` + +### Command with Multiple Arguments + +**Rust:** +```rust +#[tauri::command] +fn calculate(a: i32, b: i32, operation: String) -> i32 { + match operation.as_str() { + "add" => a + b, + "sub" => a - b, + "mul" => a * b, + "div" => a / b, + _ => 0, + } +} +``` + +**Frontend:** +```typescript +const result = await invoke('calculate', { + a: 10, + b: 5, + operation: 'add' +}); +``` + +### Async Command + +**Rust:** +```rust +#[tauri::command] +async fn fetch_data(url: String) -> Result { + // Use owned types (String, not &str) in async commands + let response = reqwest::get(&url) + .await + .map_err(|e| e.to_string())?; + + response.text() + .await + .map_err(|e| e.to_string()) +} +``` + +**Frontend:** +```typescript +try { + const data = await invoke('fetch_data', { url: 'https://api.example.com' }); +} catch (error) { + console.error('Failed:', error); +} +``` + +### Command with Result Error Handling + +**Rust:** +```rust +use thiserror::Error; + +#[derive(Debug, Error)] +enum AppError { + #[error("File not found: {0}")] + NotFound(String), + #[error("IO error: {0}")] + Io(#[from] std::io::Error), + #[error("Permission denied")] + PermissionDenied, +} + +impl serde::Serialize for AppError { + fn serialize(&self, serializer: S) -> Result + where S: serde::ser::Serializer { + serializer.serialize_str(self.to_string().as_ref()) + } +} + +#[tauri::command] +fn read_config(path: String) -> Result { + if !std::path::Path::new(&path).exists() { + return Err(AppError::NotFound(path)); + } + // ... +} +``` + +**Frontend:** +```typescript +try { + const config = await invoke('read_config', { path: '/config.json' }); +} catch (error) { + // error is the serialized error string + console.error('Config error:', error); +} +``` + +### Command with State + +**Rust:** +```rust +use std::sync::Mutex; +use tauri::State; + +struct AppState { + counter: u32, + items: Vec, +} + +#[tauri::command] +fn get_count(state: State<'_, Mutex>) -> u32 { + state.lock().unwrap().counter +} + +#[tauri::command] +fn increment(state: State<'_, Mutex>) -> u32 { + let mut s = state.lock().unwrap(); + s.counter += 1; + s.counter +} + +#[tauri::command] +fn add_item(item: String, state: State<'_, Mutex>) { + state.lock().unwrap().items.push(item); +} + +// In builder: +tauri::Builder::default() + .manage(Mutex::new(AppState { counter: 0, items: vec![] })) + .invoke_handler(tauri::generate_handler![get_count, increment, add_item]) +``` + +### Command with Window Access + +**Rust:** +```rust +use tauri::{WebviewWindow, AppHandle}; + +#[tauri::command] +fn get_window_info(window: WebviewWindow) -> String { + format!("Window label: {}", window.label()) +} + +#[tauri::command] +fn create_window(app: AppHandle) -> Result<(), String> { + tauri::WebviewWindowBuilder::new( + &app, + "new-window", + tauri::WebviewUrl::App("index.html".into()) + ) + .title("New Window") + .build() + .map_err(|e| e.to_string())?; + Ok(()) +} +``` + +### Command with Raw Binary Data + +**Rust:** +```rust +use tauri::ipc::Response; + +#[tauri::command] +fn read_binary_file(path: String) -> Result { + let data = std::fs::read(&path).map_err(|e| e.to_string())?; + Ok(Response::new(data)) // Avoids JSON serialization overhead +} + +#[tauri::command] +fn upload_file(request: tauri::ipc::Request) -> Result<(), String> { + let tauri::ipc::InvokeBody::Raw(data) = request.body() else { + return Err("Expected raw body".into()); + }; + std::fs::write("upload.bin", data).map_err(|e| e.to_string()) +} +``` + +**Frontend:** +```typescript +// Reading binary +const data = await invoke('read_binary_file', { path: '/file.bin' }); + +// Uploading binary +const fileData = new Uint8Array([1, 2, 3, 4]); +await invoke('upload_file', fileData); +``` + +--- + +## Events + +### Emit from Rust to Frontend + +**Rust:** +```rust +use tauri::Emitter; + +#[tauri::command] +fn start_background_task(app: tauri::AppHandle) { + std::thread::spawn(move || { + for i in 0..100 { + std::thread::sleep(std::time::Duration::from_millis(100)); + app.emit("progress", i).unwrap(); + } + app.emit("complete", "Task finished").unwrap(); + }); +} + +// Emit to specific window +#[tauri::command] +fn notify_window(app: tauri::AppHandle, window_label: String, message: String) { + app.emit_to(&window_label, "notification", message).unwrap(); +} +``` + +**Frontend:** +```typescript +import { listen, once } from '@tauri-apps/api/event'; + +// Listen continuously +const unlisten = await listen('progress', (event) => { + console.log(`Progress: ${event.payload}%`); +}); + +// Listen once +await once('complete', (event) => { + console.log(event.payload); +}); + +// Clean up when done +unlisten(); +``` + +### Emit from Frontend to Rust + +**Frontend:** +```typescript +import { emit } from '@tauri-apps/api/event'; + +await emit('user-action', { action: 'click', target: 'button' }); +``` + +**Rust (in setup or command):** +```rust +use tauri::Listener; + +fn setup_listeners(app: &tauri::App) { + app.listen("user-action", |event| { + println!("User action: {:?}", event.payload()); + }); +} +``` + +### Window-Specific Events + +**Rust:** +```rust +use tauri::{Emitter, WebviewWindow}; + +#[tauri::command] +fn emit_to_window(window: WebviewWindow, message: String) { + window.emit("window-message", message).unwrap(); +} +``` + +--- + +## Channels (Streaming) + +### Basic Channel Pattern + +**Rust:** +```rust +use tauri::ipc::Channel; + +#[derive(Clone, serde::Serialize)] +struct ProgressUpdate { + current: u32, + total: u32, + message: String, +} + +#[tauri::command] +async fn process_files( + files: Vec, + on_progress: Channel +) -> Result<(), String> { + let total = files.len() as u32; + + for (i, file) in files.iter().enumerate() { + // Process file... + on_progress.send(ProgressUpdate { + current: i as u32 + 1, + total, + message: format!("Processing {}", file), + }).unwrap(); + } + + Ok(()) +} +``` + +**Frontend:** +```typescript +import { invoke, Channel } from '@tauri-apps/api/core'; + +interface ProgressUpdate { + current: number; + total: number; + message: string; +} + +const channel = new Channel(); +channel.onmessage = (update) => { + const percent = (update.current / update.total) * 100; + console.log(`${percent}% - ${update.message}`); +}; + +await invoke('process_files', { + files: ['file1.txt', 'file2.txt'], + onProgress: channel +}); +``` + +### Tagged Union Events (Discriminated) + +**Rust:** +```rust +use tauri::ipc::Channel; + +#[derive(Clone, serde::Serialize)] +#[serde(tag = "event", content = "data")] +enum DownloadEvent { + Started { url: String, size: u64 }, + Progress { downloaded: u64, total: u64 }, + Complete { path: String }, + Error { message: String }, +} + +#[tauri::command] +async fn download_file( + url: String, + on_event: Channel +) -> Result { + on_event.send(DownloadEvent::Started { + url: url.clone(), + size: 1000, + }).unwrap(); + + for i in 0..=100 { + on_event.send(DownloadEvent::Progress { + downloaded: i * 10, + total: 1000, + }).unwrap(); + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + + let path = "/downloads/file.zip".to_string(); + on_event.send(DownloadEvent::Complete { + path: path.clone(), + }).unwrap(); + + Ok(path) +} +``` + +**Frontend:** +```typescript +import { invoke, Channel } from '@tauri-apps/api/core'; + +type DownloadEvent = + | { event: 'Started'; data: { url: string; size: number } } + | { event: 'Progress'; data: { downloaded: number; total: number } } + | { event: 'Complete'; data: { path: string } } + | { event: 'Error'; data: { message: string } }; + +const channel = new Channel(); +channel.onmessage = (msg) => { + switch (msg.event) { + case 'Started': + console.log(`Starting download: ${msg.data.url} (${msg.data.size} bytes)`); + break; + case 'Progress': + const percent = (msg.data.downloaded / msg.data.total) * 100; + console.log(`Download: ${percent.toFixed(1)}%`); + break; + case 'Complete': + console.log(`Downloaded to: ${msg.data.path}`); + break; + case 'Error': + console.error(`Download failed: ${msg.data.message}`); + break; + } +}; + +const path = await invoke('download_file', { + url: 'https://example.com/file.zip', + onEvent: channel +}); +``` + +--- + +## IPC Selection Guide + +| Pattern | Use Case | Direction | Frequency | +|---------|----------|-----------|-----------| +| **Commands** | Request-response, data fetching | Frontend β†’ Rust | One-time | +| **Events** | Notifications, state changes | Bidirectional | Low-medium | +| **Channels** | Progress updates, streaming data | Rust β†’ Frontend | High | + +### When to Use Each + +**Commands (invoke)** +- Fetching data from Rust +- Performing actions that return results +- CRUD operations +- Most common pattern + +**Events (emit/listen)** +- Notifying UI of background changes +- Broadcasting to multiple windows +- Fire-and-forget notifications +- System events (window close, minimize) + +**Channels** +- File download/upload progress +- Long-running operations with updates +- Streaming log output +- Real-time data feeds diff --git a/skills-lock.json b/skills-lock.json new file mode 100644 index 00000000..5cd67a5e --- /dev/null +++ b/skills-lock.json @@ -0,0 +1,25 @@ +{ + "version": 1, + "skills": { + "rust-best-practices": { + "source": "apollographql/skills", + "sourceType": "github", + "computedHash": "fd336f2f772eb46ddf8658de5f9dd8faa1f35615b316665307fc82eeb4b2e658" + }, + "svelte-code-writer": { + "source": "sveltejs/ai-tools", + "sourceType": "github", + "computedHash": "c0e2cce9855f8e312cbb0a05aef164b4659c672d7723e4e598ffa6bc94890542" + }, + "svelte-core-bestpractices": { + "source": "sveltejs/ai-tools", + "sourceType": "github", + "computedHash": "bec11e369679027edf9d4acbe6d9788f8da33dc14b48b874f231d3ba13705324" + }, + "tauri-v2": { + "source": "nodnarbnitram/claude-code-extensions", + "sourceType": "github", + "computedHash": "72248dbe36769a12c38c07437063cc5a1ad2d182fee87fb72d92a8fa37ce4a0f" + } + } +}