diff --git a/tools/customlint/emptycase.go b/tools/customlint/casebody.go similarity index 60% rename from tools/customlint/emptycase.go rename to tools/customlint/casebody.go index 430b9186a9554..1d7b86e83cfa7 100644 --- a/tools/customlint/emptycase.go +++ b/tools/customlint/casebody.go @@ -11,28 +11,29 @@ 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) { @@ -40,6 +41,8 @@ func (e *emptyCasePass) 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) } @@ -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 { 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/casebody/casebody.go b/tools/customlint/testdata/casebody/casebody.go new file mode 100644 index 0000000000000..15b7fc61f8211 --- /dev/null +++ b/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: + } +} diff --git a/tools/customlint/testdata/casebody/casebody.go.golden b/tools/customlint/testdata/casebody/casebody.go.golden new file mode 100644 index 0000000000000..d5b5d58e69123 --- /dev/null +++ b/tools/customlint/testdata/casebody/casebody.go.golden @@ -0,0 +1,217 @@ + 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 + } + case 5: + break + ~~~~~ +!!! casebody: this top-level break statement is redundant + println(`unreachable`) + ~~~~~~~~~~~~~~~~~~~~~~ +!!! 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`) + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + } + } + + func SwitchLabeledBreak() { + outer: + for { + switch X { + case 1: + break outer + } + } + } + + func SwitchCodeAfterLabeledBreak() { + outer: + for { + switch X { + case 1: + break outer + println(`unreachable`) + ~~~~~~~~~~~~~~~~~~~~~~ +!!! casebody: statements after a break are not allowed in the same case body + } + } + } + + 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 + case complex64: + break + ~~~~~ +!!! casebody: this top-level break statement is redundant + println(`unreachable`) + ~~~~~~~~~~~~~~~~~~~~~~ +!!! casebody: statements after a break are not allowed in the same case body + } + } + + 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 + case <-ch2: + break + ~~~~~ +!!! casebody: this top-level break statement is redundant + println(`unreachable`) + ~~~~~~~~~~~~~~~~~~~~~~ +!!! casebody: statements after a break are not allowed in the same case body + } + } + + 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 b/tools/customlint/testdata/emptycase/emptycase.go deleted file mode 100644 index 5d3e55a94e580..0000000000000 --- a/tools/customlint/testdata/emptycase/emptycase.go +++ /dev/null @@ -1,75 +0,0 @@ -package emptycase - -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: - } -} - -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: - } -} diff --git a/tools/customlint/testdata/emptycase/emptycase.go.golden b/tools/customlint/testdata/emptycase/emptycase.go.golden deleted file mode 100644 index 8f30663b891b6..0000000000000 --- a/tools/customlint/testdata/emptycase/emptycase.go.golden +++ /dev/null @@ -1,102 +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 - } - } - - 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/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/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/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) } 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/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 31547cde51d64..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: @@ -4879,7 +4875,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..3b986a5b9d009 100644 --- a/tsc/internal/transformers/estransforms/namedevaluation.go +++ b/tsc/internal/transformers/estransforms/namedevaluation.go @@ -67,14 +67,12 @@ 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: - 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..21c5d183370a5 100644 --- a/tsc/internal/transformers/tstransforms/typeserializer.go +++ b/tsc/internal/transformers/tstransforms/typeserializer.go @@ -225,11 +225,11 @@ 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: - break + // no meaningful serialization for these invalid-parse JSDoc types case ast.KindJSDocNullableType, ast.KindJSDocNonNullableType, ast.KindJSDocOptionalType: return s.serializeTypeNode(node.Type()) default: