From fc4e8ec9dee07f955278814b6b84389fd46816dd Mon Sep 17 00:00:00 2001 From: mgechev Date: Sun, 26 Nov 2017 20:07:53 -0800 Subject: [PATCH] Add rule --- defaultrule/else-error.go | 116 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 defaultrule/else-error.go diff --git a/defaultrule/else-error.go b/defaultrule/else-error.go new file mode 100644 index 0000000..0a44300 --- /dev/null +++ b/defaultrule/else-error.go @@ -0,0 +1,116 @@ +package defaultrule + +import ( + "go/ast" + "go/token" + "strings" + + "github.com/mgechev/revive/file" + "github.com/mgechev/revive/rule" +) + +// LintElseRule lints given else constructs. +type LintElseRule struct{} + +// Apply applies the rule to given file. +func (r *LintElseRule) Apply(file *file.File, arguments rule.Arguments) []rule.Failure { + var failures []rule.Failure + + onFailure := func(failure rule.Failure) { + failures = append(failures, failure) + } + + astFile := file.GetAST() + w := &lintElseError{astFile, onFailure} + ast.Walk(w, astFile) + return failures +} + +// Name returns the rule name. +func (r *LintElseRule) Name() string { + return "no-else-return" +} + +type lintElseError struct { + file *ast.File + onFailure func(rule.Failure) +} + +func (w *lintElseError) Visit(node ast.Node) ast.Visitor { + 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 containsComments(w.file, s.Pos(), r.Pos()) { + continue + } + + w.onFailure(rule.Failure{ + Confidence: 0.9, + Failure: "redundant if ...; err != nil check, just return error instead.", + Node: v.List[i], + }) + } + } + return w +} + +func containsComments(f *ast.File, start, end token.Pos) bool { + for _, cgroup := range f.Comments { + comments := cgroup.List + if comments[0].Slash >= end { + // All comments starting with this group are after end pos. + return false + } + if comments[len(comments)-1].Slash < start { + // Comments group ends before start pos. + continue + } + for _, c := range comments { + if start <= c.Slash && c.Slash < end && !strings.HasPrefix(c.Text, "// MATCH ") { + return true + } + } + } + return false +}