Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

warning: failed to parse serde attribute #[serde(rename_all = "camelCase")] on enum struct variant #135

Closed
ctison opened this issue Dec 9, 2022 · 9 comments · Fixed by #185

Comments

@ctison
Copy link

ctison commented Dec 9, 2022

Hello,

I set #[serde(rename_all = "camelCase")1 on enum struct variants:

#[derive(Serialize, Deserialize, TS, Clone)]
#[ts(export, export_to = "../src/model/")]
#[serde(rename_all = "camelCase")]
pub enum TaskStatus {
  #[serde(rename_all = "camelCase")]
  Running { started_time: chrono::DateTime<Utc> },
  #[serde(rename_all = "camelCase")]
  Terminated {
    status: i32,
    stdout: String,
    stderr: String,
  },
}

But I have the following warnings:

warning: failed to parse serde attribute
  | 
  | #[serde(rename_all = "camelCase")]
  | 
  = note: ts-rs failed to parse this attribute. It will be ignored.

And the generated Typescript for the enum struct variants' fields are in snake_case (eg. started_time).

Version: ts-rs = { version = "6.2.1", features = ["chrono-impl", "uuid-impl"] }

Footnotes

  1. https://serde.rs/variant-attrs.html#rename_all

@linulas
Copy link

linulas commented Mar 22, 2023

It seems to be an issue with most serde field attributes, I get the same with #[serde(skip_serializing_if = "Option::is_none")], and there's a pull request for fixing the same issue for [serde(with = "...")]: #146

@escritorio-gustavo
Copy link
Contributor

escritorio-gustavo commented Jan 11, 2024

This has already been fixed by #185, sorry I failed to include this issue in the PR before it was merged, I tried to add all issues about this, but I seem to have missed this one.
@ctison, if you see this, please close the issue as completed

@NyxCode
Copy link
Collaborator

NyxCode commented Jan 11, 2024

As @escritorio-gustavo mentioned, the no-serde-warnings feature can disable these warnings.
I think I'd still like to keep this issue open, in case we want to support serde's rename_all in the future.

@NyxCode
Copy link
Collaborator

NyxCode commented Jan 11, 2024

right now, it's sadly neccessary to rename each field individually with #[serde(rename = "..")] or #[ts(rename = "..")]

@escritorio-gustavo
Copy link
Contributor

My mistake, due to #165 I thought #[serde(rename_all = "..")] was generally supported and this issue was just older than support for it, I didn't notice it didn't apply to enums

@escritorio-gustavo
Copy link
Contributor

right now, it's sadly neccessary to rename each field individually with #[serde(rename = "..")] or #[ts(rename = "..")]

Hey @NyxCode... I have forked the repo to start working on a fix.
I wrote the following test:

use serde::{Serialize, Deserialize};
use ts_rs::TS;

/// Used enum from issue to test
#[derive(Serialize, Deserialize, TS, Clone)]
#[ts(export)]
#[serde(rename_all = "camelCase")]
pub enum TaskStatus {
  #[serde(rename_all = "camelCase")]
  Running { started_time: String }, // Changed to string because the `chrono` dependency doesn't include the `serde` flag
  #[serde(rename_all = "camelCase")]
  Terminated {
    status: i32,
    stdout: String,
    stderr: String,
  },
}

#[test]
pub fn enum_struct_variant() {
    assert_eq!(TaskStatus::inline(), r#"{ "running": { startedTime: string, } } | { "terminated": { status: number, stdout: string, stderr: string, } }"#)
}

And the test is passing... Has the rename_all been fixed in the time since this issue was opened in a forgotten commit or am I doing something wrong?

@escritorio-gustavo
Copy link
Contributor

The format_field function used to format enums containng structs does receive a rename_all parameter

fn format_field(
    formatted_fields: &mut Vec<TokenStream>,
    dependencies: &mut Dependencies,
    field: &Field,
    rename_all: &Option<Inflection>,
    generics: &Generics,
) -> Result<()> 

@NyxCode
Copy link
Collaborator

NyxCode commented Jan 20, 2024

@escritorio-gustavo Hey!
Thanks for testing that, it seems you're right!
Maybe I got tripped up by an inconsistency our camelCase implementation has with serde (I'll add an other issue for that), idk.

Since you've already got the test, could you make a PR for that?

@escritorio-gustavo
Copy link
Contributor

The rename_all fix was introduced in #122

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants