json: skip nil in UnmarshalJSON and (for symmetry) MarshalJSON

R=dsymonds, r
CC=golang-dev
https://golang.org/cl/5050049

3 files changed, 42 insertions, 13 deletions

diff --git a/src/pkg/json/decode.go b/src/pkg/json/decode.go
index 268747dc60..e0cc408cbf 100644
--- a/src/pkg/json/decode.go
+++ b/src/pkg/json/decode.go
@@ -22,16 +22,11 @@ import (
 // Unmarshal parses the JSON-encoded data and stores the result
 // in the value pointed to by v.
 //
-// Unmarshal traverses the value v recursively.
-// If an encountered value implements the Unmarshaler interface,
-// Unmarshal calls its UnmarshalJSON method with a well-formed
-// JSON encoding.
-//
-// Otherwise, Unmarshal uses the inverse of the encodings that
+// Unmarshal uses the inverse of the encodings that
 // Marshal uses, allocating maps, slices, and pointers as necessary,
 // with the following additional rules:
 //
-// To unmarshal a JSON value into a nil interface value, the
+// To unmarshal JSON into a nil interface value, the
 // type stored in the interface value is one of:
 //
 //	bool, for JSON booleans
@@ -41,6 +36,12 @@ import (
 //	map[string]interface{}, for JSON objects
 //	nil for JSON null
 //
+// To unmarshal JSON into a pointer, Unmarshal first handles the case of
+// the JSON being the JSON literal null.  In that case, Unmarshal sets
+// the pointer to nil.  Otherwise, Unmarshal unmarshals the JSON into
+// the value pointed at by the pointer.  If the pointer is nil, Unmarshal
+// allocates a new value for it to point to.
+//
 // If a JSON value is not appropriate for a given target type,
 // or if a JSON number overflows the target type, Unmarshal
 // skips that field and completes the unmarshalling as best it can.
@@ -250,8 +251,8 @@ func (d *decodeState) value(v reflect.Value) {
 // indirect walks down v allocating pointers as needed,
 // until it gets to a non-pointer.
 // if it encounters an Unmarshaler, indirect stops and returns that.
-// if wantptr is true, indirect stops at the last pointer.
-func (d *decodeState) indirect(v reflect.Value, wantptr bool) (Unmarshaler, reflect.Value) {
+// if decodingNull is true, indirect stops at the last pointer so it can be set to nil.
+func (d *decodeState) indirect(v reflect.Value, decodingNull bool) (Unmarshaler, reflect.Value) {
 	// If v is a named type and is addressable,
 	// start with its address, so that if the type has pointer methods,
 	// we find them.
@@ -277,7 +278,7 @@ func (d *decodeState) indirect(v reflect.Value, wantptr bool) (Unmarshaler, refl
 			break
 		}
 
-		if pv.Elem().Kind() != reflect.Ptr && wantptr && pv.CanSet() && !isUnmarshaler {
+		if pv.Elem().Kind() != reflect.Ptr && decodingNull && pv.CanSet() {
 			return nil, pv
 		}
 		if pv.IsNil() {
diff --git a/src/pkg/json/encode.go b/src/pkg/json/encode.go
index 71d927d638..46abe4360e 100644
--- a/src/pkg/json/encode.go
+++ b/src/pkg/json/encode.go
@@ -24,8 +24,11 @@ import (
 // Marshal returns the JSON encoding of v.
 //
 // Marshal traverses the value v recursively.
-// If an encountered value implements the Marshaler interface,
-// Marshal calls its MarshalJSON method to produce JSON.
+// If an encountered value implements the Marshaler interface
+// and is not a nil pointer, Marshal calls its MarshalJSON method
+// to produce JSON.  The nil pointer exception is not strictly necessary
+// but mimics a similar, necessary exception in the behavior of
+// UnmarshalJSON.
 //
 // Otherwise, Marshal uses the following type-dependent default encodings:
 //
@@ -245,7 +248,7 @@ func (e *encodeState) reflectValueQuoted(v reflect.Value, quoted bool) {
 		return
 	}
 
-	if j, ok := v.Interface().(Marshaler); ok {
+	if j, ok := v.Interface().(Marshaler); ok && (v.Kind() != reflect.Ptr || !v.IsNil()) {
 		b, err := j.MarshalJSON()
 		if err == nil {
 			// copy JSON into buffer, checking validity.
diff --git a/src/pkg/json/stream_test.go b/src/pkg/json/stream_test.go
index 6ddaed9fe8..ce5a7e6d65 100644
--- a/src/pkg/json/stream_test.go
+++ b/src/pkg/json/stream_test.go
@@ -120,3 +120,28 @@ func TestRawMessage(t *testing.T) {
 		t.Fatalf("Marshal: have %#q want %#q", b, msg)
 	}
 }
+
+func TestNullRawMessage(t *testing.T) {
+	// TODO(rsc): Should not need the * in *RawMessage
+	var data struct {
+		X  float64
+		Id *RawMessage
+		Y  float32
+	}
+	data.Id = new(RawMessage)
+	const msg = `{"X":0.1,"Id":null,"Y":0.2}`
+	err := Unmarshal([]byte(msg), &data)
+	if err != nil {
+		t.Fatalf("Unmarshal: %v", err)
+	}
+	if data.Id != nil {
+		t.Fatalf("Raw mismatch: have non-nil, want nil")
+	}
+	b, err := Marshal(&data)
+	if err != nil {
+		t.Fatalf("Marshal: %v", err)
+	}
+	if string(b) != msg {
+		t.Fatalf("Marshal: have %#q want %#q", b, msg)
+	}
+}