Skip to content

Commit

Permalink
reflect/protodesc: add editions support
Browse files Browse the repository at this point in the history
Change-Id: Ie7fee936bdb480802cbaca34cf63c0dad34590b8
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/564815
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Stapelberg <stapelberg@google.com>
  • Loading branch information
lfolger committed Feb 19, 2024
1 parent f102ec6 commit 997075a
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 5 deletions.
3 changes: 3 additions & 0 deletions internal/filedesc/desc.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,9 @@ func (fd *Field) ProtoType(protoreflect.FieldDescriptor) {}
// WARNING: This method is exempt from the compatibility promise and may be
// removed in the future without warning.
func (fd *Field) EnforceUTF8() bool {
if fd.L0.ParentFile.L1.Syntax == protoreflect.Editions {
return fd.L1.EditionFeatures.IsUTF8Validated
}
if fd.L1.HasEnforceUTF8 {
return fd.L1.EnforceUTF8
}
Expand Down
2 changes: 1 addition & 1 deletion internal/strs/strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (

// EnforceUTF8 reports whether to enforce strict UTF-8 validation.
func EnforceUTF8(fd protoreflect.FieldDescriptor) bool {
if flags.ProtoLegacy {
if flags.ProtoLegacy || fd.Syntax() == protoreflect.Editions {
if fd, ok := fd.(interface{ EnforceUTF8() bool }); ok {
return fd.EnforceUTF8()
}
Expand Down
4 changes: 2 additions & 2 deletions reflect/protodesc/desc_resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,8 @@ func unmarshalDefault(s string, fd protoreflect.FieldDescriptor, allowUnresolvab
} else if err != nil {
return v, ev, err
}
if fd.Syntax() == protoreflect.Proto3 {
return v, ev, errors.New("cannot be specified under proto3 semantics")
if !fd.HasPresence() {
return v, ev, errors.New("cannot be specified with implicit field presence")
}
if fd.Kind() == protoreflect.MessageKind || fd.Kind() == protoreflect.GroupKind || fd.Cardinality() == protoreflect.Repeated {
return v, ev, errors.New("cannot be specified on composite types")
Expand Down
22 changes: 22 additions & 0 deletions reflect/protodesc/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,28 @@ func TestNewFile(t *testing.T) {
oneof_decl: [{name:"baz"}] # proto3 name conflict logic does not include oneof
}]}]
`),
}, {
label: "proto3 message field with defaults",
inDesc: mustParseFile(`
syntax: "proto3"
name: "test.proto"
message_type: [{name:"M" nested_type:[{
name: "M"
field: [{name:"a" number:1 type:TYPE_STRING default_value:"abc"}]
}]}]
`),
wantErr: `message field "M.M.a" has invalid default: cannot be specified with implicit field presence`,
}, {
label: "proto editions implicit presence field with defaults",
inDesc: mustParseFile(`
syntax: "proto3"
name: "test.proto"
message_type: [{name:"M" nested_type:[{
name: "M"
field: [{name:"a" number:1 type:TYPE_STRING default_value:"abc" options:{features:{field_presence:IMPLICIT}}}]
}]}]
`),
wantErr: `message field "M.M.a" has invalid default: cannot be specified with implicit field presence`,
}, {
label: "proto2 message fields with no conflict",
inDesc: mustParseFile(`
Expand Down
2 changes: 2 additions & 0 deletions reflect/protoreflect/proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,8 @@ func (s Syntax) String() string {
return "proto2"
case Proto3:
return "proto3"
case Editions:
return "editions"
default:
return fmt.Sprintf("<unknown:%d>", s)
}
Expand Down
4 changes: 2 additions & 2 deletions testing/prototest/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ func testField(t testing.TB, m protoreflect.Message, fd protoreflect.FieldDescri
m.Set(fd, v)
wantHas := true
if n == 0 {
if fd.Syntax() == protoreflect.Proto3 && fd.Message() == nil {
if !fd.HasPresence() {
wantHas = false
}
if fd.IsExtension() {
Expand All @@ -244,7 +244,7 @@ func testField(t testing.TB, m protoreflect.Message, fd protoreflect.FieldDescri
wantHas = true
}
}
if fd.Syntax() == protoreflect.Proto3 && fd.Cardinality() != protoreflect.Repeated && fd.ContainingOneof() == nil && fd.Kind() == protoreflect.EnumKind && v.Enum() == 0 {
if !fd.HasPresence() && fd.Cardinality() != protoreflect.Repeated && fd.ContainingOneof() == nil && fd.Kind() == protoreflect.EnumKind && v.Enum() == 0 {
wantHas = false
}
if got, want := m.Has(fd), wantHas; got != want {
Expand Down
2 changes: 2 additions & 0 deletions testing/prototest/prototest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ import (
_ "google.golang.org/protobuf/internal/testprotos/test/weak1"
_ "google.golang.org/protobuf/internal/testprotos/test/weak2"
test3pb "google.golang.org/protobuf/internal/testprotos/test3"
testeditionspb "google.golang.org/protobuf/internal/testprotos/testeditions"
)

func Test(t *testing.T) {
ms := []proto.Message{
(*testpb.TestAllTypes)(nil),
(*test3pb.TestAllTypes)(nil),
(*testeditionspb.TestAllTypes)(nil),
(*testpb.TestRequired)(nil),
(*irregularpb.Message)(nil),
(*testpb.TestAllExtensions)(nil),
Expand Down

0 comments on commit 997075a

Please sign in to comment.