Skip to content

Commit 1fae49e

Browse files
authored
fix(prost-build): Avoid OneOf type collision with enums and keyword names (#1341)
* 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 * PR feedback * Fix clippy
1 parent 447d351 commit 1fae49e

File tree

3 files changed

+72
-4
lines changed

3 files changed

+72
-4
lines changed

prost-build/src/code_generator.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,11 @@ impl OneofField {
7676
fields: Vec<Field>,
7777
path_index: i32,
7878
) -> Self {
79-
let has_type_name_conflict = parent
80-
.nested_type
81-
.iter()
82-
.any(|nested| to_snake(nested.name()) == descriptor.name());
79+
let nested_type_names = parent.nested_type.iter().map(DescriptorProto::name);
80+
let nested_enum_names = parent.enum_type.iter().map(EnumDescriptorProto::name);
81+
let has_type_name_conflict = nested_type_names
82+
.chain(nested_enum_names)
83+
.any(|type_name| to_upper_camel(type_name) == to_upper_camel(descriptor.name()));
8384

8485
Self {
8586
descriptor,

tests/src/oneof_name_conflict.proto

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,50 @@ message Bakery {
1212
Bread b = 2;
1313
}
1414
}
15+
16+
message EnumAndOneofConflict {
17+
enum Type {
18+
TYPE_1 = 0;
19+
TYPE_2 = 1;
20+
}
21+
22+
message TypeOne {
23+
string field = 1;
24+
}
25+
message TypeTwo {
26+
int32 field = 1;
27+
}
28+
message TypeThree {
29+
Type field = 1;
30+
}
31+
32+
oneof type {
33+
TypeOne one = 1;
34+
TypeTwo two = 2;
35+
TypeThree three = 3;
36+
}
37+
}
38+
39+
message NestedTypeWithReservedKeyword {
40+
// abstract is a reserved keyword of the languange
41+
// so it will be escaped as r#abstract.
42+
message Abstract {
43+
string field = 1;
44+
}
45+
message TypeOne {
46+
string field = 1;
47+
}
48+
message TypeTwo {
49+
int32 field = 1;
50+
}
51+
message TypeThree {
52+
Abstract field = 1;
53+
}
54+
55+
oneof abstract {
56+
Abstract abstract_ = 1;
57+
TypeOne type_one = 2;
58+
TypeTwo type_two = 3;
59+
TypeThree type_three = 4;
60+
}
61+
}

tests/src/oneof_name_conflict.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,24 @@ fn test_creation() {
1010
oneof_name_conflict::bakery::Bread { weight: 12 },
1111
)),
1212
};
13+
14+
let _ = oneof_name_conflict::EnumAndOneofConflict {
15+
r#type: Some(
16+
oneof_name_conflict::enum_and_oneof_conflict::TypeOneOf::Three(
17+
oneof_name_conflict::enum_and_oneof_conflict::TypeThree {
18+
field: oneof_name_conflict::enum_and_oneof_conflict::Type::Type1.into(),
19+
},
20+
),
21+
),
22+
};
23+
24+
let _ = oneof_name_conflict::NestedTypeWithReservedKeyword {
25+
r#abstract: Some(
26+
oneof_name_conflict::nested_type_with_reserved_keyword::AbstractOneOf::Abstract(
27+
oneof_name_conflict::nested_type_with_reserved_keyword::Abstract {
28+
field: "field".into(),
29+
},
30+
),
31+
),
32+
};
1333
}

0 commit comments

Comments
 (0)