Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: encoding/json: add omitnil option #22480

Open
blixt opened this issue Oct 28, 2017 · 14 comments

Comments

@blixt
Copy link
Contributor

@blixt blixt commented Oct 28, 2017

Note: This proposal already has as a patch from 2015 by @bakineggs, but it appears to have fallen between the cracks.

I have the following case:

type Join struct {
    ChannelId string      `json:"channel_id"`
    Accounts  []Ident     `json:"accounts,omitempty"`
    History   []TextEntry `json:"history,omitempty"`
}

This struct is used for message passing and the slices are only relevant (and set to non-nil) in some cases. However, since encoding/json does not differentiate between a nil slice and an empty slice, there will be legitimate cases where a field is excluded when it's not expected to (e.g., the History slice is set, but empty).

I reiterate the proposal by Dan in his patch referred above to support an omitnil option which allows this differentiation for slices and maps.

Note for hypothetical Go 2.0: This is already how omitempty works for pointers to Go's basic types (e.g., (*int)(nil) is omitted while pointer to 0 is not). For Go 2.0 the behavior of omitempty could change to omit both nil and 0 when specified, and then only nil would be omitted when omitnil is specified.

@robyoder

This comment has been minimized.

Copy link

@robyoder robyoder commented Jan 4, 2018

Yes please! This bit me today. We like to use omitempty to minimize the fields we need to include in a response. But we do need to differentiate between a nil slice and an empty slice, so we had to remove omitempty for slices and always include the field, so now it's an array or null. 🙁

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Jan 22, 2018

Partial dup of #11939, which we may get to someday. Mostly JSON is done but better handling of zeros is on the long-term wish list.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Jan 22, 2018

Closing in favor of #11939, which now mentions this one.

@rsc rsc closed this Jan 22, 2018
@robyoder

This comment has been minimized.

Copy link

@robyoder robyoder commented Jan 29, 2018

@rsc how is this even a partial dupe of that issue? Its focus is defining additional zero values to be counted as "empty" by omitempty. This issue is focused on the fact that there is no way to differentiate between nil and empty slices with omitempty. How does #11939 address that?

@blixt

This comment has been minimized.

Copy link
Contributor Author

@blixt blixt commented Jan 30, 2018

@rsc That proposal you referred seems to make omitempty exclude even more values from being encoded. This proposal is for doing the opposite – keep values that are today hidden from the output!

To clarify this proposal, here's an example. I want to be able to omit the Contents property for the Black Box (as it works today), but keep it for the Empty Box:

package main

import "encoding/json"
import "fmt"

type Box struct {
	Label    string
	Contents []string `json:",omitempty"`
}

func main() {
	var d []byte
	d, _ = json.Marshal(Box{Label: "Black Box"})
	fmt.Println(string(d))
	d, _ = json.Marshal(Box{Label: "Empty Box", Contents: []string{}})
	fmt.Println(string(d))
	d, _ = json.Marshal(Box{Label: "Banana Box", Contents: []string{"Banana", "Another banana"}})
	fmt.Println(string(d))
}

Here's the output I want, after changing omitempty to omitnil:

{"Label":"Black Box"}
{"Label":"Empty Box","Contents":[]}
{"Label":"Banana Box","Contents":["Banana","Another banana"]}

Actual output:

{"Label":"Black Box"}
{"Label":"Empty Box"}
{"Label":"Banana Box","Contents":["Banana","Another banana"]}
@blixt

This comment has been minimized.

Copy link
Contributor Author

@blixt blixt commented Jan 30, 2018

Further clarification: The patch mentioned above adds support to encoding/json to differentiate on []string(nil) vs. []string{} (through a new keyword omitnil so backwards compatibility is not broken), allowing nil slices to be excluded without excluding empty slices.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 30, 2018

I guess I'll reopen this, but to be honest it sounds like a bad idea to me. Distinguishing between a nil slice and an empty slice is sufficiently confusing that it is generally a mistake. I don't think that the additional complexity in the already very complex encoding/json package is worth it.

@robyoder

This comment has been minimized.

Copy link

@robyoder robyoder commented Jan 30, 2018

@ianlancetaylor well, the encoder distinguishes. It encodes an empty slice as [] and a nil slice as null. But including a null property in the JSON is kinda pointless. Including [] is not.

@bakineggs

This comment has been minimized.

Copy link

@bakineggs bakineggs commented Jan 30, 2018

In go, you shouldn't need to distinguish between a nil slice and an empty slice because they're generally treated the same. In JSON and many languages in which people parse JSON, null arrays and empty arrays are treated differently.

Ideally, the consumers of your JSON will be able to handle output that has either a null array, an empty array, or a missing key. In the real world, customers often demand that you supply output exactly how they want it. Product managers insist that engineers do the things that customers who pay boatloads of money want, so that's why I had to write the omitnil patch and run a product I built for a previous employer on a fork of go that includes this patch (and the poor souls who inherited that product have to continue maintaining that fork). It appears that my situation was not unique and others have to deal with similar demands, so it would be helpful if they didn't also have to maintain their own forks of go.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Feb 5, 2018

On hold for #11939.

@rsc rsc added the Proposal-Hold label Feb 5, 2018
@jucardi

This comment has been minimized.

Copy link

@jucardi jucardi commented Nov 27, 2018

+1 to this request. I have a struct with a a field

Metadata interface{} `json:"metadata,omitempty,omitnil"`

