mikros/serde
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #2857 from dtolnay/collide

Browse Source 
Revert the colliding aliases hard error (PRs #2562 & #2853)
This commit is contained in:
David Tolnay 2024-11-10 23:37:01 -08:00 committed by GitHub
commit 7d96352e96
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 109 additions and 223 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

136
serde_derive/src/internals/check.rs
View File

@ -1,8 +1,6 @@
use crate::internals::ast::{Container, Data, Field, Style, Variant}; use crate::internals::ast::{Container, Data, Field, Style};
use crate::internals::attr::{Default, Identifier, TagType}; use crate::internals::attr::{Default, Identifier, TagType};
use crate::internals::{ungroup, Ctxt, Derive}; use crate::internals::{ungroup, Ctxt, Derive};
use std::collections::btree_map::Entry;
use std::collections::{BTreeMap, BTreeSet};
use syn::{Member, Type}; use syn::{Member, Type};
// Cross-cutting checks that require looking at more than a single attrs object. // Cross-cutting checks that require looking at more than a single attrs object.
@ -18,7 +16,6 @@ pub fn check(cx: &Ctxt, cont: &mut Container, derive: Derive) {
check_adjacent_tag_conflict(cx, cont); check_adjacent_tag_conflict(cx, cont);
check_transparent(cx, cont, derive); check_transparent(cx, cont, derive);
check_from_and_try_from(cx, cont); 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 // If some field of a tuple struct is marked #[serde(default)] then all fields
@ -478,134 +475,3 @@ 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()
),
);
}
}
}
}
}

4
test_suite/tests/ui/conflict/alias-enum.rs
View File

@ -76,4 +76,6 @@ enum E3 {
b, b,
} }
fn main() {} fn main() {
@//fail
}

120
test_suite/tests/ui/conflict/alias-enum.stderr
View File

@ -1,71 +1,79 @@
error: alias `b` conflicts with deserialization name of other field error: expected expression, found `@`
--> tests/ui/conflict/alias-enum.rs:8:9 --> tests/ui/conflict/alias-enum.rs:80:5
| |
8 | / /// Expected error on "alias b", because this is a name of other field 80 | @//fail
9 | | /// Error on "alias a" is not expected because this is a name of this field | ^ expected expression
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 warning: unreachable pattern
--> tests/ui/conflict/alias-enum.rs:14:9 --> tests/ui/conflict/alias-enum.rs:16:9
| |
14 | / /// Expected error on "alias c", because it is already used as alias of `a` 11 | #[serde(alias = "a", alias = "b", alias = "c")]
15 | | #[serde(alias = "c")] | --- matches all the relevant values
16 | | b: (), ...
| |_____________^ 16 | b: (),
| ^ no value can reach this
|
= note: `#[warn(unreachable_patterns)]` on by default
error: alias `c` conflicts with deserialization name of other field warning: unreachable pattern
--> tests/ui/conflict/alias-enum.rs:23:9 --> tests/ui/conflict/alias-enum.rs:15:25
| |
23 | / /// Expected error on "alias c", because this is a name of other field after 11 | #[serde(alias = "a", alias = "b", alias = "c")]
24 | | /// applying rename rules | --- matches all the relevant values
25 | | #[serde(alias = "b", alias = "c")] ...
26 | | a: (), 15 | #[serde(alias = "c")]
| |_____________^ | ^^^ no value can reach this
error: alias `B` conflicts with deserialization name of other field warning: unreachable pattern
--> tests/ui/conflict/alias-enum.rs:34:9 --> tests/ui/conflict/alias-enum.rs:28:26
| |
34 | / /// Expected error on "alias B", because this is a name of field after 25 | #[serde(alias = "b", alias = "c")]
35 | | /// applying rename rules | --- matches all the relevant values
36 | | #[serde(alias = "B", alias = "c")] ...
37 | | a: (), 28 | #[serde(rename = "c")]
| |_____________^ | ^^^ no value can reach this
error: alias `b` conflicts with deserialization name of other variant warning: unreachable pattern
--> tests/ui/conflict/alias-enum.rs:44:5 --> tests/ui/conflict/alias-enum.rs:38:9
| |
44 | / /// Expected error on "alias b", because this is a name of other variant 36 | #[serde(alias = "B", alias = "c")]
45 | | /// Error on "alias a" is not expected because this is a name of this variant | --- matches all the relevant values
46 | | /// Error on "alias c" is not expected because variant `c` is skipped 37 | a: (),
47 | | #[serde(alias = "a", alias = "b", alias = "c")] 38 | b: (),
48 | | a, | ^ no value can reach this
| |_____^
error: alias `c` already used by variant a warning: unreachable pattern
--> tests/ui/conflict/alias-enum.rs:50:5 --> tests/ui/conflict/alias-enum.rs:52:5
| |
50 | / /// Expected error on "alias c", because it is already used as alias of `a` 47 | #[serde(alias = "a", alias = "b", alias = "c")]
51 | | #[serde(alias = "c")] | --- matches all the relevant values
52 | | b, ...
| |_____^ 52 | b,
| ^ no value can reach this
error: alias `c` conflicts with deserialization name of other variant warning: unreachable pattern
--> tests/ui/conflict/alias-enum.rs:60:5 --> tests/ui/conflict/alias-enum.rs:51:21
| |
60 | / /// Expected error on "alias c", because this is a name of other variant after 47 | #[serde(alias = "a", alias = "b", alias = "c")]
61 | | /// applying rename rules | --- matches all the relevant values
62 | | #[serde(alias = "b", alias = "c")] ...
63 | | a, 51 | #[serde(alias = "c")]
| |_____^ | ^^^ no value can reach this
error: alias `B` conflicts with deserialization name of other variant warning: unreachable pattern
--> tests/ui/conflict/alias-enum.rs:72:5 --> tests/ui/conflict/alias-enum.rs:65:22
| |
72 | / /// Expected error on "alias B", because this is a name of variant after 62 | #[serde(alias = "b", alias = "c")]
73 | | /// applying rename rules | --- matches all the relevant values
74 | | #[serde(alias = "B", alias = "c")] ...
75 | | a, 65 | #[serde(rename = "c")]
| |_____^ | ^^^ no value can reach this
warning: unreachable pattern
--> tests/ui/conflict/alias-enum.rs:76:5
|
74 | #[serde(alias = "B", alias = "c")]
| --- matches all the relevant values
75 | a,
76 | b,
| ^ no value can reach this

