Late return rule (#406)

* fisrt working version of late-return rule

* late-update: adds doc

* renames to early-return

* fix rule failure condition

* fix alphabetical sorting of early-return

README.md

@@ -349,7 +349,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`unhandled-error`](./RULES_DESCRIPTIONS.md#unhandled-error)   | []string   | Warns on unhandled errors returned by funcion calls    |    no    |  yes   |
 | [`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   |
 ## Configurable rules
 
 Here you can find how you can configure some of the existing rules:

RULES_DESCRIPTIONS.md

@@ -21,6 +21,7 @@ List of all available rules.
   - [deep-exit](#deep-exit)
   - [dot-imports](#dot-imports)
   - [duplicated-imports](#duplicated-imports)
+  - [early-return](#early-return)
   - [empty-block](#empty-block)
   - [empty-lines](#empty-lines)
   - [error-naming](#error-naming)
@@ -203,6 +204,29 @@ _Description_: It is possible to unintentionally import the same package twice.
 
 _Configuration_: N/A
 
+### early-return
+
+_Description_: In GO it is idiomatic to minimize nesting statements, a typical example is to avoid if-then-else constructions. This rule spots constructions like
+```go
+if cond {
+  // do something 
+} else {
+  // do other thing
+  return ...
+}
+```
+that can be rewritten into more idiomatic:
+```go
+if ! cond {
+  // do other thing
+  return ... 
+}
+
+// do something
+```
+
+_Configuration_: N/A
+
 ## empty-block
 
 _Description_: Empty blocks make code less readable and could be a symptom of a bug or unfinished refactoring.

config.go

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

rule/early-return.go

@@ -0,0 +1,78 @@
+package rule
+
+import (
+	"go/ast"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// EarlyReturnRule lints given else constructs.
+type EarlyReturnRule struct{}
+
+// Apply applies the rule to given file.
+func (r *EarlyReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+
+	w := lintEarlyReturnRule{onFailure: onFailure}
+	ast.Walk(w, file.AST)
+	return failures
+}
+
+// Name returns the rule name.
+func (r *EarlyReturnRule) Name() string {
+	return "early-return"
+}
+
+type lintEarlyReturnRule struct {
+	onFailure func(lint.Failure)
+}
+
+func (w lintEarlyReturnRule) Visit(node ast.Node) ast.Visitor {
+	switch n := node.(type) {
+	case *ast.IfStmt:
+		if n.Else == nil {
+			// no else branch
+			return w
+		}
+
+		elseBlock, ok := n.Else.(*ast.BlockStmt)
+		if !ok {
+			// is if-else-if
+			return w
+		}
+
+		lenElseBlock := len(elseBlock.List)
+		if lenElseBlock < 1 {
+			// empty else block, continue (there is another rule that warns on empty blocks)
+			return w
+		}
+
+		lenThenBlock := len(n.Body.List)
+		if lenThenBlock < 1 {
+			// then block is empty thus the stmt can be simplified
+			w.onFailure(lint.Failure{
+				Confidence: 1,
+				Node:       n,
+				Failure:    "if c { } else {... return} can be simplified to if !c { ... return }",
+			})
+
+			return w
+		}
+
+		_, lastThenStmtIsReturn := n.Body.List[lenThenBlock-1].(*ast.ReturnStmt)
+		_, lastElseStmtIsReturn := elseBlock.List[lenElseBlock-1].(*ast.ReturnStmt)
+		if lastElseStmtIsReturn && !lastThenStmtIsReturn {
+			w.onFailure(lint.Failure{
+				Confidence: 1,
+				Node:       n,
+				Failure:    "if c {...} else {... return } can be simplified to if !c { ... return } ...",
+			})
+		}
+	}
+
+	return w
+}

test/early-return_test.go

@@ -0,0 +1,12 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/rule"
+)
+
+// TestEarlyReturn tests early-return rule.
+func TestEarlyReturn(t *testing.T) {
+	testRule(t, "early-return", &rule.EarlyReturnRule{})
+}

testdata/early-return.go

@@ -0,0 +1,62 @@
+// Test of empty-blocks.
+
+package fixtures
+
+func earlyRet() bool {
+	if cond { //   MATCH /if c {...} else {... return } can be simplified to if !c { ... return } .../
+		println()
+		println()
+		println()
+	} else {
+		return false
+	}
+
+	if cond { //MATCH /if c {...} else {... return } can be simplified to if !c { ... return } .../
+		println()
+	} else {
+		return false
+	}
+
+	if cond { //MATCH /if c { } else {... return} can be simplified to if !c { ... return }/
+	} else {
+		return false
+	}
+
+	if cond {
+		println()
+	} else if cond { //MATCH /if c { } else {... return} can be simplified to if !c { ... return }/
+	} else {
+		return false
+	}
+
+	if cond {
+		println()
+	} else if cond { //MATCH /if c {...} else {... return } can be simplified to if !c { ... return } .../
+		println()
+	} else {
+		return false
+	}
+
+	// Case already covered by golint
+	if cond {
+		return true
+	} else {
+		return false
+	}
+
+	if cond { //MATCH /if c {...} else {... return } can be simplified to if !c { ... return } .../
+		println()
+		println()
+		println()
+	} else {
+		return false
+	}
+
+	if cond {
+		println()
+		println()
+		println()
+	} else {
+		println()
+	}
+}