refactor: extract shared code for linting if-else chains (#821)

* refactor: extract shared code for linting if-else chains

The rules "early-return", "indent-error-flow" and
"superfluous-else" have a similar structure. This
moves the common logic for classifying if-else chains
to a common package.

A few side benefits:
- "early-return" now handles os.Exit/log.Panicf/etc
- "superfluous-else" now handles (builtin) panic
- "superfluous-else" and "indent-error-flow" now handle if/else
  chains with 2+ "if" branches

* internal/ifelse: style fixes, renames, spelling

internal/ifelse/branch.go

@@ -0,0 +1,68 @@
+package ifelse
+
+import (
+	"fmt"
+	"go/ast"
+	"go/token"
+)
+
+// Branch contains information about a branch within an if-else chain.
+type Branch struct {
+	BranchKind
+	Call // The function called at the end for kind Panic or Exit.
+}
+
+// BlockBranch gets the Branch of an ast.BlockStmt.
+func BlockBranch(block *ast.BlockStmt) Branch {
+	blockLen := len(block.List)
+	if blockLen == 0 {
+		return Branch{BranchKind: Empty}
+	}
+
+	switch stmt := block.List[blockLen-1].(type) {
+	case *ast.ReturnStmt:
+		return Branch{BranchKind: Return}
+	case *ast.BlockStmt:
+		return BlockBranch(stmt)
+	case *ast.BranchStmt:
+		switch stmt.Tok {
+		case token.BREAK:
+			return Branch{BranchKind: Break}
+		case token.CONTINUE:
+			return Branch{BranchKind: Continue}
+		case token.GOTO:
+			return Branch{BranchKind: Goto}
+		}
+	case *ast.ExprStmt:
+		fn, ok := ExprCall(stmt)
+		if !ok {
+			break
+		}
+		kind, ok := DeviatingFuncs[fn]
+		if ok {
+			return Branch{BranchKind: kind, Call: fn}
+		}
+	}
+
+	return Branch{BranchKind: Regular}
+}
+
+// String returns a brief string representation
+func (b Branch) String() string {
+	switch b.BranchKind {
+	case Panic, Exit:
+		return fmt.Sprintf("... %v()", b.Call)
+	default:
+		return b.BranchKind.String()
+	}
+}
+
+// LongString returns a longer form string representation
+func (b Branch) LongString() string {
+	switch b.BranchKind {
+	case Panic, Exit:
+		return fmt.Sprintf("call to %v function", b.Call)
+	default:
+		return b.BranchKind.LongString()
+	}
+}

internal/ifelse/branch_kind.go

@@ -0,0 +1,98 @@
+package ifelse
+
+// BranchKind is a classifier for if-else branches. It says whether the branch is empty,
+// and whether the branch ends with a statement that deviates control flow.
+type BranchKind int
+
+const (
+	// Empty branches do nothing
+	Empty BranchKind = iota
+
+	// Return branches return from the current function
+	Return
+
+	// Continue branches continue a surrounding "for" loop
+	Continue
+
+	// Break branches break a surrounding "for" loop
+	Break
+
+	// Goto branches conclude with a "goto" statement
+	Goto
+
+	// Panic branches panic the current function
+	Panic
+
+	// Exit branches end the program
+	Exit
+
+	// Regular branches do not fit any category above
+	Regular
+)
+
+// IsEmpty tests if the branch is empty
+func (k BranchKind) IsEmpty() bool { return k == Empty }
+
+// Returns tests if the branch returns from the current function
+func (k BranchKind) Returns() bool { return k == Return }
+
+// Deviates tests if the control does not flow to the first
+// statement following the if-else chain.
+func (k BranchKind) Deviates() bool {
+	switch k {
+	case Empty, Regular:
+		return false
+	case Return, Continue, Break, Goto, Panic, Exit:
+		return true
+	default:
+		panic("invalid kind")
+	}
+}
+
+// String returns a brief string representation
+func (k BranchKind) String() string {
+	switch k {
+	case Empty:
+		return ""
+	case Regular:
+		return "..."
+	case Return:
+		return "... return"
+	case Continue:
+		return "... continue"
+	case Break:
+		return "... break"
+	case Goto:
+		return "... goto"
+	case Panic:
+		return "... panic()"
+	case Exit:
+		return "... os.Exit()"
+	default:
+		panic("invalid kind")
+	}
+}
+
+// LongString returns a longer form string representation
+func (k BranchKind) LongString() string {
+	switch k {
+	case Empty:
+		return "an empty block"
+	case Regular:
+		return "a regular statement"
+	case Return:
+		return "a return statement"
+	case Continue:
+		return "a continue statement"
+	case Break:
+		return "a break statement"
+	case Goto:
+		return "a goto statement"
+	case Panic:
+		return "a function call that panics"
+	case Exit:
+		return "a function call that exits the program"
+	default:
+		panic("invalid kind")
+	}
+}

internal/ifelse/chain.go

@@ -0,0 +1,9 @@
+package ifelse
+
+// Chain contains information about an if-else chain.
+type Chain struct {
+	If                   Branch // what happens at the end of the "if" block
+	Else                 Branch // what happens at the end of the "else" block
+	HasInitializer       bool   // is there an "if"-initializer somewhere in the chain?
+	HasPriorNonDeviating bool   // is there a prior "if" block that does NOT deviate control flow?
+}

internal/ifelse/doc.go

@@ -0,0 +1,6 @@
+// Package ifelse provides helpers for analysing the control flow in if-else chains,
+// presently used by the following rules:
+// - early-return
+// - indent-error-flow
+// - superfluous-else
+package ifelse

internal/ifelse/func.go

@@ -0,0 +1,51 @@
+package ifelse
+
+import (
+	"fmt"
+	"go/ast"
+)
+
+// Call contains the name of a function that deviates control flow.
+type Call struct {
+	Pkg  string // The package qualifier of the function, if not built-in.
+	Name string // The function name.
+}
+
+// DeviatingFuncs lists known control flow deviating function calls.
+var DeviatingFuncs = map[Call]BranchKind{
+	{"os", "Exit"}:     Exit,
+	{"log", "Fatal"}:   Exit,
+	{"log", "Fatalf"}:  Exit,
+	{"log", "Fatalln"}: Exit,
+	{"", "panic"}:      Panic,
+	{"log", "Panic"}:   Panic,
+	{"log", "Panicf"}:  Panic,
+	{"log", "Panicln"}: Panic,
+}
+
+// ExprCall gets the Call of an ExprStmt, if any.
+func ExprCall(expr *ast.ExprStmt) (Call, bool) {
+	call, ok := expr.X.(*ast.CallExpr)
+	if !ok {
+		return Call{}, false
+	}
+	switch v := call.Fun.(type) {
+	case *ast.Ident:
+		return Call{Name: v.Name}, true
+	case *ast.SelectorExpr:
+		if ident, ok := v.X.(*ast.Ident); ok {
+			return Call{Name: v.Sel.Name, Pkg: ident.Name}, true
+		}
+	}
+	return Call{}, false
+}
+
+// String returns the function name with package qualifier (if any)
+func (f Call) String() string {
+	switch {
+	case f.Pkg != "":
+		return fmt.Sprintf("%s.%s", f.Pkg, f.Name)
+	default:
+		return f.Name
+	}
+}

internal/ifelse/rule.go

@@ -0,0 +1,92 @@
+package ifelse
+
+import (
+	"go/ast"
+	"go/token"
+
+	"github.com/mgechev/revive/lint"
+)
+
+// Rule is an interface for linters operating on if-else chains
+type Rule interface {
+	CheckIfElse(chain Chain) (failMsg string)
+}
+
+// Apply evaluates the given Rule on if-else chains found within the given AST,
+// and returns the failures.
+//
+// Note that in if-else chain with multiple "if" blocks, only the *last* one is checked,
+// that is to say, given:
+//
+//	if foo {
+//	    ...
+//	} else if bar {
+//		...
+//	} else {
+//		...
+//	}
+//
+// Only the block following "bar" is linted. This is because the rules that use this function
+// do not presently have anything to say about earlier blocks in the chain.
+func Apply(rule Rule, node ast.Node, target Target) []lint.Failure {
+	v := &visitor{rule: rule, target: target}
+	ast.Walk(v, node)
+	return v.failures
+}
+
+type visitor struct {
+	failures []lint.Failure
+	target   Target
+	rule     Rule
+}
+
+func (v *visitor) Visit(node ast.Node) ast.Visitor {
+	ifStmt, ok := node.(*ast.IfStmt)
+	if !ok {
+		return v
+	}
+
+	v.visitChain(ifStmt, Chain{})
+	return nil
+}
+
+func (v *visitor) visitChain(ifStmt *ast.IfStmt, chain Chain) {
+	// look for other if-else chains nested inside this if { } block
+	ast.Walk(v, ifStmt.Body)
+
+	if ifStmt.Else == nil {
+		// no else branch
+		return
+	}
+
+	if as, ok := ifStmt.Init.(*ast.AssignStmt); ok && as.Tok == token.DEFINE {
+		chain.HasInitializer = true
+	}
+	chain.If = BlockBranch(ifStmt.Body)
+
+	switch elseBlock := ifStmt.Else.(type) {
+	case *ast.IfStmt:
+		if !chain.If.Deviates() {
+			chain.HasPriorNonDeviating = true
+		}
+		v.visitChain(elseBlock, chain)
+	case *ast.BlockStmt:
+		// look for other if-else chains nested inside this else { } block
+		ast.Walk(v, elseBlock)
+		chain.Else = BlockBranch(elseBlock)
+		if failMsg := v.rule.CheckIfElse(chain); failMsg != "" {
+			if chain.HasInitializer {
+				// if statement has a := initializer, so we might need to move the assignment
+				// onto its own line in case the body references it
+				failMsg += " (move short variable declaration to its own line if necessary)"
+			}
+			v.failures = append(v.failures, lint.Failure{
+				Confidence: 1,
+				Node:       v.target.node(ifStmt),
+				Failure:    failMsg,
+			})
+		}
+	default:
+		panic("invalid node type for else")
+	}
+}

internal/ifelse/target.go

@@ -0,0 +1,25 @@
+package ifelse
+
+import "go/ast"
+
+// Target decides what line/column should be indicated by the rule in question.
+type Target int
+
+const (
+	// TargetIf means the text refers to the "if"
+	TargetIf Target = iota
+
+	// TargetElse means the text refers to the "else"
+	TargetElse
+)
+
+func (t Target) node(ifStmt *ast.IfStmt) ast.Node {
+	switch t {
+	case TargetIf:
+		return ifStmt
+	case TargetElse:
+		return ifStmt.Else
+	default:
+		panic("bad target")
+	}
+}

rule/early-return.go

@@ -2,9 +2,8 @@ package rule
 
 import (
 	"fmt"
-	"go/ast"
-	"go/token"
 
+	"github.com/mgechev/revive/internal/ifelse"
 	"github.com/mgechev/revive/lint"
 )
 
@@ -13,16 +12,8 @@ import (
 type EarlyReturnRule struct{}
 
 // Apply applies the rule to given file.
-func (*EarlyReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+func (e *EarlyReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
-	var failures []lint.Failure
+	return ifelse.Apply(e, file.AST, ifelse.TargetIf)
-
-	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.
@@ -30,147 +21,26 @@ func (*EarlyReturnRule) Name() string {
 	return "early-return"
 }
 
-type lintEarlyReturnRule struct {
+// CheckIfElse evaluates the rule against an ifelse.Chain.
-	onFailure func(lint.Failure)
+func (e *EarlyReturnRule) CheckIfElse(chain ifelse.Chain) (failMsg string) {
-}
+	if !chain.Else.Deviates() {
-
+		// this rule only applies if the else-block deviates control flow
-func (w lintEarlyReturnRule) Visit(node ast.Node) ast.Visitor {
-	ifStmt, ok := node.(*ast.IfStmt)
-	if !ok {
-		return w
-	}
-
-	w.visitIf(ifStmt, false, false)
-	return nil
-}
-
-func (w lintEarlyReturnRule) visitIf(ifStmt *ast.IfStmt, hasNonReturnBranch, hasIfInitializer bool) {
-	// look for other if-else chains nested inside this if { } block
-	ast.Walk(w, ifStmt.Body)
-
-	if ifStmt.Else == nil {
-		// no else branch
 		return
 	}
 
-	if as, ok := ifStmt.Init.(*ast.AssignStmt); ok && as.Tok == token.DEFINE {
+	if chain.HasPriorNonDeviating && !chain.If.IsEmpty() {
-		hasIfInitializer = true
+		// if we de-indent this block then a previous branch
+		// might flow into it, affecting program behaviour
+		return
 	}
-	bodyFlow := w.branchFlow(ifStmt.Body)
 
-	switch elseBlock := ifStmt.Else.(type) {
+	if chain.If.Deviates() {
-	case *ast.IfStmt:
+		// avoid overlapping with superfluous-else
-		if bodyFlow.canFlowIntoNext() {
+		return
-			hasNonReturnBranch = true
-		}
-		w.visitIf(elseBlock, hasNonReturnBranch, hasIfInitializer)
-
-	case *ast.BlockStmt:
-		// look for other if-else chains nested inside this else { } block
-		ast.Walk(w, elseBlock)
-
-		if hasNonReturnBranch && bodyFlow != branchFlowEmpty {
-			// if we de-indent this block then a previous branch
-			// might flow into it, affecting program behaviour
-			return
-		}
-
-		if !bodyFlow.canFlowIntoNext() {
-			// avoid overlapping with superfluous-else
-			return
-		}
-
-		elseFlow := w.branchFlow(elseBlock)
-		if !elseFlow.canFlowIntoNext() {
-			failMsg := fmt.Sprintf("if c {%[1]s } else {%[2]s } can be simplified to if !c {%[2]s }%[1]s",
-				bodyFlow, elseFlow)
-
-			if hasIfInitializer {
-				// if statement has a := initializer, so we might need to move the assignment
-				// onto its own line in case the body references it
-				failMsg += " (move short variable declaration to its own line if necessary)"
-			}
-
-			w.onFailure(lint.Failure{
-				Confidence: 1,
-				Node:       ifStmt,
-				Failure:    failMsg,
-			})
-		}
-
-	default:
-		panic("invalid node type for else")
-	}
-}
-
-type branchFlowKind int
-
-const (
-	branchFlowEmpty branchFlowKind = iota
-	branchFlowReturn
-	branchFlowPanic
-	branchFlowContinue
-	branchFlowBreak
-	branchFlowGoto
-	branchFlowRegular
-)
-
-func (w lintEarlyReturnRule) branchFlow(block *ast.BlockStmt) branchFlowKind {
-	blockLen := len(block.List)
-	if blockLen == 0 {
-		return branchFlowEmpty
-	}
-
-	switch stmt := block.List[blockLen-1].(type) {
-	case *ast.ReturnStmt:
-		return branchFlowReturn
-	case *ast.BlockStmt:
-		return w.branchFlow(stmt)
-	case *ast.BranchStmt:
-		switch stmt.Tok {
-		case token.BREAK:
-			return branchFlowBreak
-		case token.CONTINUE:
-			return branchFlowContinue
-		case token.GOTO:
-			return branchFlowGoto
-		}
-	case *ast.ExprStmt:
-		if call, ok := stmt.X.(*ast.CallExpr); ok && isIdent(call.Fun, "panic") {
-			return branchFlowPanic
-		}
-	}
-
-	return branchFlowRegular
-}
-
-// Whether this branch's control can flow into the next statement following the if-else chain
-func (k branchFlowKind) canFlowIntoNext() bool {
-	switch k {
-	case branchFlowReturn, branchFlowPanic, branchFlowContinue, branchFlowBreak, branchFlowGoto:
-		return false
-	default:
-		return true
-	}
-}
-
-func (k branchFlowKind) String() string {
-	switch k {
-	case branchFlowEmpty:
-		return ""
-	case branchFlowReturn:
-		return " ... return"
-	case branchFlowPanic:
-		return " ... panic()"
-	case branchFlowContinue:
-		return " ... continue"
-	case branchFlowBreak:
-		return " ... break"
-	case branchFlowGoto:
-		return " ... goto"
-	case branchFlowRegular:
-		return " ..."
-	default:
-		panic("invalid kind")
 	}
+
+	if chain.If.IsEmpty() {
+		return fmt.Sprintf("if c { } else { %[1]v } can be simplified to if !c { %[1]v }", chain.Else)
+	}
+	return fmt.Sprintf("if c { ... } else { %[1]v } can be simplified to if !c { %[1]v } ...", chain.Else)
 }

rule/indent-error-flow.go

@@ -1,9 +1,7 @@
 package rule
 
 import (
-	"go/ast"
+	"github.com/mgechev/revive/internal/ifelse"
-	"go/token"
-
 	"github.com/mgechev/revive/lint"
 )
 
@@ -11,16 +9,8 @@ import (
 type IndentErrorFlowRule struct{}
 
 // Apply applies the rule to given file.
-func (*IndentErrorFlowRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+func (e *IndentErrorFlowRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
-	var failures []lint.Failure
+	return ifelse.Apply(e, file.AST, ifelse.TargetElse)
-
-	onFailure := func(failure lint.Failure) {
-		failures = append(failures, failure)
-	}
-
-	w := lintElse{make(map[*ast.IfStmt]bool), onFailure}
-	ast.Walk(w, file.AST)
-	return failures
 }
 
 // Name returns the rule name.
@@ -28,51 +18,23 @@ func (*IndentErrorFlowRule) Name() string {
 	return "indent-error-flow"
 }
 
-type lintElse struct {
+// CheckIfElse evaluates the rule against an ifelse.Chain.
-	ignore    map[*ast.IfStmt]bool
+func (e *IndentErrorFlowRule) CheckIfElse(chain ifelse.Chain) (failMsg string) {
-	onFailure func(lint.Failure)
+	if !chain.If.Deviates() {
-}
+		// this rule only applies if the if-block deviates control flow
+		return
+	}
 
-func (w lintElse) Visit(node ast.Node) ast.Visitor {
+	if chain.HasPriorNonDeviating {
-	ifStmt, ok := node.(*ast.IfStmt)
+		// if we de-indent the "else" block then a previous branch
-	if !ok || ifStmt.Else == nil {
+		// might flow into it, affecting program behaviour
-		return w
+		return
 	}
-	if w.ignore[ifStmt] {
+
-		if elseif, ok := ifStmt.Else.(*ast.IfStmt); ok {
+	if !chain.If.Returns() {
-			w.ignore[elseif] = true
+		// avoid overlapping with superfluous-else
-		}
+		return
-		return w
 	}
-	if elseif, ok := ifStmt.Else.(*ast.IfStmt); ok {
+
-		w.ignore[elseif] = true
+	return "if block ends with a return statement, so drop this else and outdent its block"
-		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
-		}
-	}
-	lastStmt := ifStmt.Body.List[len(ifStmt.Body.List)-1]
-	if _, ok := lastStmt.(*ast.ReturnStmt); ok {
-		extra := ""
-		if shortDecl {
-			extra = " (move short variable declaration to its own line if necessary)"
-		}
-		w.onFailure(lint.Failure{
-			Confidence: 1,
-			Node:       ifStmt.Else,
-			Category:   "indent",
-			Failure:    "if block ends with a return statement, so drop this else and outdent its block" + extra,
-		})
-	}
-	return w
 }

rule/superfluous-else.go

@@ -2,9 +2,7 @@ package rule
 
 import (
 	"fmt"
-	"go/ast"
+	"github.com/mgechev/revive/internal/ifelse"
-	"go/token"
-
 	"github.com/mgechev/revive/lint"
 )
 
@@ -12,27 +10,8 @@ import (
 type SuperfluousElseRule struct{}
 
 // Apply applies the rule to given file.
-func (*SuperfluousElseRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+func (e *SuperfluousElseRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
-	var failures []lint.Failure
+	return ifelse.Apply(e, file.AST, ifelse.TargetElse)
-	onFailure := func(failure lint.Failure) {
-		failures = append(failures, failure)
-	}
-
-	branchingFunctions := map[string]map[string]bool{
-		"os": {"Exit": true},
-		"log": {
-			"Fatal":   true,
-			"Fatalf":  true,
-			"Fatalln": true,
-			"Panic":   true,
-			"Panicf":  true,
-			"Panicln": true,
-		},
-	}
-
-	w := lintSuperfluousElse{make(map[*ast.IfStmt]bool), onFailure, branchingFunctions}
-	ast.Walk(w, file.AST)
-	return failures
 }
 
 // Name returns the rule name.
@@ -40,75 +19,23 @@ func (*SuperfluousElseRule) Name() string {
 	return "superfluous-else"
 }
 
-type lintSuperfluousElse struct {
+// CheckIfElse evaluates the rule against an ifelse.Chain.
-	ignore             map[*ast.IfStmt]bool
+func (e *SuperfluousElseRule) CheckIfElse(chain ifelse.Chain) (failMsg string) {
-	onFailure          func(lint.Failure)
+	if !chain.If.Deviates() {
-	branchingFunctions map[string]map[string]bool
+		// this rule only applies if the if-block deviates control flow
-}
+		return
-
+	}
-func (w lintSuperfluousElse) Visit(node ast.Node) ast.Visitor {
+
-	ifStmt, ok := node.(*ast.IfStmt)
+	if chain.HasPriorNonDeviating {
-	if !ok || ifStmt.Else == nil {
+		// if we de-indent the "else" block then a previous branch
-		return w
+		// might flow into it, affecting program behaviour
-	}
+		return
-	if w.ignore[ifStmt] {
+	}
-		if elseif, ok := ifStmt.Else.(*ast.IfStmt); ok {
+
-			w.ignore[elseif] = true
+	if chain.If.Returns() {
-		}
+		// avoid overlapping with indent-error-flow
-		return w
+		return
-	}
+	}
-	if elseif, ok := ifStmt.Else.(*ast.IfStmt); ok {
+
-		w.ignore[elseif] = true
+	return fmt.Sprintf("if block ends with %v, so drop this else and outdent its block", chain.If.LongString())
-		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]
-	switch stmt := lastStmt.(type) {
-	case *ast.BranchStmt:
-		tok := stmt.Tok.String()
-		if tok != "fallthrough" {
-			w.onFailure(newFailure(ifStmt.Else, "if block ends with a "+tok+" statement, so drop this else and outdent its block"+extra))
-		}
-	case *ast.ExprStmt:
-		if ce, ok := stmt.X.(*ast.CallExpr); ok { // it's a function call
-			if fc, ok := ce.Fun.(*ast.SelectorExpr); ok {
-				if id, ok := fc.X.(*ast.Ident); ok {
-					fn := fc.Sel.Name
-					pkg := id.Name
-					if w.branchingFunctions[pkg][fn] { // it's a call to a branching function
-						w.onFailure(
-							newFailure(ifStmt.Else, fmt.Sprintf("if block ends with call to %s.%s function, so drop this else and outdent its block%s", pkg, fn, extra)))
-					}
-				}
-			}
-		}
-	}
-
-	return w
-}
-
-func newFailure(node ast.Node, msg string) lint.Failure {
-	return lint.Failure{
-		Confidence: 1,
-		Node:       node,
-		Category:   "indent",
-		Failure:    msg,
-	}
 }

testdata/early-return.go

@@ -2,6 +2,8 @@
 
 package fixtures
 
+import "os"
+
 func earlyRet() bool {
 	if cond { //   MATCH /if c { ... } else { ... return } can be simplified to if !c { ... return } .../
 		println()
@@ -124,4 +126,10 @@ func earlyRet() bool {
 	} else {
 		return false
 	}
+
+	if cond { //MATCH /if c { ... } else { ... os.Exit() } can be simplified to if !c { ... os.Exit() } .../
+		println()
+	} else {
+		os.Exit(0)
+	}
 }

testdata/golint/indent-error-flow.go

@@ -38,3 +38,12 @@ func h(f func() bool, x int) string {
 	}
 }
 
+func i() string {
+	if err == author.ErrCourseNotFound {
+		return "not found"
+	} else if err == author.AnotherError {
+		return "something else"
+	} else { // MATCH /if block ends with a return statement, so drop this else and outdent its block/
+		return "okay"
+	}
+}

testdata/superfluous-else.go

@@ -4,9 +4,9 @@
 package pkg
 
 import (
-	"os"
 	"fmt"
 	"log"
+	"os"
 )
 
 func h(f func() bool) string {
@@ -74,72 +74,82 @@ func j(f func() bool) string {
 }
 
 func fatal1() string {
-		if f() {
+	if f() {
-			a := 1
+		a := 1
-			log.Fatal("x")
+		log.Fatal("x")
-		} else { // MATCH /if block ends with call to log.Fatal function, so drop this else and outdent its block/
+	} else { // MATCH /if block ends with call to log.Fatal function, so drop this else and outdent its block/
-			log.Printf("non-positive")
+		log.Printf("non-positive")
-		}
+	}
 	return "ok"
 }
 
 func fatal2() string {
-		if f() {
+	if f() {
-			a := 1
+		a := 1
-			log.Fatalf("x")
+		log.Fatalf("x")
-		} else { // MATCH /if block ends with call to log.Fatalf function, so drop this else and outdent its block/
+	} else { // MATCH /if block ends with call to log.Fatalf function, so drop this else and outdent its block/
-			log.Printf("non-positive")
+		log.Printf("non-positive")
-		}
+	}
 	return "ok"
 }
 
 func fatal3() string {
-		if f() {
+	if f() {
-			a := 1
+		a := 1
-			log.Fatalln("x")
+		log.Fatalln("x")
-		} else { // MATCH /if block ends with call to log.Fatalln function, so drop this else and outdent its block/
+	} else { // MATCH /if block ends with call to log.Fatalln function, so drop this else and outdent its block/
-			log.Printf("non-positive")
+		log.Printf("non-positive")
-		}
+	}
 	return "ok"
 }
 
 func exit1() string {
-		if f() {
+	if f() {
-			a := 1
+		a := 1
-			os.Exit(2)
+		os.Exit(2)
-		} else { // MATCH /if block ends with call to os.Exit function, so drop this else and outdent its block/
+	} else { // MATCH /if block ends with call to os.Exit function, so drop this else and outdent its block/
-			log.Printf("non-positive")
+		log.Printf("non-positive")
-		}
+	}
 	return "ok"
 }
 
 func Panic1() string {
-		if f() {
+	if f() {
-			a := 1
+		a := 1
-			log.Panic(2)
+		log.Panic(2)
-		} else { // MATCH /if block ends with call to log.Panic function, so drop this else and outdent its block/
+	} else { // MATCH /if block ends with call to log.Panic function, so drop this else and outdent its block/
-			log.Printf("non-positive")
+		log.Printf("non-positive")
-		}
+	}
 	return "ok"
 }
 
 func Panic2() string {
-		if f() {
+	if f() {
-			a := 1
+		a := 1
-			log.Panicf(2)
+		log.Panicf(2)
-		} else { // MATCH /if block ends with call to log.Panicf function, so drop this else and outdent its block/
+	} else { // MATCH /if block ends with call to log.Panicf function, so drop this else and outdent its block/
-			log.Printf("non-positive")
+		log.Printf("non-positive")
-		}
+	}
 	return "ok"
 }
 
 func Panic3() string {
-		if f() {
+	if f() {
-			a := 1
+		a := 1
-			log.Panicln(2)
+		log.Panicln(2)
-		} else { // MATCH /if block ends with call to log.Panicln function, so drop this else and outdent its block/
+	} else { // MATCH /if block ends with call to log.Panicln function, so drop this else and outdent its block/
-			log.Printf("non-positive")
+		log.Printf("non-positive")
-		}
+	}
+	return "ok"
+}
+
+func Panic4() string {
+	if f() {
+		a := 1
+		panic(2)
+	} else { // MATCH /if block ends with call to panic function, so drop this else and outdent its block/
+		log.Printf("non-positive")
+	}
 	return "ok"
 }
 
@@ -154,4 +164,14 @@ func noreg_19(f func() bool, x int) string {
 	} else {
 		// side effect
 	}
-}
+}
+
+func MultiBranch() string {
+	if _, ok := f(); ok {
+		continue
+	} else if _, err := get(); err == nil {
+		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)/
+		delete(m, x)
+	}
+}