Giter VIP home page Giter VIP logo

exhaustive's People

Contributors

aaron-mongodb avatar fastnav avatar iwahbe avatar jftuga avatar maratori avatar navijation avatar nishanths avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar

exhaustive's Issues

same name enum type in inner scope can produce incorrect diagnostic

Input like this currently produce an incorrect diagnostic.

package example

type T int

const (
	A T = iota
	B
)

func f() {
	type T int

	const (
		C T = iota
		D
	)
	
	var v T
	switch v {
	case C, D:
	}
}

The incorrect diagnostic is:

missing cases in switch of type T: A, B

Background

The analyzer only supports enum types that are at the package level. To lookup an enum type, it looks up a map using a string key equal to the enum type name. If an inner scope has a type of the same name, like in this example, it results in an incorrect diagnostic.

Next steps

It may be best to address this together with #11.

Check enum based on struct

There is alternative approach how to deal with enums in Go.
Example: https://github.com/ThreeDotsLabs/wild-workouts-go-ddd-example/blob/master/internal/trainer/domain/hour/availability.go
Article with motivation: https://threedots.tech/post/safer-enums-in-go/.

The idea is to use struct with private field and package level variables.
It would be great if exhaustive checks such enums as well, not only constants.

var (
	Available         = Availability{"available"}
	NotAvailable      = Availability{"not_available"}
	TrainingScheduled = Availability{"training_scheduled"}
)

var availabilityValues = []Availability{
	Available,
	NotAvailable,
	TrainingScheduled,
}

// Availability is enum.
//
// Using struct instead of `type Availability string` for enums allows us to ensure,
// that we have full control of what values are possible.
// With `type Availability string` you are able to create `Availability("i_can_put_anything_here")`
type Availability struct {
	a string
}

func NewAvailabilityFromString(availabilityStr string) (Availability, error) {
	for _, availability := range availabilityValues {
		if availability.String() == availabilityStr {
			return availability, nil
		}
	}
	return Availability{}, errors.Errorf("unknown '%s' availability", availabilityStr)
}

// Every type in Go have zero value. In that case it's `Availability{}`.
// It's always a good idea to check if provided value is not zero!
func (h Availability) IsZero() bool {
	return h == Availability{}
}

func (h Availability) String() string {
	return h.a
}

Report invalid enforcement comments

It is possible for a user to misplace an enforcement comment and thereby think enforcement is going to occur even when it doesn't, leading to bugs. Enforcement comments that don't end up touching a switch or map should raise an error.

The same might be said about any //exhaustive:ignore comments that aren't applied to any valid AST nodes.

provide a way to exclude *_UNSPECIFIED generated for protobuf enums

