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;