Skip to content

Commit

Permalink
update linter implementation
Browse files Browse the repository at this point in the history
  • Loading branch information
ldez committed Jun 19, 2022
1 parent d98c08b commit f44d6b5
Show file tree
Hide file tree
Showing 6 changed files with 285 additions and 7 deletions.
5 changes: 5 additions & 0 deletions .golangci.reference.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 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
ParallelTest ParallelTestSettings
Prealloc PreallocSettings
Predeclared PredeclaredSettings
Expand Down Expand Up @@ -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"`
}
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
parallelTestCfg *config.ParallelTestSettings
preallocCfg *config.PreallocSettings
predeclaredCfg *config.PredeclaredSettings
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"),
Expand Down
32 changes: 31 additions & 1 deletion test/testdata/nonamedreturns.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//args: -Enonamedreturns
// args: -Enonamedreturns
package testdata

import "fmt"
Expand All @@ -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_custom.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

0 comments on commit f44d6b5

Please sign in to comment.