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/ +}