Skip to content

Commit 5f9fffa

Browse files
MingunMingun
committed
Add checks for conflicts for aliases
- 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
1 parent 58a8d22 commit 5f9fffa

File tree

5 files changed

+360
-1
lines changed

5 files changed

+360
-1
lines changed

‎serde_derive/src/internals/check.rs‎

Lines changed: 135 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1-
use crate::internals::ast::{Container, Data, Field, Style};
1+
use crate::internals::ast::{Container, Data, Field, Style, Variant};
22
use crate::internals::attr::{Default, Identifier, TagType};
33
use crate::internals::{ungroup, Ctxt, Derive};
4+
use std::collections::btree_map::Entry;
5+
use std::collections::{BTreeMap, BTreeSet};
46
use syn::{Member, Type};
57

68
// Cross-cutting checks that require looking at more than a single attrs object.
@@ -16,6 +18,7 @@ pub fn check(cx: &Ctxt, cont: &mut Container, derive: Derive) {
1618
check_adjacent_tag_conflict(cx, cont);
1719
check_transparent(cx, cont, derive);
1820
check_from_and_try_from(cx, cont);
21+
check_name_conflicts(cx, cont, derive);
1922
}
2023

2124
// If some field of a tuple struct is marked #[serde(default)] then all fields
@@ -475,3 +478,134 @@ fn check_from_and_try_from(cx: &Ctxt, cont: &mut Container) {
475478
);
476479
}
477480
}
481+
482+
// Checks that aliases does not repeated
483+
fn check_name_conflicts(cx: &Ctxt, cont: &Container, derive: Derive) {
484+
if let Derive::Deserialize = derive {
485+
match &cont.data {
486+
Data::Enum(variants) => check_variant_name_conflicts(cx, &variants),
487+
Data::Struct(Style::Struct, fields) => check_field_name_conflicts(cx, fields),
488+
_ => {}
489+
}
490+
}
491+
}
492+
493+
// All renames already applied
494+
fn check_variant_name_conflicts(cx: &Ctxt, variants: &[Variant]) {
495+
let names: BTreeSet<_> = variants
496+
.iter()
497+
.filter_map(|variant| {
498+
if variant.attrs.skip_deserializing() {
499+
None
500+
} else {
501+
Some(variant.attrs.name().deserialize_name().to_owned())
502+
}
503+
})
504+
.collect();
505+
let mut alias_owners = BTreeMap::new();
506+
507+
for variant in variants {
508+
let name = variant.attrs.name().deserialize_name();
509+
510+
for alias in variant.attrs.aliases().intersection(&names) {
511+
// Aliases contains variant names, so filter them out
512+
if alias == name {
513+
continue;
514+
}
515+
516+
// TODO: report other variant location when this become possible
517+
cx.error_spanned_by(
518+
variant.original,
519+
format!(
520+
"alias `{}` conflicts with deserialization name of other variant",
521+
alias
522+
),
523+
);
524+
}
525+
526+
for alias in variant.attrs.aliases() {
527+
// Aliases contains variant names, so filter them out
528+
if alias == name {
529+
continue;
530+
}
531+
532+
match alias_owners.entry(alias) {
533+
Entry::Vacant(e) => {
534+
e.insert(variant);
535+
}
536+
Entry::Occupied(e) => {
537+
// TODO: report other variant location when this become possible
538+
cx.error_spanned_by(
539+
variant.original,
540+
format!(
541+
"alias `{}` already used by variant {}",
542+
alias,
543+
e.get().original.ident
544+
),
545+
);
546+
}
547+
}
548+
}
549+
550+
check_field_name_conflicts(cx, &variant.fields);
551+
}
552+
}
553+
554+
// All renames already applied
555+
fn check_field_name_conflicts(cx: &Ctxt, fields: &[Field]) {
556+
let names: BTreeSet<_> = fields
557+
.iter()
558+
.filter_map(|field| {
559+
if field.attrs.skip_deserializing() {
560+
None
561+
} else {
562+
Some(field.attrs.name().deserialize_name().to_owned())
563+
}
564+
})
565+
.collect();
566+
let mut alias_owners = BTreeMap::new();
567+
568+
for field in fields {
569+
let name = field.attrs.name().deserialize_name();
570+
571+
for alias in field.attrs.aliases().intersection(&names) {
572+
// Aliases contains field names, so filter them out
573+
if alias == name {
574+
continue;
575+
}
576+
577+
// TODO: report other field location when this become possible
578+
cx.error_spanned_by(
579+
field.original,
580+
format!(
581+
"alias `{}` conflicts with deserialization name of other field",
582+
alias
583+
),
584+
);
585+
}
586+
587+
for alias in field.attrs.aliases() {
588+
// Aliases contains field names, so filter them out
589+
if alias == name {
590+
continue;
591+
}
592+
593+
match alias_owners.entry(alias) {
594+
Entry::Vacant(e) => {
595+
e.insert(field);
596+
}
597+
Entry::Occupied(e) => {
598+
// TODO: report other field location when this become possible
599+
cx.error_spanned_by(
600+
field.original,
601+
format!(
602+
"alias `{}` already used by field {}",
603+
alias,
604+
e.get().original.ident.as_ref().unwrap()
605+
),
606+
);
607+
}
608+
}
609+
}
610+
}
611+
}

