Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

staticcheck doesn't know about my enumer #660

Closed
seebs opened this issue Nov 22, 2019 · 4 comments
Closed

staticcheck doesn't know about my enumer #660

seebs opened this issue Nov 22, 2019 · 4 comments

Comments

@seebs
Copy link

seebs commented Nov 22, 2019

2019.2.3

package main

import "fmt"

//go:generate enumer -type=timeQuantum -trimprefix=timeQuantum -text -transform=caps -output enums_timequantum.go

type timeQuantum int

const (
	timeQuantumY timeQuantum = iota
	timeQuantumYM
	timeQuantumYMD
	timeQuantumYMDH
)

func main() {
	var t timeQuantum
	err := t.UnmarshalText([]byte("YMDH"))
	fmt.Printf("t: %d, err %v, %s\n", t, err, t)
}

The generated code is pretty straightforward -- it translates "YMDH" to 3.

Note that the sequence timeQuantumYMDH, while "unused", is affecting the behavior of the program in the expected manner.

I honestly don't have any clue how this could be reasonably detected and I should probably just use a linter comment but I feel like the stringer-type cases are just common enough to be worth bringing up. (enumer is a fork of stringer that, well. Does this.)

@seebs seebs added false-positive needs-triage Newly filed issue that needs triage labels Nov 22, 2019
@ainar-g
Copy link
Contributor

ainar-g commented Nov 22, 2019

I think it's an issue with enumer. Looking at their golden tests it seems that they don't use the actual constants there:

var _DayNameToValueMap = map[string]Day{
	_DayName[0:6]:   0,
	_DayName[6:13]:  1,
	_DayName[13:22]: 2,
	_DayName[22:30]: 3,
	_DayName[30:36]: 4,
	_DayName[36:44]: 5,
	_DayName[44:50]: 6,
}

Instead of:

var _DayNameToValueMap = map[string]Day{
	_DayName[0:6]:   Monday,
	_DayName[6:13]:  Tuesday,
	_DayName[13:22]: Wednesday,
	_DayName[22:30]: Thursday,
	_DayName[30:36]: Friday,
	_DayName[36:44]: Saturday,
	_DayName[44:50]: Sunday,
}

I don't know why the authors of the tool decided to use raw numbers.

@dominikh
Copy link
Owner

I'm inclined to agree with Ainar. The original stringer doesn't have this issue because it doesn't map from strings to constants. enumer does offer this mapping, so it probably should use the constants it maps to.

@dominikh dominikh added noisy-positive waiting-for-feedback Waiting for the user to get back to us and removed false-positive needs-triage Newly filed issue that needs triage labels Nov 23, 2019
@seebs
Copy link
Author

seebs commented Nov 23, 2019

Thinking about it, though, it occurs to me that it's plausible to imagine a use case where it's practical to just omit the constant entirely; for instance, consider

    _DayName[0:6], _DayName[6:13], ...
}```
where there's nothing to map *to*, we just use the index in the array as the value. Only works with contiguous values starting at zero, but that's super common for enum-like cases.

That said, for this specific one, agreed that enumer could do a better job.

@dominikh
Copy link
Owner

I am going to close this issue as WAI. Note that only one constant from the group of constants needs to be used to mark the entire group as used. If neither you nor the tool uses any of the constants, one must wonder why the constants are being generated at all. In this specific case, it should be on the tool to actually use the constants.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants