No public description
PiperOrigin-RevId: 799008780
Change-Id: I275c7c9605c0b42d586a4f2a8516849a536ea90a
diff --git a/internal/fix/build.go b/internal/fix/build.go
index 0363547..338b8bf 100644
--- a/internal/fix/build.go
+++ b/internal/fix/build.go
@@ -197,29 +197,8 @@
continue
}
- // Don't rewrite the oneof in
- //
- // &pb.M2{OneofField: &pb.M2_MsgOneof{}}
- // &pb.M2{OneofField: &pb.M2_EnumOneof{}}
- //
- // It's a tricky case and should be very rare because "oneof
- // with a type but without a value" is not a valid protocol buffers
- // concept.
- //
- // It happens due to a mismatch between what's allowed in protocol buffers
- // and the Go structs representing protocol buffers in the open API.
- // This information will not be marshalled to the wire and the field
- // is effectively discarded during marshalling when it does not have
- // a value.
- //
- // The opaque API does not allow this. While the struct can technically
- // represent this, you cannot use the API to bring the struct into
- // this state.
- //
- // For enums: this should be rare and we don't want to guess the default
- // value.
- if fieldValue == nil && !isBasic(fieldType) {
- c.Logf("returning: RHS is nil of Type %T (looking for types.Basic)", fieldType)
+ if fieldValue == nil && !isBasic(fieldType) && !isPtr(fieldType) && !isEnum(fieldType) {
+ c.Logf("returning: RHS is nil of Type %T (looking for types.Basic or types.Pointer)", fieldType)
return false
}
@@ -235,11 +214,27 @@
unsafeRewrite = true
}
- // Handle `M{Oneof: OneofBasicField{}}`
+ // Handle `M{Oneof: OneofBasicField{}}`, `M{Oneof: OneofMsgField{}}` or
+ // `M{Oneof: OneofEnumField{}}`
if fieldValue == nil {
updates = append(updates, func() {
kv.Key.(*dst.Ident).Name = fieldName // Rename the key to field name from the oneof wrapper.
- kv.Value = c.newProtoHelperCall(nil, types.Unalias(fieldType).(*types.Basic))
+ if isBasic(fieldType) {
+ kv.Value = c.newProtoHelperCall(nil, types.Unalias(fieldType).(*types.Basic))
+ } else if isPtr(fieldType) {
+ lit := &dst.CompositeLit{
+ Type: c.selectorForProtoMessageType(fieldType),
+ }
+ c.setType(lit, fieldType)
+ kv.Value = &dst.UnaryExpr{
+ Op: token.AND,
+ X: lit,
+ }
+ c.setType(kv.Value, fieldType)
+ } else { /* must be isEnum(fieldType) */
+ kv.Value = &dst.Ident{Name: "0"}
+ c.setType(kv.Value, types.Typ[types.Invalid])
+ }
})
c.Logf("generated RHS for field %v", fieldName)
continue
diff --git a/internal/fix/build_test.go b/internal/fix/build_test.go
index 6844641..47dd165 100644
--- a/internal/fix/build_test.go
+++ b/internal/fix/build_test.go
@@ -451,6 +451,7 @@
_ = pb2.M2_builder{
// comment before
BytesOneof:/*comment1*/ m2.GetBytesOneof(), // eol comment
+ EmptyOneof: m2.GetEmptyOneof(),
EnumOneof: proto.ValueOrNil(m2.HasEnumOneof(), m2.GetEnumOneof),
IntOneof: proto.ValueOrNil(m2.HasIntOneof(), m2.GetIntOneof),
MsgOneof: m2.GetMsgOneof(),
@@ -461,6 +462,7 @@
_ = pb2.M2_builder{
// comment before
BytesOneof:/*comment1*/ m2.GetBytesOneof(), // eol comment
+ EmptyOneof: m2.GetEmptyOneof(),
EnumOneof: proto.ValueOrNil(m2.HasEnumOneof(), m2.GetEnumOneof),
IntOneof: proto.ValueOrNil(m2.HasIntOneof(), m2.GetIntOneof),
MsgOneof: m2.GetMsgOneof(),
@@ -522,6 +524,7 @@
_ = pb2.M2_builder{
S: proto.String("42"),
BytesOneof: m2.GetBytesOneof(),
+ EmptyOneof: m2.GetEmptyOneof(),
EnumOneof: proto.ValueOrNil(m2.HasEnumOneof(), m2.GetEnumOneof),
IntOneof: proto.ValueOrNil(m2.HasIntOneof(), m2.GetIntOneof),
MsgOneof: m2.GetMsgOneof(),
@@ -543,6 +546,7 @@
_ = pb2.M2_builder{
S: proto.String("42"),
BytesOneof: m2.GetBytesOneof(),
+ EmptyOneof: m2.GetEmptyOneof(),
EnumOneof: proto.ValueOrNil(m2.HasEnumOneof(), m2.GetEnumOneof),
IntOneof: proto.ValueOrNil(m2.HasIntOneof(), m2.GetIntOneof),
MsgOneof: m2.GetMsgOneof(),
@@ -570,26 +574,29 @@
desc: "oneofs: msg zero value",
in: `
_ = &pb2.M2{OneofField: &pb2.M2_MsgOneof{}}
+_ = &pb2.M2{OneofField: &pb2.M2_EmptyOneof{}}
`,
want: map[Level]string{
Green: `
_ = &pb2.M2{OneofField: &pb2.M2_MsgOneof{}}
+_ = &pb2.M2{OneofField: &pb2.M2_EmptyOneof{}}
`,
Red: `
-_ = pb2.M2_builder{OneofField: &pb2.M2_MsgOneof{}}.Build()
+_ = pb2.M2_builder{MsgOneof: &pb2.M2{}}.Build()
+_ = pb2.M2_builder{EmptyOneof: &xpb.Empty{}}.Build()
`,
},
}, {
- desc: "oneofs: enum zero value", // no rewrite (see comments in rules.go)
+ desc: "oneofs: enum zero value",
in: `
_ = &pb2.M2{OneofField: &pb2.M2_EnumOneof{}}
`,
want: map[Level]string{
Green: `
-_ = &pb2.M2{OneofField: &pb2.M2_EnumOneof{}}
+_ = pb2.M2_builder{EnumOneof: 0}.Build()
`,
Red: `
-_ = pb2.M2_builder{OneofField: &pb2.M2_EnumOneof{}}.Build()
+_ = pb2.M2_builder{EnumOneof: 0}.Build()
`,
},
}, {
@@ -617,10 +624,10 @@
want: map[Level]string{
Green: `
msg := &pb2.M2{}
-_ = &pb2.M2{
- OneofField: &pb2.M2_StringOneof{"hello"},
- OneofField2: &pb2.M2_EnumOneof2{},
-}
+_ = pb2.M2_builder{
+ StringOneof: proto.String("hello"),
+ EnumOneof2: 0,
+}.Build()
_ = &pb2.M2{
OneofField: &pb2.M2_StringOneof{"hello"},
OneofField2: &pb2.M2_MsgOneof2{msg},
@@ -636,18 +643,18 @@
`,
Yellow: `
msg := &pb2.M2{}
-_ = &pb2.M2{
- OneofField: &pb2.M2_StringOneof{"hello"},
- OneofField2: &pb2.M2_EnumOneof2{},
-}
+_ = pb2.M2_builder{
+ StringOneof: proto.String("hello"),
+ EnumOneof2: 0,
+}.Build()
_ = pb2.M2_builder{
StringOneof: proto.String("hello"),
MsgOneof2: proto.ValueOrDefault(msg),
}.Build()
-_ = &pb2.M2{
- OneofField: &pb2.M2_StringOneof{"hello"},
- OneofField2: &pb2.M2_MsgOneof2{},
-}
+_ = pb2.M2_builder{
+ StringOneof: proto.String("hello"),
+ MsgOneof2: &pb2.M2{},
+}.Build()
_ = &pb2.M2{
OneofField: &pb2.M2_StringOneof{"hello"},
OneofField2: F(),
diff --git a/internal/fix/fix.go b/internal/fix/fix.go
index 17256b5..1855a89 100644
--- a/internal/fix/fix.go
+++ b/internal/fix/fix.go
@@ -411,6 +411,9 @@
for _, imp := range c.imports.importsToAdd {
cur.InsertAfter(imp)
c.setType(imp.Path, types.Typ[types.Invalid])
+ if imp.Name != nil {
+ c.setType(imp.Name, types.Typ[types.Invalid])
+ }
}
return false // import added, abort traversal
})
diff --git a/internal/fix/testdata/proto2test_go_proto/proto2test.proto b/internal/fix/testdata/proto2test_go_proto/proto2test.proto
index e4c93d3..08a280d 100644
--- a/internal/fix/testdata/proto2test_go_proto/proto2test.proto
+++ b/internal/fix/testdata/proto2test_go_proto/proto2test.proto
@@ -7,6 +7,7 @@
package net.proto2.go.open2opaque.o2o.test;
import "google/protobuf/go_features.proto";
+import "google/protobuf/empty.proto";
message DoNotMigrateMe {
option features.(pb.go).api_level = API_OPEN;
@@ -40,6 +41,7 @@
M2 msg_oneof = 16;
Enum enum_oneof = 17;
bytes bytes_oneof = 18;
+ google.protobuf.Empty empty_oneof = 99;
}
oneof oneof_field2 {
string string_oneof2 = 19;
