From bdcc7adc94f2005be83ac21fda8c5eb4f5890400 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Fri, 29 Nov 2024 13:03:45 +0100 Subject: [PATCH] internal/impl: skip synthetic oneofs in messageInfo Calling WhichOneof should not be possible for synthetic oneofs; on the reflection level, these fields should work as if they were regular fields, not as if they were oneofs: > Reflection for proto3 optional fields should work properly. For example, a > method like Reflection::HasField() should know to look for the hasbit for a > proto3 optional field. It should not be fooled by the synthetic oneof into > thinking that there is a case member for the oneof. From the Protobuf docs at: https://github.com/protocolbuffers/protobuf/blob/main/docs/implementing_proto3_presence.md#updating-reflection This change was tested Google-internally as CL 701866153. Change-Id: Id9500d4aa492acf4ebc6d2d84be07ed81201d2aa Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/632735 Reviewed-by: Chressie Himpel LUCI-TryBot-Result: Go LUCI --- internal/impl/message_reflect.go | 4 +++- testing/prototest/message.go | 9 +++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/internal/impl/message_reflect.go b/internal/impl/message_reflect.go index ecb4623d..98ab94ae 100644 --- a/internal/impl/message_reflect.go +++ b/internal/impl/message_reflect.go @@ -85,7 +85,9 @@ func (mi *MessageInfo) makeKnownFieldsFunc(si structInfo) { mi.oneofs = map[protoreflect.Name]*oneofInfo{} for i := 0; i < md.Oneofs().Len(); i++ { od := md.Oneofs().Get(i) - mi.oneofs[od.Name()] = makeOneofInfo(od, si, mi.Exporter) + if !od.IsSynthetic() { + mi.oneofs[od.Name()] = makeOneofInfo(od, si, mi.Exporter) + } } mi.denseFields = make([]*fieldInfo, fds.Len()*2) diff --git a/testing/prototest/message.go b/testing/prototest/message.go index ebe1b871..eaf53cfe 100644 --- a/testing/prototest/message.go +++ b/testing/prototest/message.go @@ -579,8 +579,13 @@ func testOneof(t testing.TB, m protoreflect.Message, od protoreflect.OneofDescri // Set fields explicitly. m.Set(fda, newValue(m, fda, 1, nil)) } - if got, want := m.WhichOneof(od), fda; got != want { - t.Errorf("after setting oneof field %q:\nWhichOneof(%q) = %v, want %v", fda.FullName(), fda.Name(), got, want) + if !od.IsSynthetic() { + // Synthetic oneofs are used to represent optional fields in + // proto3. While they show up in protoreflect, WhichOneof does + // not work on these (only on non-synthetic, explicit oneofs). + if got, want := m.WhichOneof(od), fda; got != want { + t.Errorf("after setting oneof field %q:\nWhichOneof(%q) = %v, want %v", fda.FullName(), fda.Name(), got, want) + } } for j := 0; j < od.Fields().Len(); j++ { fdb := od.Fields().Get(j)