‎test_suite/tests/ui/conflict/alias-enum.rs‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
#![allow(non_camel_case_types)]
2+
3+
use serde_derive::Deserialize;
4+
5+
#[derive(Deserialize)]
6+
enum E {
7+
S1 {
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: (),
13+
14+
/// Expected error on "alias c", because it is already used as alias of `a`
15+
#[serde(alias = "c")]
16+
b: (),
17+
18+
#[serde(skip_deserializing)]
19+
c: (),
20+
},
21+
22+
S2 {
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: (),
27+
28+
#[serde(rename = "c")]
29+
b: (),
30+
},
31+
32+
#[serde(rename_all = "UPPERCASE")]
33+
S3 {
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: (),
38+
b: (),
39+
},
40+
}
41+
42+
#[derive(Deserialize)]
43+
enum E1 {
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,
49+
50+
/// Expected error on "alias c", because it is already used as alias of `a`
51+
#[serde(alias = "c")]
52+
b,
53+
54+
#[serde(skip_deserializing)]
55+
c,
56+
}
57+
58+
#[derive(Deserialize)]
59+
enum E2 {
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,
64+
65+
#[serde(rename = "c")]
66+
b,
67+
}
68+
69+
#[derive(Deserialize)]
70+
#[serde(rename_all = "UPPERCASE")]
71+
enum E3 {
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,
76+
b,
77+
}
78+
79+
fn main() {}

