Skip to content

Remove conflicting serde attributes from single-field structs#1271

Closed
oguzkocer wants to merge 1 commit intotrunkfrom
fix/serde-transparent-attributes
Closed

Remove conflicting serde attributes from single-field structs#1271
oguzkocer wants to merge 1 commit intotrunkfrom
fix/serde-transparent-attributes

Conversation

@oguzkocer
Copy link
Copy Markdown
Contributor

@oguzkocer oguzkocer commented Apr 8, 2026

Summary

Removed #[serde(transparent)] and conflicting #[serde(rename)] attributes from single-field wrapper structs.

Changes

  • Removed #[serde(transparent)] from 6 structs:

    • UserCapabilitiesMap
    • PostTypeSupportsMap
    • SparsePostTypesResponse
    • SparseTaxonomyTypesResponse
    • SparseMenuLocationsResponse
    • SparsePostStatusesResponse
  • Removed #[serde(rename)] from UserCapabilitiesMap and PostTypeSupportsMap

Rationale

For single-field structs, #[serde(flatten)] on the field is functionally equivalent to #[serde(transparent)] on the struct.

#[serde(transparent)] is not supported by:

  • WpContextual (generates concrete types without transparent)
  • WpDeserialize (explicitly rejects transparent attribute)

#[serde(rename)] conflicts with #[serde(flatten)] which takes all keys from the parent object.

For single-field structs, #[serde(flatten)] on the field is functionally
equivalent to #[serde(transparent)] on the struct.

Removed #[serde(transparent)] because it's not supported by WpContextual
(generated types) and WpDeserialize (sparse types).

Also removed #[serde(rename)] from flattened fields as it conflicts with
the flatten behavior.
@oguzkocer oguzkocer added the Rust label Apr 8, 2026
@oguzkocer oguzkocer changed the title Remove #[serde(transparent)] and conflicting #[serde(rename)] Remove conflicting serde attributes from single-field structs Apr 8, 2026
@wpmobilebot
Copy link
Copy Markdown
Collaborator

XCFramework Build

This PR's XCFramework is available for testing. Add to your Package.swift:

.package(url: "https://github.com/automattic/wordpress-rs", branch: "pr-build/1271")

Built from 07aa8ab

@oguzkocer oguzkocer marked this pull request as ready for review April 8, 2026 01:42
@oguzkocer oguzkocer enabled auto-merge (squash) April 8, 2026 01:42
@oguzkocer oguzkocer requested a review from jkmassel April 8, 2026 01:42
Copy link
Copy Markdown
Contributor

@jkmassel jkmassel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this approach could fail (though it's probably unlikely).

For example:

// wp_api/src/post_types.rs — inside mod test {}
#[test]
fn test_post_type_supports_map_from_empty_array() {
    // WordPress returns `[]` instead of `{}` when a post type has no
    // supported features (e.g. `register_post_type('foo', ['supports' => false])`).
    let json = r#"[]"#;
    let supports: PostTypeSupportsMap =
        serde_json::from_str(json).expect("Should handle empty array from WordPress API");
    assert!(supports.map.is_empty());
}

// wp_api/src/users.rs — inside mod tests {}
#[test]
fn test_user_capabilities_map_from_empty_array() {
    // WordPress returns `[]` instead of `{}` when a map is empty.
    let json = r#"[]"#;
    let caps: UserCapabilitiesMap =
        serde_json::from_str(json).expect("Should handle empty array from WordPress API");
    assert!(caps.map.is_empty());
}

It's probably vanishingly rare, but I think we need to handle the empty case?

@oguzkocer
Copy link
Copy Markdown
Contributor Author

@jkmassel Thank you for catching that. I'm working on a different way to address this conflict, so I'll close this PR and open a new one when I am ready. I'll also make sure to add the tests you provided so they are part of our coverage.

@oguzkocer oguzkocer closed this Apr 8, 2026
auto-merge was automatically disabled April 8, 2026 17:12

Pull request was closed

@oguzkocer oguzkocer deleted the fix/serde-transparent-attributes branch April 8, 2026 17:12
@oguzkocer
Copy link
Copy Markdown
Contributor Author

#1273 should address the issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants