diff --git a/README.md b/README.md index e1fbe3b..d0fdc88 100644 --- a/README.md +++ b/README.md @@ -265,6 +265,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | `unnecessary-stmt` | n/a | Suggests removing or simplifying unnecessary statements | no | no | | `struct-tag` | n/a | Checks common struct tags like `json`,`xml`,`yaml` | no | no | | `modifies-value-receiver` | n/a | Warns on assignments to value-passed method receivers | no | yes | +| `constant-logical-expr` | n/a | Warns on constant logical expressions | no | no | | `bool-literal-in-expr`| n/a | Suggests removing Boolean literals from logic expressions | no | no | ## Available Formatters diff --git a/config.go b/config.go index 0b0dd31..4a9d084 100644 --- a/config.go +++ b/config.go @@ -61,6 +61,7 @@ var allRules = append([]lint.Rule{ &rule.UnnecessaryStmtRule{}, &rule.StructTagRule{}, &rule.ModifiesValRecRule{}, + &rule.ConstantLogicalExprRule{}, &rule.BoolLiteralRule{}, }, defaultRules...) diff --git a/fixtures/constant-logical-expr.go b/fixtures/constant-logical-expr.go new file mode 100644 index 0000000..5adb404 --- /dev/null +++ b/fixtures/constant-logical-expr.go @@ -0,0 +1,26 @@ +package fixtures + +// from github.com/ugorji/go/codec/helper.go +func isNaN(f float64) bool { return f != f } // MATCH /expression always evaluates to false/ + +func skip(f float64) bool { return f != g } + +func foo1(f float64) bool { return foo2(2.) > foo2(2.) } // MATCH /expression always evaluates to false/ + +func foo2(f float64) bool { return f < f } // MATCH /expression always evaluates to false/ + +func foo3(f float64) bool { return f <= f } // MATCH /expression always evaluates to false/ + +func foo4(f float64) bool { return f >= f } // MATCH /expression always evaluates to false/ + +func foo5(f float64) bool { return f == f } // MATCH /expression always evaluates to true/ + +func foo6(f float64) bool { return fmt.Sprintf("%s", buf1.Bytes()) == fmt.Sprintf("%s", buf1.Bytes()) } // MATCH /expression always evaluates to true/ + +func foo7(f float64) bool { + return fFoo(fBar(isNaN(10.), bpar), 10000) || fFoo(fBar(isNaN(10.), bpar), 10000) // MATCH /left and right hand-side sub-expressions are the same/ +} + +func foo8(f float64) bool { + return fFoo(fBar(isNaN(10.), bpar), 10000) && fFoo(fBar(isNaN(10.), bpar), 10000) // MATCH /left and right hand-side sub-expressions are the same/ +} diff --git a/rule/constant-logical-expr.go b/rule/constant-logical-expr.go new file mode 100644 index 0000000..34ad9eb --- /dev/null +++ b/rule/constant-logical-expr.go @@ -0,0 +1,106 @@ +package rule + +import ( + "bytes" + "fmt" + "github.com/mgechev/revive/lint" + "go/ast" + "go/format" + "go/token" +) + +// ConstantLogicalExprRule warns on constant logical expressions. +type ConstantLogicalExprRule struct{} + +// Apply applies the rule to given file. +func (r *ConstantLogicalExprRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + var failures []lint.Failure + + onFailure := func(failure lint.Failure) { + failures = append(failures, failure) + } + + astFile := file.AST + w := &lintConstantLogicalExpr{astFile, onFailure} + ast.Walk(w, astFile) + return failures +} + +// Name returns the rule name. +func (r *ConstantLogicalExprRule) Name() string { + return "constant-logical-expr" +} + +type lintConstantLogicalExpr struct { + file *ast.File + onFailure func(lint.Failure) +} + +func (w *lintConstantLogicalExpr) Visit(node ast.Node) ast.Visitor { + switch n := node.(type) { + case *ast.BinaryExpr: + if !w.isOperatorWithLogicalResult(n.Op) { + return w + } + + if !w.areEqual(n.X, n.Y) { + return w + } + + if n.Op == token.EQL { + w.newFailure(n, "expression always evaluates to true") + return w + } + + if w.isInequalityOperator(n.Op) { + w.newFailure(n, "expression always evaluates to false") + return w + } + + w.newFailure(n, "left and right hand-side sub-expressions are the same") + } + + return w +} + +func (w *lintConstantLogicalExpr) isOperatorWithLogicalResult(t token.Token) bool { + switch t { + case token.LAND, token.LOR, token.EQL, token.LSS, token.GTR, token.NEQ, token.LEQ, token.GEQ: + return true + } + + return false +} + +func (w *lintConstantLogicalExpr) isInequalityOperator(t token.Token) bool { + switch t { + case token.LSS, token.GTR, token.NEQ, token.LEQ, token.GEQ: + return true + } + + return false +} + +func (w lintConstantLogicalExpr) areEqual(x, y ast.Expr) bool { + fset := token.NewFileSet() + var buf1 bytes.Buffer + if err := format.Node(&buf1, fset, x); err != nil { + return false // keep going in case of error + } + + var buf2 bytes.Buffer + if err := format.Node(&buf2, fset, y); err != nil { + return false // keep going in case of error + } + + return fmt.Sprintf("%s", buf1.Bytes()) == fmt.Sprintf("%s", buf2.Bytes()) +} + +func (w lintConstantLogicalExpr) newFailure(node ast.Node, msg string) { + w.onFailure(lint.Failure{ + Confidence: 1, + Node: node, + Category: "logic", + Failure: msg, + }) +} diff --git a/test/constant-logical-expr_test.go b/test/constant-logical-expr_test.go new file mode 100644 index 0000000..bb80ade --- /dev/null +++ b/test/constant-logical-expr_test.go @@ -0,0 +1,12 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/rule" +) + +// ConstantLogicalExpr rule. +func TestConstantLogicalExpr(t *testing.T) { + testRule(t, "constant-logical-expr", &rule.ConstantLogicalExprRule{}) +}