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

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.

Signed-off-by: Sebastiaan van Stijn <[email protected]>

Author: thaJeztah

stylecheck/analysis.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/st1024.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/st1024_test.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'`)