Protobuf guidelines recommends to define UNSPECIFIED at first value for all enums, to ensure 0 value (which is often received from peer which doesn't know about this enum at all) won't match any used enum value.

So, following guideline we have proto file like:

// Supported exchanges.
// New values may be added in the future.
enum Exchange {
    // Default value. This value is unused.
    EXCHANGE_UNSPECIFIED = 0;
    EXCHANGE_BITMEX = 1;
    EXCHANGE_BINANCE = 2;
}

As a result, we get something like this in auto-generated *.pb.go:

// Supported exchanges.
// New values may be added in the future.
type Exchange int32

const (
	// Default value. This value is unused.
	Exchange_EXCHANGE_UNSPECIFIED Exchange = 0
	Exchange_EXCHANGE_BITMEX      Exchange = 1
	Exchange_EXCHANGE_BINANCE     Exchange = 2
)

Next, when parsing incoming message we have a switch like:

	switch v {
	case api.Exchange_EXCHANGE_BITMEX:
		return dom.XBitmex, nil
	case api.Exchange_EXCHANGE_BINANCE:
		return dom.XBinance, nil
	default:
		return 0, fmt.Errorf("exchange: %w: %d", errUnknownEnum, v)
	}

And current implementation of your linter complains on this code:

missing cases in switch of type api.Exchange: Exchange_EXCHANGE_UNSPECIFIED

My point is:

  • Adding extra case for UNSPECIFIED value is a bad idea because it must be handled exactly as any other unknown value, i.e. in same way as default: case. And there are no good way to add it: such case will either duplicate code from default: or contains fallthrough - in both ways it's too easy to occasionally make a mistake later (by changing code in only one of these two cases or by adding new case between case UNSPECIFIED: and default:).
  • Enabling linter flag to make default: a catch-all will result in losing most of linter's power and many other mistakes won't be detected.

So, I propose to add one more flag (and related option in golangci-lint config) to define a regexp like "_UNSPECIFIED$" to ignore matched values.

Panic in latest release

I get this on running the command

github.com/nishanths/exhaustive.missingCasesTextEdit(0xc000135b00, 0xc00a4ef600, 0xc00a846500, 0xc00a84cc30, 0xc00a8451c0, 0xc004ffc0a0, 0xc0275cf800, 0x0, 0x0, 0x0, ...)
        /Users/angadn/go/pkg/mod/github.com/nishanths/[email protected]/switch.go:403 +0x891
github.com/nishanths/exhaustive.computeFix(0xc01f0e4000, 0xc000135b00, 0xc00a4ef600, 0xc00a84cc30, 0xc00a8451c0, 0xc004ffc0a0, 0xc024121800, 0xc0275cf800, 0x0, 0x0, ...)
        /Users/angadn/go/pkg/mod/github.com/nishanths/[email protected]/switch.go:267 +0xe5
github.com/nishanths/exhaustive.reportSwitch(0xc01f0e4000, 0xc00a84cc30, 0xc00a8451c0, 0x0, 0xc004ffc0a0, 0xc027542900, 0xc0275cf800, 0xc00a4ef600)
        /Users/angadn/go/pkg/mod/github.com/nishanths/[email protected]/switch.go:241 +0xff
github.com/nishanths/exhaustive.checkSwitchStatements_.func1(0x13aaeb8, 0xc00a84cc30, 0x1, 0xc0374d4200, 0x6, 0x20, 0x1)

`info.Defs[%s] == nil` panic when `go.mod` have `go 1.22`

➜ go version
go version go1.22.0 darwin/arm64
➜ exhaustive -check=switch,map ./...
panic: info.Defs[ctxKey] == nil

goroutine 7331 [running]:
github.com/nishanths/exhaustive.possibleEnumMember(0x140094ba840, 0x1400dc546e0)
	[REMOVED]/go/pkg/mod/github.com/nishanths/[email protected]/enum.go:115 +0x244
➜ echo $?
2
➜ sed -i 's/go 1.22/go 1.21/g' go.mod
➜ exhaustive -check=switch,map ./...
➜ echo $?
0
Complete stacktrace
➜ go version
go version go1.22.0 darwin/arm64
➜ exhaustive -check=switch,map ./...
panic: info.Defs[ctxKey] == nil

goroutine 7216 [running]:
github.com/nishanths/exhaustive.possibleEnumMember(0x1400d5b0580, 0x1400d5bc820)
	[REMOVED]/go/pkg/mod/github.com/nishanths/[email protected]/enum.go:115 +0x244
github.com/nishanths/exhaustive.findEnums.func1({0x1031c78f8?, 0x1400d5aa8c0?})
	[REMOVED]/go/pkg/mod/github.com/nishanths/[email protected]/enum.go:76 +0x128
golang.org/x/tools/go/ast/inspector.(*Inspector).Preorder(0x140260b7fb0, {0x14000e98930?, 0x1?, 0x102e42e54?}, 0x14001121940)
	[REMOVED]/go/pkg/mod/golang.org/x/[email protected]/go/ast/inspector/inspector.go:82 +0x98
github.com/nishanths/exhaustive.findEnums(0x0, 0x1400d5ba060, 0x103372380?, 0x1400d5bc820)
	[REMOVED]/go/pkg/mod/github.com/nishanths/[email protected]/enum.go:69 +0xa8
github.com/nishanths/exhaustive.run(0x1402d5bbad0)
	[REMOVED]/go/pkg/mod/github.com/nishanths/[email protected]/exhaustive.go:111 +0x70
golang.org/x/tools/go/analysis/internal/checker.(*action).execOnce(0x140268f2e60)
	[REMOVED]/go/pkg/mod/golang.org/x/[email protected]/go/analysis/internal/checker/checker.go:767 +0x844
sync.(*Once).doSlow(0x140013027a8?, 0x103086904?)
	/opt/homebrew/opt/go/libexec/src/sync/once.go:74 +0x100
sync.(*Once).Do(...)
	/opt/homebrew/opt/go/libexec/src/sync/once.go:65
golang.org/x/tools/go/analysis/internal/checker.(*action).exec(...)
	[REMOVED]/go/pkg/mod/golang.org/x/[email protected]/go/analysis/internal/checker/checker.go:683
golang.org/x/tools/go/analysis/internal/checker.execAll.func1(0x0?)
	[REMOVED]/go/pkg/mod/golang.org/x/[email protected]/go/analysis/internal/checker/checker.go:671 +0x4c
created by golang.org/x/tools/go/analysis/internal/checker.execAll in goroutine 1
	[REMOVED]/go/pkg/mod/golang.org/x/[email protected]/go/analysis/internal/checker/checker.go:677 +0x144
➜ echo $?
2
➜ sed -i 's/go 1.22/go 1.21/g' go.mod
➜ exhaustive -check=switch,map ./...
➜ echo $?
0

The ctxKey type looks like this:

type ctxKeyRequestID int
const ctxKey ctxKeyRequestID = 0

If I change the go.mod file back to go 1.21 then exhaustive works fine. Unfortunately I can't remove this type since it is part of the core of the service so I can't verify it other types in this service triggers the same error.

Go 1.22 includes some changes to both go/ast and go/types but I can't see anything that is the obvious source of this error.

Go 1.22 changelog:

go/ast
The following declarations related to syntactic identifier resolution are now deprecated: Ident.Obj, Object, Scope, File.Scope, File.Unresolved, Importer, Package, NewPackage. In general, identifiers cannot be accurately resolved without type information. Consider, for example, the identifier K in T{K: ""}: it could be the name of a local variable if T is a map type, or the name of a field if T is a struct type. New programs should use the go/types package to resolve identifiers; see Object, Info.Uses, and Info.Defs for details.
[...]

option to ignore switch statements on things like reflect.Kind

Most code I have written and seen using reflect.Kind etc rarely is ever exhaustive when dealing with reflected types, so I find myself always having to add //nolint:exhaustive.

Would be great to have a way to ignore certain types like this via configuration of exhaustive or it to ignore those types by default

consider making the '-fix' flag a no-op

The '-fix' implementation requires a bunch of code (and more code is more surface area for bugs). It's not clear to me that '-fix', as implemented currently, is a useful feature.

handle "default" case

currently the "default:" case is not handled well and the this checker asks to cover all cases even when default does that

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

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.

Allow exhaustive check on map enum values as well as keys

This is to support EnumFromX type conversions. e.g.

var enumFromStringMap = map[string]Enum {
   "A": EnumA,
   "B": EnumB,
   // forgot EnumC
   "D": EnumD,
}

It might end up a bit weird if both key and value are different enums. Although I'm not sure why you'd be using a map[EnumA]EnumB unless there was a one-to-one correspondence.

Check exhaustiveness of map values in addition to keys

My use case is to define an ACL layer that needs to map both to and from domain enums to external values.

type ContactPreference string

const (
	ContactPreferenceMobile ContactPreference = "MOBILE"
	ContactPreferenceHome   ContactPreference = "HOME"
	ContactPreferenceMail   ContactPreference = "MAIL"
	ContactPreferenceNone   ContactPreference = ""
)

var fromContactPreferenceMap = map[ContactPreference]string{
	ContactPreferenceMobile: "mobilePhone",
	ContactPreferenceHome:   "homePhone",
	ContactPreferenceMail:   "physicalMail",
	ContactPreferenceNone:   "",
}

var toContactPreferenceMap = map[string]ContactPreference{
	"mobilePhone":  ContactPreferenceMobile,
	"homePhone":    ContactPreferenceHome,
	"physicalMail": ContactPreferenceMail,
	"":             ContactPreferenceNone,
}

func toContactPreference(cp string) ContactPreference {
	if contactPref, ok := toContactPreferenceMap[cp]; ok {
		return contactPref
	}

	return ContactPreferenceNone
}

func fromContactPreference(cp ContactPreference) string {
	return fromContactPreferenceMap[cp]
}

In the above example, fromContactPreferenceMap will be checked for exhaustive use of ContactPreference because it's the keys of the map.
However, toContactPreferenceMap will not be checked for exhaustiveness because ContactPreference is the values in the map.

stricter requirements for directive comments

For example, the current code in master considers comments of the form:

//exhaustive:ignore-more-text

to be a valid equivalent of:

//exhaustive:ignore

We don't wish to allow the above forms as an equivalent. This will be a breaking change for users who didn't use the strictly exact form.

That said, the following should continue to be allowed (notice instead of -):

//exhaustive:ignore more-text

See discussion here: #68 (comment).

superfluous type conversion in case clause results in a false positive report

type M int

const (
	A M = iota
	B
)

func quux(v M) {
	switch v {
	case M(A):
	}
}

For the program above, exhaustive currently incorrectly reports:

missing cases in switch of type M: A, B

However, it should only report B as missing:

missing cases in switch of type M: B

Notes

The issue is that exhaustive seems to be confused by the (superfluous) type conversion in case M(A): . If it were just case A:, exhaustive correctly reports only B as missing:

missing cases in switch of type M: B

feature: check exhaustiveness of type switches for sealed interfaces

sealed interfaces include a no-op private method so that only package-local types can implement the interface. it is therefore possible to check exhaustiveness statically.

example:

type Payload interface {
    sealed()
}

type FooPayload struct {
    Foo string
}

func (p FooPayload) sealed() {}

type BarPayload struct {
    Bar int
}

func (p BarPayload) sealed() {}

// . . .

switch p := somePayload.(type) {
case FooPayload:
    fmt.Println(p.Foo)
case BarPayload:
    fmt.Printf("%d", p.Bar)
}

see https://github.com/alecthomas/go-check-sumtype , which works but does not use analysis, and thus cannot be used with tools like nogo

Analyzer incorrectly detects comment directives on nested switch statements

Problem

The analyzer seems to associate all comments within a switch statement with the switch statement. This seems to follow naturally from the definition of ast.CommentMap.Filter, which runs ast.Inspect on the specified node and grabs all comment groups from that recursive walk.

Reproduction

Apply the following patch to the ignore comment test case file.

diff --git testdata/src/ignore-comment/ignore_comment.go testdata/src/ignore-comment/ignore_comment.go
index a6c6e5c..6c2b65c 100644
--- testdata/src/ignore-comment/ignore_comment.go
+++ testdata/src/ignore-comment/ignore_comment.go
@@ -61,3 +61,22 @@ func _nested() {
                }
        }
 }
+
+func _reverse_nested() {
+       var d Direction
+
+       // this should report.
+       switch d { // want "^missing cases in switch of type Direction: E, directionInvalid$"
+       case N:
+       case S:
+       case W:
+       default:
+               // this should not report.
+               switch d { //exhaustive:ignore
+               case N:
+               case S:
+               case W:
+               default:
+               }
+       }
+}

Tests will now fail because the ignore directive from the inner switch statement propagates to the outer switch statement.

Resolution

I'm not able to find anything in the AST package that satisfies the use case exactly. Using a direct CommentMap[sw] drops inline comments placed right after the switch token, since that comment is associated with the first case clause according to the AST. The easiest solution seems to be to walk from the switchStmt to the first caseStmt and collect all comments associated with both of them before ending the walk.

A rare edge case here: the switch variable could be an anonymous function invocation that itself contains a comment directive. The walk should therefore ignore any nodes deeper than the case statements of the top level switch.

Allow exhaustiveness checks on structs

Inspired from #35. There are certain structs whose fields you'd want to initialize exhaustively, i.e. whose fields should all be explicitly populated, even if with zero values. Being able to enforce that these structs are fully populated at build time seems like a natural extension of enum case exhaustiveness, with an arguably easier implementation in the most limited case.

Example

Consider a struct representing the named arguments of a function such that all arguments are required except for the last.

