Repository: bombsimon/wsl (go)

Issue:
Error checking does not work isolated
Disabling all checks but error checking does nothing since error checking happens in `checkCuddlingBlock`. If we don't enable any other check that runs this rule we never end up there.

Since error checking is only viable for `if` checks, we should probably jump straight to error checking if we're an if statement and if checking is turned off.

Pull request:
Improve `err` checking, ensure working isolated
- Always run `err` checking even if `if` is disabled
- Don't force cuddling if no errors are checked
- Don't force cuddling if no variables intersect
- Only check cuddling if previous node is assign or decl
- Move test to default testing

Closes #177 

Patch:
diff --git a/wsl.go b/wsl.go
index 4f99e84..7e408ba 100644
--- a/wsl.go
+++ b/wsl.go
@@ -533,28 +533,56 @@ func (w *WSL) checkError(
 	previousIdents []*ast.Ident,
 	cursor *Cursor,
 ) {
+	if _, ok := w.config.Checks[CheckErr]; !ok {
+		return
+	}
+
 	defer cursor.Save()()
 
 	if _, ok := ifStmt.(*ast.IfStmt); !ok {
 		return
 	}
 
-	if _, ok := w.config.Checks[CheckErr]; !ok {
+	// Previous node must be assign or decl where the error was actually
+	// defined.
+	_, isAssign := previousNode.(*ast.AssignStmt)
+	_, isDecl := previousNode.(*ast.DeclStmt)
+
+	if !isAssign && !isDecl {
 		return
 	}
 
-	cursor.SetChecker(CheckErr)
-
+	// If there are no statements above or the statement above doesn't contain
+	// any idents there can't be any error to cuddle.
 	if stmtsAbove > 0 || len(previousIdents) == 0 {
 		return
 	}
 
+	// Ensure the previous node has at least one variable implementing the error
+	// interface.
 	if !slices.ContainsFunc(previousIdents, func(ident *ast.Ident) bool {
 		return w.implementsErr(ident)
 	}) {
 		return
 	}
 
+	// Ensure at least one variable from the previous node is used in the if
+	// statement.
+	ifIdents := identsFromNode(ifStmt, true)
+	if len(identIntersection(ifIdents, previousIdents)) == 0 {
+		return
+	}
+
+	// And that at least one ident in the if statement implements the error
+	// interface.
+	if !slices.ContainsFunc(ifIdents, func(ident *ast.Ident) bool {
+		return w.implementsErr(ident)
+	}) {
+		return
+	}
+
+	cursor.SetChecker(CheckErr)
+
 	previousNodeEnd := previousNode.End()
 
 	comments := ast.NewCommentMap(w.fset, previousNode, w.file.Comments)
@@ -663,6 +691,16 @@ func (w *WSL) checkIf(stmt *ast.IfStmt, cursor *Cursor, isElse bool) {
 	if _, ok := w.config.Checks[CheckIf]; !isElse && ok {
 		cursor.SetChecker(CheckIf)
 		w.checkCuddlingBlock(stmt, stmt.Body.List, []*ast.Ident{}, cursor, 1)
+	} else if _, ok := w.config.Checks[CheckErr]; !isElse && ok {
+		previousNode := cursor.PreviousNode()
+
+		w.checkError(
+			w.numberOfStatementsAbove(cursor),
+			stmt,
+			previousNode,
+			identsFromNode(previousNode, true),
+			cursor,
+		)
 	}
 }
 


Test patch:
diff --git a/testdata/src/with_config/if_errcheck/if_errcheck.go.golden b/testdata/src/default_config/err/err.go
similarity index 70%
rename from testdata/src/with_config/if_errcheck/if_errcheck.go.golden
rename to testdata/src/default_config/err/err.go
index c7b6353..7cfedfb 100644
--- a/testdata/src/with_config/if_errcheck/if_errcheck.go.golden
+++ b/testdata/src/default_config/err/err.go
@@ -1,10 +1,10 @@
-
 package testpkg
 
 import "errors"
 
 func fn1() {
 	err := errors.New("x") // want +1 `unnecessary whitespace \(err\)`
+
 	if err != nil {
 		panic(err)
 	}
@@ -12,6 +12,7 @@ func fn1() {
 
 func fn11() { // want +2 `unnecessary whitespace \(err\)`
 	err := errors.New("x")
+
 	if err != nil {
 		panic(err)
 	}
@@ -36,8 +37,8 @@ func fn13() {
 
 func fn2() {
 	a := 1
-
 	err := errors.New("x") // want +1 `unnecessary whitespace \(err\)`
+
 	if err != nil {
 		panic(err)
 	}
@@ -47,8 +48,8 @@ func fn2() {
 
 func fn21() {
 	a := 1 // want +2 `unnecessary whitespace \(err\)`
-
 	err := errors.New("x")
+
 	if err != nil {
 		panic(err)
 	}
@@ -58,7 +59,6 @@ func fn21() {
 
 func fn3() {
 	a := 1
-
 	err := errors.New("x") // want `missing whitespace above this line \(too many statements above if\)`
 	if err != nil {
 		panic(err)
@@ -75,3 +75,39 @@ func fn4() {
 		panic(err)
 	}
 }
+
+func fn5() {
+	err := errors.New("x") // want +1 `unnecessary whitespace \(err\)`
+
+	if err != nil {
+		panic(err)
+	}
+
+	if false {
+		_ = 1
+	}
+}
+
+func fn6() {
+	err := errors.New("x")
+
+	if false {
+		panic(err)
+	}
+}
+
+func fn7() {
+	err, ok := errors.New("x"), true
+
+	if !ok {
+		panic(err)
+	}
+}
+
+func fn8() {
+	var err = errors.New("x") // want +1 `unnecessary whitespace \(err\)`
+
+	if err != nil {
+		panic(err)
+	}
+}
diff --git a/testdata/src/with_config/if_errcheck/if_errcheck.go b/testdata/src/default_config/err/err.go.golden
similarity index 70%
rename from testdata/src/with_config/if_errcheck/if_errcheck.go
rename to testdata/src/default_config/err/err.go.golden
index 5732f48..43443c0 100644
--- a/testdata/src/with_config/if_errcheck/if_errcheck.go
+++ b/testdata/src/default_config/err/err.go.golden
@@ -4,7 +4,6 @@ import "errors"
 
 func fn1() {
 	err := errors.New("x") // want +1 `unnecessary whitespace \(err\)`
-
 	if err != nil {
 		panic(err)
 	}
@@ -12,7 +11,6 @@ func fn1() {
 
 func fn11() { // want +2 `unnecessary whitespace \(err\)`
 	err := errors.New("x")
-
 	if err != nil {
 		panic(err)
 	}
@@ -37,8 +35,8 @@ func fn13() {
 
 func fn2() {
 	a := 1
-	err := errors.New("x") // want +1 `unnecessary whitespace \(err\)`
 
+	err := errors.New("x") // want +1 `unnecessary whitespace \(err\)`
 	if err != nil {
 		panic(err)
 	}
@@ -48,8 +46,8 @@ func fn2() {
 
 func fn21() {
 	a := 1 // want +2 `unnecessary whitespace \(err\)`
-	err := errors.New("x")
 
+	err := errors.New("x")
 	if err != nil {
 		panic(err)
 	}
@@ -59,6 +57,7 @@ func fn21() {
 
 func fn3() {
 	a := 1
+
 	err := errors.New("x") // want `missing whitespace above this line \(too many statements above if\)`
 	if err != nil {
 		panic(err)
@@ -75,3 +74,37 @@ func fn4() {
 		panic(err)
 	}
 }
+
+func fn5() {
+	err := errors.New("x") // want +1 `unnecessary whitespace \(err\)`
+	if err != nil {
+		panic(err)
+	}
+
+	if false {
+		_ = 1
+	}
+}
+
+func fn6() {
+	err := errors.New("x")
+
+	if false {
+		panic(err)
+	}
+}
+
+func fn7() {
+	err, ok := errors.New("x"), true
+
+	if !ok {
+		panic(err)
+	}
+}
+
+func fn8() {
+	var err = errors.New("x") // want +1 `unnecessary whitespace \(err\)`
+	if err != nil {
+		panic(err)
+	}
+}
diff --git a/wsl_test.go b/wsl_test.go
index 86c7c91..a443718 100644
--- a/wsl_test.go
+++ b/wsl_test.go
@@ -38,12 +38,6 @@ func TestWithConfig(t *testing.T) {
 		subdir   string
 		configFn func(*Configuration)
 	}{
-		{
-			subdir: "if_errcheck",
-			configFn: func(config *Configuration) {
-				config.Checks.Add(CheckErr)
-			},
-		},
 		{
 			subdir: "no_check_decl",
 			configFn: func(config *Configuration) {