flag-parameter (new rule) (#44)

* flag-parameter (new rule)

* fix failure confidence

README.md

@@ -222,6 +222,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | `unused-parameter`    |  n/a   | Suggests to rename or remove unused function parameters          |    no    |  no   |
 | `unreachable-code`    |  n/a   | Warns on unreachable code                                        |    no    |  no   |
 | `add-constant`        |  map   | Suggests using constant for magic numbers and string literals |    no    |  no   |
+| `flag-parameter`      |  n/a   | Warns on boolean parameters that create a control coupling       |    no    |  no   |
 
 ## Available Formatters
 

config.go

@@ -57,6 +57,7 @@ var allRules = append([]lint.Rule{
 	&rule.UnusedParamRule{},
 	&rule.UnreachableCodeRule{},
 	&rule.AddConstantRule{},
+	&rule.FlagParamRule{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{

fixtures/flag-param.go

@@ -0,0 +1,11 @@
+package fixtures
+
+func foo(a bool, b int) { // MATCH /parameter 'a' seems to be a control flag, avoid control coupling/
+	if a {
+
+	}
+}
+
+func foo(a bool, b int) {
+	str := mystruct{a, b}
+}

rule/flag-param.go

@@ -0,0 +1,104 @@
+package rule
+
+import (
+	"fmt"
+	"github.com/mgechev/revive/lint"
+	"go/ast"
+)
+
+// FlagParamRule lints given else constructs.
+type FlagParamRule struct{}
+
+// Apply applies the rule to given file.
+func (r *FlagParamRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+
+	w := lintFlagParamRule{onFailure: onFailure}
+	ast.Walk(w, file.AST)
+	return failures
+}
+
+// Name returns the rule name.
+func (r *FlagParamRule) Name() string {
+	return "flag-parameter"
+}
+
+type lintFlagParamRule struct {
+	onFailure func(lint.Failure)
+}
+
+func (w lintFlagParamRule) Visit(node ast.Node) ast.Visitor {
+	fd, ok := node.(*ast.FuncDecl)
+	if !ok {
+		return w
+	}
+
+	if fd.Body == nil {
+		return nil // skip whole function declaration
+	}
+
+	for _, p := range fd.Type.Params.List {
+		t := p.Type
+
+		id, ok := t.(*ast.Ident)
+		if !ok {
+			continue
+		}
+
+		if id.Name != "bool" {
+			continue
+		}
+
+		cv := conditionVisitor{p.Names, fd, w}
+		ast.Walk(cv, fd.Body)
+	}
+
+	return w
+}
+
+type conditionVisitor struct {
+	ids    []*ast.Ident
+	fd     *ast.FuncDecl
+	linter lintFlagParamRule
+}
+
+func (w conditionVisitor) Visit(node ast.Node) ast.Visitor {
+	ifStmt, ok := node.(*ast.IfStmt)
+	if !ok {
+		return w
+	}
+
+	fselect := func(n ast.Node) bool {
+		ident, ok := n.(*ast.Ident)
+		if !ok {
+			return false
+		}
+
+		for _, id := range w.ids {
+			if ident.Name == id.Name {
+				return true
+			}
+		}
+
+		return false
+	}
+
+	uses := pick(ifStmt.Cond, fselect, nil)
+
+	if len(uses) < 1 {
+		return w
+	}
+
+	w.linter.onFailure(lint.Failure{
+		Confidence: 1,
+		Node:       w.fd.Type.Params,
+		Category:   "bad practice",
+		Failure:    fmt.Sprintf("parameter '%s' seems to be a control flag, avoid control coupling", uses[0]),
+	})
+
+	return nil
+}

test/flag-param_test.go

@@ -0,0 +1,11 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/rule"
+)
+
+func TestFlagParam(t *testing.T) {
+	testRule(t, "flag-param", &rule.FlagParamRule{})
+}