Merge pull request #411 from chavacava/identical-branches

Identical branches

README.md

@@ -350,6 +350,8 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`cognitive-complexity`](./RULES_DESCRIPTIONS.md#cognitive-complexity)          |  int   | Sets restriction for maximum Cognitive complexity.              |    no    |  no   |
 | [`string-of-int`](./RULES_DESCRIPTIONS.md#string-of-int)          |  n/a   | Warns on suspicious casts from int to string            |    no    |  yes   |
 | [`early-return`](./RULES_DESCRIPTIONS.md#early-return)          |  n/a   | Spots if-then-else statements that can be refactored to simplify code reading            |    no    |  no   |
+| [`identical-branches`](./RULES_DESCRIPTIONS.md#identical-branches)          |  n/a   | Spots if-then-else statements with identical `then` and `else` branches       |    no    |  no   |
+
 ## Configurable rules
 
 Here you can find how you can configure some of the existing rules:

RULES_DESCRIPTIONS.md

@@ -33,6 +33,7 @@ List of all available rules.
   - [flag-parameter](#flag-parameter)
   - [function-result-limit](#function-result-limit)
   - [get-return](#get-return)
+  - [identical-branches](#identical-branches)
   - [if-return](#if-return)
   - [increment-decrement](#increment-decrement)
   - [indent-error-flow](#indent-error-flow)
@@ -313,6 +314,12 @@ _Description_: Typically, functions with names prefixed with _Get_ are supposed
 
 _Configuration_: N/A
 
+## identical-branches
+
+_Description_: an `if-then-else` conditional with identical implementations in both branches is an error.
+
+_Configuration_: N/A
+
 ## if-return
 
 _Description_: Checking if an error is _nil_ to just after return the error or nil is redundant.

config.go

@@ -84,6 +84,7 @@ var allRules = append([]lint.Rule{
 	&rule.CognitiveComplexityRule{},
 	&rule.StringOfIntRule{},
 	&rule.EarlyReturnRule{},
+	&rule.IdenticalBranchesRule{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{

rule/identical-branches.go

@@ -0,0 +1,82 @@
+package rule
+
+import (
+	"go/ast"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// IdenticalBranchesRule warns on constant logical expressions.
+type IdenticalBranchesRule struct{}
+
+// Apply applies the rule to given file.
+func (r *IdenticalBranchesRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+
+	astFile := file.AST
+	w := &lintIdenticalBranches{astFile, onFailure}
+	ast.Walk(w, astFile)
+	return failures
+}
+
+// Name returns the rule name.
+func (r *IdenticalBranchesRule) Name() string {
+	return "identical-branches"
+}
+
+type lintIdenticalBranches struct {
+	file      *ast.File
+	onFailure func(lint.Failure)
+}
+
+func (w *lintIdenticalBranches) Visit(node ast.Node) ast.Visitor {
+	n, ok := node.(*ast.IfStmt)
+	if !ok {
+		return w
+	}
+
+	if n.Else == nil {
+		return w
+	}
+	branches := []*ast.BlockStmt{n.Body}
+
+	elseBranch, ok := n.Else.(*ast.BlockStmt)
+	if !ok { // if-else-if construction
+		return w
+	}
+	branches = append(branches, elseBranch)
+
+	if w.identicalBranches(branches) {
+		w.newFailure(n, "both branches of the if are identical")
+	}
+
+	return w
+}
+
+func (w *lintIdenticalBranches) identicalBranches(branches []*ast.BlockStmt) bool {
+	if len(branches) < 2 {
+		return false
+	}
+
+	ref := gofmt(branches[0])
+	for i := 1; i < len(branches); i++ {
+		if gofmt(branches[i]) != ref {
+			return false
+		}
+	}
+
+	return true
+}
+
+func (w lintIdenticalBranches) newFailure(node ast.Node, msg string) {
+	w.onFailure(lint.Failure{
+		Confidence: 1,
+		Node:       node,
+		Category:   "logic",
+		Failure:    msg,
+	})
+}

rule/utils.go

@@ -182,8 +182,8 @@ func isExprABooleanLit(n ast.Node) (lexeme string, ok bool) {
 	return oper.Name, (oper.Name == trueName || oper.Name == falseName)
 }
 
-// gofmt returns a string representation of the expression.
-func gofmt(x ast.Expr) string {
+// gofmt returns a string representation of an AST subtree.
+func gofmt(x interface{}) string {
 	buf := bytes.Buffer{}
 	fs := token.NewFileSet()
 	printer.Fprint(&buf, fs, x)

test/identical-branches_test.go

@@ -0,0 +1,12 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/rule"
+)
+
+// IdenticalBranches rule.
+func TestIdenticalBranches(t *testing.T) {
+	testRule(t, "identical-branches", &rule.IdenticalBranchesRule{})
+}

testdata/identical-branches.go

@@ -0,0 +1,40 @@
+package fixtures
+
+func identicalBranches() {
+	if true { // MATCH /both branches of the if are identical/
+
+	} else {
+
+	}
+
+	if true {
+
+	}
+
+	if true {
+		print()
+	} else {
+	}
+
+	if true { // MATCH /both branches of the if are identical/
+		print()
+	} else {
+		print()
+	}
+
+	if true {
+		if true { // MATCH /both branches of the if are identical/
+			print()
+		} else {
+			print()
+		}
+	} else {
+		println()
+	}
+
+	if true {
+		println("something")
+	} else {
+		println("else")
+	}
+}