golint: Removed redundant if ...; err != nil check
This is not within the scope of golint due to the style suggestion not appearing in CodeReviewComments or Effective Go.
This change:
* Removes testdata/iferr.go (thus removing the test)
* Removes lintIfError function and references to it
Fixes #388
Change-Id: Ic639e29cafa0152ba61b222915261c37992ed71f
GitHub-Last-Rev: 5bf0c7e9f6c740bcc227a987a0836f45d42613fd
GitHub-Pull-Request: golang/lint#388
Reviewed-on: https://go-review.googlesource.com/100841
Reviewed-by: Andrew Bonventre <andybons@golang.org>
diff --git a/lint.go b/lint.go
index 0305e8a..bb71cc2 100644
--- a/lint.go
+++ b/lint.go
@@ -200,7 +200,6 @@
f.lintNames()
f.lintVarDecls()
f.lintElses()
- f.lintIfError()
f.lintRanges()
f.lintErrorf()
f.lintErrors()
@@ -1507,63 +1506,6 @@
return false
}
-func (f *file) lintIfError() {
- f.walk(func(node ast.Node) bool {
- switch v := node.(type) {
- case *ast.BlockStmt:
- for i := 0; i < len(v.List)-1; i++ {
- // if var := whatever; var != nil { return var }
- s, ok := v.List[i].(*ast.IfStmt)
- if !ok || s.Body == nil || len(s.Body.List) != 1 || s.Else != nil {
- continue
- }
- assign, ok := s.Init.(*ast.AssignStmt)
- if !ok || len(assign.Lhs) != 1 || !(assign.Tok == token.DEFINE || assign.Tok == token.ASSIGN) {
- continue
- }
- id, ok := assign.Lhs[0].(*ast.Ident)
- if !ok {
- continue
- }
- expr, ok := s.Cond.(*ast.BinaryExpr)
- if !ok || expr.Op != token.NEQ {
- continue
- }
- if lhs, ok := expr.X.(*ast.Ident); !ok || lhs.Name != id.Name {
- continue
- }
- if rhs, ok := expr.Y.(*ast.Ident); !ok || rhs.Name != "nil" {
- continue
- }
- r, ok := s.Body.List[0].(*ast.ReturnStmt)
- if !ok || len(r.Results) != 1 {
- continue
- }
- if r, ok := r.Results[0].(*ast.Ident); !ok || r.Name != id.Name {
- continue
- }
-
- // return nil
- r, ok = v.List[i+1].(*ast.ReturnStmt)
- if !ok || len(r.Results) != 1 {
- continue
- }
- if r, ok := r.Results[0].(*ast.Ident); !ok || r.Name != "nil" {
- continue
- }
-
- // check if there are any comments explaining the construct, don't emit an error if there are some.
- if f.containsComments(s.Pos(), r.Pos()) {
- continue
- }
-
- f.errorf(v.List[i], 0.9, "redundant if ...; err != nil check, just return error instead.")
- }
- }
- return true
- })
-}
-
// receiverType returns the named type of the method receiver, sans "*",
// or "invalid-type" if fn.Recv is ill formed.
func receiverType(fn *ast.FuncDecl) string {
diff --git a/testdata/iferr.go b/testdata/iferr.go
deleted file mode 100644
index 920fa9e..0000000
--- a/testdata/iferr.go
+++ /dev/null
@@ -1,101 +0,0 @@
-// Test of redundant if err != nil
-
-// Package pkg ...
-package pkg
-
-func f() error {
- if err := f(); err != nil {
- g()
- return err
- }
- return nil
-}
-
-func g() error {
- if err := f(); err != nil { // MATCH /redundant/
- return err
- }
- return nil
-}
-
-func h() error {
- if err, x := f(), 1; err != nil {
- return err
- }
- return nil
-}
-
-func i() error {
- a := 1
- if err := f(); err != nil {
- a++
- return err
- }
- return nil
-}
-
-func j() error {
- var a error
- if err := f(); err != nil {
- return err
- }
- return a
-}
-
-func k() error {
- if err := f(); err != nil {
- // TODO: handle error better
- return err
- }
- return nil
-}
-
-func l() (interface{}, error) {
- if err := f(); err != nil {
- return nil, err
- }
- if err := f(); err != nil {
- return nil, err
- }
- if err := f(); err != nil {
- return nil, err
- }
- // Phew, it worked
- return nil
-}
-
-func m() error {
- if err := f(); err != nil {
- return err
- }
- if err := f(); err != nil {
- return err
- }
- if err := f(); err != nil {
- return err
- }
- // Phew, it worked again.
- return nil
-}
-
-func multi() error {
- a := 0
- var err error
- // unreachable code after return statements is intentional to check that it
- // doesn't confuse the linter.
- if true {
- a++
- if err := f(); err != nil { // MATCH /redundant/
- return err
- }
- return nil
- a++
- } else {
- a++
- if err = f(); err != nil { // MATCH /redundant/
- return err
- }
- return nil
- a++
- }
-}
