From f26a9e7e3071086d03259fcc86d3be90a33d32cc Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Thu, 20 Feb 2020 10:05:37 -0800 Subject: [PATCH] all: rename IsInitialized as CheckInitialized An Is prefix implies it returns a boolean. A Check prefix better suggests that it could return an error. Change-Id: I6ffcb32099a944c656c07654c294a0980efb2d0e Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/220338 Reviewed-by: Damien Neil --- encoding/protojson/decode.go | 2 +- encoding/protojson/encode.go | 2 +- encoding/prototext/decode.go | 2 +- encoding/prototext/encode.go | 2 +- internal/fuzz/wirefuzz/fuzz.go | 2 +- internal/impl/{isinit.go => checkinit.go} | 6 ++--- internal/impl/codec_field.go | 16 ++++++------ internal/impl/codec_map.go | 2 +- internal/impl/codec_message.go | 4 +-- proto/{isinit.go => checkinit.go} | 27 ++++++++++++--------- proto/{isinit_test.go => checkinit_test.go} | 6 ++--- proto/decode.go | 2 +- proto/doc.go | 2 +- proto/encode.go | 2 +- proto/methods_test.go | 2 +- reflect/protoreflect/methods.go | 16 ++++++------ runtime/protoiface/methods.go | 12 ++++----- 17 files changed, 56 insertions(+), 51 deletions(-) rename internal/impl/{isinit.go => checkinit.go} (92%) rename proto/{isinit.go => checkinit.go} (60%) rename proto/{isinit_test.go => checkinit_test.go} (90%) diff --git a/encoding/protojson/decode.go b/encoding/protojson/decode.go index 4d13d777..71c755c1 100644 --- a/encoding/protojson/decode.go +++ b/encoding/protojson/decode.go @@ -75,7 +75,7 @@ func (o UnmarshalOptions) Unmarshal(b []byte, m proto.Message) error { if o.AllowPartial { return nil } - return proto.IsInitialized(m) + return proto.CheckInitialized(m) } type decoder struct { diff --git a/encoding/protojson/encode.go b/encoding/protojson/encode.go index b8c4aa24..243d1588 100644 --- a/encoding/protojson/encode.go +++ b/encoding/protojson/encode.go @@ -123,7 +123,7 @@ func (o MarshalOptions) Marshal(m proto.Message) ([]byte, error) { if o.AllowPartial { return enc.Bytes(), nil } - return enc.Bytes(), proto.IsInitialized(m) + return enc.Bytes(), proto.CheckInitialized(m) } type encoder struct { diff --git a/encoding/prototext/decode.go b/encoding/prototext/decode.go index 9dea257d..cde2c20d 100644 --- a/encoding/prototext/decode.go +++ b/encoding/prototext/decode.go @@ -66,7 +66,7 @@ func (o UnmarshalOptions) Unmarshal(b []byte, m proto.Message) error { if o.AllowPartial { return nil } - return proto.IsInitialized(m) + return proto.CheckInitialized(m) } type decoder struct { diff --git a/encoding/prototext/encode.go b/encoding/prototext/encode.go index 43bd50a2..7801791b 100644 --- a/encoding/prototext/encode.go +++ b/encoding/prototext/encode.go @@ -118,7 +118,7 @@ func (o MarshalOptions) Marshal(m proto.Message) ([]byte, error) { if o.AllowPartial { return out, nil } - return out, proto.IsInitialized(m) + return out, proto.CheckInitialized(m) } type encoder struct { diff --git a/internal/fuzz/wirefuzz/fuzz.go b/internal/fuzz/wirefuzz/fuzz.go index 01463f87..0c084666 100644 --- a/internal/fuzz/wirefuzz/fuzz.go +++ b/internal/fuzz/wirefuzz/fuzz.go @@ -39,7 +39,7 @@ func Fuzz(data []byte) (score int) { default: panic("unmarshal ok with validation status: " + valid.String()) } - if proto.IsInitialized(m1) != nil && vinit { + if proto.CheckInitialized(m1) != nil && vinit { panic("validation reports partial message is initialized") } data1, err := proto.MarshalOptions{ diff --git a/internal/impl/isinit.go b/internal/impl/checkinit.go similarity index 92% rename from internal/impl/isinit.go rename to internal/impl/checkinit.go index 4bd978f9..b82341e5 100644 --- a/internal/impl/isinit.go +++ b/internal/impl/checkinit.go @@ -12,17 +12,17 @@ import ( piface "google.golang.org/protobuf/runtime/protoiface" ) -func (mi *MessageInfo) isInitialized(in piface.IsInitializedInput) (piface.IsInitializedOutput, error) { +func (mi *MessageInfo) checkInitialized(in piface.CheckInitializedInput) (piface.CheckInitializedOutput, error) { var p pointer if ms, ok := in.Message.(*messageState); ok { p = ms.pointer() } else { p = in.Message.(*messageReflectWrapper).pointer() } - return piface.IsInitializedOutput{}, mi.isInitializedPointer(p) + return piface.CheckInitializedOutput{}, mi.checkInitializedPointer(p) } -func (mi *MessageInfo) isInitializedPointer(p pointer) error { +func (mi *MessageInfo) checkInitializedPointer(p pointer) error { mi.init() if !mi.needsInitCheck { return nil diff --git a/internal/impl/codec_field.go b/internal/impl/codec_field.go index bc998d37..6204e653 100644 --- a/internal/impl/codec_field.go +++ b/internal/impl/codec_field.go @@ -167,7 +167,7 @@ func makeWeakMessageFieldCoder(fd pref.FieldDescriptor) pointerCoderFuncs { if !ok { return nil } - return proto.IsInitialized(m) + return proto.CheckInitialized(m) }, merge: func(dst, src pointer, f *coderFieldInfo, opts mergeOptions) { sm, ok := src.WeakFields().get(f.num) @@ -219,7 +219,7 @@ func makeMessageFieldCoder(fd pref.FieldDescriptor, ft reflect.Type) pointerCode }, isInit: func(p pointer, f *coderFieldInfo) error { m := asMessage(p.AsValueOf(ft).Elem()) - return proto.IsInitialized(m) + return proto.CheckInitialized(m) }, merge: mergeMessage, } @@ -257,7 +257,7 @@ func consumeMessageInfo(b []byte, p pointer, wtyp wire.Type, f *coderFieldInfo, } func isInitMessageInfo(p pointer, f *coderFieldInfo) error { - return f.mi.isInitializedPointer(p.Elem()) + return f.mi.checkInitializedPointer(p.Elem()) } func sizeMessage(m proto.Message, tagsize int, _ marshalOptions) int { @@ -307,7 +307,7 @@ func consumeMessageValue(b []byte, v pref.Value, _ wire.Number, wtyp wire.Type, func isInitMessageValue(v pref.Value) error { m := v.Message().Interface() - return proto.IsInitialized(m) + return proto.CheckInitialized(m) } var coderMessageValue = valueCoderFuncs{ @@ -374,7 +374,7 @@ func makeGroupFieldCoder(fd pref.FieldDescriptor, ft reflect.Type) pointerCoderF }, isInit: func(p pointer, f *coderFieldInfo) error { m := asMessage(p.AsValueOf(ft).Elem()) - return proto.IsInitialized(m) + return proto.CheckInitialized(m) }, merge: mergeMessage, } @@ -509,7 +509,7 @@ func consumeMessageSliceInfo(b []byte, p pointer, wtyp wire.Type, f *coderFieldI func isInitMessageSliceInfo(p pointer, f *coderFieldInfo) error { s := p.PointerSlice() for _, v := range s { - if err := f.mi.isInitializedPointer(v); err != nil { + if err := f.mi.checkInitializedPointer(v); err != nil { return err } } @@ -567,7 +567,7 @@ func isInitMessageSlice(p pointer, goType reflect.Type) error { s := p.PointerSlice() for _, v := range s { m := asMessage(v.AsValueOf(goType.Elem())) - if err := proto.IsInitialized(m); err != nil { + if err := proto.CheckInitialized(m); err != nil { return err } } @@ -629,7 +629,7 @@ func isInitMessageSliceValue(listv pref.Value) error { list := listv.List() for i, llen := 0, list.Len(); i < llen; i++ { m := list.Get(i).Message().Interface() - if err := proto.IsInitialized(m); err != nil { + if err := proto.CheckInitialized(m); err != nil { return err } } diff --git a/internal/impl/codec_map.go b/internal/impl/codec_map.go index dfeb9440..dd5f8c72 100644 --- a/internal/impl/codec_map.go +++ b/internal/impl/codec_map.go @@ -320,7 +320,7 @@ func isInitMap(mapv reflect.Value, mapi *mapInfo, f *coderFieldInfo) error { iter := mapRange(mapv) for iter.Next() { val := pointerOfValue(iter.Value()) - if err := mi.isInitializedPointer(val); err != nil { + if err := mi.checkInitializedPointer(val); err != nil { return err } } diff --git a/internal/impl/codec_message.go b/internal/impl/codec_message.go index 08f4d5a4..a86468a5 100644 --- a/internal/impl/codec_message.go +++ b/internal/impl/codec_message.go @@ -148,8 +148,8 @@ func (mi *MessageInfo) makeCoderMethods(t reflect.Type, si structInfo) { mi.methods.Flags |= piface.SupportUnmarshalDiscardUnknown mi.methods.Unmarshal = mi.unmarshal } - if mi.methods.IsInitialized == nil { - mi.methods.IsInitialized = mi.isInitialized + if mi.methods.CheckInitialized == nil { + mi.methods.CheckInitialized = mi.checkInitialized } if mi.methods.Merge == nil { mi.methods.Merge = mi.merge diff --git a/proto/isinit.go b/proto/checkinit.go similarity index 60% rename from proto/isinit.go rename to proto/checkinit.go index c6cde594..ca2124d0 100644 --- a/proto/isinit.go +++ b/proto/checkinit.go @@ -10,23 +10,28 @@ import ( "google.golang.org/protobuf/runtime/protoiface" ) -// IsInitialized returns an error if any required fields in m are not set. +// Deprecated: Use CheckInitialized instead. func IsInitialized(m Message) error { - return isInitialized(m.ProtoReflect()) + return CheckInitialized(m) } -// IsInitialized returns an error if any required fields in m are not set. -func isInitialized(m protoreflect.Message) error { - if methods := protoMethods(m); methods != nil && methods.IsInitialized != nil { - _, err := methods.IsInitialized(protoiface.IsInitializedInput{ +// CheckInitialized returns an error if any required fields in m are not set. +func CheckInitialized(m Message) error { + return checkInitialized(m.ProtoReflect()) +} + +// CheckInitialized returns an error if any required fields in m are not set. +func checkInitialized(m protoreflect.Message) error { + if methods := protoMethods(m); methods != nil && methods.CheckInitialized != nil { + _, err := methods.CheckInitialized(protoiface.CheckInitializedInput{ Message: m, }) return err } - return isInitializedSlow(m) + return checkInitializedSlow(m) } -func isInitializedSlow(m protoreflect.Message) error { +func checkInitializedSlow(m protoreflect.Message) error { md := m.Descriptor() fds := md.Fields() for i, nums := 0, md.RequiredNumbers(); i < nums.Len(); i++ { @@ -43,21 +48,21 @@ func isInitializedSlow(m protoreflect.Message) error { return true } for i, list := 0, v.List(); i < list.Len() && err == nil; i++ { - err = isInitialized(list.Get(i).Message()) + err = checkInitialized(list.Get(i).Message()) } case fd.IsMap(): if fd.MapValue().Message() == nil { return true } v.Map().Range(func(key protoreflect.MapKey, v protoreflect.Value) bool { - err = isInitialized(v.Message()) + err = checkInitialized(v.Message()) return err == nil }) default: if fd.Message() == nil { return true } - err = isInitialized(v.Message()) + err = checkInitialized(v.Message()) } return err == nil }) diff --git a/proto/isinit_test.go b/proto/checkinit_test.go similarity index 90% rename from proto/isinit_test.go rename to proto/checkinit_test.go index 8d8b1b69..29c9a9a4 100644 --- a/proto/isinit_test.go +++ b/proto/checkinit_test.go @@ -17,7 +17,7 @@ import ( weakpb "google.golang.org/protobuf/internal/testprotos/test/weak1" ) -func TestIsInitializedErrors(t *testing.T) { +func TestCheckInitializedErrors(t *testing.T) { type test struct { m proto.Message want string @@ -76,13 +76,13 @@ func TestIsInitializedErrors(t *testing.T) { t.SkipNow() } - err := proto.IsInitialized(tt.m) + err := proto.CheckInitialized(tt.m) got := "" if err != nil { got = fmt.Sprintf("%q", err) } if !strings.Contains(got, tt.want) { - t.Errorf("IsInitialized(m):\n got: %v\nwant contains: %v\nMessage:\n%v", got, tt.want, prototext.Format(tt.m)) + t.Errorf("CheckInitialized(m):\n got: %v\nwant contains: %v\nMessage:\n%v", got, tt.want, prototext.Format(tt.m)) } }) } diff --git a/proto/decode.go b/proto/decode.go index 14afa292..30fd5294 100644 --- a/proto/decode.go +++ b/proto/decode.go @@ -95,7 +95,7 @@ func (o UnmarshalOptions) unmarshal(b []byte, message Message) (out protoiface.U if allowPartial || (out.Flags&protoiface.UnmarshalInitialized != 0) { return out, nil } - return out, isInitialized(m) + return out, checkInitialized(m) } func (o UnmarshalOptions) unmarshalMessage(b []byte, m protoreflect.Message) error { diff --git a/proto/doc.go b/proto/doc.go index e1d13d70..c52d8c4a 100644 --- a/proto/doc.go +++ b/proto/doc.go @@ -43,7 +43,7 @@ // // • Reset clears the content of a message. // -// • IsInitialized reports whether all required fields in a message are set. +// • CheckInitialized reports whether all required fields in a message are set. // // // Optional scalar constructors diff --git a/proto/encode.go b/proto/encode.go index 950556fc..2a8e8952 100644 --- a/proto/encode.go +++ b/proto/encode.go @@ -136,7 +136,7 @@ func (o MarshalOptions) marshal(b []byte, message Message) (out protoiface.Marsh if allowPartial { return out, nil } - return out, isInitialized(m) + return out, checkInitialized(m) } func (o MarshalOptions) marshalMessage(b []byte, m protoreflect.Message) ([]byte, error) { diff --git a/proto/methods_test.go b/proto/methods_test.go index e231e295..6809f046 100644 --- a/proto/methods_test.go +++ b/proto/methods_test.go @@ -131,7 +131,7 @@ func TestSelfMarshalerWithDescriptor(t *testing.T) { } } -func TestDecodeFastIsInitialized(t *testing.T) { +func TestDecodeFastCheckInitialized(t *testing.T) { for _, test := range testValidMessages { if !test.checkFastInit { continue diff --git a/reflect/protoreflect/methods.go b/reflect/protoreflect/methods.go index 57fc0145..6be5d16e 100644 --- a/reflect/protoreflect/methods.go +++ b/reflect/protoreflect/methods.go @@ -17,12 +17,12 @@ import ( type ( methods = struct { pragma.NoUnkeyedLiterals - Flags supportFlags - Size func(sizeInput) sizeOutput - Marshal func(marshalInput) (marshalOutput, error) - Unmarshal func(unmarshalInput) (unmarshalOutput, error) - Merge func(mergeInput) mergeOutput - IsInitialized func(isInitializedInput) (isInitializedOutput, error) + Flags supportFlags + Size func(sizeInput) sizeOutput + Marshal func(marshalInput) (marshalOutput, error) + Unmarshal func(unmarshalInput) (unmarshalOutput, error) + Merge func(mergeInput) mergeOutput + CheckInitialized func(checkInitializedInput) (checkInitializedOutput, error) } supportFlags = uint64 sizeInput = struct { @@ -67,11 +67,11 @@ type ( pragma.NoUnkeyedLiterals Flags uint8 } - isInitializedInput = struct { + checkInitializedInput = struct { pragma.NoUnkeyedLiterals Message Message } - isInitializedOutput = struct { + checkInitializedOutput = struct { pragma.NoUnkeyedLiterals } ) diff --git a/runtime/protoiface/methods.go b/runtime/protoiface/methods.go index 1a0baa76..aeaf80a4 100644 --- a/runtime/protoiface/methods.go +++ b/runtime/protoiface/methods.go @@ -37,8 +37,8 @@ type Methods = struct { // Merge merges the contents of a source message into a destination message. Merge func(MergeInput) MergeOutput - // IsInitialized returns an error if any required fields in the message are not set. - IsInitialized func(IsInitializedInput) (IsInitializedOutput, error) + // CheckInitialized returns an error if any required fields in the message are not set. + CheckInitialized func(CheckInitializedInput) (CheckInitializedOutput, error) } // SupportFlags indicate support for optional features. @@ -154,14 +154,14 @@ const ( MergeComplete MergeOutputFlags = 1 << iota ) -// IsInitializedInput is input to the IsInitialized method. -type IsInitializedInput = struct { +// CheckInitializedInput is input to the CheckInitialized method. +type CheckInitializedInput = struct { pragma.NoUnkeyedLiterals Message protoreflect.Message } -// IsInitializedOutput is output from the IsInitialized method. -type IsInitializedOutput = struct { +// CheckInitializedOutput is output from the CheckInitialized method. +type CheckInitializedOutput = struct { pragma.NoUnkeyedLiterals }