Repository: mgechev/revive
Pull request #409 (2020-05-08 19:13:56)

Update golang.org/x/tools commit hash to 2212a7e
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [golang.org/x/tools](https://togithub.com/golang/tools) | require | digest | `480da3e` -> `2212a7e` |

---

### Renovate configuration

:date: **Schedule**: At any time (no schedule defined).

:vertical_traffic_light: **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

:recycle: **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

:no_bell: **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#mgechev/revive).


Patch:
diff --git a/README.md b/README.md
index 3e51462..e0c471f 100644
--- a/README.md
+++ b/README.md
@@ -316,7 +316,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`cyclomatic`](./RULES_DESCRIPTIONS.md#cyclomatic)          |  int   | Sets restriction for maximum Cyclomatic complexity.              |    no    |  no   |
 | [`max-public-structs`](./RULES_DESCRIPTIONS.md#max-public-structs)  |  int   | The maximum number of public structs in a file.                  |    no    |  no   |
 | [`file-header`](./RULES_DESCRIPTIONS.md#file-header)         | string | Header which each file should have.                              |    no    |  no   |
-| [`empty-block`](./RULES_DESCRIPTIONS.md#empty-block)         |  n/a   | Warns on empty code blocks                                       |    no    |  no   |
+| [`empty-block`](./RULES_DESCRIPTIONS.md#empty-block)         |  n/a   | Warns on empty code blocks                                       |    no    |  yes   |
 | [`superfluous-else`](./RULES_DESCRIPTIONS.md#superfluous-else)    |  n/a   | Prevents redundant else statements (extends [`indent-error-flow`](./RULES_DESCRIPTIONS.md#indent-error-flow)) |    no    |  no   |
 | [`confusing-naming`](./RULES_DESCRIPTIONS.md#confusing-naming)    |  n/a   | Warns on methods with names that differ only by capitalization   |    no    |  no   |
 | [`get-return`](./RULES_DESCRIPTIONS.md#get-return)          |  n/a   | Warns on getters that do not yield any result                    |    no    |  no   |
@@ -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:
diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md
index 901543c..4cedcd9 100644
--- a/RULES_DESCRIPTIONS.md
+++ b/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.
diff --git a/config.go b/config.go
index 95543b8..6e9806c 100644
--- a/config.go
+++ b/config.go
@@ -84,6 +84,7 @@ var allRules = append([]lint.Rule{
 	&rule.CognitiveComplexityRule{},
 	&rule.StringOfIntRule{},
 	&rule.EarlyReturnRule{},
+	&rule.IdenticalBranchesRule{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{
diff --git a/go.mod b/go.mod
index 2ea4dd0..d8d31f2 100644
--- a/go.mod
+++ b/go.mod
@@ -10,5 +10,5 @@ require (
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/olekukonko/tablewriter v0.0.4
 	github.com/pkg/errors v0.9.1
-	golang.org/x/tools v0.0.0-20200507205054-480da3ebd79c
+	golang.org/x/tools v0.0.0-20200509030707-2212a7e161a5
 )
diff --git a/go.sum b/go.sum
index 36a3f27..a683a92 100644
--- a/go.sum
+++ b/go.sum
@@ -52,6 +52,8 @@ golang.org/x/tools v0.0.0-20200505023115-26f46d2f7ef8 h1:BMFHd4OFnFtWX46Xj4DN6vv
 golang.org/x/tools v0.0.0-20200505023115-26f46d2f7ef8/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/tools v0.0.0-20200507205054-480da3ebd79c h1:TDspWmUQsjdWzrHnd5imfaJSfhR4AO/R7kG++T2cONw=
 golang.org/x/tools v0.0.0-20200507205054-480da3ebd79c/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
+golang.org/x/tools v0.0.0-20200509030707-2212a7e161a5 h1:MeC2gMlMdkd67dn17MEby3rGXRxZtWeiRXOnISfTQ74=
+golang.org/x/tools v0.0.0-20200509030707-2212a7e161a5/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
diff --git a/rule/empty-block.go b/rule/empty-block.go
index 7861394..4eb0ce8 100644
--- a/rule/empty-block.go
+++ b/rule/empty-block.go
@@ -2,6 +2,7 @@ package rule
 
 import (
 	"go/ast"
+	"go/types"
 
 	"github.com/mgechev/revive/lint"
 )
@@ -17,7 +18,12 @@ func (r *EmptyBlockRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure
 		failures = append(failures, failure)
 	}
 
-	w := lintEmptyBlock{make([]*ast.BlockStmt, 0), onFailure}
+	err := file.Pkg.TypeCheck()
+	if err != nil {
+		panic("unable to type check " + file.Name + ":" + err.Error())
+	}
+
+	w := lintEmptyBlock{make(map[*ast.BlockStmt]bool, 0), file.Pkg, onFailure}
 	ast.Walk(w, file.AST)
 	return failures
 }
@@ -28,49 +34,35 @@ func (r *EmptyBlockRule) Name() string {
 }
 
 type lintEmptyBlock struct {
-	ignore    []*ast.BlockStmt
+	ignore    map[*ast.BlockStmt]bool
+	pkg       *lint.Package
 	onFailure func(lint.Failure)
 }
 
 func (w lintEmptyBlock) Visit(node ast.Node) ast.Visitor {
-	fd, ok := node.(*ast.FuncDecl)
-	if ok {
-		w.ignore = append(w.ignore, fd.Body)
+	switch n := node.(type) {
+	case *ast.FuncDecl:
+		w.ignore[n.Body] = true
 		return w
-	}
-
-	fl, ok := node.(*ast.FuncLit)
-	if ok {
-		w.ignore = append(w.ignore, fl.Body)
+	case *ast.FuncLit:
+		w.ignore[n.Body] = true
 		return w
-	}
+	case *ast.RangeStmt:
+		t := w.pkg.TypeOf(n.X)
 
-	block, ok := node.(*ast.BlockStmt)
-	if !ok {
-		return w
-	}
-
-	if mustIgnore(block, w.ignore) {
-		return w
-	}
-
-	if len(block.List) == 0 {
-		w.onFailure(lint.Failure{
-			Confidence: 1,
-			Node:       block,
-			Category:   "logic",
-			Failure:    "this block is empty, you can remove it",
-		})
+		if _, ok := t.(*types.Chan); ok {
+			w.ignore[n.Body] = true
+		}
+	case *ast.BlockStmt:
+		if !w.ignore[n] && len(n.List) == 0 {
+			w.onFailure(lint.Failure{
+				Confidence: 1,
+				Node:       n,
+				Category:   "logic",
+				Failure:    "this block is empty, you can remove it",
+			})
+		}
 	}
 
 	return w
 }
-
-func mustIgnore(block *ast.BlockStmt, blackList []*ast.BlockStmt) bool {
-	for _, b := range blackList {
-		if b == block {
-			return true
-		}
-	}
-	return false
-}
diff --git a/rule/identical-branches.go b/rule/identical-branches.go
new file mode 100644
index 0000000..094a791
--- /dev/null
+++ b/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,
+	})
+}
diff --git a/rule/utils.go b/rule/utils.go
index 6ba542b..38677c8 100644
--- a/rule/utils.go
+++ b/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 patch:
diff --git a/test/identical-branches_test.go b/test/identical-branches_test.go
new file mode 100644
index 0000000..0d1a6c5
--- /dev/null
+++ b/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{})
+}
diff --git a/testdata/empty-block.go b/testdata/empty-block.go
index 9f04d5b..81b7262 100644
--- a/testdata/empty-block.go
+++ b/testdata/empty-block.go
@@ -2,18 +2,18 @@
 
 package fixtures
 
-func f(x int) bool {} // Must not match
+func f(x int) {} // Must not match
 
-type foo struct {}
+type foo struct{}
 
-func (f foo) f(x *int) string {} // Must not match
-func (f *foo) g(y *int) string {} // Must not match
+func (f foo) f(x *int)  {} // Must not match
+func (f *foo) g(y *int) {} // Must not match
 
-func g(f func() bool) string {
+func g(f func() bool) {
 	{ // MATCH /this block is empty, you can remove it/
 	}
 
-	a := func(e error){} // Must not match
+	_ = func(e error) {} // Must not match
 
 	if ok := f(); ok { // MATCH /this block is empty, you can remove it/
 		// only a comment
@@ -35,4 +35,13 @@ func g(f func() bool) string {
 
 	}
 
+	// issue 386
+	var c = make(chan int)
+	for range c {
+	}
+
+	var s = "a string"
+	for range s { // MATCH /this block is empty, you can remove it/
+	}
+
 }
diff --git a/testdata/identical-branches.go b/testdata/identical-branches.go
new file mode 100644
index 0000000..8b850da
--- /dev/null
+++ b/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")
+	}
+}