bool-literal-in-expr (new rule) (#54)

* bool-literal-in-expr (new rule)

* bool-literal-in-expr: add test case and fix typo

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  |
+| `bool-literal-in-expr`|  n/a   | Suggests removing Boolean literals from logic expressions        |    no    |  no   |
 
 ## Available Formatters
 

config.go

@@ -61,6 +61,7 @@ var allRules = append([]lint.Rule{
 	&rule.UnnecessaryStmtRule{},
 	&rule.StructTagRule{},
 	&rule.ModifiesValRecRule{},
+	&rule.BoolLiteralRule{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{

fixtures/bool-literal-in-expr.go

@@ -0,0 +1,54 @@
+package fixtures
+
+func foo(a, b, c, d int) bool {
+	if bar == true { // MATCH /omit Boolean literal in expression/
+
+	}
+	for f() || false != yes { // MATCH /omit Boolean literal in expression/
+
+	}
+
+	return b > c == false // MATCH /omit Boolean literal in expression/
+}
+
+// from github.com/jmespath/go-jmespath/functions.go
+func jpfToNumber(arguments []interface{}) (interface{}, error) {
+	arg := arguments[0]
+	// code skipped
+	if arg == true || // MATCH /omit Boolean literal in expression/
+		arg == false { // MATCH /omit Boolean literal in expression/
+		return nil, nil
+	}
+	return nil, errors.New("unknown type")
+}
+
+// from gopkg.in/yaml.v2/resolve.go
+func resolve(tag string, in string) (rtag string, out interface{}) {
+	if err == nil {
+		if true || intv == int64(int(intv)) { // MATCH /Boolean expression seems to always evaluate to true/
+			return yaml_INT_TAG, int(intv)
+		} else {
+			return yaml_INT_TAG, intv
+		}
+	}
+}
+
+// from github.com/miekg/dns/msg_helpers.go
+func packDataDomainNames(names []string, msg []byte, off int, compression map[string]int, compress bool) (int, error) {
+	var err error
+	for j := 0; j < len(names); j++ {
+		off, err = PackDomainName(names[j], msg, off, compression, false && compress) // MATCH /Boolean expression seems to always evaluate to false/
+		if err != nil {
+			return len(msg), err
+		}
+	}
+	return off, nil
+}
+
+func isTrue(arg bool) bool {
+	return arg
+}
+
+func main() {
+	isTrue(true)
+}

rule/bool-literal-in-expr.go

@@ -0,0 +1,72 @@
+package rule
+
+import (
+	"github.com/mgechev/revive/lint"
+	"go/ast"
+	"go/token"
+)
+
+// BoolLiteralRule warns when logic expressions contains Boolean literals.
+type BoolLiteralRule struct{}
+
+// Apply applies the rule to given file.
+func (r *BoolLiteralRule) 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 := &lintBoolLiteral{astFile, onFailure}
+	ast.Walk(w, astFile)
+
+	return failures
+}
+
+// Name returns the rule name.
+func (r *BoolLiteralRule) Name() string {
+	return "bool-literal-in-expr"
+}
+
+type lintBoolLiteral struct {
+	file      *ast.File
+	onFailure func(lint.Failure)
+}
+
+func (w *lintBoolLiteral) Visit(node ast.Node) ast.Visitor {
+	switch n := node.(type) {
+	case *ast.BinaryExpr:
+		if !isBoolOp(n.Op) {
+			return w
+		}
+
+		lexeme, ok := isExprABooleanLit(n.X)
+		if !ok {
+			lexeme, ok = isExprABooleanLit(n.Y)
+
+			if !ok {
+				return w
+			}
+		}
+
+		isConstant := (n.Op == token.LAND && lexeme == "false") || (n.Op == token.LOR && lexeme == "true")
+
+		if isConstant {
+			w.addFailure(n, "Boolean expression seems to always evaluate to "+lexeme, "logic")
+		} else {
+			w.addFailure(n, "omit Boolean literal in expression", "style")
+		}
+	}
+
+	return w
+}
+
+func (w lintBoolLiteral) addFailure(node ast.Node, msg string, cat string) {
+	w.onFailure(lint.Failure{
+		Confidence: 1,
+		Node:       node,
+		Category:   cat,
+		Failure:    msg,
+	})
+}

rule/utils.go

@@ -149,7 +149,7 @@ func srcLine(src []byte, p token.Position) string {
 
 // pick yields a list of nodes by picking them from a sub-ast with root node n.
 // Nodes are selected by applying the fselect function
-// f function is applied to each selected node before inseting it in the final result. 
+// f function is applied to each selected node before inseting it in the final result.
 // If f==nil then it defaults to the identity function (ie it returns the node itself)
 func pick(n ast.Node, fselect func(n ast.Node) bool, f func(n ast.Node) []ast.Node) []ast.Node {
 	var result []ast.Node
@@ -194,3 +194,29 @@ func (p picker) Visit(node ast.Node) ast.Visitor {
 
 	return p
 }
+
+// isBoolOp returns true if the given token corresponds to
+// a bool operator
+func isBoolOp(t token.Token) bool {
+	switch t {
+	case token.LAND, token.LOR, token.EQL, token.NEQ:
+		return true
+	}
+
+	return false
+}
+
+const (
+	trueName  = "true"
+	falseName = "false"
+)
+
+func isExprABooleanLit(n ast.Node) (lexeme string, ok bool) {
+	oper, ok := n.(*ast.Ident)
+
+	if !ok {
+		return "", false
+	}
+
+	return oper.Name, (oper.Name == trueName || oper.Name == falseName)
+}

test/bool-literal-in-expr_test.go

@@ -0,0 +1,12 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/rule"
+)
+
+// BoolLiteral rule.
+func TestBoolLiteral(t *testing.T) {
+	testRule(t, "bool-literal-in-expr", &rule.BoolLiteralRule{})
+}