diff --git a/README.md b/README.md index eb35ecb..a6e6fb1 100644 --- a/README.md +++ b/README.md @@ -552,7 +552,9 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`function-result-limit`](./RULES_DESCRIPTIONS.md#function-result-limit) | int (defaults to 3)| Specifies the maximum number of results a function can return | no | no | | [`get-return`](./RULES_DESCRIPTIONS.md#get-return) | n/a | Warns on getters that do not yield any result | no | no | | [`identical-branches`](./RULES_DESCRIPTIONS.md#identical-branches) | n/a | Spots if-then-else statements with identical `then` and `else` branches | no | no | +| [`identical-ifelseif-branches`](./RULES_DESCRIPTIONS.md#identical-ifelseif-branches) | n/a | Spots `if ... else if` chains with identical branches. | no | no | | [`identical-ifelseif-conditions`](./RULES_DESCRIPTIONS.md#identical-ifelseif-conditions) | n/a | Spots identical conditions in `if ... else if` chains. | no | no | +| [`identical-switch-branches`](./RULES_DESCRIPTIONS.md#identical-switch-branches) | n/a | Spots `switch` with identical branches. | no | no | | [`identical-switch-conditions`](./RULES_DESCRIPTIONS.md#identical-switch-conditions) | n/a | Spots identical conditions in case clauses of `switch` statements. | no | no | | [`if-return`](./RULES_DESCRIPTIONS.md#if-return) | n/a | Redundant if when returning an error. | no | no | | [`import-alias-naming`](./RULES_DESCRIPTIONS.md#import-alias-naming) | string or map[string]string (defaults to allow regex pattern `^[a-z][a-z0-9]{0,}$`) | Conventions around the naming of import aliases. | no | no | diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 78f6330..14c1be7 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -46,7 +46,9 @@ List of all available rules. - [function-result-limit](#function-result-limit) - [get-return](#get-return) - [identical-branches](#identical-branches) + - [identical-ifelseif-branches](#identical-ifelseif-branches) - [identical-ifelseif-conditions](#identical-ifelseif-conditions) + - [identical-switch-branches](#identical-switch-branches) - [identical-switch-conditions](#identical-switch-conditions) - [if-return](#if-return) - [import-alias-naming](#import-alias-naming) @@ -744,6 +746,13 @@ _Description_: An `if-then-else` conditional with identical implementations in b _Configuration_: N/A +## identical-ifelseif-branches + +_Description_: an `if ... else if` chain with identical branches makes maintenance harder +and might be a source of bugs. Duplicated branches should be consolidated in one. + +_Configuration_: N/A + ## identical-ifelseif-conditions _Description_: an `if ... else if` chain with identical conditions can lead to @@ -751,6 +760,12 @@ unreachable code and is a potential source of bugs while making the code harder _Configuration_: N/A +## identical-switch-branches + +_Description_: a `switch` with identical branches makes maintenance harder +and might be a source of bugs. Duplicated branches should be consolidated +in one case clause. + ## identical-switch-conditions _Description_: a `switch` statement with cases with the same condition can lead to diff --git a/config/config.go b/config/config.go index 1b75723..200753e 100644 --- a/config/config.go +++ b/config/config.go @@ -108,6 +108,8 @@ var allRules = append([]lint.Rule{ &rule.IdenticalSwitchConditionsRule{}, &rule.EnforceElseRule{}, &rule.IdenticalIfElseIfConditionsRule{}, + &rule.IdenticalIfElseIfBranchesRule{}, + &rule.IdenticalSwitchBranchesRule{}, }, defaultRules...) // allFormatters is a list of all available formatters to output the linting results. diff --git a/rule/identical_branches.go b/rule/identical_branches.go index 193d549..22895cd 100644 --- a/rule/identical_branches.go +++ b/rule/identical_branches.go @@ -1,15 +1,13 @@ package rule import ( - "fmt" "go/ast" - "go/token" "github.com/mgechev/revive/internal/astutils" "github.com/mgechev/revive/lint" ) -// IdenticalBranchesRule warns on constant logical expressions. +// IdenticalBranchesRule warns on if...else statements with both branches being the same. type IdenticalBranchesRule struct{} // Apply applies the rule to given file. @@ -20,7 +18,7 @@ func (*IdenticalBranchesRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fa failures = append(failures, failure) } - w := &lintIdenticalBranches{file: file, onFailure: onFailure} + w := &lintIdenticalBranches{onFailure: onFailure} for _, decl := range file.AST.Decls { fn, ok := decl.(*ast.FuncDecl) if !ok || fn.Body == nil { @@ -38,111 +36,37 @@ func (*IdenticalBranchesRule) Name() string { return "identical-branches" } -// lintIdenticalBranches implements the root or main AST walker of the rule. -// This walker will activate other walkers depending on the satement under analysis: -// - simple if ... else ... -// - if ... else if ... chain, -// - switch (not yet implemented). type lintIdenticalBranches struct { - file *lint.File // only necessary to retrieve the line number of branches onFailure func(lint.Failure) } func (w *lintIdenticalBranches) Visit(node ast.Node) ast.Visitor { - switch n := node.(type) { - case *ast.IfStmt: - if w.isIfElseIf(n) { - walker := &lintIfChainIdenticalBranches{ - onFailure: w.onFailure, - file: w.file, - rootWalker: w, - } - - ast.Walk(walker, n) - return nil // the walker already analyzed inner branches - } - - w.lintSimpleIf(n) - return w - - case *ast.SwitchStmt: - if n.Tag == nil { - return w // do not lint untagged switches (order of case evaluation might be important) - } - - w.lintSwitch(n) - return nil // switch branches already analyzed - default: + ifStmt, ok := node.(*ast.IfStmt) + if !ok { return w } -} -func (w *lintIdenticalBranches) lintSwitch(switchStmt *ast.SwitchStmt) { - doesFallthrough := func(stmts []ast.Stmt) bool { - if len(stmts) == 0 { - return false - } - - ft, ok := stmts[len(stmts)-1].(*ast.BranchStmt) - return ok && ft.Tok == token.FALLTHROUGH + if ifStmt.Else == nil { + return w // if without else } - hashes := map[string]int{} // map hash(branch code) -> branch line - for _, cc := range switchStmt.Body.List { - caseClause := cc.(*ast.CaseClause) - if doesFallthrough(caseClause.Body) { - continue // skip fallthrough branches - } - branch := &ast.BlockStmt{ - List: caseClause.Body, - } - hash := astutils.NodeHash(branch) - branchLine := w.file.ToPosition(caseClause.Pos()).Line - if matchLine, ok := hashes[hash]; ok { - w.newFailure( - switchStmt, - fmt.Sprintf(`"switch" with identical branches (lines %d and %d)`, matchLine, branchLine), - 1.0, - ) - } - - hashes[hash] = branchLine - w.walkBranch(branch) - } -} - -// walkBranch analyzes the given branch. -func (w *lintIdenticalBranches) walkBranch(branch ast.Stmt) { - if branch == nil { - return + elseBranch, ok := ifStmt.Else.(*ast.BlockStmt) + if !ok { // if-else-if construction, the rule only copes with single if...else statements + return w } - walker := &lintIdenticalBranches{ - onFailure: w.onFailure, - file: w.file, + if w.identicalBranches(ifStmt.Body, elseBranch) { + w.onFailure(lint.Failure{ + Confidence: 1.0, + Node: ifStmt, + Category: lint.FailureCategoryLogic, + Failure: "both branches of the if are identical", + }) } - ast.Walk(walker, branch) -} - -func (*lintIdenticalBranches) isIfElseIf(node *ast.IfStmt) bool { - _, ok := node.Else.(*ast.IfStmt) - return ok -} - -func (w *lintIdenticalBranches) lintSimpleIf(n *ast.IfStmt) { - if n.Else == nil { - return - } - - elseBranch, ok := n.Else.(*ast.BlockStmt) - if !ok { // if-else-if construction (should never be the case but keep the check for safer refactoring) - return - } - - if w.identicalBranches(n.Body, elseBranch) { - w.newFailure(n, "both branches of the if are identical", 1.0) - } + ast.Walk(w, ifStmt.Body) + ast.Walk(w, ifStmt.Else) + return nil } func (*lintIdenticalBranches) identicalBranches(body, elseBranch *ast.BlockStmt) bool { @@ -155,111 +79,3 @@ func (*lintIdenticalBranches) identicalBranches(body, elseBranch *ast.BlockStmt) return bodyStr == elseStr } - -func (w *lintIdenticalBranches) newFailure(node ast.Node, msg string, confidence float64) { - w.onFailure(lint.Failure{ - Confidence: confidence, - Node: node, - Category: lint.FailureCategoryLogic, - Failure: msg, - }) -} - -type lintIfChainIdenticalBranches struct { - file *lint.File // only necessary to retrieve the line number of branches - onFailure func(lint.Failure) - branches []ast.Stmt // hold branches to compare - rootWalker *lintIdenticalBranches // the walker to use to recursively analize inner branches - hasComplexCondition bool // indicates if one of the if conditions is "complex" -} - -// addBranch adds a branch to the list of branches to be compared. -func (w *lintIfChainIdenticalBranches) addBranch(branch ast.Stmt) { - if branch == nil { - return - } - - if w.branches == nil { - w.resetBranches() - } - - w.branches = append(w.branches, branch) -} - -// resetBranches resets (clears) the list of branches to compare. -func (w *lintIfChainIdenticalBranches) resetBranches() { - w.branches = []ast.Stmt{} - w.hasComplexCondition = false -} - -func (w *lintIfChainIdenticalBranches) Visit(node ast.Node) ast.Visitor { - n, ok := node.(*ast.IfStmt) - if !ok { - return w - } - - // recursively analyze the then-branch - w.rootWalker.walkBranch(n.Body) - - if n.Init == nil { // only check if without initialization to avoid false positives - w.addBranch(n.Body) - } - - if w.isComplexCondition(n.Cond) { - w.hasComplexCondition = true - } - - if n.Else != nil { - if chainedIf, ok := n.Else.(*ast.IfStmt); ok { - w.Visit(chainedIf) - } else { - w.addBranch(n.Else) - w.rootWalker.walkBranch(n.Else) - } - } - - identicalBranches := w.identicalBranches(w.branches) - for _, branchPair := range identicalBranches { - msg := fmt.Sprintf(`"if...else if" chain with identical branches (lines %d and %d)`, branchPair[0], branchPair[1]) - confidence := 1.0 - if w.hasComplexCondition { - confidence = 0.8 - } - w.rootWalker.newFailure(w.branches[0], msg, confidence) - } - - w.resetBranches() - return nil -} - -// isComplexCondition returns true if the given expression is "complex", false otherwise. -// An expression is considered complex if it has a function call. -func (*lintIfChainIdenticalBranches) isComplexCondition(expr ast.Expr) bool { - calls := astutils.PickNodes(expr, func(n ast.Node) bool { - _, ok := n.(*ast.CallExpr) - return ok - }) - - return len(calls) > 0 -} - -// identicalBranches yields pairs of (line numbers) of identical branches from the given branches. -func (w *lintIfChainIdenticalBranches) identicalBranches(branches []ast.Stmt) [][]int { - result := [][]int{} - if len(branches) < 2 { - return result // only one branch to compare thus we return - } - - hashes := map[string]int{} // branch code hash -> branch line - for _, branch := range branches { - hash := astutils.NodeHash(branch) - branchLine := w.file.ToPosition(branch.Pos()).Line - if match, ok := hashes[hash]; ok { - result = append(result, []int{match, branchLine}) - } - - hashes[hash] = branchLine - } - - return result -} diff --git a/rule/identical_ifelseif_branches.go b/rule/identical_ifelseif_branches.go new file mode 100644 index 0000000..b079bed --- /dev/null +++ b/rule/identical_ifelseif_branches.go @@ -0,0 +1,186 @@ +package rule + +import ( + "fmt" + "go/ast" + + "github.com/mgechev/revive/internal/astutils" + "github.com/mgechev/revive/lint" +) + +// IdenticalIfElseIfBranchesRule warns on if...else if chains with identical branches. +type IdenticalIfElseIfBranchesRule struct{} + +// Apply applies the rule to given file. +func (*IdenticalIfElseIfBranchesRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { + var failures []lint.Failure + + onFailure := func(failure lint.Failure) { + failures = append(failures, failure) + } + + getStmtLine := func(s ast.Stmt) int { + return file.ToPosition(s.Pos()).Line + } + + w := &rootWalkerIfElseIfIdenticalBranches{getStmtLine: getStmtLine, 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 (*IdenticalIfElseIfBranchesRule) Name() string { + return "identical-ifelseif-branches" +} + +type rootWalkerIfElseIfIdenticalBranches struct { + getStmtLine func(ast.Stmt) int + onFailure func(lint.Failure) +} + +func (w *rootWalkerIfElseIfIdenticalBranches) Visit(node ast.Node) ast.Visitor { + n, ok := node.(*ast.IfStmt) + if !ok { + return w + } + + _, isIfElseIf := n.Else.(*ast.IfStmt) + if isIfElseIf { + walker := &lintIfChainIdenticalBranches{ + onFailure: w.onFailure, + getStmtLine: w.getStmtLine, + rootWalker: w, + } + + ast.Walk(walker, n) + return nil // the walker already analyzed inner branches + } + + return w +} + +// walkBranch analyzes the given branch. +func (w *rootWalkerIfElseIfIdenticalBranches) walkBranch(branch ast.Stmt) { + if branch == nil { + return + } + + walker := &rootWalkerIfElseIfIdenticalBranches{ + onFailure: w.onFailure, + getStmtLine: w.getStmtLine, + } + + ast.Walk(walker, branch) +} + +type lintIfChainIdenticalBranches struct { + getStmtLine func(ast.Stmt) int + onFailure func(lint.Failure) + branches []ast.Stmt // hold branches to compare + rootWalker *rootWalkerIfElseIfIdenticalBranches // the walker to use to recursively analyze inner branches + hasComplexCondition bool // indicates if one of the if conditions is "complex" +} + +// addBranch adds a branch to the list of branches to be compared. +func (w *lintIfChainIdenticalBranches) addBranch(branch ast.Stmt) { + if branch == nil { + return + } + + if w.branches == nil { + w.resetBranches() + } + + w.branches = append(w.branches, branch) +} + +// resetBranches resets (clears) the list of branches to compare. +func (w *lintIfChainIdenticalBranches) resetBranches() { + w.branches = []ast.Stmt{} + w.hasComplexCondition = false +} + +func (w *lintIfChainIdenticalBranches) Visit(node ast.Node) ast.Visitor { + n, ok := node.(*ast.IfStmt) + if !ok { + return w + } + + // recursively analyze the then-branch + w.rootWalker.walkBranch(n.Body) + + if n.Init == nil { // only check if without initialization to avoid false positives + w.addBranch(n.Body) + } + + if w.isComplexCondition(n.Cond) { + w.hasComplexCondition = true + } + + if n.Else != nil { + if chainedIf, ok := n.Else.(*ast.IfStmt); ok { + w.Visit(chainedIf) + } else { + w.addBranch(n.Else) + w.rootWalker.walkBranch(n.Else) + } + } + + identicalBranches := w.identicalBranches(w.branches) + for _, branchPair := range identicalBranches { + msg := fmt.Sprintf(`"if...else if" chain with identical branches (lines %d and %d)`, branchPair[0], branchPair[1]) + confidence := 1.0 + if w.hasComplexCondition { + confidence = 0.8 + } + w.onFailure(lint.Failure{ + Confidence: confidence, + Node: w.branches[0], + Category: lint.FailureCategoryLogic, + Failure: msg, + }) + } + + w.resetBranches() + return nil +} + +// isComplexCondition returns true if the given expression is "complex", false otherwise. +// An expression is considered complex if it has a function call. +func (*lintIfChainIdenticalBranches) isComplexCondition(expr ast.Expr) bool { + calls := astutils.PickNodes(expr, func(n ast.Node) bool { + _, ok := n.(*ast.CallExpr) + return ok + }) + + return len(calls) > 0 +} + +// identicalBranches yields pairs of (line numbers) of identical branches from the given branches. +func (w *lintIfChainIdenticalBranches) identicalBranches(branches []ast.Stmt) [][]int { + result := [][]int{} + if len(branches) < 2 { + return result // no other branch to compare thus we return + } + + hashes := map[string]int{} // branch code hash -> branch line + for _, branch := range branches { + hash := astutils.NodeHash(branch) + branchLine := w.getStmtLine(branch) + if match, ok := hashes[hash]; ok { + result = append(result, []int{match, branchLine}) + } + + hashes[hash] = branchLine + } + + return result +} diff --git a/rule/identical_switch_branches.go b/rule/identical_switch_branches.go new file mode 100644 index 0000000..fc1d620 --- /dev/null +++ b/rule/identical_switch_branches.go @@ -0,0 +1,94 @@ +package rule + +import ( + "fmt" + "go/ast" + "go/token" + + "github.com/mgechev/revive/internal/astutils" + "github.com/mgechev/revive/lint" +) + +// IdenticalSwitchBranchesRule warns on identical switch branches. +type IdenticalSwitchBranchesRule struct{} + +// Apply applies the rule to given file. +func (*IdenticalSwitchBranchesRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { + var failures []lint.Failure + + onFailure := func(failure lint.Failure) { + failures = append(failures, failure) + } + + getStmtLine := func(s ast.Stmt) int { + return file.ToPosition(s.Pos()).Line + } + + w := &lintIdenticalSwitchBranches{getStmtLine: getStmtLine, 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 (*IdenticalSwitchBranchesRule) Name() string { + return "identical-switch-branches" +} + +type lintIdenticalSwitchBranches struct { + getStmtLine func(ast.Stmt) int + onFailure func(lint.Failure) +} + +func (w *lintIdenticalSwitchBranches) Visit(node ast.Node) ast.Visitor { + switchStmt, ok := node.(*ast.SwitchStmt) + if !ok { + return w + } + + if switchStmt.Tag == nil { + return w // do not lint untagged switches (order of case evaluation might be important) + } + + doesFallthrough := func(stmts []ast.Stmt) bool { + if len(stmts) == 0 { + return false + } + + ft, ok := stmts[len(stmts)-1].(*ast.BranchStmt) + return ok && ft.Tok == token.FALLTHROUGH + } + + hashes := map[string]int{} // map hash(branch code) -> branch line + for _, cc := range switchStmt.Body.List { + caseClause := cc.(*ast.CaseClause) + if doesFallthrough(caseClause.Body) { + continue // skip fallthrough branches + } + branch := &ast.BlockStmt{ + List: caseClause.Body, + } + hash := astutils.NodeHash(branch) + branchLine := w.getStmtLine(caseClause) + if matchLine, ok := hashes[hash]; ok { + w.onFailure(lint.Failure{ + Confidence: 1.0, + Node: node, + Category: lint.FailureCategoryLogic, + Failure: fmt.Sprintf(`"switch" with identical branches (lines %d and %d)`, matchLine, branchLine), + }) + } + + hashes[hash] = branchLine + ast.Walk(w, branch) + } + + return nil // switch branches already analyzed +} diff --git a/test/identical_ifelseif_branches_test.go b/test/identical_ifelseif_branches_test.go new file mode 100644 index 0000000..82d2f0a --- /dev/null +++ b/test/identical_ifelseif_branches_test.go @@ -0,0 +1,11 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/rule" +) + +func TestIdenticalIfElseIfBranches(t *testing.T) { + testRule(t, "identical_ifelseif_branches", &rule.IdenticalIfElseIfBranchesRule{}) +} diff --git a/test/identical_switch_branches_test.go b/test/identical_switch_branches_test.go new file mode 100644 index 0000000..ab52ed2 --- /dev/null +++ b/test/identical_switch_branches_test.go @@ -0,0 +1,11 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/rule" +) + +func TestIdenticalSwitchBranches(t *testing.T) { + testRule(t, "identical_switch_branches", &rule.IdenticalSwitchBranchesRule{}) +} diff --git a/testdata/identical_branches.go b/testdata/identical_branches.go index e21f2df..8b850da 100644 --- a/testdata/identical_branches.go +++ b/testdata/identical_branches.go @@ -37,149 +37,4 @@ func identicalBranches() { } else { println("else") } - - if true { // MATCH /"if...else if" chain with identical branches (lines 41 and 49)/ - print("something") - } else if true { - print("something else") - } else if true { - print("other thing") - } else if false { - println() - } else { - print("something") - } - - if true { // MATCH /"if...else if" chain with identical branches (lines 53 and 59)/ - print("something") - } else if true { - print("something else") - } else if true { - print("other thing") - } else if false { - print("something") - } else { - println() - } - - if true { - print("something") - } else if true { - print("something else") - if true { // MATCH /"if...else if" chain with identical branches (lines 69 and 71)/ - print("something") - } else if false { - print("something") - } else { - println() - } - } - - // Should not warn because even if branches are identical, the err variable is not - if err := something(); err != nil { - println(err) - } else if err := somethingElse(); err != nil { - println(err) - } - - // Multiple identical pair of branches - if a { - foo() - } else if b { - bar() - } else if c { - foo() - } else if d { - bar() - } - // MATCH:86 /"if...else if" chain with identical branches (lines 86 and 90)/ - // MATCH:86 /"if...else if" chain with identical branches (lines 88 and 92)/ - - if createFile() { // json:{"MATCH": "\"if...else if\" chain with identical branches (lines 98 and 102)","Confidence": 0.8} - doSomething() - } else if !delete() { - return new("cannot delete file") - } else if createFile() { - doSomething() - } else { - return new("file error") - } - - // Test confidence is reset - if a { // json:{"MATCH": "\"if...else if\" chain with identical branches (lines 109 and 111)","Confidence": 1} - foo() - } else if b { - foo() - } else { - bar() - } - - switch a { // MATCH /"switch" with identical branches (lines 119 and 123)/ - // expected values - case 1: - foo() - case 2: - bar() - case 3: - foo() - default: - return newError("blah") - } - - // MATCH:131 /"switch" with identical branches (lines 133 and 137)/ - // MATCH:131 /"switch" with identical branches (lines 135 and 139)/ - switch a { - // expected values - case 1: - foo() - case 2: - bar() - case 3: - foo() - default: - bar() - } - - switch a { // MATCH /"switch" with identical branches (lines 145 and 147)/ - // expected values - case 1: - foo() - case 3: - foo() - default: - if true { // MATCH /"if...else if" chain with identical branches (lines 150 and 152)/ - something() - } else if true { - something() - } else { - if true { // MATCH /both branches of the if are identical/ - print("identical") - } else { - print("identical") - } - } - } - - // Skip untagged switch - switch { - case a > b: - foo() - default: - foo() - } - - // Do not warn on fallthrough - switch a { - case 1: - foo() - fallthrough - case 2: - fallthrough - case 3: - foo() - case 4: - fallthrough - default: - bar() - } } diff --git a/testdata/identical_ifelseif_branches.go b/testdata/identical_ifelseif_branches.go new file mode 100644 index 0000000..bf19c21 --- /dev/null +++ b/testdata/identical_ifelseif_branches.go @@ -0,0 +1,80 @@ +package fixtures + +func identicalIfElseIfBranches() { + + if true { // MATCH /"if...else if" chain with identical branches (lines 5 and 13)/ + print("something") + } else if true { + print("something else") + } else if true { + print("other thing") + } else if false { + println() + } else { + print("something") + } + + if true { // MATCH /"if...else if" chain with identical branches (lines 17 and 23)/ + print("something") + } else if true { + print("something else") + } else if true { + print("other thing") + } else if false { + print("something") + } else { + println() + } + + if true { + print("something") + } else if true { + print("something else") + if true { // MATCH /"if...else if" chain with identical branches (lines 33 and 35)/ + print("something") + } else if false { + print("something") + } else { + println() + } + } + + // Should not warn because even if branches are identical, the err variable is not + if err := something(); err != nil { + println(err) + } else if err := somethingElse(); err != nil { + println(err) + } + + // Multiple identical pair of branches + if a { + foo() + } else if b { + bar() + } else if c { + foo() + } else if d { + bar() + } + // MATCH:50 /"if...else if" chain with identical branches (lines 50 and 54)/ + // MATCH:50 /"if...else if" chain with identical branches (lines 52 and 56)/ + + if createFile() { // json:{"MATCH": "\"if...else if\" chain with identical branches (lines 62 and 66)","Confidence": 0.8} + doSomething() + } else if !delete() { + return new("cannot delete file") + } else if createFile() { + doSomething() + } else { + return new("file error") + } + + // Test confidence is reset + if a { // json:{"MATCH": "\"if...else if\" chain with identical branches (lines 73 and 75)","Confidence": 1} + foo() + } else if b { + foo() + } else { + bar() + } +} diff --git a/testdata/identical_switch_branches.go b/testdata/identical_switch_branches.go new file mode 100644 index 0000000..9bf32e7 --- /dev/null +++ b/testdata/identical_switch_branches.go @@ -0,0 +1,77 @@ +package fixtures + +func identicalSwitchBranches() { + switch a { // MATCH /"switch" with identical branches (lines 6 and 10)/ + // expected values + case 1: + foo() + case 2: + bar() + case 3: + foo() + default: + return newError("blah") + } + + // MATCH:18 /"switch" with identical branches (lines 20 and 24)/ + // MATCH:18 /"switch" with identical branches (lines 22 and 26)/ + switch a { + // expected values + case 1: + foo() + case 2: + bar() + case 3: + foo() + default: + bar() + } + + switch a { // MATCH /"switch" with identical branches (lines 32 and 34)/ + // expected values + case 1: + foo() + case 3: + foo() + default: + + } + + // Skip untagged switch + switch { + case a > b: + foo() + default: + foo() + } + + // Do not warn on fallthrough + + switch a { + case 1: + foo() + fallthrough + case 2: + fallthrough + case 3: + foo() + case 4: + fallthrough + default: + bar() + } + + // skip type switch + switch v := value.(type) { + case int: + println("dup", v) + case string: + println("dup", v) + case bool: + println("dup", v) + case float64: + println("dup", v) + default: + println("dup", v) + } +}