Skip to content

Avoids OneOf type collision with enums and some nested types #1341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Avoids OneOf type collision with enums and some nested types #1341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
839517e
Avoids OneOf type collision with enums and some nested types
jknovi Sep 27, 2025
b9ddd43
PR feedback
jknovi Oct 4, 2025
3eaaadc
Fix clippy
jknovi Oct 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions prost-build/src/code_generator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,11 @@ impl OneofField {
fields: Vec<Field>,
path_index: i32,
) -> Self {
let has_type_name_conflict = parent
.nested_type
.iter()
.any(|nested| to_snake(nested.name()) == descriptor.name());
let nested_type_names = parent.nested_type.iter().map(DescriptorProto::name);
let nested_enum_names = parent.enum_type.iter().map(EnumDescriptorProto::name);
let has_type_name_conflict = nested_type_names
.chain(nested_enum_names)
.any(|type_name| to_upper_camel(type_name) == to_upper_camel(descriptor.name()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my understanding: changing from to_snake to to_upper_camel doesn't do anything, right?

The actual change is that you change the case of both types, right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The to_snake change was required for oneofs with a reserved keyword as name, for example is oneof name is type this check will miss that case since to_snake("Type") -> "r#type" so the check will compare "type" against "r#type" and hence will produce nested type Type and oneof enum Type, causing the compilation error.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comparing to camel case will cover the collision better in these cases.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LMK if you would like me to add a comment around this in the code or if this is fine resolve the conversation as ACK.


Self {
descriptor,
Expand Down
47 changes: 47 additions & 0 deletions tests/src/oneof_name_conflict.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,50 @@ message Bakery {
Bread b = 2;
}
}

message EnumAndOneofConflict {
enum Type {
TYPE_1 = 0;
TYPE_2 = 1;
}

message TypeOne {
string field = 1;
}
message TypeTwo {
int32 field = 1;
}
message TypeThree {
Type field = 1;
}

oneof type {
TypeOne one = 1;
TypeTwo two = 2;
TypeThree three = 3;
}
}

message NestedTypeWithReservedKeyword {
// abstract is a reserved keyword of the languange
// so it will be escaped as r#abstract.
message Abstract {
string field = 1;
}
message TypeOne {
string field = 1;
}
message TypeTwo {
int32 field = 1;
}
message TypeThree {
Abstract field = 1;
}

oneof abstract {
Abstract abstract_ = 1;
TypeOne type_one = 2;
TypeTwo type_two = 3;
TypeThree type_three = 4;
}
}
20 changes: 20 additions & 0 deletions tests/src/oneof_name_conflict.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,24 @@ fn test_creation() {
oneof_name_conflict::bakery::Bread { weight: 12 },
)),
};

let _ = oneof_name_conflict::EnumAndOneofConflict {
r#type: Some(
oneof_name_conflict::enum_and_oneof_conflict::TypeOneOf::Three(
oneof_name_conflict::enum_and_oneof_conflict::TypeThree {
field: oneof_name_conflict::enum_and_oneof_conflict::Type::Type1.into(),
},
),
),
};

let _ = oneof_name_conflict::NestedTypeWithReservedKeyword {
r#abstract: Some(
oneof_name_conflict::nested_type_with_reserved_keyword::AbstractOneOf::Abstract(
oneof_name_conflict::nested_type_with_reserved_keyword::Abstract {
field: "field".into(),
},
),
),
};
}
Loading