From 6c26a04a514d59c666df383eba3d417132b9fcf6 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Fri, 3 Jan 2020 15:53:47 -0800 Subject: [PATCH] internal/filedesc: use jsonName.Init method over JSONName constructor The JSONName constructor returns a struct value which shallow copies a sync.Once within it; this is a dubious pattern. Instead, add a jsonName.Init method to initialize the value. Change-Id: I190a7239b1b62a8041ee7e4e09c0fe37b64ff623 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213237 Reviewed-by: Damien Neil --- internal/encoding/tag/tag.go | 2 +- internal/filedesc/desc.go | 10 ++++++---- internal/filedesc/desc_lazy.go | 4 ++-- reflect/protodesc/desc_init.go | 4 ++-- 4 files changed, 11 insertions(+), 9 deletions(-) diff --git a/internal/encoding/tag/tag.go b/internal/encoding/tag/tag.go index a0d9db87..16c02d7b 100644 --- a/internal/encoding/tag/tag.go +++ b/internal/encoding/tag/tag.go @@ -104,7 +104,7 @@ func Unmarshal(tag string, goType reflect.Type, evs pref.EnumValueDescriptors) p case strings.HasPrefix(s, "json="): jsonName := s[len("json="):] if jsonName != strs.JSONCamelCase(string(f.L0.FullName.Name())) { - f.L1.JSONName = fdesc.JSONName(jsonName) + f.L1.JSONName.Init(jsonName) } case s == "packed": f.L1.HasPacked = true diff --git a/internal/filedesc/desc.go b/internal/filedesc/desc.go index 81857588..a9fe07c2 100644 --- a/internal/filedesc/desc.go +++ b/internal/filedesc/desc.go @@ -490,16 +490,18 @@ func (d *Base) Syntax() pref.Syntax { return d.L0.ParentFile.Syn func (d *Base) IsPlaceholder() bool { return false } func (d *Base) ProtoInternal(pragma.DoNotImplement) {} -func JSONName(s string) jsonName { - return jsonName{has: true, name: s} -} - type jsonName struct { has bool once sync.Once name string } +// Init initializes the name. It is exported for use by other internal packages. +func (js *jsonName) Init(s string) { + js.has = true + js.name = s +} + func (js *jsonName) get(fd pref.FieldDescriptor) string { if !js.has { js.once.Do(func() { diff --git a/internal/filedesc/desc_lazy.go b/internal/filedesc/desc_lazy.go index 309d739f..c43b3e25 100644 --- a/internal/filedesc/desc_lazy.go +++ b/internal/filedesc/desc_lazy.go @@ -449,7 +449,7 @@ func (fd *Field) unmarshalFull(b []byte, sb *strs.Builder, pf *File, pd pref.Des case fieldnum.FieldDescriptorProto_Name: fd.L0.FullName = appendFullName(sb, pd.FullName(), v) case fieldnum.FieldDescriptorProto_JsonName: - fd.L1.JSONName = JSONName(sb.MakeString(v)) + fd.L1.JSONName.Init(sb.MakeString(v)) case fieldnum.FieldDescriptorProto_DefaultValue: fd.L1.Default.val = pref.ValueOfBytes(v) // temporarily store as bytes; later resolved in resolveMessages case fieldnum.FieldDescriptorProto_TypeName: @@ -542,7 +542,7 @@ func (xd *Extension) unmarshalFull(b []byte, sb *strs.Builder) { b = b[m:] switch num { case fieldnum.FieldDescriptorProto_JsonName: - xd.L2.JSONName = JSONName(sb.MakeString(v)) + xd.L2.JSONName.Init(sb.MakeString(v)) case fieldnum.FieldDescriptorProto_DefaultValue: xd.L2.Default.val = pref.ValueOfBytes(v) // temporarily store as bytes; later resolved in resolveExtensions case fieldnum.FieldDescriptorProto_TypeName: diff --git a/reflect/protodesc/desc_init.go b/reflect/protodesc/desc_init.go index 6f689b1b..ef2d0717 100644 --- a/reflect/protodesc/desc_init.go +++ b/reflect/protodesc/desc_init.go @@ -133,7 +133,7 @@ func (r descsByName) initFieldsFromDescriptorProto(fds []*descriptorpb.FieldDesc f.L1.Kind = protoreflect.Kind(fd.GetType()) } if fd.JsonName != nil { - f.L1.JSONName = filedesc.JSONName(fd.GetJsonName()) + f.L1.JSONName.Init(fd.GetJsonName()) } } return fs, nil @@ -173,7 +173,7 @@ func (r descsByName) initExtensionDeclarations(xds []*descriptorpb.FieldDescript x.L1.Kind = protoreflect.Kind(xd.GetType()) } if xd.JsonName != nil { - x.L2.JSONName = filedesc.JSONName(xd.GetJsonName()) + x.L2.JSONName.Init(xd.GetJsonName()) } } return xs, nil