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.

Sign up for GitHub

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

Produce unreachable_patterns warning when deserialization names collide #2855

Merged
merged 3 commits into from
Nov 11, 2024
Merged

Produce unreachable_patterns warning when deserialization names collide #2855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3035d4f
Rename Name -> MultiName
dtolnay Nov 11, 2024
f0b5c4f
Move MultiName to a new module
dtolnay Nov 11, 2024
373edcd
Keep track of a span for alias strings
dtolnay Nov 11, 2024
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
5 changes: 3 additions & 2 deletions serde_derive/src/de.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::fragment::{Expr, Fragment, Match, Stmts};
use crate::internals::ast::{Container, Data, Field, Style, Variant};
use crate::internals::name::Name;
use crate::internals::{attr, replace_receiver, ungroup, Ctxt, Derive};
use crate::{bound, dummy, pretend, this};
use proc_macro2::{Literal, Span, TokenStream};
Expand Down Expand Up @@ -2002,7 +2003,7 @@ fn deserialize_untagged_newtype_variant(

struct FieldWithAliases<'a> {
ident: Ident,
aliases: &'a BTreeSet<String>,
aliases: &'a BTreeSet<Name>,
}

fn deserialize_generated_identifier(
Expand Down Expand Up @@ -2224,7 +2225,7 @@ fn deserialize_identifier(
let aliases = field
.aliases
.iter()
.map(|alias| Literal::byte_string(alias.as_bytes()));
.map(|alias| Literal::byte_string(alias.value.as_bytes()));
quote!(#(#aliases)|* => _serde::__private::Ok(#this_value::#ident))
});

Expand Down
132 changes: 48 additions & 84 deletions serde_derive/src/internals/attr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::internals::name::{MultiName, Name};
use crate::internals::symbol::*;
use crate::internals::{ungroup, Ctxt};
use proc_macro2::{Spacing, Span, TokenStream, TokenTree};
Expand All @@ -21,7 +22,7 @@ use syn::{parse_quote, token, Ident, Lifetime, Token};

pub use crate::internals::case::RenameRule;

struct Attr<'c, T> {
pub(crate) struct Attr<'c, T> {
cx: &'c Ctxt,
name: Symbol,
tokens: TokenStream,
Expand Down Expand Up @@ -62,7 +63,7 @@ impl<'c, T> Attr<'c, T> {
}
}

fn get(self) -> Option<T> {
pub(crate) fn get(self) -> Option<T> {
self.value
}

Expand Down Expand Up @@ -90,7 +91,7 @@ impl<'c> BoolAttr<'c> {
}
}

struct VecAttr<'c, T> {
pub(crate) struct VecAttr<'c, T> {
cx: &'c Ctxt,
name: Symbol,
first_dup_tokens: TokenStream,
Expand Down Expand Up @@ -125,63 +126,13 @@ impl<'c, T> VecAttr<'c, T> {
}
}

fn get(self) -> Vec<T> {
pub(crate) fn get(self) -> Vec<T> {
self.values
}
}

pub struct Name {
serialize: String,
serialize_renamed: bool,
deserialize: String,
deserialize_renamed: bool,
deserialize_aliases: BTreeSet<String>,
}

fn unraw(ident: &Ident) -> String {
ident.to_string().trim_start_matches("r#").to_owned()
}

