From fa894ec93ecbc84a44c5ce98c7712c670af746c8 Mon Sep 17 00:00:00 2001 From: William Poussier Date: Mon, 17 Feb 2020 23:06:32 +0100 Subject: [PATCH] feat: add the 'omitnil' struct field's option --- CHANGELOG.md | 6 ++- README.md | 78 +++++++++++++++--------------- encode.go | 9 +++- images/benchmarks/code-marshal.svg | 2 +- images/benchmarks/map.svg | 2 +- images/benchmarks/simple.svg | 2 +- instruction.go | 17 ++++--- json_test.go | 46 ++++++++++++++++++ struct.go | 22 +++++---- types.go | 8 +++ 10 files changed, 132 insertions(+), 60 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3599543..033366e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,9 @@ All notable changes to this project are documented in this file. **THIS LIBRARY IS STILL IN ALPHA AND THERE ARE NO GUARANTEES REGARDING API STABILITY YET** +## [v0.7.0] - 2020-02-17 +- Add the `omitnil` field tag's option, which specifies that a field with a nil pointer should be omitted from the encoding. This option has precedence over the `omitempty` option. See this issue for more informations about the original proposal: golang.org/issue/22480. + ## [v0.6.0] - 2020-02-14 - Add support for the `sync.Map` type. The marshaling behavior for this type is similar to the one of the Go `map`. @@ -16,7 +19,7 @@ This includes the following changes, but not limited to: - Improve the marshaling performances of many types. - Add support for marshaling `json.RawMessage` values. - Add new options `DenyList`, `NoNumberValidation`, `NoCompact`, and rename some others. -- Replace the `Marshaler` and `MarshalerCtx` interfaces by `AppendMarshaler` and `AppendMarshalerCtx` to follow the new *append* model. +- Replace the `Marshaler` and `MarshalerCtx` interfaces by `AppendMarshaler` and `AppendMarshalerCtx` to follow the new *append* model. See this issue on GitHub for more details: golang.org/issue/34701. - Remove the `IntegerBase` option, which didn't worked properly with the `string` JSON tag. > Some of the improvements have been inspired by the **github.com/segmentio/encoding** project. @@ -53,6 +56,7 @@ This includes the following changes, but not limited to: ## [v0.1.0] - 2019-08-30 Initial realease. +[v0.7.0]: https://github.com/wI2L/jettison/compare/v0.6.0...v0.7.0 [v0.6.0]: https://github.com/wI2L/jettison/compare/v0.5.0...v0.6.0 [v0.5.0]: https://github.com/wI2L/jettison/compare/v0.4.1...v0.5.0 [v0.4.1]: https://github.com/wI2L/jettison/compare/v0.4.0...v0.4.1 diff --git a/README.md b/README.md index 5339be1..9908cb9 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ Jettison uses the new [Go modules](https://github.com/golang/go/wiki/Modules). Releases are tagged according to the _SemVer_ format, prefixed with a `v`, starting from *0.2.0*. You can get the latest release using the following command. -```sh +```console $ go get github.com/wI2L/jettison ``` @@ -53,6 +53,8 @@ All notable differences with the standard library behavior are listed below. Ple - The `sync.Map` type is handled natively. The marshaling behavior is similar to the one of a standard Go `map`. The option `UnsortedMap` can also be used in cunjunction with this type to disable the default keys sort. +- The `omitnil` field tag's option can be used to specify that a field with a nil pointer should be omitted from the encoding. This option has precedence over the `omitempty` option. + ##### Bugs - Nil map keys values implementing the `encoding.TextMarshaler` interface are encoded as empty strings, while the `encoding/json` package currently panic because of that. See this [issue](https://github.com/golang/go/issues/33675) for more details.[1] @@ -128,47 +130,47 @@ OS: macOS Mojave (10.14.6) CPU: 2.6 GHz Intel Core i7 Mem: 16GB Go: go version go1.13.7 darwin/amd64 -Tag: v0.5.0 +Tag: v0.7.0 ```
Stats
 name                    time/op
-Simple/standard-8          666ns ± 4%
-Simple/jsoniter-8          770ns ± 1%
-Simple/segmentj-8          376ns ± 0%
-Simple/jettison-8          485ns ± 1%
+Simple/standard-8          662ns ± 1%
+Simple/jsoniter-8          775ns ± 1%
+Simple/segmentj-8          380ns ± 1%
+Simple/jettison-8          472ns ± 1%
 Complex/standard-8        13.8µs ± 1%
