internal/jsonrpc2: don't set result in response if there is an error

Marshalling an empty raw message produces a null rather than omitting the field
Instead we do not set the field unless there is no error.
Includes a test that reproduced the issue before the fix.

Fixes golang/go#39719

Change-Id: I683b8ea55d3425613fc956993280ff51202f49fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/239097
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>

internal/jsonrpc2/messages.go

@@ -146,7 +146,10 @@ func (msg *Response) Err() error              { return msg.err }
 func (msg *Response) isJSONRPC2Message()      {}
 
 func (r *Response) MarshalJSON() ([]byte, error) {
-	msg := &wireResponse{Result: &r.result, Error: toWireError(r.err), ID: &r.id}
+	msg := &wireResponse{Error: toWireError(r.err), ID: &r.id}
+	if msg.Error == nil {
+		msg.Result = &r.result
+	}
 	data, err := json.Marshal(msg)
 	if err != nil {
 		return data, fmt.Errorf("marshaling notification: %w", err)

internal/jsonrpc2/wire_test.go

@@ -81,6 +81,25 @@ func TestIDDecode(t *testing.T) {
 	}
 }
 
+func TestErrorResponse(t *testing.T) {
+	// originally reported in #39719, this checks that result is not present if
+	// it is an error response
+	r, _ := jsonrpc2.NewResponse(jsonrpc2.NewIntID(3), nil, fmt.Errorf("computing fix edits"))
+	data, err := json.Marshal(r)
+	if err != nil {
+		t.Fatal(err)
+	}
+	checkJSON(t, data, []byte(`{
+		"jsonrpc":"2.0",
+		"error":{
+			"code":0,
+			"message":"computing fix edits",
+			"data":null
+		},
+		"id":3
+	}`))
+}
+
 func checkJSON(t *testing.T, got, want []byte) {
 	// compare the compact form, to allow for formatting differences
 	g := &bytes.Buffer{}