Add -linecomment flag to enumer based on x/tools/cmd/stringer

[mewmew]

Hi Álvaro,

I just started looking at enumer and it looks great for my use case. Basically I want to automatically generate strings for enums, and convert strings back to enums.

However, the strings will not always be the same as the enum name. This was solved by the stringer tool by adding a -linecomment flag. With this flag, the line comment after an enum will be used for the string value, instead of the name of the enum.

https://github.com/golang/tools/blob/4fd33079060a7ef7bffe3a1c2369620f4f21ad06/cmd/stringer/stringer.go#L84

Please consider adding it to enumer. That would be very useful!

For an example usage of -linecomment see https://github.com/llir/l/blob/f6ae116b371f89c51d52b496de3487149b30335a/ir/ll/enums.go#L15

//go:generate stringer -linecomment -type Linkage

// Linkage specifies the linkage of a global identifier.
type Linkage uint8

// Linkage kinds.
const (
    LinkageNone                Linkage = iota // none
    LinkageAppending                          // appending
    LinkageAvailableExternally                // available_externally
    LinkageCommon                             // common
    LinkageInternal                           // internal
    LinkageLinkOnce                           // linkonce
    LinkageLinkOnceODR                        // linkonce_odr
    LinkagePrivate                            // private
    LinkageWeak                               // weak
    LinkageWeakODR                            // weak_odr
    // External linkage.
    LinkageExternal   // external
    LinkageExternWeak // extern_weak
)

Cheers, Robin

[gudvinr]

I'm not an author but as I see from example and probably motivation is to have snake_case mapping instead of camelCase since in json people mostly use snake_case in names.

So enumer already does that with less typing and if you want to have completely different names mapped to your constants I'd consider that bad practice.

[xescugc]

I also ran into this error, but the solution I've found is to use this https://github.com/alvaroloes/enumer#transforming-the-string-representation-of-the-enum-value, but I think that you may also want to use the trimprefix flag.

Even with that it would not be the same but close to what -linecomment does.

[gudvinr]

@xescugc but... This is not an issue. It just works that way so you don't need to write string representation by hand.

Though it may be useful if you want to have shorter version or something like that. But usually that's only makes things harder to debug and I'd probably consider that as bad design choice.

[xescugc]

I'm not sure of the priority, I think that it's low until the email service will start to be used, then the priority will be increased.

https://github.com/alvaroloes/enumer#how-to-use first line

The usage of Enumer is the same as Stringer, so you can refer to the Stringer docs for more information.

Stringer has the -linecomment, as pointed out for @mewmew, so enumer should have it, enumer should be a "drop in replace" for stringer with more options basically.

[gudvinr]

@xescugc good point. If you plan to change tools it will be easier in that case.

But stringer docs doesn't mention -linecomment option anywhere in the docs so why should it? You still can refer to existing docs.

[jpap]

I just hit the reverse-lookup limitation of stringer, which brought me here. I'd love to use enumer, but rely on the linecomment flag.

This flag is extraordinarily useful when you want to name your enums consistently, but the string form is (poorly) defined externally for which you have no control over: or suffer incompatibility when "parsing" them (string to int value lookup).

Pity the linecomment flag is undocumented. The Go team even uses it in their compiler source, which is how I found about it.

[dmarkham]

This feature we just added in v1.0.0 in the maintained fork https://github.com/dmarkham/enumer/releases

Along with go mod support. Comments/fixes/issues much appreciated.

[xescugc]

Cool! Just one think, is repo then unmantained? CC: @alvaroloes

[alvaroloes]

Hi! Thank you all for the discussion.

First, about the repo: It is true that I've been a loooong time gone, mainly working on other stuff with zero time to spend on Enumer. But the good news is that I now have more time and will be more responsive. So no, the repo is not unmaintained.

Now, about the issue: I have mixed feelings:

• On one side, the current implementation of Enumer already support some transformation options (like trimprefix or transform) that will be enough for several use cases, but linecomment has much more flexibility and, what is more, could entirely replace the need of transformers, which is a clear win regarding code complexity.
• On the other hand, it is true that linecomment opens the possibility of having a string representation that has nothing to do with the enum name, which can be confusing.