//exhaustive:enforce
type DeleteUserArgs struct {
    UserID                        string
    DeletionTimestamp   time.Time
    RemoveFromGroups bool
    //exhaustive:ignore
    Reason                       string
}

func DeleteUser(ctx context.Context, args DeleteUserArgs) error {
// ...
}

func main() {
    args := &DeleteUserArgs {
        UserID:                       "FastNav",
        DeletionTimestamp:  time.Now().Add(-time.Hour),
        // forgot to specify whether user should be removed from groups :( -> exhaustive error
        // however, reason is optional
    }
    DeleteUser(context.Background(), args)
}

The above should result in a validation error at struct initialization because the RemoveFromGroups flag was not present. This is extremely useful for struct mapper functions when new fields are added.

Enforcement Rules

Unlike implicit exhaustiveness enforcement for switch statements on enums, implicit exhaustiveness enforcement is probably unacceptable for struct initialization. It may even be a good idea to require //exhaustive:enforce directives at each point of struct initialization.

Nice to Haves

Bonus points for being able to prove field initialization logic exists either inside or outside the struct literal.

    args := &DeleteUserArgs {
        UserID:                       "FastNav",
        DeletionTimestamp:  time.Now().Add(-time.Hour),
        // reason is optional
    }
    if userHasGroups() {
        args.RemoveFromGroups = true
        args.Reason = "Grouped user violated ToS"
    } else {
        args.RemoveFromGroups = false
    }

