From 1411d5e395c725639d6b0a4da82341f91368bd65 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:06:16 -0700 Subject: [PATCH 1/8] Extend emptycase lint rule to ban comment-less redundant break cases The emptycase customlint rule previously only flagged switch/select cases with an empty body. Extend it to also flag cases whose body is solely a bare break statement, since that break is redundant in Go (switch/select cases do not fall through) and provides no more information than an empty case. As with empty cases, a case is exempted if it has an explanatory comment. Also fix the handful of existing bare break-only cases in the compiler and language server that were newly caught by this rule, by adding a short comment explaining the intentional no-op. --- tools/customlint/emptycase.go | 14 +++++++++++--- tools/customlint/testdata/emptycase/emptycase.go | 13 +++++++++++++ .../testdata/emptycase/emptycase.go.golden | 15 +++++++++++++++ tsc/internal/format/scanner.go | 2 +- tsc/internal/ls/autoimport/util.go | 2 +- tsc/internal/printer/printer.go | 2 +- tsc/internal/printer/utilities.go | 2 +- .../transformers/estransforms/namedevaluation.go | 2 +- .../transformers/tstransforms/typeserializer.go | 2 +- 9 files changed, 45 insertions(+), 9 deletions(-) diff --git a/tools/customlint/emptycase.go b/tools/customlint/emptycase.go index 430b9186a9554..5d0ae32754f04 100644 --- a/tools/customlint/emptycase.go +++ b/tools/customlint/emptycase.go @@ -75,10 +75,18 @@ func (e *emptyCasePass) checkCaseStatement(stmt ast.Stmt, nextCasePos token.Pos) panic(fmt.Sprintf("unhandled statement type %T", stmt)) } + message := "this case block is empty and will do nothing" + 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 if branch, ok := body[0].(*ast.BranchStmt); ok && branch.Tok == token.BREAK && branch.Label == nil { + // Also error on a case statement containing only a redundant "break". + message = "this case block only contains a redundant break statement and will do nothing" + } else { return } } else if len(body) != 0 { @@ -93,7 +101,7 @@ func (e *emptyCasePass) checkCaseStatement(stmt ast.Stmt, nextCasePos token.Pos) e.pass.Report(analysis.Diagnostic{ Pos: stmt.Pos(), End: afterColon, - Message: "this case block is empty and will do nothing", + Message: message, }) } diff --git a/tools/customlint/testdata/emptycase/emptycase.go b/tools/customlint/testdata/emptycase/emptycase.go index 5d3e55a94e580..7376b9361ad86 100644 --- a/tools/customlint/testdata/emptycase/emptycase.go +++ b/tools/customlint/testdata/emptycase/emptycase.go @@ -36,6 +36,19 @@ func SwitchDefaultCase() { } } +func SwitchBreak() { + switch X { + case 1: + break + case 2: + // intentionally empty + break + case 3: + println(`oops`) + break + } +} + var ( ch = make(chan int) ch2 = make(chan int) diff --git a/tools/customlint/testdata/emptycase/emptycase.go.golden b/tools/customlint/testdata/emptycase/emptycase.go.golden index 8f30663b891b6..8e4b7eb80d198 100644 --- a/tools/customlint/testdata/emptycase/emptycase.go.golden +++ b/tools/customlint/testdata/emptycase/emptycase.go.golden @@ -52,6 +52,21 @@ } } + func SwitchBreak() { + switch X { + case 1: + ~~~~~~~ +!!! emptycase: this case block only contains a redundant break statement and will do nothing + break + case 2: + // intentionally empty + break + case 3: + println(`oops`) + break + } + } + var ( ch = make(chan int) ch2 = make(chan int) diff --git a/tsc/internal/format/scanner.go b/tsc/internal/format/scanner.go index e85fae16c6eb7..d1ac0882ea150 100644 --- a/tsc/internal/format/scanner.go +++ b/tsc/internal/format/scanner.go @@ -302,7 +302,7 @@ func (s *formattingScanner) getNextToken(n *ast.Node, expectedScanAction scanAct s.lastScanAction = actionRescanJsxAttributeValue return s.s.ReScanJsxAttributeValue() case actionScan: - break + // no rescan needed; the token was already produced by the normal scan default: debug.AssertNever(expectedScanAction, "unhandled scan action kind") } diff --git a/tsc/internal/ls/autoimport/util.go b/tsc/internal/ls/autoimport/util.go index d1768a9c343c4..b3b1b75cc962b 100644 --- a/tsc/internal/ls/autoimport/util.go +++ b/tsc/internal/ls/autoimport/util.go @@ -207,7 +207,7 @@ func createCheckerPool(program checker.Program) (getChecker func() (*checker.Che case ch := <-pool: return ch, func() { pool <- ch } default: - break + // pool is empty; fall through to try creating a new checker } // Try to create a new one if under limit for { diff --git a/tsc/internal/printer/printer.go b/tsc/internal/printer/printer.go index 31547cde51d64..2aded0ae9c4b3 100644 --- a/tsc/internal/printer/printer.go +++ b/tsc/internal/printer/printer.go @@ -4879,7 +4879,7 @@ func (p *Printer) hasTrailingComma(parentNode *ast.Node, children *ast.NodeList) func (p *Printer) writeDelimiter(format ListFormat) { switch format & LFDelimitersMask { case LFNone: - break + // no delimiter for this format case LFCommaDelimited: p.writePunctuation(",") case LFBarDelimited: diff --git a/tsc/internal/printer/utilities.go b/tsc/internal/printer/utilities.go index 0ae6a6a833854..e67c3d767b855 100644 --- a/tsc/internal/printer/utilities.go +++ b/tsc/internal/printer/utilities.go @@ -452,7 +452,7 @@ func getContainingNodeArray(node *ast.Node) *ast.NodeList { case ast.IsFunctionLike(parent) || ast.IsClassLike(parent) || ast.IsInterfaceDeclaration(parent) || ast.IsTypeOrJSTypeAliasDeclaration(parent): return parent.TypeParameterList() case ast.IsInferTypeNode(parent): - break + // infer type nodes have no associated type parameter list default: panic(fmt.Sprintf("Unexpected TypeParameter parent: %#v", parent.Kind)) } diff --git a/tsc/internal/transformers/estransforms/namedevaluation.go b/tsc/internal/transformers/estransforms/namedevaluation.go index 05924916ecc5d..1c9d1531ae843 100644 --- a/tsc/internal/transformers/estransforms/namedevaluation.go +++ b/tsc/internal/transformers/estransforms/namedevaluation.go @@ -74,7 +74,7 @@ func isAnonymousFunctionDefinition(emitContext *printer.EmitContext, node *ast.E } break case ast.KindArrowFunction: - break + // arrow functions are always anonymous default: return false } diff --git a/tsc/internal/transformers/tstransforms/typeserializer.go b/tsc/internal/transformers/tstransforms/typeserializer.go index 96ef378de3779..306c70ab38663 100644 --- a/tsc/internal/transformers/tstransforms/typeserializer.go +++ b/tsc/internal/transformers/tstransforms/typeserializer.go @@ -229,7 +229,7 @@ func (s *metadataSerializer) serializeTypeNode(node *ast.Node) *ast.Node { // handle JSDoc types from an invalid parse case ast.KindJSDocAllType, ast.KindJSDocVariadicType: - break + // no meaningful serialization for these invalid-parse JSDoc types case ast.KindJSDocNullableType, ast.KindJSDocNonNullableType, ast.KindJSDocOptionalType: return s.serializeTypeNode(node.Type()) default: From e7700641475e1373a41357f899df2d97fcfe77dc Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:16:04 -0700 Subject: [PATCH 2/8] Enforce redundant breaks across case bodies Rename the emptycase analyzer to casebody now that it enforces both empty case documentation and redundant top-level break statements. Report every direct bare break in a switch or select case, even when the case has other statements or comments. Replace existing redundant breaks with explanatory comments where the case becomes empty, and add switch and select coverage for lone, commented, trailing, and nested breaks. --- .../customlint/{emptycase.go => casebody.go} | 31 ++-- tools/customlint/plugin.go | 2 +- .../emptycase.go => casebody/casebody.go} | 16 +- .../testdata/casebody/casebody.go.golden | 139 ++++++++++++++++++ .../testdata/emptycase/emptycase.go.golden | 117 --------------- tsc/internal/format/span.go | 2 - tsc/internal/ls/folding.go | 2 - tsc/internal/printer/printer.go | 8 +- .../estransforms/namedevaluation.go | 2 - .../tstransforms/typeserializer.go | 2 +- 10 files changed, 176 insertions(+), 145 deletions(-) rename tools/customlint/{emptycase.go => casebody.go} (71%) rename tools/customlint/testdata/{emptycase/emptycase.go => casebody/casebody.go} (86%) create mode 100644 tools/customlint/testdata/casebody/casebody.go.golden delete mode 100644 tools/customlint/testdata/emptycase/emptycase.go.golden diff --git a/tools/customlint/emptycase.go b/tools/customlint/casebody.go similarity index 71% rename from tools/customlint/emptycase.go rename to tools/customlint/casebody.go index 5d0ae32754f04..cafb29c323658 100644 --- a/tools/customlint/emptycase.go +++ b/tools/customlint/casebody.go @@ -11,23 +11,23 @@ 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 and redundant break statements", 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( @@ -48,7 +48,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 +60,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,7 +75,15 @@ func (e *emptyCasePass) checkCaseStatement(stmt ast.Stmt, nextCasePos token.Pos) panic(fmt.Sprintf("unhandled statement type %T", stmt)) } - message := "this case block is empty and will do nothing" + for _, statement := range body { + if branch, ok := statement.(*ast.BranchStmt); ok && branch.Tok == token.BREAK && branch.Label == nil { + e.pass.Report(analysis.Diagnostic{ + Pos: branch.Pos(), + End: branch.End(), + Message: "this top-level break statement is redundant", + }) + } + } if len(body) == 1 { // Also error on a case statement containing a single empty block. @@ -83,9 +91,6 @@ func (e *emptyCasePass) checkCaseStatement(stmt ast.Stmt, nextCasePos token.Pos) if len(block.List) != 0 { return } - } else if branch, ok := body[0].(*ast.BranchStmt); ok && branch.Tok == token.BREAK && branch.Label == nil { - // Also error on a case statement containing only a redundant "break". - message = "this case block only contains a redundant break statement and will do nothing" } else { return } @@ -101,7 +106,7 @@ func (e *emptyCasePass) checkCaseStatement(stmt ast.Stmt, nextCasePos token.Pos) e.pass.Report(analysis.Diagnostic{ Pos: stmt.Pos(), End: afterColon, - Message: message, + Message: "this case block is empty and will do nothing", }) } diff --git a/tools/customlint/plugin.go b/tools/customlint/plugin.go index 6b7bf853eeaa9..6d6438865a16a 100644 --- a/tools/customlint/plugin.go +++ b/tools/customlint/plugin.go @@ -18,7 +18,7 @@ func (f *plugin) BuildAnalyzers() ([]*analysis.Analyzer, error) { bitclearAnalyzer, checkChildrenAnalyzer, cleanupAnalyzer, - emptyCaseAnalyzer, + caseBodyAnalyzer, forbidParentAccessAnalyzer, shadowAnalyzer, unexportedAPIAnalyzer, diff --git a/tools/customlint/testdata/emptycase/emptycase.go b/tools/customlint/testdata/casebody/casebody.go similarity index 86% rename from tools/customlint/testdata/emptycase/emptycase.go rename to tools/customlint/testdata/casebody/casebody.go index 7376b9361ad86..fce6970e30e6e 100644 --- a/tools/customlint/testdata/emptycase/emptycase.go +++ b/tools/customlint/testdata/casebody/casebody.go @@ -1,4 +1,4 @@ -package emptycase +package casebody var X int @@ -46,6 +46,20 @@ func SwitchBreak() { case 3: println(`oops`) break + case 4: + for { + break + } + } +} + +func SelectBreak() { + select { + case <-ch: + break + default: + println(`oops`) + break } } diff --git a/tools/customlint/testdata/casebody/casebody.go.golden b/tools/customlint/testdata/casebody/casebody.go.golden new file mode 100644 index 0000000000000..74624ac722395 --- /dev/null +++ b/tools/customlint/testdata/casebody/casebody.go.golden @@ -0,0 +1,139 @@ + package casebody + + var X int + + func Switch() { + switch X { + case 1: + ~~~~~~~ +!!! casebody: this case block is empty and will do nothing + case 2: + ~~~~~~~ +!!! casebody: this case block is empty and will do nothing + case 3: + ~~~~~~~ +!!! casebody: this case block is empty and will do nothing + case 4: + println(`oops`) + } + } + + func SwitchCommented() { + switch X { + case 1: + // do nothing + case 2: + ~~~~~~~ +!!! casebody: this case block is empty and will do nothing + case 3: + ~~~~~~~ +!!! casebody: this case block is empty and will do nothing + case 4: + println(`oops`) + } + } + + func SwitchSingleCase() { + switch X { + case 1: + ~~~~~~~ +!!! casebody: this case block is empty and will do nothing + } + } + + func SwitchDefaultCase() { + switch X { + case 1: + ~~~~~~~ +!!! casebody: this case block is empty and will do nothing + default: + ~~~~~~~~ +!!! casebody: this case block is empty and will do nothing + } + } + + func SwitchBreak() { + switch X { + case 1: + break + ~~~~~ +!!! casebody: this top-level break statement is redundant + case 2: + // intentionally empty + break + ~~~~~ +!!! casebody: this top-level break statement is redundant + case 3: + println(`oops`) + break + ~~~~~ +!!! casebody: this top-level break statement is redundant + case 4: + for { + break + } + } + } + + func SelectBreak() { + select { + case <-ch: + break + ~~~~~ +!!! casebody: this top-level break statement is redundant + default: + println(`oops`) + break + ~~~~~ +!!! casebody: this top-level break statement is redundant + } + } + + var ( + ch = make(chan int) + ch2 = make(chan int) + ch3 = make(chan int) + ch4 = make(chan int) + ) + + func Select() { + select { + case <-ch: + ~~~~~~~~~~ +!!! casebody: this case block is empty and will do nothing + case <-ch2: + ~~~~~~~~~~~ +!!! casebody: this case block is empty and will do nothing + case <-ch3: + ~~~~~~~~~~~ +!!! casebody: this case block is empty and will do nothing + case <-ch4: + println(`oops`) + } + } + + func SelectCommented() { + select { + case <-ch: + // do nothing + } + } + + func SelectSingleCase() { + select { + case <-ch: + ~~~~~~~~~~ +!!! casebody: this case block is empty and will do nothing + } + } + + func SelectDefaultCase() { + select { + case x := <-ch: + println(x) + default: + ~~~~~~~~ +!!! casebody: this case block is empty and will do nothing + } + } + diff --git a/tools/customlint/testdata/emptycase/emptycase.go.golden b/tools/customlint/testdata/emptycase/emptycase.go.golden deleted file mode 100644 index 8e4b7eb80d198..0000000000000 --- a/tools/customlint/testdata/emptycase/emptycase.go.golden +++ /dev/null @@ -1,117 +0,0 @@ - package emptycase - - var X int - - func Switch() { - switch X { - case 1: - ~~~~~~~ -!!! emptycase: this case block is empty and will do nothing - case 2: - ~~~~~~~ -!!! emptycase: this case block is empty and will do nothing - case 3: - ~~~~~~~ -!!! emptycase: this case block is empty and will do nothing - case 4: - println(`oops`) - } - } - - func SwitchCommented() { - switch X { - case 1: - // do nothing - case 2: - ~~~~~~~ -!!! emptycase: this case block is empty and will do nothing - case 3: - ~~~~~~~ -!!! emptycase: this case block is empty and will do nothing - case 4: - println(`oops`) - } - } - - func SwitchSingleCase() { - switch X { - case 1: - ~~~~~~~ -!!! emptycase: this case block is empty and will do nothing - } - } - - func SwitchDefaultCase() { - switch X { - case 1: - ~~~~~~~ -!!! emptycase: this case block is empty and will do nothing - default: - ~~~~~~~~ -!!! emptycase: this case block is empty and will do nothing - } - } - - func SwitchBreak() { - switch X { - case 1: - ~~~~~~~ -!!! emptycase: this case block only contains a redundant break statement and will do nothing - break - case 2: - // intentionally empty - break - case 3: - println(`oops`) - break - } - } - - var ( - ch = make(chan int) - ch2 = make(chan int) - ch3 = make(chan int) - ch4 = make(chan int) - ) - - func Select() { - select { - case <-ch: - ~~~~~~~~~~ -!!! emptycase: this case block is empty and will do nothing - case <-ch2: - ~~~~~~~~~~~ -!!! emptycase: this case block is empty and will do nothing - case <-ch3: - ~~~~~~~~~~~ -!!! emptycase: this case block is empty and will do nothing - case <-ch4: - println(`oops`) - } - } - - func SelectCommented() { - select { - case <-ch: - // do nothing - } - } - - func SelectSingleCase() { - select { - case <-ch: - ~~~~~~~~~~ -!!! emptycase: this case block is empty and will do nothing - } - } - - func SelectDefaultCase() { - select { - case x := <-ch: - println(x) - default: - ~~~~~~~~ -!!! emptycase: this case block is empty and will do nothing - } - } - diff --git a/tsc/internal/format/span.go b/tsc/internal/format/span.go index ef268c873a404..a82e7c2f5c9ea 100644 --- a/tsc/internal/format/span.go +++ b/tsc/internal/format/span.go @@ -1200,12 +1200,10 @@ func (i *dynamicIndenter) shouldAddDelta(line int, kind ast.Kind, container *ast case ast.KindJsxOpeningElement, ast.KindJsxClosingElement, ast.KindJsxSelfClosingElement: return false } - break case ast.KindOpenBracketToken, ast.KindCloseBracketToken: if container.Kind != ast.KindMappedType { return false } - break } // if token line equals to the line of containing node (this is a first token in the node) - use node indentation return i.nodeStartLine != line && diff --git a/tsc/internal/ls/folding.go b/tsc/internal/ls/folding.go index 4c97a768797f1..2c79b4b10bf43 100644 --- a/tsc/internal/ls/folding.go +++ b/tsc/internal/ls/folding.go @@ -343,7 +343,6 @@ func addOutliningForLeadingCommentsForPos(ctx context.Context, pos int, sourceFi } lastSingleLineCommentEnd = commentEnd singleLineCommentCount++ - break case ast.KindMultiLineCommentTrivia: comments := combineAndAddMultipleSingleLineComments() if comments != nil { @@ -354,7 +353,6 @@ func addOutliningForLeadingCommentsForPos(ctx context.Context, pos int, sourceFi foldingRange = append(foldingRange, comment) } singleLineCommentCount = 0 - break default: debug.AssertNever(comment.Kind) } diff --git a/tsc/internal/printer/printer.go b/tsc/internal/printer/printer.go index 2aded0ae9c4b3..51673e4afd02e 100644 --- a/tsc/internal/printer/printer.go +++ b/tsc/internal/printer/printer.go @@ -2812,7 +2812,6 @@ func (p *Printer) getBinaryExpressionPrecedence(node *ast.BinaryExpression) (lef // No need to parenthesize the right operand when the binary operator and // operand are both ,: // x,(a,b) => x,a,b - break case ast.OperatorPrecedenceAssignment: // assignment is right-associative leftPrec = ast.OperatorPrecedenceConditional @@ -2825,17 +2824,14 @@ func (p *Printer) getBinaryExpressionPrecedence(node *ast.BinaryExpression) (lef // No need to parenthesize the right operand when the binary operator and // operand are both | due to the associative property of mathematics: // x|(a|b) => x|a|b - break case ast.OperatorPrecedenceBitwiseXOR: // No need to parenthesize the right operand when the binary operator and // operand are both ^ due to the associative property of mathematics: // x^(a^b) => x^a^b - break case ast.OperatorPrecedenceBitwiseAND: // No need to parenthesize the right operand when the binary operator and // operand are both & due to the associative property of mathematics: // x&(a&b) => x&a&b - break case ast.OperatorPrecedenceEquality: rightPrec = ast.OperatorPrecedenceRelational case ast.OperatorPrecedenceRelational: @@ -3316,7 +3312,7 @@ func (p *Printer) emitExpression(node *ast.Expression, precedence ast.OperatorPr case ast.KindSyntheticExpression: panic("SyntheticExpression should never be printed.") case ast.KindMissingDeclaration: - break + // Missing declarations do not emit an expression. // JSX case ast.KindJsxElement: @@ -4219,7 +4215,7 @@ func (p *Printer) emitStatement(node *ast.Statement) { case ast.KindModuleDeclaration: p.emitModuleDeclaration(node.AsModuleDeclaration()) case ast.KindMissingDeclaration: - break + // Missing declarations do not emit a statement. // Import/Export Statements case ast.KindNamespaceExportDeclaration: diff --git a/tsc/internal/transformers/estransforms/namedevaluation.go b/tsc/internal/transformers/estransforms/namedevaluation.go index 1c9d1531ae843..3b986a5b9d009 100644 --- a/tsc/internal/transformers/estransforms/namedevaluation.go +++ b/tsc/internal/transformers/estransforms/namedevaluation.go @@ -67,12 +67,10 @@ func isAnonymousFunctionDefinition(emitContext *printer.EmitContext, node *ast.E if classHasDeclaredOrExplicitlyAssignedName(emitContext, node) { return false } - break case ast.KindFunctionExpression: if node.AsFunctionExpression().Name() != nil { return false } - break case ast.KindArrowFunction: // arrow functions are always anonymous default: diff --git a/tsc/internal/transformers/tstransforms/typeserializer.go b/tsc/internal/transformers/tstransforms/typeserializer.go index 306c70ab38663..21c5d183370a5 100644 --- a/tsc/internal/transformers/tstransforms/typeserializer.go +++ b/tsc/internal/transformers/tstransforms/typeserializer.go @@ -225,7 +225,7 @@ func (s *metadataSerializer) serializeTypeNode(node *ast.Node) *ast.Node { } // TODO: why is `unique symbol` not handled as `Symbol`? This falls back to `Object` case ast.KindTypeQuery, ast.KindIndexedAccessType, ast.KindMappedType, ast.KindTypeLiteral, ast.KindAnyKeyword, ast.KindUnknownKeyword, ast.KindThisType, ast.KindImportType: - break + // These types fall back to Object. // handle JSDoc types from an invalid parse case ast.KindJSDocAllType, ast.KindJSDocVariadicType: From d196aeb9fcfa2955a3f3fe6b4fa9aaf305499808 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:21:30 -0700 Subject: [PATCH 3/8] Check type switch case bodies Include type switches in the casebody analyzer so empty cases and direct redundant break statements are handled consistently across all Go switch forms. Add type-switch coverage and document the existing intentionally ignored fourslash case. --- tools/customlint/casebody.go | 3 +++ .../customlint/testdata/casebody/casebody.go | 13 +++++++++++++ .../testdata/casebody/casebody.go.golden | 19 +++++++++++++++++++ tsc/internal/fourslash/fourslash.go | 1 + 4 files changed, 36 insertions(+) diff --git a/tools/customlint/casebody.go b/tools/customlint/casebody.go index cafb29c323658..e84caca62a5d6 100644 --- a/tools/customlint/casebody.go +++ b/tools/customlint/casebody.go @@ -33,6 +33,7 @@ func (e *caseBodyPass) run() (any, error) { for c := range in.Root().Preorder( (*ast.File)(nil), (*ast.SwitchStmt)(nil), + (*ast.TypeSwitchStmt)(nil), (*ast.SelectStmt)(nil), ) { switch n := c.Node().(type) { @@ -40,6 +41,8 @@ func (e *caseBodyPass) run() (any, error) { e.file = n case *ast.SwitchStmt: e.checkCases(n.Body) + case *ast.TypeSwitchStmt: + e.checkCases(n.Body) case *ast.SelectStmt: e.checkCases(n.Body) } diff --git a/tools/customlint/testdata/casebody/casebody.go b/tools/customlint/testdata/casebody/casebody.go index fce6970e30e6e..bc568517890ba 100644 --- a/tools/customlint/testdata/casebody/casebody.go +++ b/tools/customlint/testdata/casebody/casebody.go @@ -53,6 +53,19 @@ func SwitchBreak() { } } +func TypeSwitch(x any) { + switch x.(type) { + case int: + case string: + // intentionally empty + case bool: + break + case float64: + println(`oops`) + break + } +} + func SelectBreak() { select { case <-ch: diff --git a/tools/customlint/testdata/casebody/casebody.go.golden b/tools/customlint/testdata/casebody/casebody.go.golden index 74624ac722395..6942f2f479d3b 100644 --- a/tools/customlint/testdata/casebody/casebody.go.golden +++ b/tools/customlint/testdata/casebody/casebody.go.golden @@ -75,6 +75,25 @@ } } + func TypeSwitch(x any) { + switch x.(type) { + case int: + ~~~~~~~~~ +!!! casebody: this case block is empty and will do nothing + case string: + // intentionally empty + case bool: + break + ~~~~~ +!!! casebody: this top-level break statement is redundant + case float64: + println(`oops`) + break + ~~~~~ +!!! casebody: this top-level break statement is redundant + } + } + func SelectBreak() { select { case <-ch: diff --git a/tsc/internal/fourslash/fourslash.go b/tsc/internal/fourslash/fourslash.go index fe61e1b5c4815..0476fb1c3ca88 100644 --- a/tsc/internal/fourslash/fourslash.go +++ b/tsc/internal/fourslash/fourslash.go @@ -1406,6 +1406,7 @@ func verifyCompletionsItemDefaults(t *testing.T, actual *lsproto.CompletionItemD t.Fatalf(prefix+"Expected nil EditRange but got non-nil: %s", cmp.Diff(actual.EditRange, nil)) } case Ignored: + // The edit range is intentionally ignored. default: t.Fatalf(prefix+"Expected EditRange to be *EditRange or Ignored, got %T", editRange) } From 29e350204ada5a6bc253030df96c3e315c9be13b Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:24:01 -0700 Subject: [PATCH 4/8] Test labeled breaks in case bodies Add regression coverage confirming that the casebody analyzer permits a direct labeled break which exits an enclosing loop. --- tools/customlint/testdata/casebody/casebody.go | 10 ++++++++++ tools/customlint/testdata/casebody/casebody.go.golden | 10 ++++++++++ 2 files changed, 20 insertions(+) diff --git a/tools/customlint/testdata/casebody/casebody.go b/tools/customlint/testdata/casebody/casebody.go index bc568517890ba..3ff941ef0c174 100644 --- a/tools/customlint/testdata/casebody/casebody.go +++ b/tools/customlint/testdata/casebody/casebody.go @@ -53,6 +53,16 @@ func SwitchBreak() { } } +func SwitchLabeledBreak() { +outer: + for { + switch X { + case 1: + break outer + } + } +} + func TypeSwitch(x any) { switch x.(type) { case int: diff --git a/tools/customlint/testdata/casebody/casebody.go.golden b/tools/customlint/testdata/casebody/casebody.go.golden index 6942f2f479d3b..bca1445e94cc5 100644 --- a/tools/customlint/testdata/casebody/casebody.go.golden +++ b/tools/customlint/testdata/casebody/casebody.go.golden @@ -75,6 +75,16 @@ } } + func SwitchLabeledBreak() { + outer: + for { + switch X { + case 1: + break outer + } + } + } + func TypeSwitch(x any) { switch x.(type) { case int: From d6db6bbdccd5c38be36b19ceca3aee8ae15f1ea0 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:27:01 -0700 Subject: [PATCH 5/8] Reject code after case breaks Report the first statement following a direct break in a switch, type-switch, or select case body. This also ensures labeled breaks are only accepted when they terminate the case body. --- tools/customlint/casebody.go | 18 ++++++++++++++--- .../customlint/testdata/casebody/casebody.go | 14 +++++++++++++ .../testdata/casebody/casebody.go.golden | 20 +++++++++++++++++++ 3 files changed, 49 insertions(+), 3 deletions(-) diff --git a/tools/customlint/casebody.go b/tools/customlint/casebody.go index e84caca62a5d6..91c5ae9efb68d 100644 --- a/tools/customlint/casebody.go +++ b/tools/customlint/casebody.go @@ -13,7 +13,7 @@ import ( var caseBodyAnalyzer = &analysis.Analyzer{ Name: "casebody", - Doc: "finds empty switch/select cases and redundant break statements", + Doc: "finds empty switch/select cases, redundant break statements, and code after breaks", Requires: []*analysis.Analyzer{ inspect.Analyzer, }, @@ -78,14 +78,26 @@ func (e *caseBodyPass) checkCaseStatement(stmt ast.Stmt, nextCasePos token.Pos) panic(fmt.Sprintf("unhandled statement type %T", stmt)) } - for _, statement := range body { - if branch, ok := statement.(*ast.BranchStmt); ok && branch.Tok == token.BREAK && branch.Label == nil { + 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 i+1 < len(body) { + e.pass.Report(analysis.Diagnostic{ + Pos: body[i+1].Pos(), + End: body[i+1].End(), + Message: "this statement is unreachable after a break", + }) + break + } } if len(body) == 1 { diff --git a/tools/customlint/testdata/casebody/casebody.go b/tools/customlint/testdata/casebody/casebody.go index 3ff941ef0c174..5021efe7b73aa 100644 --- a/tools/customlint/testdata/casebody/casebody.go +++ b/tools/customlint/testdata/casebody/casebody.go @@ -50,6 +50,9 @@ func SwitchBreak() { for { break } + case 5: + break + println(`unreachable`) } } @@ -63,6 +66,17 @@ outer: } } +func SwitchCodeAfterLabeledBreak() { +outer: + for { + switch X { + case 1: + break outer + println(`unreachable`) + } + } +} + func TypeSwitch(x any) { switch x.(type) { case int: diff --git a/tools/customlint/testdata/casebody/casebody.go.golden b/tools/customlint/testdata/casebody/casebody.go.golden index bca1445e94cc5..769463c38b48d 100644 --- a/tools/customlint/testdata/casebody/casebody.go.golden +++ b/tools/customlint/testdata/casebody/casebody.go.golden @@ -72,6 +72,13 @@ for { break } + case 5: + break + ~~~~~ +!!! casebody: this top-level break statement is redundant + println(`unreachable`) + ~~~~~~~~~~~~~~~~~~~~~~ +!!! casebody: this statement is unreachable after a break } } @@ -85,6 +92,19 @@ } } + func SwitchCodeAfterLabeledBreak() { + outer: + for { + switch X { + case 1: + break outer + println(`unreachable`) + ~~~~~~~~~~~~~~~~~~~~~~ +!!! casebody: this statement is unreachable after a break + } + } + } + func TypeSwitch(x any) { switch x.(type) { case int: From 52dbc0331a598063191bef87428ab8153eb3e18f Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:29:44 -0700 Subject: [PATCH 6/8] Keep checking breaks after unreachable code Continue scanning a case body after reporting its first unreachable statement so later unlabeled breaks are still rejected. Add ordinary, type-switch, and select coverage for code following a break. --- tools/customlint/casebody.go | 5 +++-- tools/customlint/testdata/casebody/casebody.go | 7 +++++++ .../testdata/casebody/casebody.go.golden | 17 +++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/tools/customlint/casebody.go b/tools/customlint/casebody.go index 91c5ae9efb68d..4aecd359eaa47 100644 --- a/tools/customlint/casebody.go +++ b/tools/customlint/casebody.go @@ -78,6 +78,7 @@ func (e *caseBodyPass) 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 { @@ -90,13 +91,13 @@ func (e *caseBodyPass) checkCaseStatement(stmt ast.Stmt, nextCasePos token.Pos) Message: "this top-level break statement is redundant", }) } - if i+1 < len(body) { + if !reportedUnreachable && i+1 < len(body) { e.pass.Report(analysis.Diagnostic{ Pos: body[i+1].Pos(), End: body[i+1].End(), Message: "this statement is unreachable after a break", }) - break + reportedUnreachable = true } } diff --git a/tools/customlint/testdata/casebody/casebody.go b/tools/customlint/testdata/casebody/casebody.go index 5021efe7b73aa..a1d22030da756 100644 --- a/tools/customlint/testdata/casebody/casebody.go +++ b/tools/customlint/testdata/casebody/casebody.go @@ -53,6 +53,7 @@ func SwitchBreak() { case 5: break println(`unreachable`) + break } } @@ -87,6 +88,9 @@ func TypeSwitch(x any) { case float64: println(`oops`) break + case complex64: + break + println(`unreachable`) } } @@ -97,6 +101,9 @@ func SelectBreak() { default: println(`oops`) break + case <-ch2: + break + println(`unreachable`) } } diff --git a/tools/customlint/testdata/casebody/casebody.go.golden b/tools/customlint/testdata/casebody/casebody.go.golden index 769463c38b48d..7a510b329dbe8 100644 --- a/tools/customlint/testdata/casebody/casebody.go.golden +++ b/tools/customlint/testdata/casebody/casebody.go.golden @@ -79,6 +79,9 @@ println(`unreachable`) ~~~~~~~~~~~~~~~~~~~~~~ !!! casebody: this statement is unreachable after a break + break + ~~~~~ +!!! casebody: this top-level break statement is redundant } } @@ -121,6 +124,13 @@ break ~~~~~ !!! casebody: this top-level break statement is redundant + case complex64: + break + ~~~~~ +!!! casebody: this top-level break statement is redundant + println(`unreachable`) + ~~~~~~~~~~~~~~~~~~~~~~ +!!! casebody: this statement is unreachable after a break } } @@ -135,6 +145,13 @@ break ~~~~~ !!! casebody: this top-level break statement is redundant + case <-ch2: + break + ~~~~~ +!!! casebody: this top-level break statement is redundant + println(`unreachable`) + ~~~~~~~~~~~~~~~~~~~~~~ +!!! casebody: this statement is unreachable after a break } } From d07f25254d266fcc42c3e99455d755a6f1e2f9da Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:31:13 -0700 Subject: [PATCH 7/8] Clarify the case break layout rule Describe statements after a direct break as structurally disallowed rather than necessarily unreachable, since a following label may be a goto target. Add coverage showing that the layout remains banned even when the labeled statement is otherwise reachable. --- stdin.o | Bin 0 -> 2692 bytes tools/customlint/casebody.go | 2 +- .../customlint/testdata/casebody/casebody.go | 7 ++++++ .../testdata/casebody/casebody.go.golden | 20 ++++++++++++++---- 4 files changed, 24 insertions(+), 5 deletions(-) create mode 100644 stdin.o diff --git a/stdin.o b/stdin.o new file mode 100644 index 0000000000000000000000000000000000000000..0f1020bf10c4224c43bedc2181e8875822df483c GIT binary patch literal 2692 zcmd5;Yitx%6uvV%Znsn1ZbSvzlHHaD1-8@eYm2pQcvGzw6baa%-RbV^bj$9}cJ=|H zv5*1*iIMOJ5g=>^Jw^ z@1A?^z2}}Yv*pdQ9_(oqySk+L?Q>_%p5rmaH{4QV>gLm5UvG@m`YnrqsA?XqyH^RO zJkdxzz1$s?|ufjgrqZci!{|XZh<}GBuvXO}e7W-H~OwoJc5o(rb2QT}^sJ z%Va&=;O$Ln@l35()fFY4Qe-vg)#Z3di$zu_p&ms}bjwM_U++z#pG+vhh#aMnm>i92 zK~WSdD`$vlftg}MbIUO7Xm19x=73v)+kiU&H;`yf$8n^ka(7f|<;WX^8jkTi$NGz> z?09Q)O&JyyyHu@9(yj*0iq1{rSLdrUt^Z%~D;F6quGSX~S*LU0ypJO~gfNf+jt%1% zp0N@HWJ?Zu5dC)n7a|`5oJ1(a=k^~St^?4bV`GY95I@D*}o2 z3<(%dkoe`3R2qNJ@`gUK|NQB+V4hE^S0bqfn~YJLjTH`)W6;I{Y|>_gjtuCyYlveu zj#0K!`IF@iAe#kg`78mk&mxh!#zIu+zXObovNmJ-9BXuQNb9Z)x=YJz?tjG&hDR&D5p-mQ1Qs=hRdO!Htc)g&Q0p>ed$z3-?pLXn`4d-_NnzReQ|d4 z(eLl~y*K_y=fa6|&ebeB+uyeCb6acRmF%WZorCO+_jkFs?_Rrq$-(w%_9u$&DmhWt zx2mDEtLT%jR=v~expJsQczI<*nRH~eJ2)YB`P|`gQ`U}pe&g0x52g<+N)&GS@Wl(f zV=(sQp<`c?ZQ6mIPYep)bv5hCgzP)$9 z{zvP!&tf~D$utM{e0gcjHv__zupO!$)%%^_?g;g1+;$h04@J*UgS@|@*0~|EP6w8`kH#cxXB_zluBfs0() zz4~aX^c2Qe+p{!@?WGeKgvkzk?0Rf}+xl6Rj&(~aT_(2yz2djg{oyqV_*j|;q5T-Q zX!J-I*m!RKaElZz967$gyTs%@_D1{EiT3ICGq$JZ@+FMpY`NbvDU7`4w$dD)t6H8D z#j2nd59Rb3U*Ib6OmcI@rA%1B>`uYK^Y&7~UW^wi))hFOuj8CN!aSBQ=Ri7z*=*!< UKB9;t3FUBbSq2OGFPLC|0clDYzyJUM literal 0 HcmV?d00001 diff --git a/tools/customlint/casebody.go b/tools/customlint/casebody.go index 4aecd359eaa47..1d7b86e83cfa7 100644 --- a/tools/customlint/casebody.go +++ b/tools/customlint/casebody.go @@ -95,7 +95,7 @@ func (e *caseBodyPass) checkCaseStatement(stmt ast.Stmt, nextCasePos token.Pos) e.pass.Report(analysis.Diagnostic{ Pos: body[i+1].Pos(), End: body[i+1].End(), - Message: "this statement is unreachable after a break", + Message: "statements after a break are not allowed in the same case body", }) reportedUnreachable = true } diff --git a/tools/customlint/testdata/casebody/casebody.go b/tools/customlint/testdata/casebody/casebody.go index a1d22030da756..15b7fc61f8211 100644 --- a/tools/customlint/testdata/casebody/casebody.go +++ b/tools/customlint/testdata/casebody/casebody.go @@ -54,6 +54,13 @@ func SwitchBreak() { break println(`unreachable`) break + case 6: + if X != 0 { + goto afterBreak + } + break + afterBreak: + println(`reachable via goto`) } } diff --git a/tools/customlint/testdata/casebody/casebody.go.golden b/tools/customlint/testdata/casebody/casebody.go.golden index 7a510b329dbe8..d5b5d58e69123 100644 --- a/tools/customlint/testdata/casebody/casebody.go.golden +++ b/tools/customlint/testdata/casebody/casebody.go.golden @@ -78,10 +78,22 @@ !!! casebody: this top-level break statement is redundant println(`unreachable`) ~~~~~~~~~~~~~~~~~~~~~~ -!!! casebody: this statement is unreachable after a break +!!! casebody: statements after a break are not allowed in the same case body break ~~~~~ !!! casebody: this top-level break statement is redundant + case 6: + if X != 0 { + goto afterBreak + } + break + ~~~~~ +!!! casebody: this top-level break statement is redundant + afterBreak: + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +!!! casebody: statements after a break are not allowed in the same case body + println(`reachable via goto`) + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ } } @@ -103,7 +115,7 @@ break outer println(`unreachable`) ~~~~~~~~~~~~~~~~~~~~~~ -!!! casebody: this statement is unreachable after a break +!!! casebody: statements after a break are not allowed in the same case body } } } @@ -130,7 +142,7 @@ !!! casebody: this top-level break statement is redundant println(`unreachable`) ~~~~~~~~~~~~~~~~~~~~~~ -!!! casebody: this statement is unreachable after a break +!!! casebody: statements after a break are not allowed in the same case body } } @@ -151,7 +163,7 @@ !!! casebody: this top-level break statement is redundant println(`unreachable`) ~~~~~~~~~~~~~~~~~~~~~~ -!!! casebody: this statement is unreachable after a break +!!! casebody: statements after a break are not allowed in the same case body } } From 37e31dd99271efd336394ae62aea741d81a8911e Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:31:18 -0700 Subject: [PATCH 8/8] Remove generated review artifact --- stdin.o | Bin 2692 -> 0 bytes 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 stdin.o diff --git a/stdin.o b/stdin.o deleted file mode 100644 index 0f1020bf10c4224c43bedc2181e8875822df483c..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 2692 zcmd5;Yitx%6uvV%Znsn1ZbSvzlHHaD1-8@eYm2pQcvGzw6baa%-RbV^bj$9}cJ=|H zv5*1*iIMOJ5g=>^Jw^ z@1A?^z2}}Yv*pdQ9_(oqySk+L?Q>_%p5rmaH{4QV>gLm5UvG@m`YnrqsA?XqyH^RO zJkdxzz1$s?|ufjgrqZci!{|XZh<}GBuvXO}e7W-H~OwoJc5o(rb2QT}^sJ z%Va&=;O$Ln@l35()fFY4Qe-vg)#Z3di$zu_p&ms}bjwM_U++z#pG+vhh#aMnm>i92 zK~WSdD`$vlftg}MbIUO7Xm19x=73v)+kiU&H;`yf$8n^ka(7f|<;WX^8jkTi$NGz> z?09Q)O&JyyyHu@9(yj*0iq1{rSLdrUt^Z%~D;F6quGSX~S*LU0ypJO~gfNf+jt%1% zp0N@HWJ?Zu5dC)n7a|`5oJ1(a=k^~St^?4bV`GY95I@D*}o2 z3<(%dkoe`3R2qNJ@`gUK|NQB+V4hE^S0bqfn~YJLjTH`)W6;I{Y|>_gjtuCyYlveu zj#0K!`IF@iAe#kg`78mk&mxh!#zIu+zXObovNmJ-9BXuQNb9Z)x=YJz?tjG&hDR&D5p-mQ1Qs=hRdO!Htc)g&Q0p>ed$z3-?pLXn`4d-_NnzReQ|d4 z(eLl~y*K_y=fa6|&ebeB+uyeCb6acRmF%WZorCO+_jkFs?_Rrq$-(w%_9u$&DmhWt zx2mDEtLT%jR=v~expJsQczI<*nRH~eJ2)YB`P|`gQ`U}pe&g0x52g<+N)&GS@Wl(f zV=(sQp<`c?ZQ6mIPYep)bv5hCgzP)$9 z{zvP!&tf~D$utM{e0gcjHv__zupO!$)%%^_?g;g1+;$h04@J*UgS@|@*0~|EP6w8`kH#cxXB_zluBfs0() zz4~aX^c2Qe+p{!@?WGeKgvkzk?0Rf}+xl6Rj&(~aT_(2yz2djg{oyqV_*j|;q5T-Q zX!J-I*m!RKaElZz967$gyTs%@_D1{EiT3ICGq$JZ@+FMpY`NbvDU7`4w$dD)t6H8D z#j2nd59Rb3U*Ib6OmcI@rA%1B>`uYK^Y&7~UW^wi))hFOuj8CN!aSBQ=Ri7z*=*!< UKB9;t3FUBbSq2OGFPLC|0clDYzyJUM