Skip to content

Commit

Permalink
Merge pull request #2855 from dtolnay/namespan
Browse files Browse the repository at this point in the history 
Produce unreachable_patterns warning when deserialization names collide
  • Loading branch information
@dtolnay
dtolnay authored Nov 11, 2024
2 parents 60ac737 + 373edcd commit 7f1e697
Show file tree
Hide file tree
Showing 6 changed files with 174 additions and 94 deletions.
5 changes: 3 additions & 2 deletions serde_derive/src/de.rs
View file
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
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
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
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

0 comments on commit 7f1e697

Please sign in to comment.