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

Emit an error if a variant of an untagged enum is annoted with #[serde(rename)] or #[serde(alias)] #2797

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion serde_derive/src/internals/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl<'a> Container<'a> {
derive: Derive,
) -> Option<Container<'a>> {
let mut attrs = attr::Container::from_ast(cx, item);

let mut data = match &item.data {
syn::Data::Enum(data) => Data::Enum(enum_from_ast(cx, &data.variants, attrs.default())),
syn::Data::Struct(data) => {
Expand Down
9 changes: 7 additions & 2 deletions serde_derive/src/internals/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,14 @@ impl Name {
pub fn deserialize_name(&self) -> &str {
&self.deserialize
}

fn deserialize_aliases(&self) -> &BTreeSet<String> {
&self.deserialize_aliases
}

pub fn renamed(&self) -> bool {
self.serialize_renamed || self.deserialize_renamed
}
}

#[derive(Copy, Clone)]
Expand Down Expand Up @@ -241,6 +245,7 @@ pub struct Container {
}

/// Styles of representing an enum.
#[derive(PartialEq)]
pub enum TagType {
/// The default.
///
Expand Down Expand Up @@ -324,7 +329,7 @@ impl Container {
let mut serde_path = Attr::none(cx, CRATE);
let mut expecting = Attr::none(cx, EXPECTING);
let mut non_exhaustive = false;

for attr in &item.attrs {
if attr.path() != SERDE {
non_exhaustive |=
Expand Down
34 changes: 34 additions & 0 deletions serde_derive/src/internals/check.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::internals::ast::{Container, Data, Field, Style};
use crate::internals::attr::{Default, Identifier, TagType};
use crate::internals::case::RenameRule;
use crate::internals::{ungroup, Ctxt, Derive};
use syn::{Member, Type};

Expand All @@ -16,6 +17,7 @@ pub fn check(cx: &Ctxt, cont: &mut Container, derive: Derive) {
check_adjacent_tag_conflict(cx, cont);
check_transparent(cx, cont, derive);
check_from_and_try_from(cx, cont);
check_untagged_enum(cx, cont);
}

// If some field of a tuple struct is marked #[serde(default)] then all fields
Expand Down Expand Up @@ -475,3 +477,35 @@ fn check_from_and_try_from(cx: &Ctxt, cont: &mut Container) {
);
}
}

fn check_untagged_enum(cx: &Ctxt, cont: &mut Container) {
let variants = match &cont.data {
Data::Enum(variants) => variants,
Data::Struct(_, _) => return,
};
if cont.attrs.tag() == &TagType::None {
let rename_all_rules = &cont.attrs.rename_all_rules();
if rename_all_rules.serialize != RenameRule::None || rename_all_rules.deserialize != RenameRule::None {
cx.error_spanned_by(
cont.original,
"#[serde(rename_all = \"...\")] does nothing when used with #[serde(untagged)]",
);
}

for variant in variants {
if variant.attrs.aliases().len() > 1 {
cx.error_spanned_by(
variant.original,
"#[serde(alias = \"...\")] does nothing when used on a variant of an untagged enum",
);
}

if variant.attrs.name().renamed() {
cx.error_spanned_by(
variant.original,
"#[serde(rename = \"...\")] does nothing when used on a variant of an untagged enum",
);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use serde_derive::{Serialize, Deserialize};

#[derive(Serialize, Deserialize)]
#[serde(untagged, rename_all = "lowercase")]
enum E {
#[serde(alias = "foo")]
A(u8),
#[serde(rename = "bar")]
B(String),
}
fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
error: #[serde(rename_all = "...")] does nothing when used with #[serde(untagged)]
--> tests/ui/enum-representation/untagged-and-variant-renamed.rs:4:1
|
4 | / #[serde(untagged, rename_all = "lowercase")]
5 | | enum E {
6 | | #[serde(alias = "foo")]
7 | | A(u8),
8 | | #[serde(rename = "bar")]
9 | | B(String),
10 | | }
| |_^

error: #[serde(alias = "...")] does nothing when used on a variant of an untagged enum
--> tests/ui/enum-representation/untagged-and-variant-renamed.rs:6:5
|
6 | / #[serde(alias = "foo")]
7 | | A(u8),
| |_________^

error: #[serde(rename = "...")] does nothing when used on a variant of an untagged enum
--> tests/ui/enum-representation/untagged-and-variant-renamed.rs:8:5
|
8 | / #[serde(rename = "bar")]
9 | | B(String),
| |_____________^