Skip to content

nonamedreturns: add setting allow-error-in-defer #2915

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .golangci.reference.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1152,6 +1152,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

prealloc:
# IMPORTANT: we don't recommend using this linter before doing performance profiling.
# For most programs usage of prealloc will be a premature optimization.
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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-0.20220611222111-e96965ed417f
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
Expand Down
4 changes: 4 additions & 0 deletions go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions pkg/config/linters_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ type LintersSettings struct {
NilNil NilNilSettings
Nlreturn NlreturnSettings
NoLintLint NoLintLintSettings
NoNamedReturns NoNamedReturnsSettings
Prealloc PreallocSettings
Predeclared PredeclaredSettings
Promlinter PromlinterSettings
Expand Down Expand Up @@ -481,6 +482,10 @@ type NoLintLintSettings struct {
AllowUnused bool `mapstructure:"allow-unused"`
}

type NoNamedReturnsSettings struct {
AllowErrorInDefer bool `mapstructure:"allow-error-in-defer"`
}

type PreallocSettings struct {
Simple bool
RangeLoops bool `mapstructure:"range-loops"`
Expand Down
22 changes: 17 additions & 5 deletions pkg/golinters/nonamedreturns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
4 changes: 3 additions & 1 deletion pkg/lint/lintersdb/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ func (m Manager) GetAllSupportedLinterConfigs() []*linter.Config {
nilNilCfg *config.NilNilSettings
nlreturnCfg *config.NlreturnSettings
noLintLintCfg *config.NoLintLintSettings
noNamedReturnsCfg *config.NoNamedReturnsSettings
preallocCfg *config.PreallocSettings
predeclaredCfg *config.PredeclaredSettings
promlinterCfg *config.PromlinterSettings
Expand Down Expand Up @@ -215,6 +216,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
predeclaredCfg = &m.cfg.LintersSettings.Predeclared
promlinterCfg = &m.cfg.LintersSettings.Promlinter
Expand Down Expand Up @@ -604,7 +606,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"),
Expand Down
30 changes: 30 additions & 0 deletions test/testdata/nonamedreturns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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 }
225 changes: 225 additions & 0 deletions test/testdata/nonamedreturns_config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
//args: -Enonamedreturns
//config: linters-settings.nonamedreturns.allow-error-in-defer=true
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