From 4bb48df5d203f17b5375a538be43af1de8c49e17 Mon Sep 17 00:00:00 2001 From: Miles Delahunty <4904544+mdelah@users.noreply.github.com> Date: Wed, 17 May 2023 21:51:35 +1000 Subject: [PATCH 01/34] 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 | 68 +++++++++++ internal/ifelse/branch_kind.go | 98 ++++++++++++++++ internal/ifelse/chain.go | 9 ++ internal/ifelse/doc.go | 6 + internal/ifelse/func.go | 51 +++++++++ internal/ifelse/rule.go | 92 +++++++++++++++ internal/ifelse/target.go | 25 ++++ rule/early-return.go | 164 +++------------------------ rule/indent-error-flow.go | 76 ++++--------- rule/superfluous-else.go | 107 +++-------------- testdata/early-return.go | 8 ++ testdata/golint/indent-error-flow.go | 9 ++ testdata/superfluous-else.go | 108 +++++++++++------- 13 files changed, 483 insertions(+), 338 deletions(-) create mode 100644 internal/ifelse/branch.go create mode 100644 internal/ifelse/branch_kind.go create mode 100644 internal/ifelse/chain.go create mode 100644 internal/ifelse/doc.go create mode 100644 internal/ifelse/func.go create mode 100644 internal/ifelse/rule.go create mode 100644 internal/ifelse/target.go diff --git a/internal/ifelse/branch.go b/internal/ifelse/branch.go new file mode 100644 index 000000000..0d51a0a4e --- /dev/null +++ b/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() + } +} diff --git a/internal/ifelse/branch_kind.go b/internal/ifelse/branch_kind.go new file mode 100644 index 000000000..7df276fd8 --- /dev/null +++ b/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") + } +} diff --git a/internal/ifelse/chain.go b/internal/ifelse/chain.go new file mode 100644 index 000000000..d6afebff6 --- /dev/null +++ b/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? +} diff --git a/internal/ifelse/doc.go b/internal/ifelse/doc.go new file mode 100644 index 000000000..0aa2c9817 --- /dev/null +++ b/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 diff --git a/internal/ifelse/func.go b/internal/ifelse/func.go new file mode 100644 index 000000000..7ba351918 --- /dev/null +++ b/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 + } +} diff --git a/internal/ifelse/rule.go b/internal/ifelse/rule.go new file mode 100644 index 000000000..ec0db01e6 --- /dev/null +++ b/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") + } +} diff --git a/internal/ifelse/target.go b/internal/ifelse/target.go new file mode 100644 index 000000000..81ff1c303 --- /dev/null +++ b/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") + } +} diff --git a/rule/early-return.go b/rule/early-return.go index ed0fcfae4..f5f8ff55d 100644 --- a/rule/early-return.go +++ b/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 { - var failures []lint.Failure - - onFailure := func(failure lint.Failure) { - failures = append(failures, failure) - } - - w := lintEarlyReturnRule{onFailure: onFailure} - ast.Walk(w, file.AST) - return failures +func (e *EarlyReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { + return ifelse.Apply(e, file.AST, ifelse.TargetIf) } // Name returns the rule name. @@ -30,147 +21,26 @@ func (*EarlyReturnRule) Name() string { return "early-return" } -type lintEarlyReturnRule struct { - onFailure func(lint.Failure) -} - -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 +// CheckIfElse evaluates the rule against an ifelse.Chain. +func (e *EarlyReturnRule) CheckIfElse(chain ifelse.Chain) (failMsg string) { + if !chain.Else.Deviates() { + // this rule only applies if the else-block deviates control flow return } - if as, ok := ifStmt.Init.(*ast.AssignStmt); ok && as.Tok == token.DEFINE { - hasIfInitializer = true - } - bodyFlow := w.branchFlow(ifStmt.Body) - - switch elseBlock := ifStmt.Else.(type) { - case *ast.IfStmt: - if bodyFlow.canFlowIntoNext() { - 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 - } + if chain.HasPriorNonDeviating && !chain.If.IsEmpty() { + // if we de-indent this block then a previous branch + // might flow into it, affecting program behaviour + return } - 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 + if chain.If.Deviates() { + // avoid overlapping with superfluous-else + return } -} -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) } diff --git a/rule/indent-error-flow.go b/rule/indent-error-flow.go index e455801c4..4d62404e0 100644 --- a/rule/indent-error-flow.go +++ b/rule/indent-error-flow.go @@ -1,9 +1,7 @@ package rule import ( - "go/ast" - "go/token" - + "github.com/mgechev/revive/internal/ifelse" "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 { - var failures []lint.Failure - - onFailure := func(failure lint.Failure) { - failures = append(failures, failure) - } - - w := lintElse{make(map[*ast.IfStmt]bool), onFailure} - ast.Walk(w, file.AST) - return failures +func (e *IndentErrorFlowRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { + return ifelse.Apply(e, file.AST, ifelse.TargetElse) } // Name returns the rule name. @@ -28,51 +18,23 @@ func (*IndentErrorFlowRule) Name() string { return "indent-error-flow" } -type lintElse struct { - ignore map[*ast.IfStmt]bool - onFailure func(lint.Failure) -} - -func (w lintElse) Visit(node ast.Node) ast.Visitor { - ifStmt, ok := node.(*ast.IfStmt) - if !ok || ifStmt.Else == nil { - return w - } - if w.ignore[ifStmt] { - if elseif, ok := ifStmt.Else.(*ast.IfStmt); ok { - w.ignore[elseif] = true - } - return w +// CheckIfElse evaluates the rule against an ifelse.Chain. +func (e *IndentErrorFlowRule) CheckIfElse(chain ifelse.Chain) (failMsg string) { + if !chain.If.Deviates() { + // this rule only applies if the if-block deviates control flow + return } - 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 - } + + if chain.HasPriorNonDeviating { + // if we de-indent the "else" block then a previous branch + // might flow into it, affecting program behaviour + return } - 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, - }) + + if !chain.If.Returns() { + // avoid overlapping with superfluous-else + return } - return w + + return "if block ends with a return statement, so drop this else and outdent its block" } diff --git a/rule/superfluous-else.go b/rule/superfluous-else.go index a9e4380c9..9dce59a46 100644 --- a/rule/superfluous-else.go +++ b/rule/superfluous-else.go @@ -2,9 +2,7 @@ package rule import ( "fmt" - "go/ast" - "go/token" - + "github.com/mgechev/revive/internal/ifelse" "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 { - var failures []lint.Failure - 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 +func (e *SuperfluousElseRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { + return ifelse.Apply(e, file.AST, ifelse.TargetElse) } // Name returns the rule name. @@ -40,75 +19,23 @@ func (*SuperfluousElseRule) Name() string { return "superfluous-else" } -type lintSuperfluousElse struct { - ignore map[*ast.IfStmt]bool - onFailure func(lint.Failure) - branchingFunctions map[string]map[string]bool -} - -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] { - if elseif, ok := ifStmt.Else.(*ast.IfStmt); ok { - w.ignore[elseif] = true - } - 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)" +// CheckIfElse evaluates the rule against an ifelse.Chain. +func (e *SuperfluousElseRule) CheckIfElse(chain ifelse.Chain) (failMsg string) { + if !chain.If.Deviates() { + // this rule only applies if the if-block deviates control flow + return } - 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))) - } - } - } - } + if chain.HasPriorNonDeviating { + // if we de-indent the "else" block then a previous branch + // might flow into it, affecting program behaviour + return } - return w -} - -func newFailure(node ast.Node, msg string) lint.Failure { - return lint.Failure{ - Confidence: 1, - Node: node, - Category: "indent", - Failure: msg, + if chain.If.Returns() { + // avoid overlapping with indent-error-flow + return } + + return fmt.Sprintf("if block ends with %v, so drop this else and outdent its block", chain.If.LongString()) } diff --git a/testdata/early-return.go b/testdata/early-return.go index 212b6c9d6..15475b0cc 100644 --- a/testdata/early-return.go +++ b/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) + } } diff --git a/testdata/golint/indent-error-flow.go b/testdata/golint/indent-error-flow.go index 5d2fa1934..64ea06ebc 100644 --- a/testdata/golint/indent-error-flow.go +++ b/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" + } +} diff --git a/testdata/superfluous-else.go b/testdata/superfluous-else.go index bca11e512..aaade1b41 100644 --- a/testdata/superfluous-else.go +++ b/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() { - a := 1 - log.Fatal("x") - } else { // MATCH /if block ends with call to log.Fatal function, so drop this else and outdent its block/ - log.Printf("non-positive") - } + if f() { + a := 1 + log.Fatal("x") + } else { // MATCH /if block ends with call to log.Fatal function, so drop this else and outdent its block/ + log.Printf("non-positive") + } return "ok" } func fatal2() string { - if f() { - a := 1 - log.Fatalf("x") - } else { // MATCH /if block ends with call to log.Fatalf function, so drop this else and outdent its block/ - log.Printf("non-positive") - } + if f() { + a := 1 + log.Fatalf("x") + } else { // MATCH /if block ends with call to log.Fatalf function, so drop this else and outdent its block/ + log.Printf("non-positive") + } return "ok" } func fatal3() string { - if f() { - a := 1 - log.Fatalln("x") - } else { // MATCH /if block ends with call to log.Fatalln function, so drop this else and outdent its block/ - log.Printf("non-positive") - } + if f() { + a := 1 + log.Fatalln("x") + } else { // MATCH /if block ends with call to log.Fatalln function, so drop this else and outdent its block/ + log.Printf("non-positive") + } return "ok" } func exit1() string { - if f() { - a := 1 - os.Exit(2) - } else { // MATCH /if block ends with call to os.Exit function, so drop this else and outdent its block/ - log.Printf("non-positive") - } + if f() { + a := 1 + os.Exit(2) + } else { // MATCH /if block ends with call to os.Exit function, so drop this else and outdent its block/ + log.Printf("non-positive") + } return "ok" } func Panic1() string { - if f() { - a := 1 - log.Panic(2) - } else { // MATCH /if block ends with call to log.Panic function, so drop this else and outdent its block/ - log.Printf("non-positive") - } + if f() { + a := 1 + log.Panic(2) + } else { // MATCH /if block ends with call to log.Panic function, so drop this else and outdent its block/ + log.Printf("non-positive") + } return "ok" } func Panic2() string { - if f() { - a := 1 - log.Panicf(2) - } else { // MATCH /if block ends with call to log.Panicf function, so drop this else and outdent its block/ - log.Printf("non-positive") - } + if f() { + a := 1 + log.Panicf(2) + } else { // MATCH /if block ends with call to log.Panicf function, so drop this else and outdent its block/ + log.Printf("non-positive") + } return "ok" } func Panic3() string { - if f() { - a := 1 - log.Panicln(2) - } else { // MATCH /if block ends with call to log.Panicln function, so drop this else and outdent its block/ - log.Printf("non-positive") - } + if f() { + a := 1 + log.Panicln(2) + } else { // MATCH /if block ends with call to log.Panicln function, so drop this else and outdent its block/ + 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 } -} \ No newline at end of file +} + +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) + } +} From 9117f8e621b203792adcf31337d41bce96138622 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 19 May 2023 14:04:54 -0700 Subject: [PATCH 02/34] fix(deps): update github.com/chavacava/garif digest to 4752330 (#831) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index a0b349db6..38a0678c0 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.19 require ( github.com/BurntSushi/toml v1.2.1 - github.com/chavacava/garif v0.0.0-20230227094218-b8c73b2037b8 + github.com/chavacava/garif v0.0.0-20230519080132-4752330f72df github.com/fatih/color v1.15.0 github.com/fatih/structtag v1.2.0 github.com/mgechev/dots v0.0.0-20210922191527-e955255bf517 diff --git a/go.sum b/go.sum index 72eaa9bbd..86c5ec8ff 100644 --- a/go.sum +++ b/go.sum @@ -8,6 +8,8 @@ github.com/chavacava/garif v0.0.0-20221024190013-b3ef35877348 h1:cy5GCEZLUCshCGC github.com/chavacava/garif v0.0.0-20221024190013-b3ef35877348/go.mod h1:f/miWtG3SSuTxKsNK3o58H1xl+XV6ZIfbC6p7lPPB8U= github.com/chavacava/garif v0.0.0-20230227094218-b8c73b2037b8 h1:W9o46d2kbNL06lq7UNDPV0zYLzkrde/bjIqO02eoll0= github.com/chavacava/garif v0.0.0-20230227094218-b8c73b2037b8/go.mod h1:gakxgyXaaPkxvLw1XQxNGK4I37ys9iBRzNUx/B7pUCo= +github.com/chavacava/garif v0.0.0-20230519080132-4752330f72df h1:1uGdlpQT0irrGcFFOUuitqSCE6BjttfHd+k3k9OQ0fg= +github.com/chavacava/garif v0.0.0-20230519080132-4752330f72df/go.mod h1:cFP7fAFavJ2DrYBmZYBETNKwSTFJiOIirm5N4/PqY/I= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -49,6 +51,7 @@ github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PK github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= From 2a1838f501292f3cff530bc0a10d232bce441725 Mon Sep 17 00:00:00 2001 From: chavacava Date: Sat, 20 May 2023 14:44:34 +0200 Subject: [PATCH 03/34] adds default config to enable all rules work out of the box (#830) * adds default config to enable all rules out of the box --- README.md | 20 ++++++++++---------- rule/argument-limit.go | 9 +++++++-- rule/banned-characters.go | 4 ++-- rule/cognitive-complexity.go | 10 ++++++++-- rule/cyclomatic.go | 9 +++++++-- rule/file-header.go | 13 ++++++++++--- rule/function-length.go | 9 ++++++++- rule/function-result-limit.go | 10 +++++++--- rule/line-length-limit.go | 9 +++++++-- rule/max-public-structs.go | 9 ++++++++- 10 files changed, 74 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 413b06cd7..a4e852f02 100644 --- a/README.md +++ b/README.md @@ -449,10 +449,10 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`range`](./RULES_DESCRIPTIONS.md#range) | n/a | Prevents redundant variables when iterating over a collection. | yes | no | | [`receiver-naming`](./RULES_DESCRIPTIONS.md#receiver-naming) | n/a | Conventions around the naming of receivers. | yes | no | | [`indent-error-flow`](./RULES_DESCRIPTIONS.md#indent-error-flow) | n/a | Prevents redundant else statements. | yes | no | -| [`argument-limit`](./RULES_DESCRIPTIONS.md#argument-limit) | int | Specifies the maximum number of arguments a function can receive | no | no | -| [`cyclomatic`](./RULES_DESCRIPTIONS.md#cyclomatic) | int | Sets restriction for maximum Cyclomatic complexity. | no | no | -| [`max-public-structs`](./RULES_DESCRIPTIONS.md#max-public-structs) | int | The maximum number of public structs in a file. | no | no | -| [`file-header`](./RULES_DESCRIPTIONS.md#file-header) | string | Header which each file should have. | no | no | +| [`argument-limit`](./RULES_DESCRIPTIONS.md#argument-limit) | int (defaults to 8) | Specifies the maximum number of arguments a function can receive | no | no | +| [`cyclomatic`](./RULES_DESCRIPTIONS.md#cyclomatic) | int (defaults to 10) | Sets restriction for maximum Cyclomatic complexity. | no | no | +| [`max-public-structs`](./RULES_DESCRIPTIONS.md#max-public-structs) | int (defaults to 5) | The maximum number of public structs in a file. | no | no | +| [`file-header`](./RULES_DESCRIPTIONS.md#file-header) | string (defaults to none)| Header which each file should have. | no | no | | [`empty-block`](./RULES_DESCRIPTIONS.md#empty-block) | n/a | Warns on empty code blocks | no | yes | | [`superfluous-else`](./RULES_DESCRIPTIONS.md#superfluous-else) | n/a | Prevents redundant else statements (extends [`indent-error-flow`](./RULES_DESCRIPTIONS.md#indent-error-flow)) | no | no | | [`confusing-naming`](./RULES_DESCRIPTIONS.md#confusing-naming) | n/a | Warns on methods with names that differ only by capitalization | no | no | @@ -465,26 +465,26 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`add-constant`](./RULES_DESCRIPTIONS.md#add-constant) | map | Suggests using constant for magic numbers and string literals | no | no | | [`flag-parameter`](./RULES_DESCRIPTIONS.md#flag-parameter) | n/a | Warns on boolean parameters that create a control coupling | no | no | | [`unnecessary-stmt`](./RULES_DESCRIPTIONS.md#unnecessary-stmt) | n/a | Suggests removing or simplifying unnecessary statements | no | no | -| [`struct-tag`](./RULES_DESCRIPTIONS.md#struct-tag) | []string | Checks common struct tags like `json`,`xml`,`yaml` | no | no | +| [`struct-tag`](./RULES_DESCRIPTIONS.md#struct-tag) | []string | Checks common struct tags like `json`, `xml`, `yaml` | no | no | | [`modifies-value-receiver`](./RULES_DESCRIPTIONS.md#modifies-value-receiver) | n/a | Warns on assignments to value-passed method receivers | no | yes | | [`constant-logical-expr`](./RULES_DESCRIPTIONS.md#constant-logical-expr) | n/a | Warns on constant logical expressions | no | no | | [`bool-literal-in-expr`](./RULES_DESCRIPTIONS.md#bool-literal-in-expr)| n/a | Suggests removing Boolean literals from logic expressions | no | no | | [`redefines-builtin-id`](./RULES_DESCRIPTIONS.md#redefines-builtin-id)| n/a | Warns on redefinitions of builtin identifiers | no | no | -| [`function-result-limit`](./RULES_DESCRIPTIONS.md#function-result-limit) | int | Specifies the maximum number of results a function can return | no | no | +| [`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 | | [`imports-blacklist`](./RULES_DESCRIPTIONS.md#imports-blacklist) | []string | Disallows importing the specified packages | no | no | | [`range-val-in-closure`](./RULES_DESCRIPTIONS.md#range-val-in-closure)| n/a | Warns if range value is used in a closure dispatched as goroutine| no | no | | [`range-val-address`](./RULES_DESCRIPTIONS.md#range-val-address)| n/a | Warns if address of range value is used dangerously | no | yes | | [`waitgroup-by-value`](./RULES_DESCRIPTIONS.md#waitgroup-by-value) | n/a | Warns on functions taking sync.WaitGroup as a by-value parameter | no | no | | [`atomic`](./RULES_DESCRIPTIONS.md#atomic) | n/a | Check for common mistaken usages of the `sync/atomic` package | no | no | | [`empty-lines`](./RULES_DESCRIPTIONS.md#empty-lines) | n/a | Warns when there are heading or trailing newlines in a block | no | no | -| [`line-length-limit`](./RULES_DESCRIPTIONS.md#line-length-limit) | int | Specifies the maximum number of characters in a line | no | no | +| [`line-length-limit`](./RULES_DESCRIPTIONS.md#line-length-limit) | int (defaults to 80) | Specifies the maximum number of characters in a line | no | no | | [`call-to-gc`](./RULES_DESCRIPTIONS.md#call-to-gc) | n/a | Warns on explicit call to the garbage collector | no | no | | [`duplicated-imports`](./RULES_DESCRIPTIONS.md#duplicated-imports) | n/a | Looks for packages that are imported two or more times | no | no | | [`import-shadowing`](./RULES_DESCRIPTIONS.md#import-shadowing) | n/a | Spots identifiers that shadow an import | no | no | | [`bare-return`](./RULES_DESCRIPTIONS.md#bare-return) | n/a | Warns on bare returns | no | no | | [`unused-receiver`](./RULES_DESCRIPTIONS.md#unused-receiver) | n/a | Suggests to rename or remove unused method receivers | no | no | | [`unhandled-error`](./RULES_DESCRIPTIONS.md#unhandled-error) | []string | Warns on unhandled errors returned by function calls | no | yes | -| [`cognitive-complexity`](./RULES_DESCRIPTIONS.md#cognitive-complexity) | int | Sets restriction for maximum Cognitive complexity. | no | no | +| [`cognitive-complexity`](./RULES_DESCRIPTIONS.md#cognitive-complexity) | int (defaults to 7) | 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 | | [`string-format`](./RULES_DESCRIPTIONS.md#string-format) | map | Warns on specific string literals that fail one or more user-configured regular expressions | no | no | | [`early-return`](./RULES_DESCRIPTIONS.md#early-return) | n/a | Spots if-then-else statements where the predicate may be inverted to reduce nesting | no | no | @@ -492,10 +492,10 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`identical-branches`](./RULES_DESCRIPTIONS.md#identical-branches) | n/a | Spots if-then-else statements with identical `then` and `else` branches | no | no | | [`defer`](./RULES_DESCRIPTIONS.md#defer) | map | Warns on some [defer gotchas](https://blog.learngoprogramming.com/5-gotchas-of-defer-in-go-golang-part-iii-36a1ab3d6ef1) | no | no | | [`unexported-naming`](./RULES_DESCRIPTIONS.md#unexported-naming) | n/a | Warns on wrongly named un-exported symbols | no | no | -| [`function-length`](./RULES_DESCRIPTIONS.md#function-length) | n/a | Warns on functions exceeding the statements or lines max | no | no | +| [`function-length`](./RULES_DESCRIPTIONS.md#function-length) | int, int (defaults to 50 statements, 75 lines) | Warns on functions exceeding the statements or lines max | no | no | | [`nested-structs`](./RULES_DESCRIPTIONS.md#nested-structs) | n/a | Warns on structs within structs | no | no | | [`useless-break`](./RULES_DESCRIPTIONS.md#useless-break) | n/a | Warns on useless `break` statements in case clauses | no | no | -| [`banned-characters`](./RULES_DESCRIPTIONS.md#banned-characters) | n/a | Checks banned characters in identifiers | no | no | +| [`banned-characters`](./RULES_DESCRIPTIONS.md#banned-characters) | []string (defaults to []string{}) | Checks banned characters in identifiers | no | no | | [`optimize-operands-order`](./RULES_DESCRIPTIONS.md#optimize-operands-order) | n/a | Checks inefficient conditional expressions | no | no | | [`use-any`](./RULES_DESCRIPTIONS.md#use-any) | n/a | Proposes to replace `interface{}` with its alias `any` | no | no | | [`datarace`](./RULES_DESCRIPTIONS.md#datarace) | n/a | Spots potential dataraces | no | no | diff --git a/rule/argument-limit.go b/rule/argument-limit.go index 8042da15e..8120288fd 100644 --- a/rule/argument-limit.go +++ b/rule/argument-limit.go @@ -14,10 +14,16 @@ type ArgumentsLimitRule struct { sync.Mutex } +const defaultArgumentsLimit = 8 + func (r *ArgumentsLimitRule) configure(arguments lint.Arguments) { r.Lock() + defer r.Unlock() if r.total == 0 { - checkNumberOfArguments(1, arguments, r.Name()) + if len(arguments) < 1 { + r.total = defaultArgumentsLimit + return + } total, ok := arguments[0].(int64) // Alt. non panicking version if !ok { @@ -25,7 +31,6 @@ func (r *ArgumentsLimitRule) configure(arguments lint.Arguments) { } r.total = int(total) } - r.Unlock() } // Apply applies the rule to given file. diff --git a/rule/banned-characters.go b/rule/banned-characters.go index 76fa2235a..12997bae1 100644 --- a/rule/banned-characters.go +++ b/rule/banned-characters.go @@ -19,11 +19,11 @@ const bannedCharsRuleName = "banned-characters" func (r *BannedCharsRule) configure(arguments lint.Arguments) { r.Lock() - if r.bannedCharList == nil { + defer r.Unlock() + if r.bannedCharList == nil && len(arguments) > 0 { checkNumberOfArguments(1, arguments, bannedCharsRuleName) r.bannedCharList = r.getBannedCharsList(arguments) } - r.Unlock() } // Apply applied the rule to the given file. diff --git a/rule/cognitive-complexity.go b/rule/cognitive-complexity.go index a9c11a7d0..1973faef8 100644 --- a/rule/cognitive-complexity.go +++ b/rule/cognitive-complexity.go @@ -16,10 +16,17 @@ type CognitiveComplexityRule struct { sync.Mutex } +const defaultMaxCognitiveComplexity = 7 + func (r *CognitiveComplexityRule) configure(arguments lint.Arguments) { r.Lock() + defer r.Unlock() if r.maxComplexity == 0 { - checkNumberOfArguments(1, arguments, r.Name()) + + if len(arguments) < 1 { + r.maxComplexity = defaultMaxCognitiveComplexity + return + } complexity, ok := arguments[0].(int64) if !ok { @@ -27,7 +34,6 @@ func (r *CognitiveComplexityRule) configure(arguments lint.Arguments) { } r.maxComplexity = int(complexity) } - r.Unlock() } // Apply applies the rule to given file. diff --git a/rule/cyclomatic.go b/rule/cyclomatic.go index afd41818b..9f6d50043 100644 --- a/rule/cyclomatic.go +++ b/rule/cyclomatic.go @@ -17,10 +17,16 @@ type CyclomaticRule struct { sync.Mutex } +const defaultMaxCyclomaticComplexity = 10 + func (r *CyclomaticRule) configure(arguments lint.Arguments) { r.Lock() + defer r.Unlock() if r.maxComplexity == 0 { - checkNumberOfArguments(1, arguments, r.Name()) + if len(arguments) < 1 { + r.maxComplexity = defaultMaxCyclomaticComplexity + return + } complexity, ok := arguments[0].(int64) // Alt. non panicking version if !ok { @@ -28,7 +34,6 @@ func (r *CyclomaticRule) configure(arguments lint.Arguments) { } r.maxComplexity = int(complexity) } - r.Unlock() } // Apply applies the rule to given file. diff --git a/rule/file-header.go b/rule/file-header.go index 76f548f51..a7d69ff2b 100644 --- a/rule/file-header.go +++ b/rule/file-header.go @@ -21,21 +21,28 @@ var ( func (r *FileHeaderRule) configure(arguments lint.Arguments) { r.Lock() + defer r.Unlock() if r.header == "" { - checkNumberOfArguments(1, arguments, r.Name()) + if len(arguments) < 1 { + return + } + var ok bool r.header, ok = arguments[0].(string) if !ok { - panic(fmt.Sprintf("invalid argument for \"file-header\" rule: first argument should be a string, got %T", arguments[0])) + panic(fmt.Sprintf("invalid argument for \"file-header\" rule: argument should be a string, got %T", arguments[0])) } } - r.Unlock() } // Apply applies the rule to given file. func (r *FileHeaderRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { r.configure(arguments) + if r.header == "" { + return nil + } + failure := []lint.Failure{ { Node: file.AST, diff --git a/rule/function-length.go b/rule/function-length.go index d600d7a2a..341a6a027 100644 --- a/rule/function-length.go +++ b/rule/function-length.go @@ -19,13 +19,13 @@ type FunctionLength struct { func (r *FunctionLength) configure(arguments lint.Arguments) { r.Lock() + r.Unlock() if !r.configured { maxStmt, maxLines := r.parseArguments(arguments) r.maxStmt = int(maxStmt) r.maxLines = int(maxLines) r.configured = true } - r.Unlock() } // Apply applies the rule to given file. @@ -53,7 +53,14 @@ func (*FunctionLength) Name() string { return "function-length" } +const defaultFuncStmtsLimit = 50 +const defaultFuncLinesLimit = 75 + func (*FunctionLength) parseArguments(arguments lint.Arguments) (maxStmt, maxLines int64) { + if len(arguments) == 0 { + return defaultFuncStmtsLimit, defaultFuncLinesLimit + } + if len(arguments) != 2 { panic(fmt.Sprintf(`invalid configuration for "function-length" rule, expected 2 arguments but got %d`, len(arguments))) } diff --git a/rule/function-result-limit.go b/rule/function-result-limit.go index 5d2b87316..6a0748011 100644 --- a/rule/function-result-limit.go +++ b/rule/function-result-limit.go @@ -14,11 +14,16 @@ type FunctionResultsLimitRule struct { sync.Mutex } +const defaultResultsLimit = 3 + func (r *FunctionResultsLimitRule) configure(arguments lint.Arguments) { r.Lock() + defer r.Unlock() if r.max == 0 { - checkNumberOfArguments(1, arguments, r.Name()) - + if len(arguments) < 1 { + r.max = defaultResultsLimit + return + } max, ok := arguments[0].(int64) // Alt. non panicking version if !ok { panic(fmt.Sprintf(`invalid value passed as return results number to the "function-result-limit" rule; need int64 but got %T`, arguments[0])) @@ -28,7 +33,6 @@ func (r *FunctionResultsLimitRule) configure(arguments lint.Arguments) { } r.max = int(max) } - r.Unlock() } // Apply applies the rule to given file. diff --git a/rule/line-length-limit.go b/rule/line-length-limit.go index 9e512c1c2..1a414f691 100644 --- a/rule/line-length-limit.go +++ b/rule/line-length-limit.go @@ -18,10 +18,16 @@ type LineLengthLimitRule struct { sync.Mutex } +const defaultLineLengthLimit = 80 + func (r *LineLengthLimitRule) configure(arguments lint.Arguments) { r.Lock() + defer r.Unlock() if r.max == 0 { - checkNumberOfArguments(1, arguments, r.Name()) + if len(arguments) < 1 { + r.max = defaultLineLengthLimit + return + } max, ok := arguments[0].(int64) // Alt. non panicking version if !ok || max < 0 { @@ -30,7 +36,6 @@ func (r *LineLengthLimitRule) configure(arguments lint.Arguments) { r.max = int(max) } - r.Unlock() } // Apply applies the rule to given file. diff --git a/rule/max-public-structs.go b/rule/max-public-structs.go index e39f49c69..25be3e676 100644 --- a/rule/max-public-structs.go +++ b/rule/max-public-structs.go @@ -14,9 +14,17 @@ type MaxPublicStructsRule struct { sync.Mutex } +const defaultMaxPublicStructs = 5 + func (r *MaxPublicStructsRule) configure(arguments lint.Arguments) { r.Lock() + defer r.Unlock() if r.max < 1 { + if len(arguments) < 1 { + r.max = defaultMaxPublicStructs + return + } + checkNumberOfArguments(1, arguments, r.Name()) max, ok := arguments[0].(int64) // Alt. non panicking version @@ -25,7 +33,6 @@ func (r *MaxPublicStructsRule) configure(arguments lint.Arguments) { } r.max = max } - r.Unlock() } // Apply applies the rule to given file. From ae07914dc4fc1af89bed44649db1627cb7b0ccb4 Mon Sep 17 00:00:00 2001 From: Miles Delahunty <4904544+mdelah@users.noreply.github.com> Date: Tue, 23 May 2023 18:10:09 +1000 Subject: [PATCH 04/34] ifelse: option to preserve variable scope (#832) * ifelse: option to preserve variable scope --- README.md | 6 +-- RULES_DESCRIPTIONS.md | 35 ++++++++++++++-- internal/ifelse/args.go | 11 +++++ internal/ifelse/branch.go | 43 +++++++++++++++---- internal/ifelse/branch_kind.go | 3 ++ internal/ifelse/chain.go | 1 + internal/ifelse/rule.go | 23 ++++++++--- rule/early-return.go | 11 +++-- rule/indent-error-flow.go | 11 +++-- rule/superfluous-else.go | 11 +++-- test/early-return_test.go | 3 ++ test/superfluous-else_test.go | 3 ++ testdata/early-return-scope.go | 66 ++++++++++++++++++++++++++++++ testdata/superfluous-else-scope.go | 64 +++++++++++++++++++++++++++++ 14 files changed, 261 insertions(+), 30 deletions(-) create mode 100644 internal/ifelse/args.go create mode 100644 testdata/early-return-scope.go create mode 100644 testdata/superfluous-else-scope.go diff --git a/README.md b/README.md index a4e852f02..d40271783 100644 --- a/README.md +++ b/README.md @@ -448,13 +448,13 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`package-comments`](./RULES_DESCRIPTIONS.md#package-comments) | n/a | Package commenting conventions. | yes | no | | [`range`](./RULES_DESCRIPTIONS.md#range) | n/a | Prevents redundant variables when iterating over a collection. | yes | no | | [`receiver-naming`](./RULES_DESCRIPTIONS.md#receiver-naming) | n/a | Conventions around the naming of receivers. | yes | no | -| [`indent-error-flow`](./RULES_DESCRIPTIONS.md#indent-error-flow) | n/a | Prevents redundant else statements. | yes | no | +| [`indent-error-flow`](./RULES_DESCRIPTIONS.md#indent-error-flow) | []string | Prevents redundant else statements. | yes | no | | [`argument-limit`](./RULES_DESCRIPTIONS.md#argument-limit) | int (defaults to 8) | Specifies the maximum number of arguments a function can receive | no | no | | [`cyclomatic`](./RULES_DESCRIPTIONS.md#cyclomatic) | int (defaults to 10) | Sets restriction for maximum Cyclomatic complexity. | no | no | | [`max-public-structs`](./RULES_DESCRIPTIONS.md#max-public-structs) | int (defaults to 5) | The maximum number of public structs in a file. | no | no | | [`file-header`](./RULES_DESCRIPTIONS.md#file-header) | string (defaults to none)| Header which each file should have. | no | no | | [`empty-block`](./RULES_DESCRIPTIONS.md#empty-block) | n/a | Warns on empty code blocks | no | yes | -| [`superfluous-else`](./RULES_DESCRIPTIONS.md#superfluous-else) | n/a | Prevents redundant else statements (extends [`indent-error-flow`](./RULES_DESCRIPTIONS.md#indent-error-flow)) | no | no | +| [`superfluous-else`](./RULES_DESCRIPTIONS.md#superfluous-else) | []string | Prevents redundant else statements (extends [`indent-error-flow`](./RULES_DESCRIPTIONS.md#indent-error-flow)) | no | no | | [`confusing-naming`](./RULES_DESCRIPTIONS.md#confusing-naming) | n/a | Warns on methods with names that differ only by capitalization | no | no | | [`get-return`](./RULES_DESCRIPTIONS.md#get-return) | n/a | Warns on getters that do not yield any result | no | no | | [`modifies-parameter`](./RULES_DESCRIPTIONS.md#modifies-parameter) | n/a | Warns on assignments to function parameters | no | no | @@ -487,7 +487,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`cognitive-complexity`](./RULES_DESCRIPTIONS.md#cognitive-complexity) | int (defaults to 7) | 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 | | [`string-format`](./RULES_DESCRIPTIONS.md#string-format) | map | Warns on specific string literals that fail one or more user-configured regular expressions | no | no | -| [`early-return`](./RULES_DESCRIPTIONS.md#early-return) | n/a | Spots if-then-else statements where the predicate may be inverted to reduce nesting | no | no | +| [`early-return`](./RULES_DESCRIPTIONS.md#early-return) | []string | Spots if-then-else statements where the predicate may be inverted to reduce nesting | no | no | | [`unconditional-recursion`](./RULES_DESCRIPTIONS.md#unconditional-recursion) | n/a | Warns on function calls that will lead to (direct) infinite recursion | no | no | | [`identical-branches`](./RULES_DESCRIPTIONS.md#identical-branches) | n/a | Spots if-then-else statements with identical `then` and `else` branches | no | no | | [`defer`](./RULES_DESCRIPTIONS.md#defer) | map | Warns on some [defer gotchas](https://blog.learngoprogramming.com/5-gotchas-of-defer-in-go-golang-part-iii-36a1ab3d6ef1) | no | no | diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index c52719ba0..02ad6069c 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -299,7 +299,7 @@ if cond { ``` where the `if` condition may be inverted in order to reduce nesting: ```go -if ! cond { +if !cond { // do other thing return ... } @@ -307,7 +307,16 @@ if ! cond { // do something ``` -_Configuration_: N/A +_Configuration_: ([]string) rule flags. Available flags are: + +* _preserveScope_: do not suggest refactorings that would increase variable scope + +Example: + +```toml +[rule.exported] + arguments =["preserveScope"] +``` ## empty-block @@ -448,7 +457,16 @@ This rule highlights redundant _else-blocks_ that can be eliminated from the cod More information [here](https://github.com/golang/go/wiki/CodeReviewComments#indent-error-flow) -_Configuration_: N/A +_Configuration_: ([]string) rule flags. Available flags are: + +* _preserveScope_: do not suggest refactorings that would increase variable scope + +Example: + +```toml +[rule.exported] + arguments =["preserveScope"] +``` ## imports-blacklist @@ -624,7 +642,16 @@ To accept the `inline` option in JSON tags (and `outline` and `gnu` in BSON tags _Description_: To improve the readability of code, it is recommended to reduce the indentation as much as possible. This rule highlights redundant _else-blocks_ that can be eliminated from the code. -_Configuration_: N/A +_Configuration_: ([]string) rule flags. Available flags are: + +* _preserveScope_: do not suggest refactorings that would increase variable scope + +Example: + +```toml +[rule.exported] + arguments =["preserveScope"] +``` ## time-equal diff --git a/internal/ifelse/args.go b/internal/ifelse/args.go new file mode 100644 index 000000000..c6e647e69 --- /dev/null +++ b/internal/ifelse/args.go @@ -0,0 +1,11 @@ +package ifelse + +// PreserveScope is a configuration argument that prevents suggestions +// that would enlarge variable scope +const PreserveScope = "preserveScope" + +// Args contains arguments common to the early-return, indent-error-flow +// and superfluous-else rules (currently just preserveScope) +type Args struct { + PreserveScope bool +} diff --git a/internal/ifelse/branch.go b/internal/ifelse/branch.go index 0d51a0a4e..6e6036b89 100644 --- a/internal/ifelse/branch.go +++ b/internal/ifelse/branch.go @@ -9,29 +9,37 @@ import ( // 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. + Call // The function called at the end for kind Panic or Exit. + HasDecls bool // The branch has one or more declarations (at the top level block) } // 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} + return Empty.Branch() } - switch stmt := block.List[blockLen-1].(type) { + branch := StmtBranch(block.List[blockLen-1]) + branch.HasDecls = hasDecls(block) + return branch +} + +// StmtBranch gets the Branch of an ast.Stmt. +func StmtBranch(stmt ast.Stmt) Branch { + switch stmt := stmt.(type) { case *ast.ReturnStmt: - return Branch{BranchKind: Return} + return Return.Branch() case *ast.BlockStmt: return BlockBranch(stmt) case *ast.BranchStmt: switch stmt.Tok { case token.BREAK: - return Branch{BranchKind: Break} + return Break.Branch() case token.CONTINUE: - return Branch{BranchKind: Continue} + return Continue.Branch() case token.GOTO: - return Branch{BranchKind: Goto} + return Goto.Branch() } case *ast.ExprStmt: fn, ok := ExprCall(stmt) @@ -42,9 +50,12 @@ func BlockBranch(block *ast.BlockStmt) Branch { if ok { return Branch{BranchKind: kind, Call: fn} } + case *ast.EmptyStmt: + return Empty.Branch() + case *ast.LabeledStmt: + return StmtBranch(stmt.Stmt) } - - return Branch{BranchKind: Regular} + return Regular.Branch() } // String returns a brief string representation @@ -66,3 +77,17 @@ func (b Branch) LongString() string { return b.BranchKind.LongString() } } + +func hasDecls(block *ast.BlockStmt) bool { + for _, stmt := range block.List { + switch stmt := stmt.(type) { + case *ast.DeclStmt: + return true + case *ast.AssignStmt: + if stmt.Tok == token.DEFINE { + return true + } + } + } + return false +} diff --git a/internal/ifelse/branch_kind.go b/internal/ifelse/branch_kind.go index 7df276fd8..41601d1e1 100644 --- a/internal/ifelse/branch_kind.go +++ b/internal/ifelse/branch_kind.go @@ -49,6 +49,9 @@ func (k BranchKind) Deviates() bool { } } +// Branch returns a Branch with the given kind +func (k BranchKind) Branch() Branch { return Branch{BranchKind: k} } + // String returns a brief string representation func (k BranchKind) String() string { switch k { diff --git a/internal/ifelse/chain.go b/internal/ifelse/chain.go index d6afebff6..9891635ee 100644 --- a/internal/ifelse/chain.go +++ b/internal/ifelse/chain.go @@ -6,4 +6,5 @@ type Chain struct { 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? + AtBlockEnd bool // whether the chain is placed at the end of the surrounding block } diff --git a/internal/ifelse/rule.go b/internal/ifelse/rule.go index ec0db01e6..07ad456b6 100644 --- a/internal/ifelse/rule.go +++ b/internal/ifelse/rule.go @@ -9,7 +9,7 @@ import ( // Rule is an interface for linters operating on if-else chains type Rule interface { - CheckIfElse(chain Chain) (failMsg string) + CheckIfElse(chain Chain, args Args) (failMsg string) } // Apply evaluates the given Rule on if-else chains found within the given AST, @@ -28,8 +28,13 @@ type Rule interface { // // 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 { +func Apply(rule Rule, node ast.Node, target Target, args lint.Arguments) []lint.Failure { v := &visitor{rule: rule, target: target} + for _, arg := range args { + if arg == PreserveScope { + v.args.PreserveScope = true + } + } ast.Walk(v, node) return v.failures } @@ -38,15 +43,22 @@ type visitor struct { failures []lint.Failure target Target rule Rule + args Args } func (v *visitor) Visit(node ast.Node) ast.Visitor { - ifStmt, ok := node.(*ast.IfStmt) + block, ok := node.(*ast.BlockStmt) if !ok { return v } - v.visitChain(ifStmt, Chain{}) + for i, stmt := range block.List { + if ifStmt, ok := stmt.(*ast.IfStmt); ok { + v.visitChain(ifStmt, Chain{AtBlockEnd: i == len(block.List)-1}) + continue + } + ast.Walk(v, stmt) + } return nil } @@ -73,8 +85,9 @@ func (v *visitor) visitChain(ifStmt *ast.IfStmt, chain 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 failMsg := v.rule.CheckIfElse(chain, v.args); 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 diff --git a/rule/early-return.go b/rule/early-return.go index f5f8ff55d..90a0acc55 100644 --- a/rule/early-return.go +++ b/rule/early-return.go @@ -12,8 +12,8 @@ import ( type EarlyReturnRule struct{} // Apply applies the rule to given file. -func (e *EarlyReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { - return ifelse.Apply(e, file.AST, ifelse.TargetIf) +func (e *EarlyReturnRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure { + return ifelse.Apply(e, file.AST, ifelse.TargetIf, args) } // Name returns the rule name. @@ -22,7 +22,7 @@ func (*EarlyReturnRule) Name() string { } // CheckIfElse evaluates the rule against an ifelse.Chain. -func (e *EarlyReturnRule) CheckIfElse(chain ifelse.Chain) (failMsg string) { +func (e *EarlyReturnRule) CheckIfElse(chain ifelse.Chain, args ifelse.Args) (failMsg string) { if !chain.Else.Deviates() { // this rule only applies if the else-block deviates control flow return @@ -39,6 +39,11 @@ func (e *EarlyReturnRule) CheckIfElse(chain ifelse.Chain) (failMsg string) { return } + if args.PreserveScope && !chain.AtBlockEnd && (chain.HasInitializer || chain.If.HasDecls) { + // avoid increasing variable scope + return + } + if chain.If.IsEmpty() { return fmt.Sprintf("if c { } else { %[1]v } can be simplified to if !c { %[1]v }", chain.Else) } diff --git a/rule/indent-error-flow.go b/rule/indent-error-flow.go index 4d62404e0..b80e6486c 100644 --- a/rule/indent-error-flow.go +++ b/rule/indent-error-flow.go @@ -9,8 +9,8 @@ import ( type IndentErrorFlowRule struct{} // Apply applies the rule to given file. -func (e *IndentErrorFlowRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { - return ifelse.Apply(e, file.AST, ifelse.TargetElse) +func (e *IndentErrorFlowRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure { + return ifelse.Apply(e, file.AST, ifelse.TargetElse, args) } // Name returns the rule name. @@ -19,7 +19,7 @@ func (*IndentErrorFlowRule) Name() string { } // CheckIfElse evaluates the rule against an ifelse.Chain. -func (e *IndentErrorFlowRule) CheckIfElse(chain ifelse.Chain) (failMsg string) { +func (e *IndentErrorFlowRule) CheckIfElse(chain ifelse.Chain, args ifelse.Args) (failMsg string) { if !chain.If.Deviates() { // this rule only applies if the if-block deviates control flow return @@ -36,5 +36,10 @@ func (e *IndentErrorFlowRule) CheckIfElse(chain ifelse.Chain) (failMsg string) { return } + if args.PreserveScope && !chain.AtBlockEnd && (chain.HasInitializer || chain.Else.HasDecls) { + // avoid increasing variable scope + return + } + return "if block ends with a return statement, so drop this else and outdent its block" } diff --git a/rule/superfluous-else.go b/rule/superfluous-else.go index 9dce59a46..1ef67bf1a 100644 --- a/rule/superfluous-else.go +++ b/rule/superfluous-else.go @@ -10,8 +10,8 @@ import ( type SuperfluousElseRule struct{} // Apply applies the rule to given file. -func (e *SuperfluousElseRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { - return ifelse.Apply(e, file.AST, ifelse.TargetElse) +func (e *SuperfluousElseRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure { + return ifelse.Apply(e, file.AST, ifelse.TargetElse, args) } // Name returns the rule name. @@ -20,7 +20,7 @@ func (*SuperfluousElseRule) Name() string { } // CheckIfElse evaluates the rule against an ifelse.Chain. -func (e *SuperfluousElseRule) CheckIfElse(chain ifelse.Chain) (failMsg string) { +func (e *SuperfluousElseRule) CheckIfElse(chain ifelse.Chain, args ifelse.Args) (failMsg string) { if !chain.If.Deviates() { // this rule only applies if the if-block deviates control flow return @@ -37,5 +37,10 @@ func (e *SuperfluousElseRule) CheckIfElse(chain ifelse.Chain) (failMsg string) { return } + if args.PreserveScope && !chain.AtBlockEnd && (chain.HasInitializer || chain.Else.HasDecls) { + // avoid increasing variable scope + return + } + return fmt.Sprintf("if block ends with %v, so drop this else and outdent its block", chain.If.LongString()) } diff --git a/test/early-return_test.go b/test/early-return_test.go index 0f1b63b5f..a8f54f39d 100644 --- a/test/early-return_test.go +++ b/test/early-return_test.go @@ -3,10 +3,13 @@ package test import ( "testing" + "github.com/mgechev/revive/internal/ifelse" + "github.com/mgechev/revive/lint" "github.com/mgechev/revive/rule" ) // TestEarlyReturn tests early-return rule. func TestEarlyReturn(t *testing.T) { testRule(t, "early-return", &rule.EarlyReturnRule{}) + testRule(t, "early-return-scope", &rule.EarlyReturnRule{}, &lint.RuleConfig{Arguments: []interface{}{ifelse.PreserveScope}}) } diff --git a/test/superfluous-else_test.go b/test/superfluous-else_test.go index ab68b30d1..073777722 100644 --- a/test/superfluous-else_test.go +++ b/test/superfluous-else_test.go @@ -3,10 +3,13 @@ package test import ( "testing" + "github.com/mgechev/revive/internal/ifelse" + "github.com/mgechev/revive/lint" "github.com/mgechev/revive/rule" ) // TestSuperfluousElse rule. func TestSuperfluousElse(t *testing.T) { testRule(t, "superfluous-else", &rule.SuperfluousElseRule{}) + testRule(t, "superfluous-else-scope", &rule.SuperfluousElseRule{}, &lint.RuleConfig{Arguments: []interface{}{ifelse.PreserveScope}}) } diff --git a/testdata/early-return-scope.go b/testdata/early-return-scope.go new file mode 100644 index 000000000..a9e8118d7 --- /dev/null +++ b/testdata/early-return-scope.go @@ -0,0 +1,66 @@ +// Test data for the early-return rule with preserveScope option enabled + +package fixtures + +func fn1() { + // No initializer, match as normal + if cond { // MATCH /if c { ... } else { ... return } can be simplified to if !c { ... return } .../ + fn2() + } else { + return + } +} + +func fn2() { + // Moving the declaration of x here is fine since it goes out of scope either way + if x := fn1(); x != nil { // MATCH /if c { ... } else { ... return } can be simplified to if !c { ... return } ... (move short variable declaration to its own line if necessary)/ + fn2() + } else { + return + } +} + +func fn3() { + // Don't want to move the declaration of x here since it stays in scope afterward + if x := fn1(); x != nil { + fn2() + } else { + return + } + x := fn2() + fn3(x) +} + +func fn4() { + if cond { + var x = fn2() + fn3(x) + } else { + return + } + // Don't want to move the declaration of x here since it stays in scope afterward + y := fn2() + fn3(y) +} + +func fn5() { + if cond { + x := fn2() + fn3(x) + } else { + return + } + // Don't want to move the declaration of x here since it stays in scope afterward + y := fn2() + fn3(y) +} + +func fn6() { + if cond { // MATCH /if c { ... } else { ... return } can be simplified to if !c { ... return } .../ + x := fn2() + fn3(x) + } else { + return + } + // Moving x here is fine since it goes out of scope anyway +} diff --git a/testdata/superfluous-else-scope.go b/testdata/superfluous-else-scope.go new file mode 100644 index 000000000..d1d679759 --- /dev/null +++ b/testdata/superfluous-else-scope.go @@ -0,0 +1,64 @@ +// Test data for the superfluous-else rule with preserveScope option enabled + +package fixtures + +func fn1() { + for { + // No initializer, match as normal + if cond { + continue + } else { // MATCH /if block ends with a continue statement, so drop this else and outdent its block/ + fn2() + } + } +} + +func fn2() { + for { + // Moving the declaration of x here is fine since it goes out of scope either way + if x := fn1(); x != 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)/ + fn2() + } + } +} + +func fn3() { + for { + // Don't want to move the declaration of x here since it stays in scope afterward + if x := fn1(); x != nil { + continue + } else { + fn2() + } + x := fn2() + fn3(x) + } +} + +func fn4() { + for { + if cond { + continue + } else { + x := fn1() + fn2(x) + } + // Don't want to move the declaration of x here since it stays in scope afterward + y := fn2() + fn3(y) + } +} + +func fn4() { + for { + if cond { + continue + } else { // MATCH /if block ends with a continue statement, so drop this else and outdent its block/ + x := fn1() + fn2(x) + } + // Moving x here is fine since it goes out of scope anyway + } +} From 68bf7e5f38e8b72c89c5b6cf9e6d7b93b2fd0c76 Mon Sep 17 00:00:00 2001 From: Minko Gechev Date: Tue, 23 May 2023 10:19:56 -0700 Subject: [PATCH 05/34] build: remove go 1.17.x from ci (#833) --- .github/workflows/test.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 6a484d596..11c4a344d 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -12,7 +12,6 @@ jobs: fail-fast: false matrix: go-version: - - 1.17.x - 1.18.x - 1.19.x steps: From ca38cc365567d40eb3aa90c8be18739aa8b29f16 Mon Sep 17 00:00:00 2001 From: meanguy <78570571+meanguy@users.noreply.github.com> Date: Tue, 30 May 2023 01:24:09 -0700 Subject: [PATCH 06/34] Fix typo in example documentation for early-return (#834) The example for configuring `early-return` arguments mistakenly refers to the `exported` rule. --- RULES_DESCRIPTIONS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 02ad6069c..e6fa3d02e 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -314,7 +314,7 @@ _Configuration_: ([]string) rule flags. Available flags are: Example: ```toml -[rule.exported] +[rule.early-return] arguments =["preserveScope"] ``` From 7cfe9d8eaf236d115c0d3c8ed47d021d5d2805d9 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 6 Jun 2023 14:30:28 -0700 Subject: [PATCH 07/34] fix(deps): update module golang.org/x/tools to v0.9.3 (#837) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 38a0678c0..df4c77a81 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/mitchellh/go-homedir v1.1.0 github.com/olekukonko/tablewriter v0.0.5 github.com/pkg/errors v0.9.1 - golang.org/x/tools v0.9.1 + golang.org/x/tools v0.9.3 ) require ( diff --git a/go.sum b/go.sum index 86c5ec8ff..01f4de4b9 100644 --- a/go.sum +++ b/go.sum @@ -88,6 +88,8 @@ golang.org/x/tools v0.7.0 h1:W4OVu8VVOaIO0yzWMNdepAulS7YfoS3Zabrm8DOXXU4= golang.org/x/tools v0.7.0/go.mod h1:4pg6aUX35JBAogB10C9AtvVL+qowtN4pT3CGSQex14s= golang.org/x/tools v0.9.1 h1:8WMNJAz3zrtPmnYC7ISf5dEn3MT0gY7jBJfw27yrrLo= golang.org/x/tools v0.9.1/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc= +golang.org/x/tools v0.9.3 h1:Gn1I8+64MsuTb/HpH+LmQtNas23LhUVr3rYZ0eKuaMM= +golang.org/x/tools v0.9.3/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= From 5ef33f140626be85c0756c08be6d7e429d887d2f Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Tue, 6 Jun 2023 14:30:47 -0700 Subject: [PATCH 08/34] fix(deps): update github.com/chavacava/garif digest to 8144c22 (#836) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index df4c77a81..981ee7978 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.19 require ( github.com/BurntSushi/toml v1.2.1 - github.com/chavacava/garif v0.0.0-20230519080132-4752330f72df + github.com/chavacava/garif v0.0.0-20230531072157-8144c224b063 github.com/fatih/color v1.15.0 github.com/fatih/structtag v1.2.0 github.com/mgechev/dots v0.0.0-20210922191527-e955255bf517 diff --git a/go.sum b/go.sum index 01f4de4b9..90c82f9dd 100644 --- a/go.sum +++ b/go.sum @@ -10,6 +10,8 @@ github.com/chavacava/garif v0.0.0-20230227094218-b8c73b2037b8 h1:W9o46d2kbNL06lq github.com/chavacava/garif v0.0.0-20230227094218-b8c73b2037b8/go.mod h1:gakxgyXaaPkxvLw1XQxNGK4I37ys9iBRzNUx/B7pUCo= github.com/chavacava/garif v0.0.0-20230519080132-4752330f72df h1:1uGdlpQT0irrGcFFOUuitqSCE6BjttfHd+k3k9OQ0fg= github.com/chavacava/garif v0.0.0-20230519080132-4752330f72df/go.mod h1:cFP7fAFavJ2DrYBmZYBETNKwSTFJiOIirm5N4/PqY/I= +github.com/chavacava/garif v0.0.0-20230531072157-8144c224b063 h1:wiJ8AtgOw7gDs2I9uhrxW59fMPWXONSdpXU3/dRv0to= +github.com/chavacava/garif v0.0.0-20230531072157-8144c224b063/go.mod h1:XMyYCkEL58DF0oyW4qDjjnPWONs2HBqYKI+UIPD+Gww= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -52,6 +54,7 @@ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= From da1c99ded906641d470a8e19470734759ae075a6 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 14 Jun 2023 08:31:32 +0200 Subject: [PATCH 09/34] fix(deps): update module github.com/burntsushi/toml to v1.3.2 (#835) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 981ee7978..b0bc2da40 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/mgechev/revive go 1.19 require ( - github.com/BurntSushi/toml v1.2.1 + github.com/BurntSushi/toml v1.3.2 github.com/chavacava/garif v0.0.0-20230531072157-8144c224b063 github.com/fatih/color v1.15.0 github.com/fatih/structtag v1.2.0 diff --git a/go.sum b/go.sum index 90c82f9dd..2f069c28a 100644 --- a/go.sum +++ b/go.sum @@ -2,6 +2,8 @@ github.com/BurntSushi/toml v1.2.0 h1:Rt8g24XnyGTyglgET/PRUNlrUeu9F5L+7FilkXfZgs0 github.com/BurntSushi/toml v1.2.0/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= github.com/BurntSushi/toml v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak= github.com/BurntSushi/toml v1.2.1/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= +github.com/BurntSushi/toml v1.3.2 h1:o7IhLm0Msx3BaB+n3Ag7L8EVlByGnpq14C4YWiu/gL8= +github.com/BurntSushi/toml v1.3.2/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= github.com/chavacava/garif v0.0.0-20220630083739-93517212f375 h1:E7LT642ysztPWE0dfz43cWOvMiF42DyTRC+eZIaO4yI= github.com/chavacava/garif v0.0.0-20220630083739-93517212f375/go.mod h1:4m1Rv7xfuwWPNKXlThldNuJvutYM6J95wNuuVmn55To= github.com/chavacava/garif v0.0.0-20221024190013-b3ef35877348 h1:cy5GCEZLUCshCGCRRUjxHrDUqkB4l5cuUt3ShEckQEo= From e5d5d0902637588054d7f738788186a70175532e Mon Sep 17 00:00:00 2001 From: Onur Cinar Date: Wed, 21 Jun 2023 23:56:53 -0700 Subject: [PATCH 10/34] Adding Checker Go library to the list of Who Uses. (#842) Proposing to add the Checker Go library to the list of Who Uses. --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index d40271783..19c789c1b 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,7 @@ Here's how `revive` is different from `golint`: - [`ggz`](https://github.com/go-ggz/ggz) - An URL shortener service written in Golang - [`Codeac.io`](https://www.codeac.io?ref=revive) - Automated code review service integrates with GitHub, Bitbucket and GitLab (even self-hosted) and helps you fight technical debt. - [`DevLake`](https://github.com/apache/incubator-devlake) - Apache DevLake is an open-source dev data platform to ingest, analyze, and visualize the fragmented data from DevOps tools,which can distill insights to improve engineering productivity. +- [`checker`](https://github.com/cinar/checker) - Checker helps validating user input through rules defined in struct tags or directly through functions. *Open a PR to add your project*. From a2701259fe2f34dd6607a17f57deb837f99546a4 Mon Sep 17 00:00:00 2001 From: Stephen Brown II Date: Mon, 26 Jun 2023 11:41:58 -0500 Subject: [PATCH 11/34] Update defaults.toml to sort rules (#844) --- defaults.toml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/defaults.toml b/defaults.toml index f9e2405d4..35cc2bf9f 100644 --- a/defaults.toml +++ b/defaults.toml @@ -8,23 +8,23 @@ warningCode = 0 [rule.context-as-argument] [rule.context-keys-type] [rule.dot-imports] +[rule.empty-block] +[rule.error-naming] [rule.error-return] [rule.error-strings] -[rule.error-naming] +[rule.errorf] [rule.exported] [rule.if-return] [rule.increment-decrement] -[rule.var-naming] -[rule.var-declaration] +[rule.indent-error-flow] [rule.package-comments] [rule.range] [rule.receiver-naming] +[rule.redefines-builtin-id] +[rule.superfluous-else] [rule.time-naming] [rule.unexported-return] -[rule.indent-error-flow] -[rule.errorf] -[rule.empty-block] -[rule.superfluous-else] -[rule.unused-parameter] [rule.unreachable-code] -[rule.redefines-builtin-id] +[rule.unused-parameter] +[rule.var-declaration] +[rule.var-naming] From 2b4286ede2fbbcc790e1c5866141d1fd55c98dc8 Mon Sep 17 00:00:00 2001 From: Robert Liebowitz Date: Mon, 26 Jun 2023 12:43:19 -0400 Subject: [PATCH 12/34] Drop if-return from default ruleset (#843) The `if-return` rule was originally a golint rule which was removed from their ruleset for being out of scope. Similarly, it was dropped from revive intentionally as a result of #537. More recently, it was reintroduced into the default ruleset as a result of #799 due to a discrepancy in documentation without a discussion of whether this rule in particular belonged as a part of that default rule set. While it is no longer a goal of this project to align 100% with the golint defaults, I believe that this rule gives bad advice often enough that it should not be enabled by default. For example, consider the following code: ```go if err := func1(); err != nil { return err } if err := func2(); err != nil { return err } if err := func3(); err != nil { return err } return nil ``` The `if-return` rule considers this a violation of style, and instead suggests the following: ```go if err := func1(); err != nil { return err } if err := func2(); err != nil { return err } return func3() ``` While this is more terse, it has a few shortcomings compared to the original. In particular, it means extending the size of the diff if changing the order of checks, adding logic after the call that currently happens to be last, or choosing to wrap the error. And in that last case, it can make it less obvious that there is an unwrapped error being propagated up the call stack. This in practice has a very similar effect to disabling trailing commas; while it is not necessarily wrong as a style choice, I don't believe it warrants a position as part of the default ruleset here. See-also: https://github.com/golang/lint/issues/363 --- README.md | 1 - config/config.go | 2 +- defaults.toml | 1 - revivelib/core_test.go | 6 +++--- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 19c789c1b..e706bbd8a 100644 --- a/README.md +++ b/README.md @@ -407,7 +407,6 @@ warningCode = 0 [rule.error-strings] [rule.error-naming] [rule.exported] -[rule.if-return] [rule.increment-decrement] [rule.var-naming] [rule.var-declaration] diff --git a/config/config.go b/config/config.go index 04cd21404..f6ee2a464 100644 --- a/config/config.go +++ b/config/config.go @@ -31,7 +31,6 @@ var defaultRules = []lint.Rule{ &rule.TimeNamingRule{}, &rule.ContextKeysType{}, &rule.ContextAsArgumentRule{}, - &rule.IfReturnRule{}, &rule.EmptyBlockRule{}, &rule.SuperfluousElseRule{}, &rule.UnusedParamRule{}, @@ -87,6 +86,7 @@ var allRules = append([]lint.Rule{ &rule.UseAnyRule{}, &rule.DataRaceRule{}, &rule.CommentSpacingsRule{}, + &rule.IfReturnRule{}, }, defaultRules...) var allFormatters = []lint.Formatter{ diff --git a/defaults.toml b/defaults.toml index 35cc2bf9f..d6186454a 100644 --- a/defaults.toml +++ b/defaults.toml @@ -14,7 +14,6 @@ warningCode = 0 [rule.error-strings] [rule.errorf] [rule.exported] -[rule.if-return] [rule.increment-decrement] [rule.indent-error-flow] [rule.package-comments] diff --git a/revivelib/core_test.go b/revivelib/core_test.go index 44ab62764..9dd15056e 100644 --- a/revivelib/core_test.go +++ b/revivelib/core_test.go @@ -7,6 +7,7 @@ import ( "github.com/mgechev/revive/config" "github.com/mgechev/revive/lint" "github.com/mgechev/revive/revivelib" + "github.com/mgechev/revive/rule" ) func TestReviveLint(t *testing.T) { @@ -45,7 +46,6 @@ func TestReviveFormat(t *testing.T) { // ACT failures, exitCode, err := revive.Format("stylish", failuresChan) - // ASSERT if err != nil { t.Fatal(err) @@ -70,8 +70,7 @@ func TestReviveFormat(t *testing.T) { } } -type mockRule struct { -} +type mockRule struct{} func (r *mockRule) Name() string { return "mock-rule" @@ -93,6 +92,7 @@ func getMockRevive(t *testing.T) *revivelib.Revive { conf, true, 2048, + revivelib.NewExtraRule(&rule.IfReturnRule{}, lint.RuleConfig{}), revivelib.NewExtraRule(&mockRule{}, lint.RuleConfig{}), ) if err != nil { From 9564ad9f073d520ecd74339daa6697bda011f6ef Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 26 Jun 2023 09:43:34 -0700 Subject: [PATCH 13/34] fix(deps): update module golang.org/x/tools to v0.10.0 (#841) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- go.mod | 4 ++-- go.sum | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index b0bc2da40..ccefc19a3 100644 --- a/go.mod +++ b/go.mod @@ -11,12 +11,12 @@ require ( github.com/mitchellh/go-homedir v1.1.0 github.com/olekukonko/tablewriter v0.0.5 github.com/pkg/errors v0.9.1 - golang.org/x/tools v0.9.3 + golang.org/x/tools v0.10.0 ) require ( github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.17 // indirect github.com/mattn/go-runewidth v0.0.9 // indirect - golang.org/x/sys v0.8.0 // indirect + golang.org/x/sys v0.9.0 // indirect ) diff --git a/go.sum b/go.sum index 2f069c28a..865a5380c 100644 --- a/go.sum +++ b/go.sum @@ -77,6 +77,8 @@ golang.org/x/sys v0.6.0 h1:MVltZSvRTcU2ljQOhs94SXPftV6DCNnZViHeQps87pQ= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.9.0 h1:KS/R3tvhPqvJvwcKfnBHJwwthS11LRhmM5D59eEXa0s= +golang.org/x/sys v0.9.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/tools v0.1.12 h1:VveCTK38A2rkS8ZqFY25HIDFscX5X9OoEhJd3quQmXU= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.2.0 h1:G6AHpWxTMGY1KyEYoAQ5WTtIekUUvDNjan3ugu60JvE= @@ -95,6 +97,8 @@ golang.org/x/tools v0.9.1 h1:8WMNJAz3zrtPmnYC7ISf5dEn3MT0gY7jBJfw27yrrLo= golang.org/x/tools v0.9.1/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc= golang.org/x/tools v0.9.3 h1:Gn1I8+64MsuTb/HpH+LmQtNas23LhUVr3rYZ0eKuaMM= golang.org/x/tools v0.9.3/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc= +golang.org/x/tools v0.10.0 h1:tvDr/iQoUqNdohiYm0LmmKcBk+q86lb9EprIUFhHHGg= +golang.org/x/tools v0.10.0/go.mod h1:UJwyiVBsOA2uwvK/e5OY3GTpDUJriEd+/YlqAwLPmyM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= From 95fcfe36f6cb4903b5ac0d63b583fde86179f3c9 Mon Sep 17 00:00:00 2001 From: kerneltravel Date: Wed, 5 Jul 2023 14:12:42 +0800 Subject: [PATCH 14/34] doc: add milvus to README usage (#845) (#847) --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index e706bbd8a..e164826d4 100644 --- a/README.md +++ b/README.md @@ -55,6 +55,7 @@ Here's how `revive` is different from `golint`: - [`Codeac.io`](https://www.codeac.io?ref=revive) - Automated code review service integrates with GitHub, Bitbucket and GitLab (even self-hosted) and helps you fight technical debt. - [`DevLake`](https://github.com/apache/incubator-devlake) - Apache DevLake is an open-source dev data platform to ingest, analyze, and visualize the fragmented data from DevOps tools,which can distill insights to improve engineering productivity. - [`checker`](https://github.com/cinar/checker) - Checker helps validating user input through rules defined in struct tags or directly through functions. +- [`milvus`](https://github.com/milvus-io/milvus) - A cloud-native vector database, storage for next generation AI applications. *Open a PR to add your project*. From 6a2c5b132ec93fedcef7cdf1726518209a905fc4 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 5 Jul 2023 15:32:27 -0700 Subject: [PATCH 15/34] fix(deps): update module golang.org/x/tools to v0.11.0 (#848) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- go.mod | 4 ++-- go.sum | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index ccefc19a3..1bb14e972 100644 --- a/go.mod +++ b/go.mod @@ -11,12 +11,12 @@ require ( github.com/mitchellh/go-homedir v1.1.0 github.com/olekukonko/tablewriter v0.0.5 github.com/pkg/errors v0.9.1 - golang.org/x/tools v0.10.0 + golang.org/x/tools v0.11.0 ) require ( github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.17 // indirect github.com/mattn/go-runewidth v0.0.9 // indirect - golang.org/x/sys v0.9.0 // indirect + golang.org/x/sys v0.10.0 // indirect ) diff --git a/go.sum b/go.sum index 865a5380c..5a26346e0 100644 --- a/go.sum +++ b/go.sum @@ -79,6 +79,8 @@ golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.9.0 h1:KS/R3tvhPqvJvwcKfnBHJwwthS11LRhmM5D59eEXa0s= golang.org/x/sys v0.9.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.10.0 h1:SqMFp9UcQJZa+pmYuAKjd9xq1f0j5rLcDIk0mj4qAsA= +golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/tools v0.1.12 h1:VveCTK38A2rkS8ZqFY25HIDFscX5X9OoEhJd3quQmXU= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.2.0 h1:G6AHpWxTMGY1KyEYoAQ5WTtIekUUvDNjan3ugu60JvE= @@ -99,6 +101,8 @@ golang.org/x/tools v0.9.3 h1:Gn1I8+64MsuTb/HpH+LmQtNas23LhUVr3rYZ0eKuaMM= golang.org/x/tools v0.9.3/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc= golang.org/x/tools v0.10.0 h1:tvDr/iQoUqNdohiYm0LmmKcBk+q86lb9EprIUFhHHGg= golang.org/x/tools v0.10.0/go.mod h1:UJwyiVBsOA2uwvK/e5OY3GTpDUJriEd+/YlqAwLPmyM= +golang.org/x/tools v0.11.0 h1:EMCa6U9S2LtZXLAMoWiR/R8dAQFRqbAitmbJ2UKhoi8= +golang.org/x/tools v0.11.0/go.mod h1:anzJrxPjNtfgiYQYirP2CPGzGLxrH2u2QBhn6Bf3qY8= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= From a155d92342b6cbdc4629bd93393884f936cf3420 Mon Sep 17 00:00:00 2001 From: Ricardo Martin Marcucci Date: Sat, 29 Jul 2023 06:27:07 -0300 Subject: [PATCH 16/34] fix: support method call from structures for string-format (#840) * fix: support method call from structures for string-format * fix: added test for MR --- rule/string-format.go | 10 +++++++--- test/string-format_test.go | 6 +++++- testdata/string-format.go | 15 +++++++++++++++ 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/rule/string-format.go b/rule/string-format.go index 0e30ebf8b..3edd62477 100644 --- a/rule/string-format.go +++ b/rule/string-format.go @@ -211,10 +211,14 @@ func (lintStringFormatRule) getCallName(call *ast.CallExpr) (callName string, ok if selector, ok := call.Fun.(*ast.SelectorExpr); ok { // Scoped function call scope, ok := selector.X.(*ast.Ident) - if !ok { - return "", false + if ok { + return scope.Name + "." + selector.Sel.Name, true + } + // Scoped function call inside structure + recv, ok := selector.X.(*ast.SelectorExpr) + if ok { + return recv.Sel.Name + "." + selector.Sel.Name, true } - return scope.Name + "." + selector.Sel.Name, true } return "", false diff --git a/test/string-format_test.go b/test/string-format_test.go index f58c10c35..1d425c6dc 100644 --- a/test/string-format_test.go +++ b/test/string-format_test.go @@ -21,7 +21,11 @@ func TestStringFormat(t *testing.T) { []interface{}{ "s.Method3[2]", "!/^[Tt][Hh]/", - "must not start with 'th'"}}}) + "must not start with 'th'"}, + []interface{}{ + "s.Method4", // same as before, but called from a struct + "!/^[Ot][Tt]/", + "must not start with 'ot'"}}}) } func TestStringFormatArgumentParsing(t *testing.T) { diff --git a/testdata/string-format.go b/testdata/string-format.go index 69c320463..08e0fa21a 100644 --- a/testdata/string-format.go +++ b/testdata/string-format.go @@ -18,6 +18,16 @@ func (s stringFormatMethods) Method3(a, b, c string) { } +type stringFormatMethodsInjected struct{} + +func (s stringFormatMethodsInjected) Method4(a, b, c string) { + +} + +type container struct { + s stringFormatMethodsInjected +} + func stringFormat() { stringFormatMethod1("This string is fine", "") stringFormatMethod1("this string is not capitalized", "") // MATCH /must start with a capital letter/ @@ -27,4 +37,9 @@ func stringFormat() { d: "This string is capitalized, but ends with a period."}) // MATCH /string literal doesn't match user defined regex /[^\.]$// s := stringFormatMethods{} s.Method3("", "", "This string starts with th") // MATCH /must not start with 'th'/ + + c := container{ + s: stringFormatMethods{}, + } + c.s.Method4("Other string starts with ot") // MATCH /must not start with 'ot'/ } From 26bc59f58c5ae84e97219a47ea0e4df954efd492 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Sat, 29 Jul 2023 11:45:34 +0200 Subject: [PATCH 17/34] fix(deps): update github.com/chavacava/garif digest to 4bd63c2 (#838) * fix(deps): update github.com/chavacava/garif digest to 4bd63c2 * fix garif level compatibility --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: chavacava --- formatter/sarif.go | 2 +- go.mod | 2 +- go.sum | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/formatter/sarif.go b/formatter/sarif.go index c6288db76..c42da73eb 100644 --- a/formatter/sarif.go +++ b/formatter/sarif.go @@ -88,7 +88,7 @@ func (l *reviveRunLog) AddResult(failure lint.Failure) { location := garif.NewLocation().WithURI(filename).WithLineColumn(line, column) result.Locations = append(result.Locations, location) result.RuleId = failure.RuleName - result.Level = l.rules[failure.RuleName].Severity + result.Level = garif.ResultLevel(l.rules[failure.RuleName].Severity) l.run.Results = append(l.run.Results, result) } diff --git a/go.mod b/go.mod index 1bb14e972..87fce0171 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.19 require ( github.com/BurntSushi/toml v1.3.2 - github.com/chavacava/garif v0.0.0-20230531072157-8144c224b063 + github.com/chavacava/garif v0.0.0-20230608123814-4bd63c2919ab github.com/fatih/color v1.15.0 github.com/fatih/structtag v1.2.0 github.com/mgechev/dots v0.0.0-20210922191527-e955255bf517 diff --git a/go.sum b/go.sum index 5a26346e0..e05c9ea91 100644 --- a/go.sum +++ b/go.sum @@ -14,6 +14,8 @@ github.com/chavacava/garif v0.0.0-20230519080132-4752330f72df h1:1uGdlpQT0irrGcF github.com/chavacava/garif v0.0.0-20230519080132-4752330f72df/go.mod h1:cFP7fAFavJ2DrYBmZYBETNKwSTFJiOIirm5N4/PqY/I= github.com/chavacava/garif v0.0.0-20230531072157-8144c224b063 h1:wiJ8AtgOw7gDs2I9uhrxW59fMPWXONSdpXU3/dRv0to= github.com/chavacava/garif v0.0.0-20230531072157-8144c224b063/go.mod h1:XMyYCkEL58DF0oyW4qDjjnPWONs2HBqYKI+UIPD+Gww= +github.com/chavacava/garif v0.0.0-20230608123814-4bd63c2919ab h1:5JxePczlyGAtj6R1MUEFZ/UFud6FfsOejq7xLC2ZIb0= +github.com/chavacava/garif v0.0.0-20230608123814-4bd63c2919ab/go.mod h1:XMyYCkEL58DF0oyW4qDjjnPWONs2HBqYKI+UIPD+Gww= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= From 7bd666898c73f17ae4350e455eca2a372ac32eb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dami=C3=A1n=20Ferencz?= Date: Mon, 31 Jul 2023 03:22:40 -0300 Subject: [PATCH 18/34] feat: add rule for redundant import alias (#854) --- README.md | 1 + RULES_DESCRIPTIONS.md | 7 ++++ config/config.go | 1 + rule/redundant-import-alias.go | 52 +++++++++++++++++++++++++++++ test/redundant-import-alias_test.go | 11 ++++++ testdata/redundant-import-alias.go | 12 +++++++ 6 files changed, 84 insertions(+) create mode 100644 rule/redundant-import-alias.go create mode 100644 test/redundant-import-alias_test.go create mode 100644 testdata/redundant-import-alias.go diff --git a/README.md b/README.md index e164826d4..43800ae98 100644 --- a/README.md +++ b/README.md @@ -501,6 +501,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`use-any`](./RULES_DESCRIPTIONS.md#use-any) | n/a | Proposes to replace `interface{}` with its alias `any` | no | no | | [`datarace`](./RULES_DESCRIPTIONS.md#datarace) | n/a | Spots potential dataraces | no | no | | [`comment-spacings`](./RULES_DESCRIPTIONS.md#comment-spacings) | []string | Warns on malformed comments | no | no | +| [`redundant-import-alias`](./RULES_DESCRIPTIONS.md#redundant-import-alias) | n/a | Warns on import aliases matching the imported package name | no | no | ## Configurable rules diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index e6fa3d02e..4b48b526d 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -75,6 +75,7 @@ List of all available rules. - [use-any](#use-any) - [useless-break](#useless-break) - [waitgroup-by-value](#waitgroup-by-value) + - [redundant-import-alias](#redundant-import-alias) ## add-constant @@ -760,3 +761,9 @@ _Description_: Function parameters that are passed by value, are in fact a copy This rule warns when a `sync.WaitGroup` expected as a by-value parameter in a function or method. _Configuration_: N/A + +## redundant-import-alias + +_Description_: This rule warns on redundant import aliases. This happens when the alias used on the import statement matches the imported package name. + +_Configuration_: N/A \ No newline at end of file diff --git a/config/config.go b/config/config.go index f6ee2a464..225a570c8 100644 --- a/config/config.go +++ b/config/config.go @@ -87,6 +87,7 @@ var allRules = append([]lint.Rule{ &rule.DataRaceRule{}, &rule.CommentSpacingsRule{}, &rule.IfReturnRule{}, + &rule.RedundantImportAlias{}, }, defaultRules...) var allFormatters = []lint.Formatter{ diff --git a/rule/redundant-import-alias.go b/rule/redundant-import-alias.go new file mode 100644 index 000000000..fa5281f24 --- /dev/null +++ b/rule/redundant-import-alias.go @@ -0,0 +1,52 @@ +package rule + +import ( + "fmt" + "go/ast" + "strings" + + "github.com/mgechev/revive/lint" +) + +// RedundantImportAlias lints given else constructs. +type RedundantImportAlias struct{} + +// Apply applies the rule to given file. +func (*RedundantImportAlias) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { + var failures []lint.Failure + + for _, imp := range file.AST.Imports { + if imp.Name == nil { + continue + } + + if getImportPackageName(imp) == imp.Name.Name { + failures = append(failures, lint.Failure{ + Confidence: 1, + Failure: fmt.Sprintf("Import alias \"%s\" is redundant", imp.Name.Name), + Node: imp, + Category: "imports", + }) + } + } + + return failures +} + +// Name returns the rule name. +func (*RedundantImportAlias) Name() string { + return "redundant-import-alias" +} + +func getImportPackageName(imp *ast.ImportSpec) string { + const pathSep = "/" + const strDelim = `"` + + path := imp.Path.Value + i := strings.LastIndex(path, pathSep) + if i == -1 { + return strings.Trim(path, strDelim) + } + + return strings.Trim(path[i+1:], strDelim) +} diff --git a/test/redundant-import-alias_test.go b/test/redundant-import-alias_test.go new file mode 100644 index 000000000..e7bff5faf --- /dev/null +++ b/test/redundant-import-alias_test.go @@ -0,0 +1,11 @@ +package test + +import ( + "github.com/mgechev/revive/rule" + "testing" +) + +// TestRedundantImportAlias rule. +func TestRedundantImportAlias(t *testing.T) { + testRule(t, "redundant-import-alias", &rule.RedundantImportAlias{}) +} diff --git a/testdata/redundant-import-alias.go b/testdata/redundant-import-alias.go new file mode 100644 index 000000000..555bca261 --- /dev/null +++ b/testdata/redundant-import-alias.go @@ -0,0 +1,12 @@ +package fixtures + +import( + "crypto/md5" + "strings" + _ "crypto/md5" + str "strings" + strings "strings" // MATCH /Import alias "strings" is redundant/ + crypto "crypto/md5" + md5 "crypto/md5" // MATCH /Import alias "md5" is redundant/ +) + From 8941d190268a59c3f71e1e2e6996dc8150c81e7e Mon Sep 17 00:00:00 2001 From: Fagim Sadykov Date: Mon, 31 Jul 2023 12:09:38 +0500 Subject: [PATCH 19/34] imporve `var-naming` - add upperCaseConst option to allow UPPER_CASED constants #851 (#852) * imporve `var-naming` - add upperCaseConst option to allow UPPER_CASED constants #851 Co-authored-by: chavacava --- RULES_DESCRIPTIONS.md | 8 +- rule/var-naming.go | 84 +++++++++++++-------- test/var-naming_test.go | 5 ++ testdata/var-naming_upperCaseConst-false.go | 9 +++ testdata/var-naming_upperCaseConst-true.go | 10 +++ 5 files changed, 83 insertions(+), 33 deletions(-) create mode 100644 testdata/var-naming_upperCaseConst-false.go create mode 100644 testdata/var-naming_upperCaseConst-true.go diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 4b48b526d..1709188dd 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -670,13 +670,17 @@ _Configuration_: N/A _Description_: This rule warns when [initialism](https://github.com/golang/go/wiki/CodeReviewComments#initialisms), [variable](https://github.com/golang/go/wiki/CodeReviewComments#variable-names) or [package](https://github.com/golang/go/wiki/CodeReviewComments#package-names) naming conventions are not followed. -_Configuration_: This rule accepts two slices of strings, a whitelist and a blacklist of initialisms. By default, the rule behaves exactly as the alternative in `golint` but optionally, you can relax it (see [golint/lint/issues/89](https://github.com/golang/lint/issues/89)) +_Configuration_: This rule accepts two slices of strings and one optional slice with single map with named parameters. +(it's due to TOML hasn't "slice of any" and we keep backward compatibility with previous config version) +First slice is a whitelist and second one is a blacklist of initialisms. +In map, you can add "upperCaseConst=true" parameter to allow `UPPER_CASE` for `const` +By default, the rule behaves exactly as the alternative in `golint` but optionally, you can relax it (see [golint/lint/issues/89](https://github.com/golang/lint/issues/89)) Example: ```toml [rule.var-naming] - arguments = [["ID"], ["VM"]] + arguments = [["ID"], ["VM"], [{upperCaseConst=true}]] ``` ## var-declaration diff --git a/rule/var-naming.go b/rule/var-naming.go index fa4a18864..2aa5ae2fd 100644 --- a/rule/var-naming.go +++ b/rule/var-naming.go @@ -13,11 +13,15 @@ import ( var anyCapsRE = regexp.MustCompile(`[A-Z]`) +// regexp for constant names like `SOME_CONST`, `SOME_CONST_2`, `X123_3` (#851) +var upperCaseConstRE = regexp.MustCompile(`^[A-Z][A-Z\d]*(_[A-Z\d]+)*$`) + // VarNamingRule lints given else constructs. type VarNamingRule struct { - configured bool - whitelist []string - blacklist []string + configured bool + whitelist []string + blacklist []string + upperCaseConst bool // if true - allows to use UPPER_SOME_NAMES for constants sync.Mutex } @@ -31,6 +35,23 @@ func (r *VarNamingRule) configure(arguments lint.Arguments) { if len(arguments) >= 2 { r.blacklist = getList(arguments[1], "blacklist") } + + if len(arguments) >= 3 { + // not pretty code because should keep compatibility with TOML (no mixed array types) and new map parameters + thirdArgument := arguments[2] + asSlice, ok := thirdArgument.([]interface{}) + if !ok { + panic(fmt.Sprintf("Invalid third argument to the var-naming rule. Expecting a %s of type slice, got %T", "options", arguments[2])) + } + if len(asSlice) != 1 { + panic(fmt.Sprintf("Invalid third argument to the var-naming rule. Expecting a %s of type slice, of len==1, but %d", "options", len(asSlice))) + } + args, ok := asSlice[0].(map[string]interface{}) + if !ok { + panic(fmt.Sprintf("Invalid third argument to the var-naming rule. Expecting a %s of type slice, of len==1, with map, but %T", "options", asSlice[0])) + } + r.upperCaseConst = fmt.Sprint(args["upperCaseConst"]) == "true" + } r.configured = true } r.Unlock() @@ -52,6 +73,7 @@ func (r *VarNamingRule) Apply(file *lint.File, arguments lint.Arguments) []lint. onFailure: func(failure lint.Failure) { failures = append(failures, failure) }, + upperCaseConst: r.upperCaseConst, } // Package names need slightly different handling than other names. @@ -82,18 +104,18 @@ func (*VarNamingRule) Name() string { return "var-naming" } -func checkList(fl *ast.FieldList, thing string, w *lintNames) { +func (w *lintNames) checkList(fl *ast.FieldList, thing string) { if fl == nil { return } for _, f := range fl.List { for _, id := range f.Names { - check(id, thing, w) + w.check(id, thing) } } } -func check(id *ast.Ident, thing string, w *lintNames) { +func (w *lintNames) check(id *ast.Ident, thing string) { if id.Name == "_" { return } @@ -101,6 +123,12 @@ func check(id *ast.Ident, thing string, w *lintNames) { return } + // #851 upperCaseConst support + // if it's const + if thing == token.CONST.String() && w.upperCaseConst && upperCaseConstRE.Match([]byte(id.Name)) { + return + } + // Handle two common styles from other languages that don't belong in Go. if len(id.Name) >= 5 && allCapsRE.MatchString(id.Name) && strings.Contains(id.Name, "_") { w.onFailure(lint.Failure{ @@ -144,11 +172,12 @@ func check(id *ast.Ident, thing string, w *lintNames) { } type lintNames struct { - file *lint.File - fileAst *ast.File - onFailure func(lint.Failure) - whitelist []string - blacklist []string + file *lint.File + fileAst *ast.File + onFailure func(lint.Failure) + whitelist []string + blacklist []string + upperCaseConst bool } func (w *lintNames) Visit(n ast.Node) ast.Visitor { @@ -159,7 +188,7 @@ func (w *lintNames) Visit(n ast.Node) ast.Visitor { } for _, exp := range v.Lhs { if id, ok := exp.(*ast.Ident); ok { - check(id, "var", w) + w.check(id, "var") } } case *ast.FuncDecl: @@ -181,31 +210,24 @@ func (w *lintNames) Visit(n ast.Node) ast.Visitor { // not exported in the Go API. // See https://github.com/golang/lint/issues/144. if ast.IsExported(v.Name.Name) || !isCgoExported(v) { - check(v.Name, thing, w) + w.check(v.Name, thing) } - checkList(v.Type.Params, thing+" parameter", w) - checkList(v.Type.Results, thing+" result", w) + w.checkList(v.Type.Params, thing+" parameter") + w.checkList(v.Type.Results, thing+" result") case *ast.GenDecl: if v.Tok == token.IMPORT { return w } - var thing string - switch v.Tok { - case token.CONST: - thing = "const" - case token.TYPE: - thing = "type" - case token.VAR: - thing = "var" - } + + thing := v.Tok.String() for _, spec := range v.Specs { switch s := spec.(type) { case *ast.TypeSpec: - check(s.Name, thing, w) + w.check(s.Name, thing) case *ast.ValueSpec: for _, id := range s.Names { - check(id, thing, w) + w.check(id, thing) } } } @@ -217,23 +239,23 @@ func (w *lintNames) Visit(n ast.Node) ast.Visitor { if !ok { // might be an embedded interface name continue } - checkList(ft.Params, "interface method parameter", w) - checkList(ft.Results, "interface method result", w) + w.checkList(ft.Params, "interface method parameter") + w.checkList(ft.Results, "interface method result") } case *ast.RangeStmt: if v.Tok == token.ASSIGN { return w } if id, ok := v.Key.(*ast.Ident); ok { - check(id, "range var", w) + w.check(id, "range var") } if id, ok := v.Value.(*ast.Ident); ok { - check(id, "range var", w) + w.check(id, "range var") } case *ast.StructType: for _, f := range v.Fields.List { for _, id := range f.Names { - check(id, "struct field", w) + w.check(id, "struct field") } } } diff --git a/test/var-naming_test.go b/test/var-naming_test.go index b6ccd411c..56108b056 100644 --- a/test/var-naming_test.go +++ b/test/var-naming_test.go @@ -13,4 +13,9 @@ func TestVarNaming(t *testing.T) { }) testRule(t, "var-naming_test", &rule.VarNamingRule{}, &lint.RuleConfig{}) + + testRule(t, "var-naming_upperCaseConst-false", &rule.VarNamingRule{}, &lint.RuleConfig{}) + testRule(t, "var-naming_upperCaseConst-true", &rule.VarNamingRule{}, &lint.RuleConfig{ + Arguments: []interface{}{[]interface{}{}, []interface{}{}, []interface{}{map[string]interface{}{"upperCaseConst": true}}}, + }) } diff --git a/testdata/var-naming_upperCaseConst-false.go b/testdata/var-naming_upperCaseConst-false.go new file mode 100644 index 000000000..3bb628e32 --- /dev/null +++ b/testdata/var-naming_upperCaseConst-false.go @@ -0,0 +1,9 @@ +// should fail if upperCaseConst = false (by default) #851 + +package fixtures + +const SOME_CONST_2 = 1 // MATCH /don't use ALL_CAPS in Go names; use CamelCase/ + +const ( + SOME_CONST_3 = 3 // MATCH /don't use ALL_CAPS in Go names; use CamelCase/ +) diff --git a/testdata/var-naming_upperCaseConst-true.go b/testdata/var-naming_upperCaseConst-true.go new file mode 100644 index 000000000..272b708bc --- /dev/null +++ b/testdata/var-naming_upperCaseConst-true.go @@ -0,0 +1,10 @@ +// should pass if upperCaseConst = true + +package fixtures + +const SOME_CONST_2 = 2 + +const ( + SOME_CONST_3 = 3 + VER = 0 +) From df39256ea77c2cfa014172d0c421bf300a112384 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 4 Aug 2023 11:26:37 +0200 Subject: [PATCH 20/34] fix(deps): update module golang.org/x/tools to v0.11.1 (#855) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 87fce0171..5228b32c2 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/mitchellh/go-homedir v1.1.0 github.com/olekukonko/tablewriter v0.0.5 github.com/pkg/errors v0.9.1 - golang.org/x/tools v0.11.0 + golang.org/x/tools v0.11.1 ) require ( diff --git a/go.sum b/go.sum index e05c9ea91..946dadb06 100644 --- a/go.sum +++ b/go.sum @@ -105,6 +105,8 @@ golang.org/x/tools v0.10.0 h1:tvDr/iQoUqNdohiYm0LmmKcBk+q86lb9EprIUFhHHGg= golang.org/x/tools v0.10.0/go.mod h1:UJwyiVBsOA2uwvK/e5OY3GTpDUJriEd+/YlqAwLPmyM= golang.org/x/tools v0.11.0 h1:EMCa6U9S2LtZXLAMoWiR/R8dAQFRqbAitmbJ2UKhoi8= golang.org/x/tools v0.11.0/go.mod h1:anzJrxPjNtfgiYQYirP2CPGzGLxrH2u2QBhn6Bf3qY8= +golang.org/x/tools v0.11.1 h1:ojD5zOW8+7dOGzdnNgersm8aPfcDjhMp12UfG93NIMc= +golang.org/x/tools v0.11.1/go.mod h1:anzJrxPjNtfgiYQYirP2CPGzGLxrH2u2QBhn6Bf3qY8= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= From 4c84a170af5959c7e1b55a0487e32b5708394949 Mon Sep 17 00:00:00 2001 From: Denis Voytyuk <5462781+denisvmedia@users.noreply.github.com> Date: Wed, 9 Aug 2023 22:37:50 +0200 Subject: [PATCH 21/34] Allow import-blacklist to run against go test files (#862) --- rule/imports-blacklist.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/rule/imports-blacklist.go b/rule/imports-blacklist.go index 710662815..bb8262cae 100644 --- a/rule/imports-blacklist.go +++ b/rule/imports-blacklist.go @@ -52,10 +52,6 @@ func (r *ImportsBlacklistRule) Apply(file *lint.File, arguments lint.Arguments) var failures []lint.Failure - if file.IsTest() { - return failures // skip, test file - } - for _, is := range file.AST.Imports { path := is.Path if path != nil && r.isBlacklisted(path.Value) { From b31eb18b1fe3e8e2de8cfdcc6af3aad90ded9ba6 Mon Sep 17 00:00:00 2001 From: Fagim Sadykov Date: Fri, 11 Aug 2023 10:35:08 +0500 Subject: [PATCH 22/34] adds [allowRegex] parameter for `unused-parameter` and `unused-receiver` rules (#858) --- RULES_DESCRIPTIONS.md | 27 +++++++++- rule/unused-param.go | 67 +++++++++++++++++++++--- rule/unused-receiver.go | 66 +++++++++++++++++++++-- test/unused-param_test.go | 4 ++ test/unused-receiver_test.go | 4 ++ testdata/unused-param-custom-regex.go | 12 +++++ testdata/unused-receiver-custom-regex.go | 12 +++++ 7 files changed, 179 insertions(+), 13 deletions(-) create mode 100644 testdata/unused-param-custom-regex.go create mode 100644 testdata/unused-receiver-custom-regex.go diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 1709188dd..3d78110ce 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -735,13 +735,36 @@ _Configuration_: N/A _Description_: This rule warns on unused parameters. Functions or methods with unused parameters can be a symptom of an unfinished refactoring or a bug. -_Configuration_: N/A +_Configuration_: Supports arguments with single of `map[string]any` with option `allowRegex` to provide additional to `_` mask to allowed unused parameter names, for example: + +```toml +[rule.unused-parameter] + Arguments = [ { allowRegex = "^_" } ] +``` + +allows any names started with `_`, not just `_` itself: + +```go +func SomeFunc(_someObj *MyStruct) {} // matches rule +``` ## unused-receiver _Description_: This rule warns on unused method receivers. Methods with unused receivers can be a symptom of an unfinished refactoring or a bug. -_Configuration_: N/A +_Configuration_: Supports arguments with single of `map[string]any` with option `allowRegex` to provide additional to `_` mask to allowed unused receiver names, for example: + +```toml +[rule.unused-receiver] + Arguments = [ { allowRegex = "^_" } ] +``` + +allows any names started with `_`, not just `_` itself: + +```go +func (_my *MyStruct) SomeMethod() {} // matches rule +``` + ## use-any diff --git a/rule/unused-param.go b/rule/unused-param.go index ab3da453e..df6cd9af0 100644 --- a/rule/unused-param.go +++ b/rule/unused-param.go @@ -3,22 +3,72 @@ package rule import ( "fmt" "go/ast" + "regexp" + "sync" "github.com/mgechev/revive/lint" ) // UnusedParamRule lints unused params in functions. -type UnusedParamRule struct{} +type UnusedParamRule struct { + configured bool + // regex to check if some name is valid for unused parameter, "^_$" by default + allowRegex *regexp.Regexp + failureMsg string + sync.Mutex +} + +func (r *UnusedParamRule) configure(args lint.Arguments) { + r.Lock() + defer r.Unlock() + + if r.configured { + return + } + r.configured = true + + // while by default args is an array, i think it's good to provide structures inside it by default, not arrays or primitives + // it's more compatible to JSON nature of configurations + var allowedRegexStr string + if len(args) == 0 { + allowedRegexStr = "^_$" + r.failureMsg = "parameter '%s' seems to be unused, consider removing or renaming it as _" + } else { + // Arguments = [{}] + options := args[0].(map[string]interface{}) + // Arguments = [{allowedRegex="^_"}] + + if allowedRegexParam, ok := options["allowRegex"]; ok { + allowedRegexStr, ok = allowedRegexParam.(string) + if !ok { + panic(fmt.Errorf("error configuring %s rule: allowedRegex is not string but [%T]", r.Name(), allowedRegexParam)) + } + } + } + var err error + r.allowRegex, err = regexp.Compile(allowedRegexStr) + if err != nil { + panic(fmt.Errorf("error configuring %s rule: allowedRegex is not valid regex [%s]: %v", r.Name(), allowedRegexStr, err)) + } + + if r.failureMsg == "" { + r.failureMsg = "parameter '%s' seems to be unused, consider removing or renaming it to match " + r.allowRegex.String() + } +} // Apply applies the rule to given file. -func (*UnusedParamRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (r *UnusedParamRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure { + r.configure(args) var failures []lint.Failure onFailure := func(failure lint.Failure) { failures = append(failures, failure) } - - w := lintUnusedParamRule{onFailure: onFailure} + w := lintUnusedParamRule{ + onFailure: onFailure, + allowRegex: r.allowRegex, + failureMsg: r.failureMsg, + } ast.Walk(w, file.AST) @@ -31,7 +81,9 @@ func (*UnusedParamRule) Name() string { } type lintUnusedParamRule struct { - onFailure func(lint.Failure) + onFailure func(lint.Failure) + allowRegex *regexp.Regexp + failureMsg string } func (w lintUnusedParamRule) Visit(node ast.Node) ast.Visitor { @@ -65,12 +117,15 @@ func (w lintUnusedParamRule) Visit(node ast.Node) ast.Visitor { for _, p := range n.Type.Params.List { for _, n := range p.Names { + if w.allowRegex.FindStringIndex(n.Name) != nil { + continue + } if params[n.Obj] { w.onFailure(lint.Failure{ Confidence: 1, Node: n, Category: "bad practice", - Failure: fmt.Sprintf("parameter '%s' seems to be unused, consider removing or renaming it as _", n.Name), + Failure: fmt.Sprintf(w.failureMsg, n.Name), }) } } diff --git a/rule/unused-receiver.go b/rule/unused-receiver.go index 2289a517e..488572b7b 100644 --- a/rule/unused-receiver.go +++ b/rule/unused-receiver.go @@ -3,22 +3,72 @@ package rule import ( "fmt" "go/ast" + "regexp" + "sync" "github.com/mgechev/revive/lint" ) // UnusedReceiverRule lints unused params in functions. -type UnusedReceiverRule struct{} +type UnusedReceiverRule struct { + configured bool + // regex to check if some name is valid for unused parameter, "^_$" by default + allowRegex *regexp.Regexp + failureMsg string + sync.Mutex +} + +func (r *UnusedReceiverRule) configure(args lint.Arguments) { + r.Lock() + defer r.Unlock() + + if r.configured { + return + } + r.configured = true + + // while by default args is an array, i think it's good to provide structures inside it by default, not arrays or primitives + // it's more compatible to JSON nature of configurations + var allowedRegexStr string + if len(args) == 0 { + allowedRegexStr = "^_$" + r.failureMsg = "method receiver '%s' is not referenced in method's body, consider removing or renaming it as _" + } else { + // Arguments = [{}] + options := args[0].(map[string]interface{}) + // Arguments = [{allowedRegex="^_"}] + + if allowedRegexParam, ok := options["allowRegex"]; ok { + allowedRegexStr, ok = allowedRegexParam.(string) + if !ok { + panic(fmt.Errorf("error configuring [unused-receiver] rule: allowedRegex is not string but [%T]", allowedRegexParam)) + } + } + } + var err error + r.allowRegex, err = regexp.Compile(allowedRegexStr) + if err != nil { + panic(fmt.Errorf("error configuring [unused-receiver] rule: allowedRegex is not valid regex [%s]: %v", allowedRegexStr, err)) + } + if r.failureMsg == "" { + r.failureMsg = "method receiver '%s' is not referenced in method's body, consider removing or renaming it to match " + r.allowRegex.String() + } +} // Apply applies the rule to given file. -func (*UnusedReceiverRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { +func (r *UnusedReceiverRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure { + r.configure(args) var failures []lint.Failure onFailure := func(failure lint.Failure) { failures = append(failures, failure) } - w := lintUnusedReceiverRule{onFailure: onFailure} + w := lintUnusedReceiverRule{ + onFailure: onFailure, + allowRegex: r.allowRegex, + failureMsg: r.failureMsg, + } ast.Walk(w, file.AST) @@ -31,7 +81,9 @@ func (*UnusedReceiverRule) Name() string { } type lintUnusedReceiverRule struct { - onFailure func(lint.Failure) + onFailure func(lint.Failure) + allowRegex *regexp.Regexp + failureMsg string } func (w lintUnusedReceiverRule) Visit(node ast.Node) ast.Visitor { @@ -51,6 +103,10 @@ func (w lintUnusedReceiverRule) Visit(node ast.Node) ast.Visitor { return nil // the receiver is already named _ } + if w.allowRegex != nil && w.allowRegex.FindStringIndex(recID.Name) != nil { + return nil + } + // inspect the func body looking for references to the receiver id fselect := func(n ast.Node) bool { ident, isAnID := n.(*ast.Ident) @@ -67,7 +123,7 @@ func (w lintUnusedReceiverRule) Visit(node ast.Node) ast.Visitor { Confidence: 1, Node: recID, Category: "bad practice", - Failure: fmt.Sprintf("method receiver '%s' is not referenced in method's body, consider removing or renaming it as _", recID.Name), + Failure: fmt.Sprintf(w.failureMsg, recID.Name), }) return nil // full method body already inspected diff --git a/test/unused-param_test.go b/test/unused-param_test.go index 7b6472cef..8be149487 100644 --- a/test/unused-param_test.go +++ b/test/unused-param_test.go @@ -3,11 +3,15 @@ package test import ( "testing" + "github.com/mgechev/revive/lint" "github.com/mgechev/revive/rule" ) func TestUnusedParam(t *testing.T) { testRule(t, "unused-param", &rule.UnusedParamRule{}) + testRule(t, "unused-param-custom-regex", &rule.UnusedParamRule{}, &lint.RuleConfig{Arguments: []interface{}{ + map[string]interface{}{"allowRegex": "^xxx"}, + }}) } func BenchmarkUnusedParam(b *testing.B) { diff --git a/test/unused-receiver_test.go b/test/unused-receiver_test.go index e4bae7c3e..2fb6a0e85 100644 --- a/test/unused-receiver_test.go +++ b/test/unused-receiver_test.go @@ -3,9 +3,13 @@ package test import ( "testing" + "github.com/mgechev/revive/lint" "github.com/mgechev/revive/rule" ) func TestUnusedReceiver(t *testing.T) { testRule(t, "unused-receiver", &rule.UnusedReceiverRule{}) + testRule(t, "unused-receiver-custom-regex", &rule.UnusedReceiverRule{}, &lint.RuleConfig{Arguments: []interface{}{ + map[string]interface{}{"allowRegex": "^xxx"}, + }}) } diff --git a/testdata/unused-param-custom-regex.go b/testdata/unused-param-custom-regex.go new file mode 100644 index 000000000..24612aeaf --- /dev/null +++ b/testdata/unused-param-custom-regex.go @@ -0,0 +1,12 @@ +package fixtures + +// all will ok with xxxParam if Arguments = [{allowRegex="^xxx"}] + +func f0(xxxParam int) {} + +// still works with _ + +func f1(_ int) {} + +func f2(yyyParam int) { // MATCH /parameter 'yyyParam' seems to be unused, consider removing or renaming it to match ^xxx/ +} diff --git a/testdata/unused-receiver-custom-regex.go b/testdata/unused-receiver-custom-regex.go new file mode 100644 index 000000000..3fdcd4a1d --- /dev/null +++ b/testdata/unused-receiver-custom-regex.go @@ -0,0 +1,12 @@ +package fixtures + +// all will ok with xxxParam if Arguments = [{allowRegex="^xxx"}] + +func (xxxParam *SomeObj) f0() {} + +// still works with _ + +func (_ *SomeObj) f1() {} + +func (yyyParam *SomeObj) f2() { // MATCH /method receiver 'yyyParam' is not referenced in method's body, consider removing or renaming it to match ^xxx/ +} From b4fc3db69235d2ce2a394bbd84f9f8f0ff937e7d Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Fri, 11 Aug 2023 07:36:18 +0200 Subject: [PATCH 23/34] fix(deps): update module golang.org/x/tools to v0.12.0 (#859) --- go.mod | 4 ++-- go.sum | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index 5228b32c2..a7e990c9d 100644 --- a/go.mod +++ b/go.mod @@ -11,12 +11,12 @@ require ( github.com/mitchellh/go-homedir v1.1.0 github.com/olekukonko/tablewriter v0.0.5 github.com/pkg/errors v0.9.1 - golang.org/x/tools v0.11.1 + golang.org/x/tools v0.12.0 ) require ( github.com/mattn/go-colorable v0.1.13 // indirect github.com/mattn/go-isatty v0.0.17 // indirect github.com/mattn/go-runewidth v0.0.9 // indirect - golang.org/x/sys v0.10.0 // indirect + golang.org/x/sys v0.11.0 // indirect ) diff --git a/go.sum b/go.sum index 946dadb06..f8603b66c 100644 --- a/go.sum +++ b/go.sum @@ -83,6 +83,8 @@ golang.org/x/sys v0.9.0 h1:KS/R3tvhPqvJvwcKfnBHJwwthS11LRhmM5D59eEXa0s= golang.org/x/sys v0.9.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.10.0 h1:SqMFp9UcQJZa+pmYuAKjd9xq1f0j5rLcDIk0mj4qAsA= golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM= +golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/tools v0.1.12 h1:VveCTK38A2rkS8ZqFY25HIDFscX5X9OoEhJd3quQmXU= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.2.0 h1:G6AHpWxTMGY1KyEYoAQ5WTtIekUUvDNjan3ugu60JvE= @@ -107,6 +109,8 @@ golang.org/x/tools v0.11.0 h1:EMCa6U9S2LtZXLAMoWiR/R8dAQFRqbAitmbJ2UKhoi8= golang.org/x/tools v0.11.0/go.mod h1:anzJrxPjNtfgiYQYirP2CPGzGLxrH2u2QBhn6Bf3qY8= golang.org/x/tools v0.11.1 h1:ojD5zOW8+7dOGzdnNgersm8aPfcDjhMp12UfG93NIMc= golang.org/x/tools v0.11.1/go.mod h1:anzJrxPjNtfgiYQYirP2CPGzGLxrH2u2QBhn6Bf3qY8= +golang.org/x/tools v0.12.0 h1:YW6HUoUmYBpwSgyaGaZq1fHjrBjX1rlpZ54T6mu2kss= +golang.org/x/tools v0.12.0/go.mod h1:Sc0INKfu04TlqNoRA1hgpFZbhYXHPr4V5DzpSBTPqQM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= From 310d1d76e49e3f6b86cb992e33ddac0a8aab9805 Mon Sep 17 00:00:00 2001 From: Fagim Sadykov Date: Sat, 12 Aug 2023 11:21:11 +0500 Subject: [PATCH 24/34] per-rule file exclude filters (#850) (#857) --- README.md | 31 +++++ config/config.go | 8 ++ config/config_test.go | 21 ++++ config/testdata/rule-level-exclude-850.toml | 13 ++ lint/config.go | 28 +++++ lint/file.go | 3 + lint/filefilter.go | 128 ++++++++++++++++++++ lint/filefilter_test.go | 128 ++++++++++++++++++++ test/file-filter_test.go | 48 ++++++++ testdata/file-to-exclude.go | 2 + 10 files changed, 410 insertions(+) create mode 100644 config/testdata/rule-level-exclude-850.toml create mode 100644 lint/filefilter.go create mode 100644 lint/filefilter_test.go create mode 100644 test/file-filter_test.go create mode 100644 testdata/file-to-exclude.go diff --git a/README.md b/README.md index 43800ae98..2bdb47e9c 100644 --- a/README.md +++ b/README.md @@ -425,6 +425,37 @@ warningCode = 0 [rule.redefines-builtin-id] ``` +### Rule-level file excludes + +You also can setup custom excludes for each rule. + +It's alternative for global `-exclude` program arg. + +```toml +ignoreGeneratedHeader = false +severity = "warning" +confidence = 0.8 +errorCode = 0 +warningCode = 0 + +[rule.blank-imports] + Exclude=["**/*.pb.go"] +[rule.context-as-argument] + Exclude=["src/somepkg/*.go", "TEST"] +``` + +You can use following exclude patterns + +1. full paths to files `src/pkg/mypkg/some.go` +2. globs `src/**/*.pb.go` +3. regexes (should have prefix ~) `~\.(pb|auto|generated)\.go$` +4. well-known `TEST` (same as `**/*_test.go`) +5. special cases: + a. `*` and `~` patterns exclude all files (same effect than disabling the rule) + b. `""` (empty) pattern excludes nothing + +> NOTE: do not mess with `exclude` that can be used at top level of TOML file, that mean "exclude package patterns", not "exclude file patterns" + ## Available Rules List of all available rules. The rules ported from `golint` are left unchanged and indicated in the `golint` column. diff --git a/config/config.go b/config/config.go index 225a570c8..7643d2752 100644 --- a/config/config.go +++ b/config/config.go @@ -149,6 +149,14 @@ func parseConfig(path string, config *lint.Config) error { if err != nil { return fmt.Errorf("cannot parse the config file: %v", err) } + for k, r := range config.Rules { + err := r.Initialize() + if err != nil { + return fmt.Errorf("error in config of rule [%s] : [%v]", k, err) + } + config.Rules[k] = r + } + return nil } diff --git a/config/config_test.go b/config/config_test.go index 57b7a973c..42dff4b1f 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -61,6 +61,27 @@ func TestGetConfig(t *testing.T) { } }) } + + t.Run("rule-level file filter excludes", func(t *testing.T) { + cfg, err := GetConfig("testdata/rule-level-exclude-850.toml") + if err != nil { + t.Fatal("should be valid config") + } + r1 := cfg.Rules["r1"] + if len(r1.Exclude) > 0 { + t.Fatal("r1 should have empty excludes") + } + r2 := cfg.Rules["r2"] + if len(r2.Exclude) != 1 { + t.Fatal("r2 should have exclude set") + } + if !r2.MustExclude("some/file.go") { + t.Fatal("r2 should be initialized and exclude some/file.go") + } + if r2.MustExclude("some/any-other.go") { + t.Fatal("r2 should not exclude some/any-other.go") + } + }) } func TestGetLintingRules(t *testing.T) { diff --git a/config/testdata/rule-level-exclude-850.toml b/config/testdata/rule-level-exclude-850.toml new file mode 100644 index 000000000..97523e8e5 --- /dev/null +++ b/config/testdata/rule-level-exclude-850.toml @@ -0,0 +1,13 @@ +ignoreGeneratedHeader = false +severity = "warning" +confidence = 0.8 +errorCode = 0 +warningCode = 0 + +enableAllRules = false + +[rule.r1] + # no excludes + +[rule.r2] + exclude=["some/file.go"] \ No newline at end of file diff --git a/lint/config.go b/lint/config.go index 276305804..9b26d5841 100644 --- a/lint/config.go +++ b/lint/config.go @@ -3,16 +3,44 @@ package lint // Arguments is type used for the arguments of a rule. type Arguments = []interface{} +type FileFilters = []*FileFilter + // RuleConfig is type used for the rule configuration. type RuleConfig struct { Arguments Arguments Severity Severity Disabled bool + // Exclude - rule-level file excludes, TOML related (strings) + Exclude []string + // excludeFilters - regex-based file filters, initialized from Exclude + excludeFilters []*FileFilter +} + +// Initialize - should be called after reading from TOML file +func (rc *RuleConfig) Initialize() error { + for _, f := range rc.Exclude { + ff, err := ParseFileFilter(f) + if err != nil { + return err + } + rc.excludeFilters = append(rc.excludeFilters, ff) + } + return nil } // RulesConfig defines the config for all rules. type RulesConfig = map[string]RuleConfig +// MustExclude - checks if given filename `name` must be excluded +func (rcfg *RuleConfig) MustExclude(name string) bool { + for _, exclude := range rcfg.excludeFilters { + if exclude.MatchFileName(name) { + return true + } + } + return false +} + // DirectiveConfig is type used for the linter directive configuration. type DirectiveConfig struct { Severity Severity diff --git a/lint/file.go b/lint/file.go index dcf0e608f..23255304c 100644 --- a/lint/file.go +++ b/lint/file.go @@ -102,6 +102,9 @@ func (f *File) lint(rules []Rule, config Config, failures chan Failure) { disabledIntervals := f.disabledIntervals(rules, mustSpecifyDisableReason, failures) for _, currentRule := range rules { ruleConfig := rulesConfig[currentRule.Name()] + if ruleConfig.MustExclude(f.Name) { + continue + } currentFailures := currentRule.Apply(f, ruleConfig.Arguments) for idx, failure := range currentFailures { if failure.RuleName == "" { diff --git a/lint/filefilter.go b/lint/filefilter.go new file mode 100644 index 000000000..8da090b9c --- /dev/null +++ b/lint/filefilter.go @@ -0,0 +1,128 @@ +package lint + +import ( + "fmt" + "regexp" + "strings" +) + +// FileFilter - file filter to exclude some files for rule +// supports whole +// 1. file/dir names : pkg/mypkg/my.go, +// 2. globs: **/*.pb.go, +// 3. regexes (~ prefix) ~-tmp\.\d+\.go +// 4. special test marker `TEST` - treats as `~_test\.go` +type FileFilter struct { + // raw definition of filter inside config + raw string + // don't care what was at start, will use regexes inside + rx *regexp.Regexp + // marks filter as matching everything + matchesAll bool + // marks filter as matching nothing + matchesNothing bool +} + +// ParseFileFilter - creates [FileFilter] for given raw filter +// if empty string, it matches nothing +// if `*`, or `~`, it matches everything +// while regexp could be invalid, it could return it's compilation error +func ParseFileFilter(rawFilter string) (*FileFilter, error) { + rawFilter = strings.TrimSpace(rawFilter) + result := new(FileFilter) + result.raw = rawFilter + result.matchesNothing = len(result.raw) == 0 + result.matchesAll = result.raw == "*" || result.raw == "~" + if !result.matchesAll && !result.matchesNothing { + if err := result.prepareRegexp(); err != nil { + return nil, err + } + } + return result, nil +} + +func (ff *FileFilter) String() string { return ff.raw } + +// MatchFileName - checks if file name matches filter +func (ff *FileFilter) MatchFileName(name string) bool { + if ff.matchesAll { + return true + } + if ff.matchesNothing { + return false + } + name = strings.ReplaceAll(name, "\\", "/") + return ff.rx.MatchString(name) +} + +var fileFilterInvalidGlobRegexp = regexp.MustCompile(`[^/]\*\*[^/]`) +var escapeRegexSymbols = ".+{}()[]^$" + +func (ff *FileFilter) prepareRegexp() error { + var err error + var src = ff.raw + if src == "TEST" { + src = "~_test\\.go" + } + if strings.HasPrefix(src, "~") { + ff.rx, err = regexp.Compile(src[1:]) + if err != nil { + return fmt.Errorf("invalid file filter [%s], regexp compile error: [%v]", ff.raw, err) + } + return nil + } + /* globs */ + if strings.Contains(src, "*") { + if fileFilterInvalidGlobRegexp.MatchString(src) { + return fmt.Errorf("invalid file filter [%s], invalid glob pattern", ff.raw) + } + var rxBuild strings.Builder + rxBuild.WriteByte('^') + wasStar := false + justDirGlob := false + for _, c := range src { + if c == '*' { + if wasStar { + rxBuild.WriteString(`[\s\S]*`) + wasStar = false + justDirGlob = true + continue + } + wasStar = true + continue + } + if wasStar { + rxBuild.WriteString("[^/]*") + wasStar = false + } + if strings.ContainsRune(escapeRegexSymbols, c) { + rxBuild.WriteByte('\\') + } + rxBuild.WriteRune(c) + if c == '/' && justDirGlob { + rxBuild.WriteRune('?') + } + justDirGlob = false + } + if wasStar { + rxBuild.WriteString("[^/]*") + } + rxBuild.WriteByte('$') + ff.rx, err = regexp.Compile(rxBuild.String()) + if err != nil { + return fmt.Errorf("invalid file filter [%s], regexp compile error after glob expand: [%v]", ff.raw, err) + } + return nil + } + + // it's whole file mask, just escape dots and normilze separators + fillRx := src + fillRx = strings.ReplaceAll(fillRx, "\\", "/") + fillRx = strings.ReplaceAll(fillRx, ".", `\.`) + fillRx = "^" + fillRx + "$" + ff.rx, err = regexp.Compile(fillRx) + if err != nil { + return fmt.Errorf("invalid file filter [%s], regexp compile full path: [%v]", ff.raw, err) + } + return nil +} diff --git a/lint/filefilter_test.go b/lint/filefilter_test.go new file mode 100644 index 000000000..6c20afdcb --- /dev/null +++ b/lint/filefilter_test.go @@ -0,0 +1,128 @@ +package lint_test + +import ( + "testing" + + "github.com/mgechev/revive/lint" +) + +func TestFileFilter(t *testing.T) { + t.Run("whole file name", func(t *testing.T) { + ff, err := lint.ParseFileFilter("a/b/c.go") + if err != nil { + t.Fatal(err) + } + if !ff.MatchFileName("a/b/c.go") { + t.Fatal("should match a/b/c.go") + } + if ff.MatchFileName("a/b/d.go") { + t.Fatal("should not match") + } + }) + + t.Run("regex", func(t *testing.T) { + ff, err := lint.ParseFileFilter("~b/[cd].go$") + if err != nil { + t.Fatal(err) + } + if !ff.MatchFileName("a/b/c.go") { + t.Fatal("should match a/b/c.go") + } + if !ff.MatchFileName("b/d.go") { + t.Fatal("should match b/d.go") + } + if ff.MatchFileName("b/x.go") { + t.Fatal("should not match b/x.go") + } + }) + + t.Run("TEST well-known", func(t *testing.T) { + ff, err := lint.ParseFileFilter("TEST") + if err != nil { + t.Fatal(err) + } + if !ff.MatchFileName("a/b/c_test.go") { + t.Fatal("should match a/b/c_test.go") + } + if ff.MatchFileName("a/b/c_test_no.go") { + t.Fatal("should not match a/b/c_test_no.go") + } + }) + + t.Run("glob *", func(t *testing.T) { + ff, err := lint.ParseFileFilter("a/b/*.pb.go") + if err != nil { + t.Fatal(err) + } + if !ff.MatchFileName("a/b/xxx.pb.go") { + t.Fatal("should match a/b/xxx.pb.go") + } + if !ff.MatchFileName("a/b/yyy.pb.go") { + t.Fatal("should match a/b/yyy.pb.go") + } + if ff.MatchFileName("a/b/xxx.nopb.go") { + t.Fatal("should not match a/b/xxx.nopb.go") + } + }) + + t.Run("glob **", func(t *testing.T) { + ff, err := lint.ParseFileFilter("a/**/*.pb.go") + if err != nil { + t.Fatal(err) + } + if !ff.MatchFileName("a/x/xxx.pb.go") { + t.Fatal("should match a/x/xxx.pb.go") + } + if !ff.MatchFileName("a/xxx.pb.go") { + t.Fatal("should match a/xxx.pb.go") + } + if !ff.MatchFileName("a/x/y/z/yyy.pb.go") { + t.Fatal("should match a/x/y/z/yyy.pb.go") + } + if ff.MatchFileName("a/b/xxx.nopb.go") { + t.Fatal("should not match a/b/xxx.nopb.go") + } + }) + + t.Run("empty", func(t *testing.T) { + ff, err := lint.ParseFileFilter("") + if err != nil { + t.Fatal(err) + } + fileNames := []string{"pb.go", "a/pb.go", "a/x/xxx.pb.go", "a/x/xxx.pb_test.go"} + for _, fn := range fileNames { + if ff.MatchFileName(fn) { + t.Fatalf("should not match %s", fn) + } + } + + }) + + t.Run("just *", func(t *testing.T) { + ff, err := lint.ParseFileFilter("*") + if err != nil { + t.Fatal(err) + } + fileNames := []string{"pb.go", "a/pb.go", "a/x/xxx.pb.go", "a/x/xxx.pb_test.go"} + for _, fn := range fileNames { + if !ff.MatchFileName(fn) { + t.Fatalf("should match %s", fn) + } + } + + }) + + t.Run("just ~", func(t *testing.T) { + ff, err := lint.ParseFileFilter("~") + if err != nil { + t.Fatal(err) + } + fileNames := []string{"pb.go", "a/pb.go", "a/x/xxx.pb.go", "a/x/xxx.pb_test.go"} + for _, fn := range fileNames { + if !ff.MatchFileName(fn) { + t.Fatalf("should match %s", fn) + } + } + + }) +} diff --git a/test/file-filter_test.go b/test/file-filter_test.go new file mode 100644 index 000000000..7fce3b092 --- /dev/null +++ b/test/file-filter_test.go @@ -0,0 +1,48 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/lint" +) + +type TestFileFilterRule struct { + WasApplyed bool +} + +var _ lint.Rule = (*TestFileFilterRule)(nil) + +func (*TestFileFilterRule) Name() string { return "test-file-filter" } +func (tfr *TestFileFilterRule) Apply(*lint.File, lint.Arguments) []lint.Failure { + tfr.WasApplyed = true + return nil +} + +func TestFileExcludeFilterAtRuleLevel(t *testing.T) { + t.Run("is called if no excludes", func(t *testing.T) { + rule := &TestFileFilterRule{} + testRule(t, "file-to-exclude", rule, &lint.RuleConfig{}) + if !rule.WasApplyed { + t.Fatal("should call rule if no excludes") + } + }) + t.Run("is called if exclude not match", func(t *testing.T) { + rule := &TestFileFilterRule{} + cfg := &lint.RuleConfig{Exclude: []string{"no-matched.go"}} + cfg.Initialize() + testRule(t, "file-to-exclude", rule, cfg) + if !rule.WasApplyed { + t.Fatal("should call rule if no excludes") + } + }) + + t.Run("not called if exclude not match", func(t *testing.T) { + rule := &TestFileFilterRule{} + cfg := &lint.RuleConfig{Exclude: []string{"file-to-exclude.go"}} + cfg.Initialize() + testRule(t, "file-to-exclude", rule, cfg) + if rule.WasApplyed { + t.Fatal("should not call rule if excluded") + } + }) +} diff --git a/testdata/file-to-exclude.go b/testdata/file-to-exclude.go new file mode 100644 index 000000000..58283be64 --- /dev/null +++ b/testdata/file-to-exclude.go @@ -0,0 +1,2 @@ +// just to check FileFilter +package fixtures From 72f9108792bd94f87904dec7b58087f436200285 Mon Sep 17 00:00:00 2001 From: fregin Date: Sat, 12 Aug 2023 13:45:42 +0700 Subject: [PATCH 25/34] (var-naming) support private uppercase constants #865 (#866) --- rule/var-naming.go | 54 +++++++++++---------- testdata/var-naming_upperCaseConst-false.go | 8 +-- testdata/var-naming_upperCaseConst-true.go | 6 ++- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/rule/var-naming.go b/rule/var-naming.go index 2aa5ae2fd..c6a6305be 100644 --- a/rule/var-naming.go +++ b/rule/var-naming.go @@ -13,8 +13,8 @@ import ( var anyCapsRE = regexp.MustCompile(`[A-Z]`) -// regexp for constant names like `SOME_CONST`, `SOME_CONST_2`, `X123_3` (#851) -var upperCaseConstRE = regexp.MustCompile(`^[A-Z][A-Z\d]*(_[A-Z\d]+)*$`) +// regexp for constant names like `SOME_CONST`, `SOME_CONST_2`, `X123_3`, `_SOME_PRIVATE_CONST` (#851, #865) +var upperCaseConstRE = regexp.MustCompile(`^_?[A-Z][A-Z\d]*(_[A-Z\d]+)*$`) // VarNamingRule lints given else constructs. type VarNamingRule struct { @@ -27,34 +27,36 @@ type VarNamingRule struct { func (r *VarNamingRule) configure(arguments lint.Arguments) { r.Lock() - if !r.configured { - if len(arguments) >= 1 { - r.whitelist = getList(arguments[0], "whitelist") - } + defer r.Unlock() + if r.configured { + return + } - if len(arguments) >= 2 { - r.blacklist = getList(arguments[1], "blacklist") - } + r.configured = true + if len(arguments) >= 1 { + r.whitelist = getList(arguments[0], "whitelist") + } - if len(arguments) >= 3 { - // not pretty code because should keep compatibility with TOML (no mixed array types) and new map parameters - thirdArgument := arguments[2] - asSlice, ok := thirdArgument.([]interface{}) - if !ok { - panic(fmt.Sprintf("Invalid third argument to the var-naming rule. Expecting a %s of type slice, got %T", "options", arguments[2])) - } - if len(asSlice) != 1 { - panic(fmt.Sprintf("Invalid third argument to the var-naming rule. Expecting a %s of type slice, of len==1, but %d", "options", len(asSlice))) - } - args, ok := asSlice[0].(map[string]interface{}) - if !ok { - panic(fmt.Sprintf("Invalid third argument to the var-naming rule. Expecting a %s of type slice, of len==1, with map, but %T", "options", asSlice[0])) - } - r.upperCaseConst = fmt.Sprint(args["upperCaseConst"]) == "true" + if len(arguments) >= 2 { + r.blacklist = getList(arguments[1], "blacklist") + } + + if len(arguments) >= 3 { + // not pretty code because should keep compatibility with TOML (no mixed array types) and new map parameters + thirdArgument := arguments[2] + asSlice, ok := thirdArgument.([]interface{}) + if !ok { + panic(fmt.Sprintf("Invalid third argument to the var-naming rule. Expecting a %s of type slice, got %T", "options", arguments[2])) + } + if len(asSlice) != 1 { + panic(fmt.Sprintf("Invalid third argument to the var-naming rule. Expecting a %s of type slice, of len==1, but %d", "options", len(asSlice))) + } + args, ok := asSlice[0].(map[string]interface{}) + if !ok { + panic(fmt.Sprintf("Invalid third argument to the var-naming rule. Expecting a %s of type slice, of len==1, with map, but %T", "options", asSlice[0])) } - r.configured = true + r.upperCaseConst = fmt.Sprint(args["upperCaseConst"]) == "true" } - r.Unlock() } // Apply applies the rule to given file. diff --git a/testdata/var-naming_upperCaseConst-false.go b/testdata/var-naming_upperCaseConst-false.go index 3bb628e32..4f56a297b 100644 --- a/testdata/var-naming_upperCaseConst-false.go +++ b/testdata/var-naming_upperCaseConst-false.go @@ -1,9 +1,11 @@ -// should fail if upperCaseConst = false (by default) #851 +// should fail if upperCaseConst = false (by default) #851, #865 package fixtures -const SOME_CONST_2 = 1 // MATCH /don't use ALL_CAPS in Go names; use CamelCase/ +const SOME_CONST_2 = 1 // MATCH /don't use ALL_CAPS in Go names; use CamelCase/ +const _SOME_PRIVATE_CONST_2 = 2 // MATCH /don't use ALL_CAPS in Go names; use CamelCase/ const ( - SOME_CONST_3 = 3 // MATCH /don't use ALL_CAPS in Go names; use CamelCase/ + SOME_CONST_3 = 3 // MATCH /don't use ALL_CAPS in Go names; use CamelCase/ + _SOME_PRIVATE_CONST_3 = 3 // MATCH /don't use ALL_CAPS in Go names; use CamelCase/ ) diff --git a/testdata/var-naming_upperCaseConst-true.go b/testdata/var-naming_upperCaseConst-true.go index 272b708bc..b459923a0 100644 --- a/testdata/var-naming_upperCaseConst-true.go +++ b/testdata/var-naming_upperCaseConst-true.go @@ -3,8 +3,10 @@ package fixtures const SOME_CONST_2 = 2 +const _SOME_PRIVATE_CONST_2 = 2 const ( - SOME_CONST_3 = 3 - VER = 0 + SOME_CONST_3 = 3 + _SOME_PRIVATE_CONST_3 = 3 + VER = 0 ) From 7cb4540a46f19f7da7a1cb9cf6ab661aec3e0d06 Mon Sep 17 00:00:00 2001 From: chavacava Date: Sat, 12 Aug 2023 09:05:46 +0200 Subject: [PATCH 26/34] fix #846: time-equal garbled message when time returned from function (#868) --- rule/time-equal.go | 4 ++-- testdata/time-equal.go | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/rule/time-equal.go b/rule/time-equal.go index 72ecf26fe..3b85e18a8 100644 --- a/rule/time-equal.go +++ b/rule/time-equal.go @@ -60,9 +60,9 @@ func (l *lintTimeEqual) Visit(node ast.Node) ast.Visitor { var failure string switch expr.Op { case token.EQL: - failure = fmt.Sprintf("use %s.Equal(%s) instead of %q operator", expr.X, expr.Y, expr.Op) + failure = fmt.Sprintf("use %s.Equal(%s) instead of %q operator", gofmt(expr.X), gofmt(expr.Y), expr.Op) case token.NEQ: - failure = fmt.Sprintf("use !%s.Equal(%s) instead of %q operator", expr.X, expr.Y, expr.Op) + failure = fmt.Sprintf("use !%s.Equal(%s) instead of %q operator", gofmt(expr.X), gofmt(expr.Y), expr.Op) } l.onFailure(lint.Failure{ diff --git a/testdata/time-equal.go b/testdata/time-equal.go index be72c4e5d..f31a05a50 100644 --- a/testdata/time-equal.go +++ b/testdata/time-equal.go @@ -12,3 +12,7 @@ func t() bool { return t != u // MATCH /use !t.Equal(u) instead of "!=" operator/ } + +// issue #846 +func isNow(t time.Time) bool { return t == time.Now() } // MATCH /use t.Equal(time.Now()) instead of "==" operator/ +func isNotNow(t time.Time) bool { return time.Now() != t } // MATCH /use !time.Now().Equal(t) instead of "!=" operator/ From 16871ed55f51d0dca10bbf8894cb53b432a56e22 Mon Sep 17 00:00:00 2001 From: chavacava Date: Sat, 12 Aug 2023 10:56:53 +0200 Subject: [PATCH 27/34] fix #864: confusing-naming FP on methods of generic types (#869) --- rule/confusing-naming.go | 11 +++++++---- testdata/confusing-naming1.go | 12 +++++++++++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/rule/confusing-naming.go b/rule/confusing-naming.go index 34cdb907a..8b1c3eac4 100644 --- a/rule/confusing-naming.go +++ b/rule/confusing-naming.go @@ -27,10 +27,10 @@ type packages struct { func (ps *packages) methodNames(lp *lint.Package) pkgMethods { ps.mu.Lock() + defer ps.mu.Unlock() for _, pkg := range ps.pkgs { if pkg.pkg == lp { - ps.mu.Unlock() return pkg } } @@ -38,7 +38,6 @@ func (ps *packages) methodNames(lp *lint.Package) pkgMethods { pkgm := pkgMethods{pkg: lp, methods: make(map[string]map[string]*referenceMethod), mu: &sync.Mutex{}} ps.pkgs = append(ps.pkgs, pkgm) - ps.mu.Unlock() return pkgm } @@ -72,6 +71,7 @@ func (*ConfusingNamingRule) Name() string { // checkMethodName checks if a given method/function name is similar (just case differences) to other method/function of the same struct/file. func checkMethodName(holder string, id *ast.Ident, w *lintConfusingNames) { + if id.Name == "init" && holder == defaultStructName { // ignore init functions return @@ -137,8 +137,11 @@ func getStructName(r *ast.FieldList) string { t := r.List[0].Type - if p, _ := t.(*ast.StarExpr); p != nil { // if a pointer receiver => dereference pointer receiver types - t = p.X + switch v := t.(type) { + case *ast.StarExpr: + t = v.X + case *ast.IndexExpr: + t = v.X } if p, _ := t.(*ast.Ident); p != nil { diff --git a/testdata/confusing-naming1.go b/testdata/confusing-naming1.go index ba12b07b7..5b06bac87 100644 --- a/testdata/confusing-naming1.go +++ b/testdata/confusing-naming1.go @@ -1,5 +1,4 @@ // Test of confusing-naming rule. - // Package pkg ... package pkg @@ -60,3 +59,14 @@ type tBar struct { qwe bool zxc float32 } + +// issue #864 +type x[T any] struct{} + +func (x[T]) method() { +} + +type y[T any] struct{} + +func (y[T]) method() { +} From 19a95d9a7ff7fd4c82aad1f4462df61c1ca075f1 Mon Sep 17 00:00:00 2001 From: Martins Irbe Date: Wed, 16 Aug 2023 12:23:29 +0100 Subject: [PATCH 28/34] resolve #867: remove k[A-Z][A-Za-z\d]*$ sub-rule from `var-naming` (#871) --- rule/var-naming.go | 9 --------- testdata/golint/var-naming.go | 3 +-- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/rule/var-naming.go b/rule/var-naming.go index c6a6305be..286ff9d75 100644 --- a/rule/var-naming.go +++ b/rule/var-naming.go @@ -141,15 +141,6 @@ func (w *lintNames) check(id *ast.Ident, thing string) { }) return } - if len(id.Name) > 2 && id.Name[0] == 'k' && id.Name[1] >= 'A' && id.Name[1] <= 'Z' { - should := string(id.Name[1]+'a'-'A') + id.Name[2:] - w.onFailure(lint.Failure{ - Failure: fmt.Sprintf("don't use leading k in Go names; %s %s should be %s", thing, id.Name, should), - Confidence: 0.8, - Node: id, - Category: "naming", - }) - } should := lint.Name(id.Name, w.whitelist, w.blacklist) if id.Name == should { diff --git a/testdata/golint/var-naming.go b/testdata/golint/var-naming.go index f4e1fbc5c..4d26acdb4 100644 --- a/testdata/golint/var-naming.go +++ b/testdata/golint/var-naming.go @@ -57,8 +57,7 @@ func f_it() { // MATCH /don't use underscores in Go names; func f_it should be f // Common styles in other languages that don't belong in Go. const ( - CPP_CONST = 1 // MATCH /don't use ALL_CAPS in Go names; use CamelCase/ - kLeadingKay = 2 // MATCH /don't use leading k in Go names; const kLeadingKay should be leadingKay/ + CPP_CONST = 1 // MATCH /don't use ALL_CAPS in Go names; use CamelCase/ HTML = 3 // okay; no underscore X509B = 4 // ditto From 519ffbdd7a328599a2fd123d28b31e5bb6225a89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dami=C3=A1n=20Ferencz?= Date: Fri, 18 Aug 2023 15:21:10 -0300 Subject: [PATCH 29/34] fix: unnecesary alert for use-any when comments inside interface{} (#873) * feat: add rule for unused import alias * fix: rename rule * fix: rename rule * fix: use-any skip comments inside interface{} --- rule/use-any.go | 26 ++++++++++++++++++++++---- testdata/use-any.go | 4 ++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/rule/use-any.go b/rule/use-any.go index bdf3c936d..9c6d5ccf8 100644 --- a/rule/use-any.go +++ b/rule/use-any.go @@ -1,9 +1,8 @@ package rule import ( - "go/ast" - "github.com/mgechev/revive/lint" + "go/ast" ) // UseAnyRule lints given else constructs. @@ -14,6 +13,7 @@ func (*UseAnyRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure walker := lintUseAny{ + commentPositions: getCommentsPositions(file.AST.Comments), onFailure: func(failure lint.Failure) { failures = append(failures, failure) }, @@ -30,7 +30,8 @@ func (*UseAnyRule) Name() string { } type lintUseAny struct { - onFailure func(lint.Failure) + commentPositions []int + onFailure func(lint.Failure) } func (w lintUseAny) Visit(n ast.Node) ast.Visitor { @@ -40,7 +41,13 @@ func (w lintUseAny) Visit(n ast.Node) ast.Visitor { } if len(it.Methods.List) != 0 { - return w // it is not and empty interface + return w // it is not an empty interface + } + + for _, pos := range w.commentPositions { + if pos > int(it.Pos()) && pos < int(it.End()) { + return w // it is a comment inside the interface + } } w.onFailure(lint.Failure{ @@ -52,3 +59,14 @@ func (w lintUseAny) Visit(n ast.Node) ast.Visitor { return w } + +func getCommentsPositions(commentGroups []*ast.CommentGroup) []int { + result := []int{} + for _, commentGroup := range commentGroups { + for _, comment := range commentGroup.List { + result = append(result, int(comment.Pos())) + } + } + + return result +} diff --git a/testdata/use-any.go b/testdata/use-any.go index bc6e64759..a624d5ed2 100644 --- a/testdata/use-any.go +++ b/testdata/use-any.go @@ -17,6 +17,10 @@ func any2(a int) interface{} {} // MATCH /since GO 1.18 'interface{}' can be rep var ni interface{ Close() } +var ni interface { + // Close() +} + type nt interface{ Close() } type na = interface{ Close() } From 9acfcc8f86e4b136407091dbf8e98bd6a4eb64a4 Mon Sep 17 00:00:00 2001 From: chavacava Date: Fri, 18 Aug 2023 20:21:42 +0200 Subject: [PATCH 30/34] fix #863:false positive on return statement in a func lit passed to the deferred function (#870) --- rule/defer.go | 15 ++++++++++++--- testdata/defer.go | 14 ++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/rule/defer.go b/rule/defer.go index f8224fd4d..f3ea17920 100644 --- a/rule/defer.go +++ b/rule/defer.go @@ -97,18 +97,21 @@ func (w lintDeferRule) Visit(node ast.Node) ast.Visitor { w.newFailure("return in a defer function has no effect", n, 1.0, "logic", "return") } case *ast.CallExpr: - if !w.inADefer && isIdent(n.Fun, "recover") { + isCallToRecover := isIdent(n.Fun, "recover") + switch { + case !w.inADefer && isCallToRecover: // func fn() { recover() } // // confidence is not 1 because recover can be in a function that is deferred elsewhere w.newFailure("recover must be called inside a deferred function", n, 0.8, "logic", "recover") - } else if w.inADefer && !w.inAFuncLit && isIdent(n.Fun, "recover") { + case w.inADefer && !w.inAFuncLit && isCallToRecover: // defer helper(recover()) // // confidence is not truly 1 because this could be in a correctly-deferred func, // but it is very likely to be a misunderstanding of defer's behavior around arguments. w.newFailure("recover must be called inside a deferred function, this is executing recover immediately", n, 1, "logic", "immediate-recover") } + case *ast.DeferStmt: if isIdent(n.Call.Fun, "recover") { // defer recover() @@ -119,7 +122,12 @@ func (w lintDeferRule) Visit(node ast.Node) ast.Visitor { } w.visitSubtree(n.Call.Fun, true, false, false) for _, a := range n.Call.Args { - w.visitSubtree(a, true, false, false) // check arguments, they should not contain recover() + switch a.(type) { + case *ast.FuncLit: + continue // too hard to analyze deferred calls with func literals args + default: + w.visitSubtree(a, true, false, false) // check arguments, they should not contain recover() + } } if w.inALoop { @@ -136,6 +144,7 @@ func (w lintDeferRule) Visit(node ast.Node) ast.Visitor { w.newFailure("be careful when deferring calls to methods without pointer receiver", fn, 0.8, "bad practice", "method-call") } } + } return nil } diff --git a/testdata/defer.go b/testdata/defer.go index a32b0cca0..a035cfe08 100644 --- a/testdata/defer.go +++ b/testdata/defer.go @@ -33,3 +33,17 @@ func deferrer() { // does not work, but not currently blocked. defer helper(func() { recover() }) } + +// Issue #863 + +func verify(fn func() error) { + if err := fn(); err != nil { + panic(err) + } +} + +func f() { + defer verify(func() error { + return nil + }) +} From e758901c9af4d26b611e50cc414dc8e5f961ab56 Mon Sep 17 00:00:00 2001 From: chavacava Date: Sat, 19 Aug 2023 08:07:28 +0200 Subject: [PATCH 31/34] Revert "fix: unnecesary alert for use-any when comments inside interface{} (#873)" (#874) This reverts commit 519ffbdd7a328599a2fd123d28b31e5bb6225a89. --- rule/use-any.go | 26 ++++---------------------- testdata/use-any.go | 4 ---- 2 files changed, 4 insertions(+), 26 deletions(-) diff --git a/rule/use-any.go b/rule/use-any.go index 9c6d5ccf8..bdf3c936d 100644 --- a/rule/use-any.go +++ b/rule/use-any.go @@ -1,8 +1,9 @@ package rule import ( - "github.com/mgechev/revive/lint" "go/ast" + + "github.com/mgechev/revive/lint" ) // UseAnyRule lints given else constructs. @@ -13,7 +14,6 @@ func (*UseAnyRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { var failures []lint.Failure walker := lintUseAny{ - commentPositions: getCommentsPositions(file.AST.Comments), onFailure: func(failure lint.Failure) { failures = append(failures, failure) }, @@ -30,8 +30,7 @@ func (*UseAnyRule) Name() string { } type lintUseAny struct { - commentPositions []int - onFailure func(lint.Failure) + onFailure func(lint.Failure) } func (w lintUseAny) Visit(n ast.Node) ast.Visitor { @@ -41,13 +40,7 @@ func (w lintUseAny) Visit(n ast.Node) ast.Visitor { } if len(it.Methods.List) != 0 { - return w // it is not an empty interface - } - - for _, pos := range w.commentPositions { - if pos > int(it.Pos()) && pos < int(it.End()) { - return w // it is a comment inside the interface - } + return w // it is not and empty interface } w.onFailure(lint.Failure{ @@ -59,14 +52,3 @@ func (w lintUseAny) Visit(n ast.Node) ast.Visitor { return w } - -func getCommentsPositions(commentGroups []*ast.CommentGroup) []int { - result := []int{} - for _, commentGroup := range commentGroups { - for _, comment := range commentGroup.List { - result = append(result, int(comment.Pos())) - } - } - - return result -} diff --git a/testdata/use-any.go b/testdata/use-any.go index a624d5ed2..bc6e64759 100644 --- a/testdata/use-any.go +++ b/testdata/use-any.go @@ -17,10 +17,6 @@ func any2(a int) interface{} {} // MATCH /since GO 1.18 'interface{}' can be rep var ni interface{ Close() } -var ni interface { - // Close() -} - type nt interface{ Close() } type na = interface{ Close() } From 4ee75424787629fda1692a2561440ba40f427adf Mon Sep 17 00:00:00 2001 From: Denis Voytyuk <5462781+denisvmedia@users.noreply.github.com> Date: Mon, 21 Aug 2023 23:45:41 +0200 Subject: [PATCH 32/34] fix: false positive in import-shadowing for method names (#876) --- rule/import-shadowing.go | 11 +++++++++-- testdata/import-shadowing.go | 8 ++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/rule/import-shadowing.go b/rule/import-shadowing.go index 2bab704d0..046aeb688 100644 --- a/rule/import-shadowing.go +++ b/rule/import-shadowing.go @@ -29,6 +29,7 @@ func (*ImportShadowingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fail failures = append(failures, failure) }, alreadySeen: map[*ast.Object]struct{}{}, + skipIdents: map[*ast.Ident]struct{}{}, } ast.Walk(walker, fileAst) @@ -62,6 +63,7 @@ type importShadowing struct { importNames map[string]struct{} onFailure func(lint.Failure) alreadySeen map[*ast.Object]struct{} + skipIdents map[*ast.Ident]struct{} } // Visit visits AST nodes and checks if id nodes (ast.Ident) shadow an import name @@ -80,6 +82,10 @@ func (w importShadowing) Visit(n ast.Node) ast.Visitor { *ast.SelectorExpr, // skip analysis of selector expressions (anId.otherId): because if anId shadows an import name, it was already detected, and otherId does not shadows the import name *ast.StructType: // skip analysis of struct type because struct fields can not shadow an import name return nil + case *ast.FuncDecl: + if n.Recv != nil { + w.skipIdents[n.Name] = struct{}{} + } case *ast.Ident: if n == w.packageNameIdent { return nil // skip the ident corresponding to the package name of this file @@ -92,11 +98,12 @@ func (w importShadowing) Visit(n ast.Node) ast.Visitor { _, isImportName := w.importNames[id] _, alreadySeen := w.alreadySeen[n.Obj] - if isImportName && !alreadySeen { + _, skipIdent := w.skipIdents[n] + if isImportName && !alreadySeen && !skipIdent { w.onFailure(lint.Failure{ Confidence: 1, Node: n, - Category: "namming", + Category: "naming", Failure: fmt.Sprintf("The name '%s' shadows an import name", id), }) diff --git a/testdata/import-shadowing.go b/testdata/import-shadowing.go index 124f108fb..230f8079a 100644 --- a/testdata/import-shadowing.go +++ b/testdata/import-shadowing.go @@ -23,6 +23,14 @@ type fmt interface {} // MATCH /The name 'fmt' shadows an import name/ func (ast myAst) foo() {} // MATCH /The name 'ast' shadows an import name/ +func (a myAst) fmt() { // this should be skipped (method, not a pkg func) + var fmt string // MATCH /The name 'fmt' shadows an import name/ +} + +func (a myAst) md5() { // this should be skipped (method, not a pkg func) + strings := map[string]string{} // MATCH /The name 'strings' shadows an import name/ +} + func md5() {} // MATCH /The name 'md5' shadows an import name/ func bar(_ string) {} From a4ae369945a2857db0cfcac6f95920dbc336a38a Mon Sep 17 00:00:00 2001 From: Denis Voytyuk <5462781+denisvmedia@users.noreply.github.com> Date: Tue, 22 Aug 2023 09:18:10 +0200 Subject: [PATCH 33/34] build: shift to the supported go versions (1.20 and 1.21) (#878) --- .github/workflows/release.yml | 2 +- .github/workflows/test.yaml | 4 +- go.mod | 2 +- go.sum | 76 +---------------------------------- 4 files changed, 6 insertions(+), 78 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 708f8fc1c..92beb28ad 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -18,7 +18,7 @@ jobs: name: Set up Go uses: actions/setup-go@v2 with: - go-version: 1.19 + go-version: 1.21 - name: Log into registry run: echo "${{ secrets.GITHUB_TOKEN }}" | docker login ghcr.io -u ${{ github.actor }} --password-stdin diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 11c4a344d..f41ef1455 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -12,8 +12,8 @@ jobs: fail-fast: false matrix: go-version: - - 1.18.x - - 1.19.x + - 1.20.x + - 1.21.x steps: - name: Checkout code uses: actions/checkout@v3.0.2 diff --git a/go.mod b/go.mod index a7e990c9d..1d1802fcd 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/mgechev/revive -go 1.19 +go 1.20 require ( github.com/BurntSushi/toml v1.3.2 diff --git a/go.sum b/go.sum index f8603b66c..9b809f28b 100644 --- a/go.sum +++ b/go.sum @@ -1,39 +1,16 @@ -github.com/BurntSushi/toml v1.2.0 h1:Rt8g24XnyGTyglgET/PRUNlrUeu9F5L+7FilkXfZgs0= -github.com/BurntSushi/toml v1.2.0/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= -github.com/BurntSushi/toml v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak= -github.com/BurntSushi/toml v1.2.1/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= github.com/BurntSushi/toml v1.3.2 h1:o7IhLm0Msx3BaB+n3Ag7L8EVlByGnpq14C4YWiu/gL8= github.com/BurntSushi/toml v1.3.2/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ= -github.com/chavacava/garif v0.0.0-20220630083739-93517212f375 h1:E7LT642ysztPWE0dfz43cWOvMiF42DyTRC+eZIaO4yI= -github.com/chavacava/garif v0.0.0-20220630083739-93517212f375/go.mod h1:4m1Rv7xfuwWPNKXlThldNuJvutYM6J95wNuuVmn55To= -github.com/chavacava/garif v0.0.0-20221024190013-b3ef35877348 h1:cy5GCEZLUCshCGCRRUjxHrDUqkB4l5cuUt3ShEckQEo= -github.com/chavacava/garif v0.0.0-20221024190013-b3ef35877348/go.mod h1:f/miWtG3SSuTxKsNK3o58H1xl+XV6ZIfbC6p7lPPB8U= -github.com/chavacava/garif v0.0.0-20230227094218-b8c73b2037b8 h1:W9o46d2kbNL06lq7UNDPV0zYLzkrde/bjIqO02eoll0= -github.com/chavacava/garif v0.0.0-20230227094218-b8c73b2037b8/go.mod h1:gakxgyXaaPkxvLw1XQxNGK4I37ys9iBRzNUx/B7pUCo= -github.com/chavacava/garif v0.0.0-20230519080132-4752330f72df h1:1uGdlpQT0irrGcFFOUuitqSCE6BjttfHd+k3k9OQ0fg= -github.com/chavacava/garif v0.0.0-20230519080132-4752330f72df/go.mod h1:cFP7fAFavJ2DrYBmZYBETNKwSTFJiOIirm5N4/PqY/I= -github.com/chavacava/garif v0.0.0-20230531072157-8144c224b063 h1:wiJ8AtgOw7gDs2I9uhrxW59fMPWXONSdpXU3/dRv0to= -github.com/chavacava/garif v0.0.0-20230531072157-8144c224b063/go.mod h1:XMyYCkEL58DF0oyW4qDjjnPWONs2HBqYKI+UIPD+Gww= github.com/chavacava/garif v0.0.0-20230608123814-4bd63c2919ab h1:5JxePczlyGAtj6R1MUEFZ/UFud6FfsOejq7xLC2ZIb0= github.com/chavacava/garif v0.0.0-20230608123814-4bd63c2919ab/go.mod h1:XMyYCkEL58DF0oyW4qDjjnPWONs2HBqYKI+UIPD+Gww= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w= -github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk= -github.com/fatih/color v1.14.1 h1:qfhVLaG5s+nCROl1zJsZRxFeYrHLqWroPOQ8BWiNb4w= -github.com/fatih/color v1.14.1/go.mod h1:2oHN61fhTpgcxD3TSWCgKDiH1+x4OiDVVGH8WlgGZGg= github.com/fatih/color v1.15.0 h1:kOqh6YHBtK8aywxGerMG2Eq3H6Qgoqeo13Bk2Mv/nBs= github.com/fatih/color v1.15.0/go.mod h1:0h5ZqXfHYED7Bhv2ZJamyIOUej9KtShiJESRwBDUSsw= github.com/fatih/structtag v1.2.0 h1:/OdNE99OxoI/PqaW/SuSK9uxxT3f/tcSZgon/ssNSx4= github.com/fatih/structtag v1.2.0/go.mod h1:mBJUNpUnHmRKrKlQQlmCrh5PuhftFbNv8Ys4/aAZl94= -github.com/mattn/go-colorable v0.1.9 h1:sqDoxXbdeALODt0DAeJCVp38ps9ZogZEAXjus69YV3U= -github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= -github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU= -github.com/mattn/go-isatty v0.0.14 h1:yVuAays6BHfxijgZPzw+3Zlu5yQgKGP2/hcQbHb7S9Y= -github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94= github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= github.com/mattn/go-isatty v0.0.17 h1:BTarxUcIeDqL27Mc+vyvdWYSL28zpIhv3RoTdsLMPng= github.com/mattn/go-isatty v0.0.17/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= @@ -53,62 +30,13 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+ github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= -github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= -golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f h1:v4INt8xihDGvnrfjMDVXGxw9wrfxYyCjk0KbXjhR55s= -golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/mod v0.12.0 h1:rmsUpXtvNzj340zd98LZ4KntptpfRHwpFOHG188oHXc= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U= -golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.2.0 h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A= -golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.3.0 h1:w8ZOecv6NaNa/zC8944JTU3vz4u6Lagfk4RPQxv92NQ= -golang.org/x/sys v0.3.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.4.0 h1:Zr2JFtRQNX3BCZ8YtxRE9hNJYC8J6I1MVbMg6owUp18= -golang.org/x/sys v0.4.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU= -golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.6.0 h1:MVltZSvRTcU2ljQOhs94SXPftV6DCNnZViHeQps87pQ= -golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU= -golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.9.0 h1:KS/R3tvhPqvJvwcKfnBHJwwthS11LRhmM5D59eEXa0s= -golang.org/x/sys v0.9.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.10.0 h1:SqMFp9UcQJZa+pmYuAKjd9xq1f0j5rLcDIk0mj4qAsA= -golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM= golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/tools v0.1.12 h1:VveCTK38A2rkS8ZqFY25HIDFscX5X9OoEhJd3quQmXU= -golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= -golang.org/x/tools v0.2.0 h1:G6AHpWxTMGY1KyEYoAQ5WTtIekUUvDNjan3ugu60JvE= -golang.org/x/tools v0.2.0/go.mod h1:y4OqIKeOV/fWJetJ8bXPU1sEVniLMIyDAZWeHdV+NTA= -golang.org/x/tools v0.3.0 h1:SrNbZl6ECOS1qFzgTdQfWXZM9XBkiA6tkFrH9YSTPHM= -golang.org/x/tools v0.3.0/go.mod h1:/rWhSS2+zyEVwoJf8YAX6L2f0ntZ7Kn/mGgAWcipA5k= -golang.org/x/tools v0.4.0 h1:7mTAgkunk3fr4GAloyyCasadO6h9zSsQZbwvcaIciV4= -golang.org/x/tools v0.4.0/go.mod h1:UE5sM2OK9E/d67R0ANs2xJizIymRP5gJU295PvKXxjQ= -golang.org/x/tools v0.5.0 h1:+bSpV5HIeWkuvgaMfI3UmKRThoTA5ODJTUd8T17NO+4= -golang.org/x/tools v0.5.0/go.mod h1:N+Kgy78s5I24c24dU8OfWNEotWjutIs8SnJvn5IDq+k= -golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM= -golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= -golang.org/x/tools v0.7.0 h1:W4OVu8VVOaIO0yzWMNdepAulS7YfoS3Zabrm8DOXXU4= -golang.org/x/tools v0.7.0/go.mod h1:4pg6aUX35JBAogB10C9AtvVL+qowtN4pT3CGSQex14s= -golang.org/x/tools v0.9.1 h1:8WMNJAz3zrtPmnYC7ISf5dEn3MT0gY7jBJfw27yrrLo= -golang.org/x/tools v0.9.1/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc= -golang.org/x/tools v0.9.3 h1:Gn1I8+64MsuTb/HpH+LmQtNas23LhUVr3rYZ0eKuaMM= -golang.org/x/tools v0.9.3/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc= -golang.org/x/tools v0.10.0 h1:tvDr/iQoUqNdohiYm0LmmKcBk+q86lb9EprIUFhHHGg= -golang.org/x/tools v0.10.0/go.mod h1:UJwyiVBsOA2uwvK/e5OY3GTpDUJriEd+/YlqAwLPmyM= -golang.org/x/tools v0.11.0 h1:EMCa6U9S2LtZXLAMoWiR/R8dAQFRqbAitmbJ2UKhoi8= -golang.org/x/tools v0.11.0/go.mod h1:anzJrxPjNtfgiYQYirP2CPGzGLxrH2u2QBhn6Bf3qY8= -golang.org/x/tools v0.11.1 h1:ojD5zOW8+7dOGzdnNgersm8aPfcDjhMp12UfG93NIMc= -golang.org/x/tools v0.11.1/go.mod h1:anzJrxPjNtfgiYQYirP2CPGzGLxrH2u2QBhn6Bf3qY8= golang.org/x/tools v0.12.0 h1:YW6HUoUmYBpwSgyaGaZq1fHjrBjX1rlpZ54T6mu2kss= golang.org/x/tools v0.12.0/go.mod h1:Sc0INKfu04TlqNoRA1hgpFZbhYXHPr4V5DzpSBTPqQM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= From 9a87e6fd820317b1235e1b75bc884ce34c375dd5 Mon Sep 17 00:00:00 2001 From: mgechev Date: Thu, 24 Aug 2023 10:41:23 -0700 Subject: [PATCH 34/34] Update goreleaser --- .goreleaser.yml | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/.goreleaser.yml b/.goreleaser.yml index c029c33e5..d8c50f6e1 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -2,13 +2,8 @@ # Make sure to check the documentation at http://goreleaser.com --- archives: - - - replacements: - 386: i386 - amd64: x86_64 - darwin: Darwin - linux: Linux - windows: Windows + - id: revive + name_template: '{{ .ProjectName }}_{{ .Os }}_{{ .Arch }}{{ if .Arm }}v{{ .Arm }}{{ end }}' before: hooks: - "go mod download"