From fb3c2d09af6e2d84f636940969a3b14ac048d67c Mon Sep 17 00:00:00 2001 From: mgechev Date: Sat, 9 Jun 2018 13:18:47 -0700 Subject: [PATCH 1/4] Update list of contributors --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 80dfd38..2a14046 100644 --- a/README.md +++ b/README.md @@ -317,9 +317,9 @@ Currently, type checking is enabled by default. If you want to run the linter wi ## Contributors -| [mgechev](https://github.com/mgechev) | [paul-at-start](https://github.com/paul-at-start) | [chavacava](https://github.com/chavacava) | -| :---------------------------------------------------------------------------------------------------------------------------: | :----------------------------------------------------------------------------------------------------------------------------------------: | :---------------------------------------------------------------------------------------------------------------------------------: | -| [mgechev](https://github.com/mgechev) | [paul-at-start](https://github.com/paul-at-start) | [chavacava](https://github.com/chavacava) | +| [mgechev](https://github.com/mgechev) | [chavacava](https://github.com/chavacava) | [paul-at-start](https://github.com/paul-at-start) | +| :---------------------------------------------------------------------------------------------------------------------------: | :---------------------------------------------------------------------------------------------------------------------------------: | :----------------------------------------------------------------------------------------------------------------------------------------: | +| [mgechev](https://github.com/mgechev) | [chavacava](https://github.com/chavacava) | [paul-at-start](https://github.com/paul-at-start) | ## License From 1b2ffe282e90c75ce1e21ec24573d37a2d595ff0 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Mon, 11 Jun 2018 14:22:06 -0400 Subject: [PATCH 2/4] Improve tests (#18) * Use subtests * Make unexported-return type check --- fixtures/golint/unexported-return.go | 10 ++++++++-- test/golint_test.go | 22 ++++++++++------------ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/fixtures/golint/unexported-return.go b/fixtures/golint/unexported-return.go index ef20638..8a711ac 100644 --- a/fixtures/golint/unexported-return.go +++ b/fixtures/golint/unexported-return.go @@ -12,14 +12,20 @@ func Exported() hidden { // MATCH /exported func Exported returns unexported typ // ExpErr returns a builtin type. func ExpErr() error { // ok + return nil } -func (hidden) ExpOnHidden() hidden { // ok +func (h hidden) ExpOnHidden() hidden { // ok + return h +} + +func (hidden) ForInterface() error { + return nil } // Interface is exported. type Interface interface { - ExpOnHidden() + ForInterface() error } // ExportedAsInterface returns a hidden type as an exported interface, which is fine. diff --git a/test/golint_test.go b/test/golint_test.go index f63d8e7..492d8f5 100644 --- a/test/golint_test.go +++ b/test/golint_test.go @@ -7,9 +7,8 @@ import ( "regexp" "testing" - "github.com/mgechev/revive/rule" - "github.com/mgechev/revive/lint" + "github.com/mgechev/revive/rule" ) var lintMatch = flag.String("lint.match", "", "restrict fixtures matches to this pattern") @@ -55,16 +54,15 @@ func TestAll(t *testing.T) { if !rx.MatchString(fi.Name()) { continue } - //t.Logf("Testing %s", fi.Name()) - src, err := ioutil.ReadFile(path.Join(baseDir, fi.Name())) - if err != nil { - t.Fatalf("Failed reading %s: %v", fi.Name(), err) - } + t.Run(fi.Name(), func(t *testing.T) { + src, err := ioutil.ReadFile(path.Join(baseDir, fi.Name())) + if err != nil { + t.Fatalf("Failed reading %s: %v", fi.Name(), err) + } - err = assertFailures(t, baseDir, fi, src, rules, map[string]lint.RuleConfig{}) - if err != nil { - t.Errorf("Linting %s: %v", fi.Name(), err) - continue - } + if err := assertFailures(t, baseDir, fi, src, rules, map[string]lint.RuleConfig{}); err != nil { + t.Errorf("Linting %s: %v", fi.Name(), err) + } + }) } } From 32df8ca8800dd58e1cd9e368a1bb67d4401c2bd0 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Mon, 11 Jun 2018 14:22:33 -0400 Subject: [PATCH 3/4] Avoid false positives when returning interface values (#6) * Use subtests * Make unexported-return type check * Avoid false positives when returning interface values Fixes #5. --- rule/unexported-return.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/rule/unexported-return.go b/rule/unexported-return.go index c14aff1..27ca4c4 100644 --- a/rule/unexported-return.go +++ b/rule/unexported-return.go @@ -84,8 +84,16 @@ func (w lintUnexportedReturn) Visit(n ast.Node) ast.Visitor { func exportedType(typ types.Type) bool { switch T := typ.(type) { case *types.Named: + obj := T.Obj() + switch { // Builtin types have no package. - return T.Obj().Pkg() == nil || T.Obj().Exported() + case obj.Pkg() == nil: + case obj.Exported(): + default: + _, ok := T.Underlying().(*types.Interface) + return ok + } + return true case *types.Map: return exportedType(T.Key()) && exportedType(T.Elem()) case interface { From c9bde6c5035419ff7af6668e3abb51a0dc3642da Mon Sep 17 00:00:00 2001 From: chavacava Date: Mon, 11 Jun 2018 21:08:59 +0200 Subject: [PATCH 4/4] empty-block: ignore checking blocks of funcs and func literals (#17) * Adds rule superfluous-else (an extension of indent-error-flow) * Fix superfluous-else rule struct namming. * Adds superfuous-else rule to the rules table * Modifies empty-block rule to ignore bodies of func literals and funcs * add test cases on functions with a receiver for completeness --- fixtures/empty-block.go | 9 +++++++-- rule/empty-block.go | 29 +++++++++++++++++++++++++++-- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/fixtures/empty-block.go b/fixtures/empty-block.go index 79b9dde..9f04d5b 100644 --- a/fixtures/empty-block.go +++ b/fixtures/empty-block.go @@ -2,14 +2,19 @@ package fixtures -func f(x int) bool { // MATCH /this block is empty, you can remove it/ +func f(x int) bool {} // Must not match -} +type foo struct {} + +func (f foo) f(x *int) string {} // Must not match +func (f *foo) g(y *int) string {} // Must not match func g(f func() bool) string { { // MATCH /this block is empty, you can remove it/ } + a := func(e error){} // Must not match + if ok := f(); ok { // MATCH /this block is empty, you can remove it/ // only a comment } else { diff --git a/rule/empty-block.go b/rule/empty-block.go index 11e4af3..eed573f 100644 --- a/rule/empty-block.go +++ b/rule/empty-block.go @@ -17,7 +17,7 @@ func (r *EmptyBlockRule) Apply(file *lint.File, arguments lint.Arguments) []lint failures = append(failures, failure) } - w := lintEmptyBlock{make(map[*ast.IfStmt]bool), onFailure} + w := lintEmptyBlock{make([]*ast.BlockStmt, 0), onFailure} ast.Walk(w, file.AST) return failures } @@ -28,16 +28,32 @@ func (r *EmptyBlockRule) Name() string { } type lintEmptyBlock struct { - ignore map[*ast.IfStmt]bool + ignore []*ast.BlockStmt onFailure func(lint.Failure) } func (w lintEmptyBlock) Visit(node ast.Node) ast.Visitor { + fd, ok := node.(*ast.FuncDecl) + if ok { + w.ignore = append(w.ignore, fd.Body) + return w + } + + fl, ok := node.(*ast.FuncLit) + if ok { + w.ignore = append(w.ignore, fl.Body) + return w + } + block, ok := node.(*ast.BlockStmt) if !ok { return w } + if mustIgnore(block, w.ignore) { + return w + } + if len(block.List) == 0 { w.onFailure(lint.Failure{ Confidence: 1, @@ -50,3 +66,12 @@ func (w lintEmptyBlock) Visit(node ast.Node) ast.Visitor { return w } + +func mustIgnore(block *ast.BlockStmt, blackList []*ast.BlockStmt) bool { + for _, b := range blackList { + if b == block { + return true + } + } + return false +}