feature: new rule enforce-else (#1436)

README.md

@@ -534,6 +534,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`early-return`](./RULES_DESCRIPTIONS.md#early-return)          | []string   | Spots if-then-else statements where the predicate may be inverted to reduce nesting |    no    |  no   |
 | [`empty-block`](./RULES_DESCRIPTIONS.md#empty-block)         |  n/a   | Warns on empty code blocks                                       |    no    |  yes   |
 | [`empty-lines`](./RULES_DESCRIPTIONS.md#empty-lines)   | n/a | Warns when there are heading or trailing newlines in a block              |    no    |  no   |
+| [`enforce-else`](./RULES_DESCRIPTIONS.md#enforce-else) |  n/a  |  Enforces `else` branch at the end of `if...else if` chains. |    no    |  no   |
 | [`enforce-map-style`](./RULES_DESCRIPTIONS.md#enforce-map-style) |  string (defaults to "any")  |  Enforces consistent usage of `make(map[type]type)` or `map[type]type{}` for map initialization. Does not affect `make(map[type]type, size)` constructions. |    no    |  no   |
 | [`enforce-repeated-arg-type-style`](./RULES_DESCRIPTIONS.md#enforce-repeated-arg-type-style) |  string (defaults to "any")  |  Enforces consistent style for repeated argument and/or return value types. |    no    |  no   |
 | [`enforce-slice-style`](./RULES_DESCRIPTIONS.md#enforce-slice-style) |  string (defaults to "any")  |  Enforces consistent usage of `make([]type, 0)` or `[]type{}` for slice initialization. Does not affect `make(map[type]type, non_zero_len, or_non_zero_cap)` constructions. |    no    |  no   |

RULES_DESCRIPTIONS.md

@@ -28,6 +28,7 @@ List of all available rules.
   - [early-return](#early-return)
   - [empty-block](#empty-block)
   - [empty-lines](#empty-lines)
+  - [enforce-else](#enforce-else)
   - [enforce-map-style](#enforce-map-style)
   - [enforce-repeated-arg-type-style](#enforce-repeated-arg-type-style)
   - [enforce-slice-style](#enforce-slice-style)
@@ -443,6 +444,14 @@ this rule warns when there are heading or trailing newlines in code blocks.
 
 _Configuration_: N/A
 
+## enforce-else
+
+_Description_: This rule warns if an `if` statement followed by one or more `else if` statements does not have a final `else` statement.
+
+This is consistent with the requirement to have a `default` clause in a `switch` statement (see [`enforce-switch-style` rule](#enforce-switch-style)).
+
+_Configuration_: N/A
+
 ## enforce-map-style
 
 _Description_: This rule enforces consistent usage of `make(map[type]type)` or `map[type]type{}` for map initialization.

config/config.go

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

rule/enforce_else.go

@@ -0,0 +1,130 @@
+package rule
+
+import (
+	"go/ast"
+	"go/token"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// EnforceElseRule enforces else branches in if... else if chains.
+type EnforceElseRule struct{}
+
+// Apply applies the rule to given file.
+func (*EnforceElseRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+
+	w := &lintEnforceElseRule{
+		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 (*EnforceElseRule) Name() string {
+	return "enforce-else"
+}
+
+type lintEnforceElseRule struct {
+	onFailure func(lint.Failure)
+	chain     []ast.Node
+}
+
+func (w *lintEnforceElseRule) addBranchToChain(branch ast.Node) {
+	if w.chain == nil {
+		w.chain = []ast.Node{}
+	}
+
+	w.chain = append(w.chain, branch)
+}
+
+func (w *lintEnforceElseRule) inIfChain() bool {
+	return len(w.chain) > 1
+}
+
+func (w *lintEnforceElseRule) resetChain() {
+	w.chain = []ast.Node{}
+}
+
+func (w *lintEnforceElseRule) Visit(node ast.Node) ast.Visitor {
+	ifStmt, ok := node.(*ast.IfStmt)
+	if !ok {
+		return w
+	}
+
+	w.walkBranch(ifStmt.Body)
+
+	switch ifStmt.Else {
+	case nil:
+		w.addBranchToChain(ifStmt.Body)
+		mustReport := w.inIfChain() && !w.allBranchesEndWithJumpStmt(w.chain)
+		if mustReport {
+			w.onFailure(lint.Failure{
+				Confidence: 1,
+				Node:       ifStmt,
+				Category:   lint.FailureCategoryMaintenance,
+				Failure:    `"if ... else if" constructs should end with "else" clauses`,
+			})
+		}
+
+	default: // there is an else branch
+		if chainedIf, ok := ifStmt.Else.(*ast.IfStmt); ok {
+			w.addBranchToChain(ifStmt.Body)
+			w.Visit(chainedIf)
+		} else {
+			w.walkBranch(ifStmt.Else)
+		}
+	}
+
+	w.resetChain()
+	return nil // if branches already analyzed
+}
+
+func (*lintEnforceElseRule) allBranchesEndWithJumpStmt(branches []ast.Node) bool {
+	for _, branch := range branches {
+		block, ok := branch.(*ast.BlockStmt)
+		if !ok || len(block.List) == 0 {
+			return false
+		}
+
+		lastStmt := block.List[len(block.List)-1]
+
+		if _, ok := lastStmt.(*ast.ReturnStmt); ok {
+			continue
+		}
+
+		if jump, ok := lastStmt.(*ast.BranchStmt); ok && jump.Tok == token.BREAK {
+			continue
+		}
+
+		return false
+	}
+
+	return true
+}
+
+func (w *lintEnforceElseRule) walkBranch(branch ast.Stmt) {
+	if branch == nil {
+		return
+	}
+
+	walker := &lintEnforceElseRule{
+		onFailure: w.onFailure,
+	}
+
+	ast.Walk(walker, branch)
+}

test/enforce_else_test.go

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

testdata/enforce_else.go

@@ -0,0 +1,53 @@
+package fixtures
+
+func enforceElse() {
+	if true {
+		// something
+	} else if true { // MATCH /"if ... else if" constructs should end with "else" clauses/
+		// something else
+	}
+
+	if true {
+		return
+	} else if true {
+		return
+	}
+
+	if true {
+		break
+	} else if true {
+		break
+	}
+
+	if true {
+		return
+	} else if true { // MATCH /"if ... else if" constructs should end with "else" clauses/
+		// something else
+	}
+
+	if true {
+		// something
+	} else if true { // MATCH /"if ... else if" constructs should end with "else" clauses/
+		return
+	}
+
+	if true {
+		// something
+	} else if true { // MATCH /"if ... else if" constructs should end with "else" clauses/
+		if true {
+			// something
+		} else if true { // MATCH /"if ... else if" constructs should end with "else" clauses/
+			return
+		}
+	}
+
+	if true {
+		if true {
+			// something
+		} else if true { // MATCH /"if ... else if" constructs should end with "else" clauses/
+			return
+		}
+	} else if true { // MATCH /"if ... else if" constructs should end with "else" clauses/
+		// something
+	}
+}