Fix protomodule.NewMessage handling of protobuf field presence (#113)

* Fix protomodule.NewMessage handling of protobuf field presence

* revert unnecessary go bump

go/protomodule/protomodule_message.go

@@ -40,15 +40,20 @@ func NewMessage(msg proto.Message) (*protoMessage, error) {
 	// Copy any existing set fields
 	var rangeErr error
 	msgReflect.Range(func(fd protoreflect.FieldDescriptor, v protoreflect.Value) bool {
-		// TODO: Range only iterates over populated fields so isFieldSet may be redundant
-		if isFieldSet(v, fd) {
+		// Protobuf field presense is complex: https://github.com/protocolbuffers/protobuf/blob/d67f921f90e3aa03f65c3e1e507ca7017c8327a6/docs/field_presence.md
+		// There are two different manifestations of presence for protobufs: no
+		// presence, where the generated message API stores field values (only),
+		// and explicit presence, where the API also stores whether or not a
+		// field has been set.
+		// For fields with explicit presence, we trust `Range`: if it iterates over them, we keep the field.
+		// For fields with no presence, we manually double-check whether the field is equal to the default value. If so, we omit it.
+		if fd.HasPresence() || isFieldSet(v, fd) {
 			starlarkValue, err := valueToStarlark(v, fd)
 			if err != nil {
 				rangeErr = err
 				return false
 			}
 			fields[string(fd.Name())] = starlarkValue
-
 		}
 		return true
 	})

go/protomodule/protomodule_test.go

@@ -547,6 +547,21 @@ func TestProtoJson(t *testing.T) {
 			src:  `proto.decode_json(proto.package("skycfg.test_proto").MessageV3, "{\"f_int32\": 1010}").f_int32`,
 			want: "1010",
 		},
+		{
+			// This is a bit of a weird test. Protobuf's have complex behavior around whether a field is present.
+			// Reference: https://github.com/protocolbuffers/protobuf/blob/main/docs/field_presence.md
+			// This is particularly relevant for JSON encoding and decoding.
+			// This test specifically checks two things:
+			//   1. skycfg does not track presence for default scalar values (i.e., the empty field may be "not present" after a serialization round-trip)
+			//   2. skycfg _does_ track presence for `oneof` fields
+			// Without this behavior, working with "json-like" protos, e.g., google.protobuf.Value, becomes challenging.
+			name: "proto.decode_json oneof field presence",
+			src: `proto.encode_json(proto.decode_json(
+				proto.package("skycfg.test_proto").MessageV3,
+				"{\"f_string\":\"\",\"f_oneof_a\":\"\"}",
+			))`,
+			want: `"{\"f_oneof_a\":\"\"}"`,
+		},
 	})
 }