From 997075a7f5c6f51258fc5969ab578574d3dfbf48 Mon Sep 17 00:00:00 2001 From: Lasse Folger Date: Fri, 16 Feb 2024 14:36:44 +0100 Subject: [PATCH] reflect/protodesc: add editions support Change-Id: Ie7fee936bdb480802cbaca34cf63c0dad34590b8 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/564815 LUCI-TryBot-Result: Go LUCI Reviewed-by: Michael Stapelberg --- internal/filedesc/desc.go | 3 +++ internal/strs/strings.go | 2 +- reflect/protodesc/desc_resolve.go | 4 ++-- reflect/protodesc/file_test.go | 22 ++++++++++++++++++++++ reflect/protoreflect/proto.go | 2 ++ testing/prototest/message.go | 4 ++-- testing/prototest/prototest_test.go | 2 ++ 7 files changed, 34 insertions(+), 5 deletions(-) diff --git a/internal/filedesc/desc.go b/internal/filedesc/desc.go index c0f784211..1da75cd29 100644 --- a/internal/filedesc/desc.go +++ b/internal/filedesc/desc.go @@ -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 } diff --git a/internal/strs/strings.go b/internal/strs/strings.go index 0b74e7658..a6e7df244 100644 --- a/internal/strs/strings.go +++ b/internal/strs/strings.go @@ -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() } diff --git a/reflect/protodesc/desc_resolve.go b/reflect/protodesc/desc_resolve.go index 27d7e3501..254ca5854 100644 --- a/reflect/protodesc/desc_resolve.go +++ b/reflect/protodesc/desc_resolve.go @@ -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") diff --git a/reflect/protodesc/file_test.go b/reflect/protodesc/file_test.go index 2037067fd..eeb9b60be 100644 --- a/reflect/protodesc/file_test.go +++ b/reflect/protodesc/file_test.go @@ -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(` diff --git a/reflect/protoreflect/proto.go b/reflect/protoreflect/proto.go index ec6572dfd..00b01fbd8 100644 --- a/reflect/protoreflect/proto.go +++ b/reflect/protoreflect/proto.go @@ -175,6 +175,8 @@ func (s Syntax) String() string { return "proto2" case Proto3: return "proto3" + case Editions: + return "editions" default: return fmt.Sprintf("", s) } diff --git a/testing/prototest/message.go b/testing/prototest/message.go index dfdad17cc..ee1bd4797 100644 --- a/testing/prototest/message.go +++ b/testing/prototest/message.go @@ -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() { @@ -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 { diff --git a/testing/prototest/prototest_test.go b/testing/prototest/prototest_test.go index 4307d447c..2e3d816b6 100644 --- a/testing/prototest/prototest_test.go +++ b/testing/prototest/prototest_test.go @@ -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),