This proposal is to add a new vet checker that warns when an exported error field of a struct is marshaled. Currently what will happen is that as error is an interface this will use the marshaling of the underlying type. error values created by errors.New() and fmt.Errorf() are currently implemented by structs with unexported string fields. The result is that the marshaled value is "{}".

This behavior is well defined, but many users expect to see the "err.Error()" value. These would consider their code buggy. So this plausibly satisfies vet's correctness criteria.

Similar issues: #26946, #41241, #26889, #23549.

Example:
https://play.golang.org/p/XHoSKtGwAUN

package main

import (
	"encoding/json"
	"fmt"
)
type Response struct {
	Err error `json:"error"`
}
func main() {
	v := &Response{Err: fmt.Errorf("oops")}
	b, err := json.Marshal(v)
	if err != nil {
		panic(err)
	}
	fmt.Printf("Marshaled value: %s\n", b)
}

Output:

Marshaled value: {"error":{}}

The current focus is on json.Marshal(), but could be extended to similar standard library functions.

To avoid spurious reports the proposed conditions to warn would be when:

• there is a json.Marshal(e) expression,
• we can locally identify a non-interface type T for e, and
• marshaling T reaches an exported error field (follow pointers, struct sub-fields, map fields, etc.) without traversing an interface or a type with MarshalJSON() defined on it.

These reports can potentially be considered false positives when all of the underlying types of the error values stored in the field have an MarshalJSON function defined or have a well defined serialization: type MyErr string; func (MyErr) Error() string {...}.

This would not report flows through interface{} variables for functions wrapping json.Marshall(). Example of this pattern but not necessarily for error values here.