Add ST1024: Avoid named error return values named 'err'

stylecheck/st1024/st1024.go

@@ -0,0 +1,81 @@
+package st1024
+
+import (
+	"go/types"
+
+	"golang.org/x/tools/go/analysis"
+	"honnef.co/go/tools/analysis/lint"
+	"honnef.co/go/tools/analysis/report"
+	"honnef.co/go/tools/internal/passes/buildir"
+)
+
+var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{
+	Analyzer: &analysis.Analyzer{
+		Name:     "ST1024",
+		Run:      run,
+		Requires: []*analysis.Analyzer{buildir.Analyzer},
+	},
+	Doc: &lint.RawDocumentation{
+		Title: "Avoid named error return values named 'err'",
+		Text: `Named return values called 'err' are discouraged because they can lead
+to confusion and subtle mistakes.
+
+In most Go code, 'err' is used for short-lived local variables. Naming a
+return parameter 'err' can induce cognitive load.
+
+For example:
+
+	func fn() (err error) {
+		if err := doSomething(); err != nil { // shadows the return
+			return err
+		}
+		defer func() {qqq
+			// This 'err' is the named return, not the one above
+			if err != nil {
+				log.Println("deferred error:", err)
+			}
+		}()
+		return nil
+	}
+
+Using a distinct name for the named return makes the return value’s
+role explicit and avoids confusion:
+
+	func fn() (retErr error) {
+		if err := doSomething(); err != nil {
+			return err
+		}
+		defer func() {
+			if retErr != nil {
+				log.Println("deferred error:", retErr)
+			}
+		}()
+		return nil
+	}
+
+This check is opt-in and not enabled by default.`,
+		Since:      "2025.2",
+		NonDefault: true,
+		MergeIf:    lint.MergeIfAll,
+	},
+})
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	errorType := types.Universe.Lookup("error").Type()
+
+fnLoop:
+	for _, fn := range pass.ResultOf[buildir.Analyzer].(*buildir.IR).SrcFuncs {
+		sig := fn.Type().(*types.Signature)
+		rets := sig.Results()
+		if rets == nil {
+			continue
+		}
+		for i := range rets.Len() {
+			if r := rets.At(i); types.Unalias(r.Type()) == errorType && r.Name() == "err" {
+				report.Report(pass, rets.At(i), "named error return should not be named 'err'", report.ShortRange())
+				continue fnLoop
+			}
+		}
+	}
+	return nil, nil
+}

stylecheck/st1024/testdata/go1.0/CheckNamedErrorReturns/CheckNamedErrorReturns.go

@@ -0,0 +1,67 @@
+package pkg
+
+// no names
+func fnA0()                {}
+func fnA1() error          { return nil }
+func fnA2() (int, error)   { return 0, nil }
+func fnA3() (error, int)   { return nil, 0 }
+func fnA4() (error, error) { return nil, nil }
+
+// names, but not 'err'
+func fnB1() (a error)          { return nil }
+func fnB2() (a int, b error)   { return 0, nil }
+func fnB3() (a error, b int)   { return nil, 0 }
+func fnB4() (a error, b error) { return nil, nil }
+
+// names, and '_'
+func fnC1() (_ error)          { return nil }
+func fnC2() (a int, _ error)   { return 0, nil }
+func fnC3() (_ error, b int)   { return nil, 0 }
+func fnC4() (_ error, b error) { return nil, nil }
+func fnC5() (a error, _ error) { return nil, nil }
+
+// false positives: non-errors named 'err'
+func fnE1() (err int)          { return 0 }
+func fnE2() (err int, b error) { return 0, nil }
+func fnE3() (a error, err int) { return nil, 0 }
+
+// bad ones: errors named 'err'
+func fnD1() (err error)          { return nil }      //@ diag(`named error return should not be named 'err'`)
+func fnD2() (a int, err error)   { return 0, nil }   //@ diag(`named error return should not be named 'err'`)
+func fnD3() (err error, b int)   { return nil, 0 }   //@ diag(`named error return should not be named 'err'`)
+func fnD4() (err error, b error) { return nil, nil } //@ diag(`named error return should not be named 'err'`)
+func fnD5() (a error, err error) { return nil, nil } //@ diag(`named error return should not be named 'err'`)
+
+type structA struct{}
+
+// no names
+func (structA) fnA0()                {}
+func (structA) fnA1() error          { return nil }
+func (structA) fnA2() (int, error)   { return 0, nil }
+func (structA) fnA3() (error, int)   { return nil, 0 }
+func (structA) fnA4() (error, error) { return nil, nil }
+
+// names, but not 'err'
+func (structA) fnB1() (a error)          { return nil }
+func (structA) fnB2() (a int, b error)   { return 0, nil }
+func (structA) fnB3() (a error, b int)   { return nil, 0 }
+func (structA) fnB4() (a error, b error) { return nil, nil }
+
+// names, and '_'
+func (structA) fnC1() (_ error)          { return nil }
+func (structA) fnC2() (a int, _ error)   { return 0, nil }
+func (structA) fnC3() (_ error, b int)   { return nil, 0 }
+func (structA) fnC4() (_ error, b error) { return nil, nil }
+func (structA) fnC5() (a error, _ error) { return nil, nil }
+
+// false positives: non-errors named 'err'
+func (structA) fnE1() (err int)          { return 0 }
+func (structA) fnE2() (err int, b error) { return 0, nil }
+func (structA) fnE3() (a error, err int) { return nil, 0 }
+
+// bad ones: errors named 'err'
+func (structA) fnD1() (err error)          { return nil }      //@ diag(`named error return should not be named 'err'`)
+func (structA) fnD2() (a int, err error)   { return 0, nil }   //@ diag(`named error return should not be named 'err'`)
+func (structA) fnD3() (err error, b int)   { return nil, 0 }   //@ diag(`named error return should not be named 'err'`)
+func (structA) fnD4() (err error, b error) { return nil, nil } //@ diag(`named error return should not be named 'err'`)
+func (structA) fnD5() (a error, err error) { return nil, nil } //@ diag(`named error return should not be named 'err'`)

