Serde Code Review

Serde Code Review

Review Workflow

1. Check Cargo.toml — Note serde features (derive, rc), format crates (serde_json, toml, bincode, etc.), and Rust edition (2024 has breaking changes affecting serde code) 2. Check derive usage — Verify Serialize and Deserialize are derived appropriately 3. Check enum representations — Enum tagging affects wire format compatibility and readability 4. Check field attributes — Renaming, defaults, skipping affect API contracts 5. Check edition 2024 compatibility — Reserved gen keyword, RPIT lifetime capture changes, never_type_fallback 6. Verify round-trip correctness — Serialized data must deserialize back to the same value

Gates (before reporting findings)

Run in order. Do not write a finding until the step that applies has passed.

1. Serde context on disk — Pass when: You have read the relevant Cargo.toml (crate or workspace root) and can state Rust edition, serde / serde_derive features if non-default (derive, rc), and which format crates apply (serde_json, toml, bincode, etc.) for the code under review. Then apply edition-specific checklist items (e.g. gen, RPIT/never_type_fallback) only when that file supports them.

2. Per-finding evidence — Pass when: Each issue cites [FILE:LINE] from the current tree for the struct/enum, Serialize/Deserialize impl, or attribute block in question (not from memory, docs-only, or another branch).

3. Category check vs protocol — Pass when: For the finding type (derive attrs, enum tagging, flatten, custom impl, sqlx + serde alignment), you ran the matching checks from the review-verification-protocol skill (e.g. full type definition + serde attrs before “wrong representation”; confirmed edition in Cargo.toml before edition-2024-only findings). Then add the finding.

4. Output shape — Pass when: The report lines match Output Format below (severity + description).

Output Format

Report findings as:

[FILE:LINE] ISSUE_TITLE
Severity: Critical | Major | Minor | Informational
Description of the issue and why it matters.

Quick Reference

Review Checklist

Derive Usage

• [ ] #[derive(Serialize, Deserialize)] on types that cross serialization boundaries
• [ ] #[derive(Debug)] alongside serde derives (debugging serialization issues)
• [ ] Feature-gated derives when serde is optional: #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
• [ ] Prefer #[expect(unused)] over #[allow(unused)] for serde-only fields (self-cleaning lint suppression, stable since 1.81)

Enum Representation

