Skip to content

Commit 839517e

Browse files
jknovijknovi
committed
Avoids OneOf type collision with enums and some nested types
When detecting if a oneof type will clash with other types we only compare against nested types, and for those only the snake_case version is validated against the oneof name. This allow room for conflicts when: - The collision would happen with an enum type. - The oneof name is a reserved word that when snaked gets sanitized as r#<word>. In this change the check for collisions is updated to consider enum types and compered against their capitalized camel case version which
1 parent 812358c commit 839517e

File tree

5 files changed

+271
-1
lines changed

5 files changed

+271
-1
lines changed

‎prost-build/src/code_generator.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,9 @@ impl OneofField {
7979
let has_type_name_conflict = parent
8080
.nested_type
8181
.iter()
82-
.any(|nested| to_snake(nested.name()) == descriptor.name());
82+
.map(DescriptorProto::name)
83+
.chain(parent.enum_type.iter().map(EnumDescriptorProto::name))
84+
.any(|type_name| to_upper_camel(type_name) == to_upper_camel(descriptor.name()));
8385

8486
Self {
8587
descriptor,

‎prost-build/src/fixtures/oneof_conflict/_oneof_conflict.rs‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// This file is @generated by prost-build.
2+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
3+
pub struct EnumAndOneOfConflict {
4+
#[prost(oneof = "enum_and_one_of_conflict::TypeOneOf", tags = "1, 2, 3")]
5+
pub r#type: ::core::option::Option<enum_and_one_of_conflict::TypeOneOf>,
6+
}
7+
/// Nested message and enum types in `EnumAndOneOfConflict`.
8+
pub mod enum_and_one_of_conflict {
9+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
10+
pub struct TypeOne {
11+
#[prost(string, tag = "1")]
12+
pub field: ::prost::alloc::string::String,
13+
}
14+
#[derive(Clone, Copy, PartialEq, Eq, Hash, ::prost::Message)]
15+
pub struct TypeTwo {
16+
#[prost(int32, tag = "1")]
17+
pub field: i32,
18+
}
19+
#[derive(Clone, Copy, PartialEq, Eq, Hash, ::prost::Message)]
20+
pub struct TypeThree {
21+
#[prost(enumeration = "Type", tag = "1")]
22+
pub field: i32,
23+
}
24+
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, ::prost::Enumeration)]
25+
#[repr(i32)]
26+
pub enum Type {
27+
Type1 = 0,
28+
Type2 = 1,
29+
}
30+
impl Type {
31+
/// String value of the enum field names used in the ProtoBuf definition.
32+
///
33+
/// The values are not transformed in any way and thus are considered stable
34+
/// (if the ProtoBuf definition does not change) and safe for programmatic use.
35+
pub fn as_str_name(&self) -> &'static str {
36+
match self {
37+
Self::Type1 => "TYPE_1",
38+
Self::Type2 => "TYPE_2",
39+
}
40+
}
41+
/// Creates an enum from field names used in the ProtoBuf definition.
42+
pub fn from_str_name(value: &str) -> ::core::option::Option<Self> {
43+
match value {
44+
"TYPE_1" => Some(Self::Type1),
45+
"TYPE_2" => Some(Self::Type2),
46+
_ => None,
47+
}
48+
}
49+
}
50+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Oneof)]
51+
pub enum TypeOneOf {
52+
#[prost(message, tag = "1")]
53+
TypeOne(TypeOne),
54+
#[prost(message, tag = "2")]
55+
TypeTwo(TypeTwo),
56+
#[prost(message, tag = "3")]
57+
TypeThree(TypeThree),
58+
}
59+
}
60+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
61+
pub struct NestedTypeAndOneOfConflict {
62+
#[prost(oneof = "nested_type_and_one_of_conflict::AbstractOneOf", tags = "1, 2, 3, 4")]
63+
pub r#abstract: ::core::option::Option<nested_type_and_one_of_conflict::AbstractOneOf>,
64+
}
65+
/// Nested message and enum types in `NestedTypeAndOneOfConflict`.
66+
pub mod nested_type_and_one_of_conflict {
67+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
68+
pub struct Abstract {
69+
#[prost(string, tag = "1")]
70+
pub field: ::prost::alloc::string::String,
71+
}
72+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
73+
pub struct TypeOne {
74+
#[prost(string, tag = "1")]
75+
pub field: ::prost::alloc::string::String,
76+
}
77+
#[derive(Clone, Copy, PartialEq, Eq, Hash, ::prost::Message)]
78+
pub struct TypeTwo {
79+
#[prost(int32, tag = "1")]
80+
pub field: i32,
81+
}
82+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
83+
pub struct TypeThree {
84+
#[prost(message, optional, tag = "1")]
85+
pub field: ::core::option::Option<Abstract>,
86+
}
87+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Oneof)]
88+
pub enum AbstractOneOf {
89+
#[prost(message, tag = "1")]
90+
Abstract(Abstract),
91+
#[prost(message, tag = "2")]
92+
TypeOne(TypeOne),
93+
#[prost(message, tag = "3")]
94+
TypeTwo(TypeTwo),
95+
#[prost(message, tag = "4")]
96+
TypeThree(TypeThree),
97+
}
98+
}

