diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 0f4d810a1c6d..daf3c5628d05 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -1154,6 +1154,11 @@ linters-settings: # Default: false require-specific: true + nonamedreturns: + # Do not complain about named error, if it is assigned inside defer. + # Default: false + allow-error-in-defer: true + paralleltest: # Ignore missing calls to `t.Parallel()` and only report incorrect uses of it. # Default: false diff --git a/go.mod b/go.mod index b1990b3d4b07..4ccf9c1dc269 100644 --- a/go.mod +++ b/go.mod @@ -24,7 +24,7 @@ require ( github.com/denis-tingaikin/go-header v0.4.3 github.com/esimonov/ifshort v1.0.4 github.com/fatih/color v1.13.0 - github.com/firefart/nonamedreturns v1.0.1 + github.com/firefart/nonamedreturns v1.0.2 github.com/fzipp/gocyclo v0.5.1 github.com/go-critic/go-critic v0.6.3 github.com/go-xmlfmt/xmlfmt v0.0.0-20191208150333-d5b6f63a941b diff --git a/go.sum b/go.sum index 1a4281bfd41b..6937633f30ad 100644 --- a/go.sum +++ b/go.sum @@ -194,8 +194,8 @@ github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w= github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk= github.com/fatih/structtag v1.2.0 h1:/OdNE99OxoI/PqaW/SuSK9uxxT3f/tcSZgon/ssNSx4= github.com/fatih/structtag v1.2.0/go.mod h1:mBJUNpUnHmRKrKlQQlmCrh5PuhftFbNv8Ys4/aAZl94= -github.com/firefart/nonamedreturns v1.0.1 h1:fSvcq6ZpK/uBAgJEGMvzErlzyM4NELLqqdTofVjVNag= -github.com/firefart/nonamedreturns v1.0.1/go.mod h1:D3dpIBojGGNh5UfElmwPu73SwDCm+VKhHYqwlNOk2uQ= +github.com/firefart/nonamedreturns v1.0.2 h1:TJ4bRW6KQaNo5t4JVQdjRAOAB7LTxDvkN16c05XtXaA= +github.com/firefart/nonamedreturns v1.0.2/go.mod h1:dLEDwtlLtW+0788N0RCymzvtvfanAFP8V1ZTkY0HwD8= github.com/frankban/quicktest v1.14.3 h1:FJKSZTDHjyhriyC81FLQ0LY93eSai0ZyR/ZIkd3ZUKE= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 32c296ff55ca..3b5ceddf32de 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -158,6 +158,7 @@ type LintersSettings struct { NilNil NilNilSettings Nlreturn NlreturnSettings NoLintLint NoLintLintSettings + NoNamedReturns NoNamedReturnsSettings ParallelTest ParallelTestSettings Prealloc PreallocSettings Predeclared PredeclaredSettings @@ -484,6 +485,9 @@ type NoLintLintSettings struct { AllowUnused bool `mapstructure:"allow-unused"` } +type NoNamedReturnsSettings struct { + AllowErrorInDefer bool `mapstructure:"allow-error-in-defer"` +} type ParallelTestSettings struct { IgnoreMissing bool `mapstructure:"ignore-missing"` } diff --git a/pkg/golinters/nonamedreturns.go b/pkg/golinters/nonamedreturns.go index 8c943d83eeb9..a8166f81672b 100644 --- a/pkg/golinters/nonamedreturns.go +++ b/pkg/golinters/nonamedreturns.go @@ -4,14 +4,26 @@ import ( "github.com/firefart/nonamedreturns/analyzer" "golang.org/x/tools/go/analysis" + "github.com/golangci/golangci-lint/pkg/config" "github.com/golangci/golangci-lint/pkg/golinters/goanalysis" ) -func NewNoNamedReturns() *goanalysis.Linter { +func NewNoNamedReturns(settings *config.NoNamedReturnsSettings) *goanalysis.Linter { + a := analyzer.Analyzer + + var cfg map[string]map[string]interface{} + if settings != nil { + cfg = map[string]map[string]interface{}{ + a.Name: { + analyzer.FlagAllowErrorInDefer: settings.AllowErrorInDefer, + }, + } + } + return goanalysis.NewLinter( - "nonamedreturns", - "Reports all named returns", - []*analysis.Analyzer{analyzer.Analyzer}, - nil, + a.Name, + a.Doc, + []*analysis.Analyzer{a}, + cfg, ).WithLoadMode(goanalysis.LoadModeSyntax) } diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 889e490fda29..58db15a6075c 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -147,6 +147,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { nilNilCfg *config.NilNilSettings nlreturnCfg *config.NlreturnSettings noLintLintCfg *config.NoLintLintSettings + noNamedReturnsCfg *config.NoNamedReturnsSettings parallelTestCfg *config.ParallelTestSettings preallocCfg *config.PreallocSettings predeclaredCfg *config.PredeclaredSettings @@ -216,6 +217,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { nilNilCfg = &m.cfg.LintersSettings.NilNil nlreturnCfg = &m.cfg.LintersSettings.Nlreturn noLintLintCfg = &m.cfg.LintersSettings.NoLintLint + noNamedReturnsCfg = &m.cfg.LintersSettings.NoNamedReturns preallocCfg = &m.cfg.LintersSettings.Prealloc parallelTestCfg = &m.cfg.LintersSettings.ParallelTest predeclaredCfg = &m.cfg.LintersSettings.Predeclared @@ -623,7 +625,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { WithURL("https://github.com/sonatard/noctx"). WithNoopFallback(m.cfg), - linter.NewConfig(golinters.NewNoNamedReturns()). + linter.NewConfig(golinters.NewNoNamedReturns(noNamedReturnsCfg)). WithSince("v1.46.0"). WithPresets(linter.PresetStyle). WithURL("https://github.com/firefart/nonamedreturns"), diff --git a/test/testdata/configs/nonamedreturns.yml b/test/testdata/configs/nonamedreturns.yml new file mode 100644 index 000000000000..b73a2c1cb645 --- /dev/null +++ b/test/testdata/configs/nonamedreturns.yml @@ -0,0 +1,3 @@ +linters-settings: + nonamedreturns: + allow-error-in-defer: true diff --git a/test/testdata/nonamedreturns.go b/test/testdata/nonamedreturns.go index 5850281db892..9b20eaf22d07 100644 --- a/test/testdata/nonamedreturns.go +++ b/test/testdata/nonamedreturns.go @@ -1,4 +1,4 @@ -//args: -Enonamedreturns +// args: -Enonamedreturns package testdata import "fmt" @@ -24,6 +24,17 @@ var e = func() (err error) { // ERROR `named return "err" with type "error" foun return } +var e2 = func() (_ error) { + return +} + +func deferWithError() (err error) { // ERROR `named return "err" with type "error" found` + defer func() { + err = nil // use flag to allow this + }() + return +} + var ( f = func() { return @@ -37,6 +48,10 @@ var ( err = nil return } + + h2 = func() (_ error) { + return + } ) // this should not match as the implementation does not need named parameters (see below) @@ -50,11 +65,24 @@ func funcDefintionImpl2(arg1, arg2 interface{}) (num int, err error) { // ERROR return 0, nil } +func funcDefintionImpl3(arg1, arg2 interface{}) (num int, _ error) { // ERROR `named return "num" with type "int" found` + return 0, nil +} + +func funcDefintionImpl4(arg1, arg2 interface{}) (_ int, _ error) { + return 0, nil +} + var funcVar = func() (msg string) { // ERROR `named return "msg" with type "string" found` msg = "c" return msg } +var funcVar2 = func() (_ string) { + msg := "c" + return msg +} + func test() { a := funcVar() _ = a @@ -92,3 +120,5 @@ func myLog(format string, args ...interface{}) { type obj struct{} func (o *obj) func1() (err error) { return nil } // ERROR `named return "err" with type "error" found` + +func (o *obj) func2() (_ error) { return nil } diff --git a/test/testdata/nonamedreturns_custom.go b/test/testdata/nonamedreturns_custom.go new file mode 100644 index 000000000000..0e0ceacb99b6 --- /dev/null +++ b/test/testdata/nonamedreturns_custom.go @@ -0,0 +1,225 @@ +// args: -Enonamedreturns +// config_path: testdata/configs/nonamedreturns.yml +package testdata + +func simple() (err error) { + defer func() { + err = nil + }() + return +} + +func twoReturnParams() (i int, err error) { // ERROR `named return "i" with type "int" found` + defer func() { + i = 0 + err = nil + }() + return +} + +func allUnderscoresExceptError() (_ int, err error) { + defer func() { + err = nil + }() + return +} + +func customName() (myName error) { + defer func() { + myName = nil + }() + return +} + +func errorIsNoAssigned() (err error) { // ERROR `named return "err" with type "error" found` + defer func() { + _ = err + processError(err) + if err == nil { + } + switch err { + case nil: + default: + } + }() + return +} + +func shadowVariable() (err error) { + defer func() { + err := 123 // linter doesn't understand that this is different variable (even if different type) (yet?) + _ = err + }() + return +} + +func shadowVariable2() (err error) { + defer func() { + a, err := doSomething() // linter doesn't understand that this is different variable (yet?) + _ = a + _ = err + }() + return +} + +type myError = error // linter doesn't understand that this is the same type (yet?) + +func customType() (err myError) { // ERROR `named return "err" with type "myError" found` + defer func() { + err = nil + }() + return +} + +func notTheLast() (err error, _ int) { + defer func() { + err = nil + }() + return +} + +func twoErrorsCombined() (err1, err2 error) { + defer func() { + err1 = nil + err2 = nil + }() + return +} + +func twoErrorsSeparated() (err1 error, err2 error) { + defer func() { + err1 = nil + err2 = nil + }() + return +} + +func errorSlice() (err []error) { // ERROR `named return "err" with type "\[\]error" found` + defer func() { + err = nil + }() + return +} + +func deferWithVariable() (err error) { // ERROR `named return "err" with type "error" found` + f := func() { + err = nil + } + defer f() // linter can't catch closure passed via variable (yet?) + return +} + +func uberMultierr() (err error) { // ERROR `named return "err" with type "error" found` + defer func() { + multierrAppendInto(&err, nil) // linter doesn't allow it (yet?) + }() + return +} + +func deferInDefer() (err error) { + defer func() { + defer func() { + err = nil + }() + }() + return +} + +func twoDefers() (err error) { + defer func() {}() + defer func() { + err = nil + }() + return +} + +func callFunction() (err error) { + defer func() { + _, err = doSomething() + }() + return +} + +func callFunction2() (err error) { + defer func() { + var a int + a, err = doSomething() + _ = a + }() + return +} + +func deepInside() (err error) { + if true { + switch true { + case false: + for i := 0; i < 10; i++ { + go func() { + select { + default: + defer func() { + if true { + switch true { + case false: + for j := 0; j < 10; j++ { + go func() { + select { + default: + err = nil + } + }() + } + } + } + }() + } + }() + } + } + } + return +} + +var goodFuncLiteral = func() (err error) { + defer func() { + err = nil + }() + return +} + +var badFuncLiteral = func() (err error) { // ERROR `named return "err" with type "error" found` + defer func() { + _ = err + }() + return +} + +func funcLiteralInsideFunc() error { + do := func() (err error) { + defer func() { + err = nil + }() + return + } + return do() +} + +type x struct{} + +func (x) goodMethod() (err error) { + defer func() { + err = nil + }() + return +} + +func (x) badMethod() (err error) { // ERROR `named return "err" with type "error" found` + defer func() { + _ = err + }() + return +} + +func processError(error) {} +func doSomething() (int, error) { return 10, nil } +func multierrAppendInto(*error, error) bool { return false } // https://pkg.go.dev/go.uber.org/multierr#AppendInto