reflect/protodesc: fix round-tripping for package field
If a .proto file does not have a package statement,
then protoc will not populate the package field.
Emulate the same behavior for ToFileDescriptorProto.
Fixes golang/protobuf#1195
Change-Id: I617102e50c96f0fa1bac6a6bc4de1cc485750bb9
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/259899
Reviewed-by: Damien Neil <dneil@google.com>
Trust: Joe Tsai <thebrokentoaster@gmail.com>
diff --git a/reflect/protodesc/file_test.go b/reflect/protodesc/file_test.go
index 50a0ed9..29b4fa7 100644
--- a/reflect/protodesc/file_test.go
+++ b/reflect/protodesc/file_test.go
@@ -98,7 +98,7 @@
wantErr: `path must be populated`,
}, {
label: "empty package and syntax",
- inDesc: mustParseFile(`name:"weird" package:""`),
+ inDesc: mustParseFile(`name:"weird"`),
}, {
label: "invalid syntax",
inDesc: mustParseFile(`name:"weird" syntax:"proto9"`),
@@ -111,7 +111,6 @@
label: "unresolvable import",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
dependency: "dep.proto"
`),
wantErr: `could not resolve import "dep.proto": not found`,
@@ -119,7 +118,6 @@
label: "unresolvable import but allowed",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
dependency: "dep.proto"
`),
inOpts: FileOptions{AllowUnresolvable: true},
@@ -127,7 +125,6 @@
label: "duplicate import",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
dependency: ["dep.proto", "dep.proto"]
`),
inOpts: FileOptions{AllowUnresolvable: true},
@@ -136,7 +133,6 @@
label: "invalid weak import",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
dependency: "dep.proto"
weak_dependency: [-23]
`),
@@ -146,7 +142,6 @@
label: "normal weak and public import",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
dependency: "dep.proto"
weak_dependency: [0]
public_dependency: [0]
@@ -160,7 +155,6 @@
},
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
dependency: ["public.proto", "leaf.proto"]
`),
}, {
@@ -237,7 +231,6 @@
label: "resolve the wrong type",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{
name: "M"
field: [{name:"F" number:1 label:LABEL_OPTIONAL type:TYPE_MESSAGE type_name:"E"}]
@@ -249,7 +242,6 @@
label: "auto-resolve unknown kind",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{
name: "M"
field: [{name:"F" number:1 label:LABEL_OPTIONAL type_name:"E"}]
@@ -258,7 +250,6 @@
`),
wantDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{
name: "M"
field: [{name:"F" number:1 label:LABEL_OPTIONAL type:TYPE_ENUM type_name:".M.E"}]
@@ -389,7 +380,6 @@
label: "namespace conflict on enum value",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
enum_type: [{
name: "foo"
value: [{name:"foo" number:0}]
@@ -400,7 +390,6 @@
label: "no namespace conflict on message field",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{
name: "foo"
field: [{name:"foo" number:1 label:LABEL_OPTIONAL type:TYPE_STRING}]
@@ -410,7 +399,6 @@
label: "invalid name",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name: "$"}]
`),
wantErr: `descriptor "" has an invalid nested name: "$"`,
@@ -418,7 +406,6 @@
label: "invalid empty enum",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{name:"E"}]}]
`),
wantErr: `enum "M.E" must contain at least one value declaration`,
@@ -426,7 +413,6 @@
label: "invalid enum value without number",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{name:"E" value:[{name:"one"}]}]}]
`),
wantErr: `enum value "M.one" must have a specified number`,
@@ -434,14 +420,12 @@
label: "valid enum",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{name:"E" value:[{name:"one" number:1}]}]}]
`),
}, {
label: "invalid enum reserved names",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
reserved_name: [""]
@@ -455,7 +439,6 @@
label: "duplicate enum reserved names",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
reserved_name: ["foo", "foo"]
@@ -466,7 +449,6 @@
label: "valid enum reserved names",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
reserved_name: ["foo", "bar"]
@@ -477,7 +459,6 @@
label: "use of enum reserved names",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
reserved_name: ["foo", "bar"]
@@ -489,7 +470,6 @@
label: "invalid enum reserved ranges",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
reserved_range: [{start:5 end:4}]
@@ -500,7 +480,6 @@
label: "overlapping enum reserved ranges",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
reserved_range: [{start:1 end:1000}, {start:10 end:100}]
@@ -511,7 +490,6 @@
label: "valid enum reserved names",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
reserved_range: [{start:1 end:10}, {start:100 end:1000}]
@@ -522,7 +500,6 @@
label: "use of enum reserved range",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
reserved_range: [{start:1 end:10}, {start:100 end:1000}]
@@ -534,7 +511,6 @@
label: "unused enum alias feature",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
value: [{name:"baz" number:500}]
@@ -546,7 +522,6 @@
label: "enum number conflicts",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
value: [{name:"foo" number:0}, {name:"bar" number:1}, {name:"baz" number:1}]
@@ -557,7 +532,6 @@
label: "aliased enum numbers",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
value: [{name:"foo" number:0}, {name:"bar" number:1}, {name:"baz" number:1}]
@@ -569,7 +543,6 @@
inDesc: mustParseFile(`
syntax: "proto3"
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
value: [{name:"baz" number:500}]
@@ -581,7 +554,6 @@
inDesc: mustParseFile(`
syntax: "proto3"
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
value: [{name:"baz" number:0}]
@@ -592,7 +564,6 @@
inDesc: mustParseFile(`
syntax: "proto3"
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
value: [{name:"e_Foo" number:0}, {name:"fOo" number:1}]
@@ -603,7 +574,6 @@
label: "proto2 enum has name prefix check",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
value: [{name:"e_Foo" number:0}, {name:"fOo" number:1}]
@@ -614,7 +584,6 @@
inDesc: mustParseFile(`
syntax: "proto3"
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
value: [{name:"e_Foo" number:0}, {name:"fOo" number:0}]
@@ -626,7 +595,6 @@
inDesc: mustParseFile(`
syntax: "proto3"
name: "test.proto"
- package: ""
message_type: [{name:"M" enum_type:[{
name: "E"
value: [{name:"e_Foo" number:0}, {name:"fOo" number:0}]
@@ -637,7 +605,6 @@
label: "invalid message reserved names",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
reserved_name: ["$"]
@@ -650,7 +617,6 @@
label: "valid message reserved names",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
reserved_name: ["foo", "bar"]
@@ -662,7 +628,6 @@
label: "valid message reserved names",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
reserved_name: ["foo", "bar"]
@@ -674,7 +639,6 @@
label: "invalid reserved number",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
reserved_range: [{start:1 end:1}]
@@ -686,7 +650,6 @@
label: "invalid reserved ranges",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
reserved_range: [{start:2 end:2}]
@@ -698,7 +661,6 @@
label: "overlapping reserved ranges",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
reserved_range: [{start:1 end:10}, {start:2 end:9}]
@@ -710,7 +672,6 @@
label: "use of reserved message field number",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
reserved_range: [{start:10 end:20}, {start:20 end:30}, {start:30 end:31}]
@@ -722,7 +683,6 @@
label: "invalid extension ranges",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
extension_range: [{start:-500 end:2}]
@@ -734,7 +694,6 @@
label: "overlapping reserved and extension ranges",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
reserved_range: [{start:15 end:20}, {start:1 end:3}, {start:7 end:10}]
@@ -746,7 +705,6 @@
label: "message field conflicting number",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
field: [
@@ -761,7 +719,6 @@
inDesc: mustParseFile(`
syntax: "proto3"
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
options: {message_set_wire_format:true}
@@ -778,7 +735,6 @@
label: "valid MessageSet",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
extension_range: [{start:1 end:100000}]
@@ -797,7 +753,6 @@
inDesc: mustParseFile(`
syntax: "proto3"
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
extension_range: [{start:1 end:100000}]
@@ -809,7 +764,6 @@
inDesc: mustParseFile(`
syntax: "proto3"
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
field: [
@@ -824,7 +778,6 @@
inDesc: mustParseFile(`
syntax: "proto3"
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
field: [{name:"_b_a_z_" number:1 label:LABEL_OPTIONAL type:TYPE_STRING oneof_index:0}]
@@ -835,7 +788,6 @@
label: "proto2 message fields with no conflict",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
message_type: [{name:"M" nested_type:[{
name: "M"
field: [
@@ -848,7 +800,6 @@
label: "proto3 message with unresolved enum",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
syntax: "proto3"
message_type: [{
name: "M"
@@ -867,14 +818,12 @@
label: "empty service",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
service: [{name:"service"}]
`),
}, {
label: "service with method with unresolved",
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
service: [{
name: "service"
method: [{
@@ -893,7 +842,6 @@
},
inDesc: mustParseFile(`
name: "test.proto"
- package: ""
dependency: ["proto2_enum.proto", "proto3_message.proto"]
service: [{
name: "service"
diff --git a/reflect/protodesc/proto.go b/reflect/protodesc/proto.go
index a1b7c50..a7c5cef 100644
--- a/reflect/protodesc/proto.go
+++ b/reflect/protodesc/proto.go
@@ -21,9 +21,11 @@
func ToFileDescriptorProto(file protoreflect.FileDescriptor) *descriptorpb.FileDescriptorProto {
p := &descriptorpb.FileDescriptorProto{
Name: proto.String(file.Path()),
- Package: proto.String(string(file.Package())),
Options: proto.Clone(file.Options()).(*descriptorpb.FileOptions),
}
+ if file.Package() != "" {
+ p.Package = proto.String(string(file.Package()))
+ }
for i, imports := 0, file.Imports(); i < imports.Len(); i++ {
imp := imports.Get(i)
p.Dependency = append(p.Dependency, imp.Path())