• [ ] Enum tagging is explicit (not relying on serde's default externally-tagged format when another is intended)
• [ ] Tag names are stable and won't collide with field names
• [ ] #[serde(rename_all = "...")] used consistently across the API

Field Configuration

• [ ] #[serde(skip_serializing_if = "Option::is_none")] for optional fields (clean JSON output)
• [ ] #[serde(default)] for fields that should have fallback values during deserialization
• [ ] #[serde(rename = "...")] when Rust field names differ from wire format
• [ ] #[serde(flatten)] used judiciously (can cause key collisions)
• [ ] No #[serde(deny_unknown_fields)] on types that need forward compatibility
• [ ] No fields or variants named gen — reserved keyword in edition 2024 (use r#gen or rename)

Database Integration (sqlx)

• [ ] #[derive(sqlx::Type)] enums use consistent representation with serde
• [ ] Enum variant casing matches between serde (rename_all) and sqlx (rename_all)

Edition 2024 Compatibility

• [ ] No fields or enum variants named gen (reserved keyword — use r#gen with #[serde(rename = "gen")] or choose a different name)
• [ ] Custom Serialize/Deserialize impls returning impl Trait account for RPIT lifetime capture changes (all in-scope lifetimes captured by default; use + use<'a> for precise control)
• [ ] Deserialization error paths handle never_type_fallback — ! falls back to ! instead of (), which affects match exhaustiveness on Result<T, !> patterns

Correctness

• [ ] Round-trip tests exist for complex types (serialize → deserialize → assert_eq)
• [ ] PartialEq derived for types with round-trip tests
• [ ] No lossy conversions (e.g., f64 → i64 in JSON numbers)
• [ ] Decimal used for money/precision-sensitive values, not f64

Severity Calibration

Critical

• Enum representation mismatch between serializer and deserializer (data loss)
• Missing #[serde(rename)] causing API-breaking field name changes
• #[serde(flatten)] causing silent key collisions
• Lossy numeric conversions (f64 precision loss for monetary values)

Major

• Inconsistent rename_all across related types (confusing API)
• Missing skip_serializing_if causing null/empty noise in output
• deny_unknown_fields on types consumed by evolving APIs (breaks forward compatibility)
• Missing round-trip tests for complex enum representations
• Field or variant named gen without r#gen escape (edition 2024 compile failure)

Minor

• Unnecessary #[serde(default)] on required fields
• Using string representation for enums when numeric would be more efficient
• Verbose custom implementations where derive + attributes suffice
• Using #[allow(unused)] instead of #[expect(unused)] for serde-only fields (prefer self-cleaning lint suppression)

Informational

• Suggestions to switch enum representation for cleaner wire format
• Suggestions to add #[non_exhaustive] alongside serde for forward compatibility

Valid Patterns (Do NOT Flag)

• Externally tagged enums — serde's default, valid for many use cases
• `#[serde(untagged)]` enums — Valid when discriminated by structure, not by tag
• `serde_json::Value` for dynamic data — Appropriate for truly schema-less fields
• `#[serde(skip)]` on computed fields — Correct for derived/cached values
• `#[serde(with = "...")]` for custom formats — Standard for dates, UUIDs, etc.
• `r#gen` with `#[serde(rename = "gen")]` — Correct edition 2024 workaround for gen fields in wire formats
• `+ use<'a>` on custom serializer return types — Precise RPIT lifetime capture (edition 2024)

Before Submitting Findings

Complete Gates (before reporting findings) above; gate 3 incorporates the review-verification-protocol skill for serde-related issue types.

Custom Serialization

When Custom Implementation is Needed

Derive handles most cases. Custom implementations are warranted for:

• Format-specific representations (dates, UUIDs, durations)
• Validation during deserialization
• Backwards-compatible format changes
• Types from external crates without serde support

serde(with) Module Pattern

The cleanest approach for custom field serialization. Create a module with serialize and deserialize functions:

mod iso_date {
    use chrono::{DateTime, Utc};
    use serde::{self, Deserialize, Deserializer, Serializer};

    pub fn serialize<S>(date: &DateTime<Utc>, s: S) -> Result<S::Ok, S::Error>
    where S: Serializer {
        s.serialize_str(&date.to_rfc3339())
    }

    pub fn deserialize<'de, D>(d: D) -> Result<DateTime<Utc>, D::Error>
    where D: Deserializer<'de> {
        let s = String::deserialize(d)?;
        DateTime::parse_from_rfc3339(&s)
            .map(|dt| dt.with_timezone(&Utc))
            .map_err(serde::de::Error::custom)
    }
}

#[derive(Serialize, Deserialize, PartialEq, Debug)]
struct Event {
    #[serde(with = "iso_date")]
    created_at: DateTime<Utc>,
}

For Option<T> fields, use serialize_with + deserialize_with separately or use the serde_with crate.

Validating Deserialization

When deserialized data needs validation, implement Deserialize manually or use #[serde(try_from)]:

#[derive(Serialize, Deserialize)]
#[serde(try_from = "String")]
struct Email(String);

impl TryFrom<String> for Email {
    type Error = String;
    fn try_from(s: String) -> Result<Self, Self::Error> {
        if s.contains('@') {
            Ok(Email(s))
        } else {
            Err(format!("invalid email: {s}"))
        }
    }
}
// Deserialization now validates automatically

Edition 2024: RPIT Lifetime Capture in Custom Serializers

In edition 2024, -> impl Trait captures ALL in-scope lifetimes by default. This affects custom serialization helpers that return impl Trait — particularly deserializer combinators and visitor factories.

// Edition 2021: only captures lifetimes explicitly in bounds
// Edition 2024: captures 'de AND 'a by default
fn make_visitor<'de, 'a>(context: &'a str) -> impl Visitor<'de> {
    MyVisitor { context }
}

If you need the returned type NOT to capture a lifetime, use the precise capture syntax:

// GOOD — explicitly captures only 'de, excludes 'a
fn make_visitor<'de, 'a>(context: &'a str) -> impl Visitor<'de> + use<'de> {
    MyVisitor { context: context.to_owned() }
}

Most serde with modules are unaffected because they return Result, not impl Trait. This primarily impacts advanced patterns: custom visitor factories, deserializer adapters, and combinator libraries that return opaque types.

Edition 2024: never_type_fallback and Deserialization Errors

In edition 2024, the ! (never) type falls back to ! instead of (). This can surface in deserialization code that uses infallible patterns or match expressions on Result<T, !>:

// Edition 2021: ! falls back to (), match is exhaustive
// Edition 2024: ! falls back to !, may change type inference

// If you have a custom deserializer that returns Result<T, !> for infallible paths,
// match arms and type inference may behave differently. Prefer explicit error types:

// BAD — relies on never type fallback behavior
fn infallible_deserialize<T: Default>() -> Result<T, !> {
    Ok(T::default())
}

// GOOD — use a concrete error type even for infallible paths
fn infallible_deserialize<T: Default>() -> Result<T, serde::de::value::Error> {
    Ok(T::default())
}

In practice, most serde code uses serde::de::Error trait bounds and concrete error types, so this is a low-frequency issue. Flag it when you see explicit ! in deserialization return types.

Common Pitfalls

Lossy Numeric Conversions

JSON numbers are IEEE 754 doubles. Values outside f64 precision range get silently truncated.

// BAD for monetary values - f64 loses precision
#[derive(Serialize, Deserialize)]
struct Invoice {
    amount: f64,  // 0.1 + 0.2 ≠ 0.3
}

// GOOD - use decimal types
use rust_decimal::Decimal;

#[derive(Serialize, Deserialize)]
struct Invoice {
    amount: Decimal,  // exact decimal arithmetic
}

Untagged Enum Ambiguity

With #[serde(untagged)], serde tries variants in declaration order. If two variants can match the same input, the first wins silently.

// AMBIGUOUS - both variants match {"value": 42}
#[serde(untagged)]
enum Data {
    Full { value: i64, extra: Option<String> },
    Simple { value: i64 },
}
// Always deserializes as Full (tried first)

deny_unknown_fields Breaks Forward Compatibility

Adding #[serde(deny_unknown_fields)] means older code fails to deserialize data from newer versions that add fields.

// Version 1: works fine
#[derive(Deserialize)]
#[serde(deny_unknown_fields)]
struct Config { port: u16 }

// Version 2 adds `host` field
// Now V1 code fails on V2 config files — breaking change

Use deny_unknown_fields only for strict input validation (user-facing forms, CLI config) where unknown fields indicate user error.

Round-Trip Testing

Every type with custom serialization should have a round-trip test:

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn round_trip_event() {
        let original = Event {
            created_at: Utc::now(),
        };
        let json = serde_json::to_string(&original).unwrap();
        let deserialized: Event = serde_json::from_str(&json).unwrap();
        assert_eq!(original, deserialized);
    }

    #[test]
    fn deserialize_from_known_format() {
        // Test against a known JSON string to catch format regressions
        let json = r#"{"created_at": "2024-01-15T10:30:00Z"}"#;
        let event: Event = serde_json::from_str(json).unwrap();
        assert_eq!(event.created_at.year(), 2024);
    }
}

Review Questions

1. Are custom serialization modules (with) used instead of full manual implementations where possible? 2. Is try_from used for validating deserialization? 3. Are monetary/precision values using Decimal, not f64? 4. Are untagged enums free from variant ambiguity? 5. Is deny_unknown_fields avoided on evolving APIs? 6. Do custom serializations have round-trip tests? 7. Do custom serializer/deserializer helpers returning impl Trait account for edition 2024 RPIT lifetime capture? 8. Are deserialization error types concrete (not relying on ! type fallback)?

Derive Patterns

Enum Tagging Strategies

Serde supports four enum representations. The choice affects wire format, readability, and compatibility.

Externally Tagged (Default)

#[derive(Serialize, Deserialize)]
enum Message {
    Text(String),
    Image { url: String, alt: String },
}
// JSON: {"Text": "hello"} or {"Image": {"url": "...", "alt": "..."}}

Simple but produces awkward JSON for variants with data.

Internally Tagged

#[derive(Serialize, Deserialize)]
#[serde(tag = "type")]
enum Message {
    Text { content: String },
    Image { url: String, alt: String },
}
// JSON: {"type": "Text", "content": "hello"}

Clean and common for API types. Requires all variants to be struct-like (no tuple variants). The tag field name must not collide with variant field names.

Adjacently Tagged

#[derive(Serialize, Deserialize)]
#[serde(tag = "type", content = "data")]
enum Message {
    Text(String),
    Image { url: String, alt: String },
}
// JSON: {"type": "Text", "data": "hello"}

Supports both tuple and struct variants. Good for message protocols where type and payload are separate.

Untagged

#[derive(Serialize, Deserialize)]
#[serde(untagged)]
enum ApiResponse {
    Success { data: Value },
    Error { error: String, code: u32 },
}
// JSON: {"data": {...}} or {"error": "...", "code": 404}

Discriminated by structure, not a tag. Serde tries each variant in order until one succeeds. Watch for ambiguity — if two variants could match the same input, serde uses the first match.

Field Attributes

rename_all

Converts Rust's snake_case to the wire format's convention.

#[derive(Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
struct ApiResponse {
    user_name: String,      // → "userName"
    created_at: DateTime,   // → "createdAt"
}

#[derive(Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
enum Status {
    InProgress,  // → "in_progress"
    Complete,    // → "complete"
}

Common values: camelCase, snake_case, SCREAMING_SNAKE_CASE, kebab-case, lowercase, UPPERCASE.

skip_serializing_if

Omits fields from output when a condition is true. Essential for clean API responses.

#[derive(Serialize)]
struct User {
    name: String,
    #[serde(skip_serializing_if = "Option::is_none")]
    email: Option<String>,
    #[serde(skip_serializing_if = "Vec::is_empty")]
    tags: Vec<String>,
}
// With email=None, tags=[]: {"name": "Alice"}
// Without skip: {"name": "Alice", "email": null, "tags": []}

default

Provides fallback values during deserialization when a field is missing.

#[derive(Deserialize)]
struct Config {
    host: String,
    #[serde(default = "default_port")]
    port: u16,
    #[serde(default)]  // uses Default::default()
    debug: bool,
}

fn default_port() -> u16 { 8080 }

flatten

Inlines a struct's fields into the parent. Useful for composition but can cause subtle issues.

#[derive(Serialize, Deserialize)]
struct Request {
    id: Uuid,
    #[serde(flatten)]
    metadata: Metadata,  // metadata fields appear at top level
}

#[derive(Serialize, Deserialize)]
struct Metadata {
    timestamp: DateTime<Utc>,
    source: String,
}
// JSON: {"id": "...", "timestamp": "...", "source": "..."}

Pitfall: If Request and Metadata have a field with the same name, one silently wins. Use #[serde(flatten)] only when field names are guaranteed not to collide.

Edition 2024: Reserved gen Keyword

In Rust edition 2024, gen is a reserved keyword. Any serde field or enum variant named gen will fail to compile. Use r#gen as the Rust identifier and #[serde(rename)] to preserve the wire format name.

// BAD — fails to compile on edition 2024
#[derive(Serialize, Deserialize)]
struct Model {
    gen: u32,
}

// GOOD — compiles on edition 2024, wire format unchanged
#[derive(Serialize, Deserialize)]
struct Model {
    #[serde(rename = "gen")]
    r#gen: u32,
}

// GOOD — enum variant
#[derive(Serialize, Deserialize)]
enum Phase {
    #[serde(rename = "gen")]
    Generation,
    Evaluation,
}

This also applies to #[serde(alias = "gen")] — the alias string is fine, but the Rust identifier must use r#gen.

Edition 2024: #[expect] for Serde-Only Fields

Fields that exist solely for deserialization (e.g., skipped during serialization) may trigger unused warnings. Prefer #[expect(dead_code)] over #[allow(dead_code)] — it warns you when the suppression becomes unnecessary.

// BAD — allow stays forever even if the field becomes used
#[allow(dead_code)]
#[serde(skip_serializing)]
legacy_id: Option<String>,

// GOOD — expect warns when suppression is no longer needed
#[expect(dead_code)]
#[serde(skip_serializing)]
legacy_id: Option<String>,

Database Type Alignment

When types are used with both serde and sqlx, keep representations consistent:

#[derive(Debug, Clone, Serialize, Deserialize, sqlx::Type)]
#[serde(rename_all = "snake_case")]
#[sqlx(type_name = "varchar", rename_all = "snake_case")]
pub enum Status {
    Pending,
    InProgress,
    Complete,
}

Both serde and sqlx will use "pending", "in_progress", "complete". Mismatched casing between the two causes bugs that are hard to trace.

Review Questions

1. Is the enum tagging strategy explicit and appropriate for the wire format? 2. Is rename_all consistent across related types? 3. Are optional fields using skip_serializing_if for clean output? 4. Does #[serde(flatten)] risk field name collisions? 5. Do serde and sqlx enum representations match? 6. Are any fields or variants named gen (edition 2024 reserved keyword)? 7. Are lint suppressions on serde-only fields using #[expect] instead of #[allow]?