Skip to content

Commit

Permalink
Add checks for conflicts for aliases
Browse files Browse the repository at this point in the history
- Check that alias is not the same as name of other field (it still can be the name of owning field/variant)
- Check that aliases are unique, i. e. two different fields does not use the same alias
  • Loading branch information
Mingun committed Aug 8, 2023
1 parent 05a5b7e commit 4944daf
Show file tree
Hide file tree
Showing 5 changed files with 360 additions and 1 deletion.
136 changes: 135 additions & 1 deletion serde_derive/src/internals/check.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::internals::ast::{Container, Data, Field, Style};
use crate::internals::ast::{Container, Data, Field, Style, Variant};
use crate::internals::attr::{Default, Identifier, TagType};
use crate::internals::{ungroup, Ctxt, Derive};
use std::collections::btree_map::Entry;
use std::collections::{BTreeMap, BTreeSet};
use syn::{Member, Type};

// Cross-cutting checks that require looking at more than a single attrs object.
Expand All @@ -16,6 +18,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_name_conflicts(cx, cont, derive);
}

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

// Checks that aliases does not repeated
fn check_name_conflicts(cx: &Ctxt, cont: &Container, derive: Derive) {
if let Derive::Deserialize = derive {
match &cont.data {
Data::Enum(variants) => check_variant_name_conflicts(cx, &variants),
Data::Struct(Style::Struct, fields) => check_field_name_conflicts(cx, fields),
_ => {}
}
}
}

// All renames already applied
fn check_variant_name_conflicts(cx: &Ctxt, variants: &[Variant]) {
let names: BTreeSet<_> = variants
.iter()
.filter_map(|variant| {
if variant.attrs.skip_deserializing() {
None
} else {
Some(variant.attrs.name().deserialize_name().to_owned())
}
})
.collect();
let mut alias_owners = BTreeMap::new();

for variant in variants {
let name = variant.attrs.name().deserialize_name();

for alias in variant.attrs.aliases().intersection(&names) {
// Aliases contains variant names, so filter them out
if alias == name {
continue;
}

// TODO: report other variant location when this become possible
cx.error_spanned_by(
variant.original,
format!(
"alias `{}` conflicts with deserialization name of other variant",
alias
),
);
}

for alias in variant.attrs.aliases() {
// Aliases contains variant names, so filter them out
if alias == name {
continue;
}

match alias_owners.entry(alias) {
Entry::Vacant(e) => {
e.insert(variant);
}
Entry::Occupied(e) => {
// TODO: report other variant location when this become possible
cx.error_spanned_by(
variant.original,
format!(
"alias `{}` already used by variant {}",
alias,
e.get().original.ident
),
);
}
}
}

check_field_name_conflicts(cx, &variant.fields);
}
}

// All renames already applied
fn check_field_name_conflicts(cx: &Ctxt, fields: &[Field]) {
let names: BTreeSet<_> = fields
.iter()
.filter_map(|field| {
if field.attrs.skip_deserializing() {
None
} else {
Some(field.attrs.name().deserialize_name().to_owned())
}
})
.collect();
let mut alias_owners = BTreeMap::new();

for field in fields {
let name = field.attrs.name().deserialize_name();

for alias in field.attrs.aliases().intersection(&names) {
// Aliases contains field names, so filter them out
if alias == name {
continue;
}

// TODO: report other field location when this become possible
cx.error_spanned_by(
field.original,
format!(
"alias `{}` conflicts with deserialization name of other field",
alias
),
);
}

for alias in field.attrs.aliases() {
// Aliases contains field names, so filter them out
if alias == name {
continue;
}

match alias_owners.entry(alias) {
Entry::Vacant(e) => {
e.insert(field);
}
Entry::Occupied(e) => {
// TODO: report other field location when this become possible
cx.error_spanned_by(
field.original,
format!(
"alias `{}` already used by field {}",
alias,
e.get().original.ident.as_ref().unwrap()
),
);
}
}
}
}
}
79 changes: 79 additions & 0 deletions test_suite/tests/ui/conflict/alias-enum.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
#![allow(non_camel_case_types)]

use serde_derive::Deserialize;

#[derive(Deserialize)]
enum E {
S1 {
/// Expected error on "alias b", because this is a name of other field
/// Error on "alias a" is not expected because this is a name of this field
/// Error on "alias c" is not expected because field `c` is skipped
#[serde(alias = "a", alias = "b", alias = "c")]
a: (),

/// Expected error on "alias c", because it is already used as alias of `a`
#[serde(alias = "c")]
b: (),

#[serde(skip_deserializing)]
c: (),
},

S2 {
/// Expected error on "alias c", because this is a name of other field after
/// applying rename rules
#[serde(alias = "b", alias = "c")]
a: (),

#[serde(rename = "c")]
b: (),
},

#[serde(rename_all = "UPPERCASE")]
S3 {
/// Expected error on "alias B", because this is a name of field after
/// applying rename rules
#[serde(alias = "B", alias = "c")]
a: (),
b: (),
},
}

#[derive(Deserialize)]
enum E1 {
/// Expected error on "alias b", because this is a name of other variant
/// Error on "alias a" is not expected because this is a name of this variant
/// Error on "alias c" is not expected because variant `c` is skipped
#[serde(alias = "a", alias = "b", alias = "c")]
a,

/// Expected error on "alias c", because it is already used as alias of `a`
#[serde(alias = "c")]
b,

#[serde(skip_deserializing)]
c,
}

#[derive(Deserialize)]
enum E2 {
/// Expected error on "alias c", because this is a name of other variant after
/// applying rename rules
#[serde(alias = "b", alias = "c")]
a,

#[serde(rename = "c")]
b,
}