but if the value is nil, it still serializes it as "metadata": null

@tariq1890

This comment has been minimized.

Copy link

@tariq1890 tariq1890 commented Dec 16, 2018

+1 to this request

@blixt

This comment has been minimized.

Copy link
Contributor Author

@blixt blixt commented Feb 21, 2019

@rsc You had mentioned a JSON sweep for 1.12, was this reviewed?

To summarize:

  • Today:
    • []int(nil) becomes "field": null by default
    • []int{} becomes "field": [] by default
    • omitempty will make both cases omit the field (which is unexpected for the empty slice case, basically making omitempty not an option because of course JSON should represent an empty array)
  • Proposed:
    • omitnil would only omit the []int(nil) case (note: not a breaking change)

And to respond to @ianlancetaylor's comment:

Distinguishing between a nil slice and an empty slice is sufficiently confusing that it is generally a mistake

If this is something you stand behind, then please let's make []int(nil) encode into [], not null! Making this change would be backwards incompatible, unfortunately.

@magiconair

This comment has been minimized.

Copy link
Contributor

@magiconair magiconair commented Oct 12, 2019

We need the omitnil behavior to minimize the data that is reported from an IoT device.

A lot of our fields are pointers to types because they are generated by another tool which uses pointers to distinguish between required and optional fields. (GraphQL)

type Battery {
    Voltage *float32 `json:"voltage"`
    Current *float32 `json:"current"`
    ... 150 additional optional fields ...
}

There is a difference between "sensor read 0V" or "sensor did not read a value at all". To limit the amount of data that is sent we want to omit all fields with nil values - not just slices.

This should be configurable on the encoder as well as the fields, e.g. with an SetOmitNil() function like you do with the indent settings.

To that extent I would disagree with @rsc that "JSON is done" since in more complex projects we don't always have full control over the types that we're using and in our case we have way too many different types to re-generate them with different tags just to satisfy the Go JSON encoder behavior.

Also, encoding behavior of the same type may change per use-case. For the IoT case we want to omit the fields to save data but for an API we may want to show all fields to make the type discoverable. jsonpb has an EmitDefaults option for that case but that only works for protobuf objects. The json tags hard-code the encoding behavior.

Additionally, it should be possible to register custom encoder/decoder functions per type for example to allow time.Time fields to render in a specific format if we have to use a type from an imported package. I can think of other examples of float formatting.

Another thing was the long discussion on whether the JSON decoder should reject/report fields for which there is no field to support config validation.

We cannot use a more efficient encoding scheme like protobuf for various reasons. AWS IoT rules are only triggered on JSON payloads is one of them but more importantly, almost all tools work with JSON data so adding different serialization schemes - even if they are more efficient - adds an extra maintenance burden.

Having a high-quality and maintained JSON encoder in the standard library is important. But so is making it flexible enough since - for better or for worse - JSON is the lingua-franca in a lot of projects. I think in that respect the Go JSON encoder falls a bit short.

What I'm currently doing is to maintain a fork of the JSON package and keep it as an internal package to support this:

var b bytes.Buffer
e := json.NewEncoder(&b)
e.SetOmitNil(true)
err := e.Encode(v)

The patch below is quite simple but I do understand that LoC is not the only measure and that every feature has to be supported forever. So there has to be a clear benefit.

diff --git a/src/encoding/json/encode.go b/src/encoding/json/encode.go
index 67412763d6..ded4ac4867 100644
--- a/src/encoding/json/encode.go
+++ b/src/encoding/json/encode.go
@@ -342,6 +342,8 @@ type encOpts struct {
 	quoted bool
 	// escapeHTML causes '<', '>', and '&' to be escaped in JSON strings.
 	escapeHTML bool
+	// omitnil causes nil fields to be skipped
+	omitnil bool
 }
 
 type encoderFunc func(e *encodeState, v reflect.Value, opts encOpts)
@@ -650,6 +652,9 @@ FieldLoop:
 			fv = fv.Field(i)
 		}
 
+		if opts.omitnil && fv.Kind() == reflect.Ptr && fv.IsNil() {
+			continue
+		}
 		if f.omitEmpty && isEmptyValue(fv) {
 			continue
 		}
diff --git a/src/encoding/json/stream.go b/src/encoding/json/stream.go
index e29127499b..8c2d08d311 100644
--- a/src/encoding/json/stream.go
+++ b/src/encoding/json/stream.go
@@ -178,6 +178,7 @@ type Encoder struct {
 	w          io.Writer
 	err        error
 	escapeHTML bool
+	omitnil    bool
 
 	indentBuf    *bytes.Buffer
 	indentPrefix string
@@ -199,7 +200,7 @@ func (enc *Encoder) Encode(v interface{}) error {
 		return enc.err
 	}
 	e := newEncodeState()
-	err := e.marshal(v, encOpts{escapeHTML: enc.escapeHTML})
+	err := e.marshal(v, encOpts{escapeHTML: enc.escapeHTML, omitnil:enc.omitnil})
 	if err != nil {
 		return err
 	}
@@ -250,6 +251,10 @@ func (enc *Encoder) SetEscapeHTML(on bool) {
 	enc.escapeHTML = on
 }
 
+func (enc *Encoder) SetOmitNil(on bool) {
+	enc.omitnil = on
+}
+
 // RawMessage is a raw encoded JSON value.
 // It implements Marshaler and Unmarshaler and can
 // be used to delay JSON decoding or precompute a JSON encoding.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.