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

add exhaustive linter #1166

Merged
merged 12 commits into from May 29, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion .golangci.example.yml
Expand Up @@ -98,6 +98,11 @@ linters-settings:
# path to a file containing a list of functions to exclude from checking
# see https://github.com/kisielk/errcheck#excluding-functions for details
exclude: /path/to/file.txt
exhaustive:
# indicates that switch statements are to be considered exhaustive if a
# 'default' case is present, even if all enum members aren't listed in the
# switch
default-signifies-exhaustive: false
funlen:
lines: 60
statements: 40
Expand Down Expand Up @@ -174,7 +179,7 @@ linters-settings:
modules: # List of blocked modules
# - github.com/uudashr/go-module: # Blocked module
# recommendations: # Recommended modules that should be used instead (Optional)
# - golang.org/x/mod
# - golang.org/x/mod
# reason: "`mod` is the official go.mod parser library." # Reason why the recommended module should be used (Optional)
versions: # List of blocked module version constraints
# - github.com/mitchellh/go-homedir: # Blocked module with version constraint
Expand Down
3 changes: 3 additions & 0 deletions .golangci.yml
Expand Up @@ -9,6 +9,8 @@ linters-settings:
- github.com/sirupsen/logrus: "logging is allowed only by logutils.Log"
dupl:
threshold: 100
exhaustive:
default-signifies-exhaustive: false
funlen:
lines: 100
statements: 50
Expand Down Expand Up @@ -104,6 +106,7 @@ linters:

# don't enable:
# - asciicheck
# - exhaustive
Copy link
Member

@jirfag jirfag May 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not enable it? I think dogfooding linters is important

Copy link
Contributor Author

@nishanths nishanths May 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, I can enable it.