Proving definitively that all required fields are initialized is undecidable in general. The most viable approach is probably the one used by e.g. Java to prove that local variables are initialized.

Allow explicit allowlist mode for switch-case checking

Hello there. This package seems very useful for many use cases in our team's projects. However, in a significant number of cases, switch-case is used as a shorthand for something like enum == X || enum == Y || enum == Z, and in most of those cases I don't think exhaustiveness checking is warranted. On the other hand, it would be very useful for situations like enum to enum mappers.

Do you think it would be possible to define a separate mode of enforcement which only looks at explicitly labeled "exhaustive switch" blocks? e.g. a //exhaustive:enforce comment above switches + -enforcement-mode=explicit?

borrow possibly useful ideas from swift's TypeCheckSwitchStmt.cpp file

See if we can borrow any useful ideas related to exhaustiveness checking.

  /// 1. An algorithm for computing the exhaustiveness of a switch statement
  ///    using an algebra of spaces based on Fengyun Liu's
  ///    "A Generic Algorithm for Checking Exhaustivity of Pattern Matching".
  /// 2. An algorithm for computing warnings for pattern matching based on
  ///    Luc Maranget's "Warnings for pattern matching".
  ///
  /// The main algorithm centers around the computation of the difference and
  /// the containment of the "Spaces" given in each case, which reduces the
  /// definition of exhaustiveness to checking if the difference of the space
  /// 'S' of the user's written patterns and the space 'T' of the pattern
  /// condition is empty.

https://github.com/apple/swift/blob/ea142dba029b9c14bd778b04daca646319d53c4e/lib/Sema/TypeCheckSwitchStmt.cpp

Validate `map` keys the same way as `switch` cases

There are places when we need to convert enum to some different value (enum or string, for example).
Go's map is less verbose than switch and is more suitable for those cases.
Let's validate map keys with exhaustive linter.

Example

type Enum int

const (
	A Enum = iota
	B
	C
)

func EnumToString_Switch(e Enum) string {
	switch e {
	case A:
		return "A"
	case B:
		return "B"
	case C:
		return "C"
	}
}

func EnumToString_Map(e Enum) string {
	return map[Enum]string{
		A: "A",
		B: "B",
		C: "C",
	}[e]
}

go test ./... takes 35–45 seconds to complete

% make test
go test -count=1 ./...
?   	github.com/nishanths/exhaustive/cmd/exhaustive	[no test files]
ok  	github.com/nishanths/exhaustive	45.274s
%

Run a profiler and find out why. Is it simply a large number of tests, or does some of the source code have inherently poor performance?

request a new option enforce-enum-types

In project opentelemetry-collector-contrib, we want to use exhaustive linter to confirm the way of using go.opentelemetry.io/collector/pdata/pmetric.MetricType.

