diff --git a/.golangci.reference.yml b/.golangci.reference.yml index 8f337f182f2a..5b1a77ecef94 100644 --- a/.golangci.reference.yml +++ b/.golangci.reference.yml @@ -1159,9 +1159,9 @@ linters-settings: require-specific: true nonamedreturns: - # Do not complain about named error, if it is assigned inside defer. + # Report named error if it is assigned inside defer. # Default: false - allow-error-in-defer: true + report-error-in-defer: true paralleltest: # Ignore missing calls to `t.Parallel()` and only report incorrect uses of it. diff --git a/pkg/config/linters_settings.go b/pkg/config/linters_settings.go index 3b5ceddf32de..c91f4f057e7d 100644 --- a/pkg/config/linters_settings.go +++ b/pkg/config/linters_settings.go @@ -486,7 +486,7 @@ type NoLintLintSettings struct { } type NoNamedReturnsSettings struct { - AllowErrorInDefer bool `mapstructure:"allow-error-in-defer"` + ReportErrorInDefer bool `mapstructure:"report-error-in-defer"` } type ParallelTestSettings struct { IgnoreMissing bool `mapstructure:"ignore-missing"` diff --git a/pkg/golinters/nonamedreturns.go b/pkg/golinters/nonamedreturns.go index a8166f81672b..3dff2f75909f 100644 --- a/pkg/golinters/nonamedreturns.go +++ b/pkg/golinters/nonamedreturns.go @@ -15,7 +15,7 @@ func NewNoNamedReturns(settings *config.NoNamedReturnsSettings) *goanalysis.Lint if settings != nil { cfg = map[string]map[string]interface{}{ a.Name: { - analyzer.FlagAllowErrorInDefer: settings.AllowErrorInDefer, + analyzer.FlagReportErrorInDefer: settings.ReportErrorInDefer, }, } } @@ -25,5 +25,5 @@ func NewNoNamedReturns(settings *config.NoNamedReturnsSettings) *goanalysis.Lint a.Doc, []*analysis.Analyzer{a}, cfg, - ).WithLoadMode(goanalysis.LoadModeSyntax) + ).WithLoadMode(goanalysis.LoadModeTypesInfo) } diff --git a/pkg/lint/lintersdb/manager.go b/pkg/lint/lintersdb/manager.go index 58db15a6075c..e6c069b7f56f 100644 --- a/pkg/lint/lintersdb/manager.go +++ b/pkg/lint/lintersdb/manager.go @@ -627,6 +627,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config { linter.NewConfig(golinters.NewNoNamedReturns(noNamedReturnsCfg)). WithSince("v1.46.0"). + WithLoadForGoAnalysis(). WithPresets(linter.PresetStyle). WithURL("https://github.com/firefart/nonamedreturns"), diff --git a/test/testdata/configs/nonamedreturns.yml b/test/testdata/configs/nonamedreturns.yml index b73a2c1cb645..8976df8af9fa 100644 --- a/test/testdata/configs/nonamedreturns.yml +++ b/test/testdata/configs/nonamedreturns.yml @@ -1,3 +1,3 @@ linters-settings: nonamedreturns: - allow-error-in-defer: true + report-error-in-defer: true diff --git a/test/testdata/nonamedreturns.go b/test/testdata/nonamedreturns.go index 9b20eaf22d07..c2afbd27ed7d 100644 --- a/test/testdata/nonamedreturns.go +++ b/test/testdata/nonamedreturns.go @@ -1,124 +1,281 @@ // args: -Enonamedreturns package testdata -import "fmt" +import "errors" -type asdf struct { - test string +func simple() (err error) { + defer func() { + err = nil + }() + return } -func noParams() { +func twoReturnParams() (i int, err error) { // ERROR `named return "i" with type "int" found` + defer func() { + i = 0 + err = nil + }() return } -var c = func() { +func allUnderscoresExceptError() (_ int, err error) { + defer func() { + err = nil + }() return } -var d = func() error { - return nil +func customName() (myName error) { + defer func() { + myName = nil + }() + return } -var e = func() (err error) { // ERROR `named return "err" with type "error" found` - err = nil +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 } -var e2 = func() (_ error) { +func shadowVariable() (err error) { // ERROR `named return "err" with type "error" found` + defer func() { + err := errors.New("xxx") + _ = err + }() return } -func deferWithError() (err error) { // ERROR `named return "err" with type "error" found` +func shadowVariableButAssign() (err error) { defer func() { - err = nil // use flag to allow this + { + err := errors.New("xxx") + _ = err + } + err = nil }() return } -var ( - f = func() { - return - } +func shadowVariable2() (err error) { // ERROR `named return "err" with type "error" found` + defer func() { + a, err := doSomething() + _ = a + _ = err + }() + return +} - g = func() error { - return nil - } +type errorAlias = error - h = func() (err error) { // ERROR `named return "err" with type "error" found` +func errorAliasIsTheSame() (err errorAlias) { + defer func() { err = nil - return - } + }() + return +} + +type myError error // linter doesn't check underlying type (yet?) + +func customTypeWithErrorUnderline() (err myError) { // ERROR `named return "err" with type "myError" found` + defer func() { + err = nil + }() + return +} + +type myError2 interface{ error } // linter doesn't check interfaces + +func customTypeWithTheSameInterface() (err myError2) { // ERROR `named return "err" with type "myError2" found` + defer func() { + err = nil + }() + return +} + +var _ error = myError3{} + +type myError3 struct{} // linter doesn't check interfaces + +func (m myError3) Error() string { return "" } + +func customTypeImplementingErrorInterface() (err myError3) { // ERROR `named return "err" with type "myError3" found` + defer func() { + err = struct{}{} + }() + return +} - h2 = func() (_ error) { +func shadowErrorType() { + type error interface { // linter understands that this is not built-in error, even if it has the same name + Error() string + } + do := func() (err error) { // ERROR `named return "err" with type "error" found` + defer func() { + err = nil + }() return } -) + do() +} -// this should not match as the implementation does not need named parameters (see below) -type funcDefintion func(arg1, arg2 interface{}) (num int, err error) +func notTheLast() (err error, _ int) { + defer func() { + err = nil + }() + return +} -func funcDefintionImpl(arg1, arg2 interface{}) (int, error) { - return 0, nil +func twoErrorsCombined() (err1, err2 error) { + defer func() { + err1 = nil + err2 = nil + }() + return } -func funcDefintionImpl2(arg1, arg2 interface{}) (num int, err error) { // ERROR `named return "num" with type "int" found` - return 0, nil +func twoErrorsSeparated() (err1 error, err2 error) { + defer func() { + err1 = nil + err2 = nil + }() + return } -func funcDefintionImpl3(arg1, arg2 interface{}) (num int, _ error) { // ERROR `named return "num" with type "int" found` - return 0, nil +func errorSlice() (err []error) { // ERROR `named return "err" with type "\[\]error" found` + defer func() { + err = nil + }() + return } -func funcDefintionImpl4(arg1, arg2 interface{}) (_ int, _ error) { - return 0, nil +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 } -var funcVar = func() (msg string) { // ERROR `named return "msg" with type "string" found` - msg = "c" - return msg +func uberMultierr() (err error) { // ERROR `named return "err" with type "error" found` + defer func() { + multierrAppendInto(&err, nil) // linter doesn't allow it (yet?) + }() + return } -var funcVar2 = func() (_ string) { - msg := "c" - return msg +func deferInDefer() (err error) { + defer func() { + defer func() { + err = nil + }() + }() + return } -func test() { - a := funcVar() - _ = a +func twoDefers() (err error) { + defer func() {}() + defer func() { + err = nil + }() + return +} - var function funcDefintion - function = funcDefintionImpl - i, err := function("", "") - _ = i - _ = err - function = funcDefintionImpl2 - i, err = function("", "") - _ = i - _ = err +func callFunction() (err error) { + defer func() { + _, err = doSomething() + }() + return } -func good(i string) string { - return i +func callFunction2() (err error) { + defer func() { + var a int + a, err = doSomething() + _ = a + }() + return } -func bad(i string, a, b int) (ret1 string, ret2 interface{}, ret3, ret4 int, ret5 asdf) { // ERROR `named return "ret1" with type "string" found` - x := "dummy" - return fmt.Sprintf("%s", x), nil, 1, 2, asdf{} +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 } -func bad2() (msg string, err error) { // ERROR `named return "msg" with type "string" found` - msg = "" - err = nil +var goodFuncLiteral = func() (err error) { + defer func() { + err = nil + }() return } -func myLog(format string, args ...interface{}) { +var badFuncLiteral = func() (err error) { // ERROR `named return "err" with type "error" found` + defer func() { + _ = err + }() return } -type obj struct{} +func funcLiteralInsideFunc() error { + do := func() (err error) { + defer func() { + err = nil + }() + return + } + return do() +} -func (o *obj) func1() (err error) { return nil } // ERROR `named return "err" with type "error" found` +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 (o *obj) func2() (_ error) { return nil } +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 diff --git a/test/testdata/nonamedreturns_custom.go b/test/testdata/nonamedreturns_custom.go index 0e0ceacb99b6..c9289e1e9960 100644 --- a/test/testdata/nonamedreturns_custom.go +++ b/test/testdata/nonamedreturns_custom.go @@ -2,224 +2,124 @@ // 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 -} +import "fmt" -func allUnderscoresExceptError() (_ int, err error) { - defer func() { - err = nil - }() - return +type asdf struct { + test string } -func customName() (myName error) { - defer func() { - myName = nil - }() +func noParams() { 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: - } - }() +var c = func() { 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 +var d = func() error { + return nil } -func shadowVariable2() (err error) { - defer func() { - a, err := doSomething() // linter doesn't understand that this is different variable (yet?) - _ = a - _ = err - }() +var e = func() (err error) { // ERROR `named return "err" with type "error" found` + err = nil 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 - }() +var e2 = func() (_ error) { return } -func notTheLast() (err error, _ int) { +func deferWithError() (err error) { // ERROR `named return "err" with type "error" found` defer func() { - err = nil + err = nil // use flag to allow this }() return } -func twoErrorsCombined() (err1, err2 error) { - defer func() { - err1 = nil - err2 = nil - }() - return -} +var ( + f = func() { + return + } -func twoErrorsSeparated() (err1 error, err2 error) { - defer func() { - err1 = nil - err2 = nil - }() - return -} + g = func() error { + return nil + } -func errorSlice() (err []error) { // ERROR `named return "err" with type "\[\]error" found` - defer func() { + h = func() (err error) { // ERROR `named return "err" with type "error" found` err = nil - }() - return -} + return + } -func deferWithVariable() (err error) { // ERROR `named return "err" with type "error" found` - f := func() { - err = nil + h2 = func() (_ error) { + return } - 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 -} +// this should not match as the implementation does not need named parameters (see below) +type funcDefintion func(arg1, arg2 interface{}) (num int, err error) -func deferInDefer() (err error) { - defer func() { - defer func() { - err = nil - }() - }() - return +func funcDefintionImpl(arg1, arg2 interface{}) (int, error) { + return 0, nil } -func twoDefers() (err error) { - defer func() {}() - defer func() { - err = nil - }() - return +func funcDefintionImpl2(arg1, arg2 interface{}) (num int, err error) { // ERROR `named return "num" with type "int" found` + return 0, nil } -func callFunction() (err error) { - defer func() { - _, err = doSomething() - }() - return +func funcDefintionImpl3(arg1, arg2 interface{}) (num int, _ error) { // ERROR `named return "num" with type "int" found` + return 0, nil } -func callFunction2() (err error) { - defer func() { - var a int - a, err = doSomething() - _ = a - }() - return +func funcDefintionImpl4(arg1, arg2 interface{}) (_ int, _ error) { + return 0, nil } -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 funcVar = func() (msg string) { // ERROR `named return "msg" with type "string" found` + msg = "c" + return msg } -var goodFuncLiteral = func() (err error) { - defer func() { - err = nil - }() - return +var funcVar2 = func() (_ string) { + msg := "c" + return msg } -var badFuncLiteral = func() (err error) { // ERROR `named return "err" with type "error" found` - defer func() { - _ = err - }() - return +func test() { + a := funcVar() + _ = a + + var function funcDefintion + function = funcDefintionImpl + i, err := function("", "") + _ = i + _ = err + function = funcDefintionImpl2 + i, err = function("", "") + _ = i + _ = err } -func funcLiteralInsideFunc() error { - do := func() (err error) { - defer func() { - err = nil - }() - return - } - return do() +func good(i string) string { + return i } -type x struct{} +func bad(i string, a, b int) (ret1 string, ret2 interface{}, ret3, ret4 int, ret5 asdf) { // ERROR `named return "ret1" with type "string" found` + x := "dummy" + return fmt.Sprintf("%s", x), nil, 1, 2, asdf{} +} -func (x) goodMethod() (err error) { - defer func() { - err = nil - }() +func bad2() (msg string, err error) { // ERROR `named return "msg" with type "string" found` + msg = "" + err = nil return } -func (x) badMethod() (err error) { // ERROR `named return "err" with type "error" found` - defer func() { - _ = err - }() +func myLog(format string, args ...interface{}) { 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 +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 }