Introduce weirder names for the oneof generated structs to minimize collisions.

Given:
oneof hello_world {}

We now generate:
fn hello_world(&self) -> HelloWorldOneof
fn hello_world_case(&self) -> HelloWorldCase

This change is to help mitigate a realistic risk of collisions in cases like:

message M {
  oneof x { ... }
  enum X { ... }
}

PiperOrigin-RevId: 714998247
pull/19943/head
Protobuf Team Bot 1 month ago committed by Copybara-Service
parent dca34c7c05
commit 5106654585
  1. 10
      rust/test/shared/accessors_proto3_test.rs
  2. 4
      rust/test/shared/accessors_test.rs
  3. 14
      rust/test/shared/fields_with_imported_types_test.rs
  4. 12
      src/google/protobuf/compiler/rust/naming.cc
  5. 6
      src/google/protobuf/compiler/rust/naming.h
  6. 28
      src/google/protobuf/compiler/rust/oneof.cc

@ -173,7 +173,7 @@ fn test_foreign_enum_accessors() {
#[gtest]
fn test_oneof_accessors() {
use test_all_types::OneofField::*;
use test_all_types::OneofFieldOneof::*;
let mut msg = TestAllTypes::new();
assert_that!(msg.oneof_field(), matches_pattern!(not_set(_)));
@ -202,7 +202,7 @@ fn test_oneof_accessors() {
#[gtest]
fn test_oneof_accessors_view_long_lifetime() {
use test_all_types::OneofField::*;
use test_all_types::OneofFieldOneof::*;
let mut msg = TestAllTypes::new();
msg.set_oneof_uint32(7);
@ -219,18 +219,18 @@ fn test_oneof_accessors_view_long_lifetime() {
#[gtest]
fn test_oneof_enum_accessors() {
use unittest_proto3_rust_proto::{
test_oneof2::{Foo, FooCase, NestedEnum},
test_oneof2::{FooCase, FooOneof, NestedEnum},
TestOneof2,
};
let mut msg = TestOneof2::new();
assert_that!(msg.foo_enum_opt(), eq(Optional::Unset(NestedEnum::Unknown)));
assert_that!(msg.foo(), matches_pattern!(Foo::not_set(_)));
assert_that!(msg.foo(), matches_pattern!(FooOneof::not_set(_)));
assert_that!(msg.foo_case(), matches_pattern!(FooCase::not_set));
msg.set_foo_enum(NestedEnum::Bar);
assert_that!(msg.foo_enum_opt(), eq(Optional::Set(NestedEnum::Bar)));
assert_that!(msg.foo(), matches_pattern!(Foo::FooEnum(eq(NestedEnum::Bar))));
assert_that!(msg.foo(), matches_pattern!(FooOneof::FooEnum(eq(NestedEnum::Bar))));
assert_that!(msg.foo_case(), matches_pattern!(FooCase::FooEnum));
}

@ -767,7 +767,7 @@ fn test_default_import_enum_accessors() {
#[gtest]
fn test_oneof_accessors() {
use unittest_rust_proto::test_oneof2::{Foo::*, FooCase, NestedEnum};
use unittest_rust_proto::test_oneof2::{FooCase, FooOneof::*, NestedEnum};
use unittest_rust_proto::TestOneof2;
let mut msg = TestOneof2::new();
@ -818,7 +818,7 @@ fn test_oneof_accessors() {
#[gtest]
fn test_msg_oneof_default_accessors() {
use unittest_rust_proto::test_oneof2::{Bar::*, BarCase, NestedEnum};
use unittest_rust_proto::test_oneof2::{BarCase, BarOneof::*, NestedEnum};
let mut msg = unittest_rust_proto::TestOneof2::new();
assert_that!(msg.bar(), matches_pattern!(not_set(_)));

@ -30,13 +30,21 @@ fn test_enum_field_generated() {
#[gtest]
fn test_oneof_message_field_generated() {
use fields_with_imported_types_rust_proto::msg_with_fields_with_imported_types::ImportedTypesOneof::not_set;
use fields_with_imported_types_rust_proto::MsgWithFieldsWithImportedTypes;
use fields_with_imported_types_rust_proto::{
msg_with_fields_with_imported_types, MsgWithFieldsWithImportedTypes,
};
use imported_types_rust_proto::ImportedEnum;
use imported_types_rust_proto::ImportedMessageView;
let msg = MsgWithFieldsWithImportedTypes::new();
assert_that!(msg.imported_message_oneof(), matches_pattern!(ImportedMessageView { .. }));
assert_that!(msg.imported_enum_oneof(), eq(ImportedEnum::Unknown));
assert_that!(msg.imported_types_oneof(), matches_pattern!(not_set(_)));
assert_that!(
msg.imported_types_oneof(),
matches_pattern!(msg_with_fields_with_imported_types::ImportedTypesOneofOneof::not_set(_))
);
assert_that!(
msg.imported_types_oneof_case(),
matches_pattern!(msg_with_fields_with_imported_types::ImportedTypesOneofCase::not_set)
);
}

@ -333,13 +333,19 @@ std::string EnumValueRsName(const MultiCasePrefixStripper& stripper,
return RsSafeName(name);
}
std::string OneofViewEnumRsName(const OneofDescriptor& oneof) {
std::string DeprecatedOneofViewEnumRsName(const OneofDescriptor& oneof) {
return RsSafeName(SnakeToUpperCamelCase(oneof.name()));
}
std::string OneofViewEnumRsName(const OneofDescriptor& oneof) {
return SnakeToUpperCamelCase(oneof.name()) + "Oneof";
}
std::string OneofCaseEnumRsName(const OneofDescriptor& oneof) {
// Note: This is the name used for the cpp Case enum, we use it for both
// the Rust Case enum as well as for the cpp case enum in the cpp thunk.
return SnakeToUpperCamelCase(oneof.name()) + "Case";
}
std::string OneofCaseEnumCppName(const OneofDescriptor& oneof) {
return SnakeToUpperCamelCase(oneof.name()) + "Case";
}

@ -63,8 +63,14 @@ std::string RsViewType(Context& ctx, const FieldDescriptor& field,
std::string EnumRsName(const EnumDescriptor& desc);
std::string EnumValueRsName(const EnumValueDescriptor& value);
// The old names for these types, which we intend to imminently burn down
// to avoid collisions.
std::string DeprecatedOneofViewEnumRsName(const OneofDescriptor& oneof);
std::string OneofViewEnumRsName(const OneofDescriptor& oneof);
std::string OneofCaseEnumRsName(const OneofDescriptor& oneof);
std::string OneofCaseEnumCppName(const OneofDescriptor& oneof);
std::string OneofCaseRsName(const FieldDescriptor& oneof_field);
std::string FieldInfoComment(Context& ctx, const FieldDescriptor& field);

@ -37,7 +37,7 @@ namespace rust {
// Example:
// For this oneof:
// message SomeMsg {
// oneof some_oneof {
// oneof some {
// int32 field_a = 7;
// SomeMsg field_b = 9;
// }
@ -52,7 +52,7 @@ namespace rust {
// }
//
// #[repr(C)]
// pub enum SomeOneofCase {
// pub enum SomeCase {
// FieldA = 7,
// FieldB = 9,
// not_set = 0
@ -60,25 +60,15 @@ namespace rust {
// }
// impl SomeMsg {
// pub fn some_oneof(&self) -> SomeOneof {...}
// pub fn some_oneof_case(&self) -> SomeOneofCase {...}
// pub fn some_oneof_case(&self) -> SomeCase {...}
// }
// impl SomeMsgMut {
// pub fn some_oneof(&self) -> SomeOneof {...}
// pub fn some_oneof_case(&self) -> SomeOneofCase {...}
// pub fn some_oneof_case(&self) -> SomeCase {...}
// }
// impl SomeMsgView {
// pub fn some_oneof(self) -> SomeOneof {...}
// pub fn some_oneof_case(self) -> SomeOneofCase {...}
// }
//
// An additional "Case" enum which just reflects the corresponding slot numbers
// is emitted for usage with the FFI (exactly matching the Case struct that both
// cpp and upb generate).
//
// #[repr(C)] pub(super) enum SomeOneofCase {
// FieldA = 7,
// FieldB = 9,
// not_set = 0
// pub fn some_oneof_case(self) -> SomeCase {...}
// }
namespace {
@ -134,6 +124,7 @@ std::string RsTypeNameView(Context& ctx, const FieldDescriptor& field) {
void GenerateOneofDefinition(Context& ctx, const OneofDescriptor& oneof) {
ctx.Emit(
{
{"deprecated_view_enum_name", DeprecatedOneofViewEnumRsName(oneof)},
{"view_enum_name", OneofViewEnumRsName(oneof)},
{"view_fields",
[&] {
@ -164,9 +155,11 @@ void GenerateOneofDefinition(Context& ctx, const OneofDescriptor& oneof) {
pub enum $view_enum_name$<'msg> {
$view_fields$
#[allow(non_camel_case_types)]
not_set(std::marker::PhantomData<&'msg ()>) = 0
}
#[deprecated(note="Use $view_enum_name$ instead (will be imminently moved)")]
pub use $view_enum_name$ as $deprecated_view_enum_name$;
)rs");
// Note: This enum is used as the Thunk return type for getting which case is
@ -205,7 +198,6 @@ void GenerateOneofDefinition(Context& ctx, const OneofDescriptor& oneof) {
pub enum $case_enum_name$ {
$cases$
#[allow(non_camel_case_types)]
not_set = 0
}
@ -313,7 +305,7 @@ void GenerateOneofThunkCc(Context& ctx, const OneofDescriptor& oneof) {
ctx.Emit(
{
{"oneof_name", oneof.name()},
{"case_enum_name", OneofCaseEnumRsName(oneof)},
{"case_enum_name", OneofCaseEnumCppName(oneof)},
{"case_thunk", ThunkName(ctx, oneof, "case")},
{"QualifiedMsg", cpp::QualifiedClassName(oneof.containing_type())},
},

Loading…
Cancel
Save