related issue:
open-telemetry/opentelemetry-collector-contrib#7532
open-telemetry/opentelemetry-collector-contrib#23242

However in this project, there will be a lot of code provided by third-party vendors. These third-party code may come with their own enum types that may violate the exhaustive lint requirements. But in reality, these third-party enum types are not something we are concerned about.

What we want to achieve is to only check the go.opentelemetry.io/collector/pdata/pmetric.MetricType enum type while ignoring all other enum types. But for now, there is no option we can achieve, so we have to use --explicit-exhaustive-switch option, manually pick out all places in code where pmetric enum type is used, then add //exhaustive:enforce comment. This method is very prone to omissions and errors since it is manual.

So I wonder if we can support another option like enforce-enum-types, which is just like ignore-enum-types, but the behavior is opposite. And maybe this option should come up with explicit-exhaustive-switch. When explicit-exhaustive-switch is enabled and an enum type A is specified in enforce-enum-types, then all switch or map using enum type A and the switch with //exhaustive:enforce will be checked.

Feature Request: Allow specifying that a type isn't an enum

The definition of an enum specified by your docs includes cases where user's might not want exhaustive analysis to be enforced. I encountered this in a case similar to this:

package token

type Token string

...

const SpecialValue Token = "_"

In the example, Token isn't an enum: it has an arbitrary number of valid values. Some code switches on Token looking for certain values, but there is never an expectation to be exhaustive. exhaustive defines Token as an enum with the single value SpecialValue and so throws false positives.

I can add //exhaustive:ignore to downstream locations that consume Token, but that pushes the burden onto users of my type. It also forces me to declare that Token isn't an enum in multiple places.

I would like to be able to tell exhaustive to ignore a type or a constant value. Both of these syntaxes make sense to me:

//exhaustive:ignore
type Token string
//exhaustive:ignore
const SpecialValue Token = "_"

Let me know if this change works for you. If you are amenable, I would be happy to open a PR.

report ineffectual ignore comments

By "ineffectual" we mean a switch statement that is exhaustive but has a currently unnecessary //exhaustive:ignore comment associated with it.

The flag will be disabled by default.

analyzeCaseClauseExpr may not correctly handle "." import enum members in case clause

exhaustive/switch.go

Lines 187 to 223 in 04ce995

func analyzeCaseClauseExpr(e ast.Expr, info *types.Info, samePkg bool, found func(identName string)) {
e = astutil.Unparen(e)
if samePkg {
ident, ok := e.(*ast.Ident)
if !ok {
return
}
found(ident.Name)
return
}
selExpr, ok := e.(*ast.SelectorExpr)
if !ok {
return
}
// Check that X (which is everything except the rightmost *ast.Ident, or
// the Sel) is also an *ast.Ident, and particularly that it is a package
// name identifier.
x := astutil.Unparen(selExpr.X)
ident, ok := x.(*ast.Ident)
if !ok {
return
}
if !isPackageNameIdent(ident, info) {
return
}
// TODO: ident represents a package at this point; check if it represents
// the enum package? (Is this additional check necessary? Wouldn't the type
// checker have already failed if this wasn't the case?)
// This may need additional thought for type aliases, too.
found(selExpr.Sel.Name)
}

panics with reverted switch + external package

package somepkg

type Enum int

const (
	EnumValue1 Enum = iota
	EnumValue2
	EnumValue3
)
package main

import "somepkg"

type nested struct {
	Value somepkg.Enum
}

type ext struct {
	Foo1 nested
	Foo2 nested
	Foo3 nested
	Foo4 nested
}

func check(){
	s := ext{}
	switch somepkg.EnumValue2 {
	case s.Foo1.Value, s.Foo2.Value, s.Foo3.Value:
	}
}

in this situation error appears:

golangci-lint has version v1.41.0 built from (unknown, mod sum: "h1:1kfytWKEn2EPjniwD5xCwWtHAHR57q+vmDX3+VBoJac=") on (unknown)
ERRO [runner] Panic: exhaustive: package "main" (isInitialPkg: true, needAnalyzeSource: true): interface conversion: ast.Expr is *ast.SelectorExpr, not *ast.Ident: goroutine 86804 [running]:
runtime/debug.Stack()
        /usr/lib/go/src/runtime/debug/stack.go:24 +0x65
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func1()
        /home/boris/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_action.go:101 +0x155
