diff --git a/README.md b/README.md index 2c710ce..6ad2c30 100644 --- a/README.md +++ b/README.md @@ -318,9 +318,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 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/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/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 +} 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 { 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) + } + }) } }