-Complex/jsoniter-8        14.1µs ± 2%
-Complex/segmentj-8        9.77µs ± 2%
-Complex/jettison-8        6.84µs ± 1%
-CodeMarshal/standard-8    7.27ms ± 1%
-CodeMarshal/jsoniter-8    8.28ms ± 1%
-CodeMarshal/segmentj-8    5.55ms ± 0%
-CodeMarshal/jettison-8    5.99ms ± 1%
-Map/standard-8            2.19µs ± 0%
-Map/jsoniter-8            1.84µs ± 1%
+Complex/jsoniter-8        14.1µs ± 1%
+Complex/segmentj-8        9.72µs ± 1%
+Complex/jettison-8        6.86µs ± 1%
+CodeMarshal/standard-8    7.25ms ± 0%
+CodeMarshal/jsoniter-8    8.27ms ± 1%
+CodeMarshal/segmentj-8    5.54ms ± 0%
+CodeMarshal/jettison-8    6.02ms ± 0%
+Map/standard-8            2.19µs ± 1%
+Map/jsoniter-8            1.83µs ± 1%
 Map/segmentj-8            1.92µs ± 0%
-Map/jettison-8             917ns ± 1%
-Map/jettison-nosort-8      607ns ± 2%
+Map/jettison-8             904ns ± 1%
+Map/jettison-nosort-8      600ns ± 1%
 
 name                    speed
-Simple/standard-8        203MB/s ± 4%
-Simple/jsoniter-8        175MB/s ± 1%
-Simple/segmentj-8        359MB/s ± 0%
-Simple/jettison-8        278MB/s ± 1%
+Simple/standard-8        204MB/s ± 1%
+Simple/jsoniter-8        174MB/s ± 1%
+Simple/segmentj-8        355MB/s ± 2%
+Simple/jettison-8        286MB/s ± 1%
 Complex/standard-8      61.8MB/s ± 1%
-Complex/jsoniter-8      58.1MB/s ± 2%
-Complex/segmentj-8      88.1MB/s ± 2%
-Complex/jettison-8       124MB/s ± 2%
-CodeMarshal/standard-8   267MB/s ± 1%
-CodeMarshal/jsoniter-8   234MB/s ± 1%
-CodeMarshal/segmentj-8   349MB/s ± 0%
-CodeMarshal/jettison-8   324MB/s ± 1%
-Map/standard-8          38.9MB/s ± 0%
-Map/jsoniter-8          46.2MB/s ± 1%
-Map/segmentj-8          44.2MB/s ± 1%
-Map/jettison-8          92.7MB/s ± 1%
-Map/jettison-nosort-8    140MB/s ± 2%
+Complex/jsoniter-8      58.1MB/s ± 1%
+Complex/segmentj-8      88.5MB/s ± 1%
+Complex/jettison-8       124MB/s ± 1%
+CodeMarshal/standard-8   268MB/s ± 0%
+CodeMarshal/jsoniter-8   235MB/s ± 1%
+CodeMarshal/segmentj-8   350MB/s ± 0%
+CodeMarshal/jettison-8   322MB/s ± 0%
+Map/standard-8          38.9MB/s ± 1%
+Map/jsoniter-8          46.4MB/s ± 1%
+Map/segmentj-8          44.3MB/s ± 0%
+Map/jettison-8          94.0MB/s ± 1%
+Map/jettison-nosort-8    142MB/s ± 1%
 
 name                    alloc/op
 Simple/standard-8           144B ± 0%
@@ -179,12 +181,12 @@ Complex/standard-8        4.76kB ± 0%
 Complex/jsoniter-8        4.65kB ± 0%
 Complex/segmentj-8        3.25kB ± 0%
 Complex/jettison-8        1.38kB ± 0%
-CodeMarshal/standard-8    1.96MB ± 1%
-CodeMarshal/jsoniter-8    1.99MB ± 5%
-CodeMarshal/segmentj-8    1.98MB ± 2%
-CodeMarshal/jettison-8    1.98MB ± 2%
+CodeMarshal/standard-8    1.95MB ± 1%
+CodeMarshal/jsoniter-8    1.99MB ± 2%
+CodeMarshal/segmentj-8    1.97MB ± 2%
+CodeMarshal/jettison-8    1.97MB ± 2%
 Map/standard-8              848B ± 0%
-Map/jsoniter-8              924B ± 0%
+Map/jsoniter-8              925B ± 0%
 Map/segmentj-8              592B ± 0%
 Map/jettison-8             96.0B ± 0%
 Map/jettison-nosort-8       160B ± 0%
diff --git a/encode.go b/encode.go
index e8c6060..3b6dacd 100644
--- a/encode.go
+++ b/encode.go
@@ -208,7 +208,7 @@ fieldLoop:
 		// Find the nested struct field by following
 		// the offset sequence, indirecting encountered
 		// pointers as needed.
