Additional methods for Set type (#497)

* Added set.add(elem)

* Added set.pop()

* Added set.remove(key)

* Added set.discard(key)

* Added set.clear()

* go fmt

* documentation

* Adding test coverage, fixing failures

* Update docs

* Suggestions from code review - ordering, doc fixes, arg counts

* Documentation formatting, test coverage, check before mutating frozen sets

* Applying suggestions for review

doc/spec.md

@@ -153,6 +153,11 @@ reproducibility is paramount, such as build tools.
     * [list·insert](#list·insert)
     * [list·pop](#list·pop)
     * [list·remove](#list·remove)
+    * [set·add](#set·add)
+    * [set·clear](#set·clear)
+    * [set·discard](#set·discard)
+    * [set·pop](#set·pop)
+    * [set·remove](#set·remove)
     * [set·union](#set·union)
     * [string·capitalize](#string·capitalize)
     * [string·codepoint_ords](#string·codepoint_ords)
@@ -966,7 +971,15 @@ Sets are instantiated by calling the built-in `set` function, which
 returns a set containing all the elements of its optional argument,
 which must be an iterable sequence.  Sets have no literal syntax.
 
-The only method of a set is `union`, which is equivalent to the `|` operator.
+A set has these methods:
+
+* [`add`](#set·add)
+* [`clear`](#set·clear)
+* [`discard`](#set·discard)
+* [`pop`](#set·pop)
+* [`remove`](#set·remove)
+* [`union`](#set·union)
+
 
 A set used in a Boolean context is considered true if it is non-empty.
 
@@ -3740,6 +3753,79 @@ x.remove(2)                             # None (x == [1, 3])
 x.remove(2)                             # error: element not found
 ```
 
+<a id='set·add'></a>
+### set·add
+
+If `x` is not an element of set `S`, `S.add(x)` adds it to the set or fails if the set is frozen.
+If `x` already an element of the set, `add(x)` has no effect.
+
+It returns None.
+
+```python
+x = set([1, 2])
+x.add(3)                             # None
+x                                    # set([1, 2, 3])
+x.add(3)                             # None
+x                                    # set([1, 2, 3])
+```
+
+<a id='set·clear'></a>
+### set·clear
+
+`S.clear()` removes all items from the set or fails if the set is non-empty and frozen.
+
+It returns None.
+
+```python
+x = set([1, 2, 3])
+x.clear(2)                               # None
+x                                        # set([])
+```
+
+<a id='set·discard'></a>
+### set·discard
+
+If `x` is an element of set `S`, `S.discard(x)` removes `x` from the set, or fails if the
+set is frozen. If `x` is not an element of the set, discard has no effect.
+
+It returns None.
+
+```python
+x = set([1, 2, 3])
+x.discard(2)                             # None
+x                                        # set([1, 3])
+x.discard(2)                             # None
+x                                        # set([1, 3])
+```
+
+<a id='set·pop'></a>
+### set·pop
+
+`S.pop()` removes the first inserted item from the set and returns it.
+
+`pop` fails if the set is empty or frozen.
+
+```python
+x = set([1, 2])
+x.pop()                                 # 1
+x.pop()                                 # 2
+x.pop()                                 # error: empty set
+```
+
+<a id='set·remove'></a>
+### set·remove
+
+`S.remove(x)` removes `x` from the set and returns None.
+
+`remove` fails if the set does not contain `x` or is frozen.
+
+```python
+x = set([1, 2, 3])
+x.remove(2)                             # None
+x                                       # set([1, 3])
+x.remove(2)                             # error: element not found
+```
+
 <a id='set·union'></a>
 ### set·union
 

starlark/library.go

@@ -140,7 +140,12 @@ var (
 	}
 
 	setMethods = map[string]*Builtin{
-		"union": NewBuiltin("union", set_union),
+		"add":     NewBuiltin("add", set_add),
+		"clear":   NewBuiltin("clear", set_clear),
+		"discard": NewBuiltin("discard", set_discard),
+		"pop":     NewBuiltin("pop", set_pop),
+		"remove":  NewBuiltin("remove", set_remove),
+		"union":   NewBuiltin("union", set_union),
 	}
 )
 
@@ -2168,6 +2173,85 @@ func string_splitlines(_ *Thread, b *Builtin, args Tuple, kwargs []Tuple) (Value
 	return NewList(list), nil
 }
 
+// https://github.com/google/starlark-go/blob/master/doc/spec.md#set·add.
+func set_add(_ *Thread, b *Builtin, args Tuple, kwargs []Tuple) (Value, error) {
+	var elem Value
+	if err := UnpackPositionalArgs(b.Name(), args, kwargs, 1, &elem); err != nil {
+		return nil, err
+	}
+	if found, err := b.Receiver().(*Set).Has(elem); err != nil {
+		return nil, nameErr(b, err)
+	} else if found {
+		return None, nil
+	}
+	err := b.Receiver().(*Set).Insert(elem)
+	if err != nil {
+		return nil, nameErr(b, err)
+	}
+	return None, nil
+}
+
+// https://github.com/google/starlark-go/blob/master/doc/spec.md#set·clear.
+func set_clear(_ *Thread, b *Builtin, args Tuple, kwargs []Tuple) (Value, error) {
+	if err := UnpackPositionalArgs(b.Name(), args, kwargs, 0); err != nil {
+		return nil, err
+	}
+	if b.Receiver().(*Set).Len() > 0 {
+		if err := b.Receiver().(*Set).Clear(); err != nil {
+			return nil, nameErr(b, err)
+		}
+	}
+	return None, nil
+}
+
+// https://github.com/google/starlark-go/blob/master/doc/spec.md#set·discard.
+func set_discard(_ *Thread, b *Builtin, args Tuple, kwargs []Tuple) (Value, error) {
+	var k Value
+	if err := UnpackPositionalArgs(b.Name(), args, kwargs, 1, &k); err != nil {
+		return nil, err
+	}
+	if found, err := b.Receiver().(*Set).Has(k); err != nil {
+		return nil, nameErr(b, err)
+	} else if !found {
+		return None, nil
+	}
+	if _, err := b.Receiver().(*Set).Delete(k); err != nil {
+		return nil, nameErr(b, err) // set is frozen
+	}
+	return None, nil
+}
+
+// https://github.com/google/starlark-go/blob/master/doc/spec.md#set·pop.
+func set_pop(_ *Thread, b *Builtin, args Tuple, kwargs []Tuple) (Value, error) {
+	if err := UnpackPositionalArgs(b.Name(), args, kwargs, 0); err != nil {
+		return nil, err
+	}
+	recv := b.Receiver().(*Set)
+	k, ok := recv.ht.first()
+	if !ok {
+		return nil, nameErr(b, "empty set")
+	}
+	_, err := recv.Delete(k)
+	if err != nil {
+		return nil, nameErr(b, err) // set is frozen
+	}
+	return k, nil
+}
+
+// https://github.com/google/starlark-go/blob/master/doc/spec.md#set·remove.
+func set_remove(_ *Thread, b *Builtin, args Tuple, kwargs []Tuple) (Value, error) {
+	var k Value
+	if err := UnpackPositionalArgs(b.Name(), args, kwargs, 1, &k); err != nil {
+		return nil, err
+	}
+	if found, err := b.Receiver().(*Set).Delete(k); err != nil {
+		return nil, nameErr(b, err) // dict is frozen or key is unhashable
+	} else if found {
+		return None, nil
+	}
+	return nil, nameErr(b, "missing key")
+}
+
 // https://github.com/google/starlark-go/blob/master/doc/spec.md#set·union.
 func set_union(_ *Thread, b *Builtin, args Tuple, kwargs []Tuple) (Value, error) {
 	var iterable Iterable

starlark/testdata/builtins.star

@@ -196,7 +196,7 @@ assert.eq(getattr(hf, "x"), 2)
 assert.eq(hf.x, 2)
 # built-in types can have attributes (methods) too.
 myset = set([])
-assert.eq(dir(myset), ["union"])
+assert.eq(dir(myset), ["add", "clear", "discard", "pop", "remove", "union"])
 assert.true(hasattr(myset, "union"))
 assert.true(not hasattr(myset, "onion"))
 assert.eq(str(getattr(myset, "union")), "<built-in method union of set value>")

starlark/testdata/set.star

@@ -15,7 +15,7 @@
 # - set += iterable, perhaps?
 # Test iterator invalidation.
 
-load("assert.star", "assert")
+load("assert.star", "assert", "freeze")
 
 # literals
 # Parser does not currently support {1, 2, 3}.
@@ -116,3 +116,52 @@ assert.eq(iter(), [1, 2, 3])
 
 # sets are not indexable
 assert.fails(lambda : x[0], "unhandled.*operation")
+
+# adding and removing
+add_set = set([1,2,3])
+add_set.add(4)
+assert.true(4 in add_set)
+freeze(add_set) # no mutation of frozen set because key already present
+add_set.add(4)
+assert.fails(lambda: add_set.add(5), "add: cannot insert into frozen hash table")
+
+# remove
+remove_set = set([1,2,3])
+remove_set.remove(3)
+assert.true(3 not in remove_set)
+assert.fails(lambda: remove_set.remove(3), "remove: missing key")
+freeze(remove_set)
+assert.fails(lambda: remove_set.remove(3), "remove: cannot delete from frozen hash table")
+
+# discard
+discard_set = set([1,2,3])
+discard_set.discard(3)
+assert.true(3 not in discard_set)
+assert.eq(discard_set.discard(3), None)
+freeze(discard_set)
+assert.eq(discard_set.discard(3), None)  # no mutation of frozen set because key doesn't exist
+assert.fails(lambda: discard_set.discard(1), "discard: cannot delete from frozen hash table")
+
+
+# pop
+pop_set = set([1,2,3])
+assert.eq(pop_set.pop(), 1)
+assert.eq(pop_set.pop(), 2)
+assert.eq(pop_set.pop(), 3)
+assert.fails(lambda: pop_set.pop(), "pop: empty set")
+pop_set.add(1)
+pop_set.add(2)
+freeze(pop_set)
+assert.fails(lambda: pop_set.pop(), "pop: cannot delete from frozen hash table")
+
+
+# clear
+clear_set = set([1,2,3])
+clear_set.clear()
+assert.eq(len(clear_set), 0)
+freeze(clear_set) # no mutation of frozen set because its already empty
+assert.eq(clear_set.clear(), None) 
+
+other_clear_set = set([1,2,3])
+freeze(other_clear_set)
+assert.fails(lambda: other_clear_set.clear(), "clear: cannot clear frozen hash table")