From cbe45ffc797a63287b888eeb88b33abbdb84888a Mon Sep 17 00:00:00 2001
From: chavacava <salvadorcavadini+github@gmail.com>
Date: Fri, 8 Jun 2018 16:06:29 +0200
Subject: [PATCH] Adds rule superfluous-else (an extension of
 indent-error-flow) (#13)

* Adds rule superfluous-else (an extension of indent-error-flow)

* Fix superfluous-else rule struct namming.

* Adds superfuous-else rule to the rules table
---
 README.md                     |  1 +
 fixtures/superfluous-else.go  | 73 +++++++++++++++++++++++++++++++
 rule/superfluous-else.go      | 81 +++++++++++++++++++++++++++++++++++
 test/superfluous-else_test.go | 12 ++++++
 4 files changed, 167 insertions(+)
 create mode 100644 fixtures/superfluous-else.go
 create mode 100644 rule/superfluous-else.go
 create mode 100644 test/superfluous-else_test.go

diff --git a/README.md b/README.md
index 3a73c6f..a4d6514 100644
--- a/README.md
+++ b/README.md
@@ -167,6 +167,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | `cyclomatic`          |  int   | Sets restriction for maximum Cyclomatic complexity.              |    no    |  no   |
 | `max-public-structs`  |  int   | The maximum number of public structs in a file.                  |    no    |  no   |
 | `file-header`         | string | Header which each file should have.                              |    no    |  no   |
+| `superfluous-else`    |  n/a   | Prevents redundant else statements (extends `indent-error-flow`) |    no    |  no   |
 
 ## Available Formatters
 
diff --git a/fixtures/superfluous-else.go b/fixtures/superfluous-else.go
new file mode 100644
index 0000000..436fa76
--- /dev/null
+++ b/fixtures/superfluous-else.go
@@ -0,0 +1,73 @@
+// Test of return+else warning.
+
+// Package pkg ...
+package pkg
+
+import (
+	"fmt"
+	"log"
+)
+
+func h(f func() bool) string {
+	for {
+		if ok := f(); ok {
+			a := 1
+			continue
+		} else { // MATCH /if block ends with a continue statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary)/
+			return "it's NOT okay!"
+		}
+	}
+}
+
+func i(f func() bool) string {
+	for {
+		if f() {
+			a := 1
+			continue
+		} else { // MATCH /if block ends with a continue statement, so drop this else and outdent its block/
+			log.Printf("non-positive")
+		}
+	}
+
+	return "ok"
+}
+
+func j(f func() bool) string {
+	for {
+		if f() {
+			break
+		} else { // MATCH /if block ends with a break statement, so drop this else and outdent its block/
+			log.Printf("non-positive")
+		}
+	}
+
+	return "ok"
+}
+
+func k() {
+	var a = 10
+	/* do loop execution */
+LOOP:
+	for a < 20 {
+		if a == 15 {
+			a = a + 1
+			goto LOOP
+		} else { // MATCH /if block ends with a goto statement, so drop this else and outdent its block/
+			fmt.Printf("value of a: %d\n", a)
+			a++
+		}
+	}
+}
+
+func j(f func() bool) string {
+	for {
+		if f() {
+			a := 1
+			fallthrough
+		} else {
+			log.Printf("non-positive")
+		}
+	}
+
+	return "ok"
+}
diff --git a/rule/superfluous-else.go b/rule/superfluous-else.go
new file mode 100644
index 0000000..a46ddd4
--- /dev/null
+++ b/rule/superfluous-else.go
@@ -0,0 +1,81 @@
+package rule
+
+import (
+	"go/ast"
+	"go/token"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// SuperfluousElseRule lints given else constructs.
+type SuperfluousElseRule struct{}
+
+// Apply applies the rule to given file.
+func (r *SuperfluousElseRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
+	var failures []lint.Failure
+
+	onFailure := func(failure lint.Failure) {
+		failures = append(failures, failure)
+	}
+
+	w := lintSuperfluousElse{make(map[*ast.IfStmt]bool), onFailure}
+	ast.Walk(w, file.AST)
+	return failures
+}
+
+// Name returns the rule name.
+func (r *SuperfluousElseRule) Name() string {
+	return "superfluous-else"
+}
+
+type lintSuperfluousElse struct {
+	ignore    map[*ast.IfStmt]bool
+	onFailure func(lint.Failure)
+}
+
+func (w lintSuperfluousElse) Visit(node ast.Node) ast.Visitor {
+	ifStmt, ok := node.(*ast.IfStmt)
+	if !ok || ifStmt.Else == nil {
+		return w
+	}
+	if w.ignore[ifStmt] {
+		return w
+	}
+	if elseif, ok := ifStmt.Else.(*ast.IfStmt); ok {
+		w.ignore[elseif] = true
+		return w
+	}
+	if _, ok := ifStmt.Else.(*ast.BlockStmt); !ok {
+		// only care about elses without conditions
+		return w
+	}
+	if len(ifStmt.Body.List) == 0 {
+		return w
+	}
+	shortDecl := false // does the if statement have a ":=" initialization statement?
+	if ifStmt.Init != nil {
+		if as, ok := ifStmt.Init.(*ast.AssignStmt); ok && as.Tok == token.DEFINE {
+			shortDecl = true
+		}
+	}
+	extra := ""
+	if shortDecl {
+		extra = " (move short variable declaration to its own line if necessary)"
+	}
+
+	lastStmt := ifStmt.Body.List[len(ifStmt.Body.List)-1]
+	if stmt, ok := lastStmt.(*ast.BranchStmt); ok {
+		token := stmt.Tok.String()
+		if token != "fallthrough" {
+			w.onFailure(lint.Failure{
+				Confidence: 1,
+				Node:       ifStmt.Else,
+				Category:   "indent",
+				URL:        "#indent-error-flow",
+				Failure:    "if block ends with a " + token + " statement, so drop this else and outdent its block" + extra,
+			})
+		}
+	}
+
+	return w
+}
diff --git a/test/superfluous-else_test.go b/test/superfluous-else_test.go
new file mode 100644
index 0000000..ab68b30
--- /dev/null
+++ b/test/superfluous-else_test.go
@@ -0,0 +1,12 @@
+package test
+
+import (
+	"testing"
+
+	"github.com/mgechev/revive/rule"
+)
+
+// TestSuperfluousElse rule.
+func TestSuperfluousElse(t *testing.T) {
+	testRule(t, "superfluous-else", &rule.SuperfluousElseRule{})
+}