panic({0x124f940, 0xc01b19c0f0})
        /usr/lib/go/src/runtime/panic.go:1038 +0x215
github.com/nishanths/exhaustive.missingCasesTextEdit(0x0, 0x1565c10, 0x0, 0xc009ae12c0, 0xc00ad74100, 0xc01b19c000)
        /home/boris/go/pkg/mod/github.com/nishanths/[email protected]/switch.go:395 +0x65c
github.com/nishanths/exhaustive.computeFix(0x2, 0xc00fd57c80, 0x203005, 0xc009ae12c0, 0xc016c7f5f0, 0x6, 0xc01b19c000)
        /home/boris/go/pkg/mod/github.com/nishanths/[email protected]/switch.go:257 +0xc7
github.com/nishanths/exhaustive.reportSwitch(0xc01df17d40, 0xc009ae12c0, 0x0, 0xc00ad74100, 0x2, 0x131c080, 0x0, 0x0)
        /home/boris/go/pkg/mod/github.com/nishanths/[email protected]/switch.go:229 +0xd1
github.com/nishanths/exhaustive.checkSwitchStatements.func1({0x1562668, 0xc009ae12c0}, 0xa, {0xc009e8e300, 0xc0221f1ca0, 0xc01484b810})
        /home/boris/go/pkg/mod/github.com/nishanths/[email protected]/switch.go:148 +0x526
golang.org/x/tools/go/ast/inspector.(*Inspector).WithStack(0xc012674b10, {0xc003393b68, 0x1285160, 0xc015f7b390}, 0xc00a443b78)
        /home/boris/go/pkg/mod/golang.org/x/[email protected]/go/ast/inspector/inspector.go:126 +0x19a
github.com/nishanths/exhaustive.checkSwitchStatements(0xc01df17d40, 0xf1fdba, 0x1be59a0, 0x14)
        /home/boris/go/pkg/mod/github.com/nishanths/[email protected]/switch.go:29 +0x99
github.com/nishanths/exhaustive.run(0xc01df17d40)
        /home/boris/go/pkg/mod/github.com/nishanths/[email protected]/exhaustive.go:181 +0x1a5
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyze(0xc006b309b0)
        /home/boris/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_action.go:187 +0x9c4
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func2()
        /home/boris/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_action.go:105 +0x1d
github.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0xc00187ef50, {0x1381a1e, 0xa}, 0xc00351ff60)
        /home/boris/go/pkg/mod/github.com/golangci/[email protected]/pkg/timeutils/stopwatch.go:111 +0x4a
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe(0xc006b309b0)
        /home/boris/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_action.go:104 +0x85
github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze.func2(0x0)
        /home/boris/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_loadingpackage.go:80 +0x67
created by github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze
        /home/boris/go/pkg/mod/github.com/golangci/[email protected]/pkg/golinters/goanalysis/runner_loadingpackage.go:75 +0x1fd 
WARN [runner] Can't run linter goanalysis_metalinter: goanalysis_metalinter: exhaustive: package "main" (isInitialPkg: true, needAnalyzeSource: true): interface conversion: ast.Expr is *ast.SelectorExpr, not *ast.Ident 

"_" should be excluded

It is quite common to skip entries in iota lists like this so you can explicitly distinguish an unset value:

const (
	_ SortDirection = iota
	Asc
	Desc
)

However, this throws a false positive:

missing cases in switch of type SortDirection: _ (exhaustive)

So i suggest removing the _ when reading the enum.

False positive if dot import is used

Linter complains (false positive) about valid switch if dot import is used.
Example:

package first

type Enum int

const (
	A Enum = iota
	B
	C
)

===

package second

import . "first"

func f(e Enum) {
	switch e {
		case A:
		case B:
		case C:
		default:
	}
}

What if I have include some cases in default?

For example, I have:

type OS int 