‎prost-build/src/fixtures/oneof_conflict/_oneof_conflict_formatted.rs‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
// This file is @generated by prost-build.
2+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
3+
pub struct EnumAndOneOfConflict {
4+
#[prost(oneof = "enum_and_one_of_conflict::TypeOneOf", tags = "1, 2, 3")]
5+
pub r#type: ::core::option::Option<enum_and_one_of_conflict::TypeOneOf>,
6+
}
7+
/// Nested message and enum types in `EnumAndOneOfConflict`.
8+
pub mod enum_and_one_of_conflict {
9+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
10+
pub struct TypeOne {
11+
#[prost(string, tag = "1")]
12+
pub field: ::prost::alloc::string::String,
13+
}
14+
#[derive(Clone, Copy, PartialEq, Eq, Hash, ::prost::Message)]
15+
pub struct TypeTwo {
16+
#[prost(int32, tag = "1")]
17+
pub field: i32,
18+
}
19+
#[derive(Clone, Copy, PartialEq, Eq, Hash, ::prost::Message)]
20+
pub struct TypeThree {
21+
#[prost(enumeration = "Type", tag = "1")]
22+
pub field: i32,
23+
}
24+
#[derive(
25+
Clone,
26+
Copy,
27+
Debug,
28+
PartialEq,
29+
Eq,
30+
Hash,
31+
PartialOrd,
32+
Ord,
33+
::prost::Enumeration
34+
)]
35+
#[repr(i32)]
36+
pub enum Type {
37+
Type1 = 0,
38+
Type2 = 1,
39+
}
40+
impl Type {
41+
/// String value of the enum field names used in the ProtoBuf definition.
42+
///
43+
/// The values are not transformed in any way and thus are considered stable
44+
/// (if the ProtoBuf definition does not change) and safe for programmatic use.
45+
pub fn as_str_name(&self) -> &'static str {
46+
match self {
47+
Self::Type1 => "TYPE_1",
48+
Self::Type2 => "TYPE_2",
49+
}
50+
}
51+
/// Creates an enum from field names used in the ProtoBuf definition.
52+
pub fn from_str_name(value: &str) -> ::core::option::Option<Self> {
53+
match value {
54+
"TYPE_1" => Some(Self::Type1),
55+
"TYPE_2" => Some(Self::Type2),
56+
_ => None,
57+
}
58+
}
59+
}
60+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Oneof)]
61+
pub enum TypeOneOf {
62+
#[prost(message, tag = "1")]
63+
TypeOne(TypeOne),
64+
#[prost(message, tag = "2")]
65+
TypeTwo(TypeTwo),
66+
#[prost(message, tag = "3")]
67+
TypeThree(TypeThree),
68+
}
69+
}
70+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
71+
pub struct NestedTypeAndOneOfConflict {
72+
#[prost(
73+
oneof = "nested_type_and_one_of_conflict::AbstractOneOf",
74+
tags = "1, 2, 3, 4"
75+
)]
76+
pub r#abstract: ::core::option::Option<
77+
nested_type_and_one_of_conflict::AbstractOneOf,
78+
>,
79+
}
80+
/// Nested message and enum types in `NestedTypeAndOneOfConflict`.
81+
pub mod nested_type_and_one_of_conflict {
82+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
83+
pub struct Abstract {
84+
#[prost(string, tag = "1")]
85+
pub field: ::prost::alloc::string::String,
86+
}
87+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
88+
pub struct TypeOne {
89+
#[prost(string, tag = "1")]
90+
pub field: ::prost::alloc::string::String,
91+
}
92+
#[derive(Clone, Copy, PartialEq, Eq, Hash, ::prost::Message)]
93+
pub struct TypeTwo {
94+
#[prost(int32, tag = "1")]
95+
pub field: i32,
96+
}
97+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
98+
pub struct TypeThree {
99+
#[prost(message, optional, tag = "1")]
100+
pub field: ::core::option::Option<Abstract>,
101+
}
102+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Oneof)]
103+
pub enum AbstractOneOf {
104+
#[prost(message, tag = "1")]
105+
Abstract(Abstract),
106+
#[prost(message, tag = "2")]
107+
TypeOne(TypeOne),
108+
#[prost(message, tag = "3")]
109+
TypeTwo(TypeTwo),
110+
#[prost(message, tag = "4")]
111+
TypeThree(TypeThree),
112+
}
113+
}

