Repository: mgechev/revive (go)

Issue:
useless-break rule does not handle range-loops correctly
**Describe the bug**
The useless-break rule provides a specific message when a useless break is found in a switch (or select) statement inside a loop.
For example, for the following code:

```go
for {
	switch {
	case c1:
		break
	}
}
```
the rule will emit a failure: _useless break in case clause (WARN: this break statement affects this switch or select statement and not the loop enclosing it)_

The same kind of message is expected in the case of for-range loops:
```go
for _, node := range desc.Args {
	switch node := node.(type) {
	case *ast.FuncLit:
		found = true
		funcLit = node
		break // this break is useless
	}
}
```

but the rule does not emit the same message, just: _useless break in case clause_

**To Reproduce**
Steps to reproduce the behavior:
1. I updated revive `go get -u github.com/mgechev/revive`
2. I run it with the following flags & configuration file:

```shell
# flags

revive -config conf.toml testdata/useless-break.go
```

```toml
# config file
ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0

[rule.useless-break]
```

**Expected behavior**
testdata/useless-break.go:12:3: useless break in case clause
testdata/useless-break.go:21:3: useless break in case clause
testdata/useless-break.go:34:4: useless break in case clause (WARN: this break statement affects this switch or select statement and not the loop enclosing it)
**testdata/useless-break.go:43:4: useless break in case clause (WARN: this break statement affects this switch or select statement and not the loop enclosing it)**
testdata/useless-break.go:86:3: useless break in case clause



Pull request description:
fix issue 556
Closes #556 


Patch:
diff --git a/rule/useless-break.go b/rule/useless-break.go
index 9e9c829..a631bd1 100644
--- a/rule/useless-break.go
+++ b/rule/useless-break.go
@@ -41,6 +41,11 @@ func (w *lintUselessBreak) Visit(node ast.Node) ast.Visitor {
 		ast.Walk(w, v.Body)
 		w.inLoopBody = false
 		return nil
+	case *ast.RangeStmt:
+		w.inLoopBody = true
+		ast.Walk(w, v.Body)
+		w.inLoopBody = false
+		return nil
 	case *ast.CommClause:
 		for _, n := range v.Body {
 			w.inspectCaseStatement(n)


Test patch:
diff --git a/testdata/useless-break.go b/testdata/useless-break.go
index 1e6457d..aa62178 100644
--- a/testdata/useless-break.go
+++ b/testdata/useless-break.go
@@ -1,6 +1,9 @@
 package fixtures
 
-import "reflect"
+import (
+	ast "go/ast"
+	"reflect"
+)
 
 func UselessBreaks() {
 
@@ -28,7 +31,7 @@ func UselessBreaks() {
 	for {
 		switch {
 		case c1:
-			break // MATCH /useless break in case clause (WARN: this break statement affects the switch or select statement and not the loop enclosing it)/
+			break // MATCH /useless break in case clause (WARN: this break statement affects this switch or select statement and not the loop enclosing it)/
 		}
 	}
 
@@ -37,7 +40,7 @@ func UselessBreaks() {
 		case *ast.FuncLit:
 			found = true
 			funcLit = node
-			break // MATCH /useless break in case clause (WARN: this break statement affects the switch or select statement and not the loop enclosing it)/
+			break // MATCH /useless break in case clause (WARN: this break statement affects this switch or select statement and not the loop enclosing it)/
 		}
 	}