feature: new rule identical-switch-conditions (#1451)

README.md

@@ -551,6 +551,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`function-result-limit`](./RULES_DESCRIPTIONS.md#function-result-limit) |  int (defaults to 3)| Specifies the maximum number of results a function can return    |    no    |  no   |
 | [`get-return`](./RULES_DESCRIPTIONS.md#get-return)          |  n/a   | Warns on getters that do not yield any result                    |    no    |  no   |
 | [`identical-branches`](./RULES_DESCRIPTIONS.md#identical-branches)          |  n/a   | Spots if-then-else statements with identical `then` and `else` branches       |    no    |  no   |
+| [`identical-switch-conditions`](./RULES_DESCRIPTIONS.md#identical-switch-conditions)          |  n/a   | Spots identical conditions in case clauses of `switch` statements.       |    no    |  no   |
 | [`if-return`](./RULES_DESCRIPTIONS.md#if-return)           |  n/a   | Redundant if when returning an error.                            |   no    |  no   |
 | [`import-alias-naming`](./RULES_DESCRIPTIONS.md#import-alias-naming)         | string or map[string]string (defaults to allow regex pattern `^[a-z][a-z0-9]{0,}$`) | Conventions around the naming of import aliases.                              |    no    |  no   |
 | [`import-shadowing`](./RULES_DESCRIPTIONS.md#import-shadowing)   | n/a    | Spots identifiers that shadow an import    |    no    |  no   |

RULES_DESCRIPTIONS.md

@@ -45,6 +45,7 @@ List of all available rules.
   - [function-result-limit](#function-result-limit)
   - [get-return](#get-return)
   - [identical-branches](#identical-branches)
+  - [identical-switch-conditions](#identical-switch-conditions)
   - [if-return](#if-return)
   - [import-alias-naming](#import-alias-naming)
   - [import-shadowing](#import-shadowing)
@@ -733,6 +734,13 @@ _Description_: An `if-then-else` conditional with identical implementations in b
 
 _Configuration_: N/A
 
+## identical-switch-conditions
+
+_Description_: a `switch` statement with cases with the same condition can lead to
+unreachable code and is a potential source of bugs while making the code harder to read and maintain.
+
+_Configuration_: N/A
+
 ## if-return
 
 _Description_: Checking if an error is _nil_ to just after return the error or nil is redundant.

config/config.go

@@ -105,6 +105,7 @@ var allRules = append([]lint.Rule{
 	&rule.UnnecessaryFormatRule{},
 	&rule.UseFmtPrintRule{},
 	&rule.EnforceSwitchStyleRule{},
+	&rule.IdenticalSwitchConditionsRule{},
 }, defaultRules...)
 
 // allFormatters is a list of all available formatters to output the linting results.

rule/identical_switch_conditions.go

@@ -0,0 +1,78 @@
+package rule
+
+import (
+	"fmt"
+	"go/ast"
+	"go/token"
+
+	"github.com/mgechev/revive/internal/astutils"
+	"github.com/mgechev/revive/lint"
+)
+
+// IdenticalSwitchConditionsRule warns on switch case clauses with identical conditions.
+type IdenticalSwitchConditionsRule struct{}
+
+// Apply applies the rule to given file.
+func (*IdenticalSwitchConditionsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+
+	w := &lintIdenticalSwitchConditions{toPosition: file.ToPosition, onFailure: onFailure}
+	for _, decl := range file.AST.Decls {
+		fn, ok := decl.(*ast.FuncDecl)
+		if !ok || fn.Body == nil {
+			continue
+		}
+
+		ast.Walk(w, fn.Body)
+	}
+
+	return failures
+}
+
+// Name returns the rule name.
+func (*IdenticalSwitchConditionsRule) Name() string {
+	return "identical-switch-conditions"
+}
+
+type lintIdenticalSwitchConditions struct {
+	toPosition func(token.Pos) token.Position
+	onFailure  func(lint.Failure)
+}
+
+func (w *lintIdenticalSwitchConditions) Visit(node ast.Node) ast.Visitor {
+	switchStmt, ok := node.(*ast.SwitchStmt)
+	if !ok { // not a switch statement, keep walking the AST
+		return w
+	}
+
+	if switchStmt.Tag != nil {
+		return w // Not interested in tagged switches
+	}
+
+	hashes := map[string]int{} // map hash(condition code) -> condition line
+	for _, cc := range switchStmt.Body.List {
+		caseClause := cc.(*ast.CaseClause)
+		caseClauseLine := w.toPosition(caseClause.Pos()).Line
+		for _, expr := range caseClause.List {
+			hash := astutils.NodeHash(expr)
+			if matchLine, ok := hashes[hash]; ok {
+				w.onFailure(lint.Failure{
+					Confidence: 1.0,
+					Node:       caseClause,
+					Category:   lint.FailureCategoryLogic,
+					Failure:    fmt.Sprintf(`case clause at line %d has the same condition`, matchLine),
+				})
+			}
+
+			hashes[hash] = caseClauseLine
+		}
+
+		ast.Walk(w, caseClause)
+	}
+
+	return nil // switch branches already analyzed
+}

test/identical_switch_conditions_test.go

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

testdata/identical_switch_conditions.go

@@ -0,0 +1,55 @@
+package fixtures
+
+func enforceSwitchStyle3() {
+
+	switch expression { // skipt tagged switch
+	case value:
+	default:
+	}
+
+	switch {
+	case a > 0, a < 0:
+	case a == 0:
+	case a < 0: // MATCH /case clause at line 11 has the same condition/
+	default:
+	}
+
+	switch {
+	case a > 0, a < 0, a > 0: // MATCH /case clause at line 18 has the same condition/
+	case a == 0:
+	case a < 0: // MATCH /case clause at line 18 has the same condition/
+	default:
+	}
+
+	switch something {
+	case 1:
+		switch {
+		case a > 0, a < 0, a > 0: // MATCH /case clause at line 27 has the same condition/
+		case a == 0:
+		}
+	default:
+	}
+
+	switch {
+	case a == 0:
+		switch {
+		case a > 0, a < 0, a > 0: // MATCH /case clause at line 36 has the same condition/
+		case a == 0:
+		}
+	default:
+	}
+
+	switch {
+	case lnOpts.IsSocketOpts():
+		// ...
+		// check for timeout
+		fallthrough
+	case lnOpts.IsTimeout(), lnOpts.IsSocketOpts(): // MATCH /case clause at line 43 has the same condition/
+		// timeout listener with socket options.
+		// ...
+	case lnOpts.IsTimeout(): // MATCH /case clause at line 47 has the same condition/
+		// ...
+	default:
+		// ...
+	}
+}