dmarkham / enumer

A Go tool to auto generate methods for your enums
Other
411 stars 63 forks source link

make enumer really case insensitive #37

Closed mvrhov closed 4 years ago

mvrhov commented 4 years ago

Right now if you define different than default transformer e.g title-lower the *String method will fail if the input string has e.g first letter uppercase.

Now what's missing is the test that really verifies that the *String method is really case insensitive. And unfortunately I have no idea on how to implement that.

codecov-commenter commented 4 years ago

Codecov Report

Merging #37 into master will decrease coverage by 0.14%. The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #37      +/-   ##
==========================================
- Coverage   66.15%   66.00%   -0.15%     
==========================================
  Files           3        3              
  Lines         452      453       +1     
==========================================
  Hits          299      299              
- Misses        142      143       +1     
  Partials       11       11              
Impacted Files Coverage Δ
enumer.go 100.00% <ø> (ø)
stringer.go 61.11% <0.00%> (-0.16%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0a1f0e6...b28c903. Read the comment docs.

dmarkham commented 4 years ago

@mvrhov Thank you for the patch! Do you have sample code for me to reproduce the bug? I'm looking at your fix and just trying to understand better what you have found.

mvrhov commented 4 years ago
//go:generate go run github.com/dmarkham/enumer -type=ProcessingStatus -json -sql -text -trimprefix=ProcessingStatus -transform=title-lower
type ProcessingStatus int

const (
    ProcessingStatusUnknown ProcessingStatus = iota + 1
    ProcessingStatusCancelled
    ProcessingStatusToAssign
    ProcessingStatusReady
    ProcessingStatusCompleted
)

// this results in "Ready does not belong to ProcessingStatus values" and you can choose any other case with the mix of lower/upper characters that they are actually in the list e.g CoMpLeTeD and it will also fail.
v := ProcessingStatusString("Ready") 
dmarkham commented 4 years ago

Thank you for this report and code I will dig in and try to better understand what went wrong and get your patch in asap.

dmarkham commented 4 years ago

Ok I think i understand better now how I messed up thank you @mvrhov for some reason I want to change this patch just a little. Just to be extra safe I think I want to do the the exact match (with case) first. Then try the lowercase check. Just in case someone is trying to use case. This way I think we will not break anything else we did not anticipate.

Something like this? What do you think?

func %[1]sString(s string) (%[1]s, error) {
    if val, ok := _%[1]sNameToValueMap[s]; ok {
        return val, nil
    }
    s = strings.ToLower(s)
    if val, ok := _%[1]sNameToValueMap[s]; ok {
        return val, nil
    }
    return 0, fmt.Errorf("%%s does not belong to %[1]s values", s)
}
mvrhov commented 4 years ago

if you think that this is a better fix then by all means