4
test_suite/tests/ui/conflict/alias.rs
View File

@ -37,4 +37,6 @@ struct S3 {
b: (), b: (),
} }
fn main() {} fn main() {
@//fail
}

68
test_suite/tests/ui/conflict/alias.stderr
View File

@ -1,35 +1,43 @@
error: alias `b` conflicts with deserialization name of other field error: expected expression, found `@`
--> tests/ui/conflict/alias.rs:5:5 --> tests/ui/conflict/alias.rs:41: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` 41 | @//fail
12 | | #[serde(alias = "c")] | ^ expected expression
13 | | b: (),
| |_________^
error: alias `c` conflicts with deserialization name of other field warning: unreachable pattern
--> tests/ui/conflict/alias.rs:21:5 --> tests/ui/conflict/alias.rs:13:5
| |
21 | / /// Expected error on "alias c", because this is a name of other field after 8 | #[serde(alias = "a", alias = "b", alias = "c")]
22 | | /// applying rename rules | --- matches all the relevant values
23 | | #[serde(alias = "b", alias = "c")] ...
24 | | a: (), 13 | b: (),
| |_________^ | ^ no value can reach this
|
= note: `#[warn(unreachable_patterns)]` on by default
error: alias `B` conflicts with deserialization name of other field warning: unreachable pattern
--> tests/ui/conflict/alias.rs:33:5 --> tests/ui/conflict/alias.rs:12:21
| |
33 | / /// Expected error on "alias B", because this is a name of field after 8 | #[serde(alias = "a", alias = "b", alias = "c")]
34 | | /// applying rename rules | --- matches all the relevant values
35 | | #[serde(alias = "B", alias = "c")] ...
36 | | a: (), 12 | #[serde(alias = "c")]
| |_________^ | ^^^ no value can reach this
warning: unreachable pattern
--> tests/ui/conflict/alias.rs:26:22
|
23 | #[serde(alias = "b", alias = "c")]
| --- matches all the relevant values
...
26 | #[serde(rename = "c")]
| ^^^ no value can reach this
warning: unreachable pattern
--> tests/ui/conflict/alias.rs:37:5
|
35 | #[serde(alias = "B", alias = "c")]
| --- matches all the relevant values
36 | a: (),
37 | b: (),
| ^ no value can reach this