Repository: mgechev/revive (go)

Issue:
`useless-break` flags necessary `break` in `case`
**Describe the bug**
`useless-break` flags a `break` statement within a `case`, followed by a label

**To Reproduce**
Steps to reproduce the behavior:
On version `v1.7.0` (8ece20b0789c517bd3a6742db0daa4dd5928146d)

```shell
revive --config revive.toml demo/...
```

```toml
[rule.useless-break]
Enabled = true
```

```go
// demo/main/main.go
package main

import "fmt"

func main() {
	switch 1 {
	case 1:
		fmt.Println("foo bar")
		if false { // Add code that jumps to the label so it compiles.
			goto SOME_LABEL
		}
		break
	SOME_LABEL:
		panic(nil)
	}
}
```

**Expected behavior**
No lint error, since the `break` prevents the control flow from entering `SOME_LABEL`.

**Logs**
`demo/main/main.go:12:3: useless break in case clause`

**Desktop (please complete the following information):**
 - OS: `openSUSE Leap 15.6`
 - `go1.24.1 linux/amd64`


Pull request description:
fix false positive in useless-break when a label follows a break
Closes #1281


Patch:
diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md
index 2b795ef..6f65758 100644
--- a/RULES_DESCRIPTIONS.md
+++ b/RULES_DESCRIPTIONS.md
@@ -64,8 +64,8 @@ List of all available rules.
   - [range](#range)
   - [receiver-naming](#receiver-naming)
   - [redefines-builtin-id](#redefines-builtin-id)
-  - [redundant-import-alias](#redundant-import-alias)
   - [redundant-build-tag](#redundant-build-tag)
+  - [redundant-import-alias](#redundant-import-alias)
   - [redundant-test-main-exit](#redundant-test-main-exit)
   - [string-format](#string-format)
   - [string-of-int](#string-of-int)
@@ -866,16 +866,16 @@ Even if possible, redefining these built in names can lead to bugs very difficul
 
 _Configuration_: N/A
 
-## redundant-import-alias
+## redundant-build-tag
 
-_Description_: This rule warns on redundant import aliases. This happens when the alias used on the import statement matches the imported package name.
+_Description_: This rule warns about redundant build tag comments `// +build` when `//go:build` is present.
+`gofmt` in Go 1.17+ automatically adds the `//go:build` constraint, making the `// +build` comment unnecessary.
 
 _Configuration_: N/A
 
-## redundant-build-tag
+## redundant-import-alias
 
-_Description_: This rule warns about redundant build tag comments `// +build` when `//go:build` is present.
-`gofmt` in Go 1.17+ automatically adds the `//go:build` constraint, making the `// +build` comment unnecessary.
+_Description_: This rule warns on redundant import aliases. This happens when the alias used on the import statement matches the imported package name.
 
 _Configuration_: N/A
 
diff --git a/go.mod b/go.mod
index b9cf8fd..ae5ad5d 100644
--- a/go.mod
+++ b/go.mod
@@ -2,7 +2,7 @@ module github.com/mgechev/revive
 
 go 1.23.0
 
-toolchain go1.24.1
+toolchain go1.24.2
 
 require (
 	github.com/BurntSushi/toml v1.5.0
diff --git a/rule/useless_break.go b/rule/useless_break.go
index 8db20c9..b99cc81 100644
--- a/rule/useless_break.go
+++ b/rule/useless_break.go
@@ -47,36 +47,39 @@ func (w *lintUselessBreak) Visit(node ast.Node) ast.Visitor {
 		w.inLoopBody = false
 		return nil
 	case *ast.CommClause:
-		for _, n := range v.Body {
-			w.inspectCaseStatement(n)
-		}
+		w.inspectCaseStatement(v.Body)
 		return nil
 	case *ast.CaseClause:
-		for _, n := range v.Body {
-			w.inspectCaseStatement(n)
-		}
+		w.inspectCaseStatement(v.Body)
 		return nil
 	}
 	return w
 }
 
-func (w *lintUselessBreak) inspectCaseStatement(n ast.Stmt) {
-	switch s := n.(type) {
-	case *ast.BranchStmt:
-		if s.Tok != token.BREAK {
-			return // not a break statement
-		}
-		if s.Label != nil {
-			return // labeled break statement, usually affects a nesting loop
-		}
-		msg := "useless break in case clause"
-		if w.inLoopBody {
-			msg += " (WARN: this break statement affects this switch or select statement and not the loop enclosing it)"
-		}
-		w.onFailure(lint.Failure{
-			Confidence: 1,
-			Node:       s,
-			Failure:    msg,
-		})
+func (w *lintUselessBreak) inspectCaseStatement(body []ast.Stmt) {
+	l := len(body)
+	if l == 0 {
+		return // empty body, nothing to do
 	}
+
+	s := body[l-1] // pick the last statement
+	if !isUnlabelledBreak(s) {
+		return
+	}
+
+	msg := "useless break in case clause"
+	if w.inLoopBody {
+		msg += " (WARN: this break statement affects this switch or select statement and not the loop enclosing it)"
+	}
+
+	w.onFailure(lint.Failure{
+		Confidence: 1,
+		Node:       s,
+		Failure:    msg,
+	})
+}
+
+func isUnlabelledBreak(stmt ast.Stmt) bool {
+	s, ok := stmt.(*ast.BranchStmt)
+	return ok && s.Tok == token.BREAK && s.Label == nil
 }


Test patch:
diff --git a/testdata/useless_break.go b/testdata/useless_break.go
index aa62178..f8b5261 100644
--- a/testdata/useless_break.go
+++ b/testdata/useless_break.go
@@ -87,4 +87,16 @@ func UselessBreaks() {
 	default:
 		s.errorf("range can't iterate over %v", val)
 	}
+
+	// issue #1281
+	switch 1 {
+	case 1:
+		fmt.Println("foo")
+		break // MATCH /useless break in case clause/
+	case 2:
+		fmt.Println("bar")
+		break 
+		fmt.Println("baz")
+	case 3:
+	}
 }