‎test_suite/tests/ui/conflict/alias-enum.stderr‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
error: alias `b` conflicts with deserialization name of other field
2+
--> tests/ui/conflict/alias-enum.rs:8:9
3+
|
4+
8 | / /// Expected error on "alias b", because this is a name of other field
5+
9 | | /// Error on "alias a" is not expected because this is a name of this field
6+
10 | | /// Error on "alias c" is not expected because field `c` is skipped
7+
11 | | #[serde(alias = "a", alias = "b", alias = "c")]
8+
12 | | a: (),
9+
| |_____________^
10+
11+
error: alias `c` already used by field a
12+
--> tests/ui/conflict/alias-enum.rs:14:9
13+
|
14+
14 | / /// Expected error on "alias c", because it is already used as alias of `a`
15+
15 | | #[serde(alias = "c")]
16+
16 | | b: (),
17+
| |_____________^
18+
19+
error: alias `c` conflicts with deserialization name of other field
20+
--> tests/ui/conflict/alias-enum.rs:23:9
21+
|
22+
23 | / /// Expected error on "alias c", because this is a name of other field after
23+
24 | | /// applying rename rules
24+
25 | | #[serde(alias = "b", alias = "c")]
25+
26 | | a: (),
26+
| |_____________^
27+
28+
error: alias `B` conflicts with deserialization name of other field
29+
--> tests/ui/conflict/alias-enum.rs:34:9
30+
|
31+
34 | / /// Expected error on "alias B", because this is a name of field after
32+
35 | | /// applying rename rules
33+
36 | | #[serde(alias = "B", alias = "c")]
34+
37 | | a: (),
35+
| |_____________^
36+
37+
error: alias `b` conflicts with deserialization name of other variant
38+
--> tests/ui/conflict/alias-enum.rs:44:5
39+
|
40+
44 | / /// Expected error on "alias b", because this is a name of other variant
41+
45 | | /// Error on "alias a" is not expected because this is a name of this variant
42+
46 | | /// Error on "alias c" is not expected because variant `c` is skipped
43+
47 | | #[serde(alias = "a", alias = "b", alias = "c")]
44+
48 | | a,
45+
| |_____^
46+
47+
error: alias `c` already used by variant a
48+
--> tests/ui/conflict/alias-enum.rs:50:5
49+
|
50+
50 | / /// Expected error on "alias c", because it is already used as alias of `a`
51+
51 | | #[serde(alias = "c")]
52+
52 | | b,
53+
| |_____^
54+
55+
error: alias `c` conflicts with deserialization name of other variant
56+
--> tests/ui/conflict/alias-enum.rs:60:5
57+
|
58+
60 | / /// Expected error on "alias c", because this is a name of other variant after
59+
61 | | /// applying rename rules
60+
62 | | #[serde(alias = "b", alias = "c")]
61+
63 | | a,
62+
| |_____^
63+
64+
error: alias `B` conflicts with deserialization name of other variant
65+
--> tests/ui/conflict/alias-enum.rs:72:5
66+
|
67+
72 | / /// Expected error on "alias B", because this is a name of variant after
68+
73 | | /// applying rename rules
69+
74 | | #[serde(alias = "B", alias = "c")]
70+
75 | | a,
71+
| |_____^

‎test_suite/tests/ui/conflict/alias.rs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
use serde_derive::Deserialize;
2+
3+
#[derive(Deserialize)]
4+
struct S1 {
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: (),
10+
11+
/// Expected error on "alias c", because it is already used as alias of `a`
12+
#[serde(alias = "c")]
13+
b: (),
14+
15+
#[serde(skip_deserializing)]
16+
c: (),
17+
}
18+
19+
#[derive(Deserialize)]
20+
struct S2 {
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: (),
25+
26+
#[serde(rename = "c")]
27+
b: (),
28+
}
29+
30+
#[derive(Deserialize)]
31+
#[serde(rename_all = "UPPERCASE")]
32+
struct S3 {
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: (),
37+
b: (),
38+
}
39+
40+
fn main() {}

‎test_suite/tests/ui/conflict/alias.stderr‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
error: alias `b` conflicts with deserialization name of other field
2+
--> tests/ui/conflict/alias.rs:5:5
3+
|
4+
5 | / /// Expected error on "alias b", because this is a name of other field
5+
6 | | /// Error on "alias a" is not expected because this is a name of this field
6+
7 | | /// Error on "alias c" is not expected because field `c` is skipped
7+
8 | | #[serde(alias = "a", alias = "b", alias = "c")]
8+
9 | | a: (),
9+
| |_________^
10+
11+
error: alias `c` already used by field a
12+
--> tests/ui/conflict/alias.rs:11:5
13+
|
14+
11 | / /// Expected error on "alias c", because it is already used as alias of `a`
15+
12 | | #[serde(alias = "c")]
16+
13 | | b: (),
17+
| |_________^
18+
19+
error: alias `c` conflicts with deserialization name of other field
20+
--> tests/ui/conflict/alias.rs:21:5
21+
|
22+
21 | / /// Expected error on "alias c", because this is a name of other field after
23+
22 | | /// applying rename rules
24+
23 | | #[serde(alias = "b", alias = "c")]
25+
24 | | a: (),
26+
| |_________^
27+
28+
error: alias `B` conflicts with deserialization name of other field
29+
--> tests/ui/conflict/alias.rs:33:5
30+
|
31+
33 | / /// Expected error on "alias B", because this is a name of field after
32+
34 | | /// applying rename rules
33+
35 | | #[serde(alias = "B", alias = "c")]
34+
36 | | a: (),
35+
| |_________^

0 commit comments

Comments
 (0)