From ef185ee6a0cf45ba49b4d35b40156ea97145af51 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 22 Apr 2024 16:23:33 -0700 Subject: [PATCH] collections: Don't panic when Has called on uninitialized Set The convention for Go maps is that read operations treat an uninitialized map as if empty, and collections.Set is intended to behave in a map-like way, so Set.Has should return false when called on a totally-uninitialized set. Previously this would panic because an uninitialized set has a nil key function. --- internal/collections/set.go | 5 +++++ internal/collections/set_test.go | 14 ++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/internal/collections/set.go b/internal/collections/set.go index 2cb30ee672..fb8c1b8259 100644 --- a/internal/collections/set.go +++ b/internal/collections/set.go @@ -49,6 +49,11 @@ func NewSetCmp[T comparable]() Set[T] { // Has returns true if the given value is present in the set, or false // otherwise. func (s Set[T]) Has(v T) bool { + if len(s.members) == 0 { + // We'll skip calling "s.key" in this case, so that we don't panic + // if called on an uninitialized Set. + return false + } k := s.key(v) _, ok := s.members[k] return ok diff --git a/internal/collections/set_test.go b/internal/collections/set_test.go index ed9a1939cc..b7b4acb188 100644 --- a/internal/collections/set_test.go +++ b/internal/collections/set_test.go @@ -37,3 +37,17 @@ func TestSet(t *testing.T) { t.Errorf("set doesn't have \"b\" after adding it") } } + +func TestSetUninit(t *testing.T) { + // An zero-value set should behave like it's empty for read-only operations. + var zeroSet Set[string] + if got, want := zeroSet.Len(), 0; got != want { + t.Errorf("wrong number of elements\ngot: %d\nwant: %d", got, want) + } + if zeroSet.Has("anything") { + // (this is really just testing that we can call Has without panicking; + // it's unlikely that this would ever fail by successfully lying about + // a particular member being present.) + t.Error("Has reported that \"anything\" is present") + } +}