From 745bcf65aa5b601714a58cd6ea0f63307884a031 Mon Sep 17 00:00:00 2001 From: SalvadorC Date: Wed, 17 Apr 2019 22:55:52 +0200 Subject: [PATCH] bare-return (first working version) (#117) New rule bare-return --- README.md | 1 + RULES_DESCRIPTIONS.md | 7 ++++ config.go | 1 + fixtures/bare-return.go | 33 +++++++++++++++++ rule/bare-return.go | 77 ++++++++++++++++++++++++++++++++++++++++ test/bare-return_test.go | 11 ++++++ 6 files changed, 130 insertions(+) create mode 100644 fixtures/bare-return.go create mode 100644 rule/bare-return.go create mode 100644 test/bare-return_test.go diff --git a/README.md b/README.md index f0fb2da..23af769 100644 --- a/README.md +++ b/README.md @@ -295,6 +295,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`call-to-gc`](./RULES_DESCRIPTIONS.md#call-to-gc) | n/a | Warns on explicit call to the garbage collector | no | no | | [`duplicated-imports`](./RULES_DESCRIPTIONS#duplicated-imports) | n/a | Looks for packages that are imported two or more times | no | no | | [`import-shadowing`](./RULES_DESCRIPTIONS.md#import-shadowing) | n/a | Spots identifiers that shadow an import | no | no | +| [`bare-return`](./RULES_DESCRIPTIONS#bare-return) | n/a | Warns on bare returns | no | no | ## Configurable rules diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 0e9a586..5e05556 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -7,6 +7,7 @@ List of all available rules. - [add-constant](#add-constant) - [argument-limit](#argument-limit) - [atomic](#atomic) + - [bare-return](#bare-return) - [blank-imports](#blank-imports) - [bool-literal-in-expr](#bool-literal-in-expr) - [call-to-gc](#call-to-gc) @@ -93,6 +94,12 @@ _Description_: Check for commonly mistaken usages of the `sync/atomic` package _Configuration_: N/A +## bare-return + +_Description_: Warns on bare (a.k.a. naked) returns + +_Configuration_: N/A + ## blank-imports _Description_: Blank import should be only in a main or test package, or have a comment justifying it. diff --git a/config.go b/config.go index b1f5855..a342189 100644 --- a/config.go +++ b/config.go @@ -75,6 +75,7 @@ var allRules = append([]lint.Rule{ &rule.CallToGCRule{}, &rule.DuplicatedImportsRule{}, &rule.ImportShadowingRule{}, + &rule.BareReturnRule{}, }, defaultRules...) var allFormatters = []lint.Formatter{ diff --git a/fixtures/bare-return.go b/fixtures/bare-return.go new file mode 100644 index 0000000..d2d3f8e --- /dev/null +++ b/fixtures/bare-return.go @@ -0,0 +1,33 @@ +package fixtures + +func bare1() (int, int, error) { + go func() (a int) { + return // MATCH /avoid using bare returns, please add return expressions/ + }(5) +} + +func bare2(a, b int) (int, error, int) { + defer func() (a int) { + return // MATCH /avoid using bare returns, please add return expressions/ + }(5) +} + +func bare3(a string, b int) (a int, b float32, c string, d string) { + go func() (a int, b int) { + return a, b + }(5, 6) + + defer func() (a int) { + return a + }(5) + + return // MATCH /avoid using bare returns, please add return expressions/ +} + +func bare4(a string, b int) string { + return a +} + +func bare5(a string, b int) { + return +} diff --git a/rule/bare-return.go b/rule/bare-return.go new file mode 100644 index 0000000..dbfad2f --- /dev/null +++ b/rule/bare-return.go @@ -0,0 +1,77 @@ +package rule + +import ( + "go/ast" + + "github.com/mgechev/revive/lint" +) + +// BareReturnRule lints given else constructs. +type BareReturnRule struct{} + +// Apply applies the rule to given file. +func (r *BareReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { + var failures []lint.Failure + + onFailure := func(failure lint.Failure) { + failures = append(failures, failure) + } + + w := lintBareReturnRule{onFailure: onFailure} + ast.Walk(w, file.AST) + return failures +} + +// Name returns the rule name. +func (r *BareReturnRule) Name() string { + return "bare-return" +} + +type lintBareReturnRule struct { + onFailure func(lint.Failure) +} + +func (w lintBareReturnRule) Visit(node ast.Node) ast.Visitor { + switch n := node.(type) { + case *ast.FuncDecl: + w.checkFunc(n.Type.Results, n.Body) + case *ast.FuncLit: // to cope with deferred functions and go-routines + w.checkFunc(n.Type.Results, n.Body) + } + + return w +} + +// checkFunc will verify if the given function has named result and bare returns +func (w lintBareReturnRule) checkFunc(results *ast.FieldList, body *ast.BlockStmt) { + hasNamedResults := results != nil && len(results.List) > 0 && results.List[0].Names != nil + if !hasNamedResults || body == nil { + return // nothing to do + } + + brf := bareReturnFinder{w.onFailure} + ast.Walk(brf, body) +} + +type bareReturnFinder struct { + onFailure func(lint.Failure) +} + +func (w bareReturnFinder) Visit(node ast.Node) ast.Visitor { + rs, ok := node.(*ast.ReturnStmt) + if !ok { + return w + } + + if len(rs.Results) > 0 { + return w + } + + w.onFailure(lint.Failure{ + Confidence: 1, + Node: rs, + Failure: "avoid using bare returns, please add return expressions", + }) + + return w +} diff --git a/test/bare-return_test.go b/test/bare-return_test.go new file mode 100644 index 0000000..1741334 --- /dev/null +++ b/test/bare-return_test.go @@ -0,0 +1,11 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/rule" +) + +func TestBareReturn(t *testing.T) { + testRule(t, "bare-return", &rule.BareReturnRule{}) +}