diff --git a/README.md b/README.md index 3ad8372..417772b 100644 --- a/README.md +++ b/README.md @@ -220,6 +220,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | `confusing-results` | n/a | Suggests to name potentially confusing function results | no | no | | `deep-exit` | n/a | Looks for program exits in funcs other than `main()` or `init()` | no | no | | `unused-parameter` | n/a | Suggests to rename or remove unused function parameters | no | no | +| `unreachable-code` | n/a | Warns on unreachable code | no | no | ## Available Formatters diff --git a/config.go b/config.go index 4ce1020..25a7f72 100644 --- a/config.go +++ b/config.go @@ -55,6 +55,7 @@ var allRules = append([]lint.Rule{ &rule.ConfusingResultsRule{}, &rule.DeepExitRule{}, &rule.UnusedParamRule{}, + &rule.UnreachableCodeRule{}, }, defaultRules...) var allFormatters = []lint.Formatter{ diff --git a/fixtures/unreachable-code.go b/fixtures/unreachable-code.go new file mode 100644 index 0000000..346e1cb --- /dev/null +++ b/fixtures/unreachable-code.go @@ -0,0 +1,37 @@ +package fixtures + +import ( + "fmt" + "log" + "os" +) + +func foo() int { + log.Fatalf("%s", "About to fail") // ignore + return 0 // MATCH /unreachable code after this statement/ + return 1 + Println("unreachable") +} + +func f() { + fmt.Println("Hello, playground") + if true { + return // MATCH /unreachable code after this statement/ + Println("unreachable") + os.Exit(2) // ignore + Println("also unreachable") + } + return // MATCH /unreachable code after this statement/ + fmt.Println("Bye, playground") +} + +func g() { + fmt.Println("Hello, playground") + if true { + return // ignore if next stmt is labeled + label: + os.Exit(2) // ignore + } + + fmt.Println("Bye, playground") +} diff --git a/rule/unreachable-code.go b/rule/unreachable-code.go new file mode 100644 index 0000000..6e9a4ca --- /dev/null +++ b/rule/unreachable-code.go @@ -0,0 +1,114 @@ +package rule + +import ( + "go/ast" + + "github.com/mgechev/revive/lint" +) + +// UnreachableCodeRule lints unreachable code. +type UnreachableCodeRule struct{} + +// Apply applies the rule to given file. +func (r *UnreachableCodeRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + var failures []lint.Failure + onFailure := func(failure lint.Failure) { + failures = append(failures, failure) + } + + var branchingFunctions = map[string]map[string]bool{ + "os": map[string]bool{"Exit": true}, + "log": map[string]bool{ + "Fatal": true, + "Fatalf": true, + "Fatalln": true, + "Panic": true, + "Panicf": true, + "Panicln": true, + }, + } + + w := lintUnreachableCode{onFailure, branchingFunctions} + ast.Walk(w, file.AST) + return failures +} + +// Name returns the rule name. +func (r *UnreachableCodeRule) Name() string { + return "unreachable-code" +} + +type lintUnreachableCode struct { + onFailure func(lint.Failure) + branchingFunctions map[string]map[string]bool +} + +func (w lintUnreachableCode) Visit(node ast.Node) ast.Visitor { + blk, ok := node.(*ast.BlockStmt) + if !ok { + return w + } + + if len(blk.List) < 2 { + return w + } +loop: + for i, stmt := range blk.List[:len(blk.List)-1] { + // println("iterating ", len(blk.List)) + next := blk.List[i+1] + if _, ok := next.(*ast.LabeledStmt); ok { + continue // skip if next statement is labeled + } + + switch s := stmt.(type) { + case *ast.ReturnStmt: + w.onFailure(newUnreachableCodeFailure(s)) + break loop + case *ast.BranchStmt: + token := s.Tok.String() + if token != "fallthrough" { + w.onFailure(newUnreachableCodeFailure(s)) + break loop + } + case *ast.ExprStmt: + ce, ok := s.X.(*ast.CallExpr) + if !ok { + continue + } + // it's a function call + fc, ok := ce.Fun.(*ast.SelectorExpr) + if !ok { + continue + } + + id, ok := fc.X.(*ast.Ident) + + if !ok { + continue + } + fn := fc.Sel.Name + pkg := id.Name + if !w.branchingFunctions[pkg][fn] { // it isn't a call to a branching function + continue + } + + if _, ok := next.(*ast.ReturnStmt); ok { // return statement needed to satisfy function signature + continue + } + + w.onFailure(newUnreachableCodeFailure(s)) + break loop + } + } + + return w +} + +func newUnreachableCodeFailure(node ast.Node) lint.Failure { + return lint.Failure{ + Confidence: 1, + Node: node, + Category: "logic", + Failure: "unreachable code after this statement", + } +} diff --git a/test/unreachable-code_test.go b/test/unreachable-code_test.go new file mode 100644 index 0000000..e8c3de9 --- /dev/null +++ b/test/unreachable-code_test.go @@ -0,0 +1,11 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/rule" +) + +func TestUnreachableCode(t *testing.T) { + testRule(t, "unreachable-code", &rule.UnreachableCodeRule{}) +}