Right now, I'm leaning towards adding linecomment and deprecate the transformers (less code, less to maintain, fewer bugs). Let me know if you have any thought about this.

[gudvinr]

@alvaroloes this is only my opinion but I'd rather have your implementation of transformers rather than linecomment option. It's still a good thing to be compatible with stringer but for me this means at least rewriting all the code already relying on snake_case transform and writing new code with increasing verbosity by adding comments to every entry. I don't think linecomment can (and should) replace transformers since these transformer options are more generic solution than one that stringer uses. Manually copying and pasting repeated strings is stupidly annoying.

[alvaroloes]

Ok, I see your point about the transformers and I agree. They clearly add value.

Now the discussion is whether we should add the linecomment option or not. I would like to follow a similar philosophy as the Go team follows for adding new features to the language (not that strict, though): If we finally decide to add it, it should be because the value it adds to Enumer is bigger than the extra complexity it brings.

So far, these are the reasons I have read supporting the addition of linecomment

1. "Stringer has the -linecomment so Enumer should have it. Enumer should be a "drop in replace" for stringer with more options basically."

2. Useful when you have no control over the string representation of the enum. I guess this occurs when the Go enum is created to deserialize strings coming from another service.

(Note: Reason number 1 doesn't have to be true forever. If we consider it adequate, we can divorce Enumer from Stringer)

These are the drawbacks I see if we support linecomment

1. Too much flexibility (we could even have string representation with spaces) that could confuse when debugging.
2. We open the door to losing the 1-1 relationship between the enum name and the string representation: we could have two enum names with the same string representation. This adds complexity to the autogenerated reverse lookup method (this is similar to what happens in https://github.com/alvaroloes/enumer/pull/31 and https://github.com/alvaroloes/enumer/pull/38).

Of course, the drawbacks can be overcome. In order to decide, it would be good to have some use cases in real projects that can't be solved with transformations.

@mewmew @jpap @xescugc Do you have any real use case that can shed some light here?

[mewmew]

@mewmew @jpap @xescugc Do you have any real use case that can shed some light here?

Sure. The llir/llvm/ir/enum package contains a lot of enums for which the stringer tool is used to generate String methods.

The corresponding llir/llvm/asm/enum package contains FooFromString functions for each respective ir.Foo enum type.

Take for instance ir.CallingConv, the corresponding asm.CallingConvFromString function is implemented as follows:

From https://github.com/llir/llvm/blob/a687e6f5002a2c8587344c7a0d66f1c6718217e1/asm/enum/callingconv_string.go#L1:

// Code generated by "string2enum -linecomment -type CallingConv ../../ir/enum"; DO NOT EDIT.

package enum

import "fmt"
import "github.com/llir/llvm/ir/enum"

func _() {
    // An "invalid array index" compiler error signifies that the constant values have changed.
    // Re-run the string2enum command to generate them again.
    var x [1]struct{}
    _ = x[enum.CallingConvNone-0]
...
    _ = x[enum.CallingConvAArch64VectorCall-97]
}

const (
    _CallingConv_name_0 = "noneccc"
    _CallingConv_name_1 = "fastcccoldccghccccc 11webkit_jsccanyregccpreserve_mostccpreserve_allccswiftcccxx_fast_tlscc"
    _CallingConv_name_2 = "x86_stdcallccx86_fastcallccarm_apcsccarm_aapcsccarm_aapcs_vfpccmsp430_intrccx86_thiscallccptx_kernelptx_device"
    _CallingConv_name_3 = "spir_funcspir_kernelintel_ocl_biccx86_64_sysvccwin64ccx86_vectorcallcchhvmcchhvm_cccx86_intrccavr_intrccavr_signalcccc 86amdgpu_vsamdgpu_gsamdgpu_psamdgpu_csamdgpu_kernelx86_regcallccamdgpu_hscc 94amdgpu_lsamdgpu_esaarch64_vector_pcs"
)

var (
    _CallingConv_index_0 = [...]uint8{0, 4, 7}
    _CallingConv_index_1 = [...]uint8{0, 6, 12, 17, 22, 33, 41, 56, 70, 77, 91}
    _CallingConv_index_2 = [...]uint8{0, 13, 27, 37, 48, 63, 76, 90, 100, 110}
    _CallingConv_index_3 = [...]uint8{0, 9, 20, 34, 47, 54, 70, 76, 84, 94, 104, 116, 121, 130, 139, 148, 157, 170, 183, 192, 197, 206, 215, 233}
)

func CallingConvFromString(s string) enum.CallingConv {
    if len(s) == 0 {
        return 0
    }
    for i := range _CallingConv_index_0[:len(_CallingConv_index_0)-1] {
        if s == _CallingConv_name_0[_CallingConv_index_0[i]:_CallingConv_index_0[i+1]] {
            return enum.CallingConv(i + 0)
        }
    }
    for i := range _CallingConv_index_1[:len(_CallingConv_index_1)-1] {
        if s == _CallingConv_name_1[_CallingConv_index_1[i]:_CallingConv_index_1[i+1]] {
            return enum.CallingConv(i + 8)
        }
    }
    for i := range _CallingConv_index_2[:len(_CallingConv_index_2)-1] {
        if s == _CallingConv_name_2[_CallingConv_index_2[i]:_CallingConv_index_2[i+1]] {
            return enum.CallingConv(i + 64)
        }
    }
    for i := range _CallingConv_index_3[:len(_CallingConv_index_3)-1] {
        if s == _CallingConv_name_3[_CallingConv_index_3[i]:_CallingConv_index_3[i+1]] {
            return enum.CallingConv(i + 75)
        }
    }
    panic(fmt.Errorf("unable to locate CallingConv enum corresponding to %q", s))
}

Edit: for the string2enum tool used to generate the code above, refer to https://github.com/mewspring/tools/tree/master/cmd/string2enum

[gudvinr]

@alvaroloes When you write an app and you are the one who defines architecture choices then it's fine and you can easily use these generated enums with transformed names.

But sometimes you must rely on some others conventions and then it may hurt. E.g. you have some definitions that has to be converted as is but transformation does it wrong (in terms of conventions which may be crippled in first place but you can't change them).

Some ephemeral example:

type WriterName uint8
const (
    _ = iota
    MonaLisa        WriterName
    LeonardoDaVinci WriterName
    // ...
)

After transforming that should be mona_lisa and leonardo_da_vinci respectively. But original author of some API that you're using decided that names are mona_lisa and leonardo_davinci.

This is not quite real use case but I can see a point here and same approach may be needed by someone. Yet it can be helpful not too often to consider it useful.

[alvaroloes]

Thank you very much @mewmew and @gudvinr I would LOVE to see proper enum support from the Go team. Hopefully, https://github.com/golang/go/issues/28987, https://github.com/golang/go/issues/28438, or even https://github.com/golang/go/issues/32176 become true one day.

Ok, I think that linecomment can be added and let the user decide whether to use the transformers (in case it has control over the values) or not.

I've been thinking and the simplest way to deal with the possible duplicate "linecomment" value when doing a reverse lookup is to do nothing. This means that if, for example, you call PillString("Aspirin") and you have two enum values with the "linecomment" Aspirin, then any of the enum values could be returned (which one is undefined)

Good stuff! I'll add the linecomment functionality and update the Readme accordingly.

Thank you all!

[gudvinr]

@alvaroloes

This means that if, for example, you call PillString("Aspirin") and you have two enum values with the "linecomment" Aspirin, then any of the enum values could be returned (which one is undefined)

In your implementation you can restrict these cases and throw an error during generation. I don't think this is correct thing to do anyway.

[alvaroloes]

Well, the latest revision has several improvements, including -linecomment. You can have transformations and line comments. But when you use the flag -linecomment, the comment will take precedence.

Enumer is now using go modules and semantic versioning (last version is v1.1.2), so you can install the latest version by doing:

go get -u github.com/alvaroloes/enumer

Thanks to everybody who participated in this discussion.