From 9fd8f2ca1e234635f8bccd1a277e2449fd45f698 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Thu, 11 Apr 2024 17:16:40 -0700 Subject: [PATCH] Adjust single-line formatting behavior Changes made: * Further separate the behavior of SpaceAfterColon and SpaceAfterComma from Multiline. That is, use of Multiline does not affect the behavior of SpaceAfterColon or SpaceAfterComma unless it has not been set. For example, specifying (SpaceAfterColon(false), Multiline(true)) should avoid the space after the colon even if we expect this combination of options to be seldom used. * Exclude whitespace formatting flags from DefaultOptionsV1 since v1 has API support for both single-line and multi-line output, so setting (or clearing) them cannot be classified as v1 behavior. Similarly, exclude whitespace formatting flags from DefaultOptionsV2. * Simplify the TestMarshal cases. * Add explicit TestEncoder cases to exercise all Encoder.WriteValue code paths. --- arshal_default.go | 5 +- arshal_test.go | 90 ++++--------------------------- internal/jsonflags/flags.go | 5 ++ internal/jsonopts/options.go | 16 +----- internal/jsonopts/options_test.go | 18 +++++-- jsontext/encode.go | 38 +++++++------ jsontext/encode_test.go | 30 +++++++++++ jsontext/options.go | 56 +++++++++---------- jsontext/value.go | 3 +- options.go | 7 ++- 10 files changed, 114 insertions(+), 154 deletions(-) diff --git a/arshal_default.go b/arshal_default.go index 4bbcd05..30abd36 100644 --- a/arshal_default.go +++ b/arshal_default.go @@ -975,10 +975,9 @@ func makeStructArshaler(t reflect.Type) *arshaler { // Append any delimiters or optional whitespace. b := xe.Buf if xe.Tokens.Last.Length() > 0 { + b = append(b, ',') if xe.Flags.Get(jsonflags.SpaceAfterComma) { - b = append(b, ',', ' ') - } else { - b = append(b, ',') + b = append(b, ' ') } } if xe.Flags.Get(jsonflags.Multiline) { diff --git a/arshal_test.go b/arshal_test.go index f189e61..5ee6481 100644 --- a/arshal_test.go +++ b/arshal_test.go @@ -1233,89 +1233,19 @@ func TestMarshal(t *testing.T) { }`, }, { name: jsontest.Name("Structs/SpaceAfterColonAndComma"), - opts: []Options{ - jsontext.SpaceAfterColon(true), - jsontext.SpaceAfterComma(true), - }, - in: structAll{ - Bool: true, - String: "hello", - Bytes: []byte{1, 2, 3}, - Int: -64, - Uint: +64, - Float: 3.14159, - Map: map[string]string{"key": "value"}, - StructScalars: structScalars{ - Bool: true, - String: "hello", - Bytes: []byte{1, 2, 3}, - Int: -64, - Uint: +64, - Float: 3.14159, - }, - StructMaps: structMaps{ - MapBool: map[string]bool{"": true}, - MapString: map[string]string{"": "hello"}, - MapBytes: map[string][]byte{"": {1, 2, 3}}, - MapInt: map[string]int64{"": -64}, - MapUint: map[string]uint64{"": +64}, - MapFloat: map[string]float64{"": 3.14159}, - }, - StructSlices: structSlices{ - SliceBool: []bool{true}, - SliceString: []string{"hello"}, - SliceBytes: [][]byte{{1, 2, 3}}, - SliceInt: []int64{-64}, - SliceUint: []uint64{+64}, - SliceFloat: []float64{3.14159}, - }, - Slice: []string{"fizz", "buzz"}, - Array: [1]string{"goodbye"}, - Pointer: new(structAll), - Interface: (*structAll)(nil), - }, - want: `{"Bool": true, "String": "hello", "Bytes": "AQID", "Int": -64, "Uint": 64, "Float": 3.14159, "Map": {"key": "value"}, "StructScalars": {"Bool": true, "String": "hello", "Bytes": "AQID", "Int": -64, "Uint": 64, "Float": 3.14159}, "StructMaps": {"MapBool": {"": true}, "MapString": {"": "hello"}, "MapBytes": {"": "AQID"}, "MapInt": {"": -64}, "MapUint": {"": 64}, "MapFloat": {"": 3.14159}}, "StructSlices": {"SliceBool": [true], "SliceString": ["hello"], "SliceBytes": ["AQID"], "SliceInt": [-64], "SliceUint": [64], "SliceFloat": [3.14159]}, "Slice": ["fizz", "buzz"], "Array": ["goodbye"], "Pointer": {"Bool": false, "String": "", "Bytes": "", "Int": 0, "Uint": 0, "Float": 0, "Map": {}, "StructScalars": {"Bool": false, "String": "", "Bytes": "", "Int": 0, "Uint": 0, "Float": 0}, "StructMaps": {"MapBool": {}, "MapString": {}, "MapBytes": {}, "MapInt": {}, "MapUint": {}, "MapFloat": {}}, "StructSlices": {"SliceBool": [], "SliceString": [], "SliceBytes": [], "SliceInt": [], "SliceUint": [], "SliceFloat": []}, "Slice": [], "Array": [""], "Pointer": null, "Interface": null}, "Interface": null}`, + opts: []Options{jsontext.SpaceAfterColon(true), jsontext.SpaceAfterComma(true)}, + in: structOmitZeroAll{Int: 1, Uint: 1}, + want: `{"Int": 1, "Uint": 1}`, + }, { + name: jsontest.Name("Structs/SpaceAfterColon"), + opts: []Options{jsontext.SpaceAfterColon(true)}, + in: structOmitZeroAll{Int: 1, Uint: 1}, + want: `{"Int": 1,"Uint": 1}`, }, { name: jsontest.Name("Structs/SpaceAfterComma"), opts: []Options{jsontext.SpaceAfterComma(true)}, - in: structAll{ - Bool: true, - String: "hello", - Bytes: []byte{1, 2, 3}, - Int: -64, - Uint: +64, - Float: 3.14159, - Map: map[string]string{"key": "value"}, - StructScalars: structScalars{ - Bool: true, - String: "hello", - Bytes: []byte{1, 2, 3}, - Int: -64, - Uint: +64, - Float: 3.14159, - }, - StructMaps: structMaps{ - MapBool: map[string]bool{"": true}, - MapString: map[string]string{"": "hello"}, - MapBytes: map[string][]byte{"": {1, 2, 3}}, - MapInt: map[string]int64{"": -64}, - MapUint: map[string]uint64{"": +64}, - MapFloat: map[string]float64{"": 3.14159}, - }, - StructSlices: structSlices{ - SliceBool: []bool{true}, - SliceString: []string{"hello"}, - SliceBytes: [][]byte{{1, 2, 3}}, - SliceInt: []int64{-64}, - SliceUint: []uint64{+64}, - SliceFloat: []float64{3.14159}, - }, - Slice: []string{"fizz", "buzz"}, - Array: [1]string{"goodbye"}, - Pointer: new(structAll), - Interface: (*structAll)(nil), - }, - want: `{"Bool":true, "String":"hello", "Bytes":"AQID", "Int":-64, "Uint":64, "Float":3.14159, "Map":{"key":"value"}, "StructScalars":{"Bool":true, "String":"hello", "Bytes":"AQID", "Int":-64, "Uint":64, "Float":3.14159}, "StructMaps":{"MapBool":{"":true}, "MapString":{"":"hello"}, "MapBytes":{"":"AQID"}, "MapInt":{"":-64}, "MapUint":{"":64}, "MapFloat":{"":3.14159}}, "StructSlices":{"SliceBool":[true], "SliceString":["hello"], "SliceBytes":["AQID"], "SliceInt":[-64], "SliceUint":[64], "SliceFloat":[3.14159]}, "Slice":["fizz", "buzz"], "Array":["goodbye"], "Pointer":{"Bool":false, "String":"", "Bytes":"", "Int":0, "Uint":0, "Float":0, "Map":{}, "StructScalars":{"Bool":false, "String":"", "Bytes":"", "Int":0, "Uint":0, "Float":0}, "StructMaps":{"MapBool":{}, "MapString":{}, "MapBytes":{}, "MapInt":{}, "MapUint":{}, "MapFloat":{}}, "StructSlices":{"SliceBool":[], "SliceString":[], "SliceBytes":[], "SliceInt":[], "SliceUint":[], "SliceFloat":[]}, "Slice":[], "Array":[""], "Pointer":null, "Interface":null}, "Interface":null}`, + in: structOmitZeroAll{Int: 1, Uint: 1, Slice: []string{"a", "b"}}, + want: `{"Int":1, "Uint":1, "Slice":["a", "b"]}`, }, { name: jsontest.Name("Structs/Stringified"), opts: []Options{jsontext.Multiline(true)}, diff --git a/internal/jsonflags/flags.go b/internal/jsonflags/flags.go index 8ed0bfa..119bccf 100644 --- a/internal/jsonflags/flags.go +++ b/internal/jsonflags/flags.go @@ -68,6 +68,11 @@ const ( // AnyWhitespace reports whether the encoded output might have any whitespace. AnyWhitespace = Multiline | SpaceAfterColon | SpaceAfterComma + + // WhitespaceFlags is the set of flags related to whitespace formatting. + // In contrast to AnyWhitespace, this includes Indent and IndentPrefix + // as those settings take no effect if Multiline is false. + WhitespaceFlags = AnyWhitespace | Indent | IndentPrefix ) // Encoder and decoder flags. diff --git a/internal/jsonopts/options.go b/internal/jsonopts/options.go index aee297f..c23c280 100644 --- a/internal/jsonopts/options.go +++ b/internal/jsonopts/options.go @@ -46,19 +46,17 @@ type ArshalValues struct { // DefaultOptionsV2 is the set of all options that define default v2 behavior. var DefaultOptionsV2 = Struct{ Flags: jsonflags.Flags{ - Presence: uint64(jsonflags.AllFlags), + Presence: uint64(jsonflags.AllFlags & ^jsonflags.WhitespaceFlags), Values: uint64(0), }, - CoderValues: CoderValues{Indent: "\t"}, // Indent is set, but Multiline is set to false } // DefaultOptionsV1 is the set of all options that define default v1 behavior. var DefaultOptionsV1 = Struct{ Flags: jsonflags.Flags{ - Presence: uint64(jsonflags.AllFlags), + Presence: uint64(jsonflags.AllFlags & ^jsonflags.WhitespaceFlags), Values: uint64(jsonflags.DefaultV1Flags), }, - CoderValues: CoderValues{Indent: "\t"}, // Indent is set, but Multiline is set to false } // CopyCoderOptions copies coder-specific options from src to dst. @@ -130,22 +128,12 @@ func (dst *Struct) Join(srcs ...Options) { case nil: continue case jsonflags.Bools: - switch src { - case jsonflags.Multiline | 1: - dst.Flags.Clear(jsonflags.SpaceAfterComma | jsonflags.SpaceAfterColon) - case jsonflags.SpaceAfterComma | 1, jsonflags.SpaceAfterColon | 1: - if dst.Flags.Get(jsonflags.Multiline) { - continue - } - } dst.Flags.Set(src) case Indent: dst.Flags.Set(jsonflags.Multiline | jsonflags.Indent | 1) - dst.Flags.Clear(jsonflags.SpaceAfterComma | jsonflags.SpaceAfterColon) dst.Indent = string(src) case IndentPrefix: dst.Flags.Set(jsonflags.Multiline | jsonflags.IndentPrefix | 1) - dst.Flags.Clear(jsonflags.SpaceAfterComma | jsonflags.SpaceAfterColon) dst.IndentPrefix = string(src) case ByteLimit: dst.Flags.Set(jsonflags.ByteLimit | 1) diff --git a/internal/jsonopts/options_test.go b/internal/jsonopts/options_test.go index acebb90..0a70039 100644 --- a/internal/jsonopts/options_test.go +++ b/internal/jsonopts/options_test.go @@ -69,10 +69,22 @@ func TestJoin(t *testing.T) { CoderValues: CoderValues{Indent: "\t"}, }, }, { - in: &DefaultOptionsV1, want: &DefaultOptionsV1, // v1 fully replaces before + in: &DefaultOptionsV1, want: func() *Struct { + v1 := DefaultOptionsV1 + v1.Flags.Set(jsonflags.Indent | 1) + v1.Flags.Set(jsonflags.Multiline | 0) + v1.Indent = "\t" + return &v1 + }(), // v1 fully replaces before (except for whitespace related flags) }, { - in: &DefaultOptionsV2, want: &DefaultOptionsV2}, // v2 fully replaces before - } + in: &DefaultOptionsV2, want: func() *Struct { + v2 := DefaultOptionsV2 + v2.Flags.Set(jsonflags.Indent | 1) + v2.Flags.Set(jsonflags.Multiline | 0) + v2.Indent = "\t" + return &v2 + }(), // v2 fully replaces before (except for whitespace related flags) + }} got := new(Struct) for i, tt := range tests { got.Join(tt.in) diff --git a/jsontext/encode.go b/jsontext/encode.go index b339d55..db22dac 100644 --- a/jsontext/encode.go +++ b/jsontext/encode.go @@ -116,8 +116,17 @@ func (e *encoderState) reset(b []byte, w io.Writer, opts ...Options) { } e.Struct = jsonopts.Struct{} e.Struct.Join(opts...) - if e.Flags.Get(jsonflags.Multiline) && !e.Flags.Has(jsonflags.Indent) { - e.Indent = "\t" + if e.Flags.Get(jsonflags.Multiline) { + if !e.Flags.Has(jsonflags.SpaceAfterColon) { + e.Flags.Set(jsonflags.SpaceAfterColon | 1) + } + if !e.Flags.Has(jsonflags.SpaceAfterComma) { + e.Flags.Set(jsonflags.SpaceAfterComma | 0) + } + if !e.Flags.Has(jsonflags.Indent) { + e.Flags.Set(jsonflags.Indent | 1) + e.Indent = "\t" + } } } @@ -274,7 +283,6 @@ func (e *encoderState) UnwriteEmptyObjectMember(prevName *string) bool { b = b[:len(b)-n] b = jsonwire.TrimSuffixWhitespace(b) b = jsonwire.TrimSuffixByte(b, ':') - b = jsonwire.TrimSuffixWhitespace(b) b = jsonwire.TrimSuffixString(b) b = jsonwire.TrimSuffixWhitespace(b) b = jsonwire.TrimSuffixByte(b, ',') @@ -582,15 +590,15 @@ func (e *encoderState) WriteValue(v Value) error { // appendWhitespace appends whitespace that immediately precedes the next token. func (e *encoderState) appendWhitespace(b []byte, next Kind) []byte { if delim := e.Tokens.needDelim(next); delim == ':' { - if e.Flags.Get(jsonflags.Multiline | jsonflags.SpaceAfterColon) { - return append(b, ' ') + if e.Flags.Get(jsonflags.SpaceAfterColon) { + b = append(b, ' ') } } else { - if e.Flags.Get(jsonflags.Multiline) { - return e.AppendIndent(b, e.Tokens.NeedIndent(next)) - } if delim == ',' && e.Flags.Get(jsonflags.SpaceAfterComma) { - return append(b, ' ') + b = append(b, ' ') + } + if e.Flags.Get(jsonflags.Multiline) { + b = e.AppendIndent(b, e.Tokens.NeedIndent(next)) } } return b @@ -726,7 +734,7 @@ func (e *encoderState) reformatObject(dst []byte, src Value, depth int) ([]byte, } dst = append(dst, ':') n += len(":") - if e.Flags.Get(jsonflags.Multiline | jsonflags.SpaceAfterColon) { + if e.Flags.Get(jsonflags.SpaceAfterColon) { dst = append(dst, ' ') } @@ -748,10 +756,9 @@ func (e *encoderState) reformatObject(dst []byte, src Value, depth int) ([]byte, } switch src[n] { case ',': + dst = append(dst, ',') if e.Flags.Get(jsonflags.SpaceAfterComma) { - dst = append(dst, ',', ' ') - } else { - dst = append(dst, ',') + dst = append(dst, ' ') } n += len(",") continue @@ -819,10 +826,9 @@ func (e *encoderState) reformatArray(dst []byte, src Value, depth int) ([]byte, } switch src[n] { case ',': + dst = append(dst, ',') if e.Flags.Get(jsonflags.SpaceAfterComma) { - dst = append(dst, ',', ' ') - } else { - dst = append(dst, ',') + dst = append(dst, ' ') } n += len(",") continue diff --git a/jsontext/encode_test.go b/jsontext/encode_test.go index 73f1391..376176a 100644 --- a/jsontext/encode_test.go +++ b/jsontext/encode_test.go @@ -394,6 +394,36 @@ var encoderErrorTestdata = []struct { {ArrayEnd, nil, ""}, }, wantOut: "[]\n", +}, { + name: jsontest.Name("Format/Object/SpaceAfterColon"), + opts: []Options{SpaceAfterColon(true)}, + calls: []encoderMethodCall{{Value(`{"fizz":"buzz","wizz":"wuzz"}`), nil, ""}}, + wantOut: "{\"fizz\": \"buzz\",\"wizz\": \"wuzz\"}\n", +}, { + name: jsontest.Name("Format/Object/SpaceAfterComma"), + opts: []Options{SpaceAfterComma(true)}, + calls: []encoderMethodCall{{Value(`{"fizz":"buzz","wizz":"wuzz"}`), nil, ""}}, + wantOut: "{\"fizz\":\"buzz\", \"wizz\":\"wuzz\"}\n", +}, { + name: jsontest.Name("Format/Object/SpaceAfterColonAndComma"), + opts: []Options{SpaceAfterColon(true), SpaceAfterComma(true)}, + calls: []encoderMethodCall{{Value(`{"fizz":"buzz","wizz":"wuzz"}`), nil, ""}}, + wantOut: "{\"fizz\": \"buzz\", \"wizz\": \"wuzz\"}\n", +}, { + name: jsontest.Name("Format/Object/NoSpaceAfterColon+SpaceAfterComma+Multiline"), + opts: []Options{SpaceAfterColon(false), SpaceAfterComma(true), Multiline(true)}, + calls: []encoderMethodCall{{Value(`{"fizz":"buzz","wizz":"wuzz"}`), nil, ""}}, + wantOut: "{\n\t\"fizz\":\"buzz\", \n\t\"wizz\":\"wuzz\"\n}\n", +}, { + name: jsontest.Name("Format/Array/SpaceAfterComma"), + opts: []Options{SpaceAfterComma(true)}, + calls: []encoderMethodCall{{Value(`["fizz","buzz"]`), nil, ""}}, + wantOut: "[\"fizz\", \"buzz\"]\n", +}, { + name: jsontest.Name("Format/Array/NoSpaceAfterComma+Multiline"), + opts: []Options{SpaceAfterComma(false), Multiline(true)}, + calls: []encoderMethodCall{{Value(`["fizz","buzz"]`), nil, ""}}, + wantOut: "[\n\t\"fizz\",\n\t\"buzz\"\n]\n", }} // TestEncoderErrors test that Encoder errors occur when we expect and diff --git a/jsontext/options.go b/jsontext/options.go index 78ac6d5..582b91d 100644 --- a/jsontext/options.go +++ b/jsontext/options.go @@ -78,54 +78,50 @@ func EscapeForJS(v bool) Options { } } -// Multiline specifies that the JSON output should expand to multiple lines, -// where every JSON object member or JSON array element -// appears on a new, indented line according to the nesting depth. -// If an indent is not already specified, then it defaults to using "\t". -// -// Multiline will override SpaceAfterColon and SpaceAfterComma. -// -// If set to false, then the output is compact, -// where no whitespace is emitted between JSON values. +// SpaceAfterColon specifies that the JSON output should emit a space character +// after each colon separator following a JSON object name. +// If false, then no space character appears after the colon separator. // // This only affects encoding and is ignored when decoding. -func Multiline(v bool) Options { +func SpaceAfterColon(v bool) Options { if v { - return jsonflags.Multiline | 1 + return jsonflags.SpaceAfterColon | 1 } else { - return jsonflags.Multiline | 0 + return jsonflags.SpaceAfterColon | 0 } } -// SpaceAfterColon specifies that the JSON output should emit single-line output -// where each key has a space after the colon. -// -// If set to false, then the output is compact with no white space after the key and colon. -// -// This option is overriden by Multiline, WithIndent, and WithIndentPrefix. +// SpaceAfterComma specifies that the JSON output should emit a space character +// after each comma separator following a JSON object value or array element. +// If false, then no space character appears after the comma separator. // // This only affects encoding and is ignored when decoding. -func SpaceAfterColon(v bool) Options { +func SpaceAfterComma(v bool) Options { if v { - return jsonflags.SpaceAfterColon | 1 + return jsonflags.SpaceAfterComma | 1 } else { - return jsonflags.SpaceAfterColon | 0 + return jsonflags.SpaceAfterComma | 0 } } -// SpaceAfterComma specifies that the JSON output should emit single-line output -// where each non-final element has a space after the comma. +// Multiline specifies that the JSON output should expand to multiple lines, +// where every JSON object member or JSON array element appears on +// a new, indented line according to the nesting depth. // -// If set to false, then the output is compact with no white space after the element and comma. +// If [SpaceAfterColon] is not specified, then the default is true. +// If [SpaceAfterComma] is not specified, then the default is false. +// If [WithIndent] is not specified, then the default is "\t". // -// This option is overriden by Multiline, WithIndent, and WithIndentPrefix. +// If set to false, then the output is a single-line, +// where the only whitespace emitted is determined by the current +// values of [SpaceAfterColon] and [SpaceAfterComma]. // // This only affects encoding and is ignored when decoding. -func SpaceAfterComma(v bool) Options { +func Multiline(v bool) Options { if v { - return jsonflags.SpaceAfterComma | 1 + return jsonflags.Multiline | 1 } else { - return jsonflags.SpaceAfterComma | 0 + return jsonflags.Multiline | 0 } } @@ -135,8 +131,6 @@ func SpaceAfterComma(v bool) Options { // followed by one or more copies of indent according to the nesting depth. // The indent must only be composed of space or tab characters. // -// WithIndent will override SpaceAfterColon and SpaceAfterComma. -// // If the intent to emit indented output without a preference for // the particular indent string, then use [Multiline] instead. // @@ -173,8 +167,6 @@ func WithIndent(indent string) Options { // (see [WithIndent]) according to the nesting depth. // The prefix must only be composed of space or tab characters. // -// WithIndentPrefix will override SpaceAfterColon and SpaceAfterComma. -// // This only affects encoding and is ignored when decoding. // Use of this option implies [Multiline] being set to true. func WithIndentPrefix(prefix string) Options { diff --git a/jsontext/value.go b/jsontext/value.go index d9b5c34..fd82b6a 100644 --- a/jsontext/value.go +++ b/jsontext/value.go @@ -143,8 +143,6 @@ func (v *Value) reformat(canonical, multiline bool, prefix, indent string) error e := getBufferedEncoder() defer putBufferedEncoder(e) eo := &e.s.Struct - eo.Flags.Set(jsonflags.SpaceAfterColon | 0) - eo.Flags.Set(jsonflags.SpaceAfterComma | 0) if canonical { eo.Flags.Set(jsonflags.AllowInvalidUTF8 | 0) // per RFC 8785, section 3.2.4 eo.Flags.Set(jsonflags.AllowDuplicateNames | 0) // per RFC 8785, section 3.1 @@ -165,6 +163,7 @@ func (v *Value) reformat(canonical, multiline bool, prefix, indent string) error eo.Flags.Set(jsonflags.PreserveRawStrings | 1) if multiline { eo.Flags.Set(jsonflags.Multiline | 1) + eo.Flags.Set(jsonflags.SpaceAfterColon | 1) eo.Flags.Set(jsonflags.Indent | 1) eo.Flags.Set(jsonflags.IndentPrefix | 1) eo.IndentPrefix = prefix diff --git a/options.go b/options.go index a43e31e..8ff2d68 100644 --- a/options.go +++ b/options.go @@ -96,10 +96,9 @@ func GetOption[T any](opts Options, setter func(T) Options) (T, bool) { } // DefaultOptionsV2 is the full set of all options that define v2 semantics. -// It is equivalent to all boolean options under [Options], -// [encoding/json.Options], and [encoding/json/jsontext.Options] -// being set to false. All non-boolean options are set to the zero value, -// except for [jsontext.WithIndent], which defaults to "\t". +// It is equivalent to all options under [Options], [encoding/json.Options], +// and [encoding/json/jsontext.Options] being set to false or the zero value, +// except for the options related to whitespace formatting. func DefaultOptionsV2() Options { return &jsonopts.DefaultOptionsV2 }