Flag for case sensitive in <Type>String(s string)

[rucciva]

in my case, i'm converting iota to 1 character string and they includes both lower and upper version of some alphabets. i've read #21 , so maybe add a flag to enable case sensitive in <Type>String(s string).

[dmarkham]

yea I knew someone would not be very happy with this change.. If you have a nice patch for a new flag I'm happy to review.

[dmarkham]

I wonder if instead of a flag we just add a new case sensitive method? <Type>SensitiveString(s string) maybe someone has a better name for it.

[gabe565]

@dmarkham @rucciva This just caused a bug in an app I built, so I decided to propose some fixes. The current behavior causes a silent bug if there are two consts with the same value other than a case change, like here.

My two ideas to fix are:

1. Add a -casesensitive flag, which disables all generation of lower-cased values.
   • This required more code changes up front, and users have to know to add the -casesensitive flag, but the resulting functionality completely eliminates the possibility of choosing the incorrect value.
   • This method also has the benefit of not changing existing code after users update enumer. Code would only change after adding -casesensitive.
   • Unfortunately, users may not realize there is a -casesensitive flag and could introduce bugs in their code in the future.
   • See PR feat: Add -casesensitive flag #96
2. Split _typeNameToValueMap into _typeNameToValueMap and _typeLowerNameToValueMap.
   • _typeNameToValueMap is checked first with a fallback to the lower-cased map. This lets the current case-insensitivity behavior stay the same, while also choosing the correct value if there are overlaps.
   • This route seems like a safer option since users don't need to be aware of a flag, but code will be slightly different after people update enumer. Is that ok?
   • Unfortunately, the golden files will all need to be updated if we go this route. Is there are way to re-generate them? (Used the golden test to regenerate).
   • See PR fix: Move lower-cased names to a separate map to fix overlapping cases #97

I opened a PR for both of these solutions, but only one needs to be merged...although both being merged might be nice. The first solution would be nice for times where casing should explicitly be respected, while the second prevents a bug without requiring a flag.

Any thoughts?

[dmarkham]

@samiam2013 thoughts?

[gabe565]

Just want to note that a workaround with the current behavior is to define consts with upper-case values before the lower-cased ones. Here's a quick repro:

Click to expand...

package main

import "fmt"

//go:generate enumer -type ExampleLowerFirst -linecomment

type ExampleLowerFirst uint8

const (
	LFLower ExampleLowerFirst = iota // example
	LFUpper                          // EXAMPLE
)

//go:generate enumer -type ExampleUpperFirst -linecomment

type ExampleUpperFirst uint8

const (
	UFUpper ExampleUpperFirst = iota // EXAMPLE
	UFLower                          // example
)

func main() {
	fmt.Println("ExampleLowerFirst:")
	var e1 ExampleLowerFirst
	e1, _ = ExampleLowerFirstString("EXAMPLE")
	fmt.Println(uint8(e1), e1)
	e1, _ = ExampleLowerFirstString("example")
	fmt.Println(uint8(e1), e1)

	fmt.Println("ExampleUpperFirst:")
	var e2 ExampleUpperFirst
	e2, _ = ExampleUpperFirstString("EXAMPLE")
	fmt.Println(uint8(e2), e2)
	e2, _ = ExampleUpperFirstString("example")
	fmt.Println(uint8(e2), e2)
}

The code above results in:

ExampleLowerFirst:
1 EXAMPLE
1 EXAMPLE
ExampleUpperFirst:
0 EXAMPLE
1 example

Both of my PRs correct this behavior, resulting in:

ExampleLowerFirst:
1 EXAMPLE
0 example
ExampleUpperFirst:
0 EXAMPLE
1 example

The more I think about it, the more I'm preferring the second option since it fixes this bug without any developer intervention, other than re-generating.