-		for _, s := range f.embedSeqs {
+		for _, s := range f.embedSeq {
 			v = unsafe.Pointer(uintptr(v) + s.offset)
 			if s.indir {
 				if v = *(*unsafe.Pointer)(v); v == nil {
@@ -219,8 +219,13 @@ fieldLoop:
 				}
 			}
 		}
+		// Ignore the field if it is a nil pointer and has
+		// the omitnil option in his tag.
+		if f.omitNil && *(*unsafe.Pointer)(v) == nil {
+			continue
+		}
 		// Ignore the field if it represents the zero-value
-		// of its type and has the omitempty option in its tag.
+		// of its type and has the omitempty option in his tag.
 		// Empty func is non-nil only if the field has the
 		// omitempty option in its tag.
 		if f.omitEmpty && f.empty(v) {
diff --git a/images/benchmarks/code-marshal.svg b/images/benchmarks/code-marshal.svg
index 06f57d6..767a094 100644
--- a/images/benchmarks/code-marshal.svg
+++ b/images/benchmarks/code-marshal.svg
@@ -1 +1 @@
-ns/opB/opjettisonjsonitersegmentjstandard025000005000000750000010000000
\ No newline at end of file
+ns/opB/opjettisonjsonitersegmentjstandard025000005000000750000010000000
\ No newline at end of file
diff --git a/images/benchmarks/map.svg b/images/benchmarks/map.svg
index 7224a47..5ea4e97 100644
--- a/images/benchmarks/map.svg
+++ b/images/benchmarks/map.svg
@@ -1 +1 @@
-ns/opB/opjettisonjettison-nosortjsonitersegmentjstandard0600120018002400
\ No newline at end of file
+ns/opB/opjettisonjettison-nosortjsonitersegmentjstandard0600120018002400
\ No newline at end of file
diff --git a/images/benchmarks/simple.svg b/images/benchmarks/simple.svg
index 4e67e22..f48afd2 100644
--- a/images/benchmarks/simple.svg
+++ b/images/benchmarks/simple.svg
@@ -1 +1 @@
-ns/opB/opjettisonjsonitersegmentjstandard02505007501000
\ No newline at end of file
+ns/opB/opjettisonjsonitersegmentjstandard02505007501000
\ No newline at end of file
diff --git a/instruction.go b/instruction.go
index dc1d1c4..2263205 100644
--- a/instruction.go
+++ b/instruction.go
@@ -285,7 +285,7 @@ func newStructInstr(t reflect.Type, canAddr bool) instruction {
 
 func newStructFieldsInstr(t reflect.Type, canAddr bool) instruction {
 	if t.NumField() == 0 {
-		// fast instruction for empty struct.
+		// Fast path for empty struct.
 		return func(_ unsafe.Pointer, dst []byte, opts encOpts) ([]byte, error) {
 			return append(dst, "{}"...), nil
 		}
@@ -298,9 +298,15 @@ func newStructFieldsInstr(t reflect.Type, canAddr bool) instruction {
 		f := &dupl[i]
 		ftyp := typeByIndex(t, f.index)
 		etyp := ftyp
+
 		if etyp.Kind() == reflect.Ptr {
 			etyp = etyp.Elem()
 		}
+		if !isNilable(ftyp) {
+			// Disable the omitnil option, to
+			// eliminate a check at runtime.
+			f.omitNil = false
+		}
 		// Generate instruction and empty func of the field.
 		// Only strings, floats, integers, and booleans
 		// types can be quoted.
@@ -401,11 +407,10 @@ func wrapQuotedInstr(ins instruction) instruction {
 	return func(p unsafe.Pointer, dst []byte, opts encOpts) ([]byte, error) {
 		dst = append(dst, '"')
 		var err error
-		if dst, err = ins(p, dst, opts); err != nil {
-			return dst, err
+		dst, err = ins(p, dst, opts)
+		if err == nil {
+			dst = append(dst, '"')
 		}
-		dst = append(dst, '"')
-
-		return dst, nil
+		return dst, err
 	}
 }
diff --git a/json_test.go b/json_test.go
index 4007326..5b38a55 100644
--- a/json_test.go
+++ b/json_test.go
@@ -1117,6 +1117,52 @@ func TestStructFieldOmitempty(t *testing.T) {
 	marshalCompare(t, xx, "")
 }
 
+// TestStructFieldOmitnil tests that the fields of a
+// struct with the omitnil option are not encoded
+// when they have a nil value.
+func TestStructFieldOmitnil(t *testing.T) {
+	// nolint:staticcheck
+	type x struct {
+		Sn  string                 `json:"sn,omitnil"`
+		In  int                    `json:"in,omitnil"`
+		Un  uint                   `json:"un,omitnil"`
+		Fn  float64                `json:"fn,omitnil"`
+		Bn  bool                   `json:"bn,omitnil"`
+		Sln []string               `json:"sln,omitnil"`
+		Mpn map[string]interface{} `json:"mpn,omitnil"`
+		Stn struct{}               `json:"stn,omitnil"`
+		Ptn *string                `json:"ptn,omitnil"`
+		Ifn interface{}            `json:"ifn,omitnil"`
+	}
+	var (
+		xx     = x{}
+		before = `{"sn":"","in":0,"un":0,"fn":0,"bn":false,"stn":{}}`
+		after  = `{"sn":"","in":0,"un":0,"fn":0,"bn":false,"sln":[],"mpn":{},"stn":{},"ptn":"Loreum","ifn":42}`
+	)
+	b, err := Marshal(xx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := string(b); got != before {
+		t.Errorf("before: got: %#q, want: %#q", got, before)
+	}
+	s := "Loreum"
+
+	xx.Sln = make([]string, 0)
+	xx.Mpn = map[string]interface{}{}
+	xx.Stn = struct{}{}
+	xx.Ptn = &s
+	xx.Ifn = 42
+
+	b, err = Marshal(xx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := string(b); got != after {
+		t.Errorf("after: got: %#q, want: %#q", got, after)
+	}
+}
+
 // TestQuotedStructFields tests that the fields of
 // a struct with the string option are quoted during
 // marshaling if the type support it.
diff --git a/struct.go b/struct.go
index 86284ac..cf5f717 100644
--- a/struct.go
+++ b/struct.go
@@ -28,13 +28,14 @@ type field struct {
 	tag        bool
 	quoted     bool
 	omitEmpty  bool
+	omitNil    bool
 	instr      instruction
 	empty      emptyFunc
 
-	// embedSeqs represents the sequence of offsets
+	// embedSeq represents the sequence of offsets
 	// and indirections to follow to reach the field
 	// through one or more anonymous fields.
-	embedSeqs []seq
+	embedSeq []seq
 }
 
 type typeCount map[reflect.Type]int
@@ -89,7 +90,7 @@ func structFields(t reflect.Type) []field {
 				continue
 			}
 			seen[f.typ] = true
-			// Scan the type for fields to encoded.
+			// Scan the type for fields to encode.
 			flds, next = scanFields(f, flds, next, ccnt, ncnt)
 		}
 	}
@@ -274,13 +275,14 @@ func scanFields(f field, fields, next []field, cnt, ncnt typeCount) ([]field, []
 				tag:        tagged,
 				index:      index,
 				omitEmpty:  opts.Contains("omitempty"),
+				omitNil:    opts.Contains("omitnil"),
 				quoted:     opts.Contains("string") && isBasicType(typ),
 				keyNonEsc:  []byte(`"` + name + `":`),
-				keyEscHTML: append([]byte(nil), escBuf.Bytes()...),    // copy
-				embedSeqs:  append(f.embedSeqs[:0:0], f.embedSeqs...), // clone
+				keyEscHTML: append([]byte(nil), escBuf.Bytes()...),  // copy
+				embedSeq:   append(f.embedSeq[:0:0], f.embedSeq...), // clone
 			}
 			// Add final offset to sequences.
-			nf.embedSeqs = append(nf.embedSeqs, seq{sf.Offset, false})
+			nf.embedSeq = append(nf.embedSeq, seq{sf.Offset, false})
 			fields = append(fields, nf)
 
 			if cnt[f.typ] > 1 {
@@ -298,10 +300,10 @@ func scanFields(f field, fields, next []field, cnt, ncnt typeCount) ([]field, []
 		ncnt[typ]++
 		if ncnt[typ] == 1 {
 			next = append(next, field{
-				typ:       typ,
-				name:      typ.Name(),
-				index:     index,
-				embedSeqs: append(f.embedSeqs, seq{sf.Offset, isPtr}),
+				typ:      typ,
+				name:     typ.Name(),
+				index:    index,
+				embedSeq: append(f.embedSeq, seq{sf.Offset, isPtr}),
 			})
 		}
 	}
diff --git a/types.go b/types.go
index 03c5820..b4aaab4 100644
--- a/types.go
+++ b/types.go
@@ -77,6 +77,14 @@ func isInlined(t reflect.Type) bool {
 	}
 }
 
+func isNilable(t reflect.Type) bool {
+	switch t.Kind() {
+	case reflect.Ptr, reflect.Interface, reflect.Slice, reflect.Map:
+		return true
+	}
+	return false
+}
+
 // cachedEmptyFuncOf is similar to emptyFuncOf, but
 // returns a cached function, to avoid duplicates.
 func cachedEmptyFuncOf(t reflect.Type) emptyFunc {