Update or replace the enumer go:generate tool

[na--]

Currently, we are using https://github.com/alvaroloes/enumer to semi-automatically generate some helper methods for a several enum types we have in k6:

k6/.github/workflows/lint.yml

Line 31 in c09919b

go install github.com/alvaroloes/[email protected]

k6/metrics/system_tag.go

Lines 12 to 13 in c09919b

	//go:generate enumer -type=SystemTag -transform=snake -trimprefix=Tag -output system_tag_gen.go
	type SystemTag uint32

k6/lib/execution.go

Lines 34 to 35 in c09919b

	//go:generate enumer -type=ExecutionStatus -trimprefix ExecutionStatus -output execution_status_gen.go
	type ExecutionStatus uint32

(and others)

Unfortunately, it hasn't been updated since 2019, and the current version produces code that has linter errors. It's also produces some methods we don't really use in all types...

The active fork of that tool seems to be https://github.com/dmarkham/enumer, and it fixes some things. Unfortunately, it also makes the second problem even worse. For example, it adds lowercase support (dmarkham/enumer#22) in a way that can't be disabled (dmarkham/enumer#40), which is not something we necessarily want in all of our types... 😞

[nrxr]

This could be an easier approach, which eases maintainability and will remove the need for generators. Using a method String() for the SystemType type also satisfies the interface expected by fmt's %s verb (as you can see in the tests).

// systemtag.go
package systemtag

type SystemTag uint32

const (
	TagProto SystemTag = 1 << iota
	TagSubproto
	TagStatus
)

var tagToValue = map[SystemTag]string{
	TagProto:    "proto",
	TagSubproto: "subproto",
	TagStatus:   "status",
}

func (st SystemTag) String() string {
	return tagToValue[st]
}

// systemtag_test.go
package systemtag

import "testing"

func TestSystemTag(t *testing.T) {
	cases := []struct {
		name   string
		tag    SystemTag
		expect string
	}{
		{
			name:   "TagProto = proto",
			tag:    TagProto,
			expect: "proto",
		},
	}

	for _, c := range cases {
		if c.tag.String() != c.expect {
			t.Errorf("received %s and expected %s", c.tag, c.expect)
		}
	}
}

[na--]

Thanks for the suggestion, @nrxr, but it's not a viable one, since it has a few significant downsides to the enumer approach:

• It will have to be maintained by hand for all of the 8 types that currently need it and any future ones.
• Hand-rolling the string values is error prone, and we don't just need a String() method, we may also need to support marshaling and unmarshaling for JSON and text.
• Using a map for strings is very inefficient, since it requires a map lookup every time, whereas (for sequential values), it can be made much more efficiently like this:

  k6/lib/execution_status_gen.go

  Lines 10 to 19 in 1785ec0

  	const _ExecutionStatusName = "CreatedInitVUsInitExecutorsInitDonePausedBeforeRunStartedSetupRunningTeardownEndedInterrupted"
  	var _ExecutionStatusIndex = [...]uint8{0, 7, 14, 27, 35, 50, 57, 62, 69, 77, 82, 93}
  	func (i ExecutionStatus) String() string {
  	if i >= ExecutionStatus(len(_ExecutionStatusIndex)-1) {
  	return fmt.Sprintf("ExecutionStatus(%d)", i)
  	}
  	return _ExecutionStatusName[_ExecutionStatusIndex[i]:_ExecutionStatusIndex[i+1]]
  	}

[nrxr]

• It will have to be maintained by hand for all of the 8 types that currently need it and any future ones.

I understand, yet, it seems these are very rarely updated and by tying the code to a generator you can get stuck in a situation like the current one.

• Hand-rolling the string values is error prone, and we don't just need a String() method, we may also need to support marshaling and unmarshaling for JSON and text.

Yes, is error-prone but nothing that a unit test of constant to value (as the one I wrote early on) could not catch on-time. Right now, you're trusting the output of an unmaintained generator :-)

• Using a map for strings is very inefficient, since it requires a map lookup every time, whereas (for sequential values), it can be made much more efficiently like this:

  k6/lib/execution_status_gen.go

  Lines 10 to 19 in 1785ec0

  	const _ExecutionStatusName = "CreatedInitVUsInitExecutorsInitDonePausedBeforeRunStartedSetupRunningTeardownEndedInterrupted"
  	var _ExecutionStatusIndex = [...]uint8{0, 7, 14, 27, 35, 50, 57, 62, 69, 77, 82, 93}
  	func (i ExecutionStatus) String() string {
  	if i >= ExecutionStatus(len(_ExecutionStatusIndex)-1) {
  	return fmt.Sprintf("ExecutionStatus(%d)", i)
  	}
  	return _ExecutionStatusName[_ExecutionStatusIndex[i]:_ExecutionStatusIndex[i+1]]
  	}

I got interested on this, it seems there are less clever ways to implement it way more efficiently. The net/http library does something like this:

https://github.com/nrxr/enumbenchmarks/blob/340c2c95f29040370ff4e283accdded7e5691f47/idiomatic/executionstatus/executionstatus.go#L19-L48

And using a simple string slice instead of a string + index, you get even better results:

https://github.com/nrxr/enumbenchmarks/blob/340c2c95f29040370ff4e283accdded7e5691f47/manual/executionstatus/executionstatus.go#L19-L31

Also, the ExecutionStatusString function can be implemented a bit faster using a map with the swapped types from what I sent before and even faster using the idiomatic way.

If performance is the important bit, a combination of String() from the manual implementation with the String of the idiomatic implementation would be the ideal.

This code is way cleaner than what enumer generates, in my opinion. Yet, I guess writing a generator that creates code like this using stringer as the starting point wouldn't be so hard if you would prefer the generator-way.

The implementations here would be backwards-compatible with what you already have in the project: https://github.com/nrxr/enumbenchmarks

[na--]

Awesome work! 🎉 ❤️ Thanks! 🙇

This code is way cleaner than what enumer generates, in my opinion.

Completely agree!

Yet, I guess writing a generator that creates code like this using stringer as the starting point wouldn't be so hard if you would prefer the generator-way.

Yeah, I'd still probably somewhat prefer a better generator that uses this approach. It will have uses outside of k6, and we'd only need to write tests for the generator, not for the base functionality of every enum set that uses it.

[na--]

@nrxr, is such a generator an OSS project that you are interested in creating?

This is a relatively lower prio issue for us, I just opened it when I was dealing with #2995 because I saw that there were linting issues in the generated code from enumer. We might get around to implementing it ourselves eventually, but if a code generator that produces more readable and performant code existed, we'd eagerly switch to it.

[nrxr]

@na-- I'm actually considering it for this weekend. I'll let you know. I got really involved with this issue now and want to see it to fruition.

My expectation would be a generator that makes code that can be maintained manually too, if the generator were to fail, and yet is more performant than current options.