#[derive(Deserialize)]
#[serde(rename_all = "UPPERCASE")]
enum E3 {
/// Expected error on "alias B", because this is a name of variant after
/// applying rename rules
#[serde(alias = "B", alias = "c")]
a,
b,
}

fn main() {}
71 changes: 71 additions & 0 deletions test_suite/tests/ui/conflict/alias-enum.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
error: alias `b` conflicts with deserialization name of other field
--> tests/ui/conflict/alias-enum.rs:8:9
|
8 | / /// Expected error on "alias b", because this is a name of other field
9 | | /// Error on "alias a" is not expected because this is a name of this field
10 | | /// Error on "alias c" is not expected because field `c` is skipped
11 | | #[serde(alias = "a", alias = "b", alias = "c")]
12 | | a: (),
| |_____________^

error: alias `c` already used by field a
--> tests/ui/conflict/alias-enum.rs:14:9
|
14 | / /// Expected error on "alias c", because it is already used as alias of `a`
15 | | #[serde(alias = "c")]
16 | | b: (),
| |_____________^

error: alias `c` conflicts with deserialization name of other field
--> tests/ui/conflict/alias-enum.rs:23:9
|
23 | / /// Expected error on "alias c", because this is a name of other field after
24 | | /// applying rename rules
25 | | #[serde(alias = "b", alias = "c")]
26 | | a: (),
| |_____________^

error: alias `B` conflicts with deserialization name of other field
--> tests/ui/conflict/alias-enum.rs:34:9
|
34 | / /// Expected error on "alias B", because this is a name of field after
35 | | /// applying rename rules
36 | | #[serde(alias = "B", alias = "c")]
37 | | a: (),
| |_____________^

error: alias `b` conflicts with deserialization name of other variant
--> tests/ui/conflict/alias-enum.rs:44:5
|
44 | / /// Expected error on "alias b", because this is a name of other variant
45 | | /// Error on "alias a" is not expected because this is a name of this variant
46 | | /// Error on "alias c" is not expected because variant `c` is skipped
47 | | #[serde(alias = "a", alias = "b", alias = "c")]
48 | | a,
| |_____^

error: alias `c` already used by variant a
--> tests/ui/conflict/alias-enum.rs:50:5
|
50 | / /// Expected error on "alias c", because it is already used as alias of `a`
51 | | #[serde(alias = "c")]
52 | | b,
| |_____^

error: alias `c` conflicts with deserialization name of other variant
--> tests/ui/conflict/alias-enum.rs:60:5
|
60 | / /// Expected error on "alias c", because this is a name of other variant after
61 | | /// applying rename rules
62 | | #[serde(alias = "b", alias = "c")]
63 | | a,
| |_____^

error: alias `B` conflicts with deserialization name of other variant
--> tests/ui/conflict/alias-enum.rs:72:5
|
72 | / /// Expected error on "alias B", because this is a name of variant after
73 | | /// applying rename rules
74 | | #[serde(alias = "B", alias = "c")]
75 | | a,
| |_____^
40 changes: 40 additions & 0 deletions test_suite/tests/ui/conflict/alias.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
use serde_derive::Deserialize;

#[derive(Deserialize)]
struct S1 {
/// Expected error on "alias b", because this is a name of other field
/// Error on "alias a" is not expected because this is a name of this field
/// Error on "alias c" is not expected because field `c` is skipped
#[serde(alias = "a", alias = "b", alias = "c")]
a: (),

/// Expected error on "alias c", because it is already used as alias of `a`
#[serde(alias = "c")]
b: (),

#[serde(skip_deserializing)]
c: (),
}

#[derive(Deserialize)]
struct S2 {
/// Expected error on "alias c", because this is a name of other field after
/// applying rename rules
#[serde(alias = "b", alias = "c")]
a: (),

#[serde(rename = "c")]
b: (),
}

#[derive(Deserialize)]
#[serde(rename_all = "UPPERCASE")]
struct S3 {
/// Expected error on "alias B", because this is a name of field after
/// applying rename rules
#[serde(alias = "B", alias = "c")]
a: (),
b: (),
}

fn main() {}
35 changes: 35 additions & 0 deletions test_suite/tests/ui/conflict/alias.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error: alias `b` conflicts with deserialization name of other field
--> tests/ui/conflict/alias.rs:5:5
|
5 | / /// Expected error on "alias b", because this is a name of other field
6 | | /// Error on "alias a" is not expected because this is a name of this field
7 | | /// Error on "alias c" is not expected because field `c` is skipped
8 | | #[serde(alias = "a", alias = "b", alias = "c")]
9 | | a: (),
| |_________^

error: alias `c` already used by field a
--> tests/ui/conflict/alias.rs:11:5
|
11 | / /// Expected error on "alias c", because it is already used as alias of `a`
12 | | #[serde(alias = "c")]
13 | | b: (),
| |_________^

error: alias `c` conflicts with deserialization name of other field
--> tests/ui/conflict/alias.rs:21:5
|
21 | / /// Expected error on "alias c", because this is a name of other field after
22 | | /// applying rename rules
23 | | #[serde(alias = "b", alias = "c")]
24 | | a: (),
| |_________^

error: alias `B` conflicts with deserialization name of other field
--> tests/ui/conflict/alias.rs:33:5
|
33 | / /// Expected error on "alias B", because this is a name of field after
34 | | /// applying rename rules
35 | | #[serde(alias = "B", alias = "c")]
36 | | a: (),
| |_________^

0 comments on commit 4944daf

Please sign in to comment.