Skip to content

Commit 24b47b7

Browse files
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 24b47b7

File tree

4 files changed

+157
-1
lines changed

4 files changed

+157
-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,
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
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(
63+
oneof = "nested_type_and_one_of_conflict::AbstractOneOf",
64+
tags = "1, 2, 3, 4"
65+
)]
66+
pub r#abstract: ::core::option::Option<nested_type_and_one_of_conflict::AbstractOneOf>,
67+
}
68+
/// Nested message and enum types in `NestedTypeAndOneOfConflict`.
69+
pub mod nested_type_and_one_of_conflict {
70+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
71+
pub struct Abstract {
72+
#[prost(string, tag = "1")]
73+
pub field: ::prost::alloc::string::String,
74+
}
75+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
76+
pub struct TypeOne {
77+
#[prost(string, tag = "1")]
78+
pub field: ::prost::alloc::string::String,
79+
}
80+
#[derive(Clone, Copy, PartialEq, Eq, Hash, ::prost::Message)]
81+
pub struct TypeTwo {
82+
#[prost(int32, tag = "1")]
83+
pub field: i32,
84+
}
85+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Message)]
86+
pub struct TypeThree {
87+
#[prost(message, optional, tag = "1")]
88+
pub field: ::core::option::Option<Abstract>,
89+
}
90+
#[derive(Clone, PartialEq, Eq, Hash, ::prost::Oneof)]
91+
pub enum AbstractOneOf {
92+
#[prost(message, tag = "1")]
93+
Abstract(Abstract),
94+
#[prost(message, tag = "2")]
95+
TypeOne(TypeOne),
96+
#[prost(message, tag = "3")]
97+
TypeTwo(TypeTwo),
98+
#[prost(message, tag = "4")]
99+
TypeThree(TypeThree),
100+
}
101+
}
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: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,4 +597,23 @@ 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+
"src/fixtures/oneof_conflict/_oneof_conflict.rs",
616+
tempdir.path().join("oneof_conflict.rs")
617+
);
618+
}
600619
}

0 commit comments

Comments
 (0)