Repository: mgechev/revive (go)

Issue:
False positive on `for` loop
**Describe the bug**

`for` loops where the condition has side-effects and the block is empty can be valid.

**To Reproduce**

```
package main

import "fmt"

func main() {
	next := 0
	iter := func(v *int) bool {
		*v = next
		next++
		fmt.Println(*v)
		return next < 10
	}

	z := 0
	for iter(&z) {
	}
}
```

```
$ revive .
main.go:1:1: should have a package comment
main.go:15:15: this block is empty, you can remove it
```

Such code may be present in locations where you have an iterator interface and want to skip over rest-of the entries, but still read them.

**Expected behavior**

No warning.

**Desktop (please complete the following information):**
 - OS: Windows 11
 - go version go1.20.1 windows/amd64

Pull request:
Prevent empty-block when looping over call expr
Fix false positive for common use-case of looping over a function with side-effects mentioned in https://github.com/mgechev/revive/issues/810.

closes #810 

Patch:
diff --git a/rule/empty-block.go b/rule/empty-block.go
index 1765255..25a052a 100644
--- a/rule/empty-block.go
+++ b/rule/empty-block.go
@@ -43,6 +43,13 @@ func (w lintEmptyBlock) Visit(node ast.Node) ast.Visitor {
 	case *ast.SelectStmt:
 		w.ignore[n.Body] = true
 		return w
+	case *ast.ForStmt:
+		if len(n.Body.List) == 0 && n.Init == nil && n.Post == nil && n.Cond != nil {
+			if _, isCall := n.Cond.(*ast.CallExpr); isCall {
+				w.ignore[n.Body] = true
+				return w
+			}
+		}
 	case *ast.RangeStmt:
 		if len(n.Body.List) == 0 {
 			w.onFailure(lint.Failure{


Test patch:
diff --git a/revivelib/core_test.go b/revivelib/core_test.go
index aa0ddb4..44ab627 100644
--- a/revivelib/core_test.go
+++ b/revivelib/core_test.go
@@ -26,17 +26,12 @@ func TestReviveLint(t *testing.T) {
 		failureList = append(failureList, failure)
 	}
 
-	const expected = 3
+	const expected = 5
 
 	got := len(failureList)
 	if got != expected {
 		t.Fatalf("Expected failures to have %d failures, but it has %d.", expected, got)
 	}
-
-	errmsg := "redundant if ...; err != nil check, just return error instead."
-	if failureList[0].Failure != errmsg {
-		t.Fatalf("Expected failure[0] to be '%s', but it was '%s'", errmsg, failureList[0].Failure)
-	}
 }
 
 func TestReviveFormat(t *testing.T) {
@@ -57,13 +52,15 @@ func TestReviveFormat(t *testing.T) {
 	}
 
 	errorMsgs := []string{
-		"(15, 2)  https://revive.run/r#if-return  redundant if ...; err != nil check, just return error instead.",
-		"(88, 3)  https://revive.run/r#if-return  redundant if ...; err != nil check, just return error instead.",
-		"(95, 3)  https://revive.run/r#if-return  redundant if ...; err != nil check, just return error instead.",
+		"(91, 3)  https://revive.run/r#unreachable-code  unreachable code after this statement",
+		"(98, 3)  https://revive.run/r#unreachable-code  unreachable code after this statement",
+		"(15, 2)  https://revive.run/r#if-return         redundant if ...; err != nil check, just return error instead.",
+		"(88, 3)  https://revive.run/r#if-return         redundant if ...; err != nil check, just return error instead.",
+		"(95, 3)  https://revive.run/r#if-return         redundant if ...; err != nil check, just return error instead.",
 	}
 	for _, errorMsg := range errorMsgs {
 		if !strings.Contains(failures, errorMsg) {
-			t.Fatalf("Expected formatted failures '%s' to contain '%s', but it didn't.", failures, errorMsg)
+			t.Fatalf("Expected formatted failures\n'%s'\nto contain\n'%s', but it didn't.", failures, errorMsg)
 		}
 	}
 
diff --git a/testdata/empty-block.go b/testdata/empty-block.go
index ee257d0..5daaf64 100644
--- a/testdata/empty-block.go
+++ b/testdata/empty-block.go
@@ -11,7 +11,6 @@ type foo struct{}
 func (f foo) f(x *int)  {} // Must not match
 func (f *foo) g(y *int) {} // Must not match
 
-
 func h() {
 	go http.ListenAndServe()
 	select {} // Must not match
@@ -43,6 +42,10 @@ func g(f func() bool) {
 
 	}
 
+	for true { // MATCH /this block is empty, you can remove it/
+
+	}
+
 	// issue 386, then overwritten by issue 416
 	var c = make(chan int)
 	for range c { // MATCH /this block is empty, you can remove it/
@@ -57,4 +60,34 @@ func g(f func() bool) {
 		if ok { // MATCH /this block is empty, you can remove it/
 		}
 	}
+
+	// issue 810
+	next := 0
+	iter := func(v *int) bool {
+		*v = next
+		next++
+		fmt.Println(*v)
+		return next < 10
+	}
+
+	z := 0
+	for iter(&z) { // Must not match
+	}
+
+	for process() { // Must not match
+	}
+
+	var it iterator
+	for it.next() { // Must not match
+	}
+}
+
+func process() bool {
+	return false
+}
+
+type iterator struct{}
+
+func (it *iterator) next() bool {
+	return false
 }