const (
Windows OS = iota
Darwin
Linux
BSD
)

and I want to write something like:

case something {
case darwin:
    fmt.Println("MacOS")
default:
   panic("something")
}

However, this will be reported as non-exhaustive.
Should I add something duplicate like below?

case Windows, Linux, BSD:
    panic("something")
case default:
    panic("something")

bug: duplicate case is reported

We are using facebook/ent to generate codes.

It generates the enum field like this:

// AccessControl defines the type for the access_control enum field.
type AccessControl string

// AccessControlAll is the default AccessControl.
const DefaultAccessControl = AccessControlAll

// AccessControl values.
const (
	AccessControlAll       AccessControl = "all"
	AccessControlAny       AccessControl = "any"
	AccessControlAnonymous AccessControl = "anonymous"
)

func (ac AccessControl) String() string {
	return string(ac)
}

// AccessControlValidator is a validator for the "access_control" field enum values. It is called by the builders before save.
func AccessControlValidator(ac AccessControl) error {
	switch ac {
	case AccessControlAll, AccessControlAny, AccessControlAnonymous:
		return nil
	default:
		return fmt.Errorf("permissionpath: invalid enum value for access_control field: %q", ac)
	}
}

And exhaustive reports: missing cases in switch of type AccessControl: DefaultAccessControl, which is a false positive.

Doesn't detect iota enums without type annotation

When enum values are declared using = iota, exhaustive is unable to detect it without the type annotation.

For example

// not detected in switches
type IotaEnumNoTypes uint8
const (
	EnumOneNoTypes = iota
	EnumTwoNoTypes
)
// gets detected
type IotaEnum uint8
const (
	EnumOne IotaEnum = iota
	EnumTwo
)

NOTE the EnumOne IotaEnum = iota

Can run exhastive against this sample demo file to reproduce
package abc

import "fmt"

type IotaEnum uint8

const (
	EnumOne IotaEnum = iota
	EnumTwo
)

func iotaEnumToStr(enum IotaEnum) string {
	switch enum {
	case EnumOne:
		return "one"
	}
	return "none"
}

type IotaEnumWithTypes uint8

const (
	EnumOneWithType IotaEnumWithTypes = iota
	EnumTwoWithType IotaEnumWithTypes = 1
)

func iotaEnumWithTypeToStr(enum IotaEnumWithTypes) string {
	switch enum {
	case EnumOneWithType:
		return "one"
	}
	return "none"
}

type IotaEnumNoTypes uint8

const (
	EnumOneNoTypes = iota
	EnumTwoNoTypes
)

func iotaEnumNoTypeToStr(enum IotaEnumNoTypes) string {
	switch enum {
	case EnumOneNoTypes:
		return "one"
	}
	return "none"
}

func main() {
	fmt.Println(iotaEnumToStr(EnumOne))
	fmt.Println(iotaEnumWithTypeToStr(EnumOneWithType))
	fmt.Println(iotaEnumNoTypeToStr(EnumOneNoTypes))
}

Output:

main.go:13:2: missing cases in switch of type abc.IotaEnum: abc.EnumTwo
main.go:28:2: missing cases in switch of type abc.IotaEnumWithTypes: abc.EnumTwoWithType

hardcoded ignore list of certain std lib types that can never be enum types

For example, exhaustive considers the time.Duration type in the standard library to be an enum type. It considers the constants time.Nanosecond, time.Millisecond, etc. to be the enum members. However the type isn't really an enum type, despite it appearing so based on exhaustive's heuristics.

Proposal: Add such types to a hardcoded ignore list in exhaustive as types that can never be enums.

Candidate types so far:

time.Duration
...
# TODO(nishanths) read through the standard library and add any other similar types

Follow-up note: All that said, though time.Duration is considered an enum by exhaustive, this fact doesn't generally lead to false positive diagnostics, because code usually doesn't switch on time.Duration values in the following fashion.

//  The following style of switch on a time.Duration is not common.
switch d {
    case 2 * time.Second:
    case 5 * time.Second:
    case 10 * time.Second:
    case time.Hour:
}

The lack of such switch statements using a time.Duration value means that exhaustive doesn't produce false positive diagnostics.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.