‎prost-build/src/fixtures/oneof_conflict/oneof_conflict.proto‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
syntax = "proto3";
2+
3+
package oneof_conflict;
4+
5+
message EnumAndOneOfConflict {
6+
enum Type {
7+
TYPE_1 = 0;
8+
TYPE_2 = 1;
9+
}
10+
11+
message TypeOne { string field = 1; }
12+
message TypeTwo { int32 field = 1; }
13+
message TypeThree { Type field = 1; }
14+
15+
oneof type {
16+
TypeOne type_one = 1;
17+
TypeTwo type_two = 2;
18+
TypeThree type_three = 3;
19+
}
20+
}
21+
22+
message NestedTypeAndOneOfConflict {
23+
message Abstract { string field = 1; }
24+
message TypeOne { string field = 1; }
25+
message TypeTwo { int32 field = 1; }
26+
message TypeThree { Abstract field = 1; }
27+
28+
oneof abstract {
29+
Abstract abstract_ = 1;
30+
TypeOne type_one = 2;
31+
TypeTwo type_two = 3;
32+
TypeThree type_three = 4;
33+
}
34+
}

‎prost-build/src/lib.rs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,4 +597,27 @@ mod tests {
597597
tempdir.path().join("all_deprecated.rs")
598598
);
599599
}
600+
601+
#[test]
602+
fn test_oneof_conflict() {
603+
let _ = env_logger::try_init();
604+
let tempdir = tempfile::tempdir().unwrap();
605+
606+
Config::new()
607+
.out_dir(tempdir.path())
608+
.compile_protos(
609+
&["src/fixtures/oneof_conflict/oneof_conflict.proto"],
610+
&["src/fixtures/oneof_conflict"],
611+
)
612+
.unwrap();
613+
614+
assert_eq_fixture_file!(
615+
if cfg!(feature = "format") {
616+
"src/fixtures/oneof_conflict/_oneof_conflict_formatted.rs"
617+
} else {
618+
"src/fixtures/oneof_conflict/_oneof_conflict.rs"
619+
},
620+
tempdir.path().join("oneof_conflict.rs")
621+
);
622+
}
600623
}

0 commit comments

Comments
 (0)