reflect/protoreflect: clarify Clear and Remove operation on missing entries
Follow the precedence of Go maps where deletion on a key without an entry in
the map is a noop. Similarly, document that the following methods are safe
to call with entries that do not exist:
* Map.Clear
* KnownFields.Clear
* ExtensionFieldTypes.Remove
Change the implementation for each of these to match the documented behavior.
Change-Id: Ifccff9b7b03baaeffdc366d05f6286ba60e14934
Reviewed-on: https://go-review.googlesource.com/c/148317
Reviewed-by: Herbie Ong <herbie@google.com>
diff --git a/internal/impl/message.go b/internal/impl/message.go
index a3e4b9d..c406b3d 100644
--- a/internal/impl/message.go
+++ b/internal/impl/message.go
@@ -310,8 +310,8 @@
func (emptyUnknownFields) Len() int { return 0 }
func (emptyUnknownFields) Get(pref.FieldNumber) pref.RawFields { return nil }
-func (emptyUnknownFields) Set(pref.FieldNumber, pref.RawFields) { /* noop */ }
-func (emptyUnknownFields) Range(func(pref.FieldNumber, pref.RawFields) bool) {}
+func (emptyUnknownFields) Set(pref.FieldNumber, pref.RawFields) { return } // noop
+func (emptyUnknownFields) Range(func(pref.FieldNumber, pref.RawFields) bool) { return }
func (emptyUnknownFields) IsSupported() bool { return false }
type emptyExtensionFields struct{}
@@ -319,17 +319,17 @@
func (emptyExtensionFields) Len() int { return 0 }
func (emptyExtensionFields) Has(pref.FieldNumber) bool { return false }
func (emptyExtensionFields) Get(pref.FieldNumber) pref.Value { return pref.Value{} }
-func (emptyExtensionFields) Set(pref.FieldNumber, pref.Value) { panic("invalid field") }
-func (emptyExtensionFields) Clear(pref.FieldNumber) { panic("invalid field") }
-func (emptyExtensionFields) Mutable(pref.FieldNumber) pref.Mutable { panic("invalid field") }
-func (emptyExtensionFields) Range(f func(pref.FieldNumber, pref.Value) bool) {}
+func (emptyExtensionFields) Set(pref.FieldNumber, pref.Value) { panic("extensions not supported") }
+func (emptyExtensionFields) Clear(pref.FieldNumber) { return } // noop
+func (emptyExtensionFields) Mutable(pref.FieldNumber) pref.Mutable { panic("extensions not supported") }
+func (emptyExtensionFields) Range(f func(pref.FieldNumber, pref.Value) bool) { return }
func (emptyExtensionFields) ExtensionTypes() pref.ExtensionFieldTypes { return emptyExtensionTypes{} }
type emptyExtensionTypes struct{}
func (emptyExtensionTypes) Len() int { return 0 }
func (emptyExtensionTypes) Register(pref.ExtensionType) { panic("extensions not supported") }
-func (emptyExtensionTypes) Remove(pref.ExtensionType) { panic("extensions not supported") }
+func (emptyExtensionTypes) Remove(pref.ExtensionType) { return } // noop
func (emptyExtensionTypes) ByNumber(pref.FieldNumber) pref.ExtensionType { return nil }
func (emptyExtensionTypes) ByName(pref.FullName) pref.ExtensionType { return nil }
-func (emptyExtensionTypes) Range(func(pref.ExtensionType) bool) {}
+func (emptyExtensionTypes) Range(func(pref.ExtensionType) bool) { return }
diff --git a/reflect/protoreflect/value.go b/reflect/protoreflect/value.go
index ed77d93..832f925 100644
--- a/reflect/protoreflect/value.go
+++ b/reflect/protoreflect/value.go
@@ -88,9 +88,8 @@
// For example, if Mutable is called later, can it reuse memory?
// Clear clears the field such that a subsequent call to Has reports false.
- //
- // It panics if the field number does not correspond with a known field
- // in MessageDescriptor.Fields or an extension field in ExtensionTypes.
+ // The operation does nothing if the field number does not correspond with
+ // a known field or extension field.
Clear(FieldNumber)
// Mutable returns a reference value for a field with a given field number,
@@ -196,6 +195,7 @@
// Remove removes the ExtensionType.
// It panics if a value for this extension field is still populated.
+ // The operation does nothing if there is no associated type to remove.
Remove(ExtensionType)
// ByNumber looks up an extension by field number.
@@ -283,6 +283,7 @@
Set(MapKey, Value)
// Clear clears the entry associated with they given key.
+ // The operation does nothing if there is no entry associated with the key.
Clear(MapKey)
// Mutable returns a Mutable reference for the element with a given key,