I did not enable it originally because I was unsure if users would find the default behavior of the analyzer to be too strict. (The default behavior of the analyzer is to point out missing enum members in the switch even if a default case is present. That might actually be useful though — otherwise we wouldn't have discovered the missing reflect.Complex64 etc. cases above.)

# - gochecknoglobals
# - gocognit
# - godot
Expand Down
3 changes: 2 additions & 1 deletion go.mod
Expand Up @@ -33,6 +33,7 @@ require (
github.com/mitchellh/go-homedir v1.1.0
github.com/mitchellh/go-ps v1.0.0
github.com/nakabonne/nestif v0.3.0
github.com/nishanths/exhaustive v0.0.0-20200525081945-8e46705b6132
github.com/pkg/errors v0.9.1
github.com/ryancurrah/gomodguard v1.1.0
github.com/securego/gosec/v2 v2.3.0
Expand All @@ -51,7 +52,7 @@ require (
github.com/ultraware/whitespace v0.0.4
github.com/uudashr/gocognit v1.0.1
github.com/valyala/quicktemplate v1.5.0
golang.org/x/tools v0.0.0-20200502202811-ed308ab3e770
golang.org/x/tools v0.0.0-20200519015757-0d0afa43d58a
gopkg.in/yaml.v2 v2.3.0
honnef.co/go/tools v0.0.1-2020.1.4
mvdan.cc/interfacer v0.0.0-20180901003855-c20040233aed
Expand Down
4 changes: 4 additions & 0 deletions go.sum
Expand Up @@ -267,6 +267,8 @@ github.com/nbutton23/zxcvbn-go v0.0.0-20180912185939-ae427f1e4c1d h1:AREM5mwr4u1
github.com/nbutton23/zxcvbn-go v0.0.0-20180912185939-ae427f1e4c1d/go.mod h1:o96djdrsSGy3AWPyBgZMAGfxZNfgntdJG+11KU4QvbU=
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs=
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
github.com/nishanths/exhaustive v0.0.0-20200525081945-8e46705b6132 h1:NjznefjSrral0MiR4KlB41io/d3OklvhcgQUdfZTqJE=
github.com/nishanths/exhaustive v0.0.0-20200525081945-8e46705b6132/go.mod h1:wBEpHwM2OdmeNpdCvRPUlkEbBuaFmcK4Wv8Q7FuGW3c=
github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U=
github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
github.com/onsi/ginkgo v1.12.0 h1:Iw5WCbBcaAAd0fpRb1c9r5YCylv4XDoCSigm1zLevwU=
Expand Down Expand Up @@ -511,6 +513,8 @@ golang.org/x/tools v0.0.0-20200502202811-ed308ab3e770 h1:M9Fif0OxNji8w+HvmhVQ8KJ
golang.org/x/tools v0.0.0-20200502202811-ed308ab3e770 h1:M9Fif0OxNji8w+HvmhVQ8KJtiZOsjU9RgslJGhn95XE=
golang.org/x/tools v0.0.0-20200502202811-ed308ab3e770 h1:M9Fif0OxNji8w+HvmhVQ8KJtiZOsjU9RgslJGhn95XE=
golang.org/x/tools v0.0.0-20200502202811-ed308ab3e770/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
golang.org/x/tools v0.0.0-20200519015757-0d0afa43d58a h1:gILuVKC+ZPD6g/tj6zBOdnOH1ZHI0zZ86+KLMogc6/s=
golang.org/x/tools v0.0.0-20200519015757-0d0afa43d58a/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7 h1:9zdDQZ7Thm29KFXgAX/+yaf3eVbP7djjWp/dXAppNCc=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898 h1:/atklqdjdhuosWIl6AIbOeHJjicWYPqR9bpxqxYG2pA=
Expand Down
8 changes: 8 additions & 0 deletions pkg/config/config.go
Expand Up @@ -242,6 +242,7 @@ type LintersSettings struct {
Testpackage TestpackageSettings
Nestif NestifSettings
NoLintLint NoLintLintSettings
Exhaustive ExhaustiveSettings

Custom map[string]CustomLinterSettings
}
Expand Down Expand Up @@ -339,6 +340,10 @@ type NestifSettings struct {
MinComplexity int `mapstructure:"min-complexity"`
}

type ExhaustiveSettings struct {
DefaultSignifiesExhaustive bool `mapstructure:"default-signifies-exhaustive"`
}

var defaultLintersSettings = LintersSettings{
Lll: LllSettings{
LineLength: 120,
Expand Down Expand Up @@ -389,6 +394,9 @@ var defaultLintersSettings = LintersSettings{
Nestif: NestifSettings{
MinComplexity: 5,
},
Exhaustive: ExhaustiveSettings{
DefaultSignifiesExhaustive: false,
},
}

type CustomLinterSettings struct {
Expand Down
25 changes: 25 additions & 0 deletions pkg/golinters/exhaustive.go
@@ -0,0 +1,25 @@
package golinters

import (
"github.com/nishanths/exhaustive"
"golang.org/x/tools/go/analysis"

"github.com/golangci/golangci-lint/pkg/config"
"github.com/golangci/golangci-lint/pkg/golinters/goanalysis"
)

func NewExhaustive(settings *config.ExhaustiveSettings) *goanalysis.Linter {
a := exhaustive.Analyzer

var cfg map[string]map[string]interface{}
if settings != nil {
cfg = map[string]map[string]interface{}{
a.Name: {
exhaustive.DefaultSignifiesExhaustiveFlag: settings.DefaultSignifiesExhaustive,
},
}
}

return goanalysis.NewLinter(a.Name, a.Doc, []*analysis.Analyzer{a}, cfg).
WithLoadMode(goanalysis.LoadModeTypesInfo)
}
3 changes: 2 additions & 1 deletion pkg/golinters/goanalysis/runner.go
Expand Up @@ -935,7 +935,8 @@ func sizeOfReflectValueTreeBytes(rv reflect.Value, visitedPtrs map[uintptr]struc
return rv.Len()
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64,
reflect.Uintptr, reflect.Bool, reflect.Float32, reflect.Float64, reflect.UnsafePointer:
reflect.Uintptr, reflect.Bool, reflect.Float32, reflect.Float64,
reflect.Complex64, reflect.Complex128, reflect.Func, reflect.UnsafePointer:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These missing cases

reflect.Complex64, reflect.Complex128, reflect.Func

were reported by the exhaustive linter when I temporarily enabled it for a run.

return int(rv.Type().Size())
case reflect.Invalid:
return 0
Expand Down
6 changes: 6 additions & 0 deletions pkg/lint/lintersdb/manager.go
Expand Up @@ -87,9 +87,11 @@ func enableLinterConfigs(lcs []*linter.Config, isEnabled func(lc *linter.Config)
func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
var govetCfg *config.GovetSettings
var testpackageCfg *config.TestpackageSettings
var exhaustiveCfg *config.ExhaustiveSettings
if m.cfg != nil {
govetCfg = &m.cfg.LintersSettings.Govet
testpackageCfg = &m.cfg.LintersSettings.Testpackage
exhaustiveCfg = &m.cfg.LintersSettings.Exhaustive
}
const megacheckName = "megacheck"
lcs := []*linter.Config{
Expand Down Expand Up @@ -275,6 +277,10 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
linter.NewConfig(golinters.NewExportLoopRef()).
WithPresets(linter.PresetBugs).
WithURL("https://github.com/kyoh86/exportloopref"),
linter.NewConfig(golinters.NewExhaustive(exhaustiveCfg)).
WithPresets(linter.PresetBugs).
WithLoadForGoAnalysis().
WithURL("https://github.com/nishanths/exhaustive"),
jirfag marked this conversation as resolved.
Show resolved Hide resolved
// nolintlint must be last because it looks at the results of all the previous linters for unused nolint directives
linter.NewConfig(golinters.NewNoLintLint()).
WithPresets(linter.PresetStyle).
Expand Down
17 changes: 17 additions & 0 deletions test/testdata/exhaustive.go
@@ -0,0 +1,17 @@
//args: -Eexhaustive

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should another test case including enabling DefaultSignifiesExhaustive be added?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that would help. I'll add one.

package testdata

type Direction int

const (
North Direction = iota
East
South
West
)

func processDirection(d Direction) {
switch d { // ERROR "missing cases in switch of type Direction: East, West"
case North, South:
}
}