Ban case blocks with just "break", top level break

tools/customlint/emptycase.go → tools/customlint/casebody.go

@@ -11,35 +11,38 @@ import (
 	"golang.org/x/tools/go/ast/inspector"
 )
 
-var emptyCaseAnalyzer = &analysis.Analyzer{
-	Name: "emptycase",
-	Doc:  "finds empty switch/select cases",
+var caseBodyAnalyzer = &analysis.Analyzer{
+	Name: "casebody",
+	Doc:  "finds empty switch/select cases, redundant break statements, and code after breaks",
 	Requires: []*analysis.Analyzer{
 		inspect.Analyzer,
 	},
 	Run: func(pass *analysis.Pass) (any, error) {
-		return (&emptyCasePass{pass: pass}).run()
+		return (&caseBodyPass{pass: pass}).run()
 	},
 }
 
-type emptyCasePass struct {
+type caseBodyPass struct {
 	pass *analysis.Pass
 	file *ast.File
 }
 
-func (e *emptyCasePass) run() (any, error) {
+func (e *caseBodyPass) run() (any, error) {
 	in := e.pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
 
 	for c := range in.Root().Preorder(
 		(*ast.File)(nil),
 		(*ast.SwitchStmt)(nil),
+		(*ast.TypeSwitchStmt)(nil),
 		(*ast.SelectStmt)(nil),
 	) {
 		switch n := c.Node().(type) {
 		case *ast.File:
 			e.file = n
 		case *ast.SwitchStmt:
 			e.checkCases(n.Body)
+		case *ast.TypeSwitchStmt:
+			e.checkCases(n.Body)
 		case *ast.SelectStmt:
 			e.checkCases(n.Body)
 		}
@@ -48,7 +51,7 @@ func (e *emptyCasePass) run() (any, error) {
 	return nil, nil
 }
 
-func (e *emptyCasePass) checkCases(clause *ast.BlockStmt) {
+func (e *caseBodyPass) checkCases(clause *ast.BlockStmt) {
 	endOfBlock := clause.End()
 
 	for i, stmt := range clause.List {
@@ -60,7 +63,7 @@ func (e *emptyCasePass) checkCases(clause *ast.BlockStmt) {
 	}
 }
 
-func (e *emptyCasePass) checkCaseStatement(stmt ast.Stmt, nextCasePos token.Pos) {
+func (e *caseBodyPass) checkCaseStatement(stmt ast.Stmt, nextCasePos token.Pos) {
 	var body []ast.Stmt
 	var colon token.Pos
 
@@ -75,10 +78,36 @@ func (e *emptyCasePass) checkCaseStatement(stmt ast.Stmt, nextCasePos token.Pos)
 		panic(fmt.Sprintf("unhandled statement type %T", stmt))
 	}
 
+	reportedUnreachable := false
+	for i, statement := range body {
+		branch, ok := statement.(*ast.BranchStmt)
+		if !ok || branch.Tok != token.BREAK {
+			continue
+		}
+		if branch.Label == nil {
+			e.pass.Report(analysis.Diagnostic{
+				Pos:     branch.Pos(),
+				End:     branch.End(),
+				Message: "this top-level break statement is redundant",
+			})
+		}
+		if !reportedUnreachable && i+1 < len(body) {
+			e.pass.Report(analysis.Diagnostic{
+				Pos:     body[i+1].Pos(),
+				End:     body[i+1].End(),
+				Message: "statements after a break are not allowed in the same case body",
+			})
+			reportedUnreachable = true
+		}
+	}
+
 	if len(body) == 1 {
 		// Also error on a case statement containing a single empty block.
-		block, ok := body[0].(*ast.BlockStmt)
-		if !ok || len(block.List) != 0 {
+		if block, ok := body[0].(*ast.BlockStmt); ok {
+			if len(block.List) != 0 {
+				return
+			}
+		} else {
 			return
 		}
 	} else if len(body) != 0 {

tools/customlint/plugin.go

@@ -18,7 +18,7 @@ func (f *plugin) BuildAnalyzers() ([]*analysis.Analyzer, error) {
 		bitclearAnalyzer,
 		checkChildrenAnalyzer,
 		cleanupAnalyzer,
-		emptyCaseAnalyzer,
+		caseBodyAnalyzer,
 		forbidParentAccessAnalyzer,
 		shadowAnalyzer,
 		unexportedAPIAnalyzer,

tools/customlint/testdata/casebody/casebody.go

@@ -0,0 +1,153 @@
+package casebody
+
+var X int
+
+func Switch() {
+	switch X {
+	case 1:
+	case 2:
+	case 3:
+	case 4:
+		println(`oops`)
+	}
+}
+
+func SwitchCommented() {
+	switch X {
+	case 1:
+		// do nothing
+	case 2:
+	case 3:
+	case 4:
+		println(`oops`)
+	}
+}
+
+func SwitchSingleCase() {
+	switch X {
+	case 1:
+	}
+}
+
+func SwitchDefaultCase() {
+	switch X {
+	case 1:
+	default:
+	}
+}
+
+func SwitchBreak() {
+	switch X {
+	case 1:
+		break
+	case 2:
+		// intentionally empty
+		break
+	case 3:
+		println(`oops`)
+		break
+	case 4:
+		for {
+			break
+		}
+	case 5:
+		break
+		println(`unreachable`)
+		break
+	case 6:
+		if X != 0 {
+			goto afterBreak
+		}
+		break
+	afterBreak:
+		println(`reachable via goto`)
+	}
+}
+
+func SwitchLabeledBreak() {
+outer:
+	for {
+		switch X {
+		case 1:
+			break outer
+		}
+	}
+}
+
+func SwitchCodeAfterLabeledBreak() {
+outer:
+	for {
+		switch X {
+		case 1:
+			break outer
+			println(`unreachable`)
+		}
+	}
+}
+
+func TypeSwitch(x any) {
+	switch x.(type) {
+	case int:
+	case string:
+		// intentionally empty
+	case bool:
+		break
+	case float64:
+		println(`oops`)
+		break
+	case complex64:
+		break
+		println(`unreachable`)
+	}
+}
+
+func SelectBreak() {
+	select {
+	case <-ch:
+		break
+	default:
+		println(`oops`)
+		break
+	case <-ch2:
+		break
+		println(`unreachable`)
+	}
+}
+
+var (
+	ch  = make(chan int)
+	ch2 = make(chan int)
+	ch3 = make(chan int)
+	ch4 = make(chan int)
+)
+
+func Select() {
+	select {
+	case <-ch:
+	case <-ch2:
+	case <-ch3:
+	case <-ch4:
+		println(`oops`)
+	}
+}
+
+func SelectCommented() {
+	select {
+	case <-ch:
+		// do nothing
+	}
+}
+
+func SelectSingleCase() {
+	select {
+	case <-ch:
+	}
+}
+
+func SelectDefaultCase() {
+	select {
+	case x := <-ch:
+		println(x)
+	default:
+	}
+}