Skip to content

Commit ebb08fc

Browse files
authored
Make error checking more laxed (#184)
Only perform error checking when there's a single comparison with a binary operator comparing an error with `nil`. Both `!=` and `==` are candidates for cuddling. These should be cuddled if `err` exist on the line above ```go if err != nil {} if err == nil {} ``` Nothing else should trigger this lint, all of these should be left ```go // More than error checking if err != nil || true {} // Technically an error checking, but we only check for binary // operators, not expressions. if errors.Is(err, SomeErr) {} // More than one error checking if err != nil || err2 != nil {} ``` This also matches [`gofumpt`](https://github.com/mvdan/gofumpt) which has this rule: > No empty lines before a simple error check > > ```go > foo, err := processFoo() > > if err != nil { > return err > } > ``` > > ```go > foo, err := processFoo() > if err != nil { > return err > } > ```
1 parent 628ebb7 commit ebb08fc

File tree

3 files changed

+56
-28
lines changed

3 files changed

+56
-28
lines changed

testdata/src/default_config/err/err.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ func fn1() {
88
if err != nil {
99
panic(err)
1010
}
11+
12+
err2 := errors.New("y") // want +1 `unnecessary whitespace \(err\)`
13+
14+
if err2 == nil {
15+
panic(err)
16+
}
1117
}
1218

1319
func fn11() { // want +2 `unnecessary whitespace \(err\)`
@@ -105,9 +111,10 @@ func fn7() {
105111
}
106112

107113
func fn8() {
108-
var err = errors.New("x") // want +1 `unnecessary whitespace \(err\)`
114+
aErr := errors.New("a")
115+
bErr := errors.New("b")
109116

110-
if err != nil {
111-
panic(err)
117+
if aErr != nil && bErr != nil {
118+
panic(aErr)
112119
}
113120
}

testdata/src/default_config/err/err.go.golden

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ func fn1() {
77
if err != nil {
88
panic(err)
99
}
10+
11+
err2 := errors.New("y") // want +1 `unnecessary whitespace \(err\)`
12+
if err2 == nil {
13+
panic(err)
14+
}
1015
}
1116

1217
func fn11() { // want +2 `unnecessary whitespace \(err\)`
@@ -103,8 +108,10 @@ func fn7() {
103108
}
104109

105110
func fn8() {
106-
var err = errors.New("x") // want +1 `unnecessary whitespace \(err\)`
107-
if err != nil {
108-
panic(err)
111+
aErr := errors.New("a")
112+
bErr := errors.New("b")
113+
114+
if aErr != nil && bErr != nil {
115+
panic(aErr)
109116
}
110117
}

wsl.go

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"go/format"
88
"go/token"
99
"go/types"
10-
"slices"
1110

1211
"golang.org/x/tools/go/analysis"
1312
)
@@ -540,7 +539,40 @@ func (w *WSL) checkError(
540539

541540
defer cursor.Save()()
542541

543-
if _, ok := ifStmt.(*ast.IfStmt); !ok {
542+
// It must be an if statement
543+
stmt, ok := ifStmt.(*ast.IfStmt)
544+
if !ok {
545+
return
546+
}
547+
548+
// The condition must be a binary expression (X OP Y)
549+
binaryExpr, ok := stmt.Cond.(*ast.BinaryExpr)
550+
if !ok {
551+
return
552+
}
553+
554+
// We must do not equal or equal comparison (!= or ==)
555+
if binaryExpr.Op != token.NEQ && binaryExpr.Op != token.EQL {
556+
return
557+
}
558+
559+
xIdent, ok := binaryExpr.X.(*ast.Ident)
560+
if !ok {
561+
return
562+
}
563+
564+
// X is not an error so it's not error checking
565+
if !w.implementsErr(xIdent) {
566+
return
567+
}
568+
569+
yIdent, ok := binaryExpr.Y.(*ast.Ident)
570+
if !ok {
571+
return
572+
}
573+
574+
// Y is not compared with `nil`
575+
if yIdent.Name != "nil" {
544576
return
545577
}
546578

@@ -559,26 +591,8 @@ func (w *WSL) checkError(
559591
return
560592
}
561593

562-
// Ensure the previous node has at least one variable implementing the error
563-
// interface.
564-
if !slices.ContainsFunc(previousIdents, func(ident *ast.Ident) bool {
565-
return w.implementsErr(ident)
566-
}) {
567-
return
568-
}
569-
570-
// Ensure at least one variable from the previous node is used in the if
571-
// statement.
572-
ifIdents := identsFromNode(ifStmt, true)
573-
if len(identIntersection(ifIdents, previousIdents)) == 0 {
574-
return
575-
}
576-
577-
// And that at least one ident in the if statement implements the error
578-
// interface.
579-
if !slices.ContainsFunc(ifIdents, func(ident *ast.Ident) bool {
580-
return w.implementsErr(ident)
581-
}) {
594+
// Ensure that the error was defined on the line above.
595+
if len(identIntersection([]*ast.Ident{xIdent}, previousIdents)) == 0 {
582596
return
583597
}
584598

0 commit comments

Comments
 (0)