-
Notifications
You must be signed in to change notification settings - Fork 0
Description
Project Criticism: patchable
After a thorough audit of the code, documentation, and logic of the patchable project (v0.5.5), I have identified several critical issues that severely limit its utility and safety.
1. Misleading "Patch" Semantics (Critical)
The library claims to enable "efficient partial updates," but it does not support partial updates for leaf fields.
- The Issue: The generated
{Struct}Patchtype copies fields directly from the original struct without wrapping them inOption<T>.- If you have
struct User { name: String, age: u32 }, theUserPatchstruct also hasname: Stringandage: u32.
- If you have
- Consequence: You cannot construct a valid
UserPatchwithout providing all fields.- In JSON terms,
{"name": "Alice"}is invalid forUserPatchbecauseageis missing. - You must send
{"name": "Alice", "age": 10}. - This is a Replacement, not a Patch.
- In JSON terms,
- Impact: The library fails to implement the primary pattern expected of a patching library (e.g., JSON Merge Patch style partial updates). It is essentially a "Recursive Replacement" library, only useful if you have nested
#[patchable]structs, and even then, you must fully replace the fields at the current level.
2. Flawed "Read-Only" Handling (Major)
The #[patchable_model] helper macro has a dangerous default behavior regarding skipped fields.
- The Issue: If you mark a field as
#[patchable(skip)](e.g., to prevent users from updating a sensitive field likeidorrole), the macro automatically adds#[serde(skip)]to that field on the original struct.- Code:
if has_patchable_skip_attr(field) { field.attrs.push(parse_quote! { #[serde(skip)] }); }
- Code:
- Consequence: You cannot have "Read-Only" fields that are sent to the client but cannot be patched back.
- If you protect
idfrom patching, it disappears from your JSON responses entirely.
- If you protect
- Impact: This forces users to abandon the helper macro and manually derive everything if they need standard API behavior (read-only ID fields).
3. Severe Generic & Lifetime Limitations (Major)
The macro implementation is rigid and seemingly arbitrary in its restrictions.
- No Arguments in Generics: Explicitly rejects types with generic arguments for recursive patching.
#[patchable] config: Box<Config>is rejected.#[patchable] settings: Option<Settings>is rejected.#[patchable] list: Vec<Item>is rejected.
- No Lifetimes: The macro explicitly errors if any lifetimes are present (
GenericParam::Lifetime).- You cannot patch structs that hold references (e.g.,
struct View<'a> { data: &'a str }).
- You cannot patch structs that hold references (e.g.,
4. Fragile Macro Implementation
The patchable-macro crate uses string matching and manual AST traversal that is brittle.
- It scans for types by name to decide preservation logic.
- It parses attributes using custom logic (
patchable_attr_has_param) instead of usingsyn's structured parsing fully, making it prone to edge cases.
5. Redundancy
For flat structs, Patchable adds zero value over just serde::Deserialize into the original struct (since you have to provide all fields anyway). Its only theoretical value is hierarchical delegation, but that is crippled by the generic type limitations.
Recommendations
- Wrap Patch Fields in
Option<T>: This is the standard way to implement partial updates in Rust.Nonemeans "no change",Some(val)means "update". - Decouple Patching from Serialization: Do not force
#[serde(skip)]on non-patchable fields. - Use
synpropertly: Support complex generics and lifetimes by properly propagating them instead of rejecting them. - Rename or Redefine: If the goal is not partial updates but "state synchronization", clarity is needed. But even for state synchronization, the current design is limiting.