mirror of
https://github.com/mgechev/revive.git
synced 2025-10-30 23:37:49 +02:00
refactor: remove rule enforce-else (#1495)
This commit is contained in:
chavacava
GitHub
@@ -534,7 +534,6 @@ 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 |
|
||||
|
||||
@@ -28,7 +28,6 @@ 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)
|
||||
@@ -451,14 +450,6 @@ 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.
|
||||
|
||||
@@ -106,7 +106,6 @@ var allRules = append([]lint.Rule{
|
||||
&rule.UseFmtPrintRule{},
|
||||
&rule.EnforceSwitchStyleRule{},
|
||||
&rule.IdenticalSwitchConditionsRule{},
|
||||
&rule.EnforceElseRule{},
|
||||
&rule.IdenticalIfElseIfConditionsRule{},
|
||||
&rule.IdenticalIfElseIfBranchesRule{},
|
||||
&rule.IdenticalSwitchBranchesRule{},
|
||||
|
||||
@@ -1,130 +0,0 @@
|
||||
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)
|
||||
}
|
||||
@@ -1,11 +0,0 @@
|
||||
package test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/mgechev/revive/rule"
|
||||
)
|
||||
|
||||
func TestEnforceElse(t *testing.T) {
|
||||
testRule(t, "enforce_else", &rule.EnforceElseRule{})
|
||||
}
|
||||
53
testdata/enforce_else.go
vendored
53
testdata/enforce_else.go
vendored
@@ -1,53 +0,0 @@
|
||||
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
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user