Skip to content

Commit

Permalink
protobuf: fix required/group bug in descriptor proto output of editio…
Browse files Browse the repository at this point in the history
…ns files

These need to be converted back to the appropriate label/type enums to produce valid descriptor protos under editions.

Change-Id: Ife04c4c556ffb06d1bc477725ff49058928a75ca
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/575916
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Stapelberg <stapelberg@google.com>
Reviewed-by: Michael Stapelberg <stapelberg@google.com>
  • Loading branch information
mkruskal-google authored and gopherbot committed Apr 5, 2024
1 parent bab4b5d commit b30b634
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 2 deletions.
5 changes: 3 additions & 2 deletions internal/filedesc/desc.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,8 +538,9 @@ func (md *Method) ProtoInternal(pragma.DoNotImplement) {}
// Surrogate files are can be used to create standalone descriptors
// where the syntax is only information derived from the parent file.
var (
SurrogateProto2 = &File{L1: FileL1{Syntax: protoreflect.Proto2}, L2: &FileL2{}}
SurrogateProto3 = &File{L1: FileL1{Syntax: protoreflect.Proto3}, L2: &FileL2{}}
SurrogateProto2 = &File{L1: FileL1{Syntax: protoreflect.Proto2}, L2: &FileL2{}}
SurrogateProto3 = &File{L1: FileL1{Syntax: protoreflect.Proto3}, L2: &FileL2{}}
SurrogateEdition2023 = &File{L1: FileL1{Syntax: protoreflect.Editions, Edition: Edition2023}, L2: &FileL2{}}
)

type (
Expand Down
1 change: 1 addition & 0 deletions internal/filedesc/editions.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ func init() {
unmarshalEditionDefaults(editiondefaults.Defaults)
SurrogateProto2.L1.EditionFeatures = getFeaturesFor(EditionProto2)
SurrogateProto3.L1.EditionFeatures = getFeaturesFor(EditionProto3)
SurrogateEdition2023.L1.EditionFeatures = getFeaturesFor(Edition2023)
}

func unmarshalGoFeature(b []byte, parent EditionFeatures) EditionFeatures {
Expand Down
12 changes: 12 additions & 0 deletions reflect/protodesc/proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,18 @@ func ToFieldDescriptorProto(field protoreflect.FieldDescriptor) *descriptorpb.Fi
if field.Syntax() == protoreflect.Proto3 && field.HasOptionalKeyword() {
p.Proto3Optional = proto.Bool(true)
}
if field.Syntax() == protoreflect.Editions {
// Editions have no group keyword, this type is only set so that downstream users continue
// treating this as delimited encoding.
if p.GetType() == descriptorpb.FieldDescriptorProto_TYPE_GROUP {
p.Type = descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum()
}
// Editions have no required keyword, this label is only set so that downstream users continue
// treating it as required.
if p.GetLabel() == descriptorpb.FieldDescriptorProto_LABEL_REQUIRED {
p.Label = descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum()
}
}
if field.HasDefault() {
def, err := defval.Marshal(field.Default(), field.DefaultEnumValue(), field.Kind(), defval.Descriptor)
if err != nil && field.DefaultEnumValue() != nil {
Expand Down
110 changes: 110 additions & 0 deletions reflect/protodesc/proto_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
// Copyright 2018 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package protodesc

import (
"testing"

"github.com/google/go-cmp/cmp"
"google.golang.org/protobuf/internal/filedesc"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/testing/protocmp"
"google.golang.org/protobuf/types/descriptorpb"
)

func TestEditionsRequired(t *testing.T) {
fd := new(filedesc.Field)
fd.L0.ParentFile = filedesc.SurrogateEdition2023
fd.L0.FullName = "foo_field"
fd.L1.Number = 1337
fd.L1.Cardinality = protoreflect.Required
fd.L1.Kind = protoreflect.BytesKind

want := &descriptorpb.FieldDescriptorProto{
Name: proto.String("foo_field"),
Number: proto.Int32(1337),
Label: descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
Type: descriptorpb.FieldDescriptorProto_TYPE_BYTES.Enum(),
}

got := ToFieldDescriptorProto(fd)
if diff := cmp.Diff(want, got, protocmp.Transform()); diff != "" {
t.Errorf("ToFieldDescriptor: unexpected diff (-want +got):\n%s", diff)
}
}

func TestProto2Required(t *testing.T) {
fd := new(filedesc.Field)
fd.L0.ParentFile = filedesc.SurrogateProto2
fd.L0.FullName = "foo_field"
fd.L1.Number = 1337
fd.L1.Cardinality = protoreflect.Required
fd.L1.Kind = protoreflect.BytesKind

want := &descriptorpb.FieldDescriptorProto{
Name: proto.String("foo_field"),
Number: proto.Int32(1337),
Label: descriptorpb.FieldDescriptorProto_LABEL_REQUIRED.Enum(),
Type: descriptorpb.FieldDescriptorProto_TYPE_BYTES.Enum(),
}

got := ToFieldDescriptorProto(fd)
if diff := cmp.Diff(want, got, protocmp.Transform()); diff != "" {
t.Errorf("ToFieldDescriptor: unexpected diff (-want +got):\n%s", diff)
}
}

func TestEditionsDelimited(t *testing.T) {
md := new(filedesc.Message)
md.L0.ParentFile = filedesc.SurrogateEdition2023
md.L0.FullName = "foo_message"
fd := new(filedesc.Field)
fd.L0.ParentFile = filedesc.SurrogateEdition2023
fd.L0.FullName = "foo_field"
fd.L1.Number = 1337
fd.L1.Cardinality = protoreflect.Optional
fd.L1.Kind = protoreflect.GroupKind
fd.L1.Message = md

want := &descriptorpb.FieldDescriptorProto{
Name: proto.String("foo_field"),
Number: proto.Int32(1337),
Label: descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(),
TypeName: proto.String(".foo_message"),
}

got := ToFieldDescriptorProto(fd)
if diff := cmp.Diff(want, got, protocmp.Transform()); diff != "" {
t.Errorf("ToFieldDescriptor: unexpected diff (-want +got):\n%s", diff)
}
}

func TestProto2Group(t *testing.T) {
md := new(filedesc.Message)
md.L0.ParentFile = filedesc.SurrogateProto2
md.L0.FullName = "foo_message"
fd := new(filedesc.Field)
fd.L0.ParentFile = filedesc.SurrogateProto2
fd.L0.FullName = "foo_field"
fd.L1.Number = 1337
fd.L1.Cardinality = protoreflect.Optional
fd.L1.Kind = protoreflect.GroupKind
fd.L1.Message = md

want := &descriptorpb.FieldDescriptorProto{
Name: proto.String("foo_field"),
Number: proto.Int32(1337),
Label: descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(),
Type: descriptorpb.FieldDescriptorProto_TYPE_GROUP.Enum(),
TypeName: proto.String(".foo_message"),
}

got := ToFieldDescriptorProto(fd)
if diff := cmp.Diff(want, got, protocmp.Transform()); diff != "" {
t.Errorf("ToFieldDescriptor: unexpected diff (-want +got):\n%s", diff)
}
}

0 comments on commit b30b634

Please sign in to comment.