Skip to content

Commit

Permalink
review: breaking changes in a BUGFIX version are BAD
Browse files Browse the repository at this point in the history
  • Loading branch information
ldez committed Jun 26, 2022
1 parent a026bd1 commit 801b662
Show file tree
Hide file tree
Showing 7 changed files with 297 additions and 239 deletions.
4 changes: 2 additions & 2 deletions .golangci.reference.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion pkg/config/linters_settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down
4 changes: 2 additions & 2 deletions pkg/golinters/nonamedreturns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}
}
Expand All @@ -25,5 +25,5 @@ func NewNoNamedReturns(settings *config.NoNamedReturnsSettings) *goanalysis.Lint
a.Doc,
[]*analysis.Analyzer{a},
cfg,
).WithLoadMode(goanalysis.LoadModeSyntax)
).WithLoadMode(goanalysis.LoadModeTypesInfo)
}
1 change: 1 addition & 0 deletions pkg/lint/lintersdb/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),

Expand Down
2 changes: 1 addition & 1 deletion test/testdata/configs/nonamedreturns.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
linters-settings:
nonamedreturns:
allow-error-in-defer: true
report-error-in-defer: true
285 changes: 221 additions & 64 deletions test/testdata/nonamedreturns.go
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit 801b662

Please sign in to comment.