impl Name {
fn from_attrs(
source_name: String,
ser_name: Attr<String>,
de_name: Attr<String>,
de_aliases: Option<VecAttr<String>>,
) -> Name {
let mut alias_set = BTreeSet::new();
if let Some(de_aliases) = de_aliases {
for alias_name in de_aliases.get() {
alias_set.insert(alias_name);
}
}

let ser_name = ser_name.get();
let ser_renamed = ser_name.is_some();
let de_name = de_name.get();
let de_renamed = de_name.is_some();
Name {
serialize: ser_name.unwrap_or_else(|| source_name.clone()),
serialize_renamed: ser_renamed,
deserialize: de_name.unwrap_or(source_name),
deserialize_renamed: de_renamed,
deserialize_aliases: alias_set,
}
}

/// Return the container name for the container when serializing.
pub fn serialize_name(&self) -> &str {
&self.serialize
}

/// Return the container name for the container when deserializing.
pub fn deserialize_name(&self) -> &str {
&self.deserialize
}

fn deserialize_aliases(&self) -> &BTreeSet<String> {
&self.deserialize_aliases
}
fn unraw(ident: &Ident) -> Ident {
Ident::new(ident.to_string().trim_start_matches("r#"), ident.span())
}

#[derive(Copy, Clone)]
Expand All @@ -203,7 +154,7 @@ impl RenameAllRules {

/// Represents struct or enum attribute information.
pub struct Container {
name: Name,
name: MultiName,
transparent: bool,
deny_unknown_fields: bool,
default: Default,
Expand Down Expand Up @@ -327,8 +278,8 @@ impl Container {
// #[serde(rename = "foo")]
// #[serde(rename(serialize = "foo", deserialize = "bar"))]
let (ser, de) = get_renames(cx, RENAME, &meta)?;
ser_name.set_opt(&meta.path, ser.as_ref().map(syn::LitStr::value));
de_name.set_opt(&meta.path, de.as_ref().map(syn::LitStr::value));
ser_name.set_opt(&meta.path, ser.as_ref().map(Name::from));
de_name.set_opt(&meta.path, de.as_ref().map(Name::from));
} else if meta.path == RENAME_ALL {
// #[serde(rename_all = "foo")]
// #[serde(rename_all(serialize = "foo", deserialize = "bar"))]
Expand Down Expand Up @@ -567,7 +518,7 @@ impl Container {
}

Container {
name: Name::from_attrs(unraw(&item.ident), ser_name, de_name, None),
name: MultiName::from_attrs(Name::from(&unraw(&item.ident)), ser_name, de_name, None),
transparent: transparent.get(),
deny_unknown_fields: deny_unknown_fields.get(),
default: default.get().unwrap_or(Default::None),
Expand All @@ -594,7 +545,7 @@ impl Container {
}
}

pub fn name(&self) -> &Name {
pub fn name(&self) -> &MultiName {
&self.name
}

Expand Down Expand Up @@ -781,7 +732,7 @@ fn decide_identifier(

/// Represents variant attribute information
pub struct Variant {
name: Name,
name: MultiName,
rename_all_rules: RenameAllRules,
ser_bound: Option<Vec<syn::WherePredicate>>,
de_bound: Option<Vec<syn::WherePredicate>>,
Expand Down Expand Up @@ -832,15 +783,15 @@ impl Variant {
// #[serde(rename = "foo")]
// #[serde(rename(serialize = "foo", deserialize = "bar"))]
let (ser, de) = get_multiple_renames(cx, &meta)?;
ser_name.set_opt(&meta.path, ser.as_ref().map(syn::LitStr::value));
ser_name.set_opt(&meta.path, ser.as_ref().map(Name::from));
for de_value in de {
de_name.set_if_none(de_value.value());
de_aliases.insert(&meta.path, de_value.value());
de_name.set_if_none(Name::from(&de_value));
de_aliases.insert(&meta.path, Name::from(&de_value));
}
} else if meta.path == ALIAS {
// #[serde(alias = "foo")]
if let Some(s) = get_lit_str(cx, ALIAS, &meta)? {
de_aliases.insert(&meta.path, s.value());
de_aliases.insert(&meta.path, Name::from(&s));
}
} else if meta.path == RENAME_ALL {
// #[serde(rename_all = "foo")]
Expand Down Expand Up @@ -947,7 +898,12 @@ impl Variant {
}

Variant {
name: Name::from_attrs(unraw(&variant.ident), ser_name, de_name, Some(de_aliases)),
name: MultiName::from_attrs(
Name::from(&unraw(&variant.ident)),
ser_name,
de_name,
Some(de_aliases),
),
rename_all_rules: RenameAllRules {
serialize: rename_all_ser_rule.get().unwrap_or(RenameRule::None),
deserialize: rename_all_de_rule.get().unwrap_or(RenameRule::None),
Expand All @@ -964,20 +920,23 @@ impl Variant {
}
}

pub fn name(&self) -> &Name {
pub fn name(&self) -> &MultiName {
&self.name
}

pub fn aliases(&self) -> &BTreeSet<String> {
pub fn aliases(&self) -> &BTreeSet<Name> {
self.name.deserialize_aliases()
}

pub fn rename_by_rules(&mut self, rules: RenameAllRules) {
if !self.name.serialize_renamed {
self.name.serialize = rules.serialize.apply_to_variant(&self.name.serialize);
self.name.serialize.value =
rules.serialize.apply_to_variant(&self.name.serialize.value);
}
if !self.name.deserialize_renamed {
self.name.deserialize = rules.deserialize.apply_to_variant(&self.name.deserialize);
self.name.deserialize.value = rules
.deserialize
.apply_to_variant(&self.name.deserialize.value);
}
self.name
.deserialize_aliases
Expand Down Expand Up @@ -1023,7 +982,7 @@ impl Variant {

/// Represents field attribute information
pub struct Field {
name: Name,
name: MultiName,
skip_serializing: bool,
skip_deserializing: bool,
skip_serializing_if: Option<syn::ExprPath>,
Expand Down Expand Up @@ -1082,8 +1041,11 @@ impl Field {
let mut flatten = BoolAttr::none(cx, FLATTEN);

let ident = match &field.ident {
Some(ident) => unraw(ident),
None => index.to_string(),
Some(ident) => Name::from(&unraw(ident)),
None => Name {
value: index.to_string(),
span: Span::call_site(),
},
};

if let Some(borrow_attribute) = attrs.and_then(|variant| variant.borrow.as_ref()) {
Expand Down Expand Up @@ -1119,15 +1081,15 @@ impl Field {
// #[serde(rename = "foo")]
// #[serde(rename(serialize = "foo", deserialize = "bar"))]
let (ser, de) = get_multiple_renames(cx, &meta)?;
ser_name.set_opt(&meta.path, ser.as_ref().map(syn::LitStr::value));
ser_name.set_opt(&meta.path, ser.as_ref().map(Name::from));
for de_value in de {
de_name.set_if_none(de_value.value());
de_aliases.insert(&meta.path, de_value.value());
de_name.set_if_none(Name::from(&de_value));
de_aliases.insert(&meta.path, Name::from(&de_value));
}
} else if meta.path == ALIAS {
// #[serde(alias = "foo")]
if let Some(s) = get_lit_str(cx, ALIAS, &meta)? {
de_aliases.insert(&meta.path, s.value());
de_aliases.insert(&meta.path, Name::from(&s));
}
} else if meta.path == DEFAULT {
if meta.input.peek(Token![=]) {
Expand Down Expand Up @@ -1290,7 +1252,7 @@ impl Field {
}

Field {
name: Name::from_attrs(ident, ser_name, de_name, Some(de_aliases)),
name: MultiName::from_attrs(ident, ser_name, de_name, Some(de_aliases)),
skip_serializing: skip_serializing.get(),
skip_deserializing: skip_deserializing.get(),
skip_serializing_if: skip_serializing_if.get(),
Expand All @@ -1306,20 +1268,22 @@ impl Field {
}
}

pub fn name(&self) -> &Name {
pub fn name(&self) -> &MultiName {
&self.name
}

pub fn aliases(&self) -> &BTreeSet<String> {
pub fn aliases(&self) -> &BTreeSet<Name> {
self.name.deserialize_aliases()
}

pub fn rename_by_rules(&mut self, rules: RenameAllRules) {
if !self.name.serialize_renamed {
self.name.serialize = rules.serialize.apply_to_field(&self.name.serialize);
self.name.serialize.value = rules.serialize.apply_to_field(&self.name.serialize.value);
}
if !self.name.deserialize_renamed {
self.name.deserialize = rules.deserialize.apply_to_field(&self.name.deserialize);
self.name.deserialize.value = rules
.deserialize
.apply_to_field(&self.name.deserialize.value);
}
self.name
.deserialize_aliases
Expand Down Expand Up @@ -1769,7 +1733,7 @@ fn is_primitive_path(path: &syn::Path, primitive: &str) -> bool {
// attribute on the field so there must be at least one borrowable lifetime.
fn borrowable_lifetimes(
cx: &Ctxt,
name: &str,
name: &Name,
field: &syn::Field,
) -> Result<BTreeSet<syn::Lifetime>, ()> {
let mut lifetimes = BTreeSet::new();
Expand Down
4 changes: 2 additions & 2 deletions serde_derive/src/internals/check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,13 +332,13 @@ fn check_internal_tag_field_name_conflict(cx: &Ctxt, cont: &Container) {
let name = field.attrs.name();
let ser_name = name.serialize_name();

if check_ser && ser_name == tag {
if check_ser && ser_name.value == tag {
diagnose_conflict();
return;
}

for de_name in field.attrs.aliases() {
if check_de && de_name == tag {
if check_de && de_name.value == tag {
diagnose_conflict();
return;
}
Expand Down
1 change: 1 addition & 0 deletions serde_derive/src/internals/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
pub mod ast;
pub mod attr;
pub mod name;

mod case;
mod check;
Expand Down
Loading