handle proto3 optional fields (synthetic oneofs)
fixes https://github.com/golang/open2opaque/issues/14
PiperOrigin-RevId: 730841203
Change-Id: Ie204e1ef8852226b4322c7f3b23eeb40d5f09a94
diff --git a/internal/fix/assign.go b/internal/fix/assign.go
index f3dd2f5..ab91d78 100644
--- a/internal/fix/assign.go
+++ b/internal/fix/assign.go
@@ -445,8 +445,8 @@
if _, ok := c.Parent().(*dst.BlockStmt); ok {
c.Logf("rewriting assignment with rhs Expr")
f := c.objectOf(lhsSel.Sel).(*types.Var)
- isProto2 := !isProto3Field(c.typeOf(lhsSel.X), f.Name())
- if slice, ok := types.Unalias(f.Type()).(*types.Slice); ok && isProto2 {
+ explicitPresence := fieldHasExplicitPresence(c.typeOf(lhsSel.X), f.Name())
+ if slice, ok := types.Unalias(f.Type()).(*types.Slice); ok && explicitPresence {
if basic, ok := types.Unalias(slice.Elem()).(*types.Basic); ok && basic.Kind() == types.Uint8 {
if isNeverNilSliceExpr(c, rhs) {
stmt := c.expr2stmt(sel2call(c, "Set", lhsSel, rhs, decs), lhsSel)
diff --git a/internal/fix/assign_test.go b/internal/fix/assign_test.go
index 7f72167..fb4c0fe 100644
--- a/internal/fix/assign_test.go
+++ b/internal/fix/assign_test.go
@@ -1583,3 +1583,39 @@
runTableTest(t, tt)
}
+
+func TestProto3Optional(t *testing.T) {
+ tests := []test{{
+ desc: "clear all optional fields",
+ in: `
+m := &pb3.M3{}
+m.OptB = nil
+m.OptF32 = nil
+m.OptF64 = nil
+m.OptI32 = nil
+m.OptI64 = nil
+m.OptUi32 = nil
+m.OptUi64 = nil
+m.OptS = nil
+m.OptM = nil
+m.OptE = nil
+`,
+ want: map[Level]string{
+ Green: `
+m := &pb3.M3{}
+m.ClearOptB()
+m.ClearOptF32()
+m.ClearOptF64()
+m.ClearOptI32()
+m.ClearOptI64()
+m.ClearOptUi32()
+m.ClearOptUi64()
+m.ClearOptS()
+m.ClearOptM()
+m.ClearOptE()
+`,
+ },
+ }}
+
+ runTableTests(t, tests)
+}
diff --git a/internal/fix/fixcursor.go b/internal/fix/fixcursor.go
index 9abc0e5..e0bbc34 100644
--- a/internal/fix/fixcursor.go
+++ b/internal/fix/fixcursor.go
@@ -677,9 +677,9 @@
return true
}
}
- // Handle non-enum proto2 scalars.
- isProto2 := !isProto3Field(c.typeOf(sel.X), f.Name())
- if isProto2 {
+
+ // Handle non-enum scalars.
+ if fieldHasExplicitPresence(c.typeOf(sel.X), f.Name()) {
switch ft := types.Unalias(f.Type()).(type) {
case *types.Pointer:
if _, ok := types.Unalias(ft.Elem()).(*types.Basic); ok {
@@ -695,18 +695,21 @@
return false
}
-// isProto3Field returns true if given message is a proto3 message based on given field, else false.
-// It uses the "protobuf" struct tag on the field to determine if it contains "proto3" or not.
+// fieldHasExplicitPresence reports whether the specified field has explicit
+// presence: proto2 and edition 2023+ have explicit presence, proto3 defaults
+// to implicit presence, but offers explicit presence through the optional
+// keyword. See also https://protobuf.dev/programming-guides/field_presence/
//
-// This function considers both value and pointer types to be protocol buffer messages.
-func isProto3Field(m types.Type, field string) bool {
+// This function considers both value and pointer types to be protocol buffer
+// messages.
+func fieldHasExplicitPresence(m types.Type, field string) bool {
p, ok := m.Underlying().(*types.Pointer)
if ok {
m = p.Elem()
}
s, ok := m.Underlying().(*types.Struct)
if !ok {
- return false
+ return true
}
numFields := s.NumFields()
@@ -720,10 +723,14 @@
if i := strings.Index(pb, ",def="); i > -1 {
pb = pb[:i]
}
- return strings.Contains(pb, ",proto3")
+ if !strings.Contains(pb, ",proto3") {
+ return true // not proto3? field has explicit presence
+ }
+ // proto3 optional fields are implemented with synthetic oneofs.
+ return strings.Contains(pb, ",oneof")
}
}
- return false
+ return true
}
// Exactly one of expr or t should be non-nil. expr can be nil only if t is *types.Basic
diff --git a/internal/fix/testdata/proto3test_go_proto/proto3test.proto b/internal/fix/testdata/proto3test_go_proto/proto3test.proto
index 414b4b6..6947ca7 100644
--- a/internal/fix/testdata/proto3test_go_proto/proto3test.proto
+++ b/internal/fix/testdata/proto3test_go_proto/proto3test.proto
@@ -38,4 +38,17 @@
string descriptor = 28;
}
int32 second_i32 = 30;
+
+ optional bool opt_b = 31;
+ optional bytes opt_bytes = 32;
+ optional float opt_f32 = 33;
+ optional double opt_f64 = 34;
+ optional int32 opt_i32 = 35;
+ optional int64 opt_i64 = 36;
+ optional uint32 opt_ui32 = 37;
+ optional uint64 opt_ui64 = 38;
+ optional string opt_s = 39;
+ optional M3 opt_m = 40;
+ // Repeated fields and maps cannot be optional.
+ optional Enum opt_e = 41;
}
