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.

Author: dsnet

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) {

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)},

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.

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)

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)

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

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