stylecheck/st1024/testdata/go1.9/CheckNamedErrorReturns/CheckNamedErrorReturns.go

@@ -0,0 +1,69 @@
+package pkg
+
+type Error = error
+
+// no names
+func fnA0()                {}
+func fnA1() Error          { return nil }
+func fnA2() (int, Error)   { return 0, nil }
+func fnA3() (Error, int)   { return nil, 0 }
+func fnA4() (Error, Error) { return nil, nil }
+
+// names, but not 'err'
+func fnB1() (a Error)          { return nil }
+func fnB2() (a int, b Error)   { return 0, nil }
+func fnB3() (a Error, b int)   { return nil, 0 }
+func fnB4() (a Error, b Error) { return nil, nil }
+
+// names, and '_'
+func fnC1() (_ Error)          { return nil }
+func fnC2() (a int, _ Error)   { return 0, nil }
+func fnC3() (_ Error, b int)   { return nil, 0 }
+func fnC4() (_ Error, b Error) { return nil, nil }
+func fnC5() (a Error, _ Error) { return nil, nil }
+
+// false positives: non-errors named 'err'
+func fnE1() (err int)          { return 0 }
+func fnE2() (err int, b Error) { return 0, nil }
+func fnE3() (a Error, err int) { return nil, 0 }
+
+// bad ones: errors named 'err'
+func fnD1() (err Error)          { return nil }      //@ diag(`named error return should not be named 'err'`)
+func fnD2() (a int, err Error)   { return 0, nil }   //@ diag(`named error return should not be named 'err'`)
+func fnD3() (err Error, b int)   { return nil, 0 }   //@ diag(`named error return should not be named 'err'`)
+func fnD4() (err Error, b Error) { return nil, nil } //@ diag(`named error return should not be named 'err'`)
+func fnD5() (a Error, err Error) { return nil, nil } //@ diag(`named error return should not be named 'err'`)
+
+type structA struct{}
+
+// no names
+func (structA) fnA0()                {}
+func (structA) fnA1() Error          { return nil }
+func (structA) fnA2() (int, Error)   { return 0, nil }
+func (structA) fnA3() (Error, int)   { return nil, 0 }
+func (structA) fnA4() (Error, Error) { return nil, nil }
+
+// names, but not 'err'
+func (structA) fnB1() (a Error)          { return nil }
+func (structA) fnB2() (a int, b Error)   { return 0, nil }
+func (structA) fnB3() (a Error, b int)   { return nil, 0 }
+func (structA) fnB4() (a Error, b Error) { return nil, nil }
+
+// names, and '_'
+func (structA) fnC1() (_ Error)          { return nil }
+func (structA) fnC2() (a int, _ Error)   { return 0, nil }
+func (structA) fnC3() (_ Error, b int)   { return nil, 0 }
+func (structA) fnC4() (_ Error, b Error) { return nil, nil }
+func (structA) fnC5() (a Error, _ Error) { return nil, nil }
+
+// false positives: non-errors named 'err'
+func (structA) fnE1() (err int)          { return 0 }
+func (structA) fnE2() (err int, b Error) { return 0, nil }
+func (structA) fnE3() (a Error, err int) { return nil, 0 }
+
+// bad ones: errors named 'err'
+func (structA) fnD1() (err Error)          { return nil }      //@ diag(`named error return should not be named 'err'`)
+func (structA) fnD2() (a int, err Error)   { return 0, nil }   //@ diag(`named error return should not be named 'err'`)
+func (structA) fnD3() (err Error, b int)   { return nil, 0 }   //@ diag(`named error return should not be named 'err'`)
+func (structA) fnD4() (err Error, b Error) { return nil, nil } //@ diag(`named error return should not be named 'err'`)
+func (structA) fnD5() (a Error, err Error) { return nil, nil } //@ diag(`named error return should not be named 'err'`)