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

in printed diagnostic, sort the missing constant names by AST order (not lexicographical order) #41

Closed
nishanths opened this issue May 8, 2022 · 0 comments · Fixed by #44

Comments

@nishanths
Copy link
Owner

nishanths commented May 8, 2022

Background

Given a program:

type state int

// Internal states used by Writer and Reader.
const (
	stateIdentifier state = iota
	stateHeader
	stateEncryptedHeader
	stateData
	stateFiller
	stateDone
)

func (r *Reader) xxx() {
	switch r.state {
	case stateIdentifier:
	case stateHeader:
	}
}

the exhaustive command will currently (v0.7.11) report:

missing cases in switch of type state: stateData, stateDone, stateEncryptedHeader, stateFiller

Proposal

Change the diagnostic to be the following.

missing cases in switch of type state: stateEncryptedHeader, stateData, stateFiller, stateDone

That is, sort the constant names by AST appearance order (not lexicographical order).

Notes

The AST order makes more natural sense in the context of most programs. For example, in the program above, the AST order is the sequence of states the Reader type goes through when reading its input. The switch statement would be better off listing the cases in that order. I can't think of a counterexample in which using the lexicographical order is more beneficial than the AST order.

The enumMembers type (see file enum.go) already records the list of constant names in AST order. This order can be used during the sort. Implementing the proposal will not introduce a lot of new code.

type enumMembers struct {
    Names        []string // enum member names, AST order
    // ... other fields elided ...
}

Tests in this repo will need to be adjusted. Tests in the downstream repo golangci-lint, which has a couple of integration tests that assert against exhaustive's diagnostic strings, may also need to be adjusted.

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

Successfully merging a pull request may close this issue.

1 participant