empty-block false positive

[scop]

Describe the bug

Revive can flag a false empty-block positive where the block contents is irrelevant, but there's important functionality going on in the for loop controls for the block.

To Reproduce

cat <<EOF >main.go
package main

var count int

func step() (int, bool) {
	// This could have all kinds of functionality.
	count++
	return count, count < 5
}

func main() {
	for _, c := step(); c; _, c = step() {
		// Here, the loop body is not important, the control using step() is.
	}
}
EOF

$ docker run -v "$(pwd)":/work ghcr.io/mgechev/revive:v1.13.0  ./work/main.go | grep empty
./work/main.go:12:39: this block is empty, you can remove it

Expected behavior

No empty-block flagged.

Logs
N/A

Desktop (please complete the following information):

• OS: [e.g. Ubuntu 18.04]
• Version of Go

Using official revive Docker image.

Additional context
N/A

[denisvmedia]

Thank you for your report. Let me take a look at it.

[chavacava]

Hello @scop, thanks for reporting the problem.

The detection of empty blocks is performed by just checking for empty container nodes (bodies, and BlockStmt) in the AST (Abstract Syntax Tree).
There is some little ad-hoc logic to avoid warning on things like for process() {} but covering cases like the one you report requires adding complementary logic. In these cases we need to trade-off between simplicity of the rule and its "precision" (less false positives)
I don't have actual figures but I suspect the use case is very rare.
Let's see what @denisvmedia proposes.

[scop]

Sure, understood. But naively and without having a look at revive's code myself, I suppose expanding the ad-hoc logic to cover cases where there are any function calls (not just the case mentioned above) in the loop controls would help with this without increasing complexity much at all.

[denisvmedia]

I created a draft PR with some measurements of the impact. Please take a look @scop @chavacava

[alexandear]

Related issue: #386

[denisvmedia]

I ended up creating a docs PR instead: #1624