Skip to content

Commit 8191a0d

Browse files
committed
Make TagSet.Remove a no-op when contentType is nil
Previously, syncTagSet.Remove() can panic if function parameter contentType is nil. This commit adds an early return so Remove(nil) is a silent no-op, mirroring the behavior of Go's built-in delete() on a missing key.
1 parent 1a705ce commit 8191a0d

2 files changed

Lines changed: 43 additions & 0 deletions

File tree

tag.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ type TagSet interface {
155155
Add(opts TagOptions, contentType reflect.Type, num uint64, nestedNum ...uint64) error
156156

157157
// Remove removes given tag content type from TagSet.
158+
// Remove is a no-op if contentType is nil.
158159
Remove(contentType reflect.Type)
159160

160161
tagProvider
@@ -245,7 +246,11 @@ func (t *syncTagSet) Add(opts TagOptions, contentType reflect.Type, num uint64,
245246
}
246247

247248
// Remove removes given tag content type from TagSet.
249+
// Remove is a no-op if contentType is nil.
248250
func (t *syncTagSet) Remove(contentType reflect.Type) {
251+
if contentType == nil {
252+
return
253+
}
249254
for contentType.Kind() == reflect.Pointer {
250255
contentType = contentType.Elem()
251256
}

tag_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,44 @@ func TestAddRemoveTag(t *testing.T) {
569569
tags.Remove(myFloatType)
570570
}
571571

572+
func TestTagSetRemoveNil(t *testing.T) {
573+
type myInt int
574+
myIntType := reflect.TypeOf(myInt(0))
575+
576+
tags := NewTagSet()
577+
stags := tags.(*syncTagSet)
578+
579+
// Remove nil contentType from an empty TagSet
580+
tags.Remove(nil)
581+
if len(stags.t) != 0 {
582+
t.Errorf("TagSet len is %d, want %d", len(stags.t), 0)
583+
}
584+
585+
if err := tags.Add(TagOptions{DecTag: DecTagRequired, EncTag: EncTagRequired}, myIntType, 100); err != nil {
586+
t.Errorf("TagSet.Add(%s, %d) returned error %v", myIntType.String(), 100, err)
587+
}
588+
if len(stags.t) != 1 {
589+
t.Errorf("TagSet len is %d, want %d", len(stags.t), 1)
590+
}
591+
592+
// Remove nil contentType from non-empty TagSet
593+
tags.Remove(nil)
594+
if len(stags.t) != 1 {
595+
t.Errorf("TagSet len is %d, want %d", len(stags.t), 1)
596+
}
597+
598+
expectedTI := &tagItem{
599+
num: []uint64{100},
600+
cborTagNum: []byte{0xd8, 0x64},
601+
contentType: myIntType,
602+
opts: TagOptions{DecTag: DecTagRequired, EncTag: EncTagRequired},
603+
}
604+
ti := tags.getTagItemFromType(myIntType)
605+
if !reflect.DeepEqual(ti, expectedTI) {
606+
t.Errorf("tagItem is %+v, want %+v", ti, expectedTI)
607+
}
608+
}
609+
572610
func TestAddTagTypeAliasError(t *testing.T) {
573611
type myBool = bool
574612
type myUint = uint

0 commit comments

Comments
 (0)