Ban case blocks with just "break", top level break - #64153
Open
Jake Bailey (jakebailey) wants to merge 8 commits into
Open
Ban case blocks with just "break", top level break#64153Jake Bailey (jakebailey) wants to merge 8 commits into
Jake Bailey (jakebailey) wants to merge 8 commits into
Conversation
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.
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.
typescript-automation
Bot
added
Author: Team
For Uncommitted Bug
Copilot started reviewing on behalf of
Jake Bailey (jakebailey)
September 3, 2026 19:20
View session
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.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The analyzer incorrectly reports non-final case-level breaks whose removal would change runtime behavior.
Pull request overview
Renames and extends the custom case-body lint rule while removing redundant Go break statements.
Changes:
- Adds detection for redundant case-level breaks and type-switch cases.
- Renames
emptycasetocasebodyand expands golden tests. - Removes redundant breaks or documents intentionally empty cases.
File summaries
| File | Description |
|---|---|
tsc/internal/transformers/tstransforms/typeserializer.go |
Documents empty serialization cases. |
tsc/internal/transformers/estransforms/namedevaluation.go |
Removes redundant breaks. |
tsc/internal/printer/utilities.go |
Documents an empty infer-type case. |
tsc/internal/printer/printer.go |
Removes or documents redundant breaks. |
tsc/internal/ls/folding.go |
Removes redundant breaks. |
tsc/internal/ls/autoimport/util.go |
Documents select fallback behavior. |
tsc/internal/fourslash/fourslash.go |
Documents intentionally ignored edit ranges. |
tsc/internal/format/span.go |
Removes redundant breaks. |
tsc/internal/format/scanner.go |
Documents normal scanning behavior. |
tools/customlint/testdata/emptycase/emptycase.go.golden |
Removes the superseded golden file. |
tools/customlint/testdata/casebody/casebody.go.golden |
Adds expanded lint expectations. |
tools/customlint/testdata/casebody/casebody.go |
Adds break and type-switch cases. |
tools/customlint/plugin.go |
Registers the renamed analyzer. |
tools/customlint/casebody.go |
Expands the analyzer to detect breaks and type switches. |
Review details
Suppressed comments (1)
tools/customlint/casebody.go:82
- This reports every direct
break, but a break before a later statement is not redundant: Go permits that statement to be unreachable, and removing the break causes it to execute. Restrict this diagnostic to an unlabeled break that is the final statement in the case body.
- Files reviewed: 14/14 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Add regression coverage confirming that the casebody analyzer permits a direct labeled break which exits an enclosing loop.
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.
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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Noticed this while reviewing code on another branch. In Go, you don't need to say "break", but some case blocks said just "break" like it's JS without the required commentary. Ban those cases. Also, drop any extra top-level breaks, which are redundant.