Proposal: x/tools/cmd/stringer: optionally generate MarshalText/UnmarshalText

[mvdan]

Right now, one can use stringer to generate the String method to satisfy fmt.Stringer. That satisfies most of everyone's needs out of these enum-like integer types.

However, there's one fairly common scenario that isn't covered - encoding and decoding these values, for example with JSON. At the moment, simply using stringer won't be enough, as the values will simply be encoded as their integer selves.

I propose to add a way to also generate MarshalText and UnmarshalText methods to satisfy those encoding interfaces. Those are also more generic and useful than MarshalJSON or MarshalXML, since we don't restrict ourselves to any one encoding - all encodings that can use plain text will be able to make use of this.

Since stringer has it in its name to be about fmt.Stringer, I would say that the default behaviour should be to not add these methods. So these could be enabled with an extra flag like -textencoding or -gentext.

As for the implementation - MarshalText would simply be a call to String, always returning nil error. There are multiple ways to implement UnmarshalText, but I would likely go with a switch because that would be the simplest. Suggestions welcome.

I am happy to work on this myself if the proposal is accepted.

/cc @robpike @josharian

[ghost]

As for the implementation - MarshalText would simply be a call to String, always returning nil error. There are multiple ways to implement UnmarshalText, but I would likely go with a switch because that would be the simplest. Suggestions welcome.

FWIW, in nearly all my implementations of MarshalText and UnmarshalText, I find myself changing the the string representation to match the JavaScript naming conventions. I also return errors when encountering values that I have not defined. For example:

type MessageAlertType int

const (
	MessageAlertInfo MessageAlertType = iota + 1
	MessageAlertSuccess
	MessageAlertWarning
	MessageAlertDanger
)

func (t MessageAlertType) MarshalText() (text []byte, err error) {
	switch t {
	case MessageAlertInfo:
		return []byte("info"), nil
	case MessageAlertSuccess:
		return []byte("success"), nil
	case MessageAlertWarning:
		return []byte("warning"), nil
	case MessageAlertDanger:
		return []byte("danger"), nil
	default:
		return nil, errors.New("invalid alert type: " + strconv.Itoa(int(t)))
	}
}

func (t *MessageAlertType) UnmarshalText(text []byte) error {
	switch s := string(text); s {
	case "info":
		*t = MessageAlertInfo
	case "success":
		*t = MessageAlertSuccess
	case "warning":
		*t = MessageAlertWarning
	case "danger":
		*t = MessageAlertDanger
	default:
		return errors.New("invalid alert type: " + s)
	}
	return nil
}

[dominikh]

I don't have strong opinions, but a bit of general advice: resist the urge of piling everything into a single tool. As you correctly noted, stringer is called stringer, not allthingsenum. stringer isn't very complex, and doesn't implement any of the code necessary for unmarshaling, which means that creating a separate tool wouldn't create a lot of additional work.

[dgryski]

Related: https://github.com/campoy/jsonenums

[mvdan]

@bontibon I forgot about the errors - thanks for pointing it out. I'm not sure if some name manipulation magic would be a good idea.

Regarding existing and new tools - yes, I realise there are many doing similar things and that I could just write a new one too. That has several drawbacks, though:

• If a tool just does encoding/decoding, I end up with foo_string.go and foo_text.go (and easily multiplied by a number of types, too)
• If a tool does both, it would either copy most of its code, or roll its own version
• All the existing tools seem to revolve around JSON only, not text encoding

So while yes, I agree that adding this has its downsides, I also think it wouldn't complicate stringer that much more. And it seems to me like a natural evolution from what it does at the moment.

[jimmyfrasche]

Personally I'd make a separate tool but factor the common machinery for recognizing enums out of stringer into a package under golang.org/x/tools/cmd/internal that both tools use.

[mvdan]

I hadn't thought about a second tool in x/tools - that works for me too. Anyone think it's a bad idea?

We can bikeshed about the name later.

[rsc]

Stringer is stringer. As @dominikh said, make a different tool for marshaling, outside the Go repos. (As @bontibon said, different marshaling will want different conventions anyway.)