Skip to content

Commit

Permalink
unused-param,unused-receiver: refactor configure func (#1157)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexandear authored Dec 4, 2024
1 parent 7f7d024 commit 09fb350
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 34 deletions.
35 changes: 18 additions & 17 deletions rule/unused_param.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"github.com/mgechev/revive/lint"
)

var allowBlankIdentifierRegex = regexp.MustCompile("^_$")

// UnusedParamRule lints unused params in functions.
type UnusedParamRule struct {
// regex to check if some name is valid for unused parameter, "^_$" by default
Expand All @@ -21,30 +23,29 @@ type UnusedParamRule struct {
func (r *UnusedParamRule) configure(args lint.Arguments) {
// 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 allowRegexStr string
r.allowRegex = allowBlankIdentifierRegex
r.failureMsg = "parameter '%s' seems to be unused, consider removing or renaming it as _"
if len(args) == 0 {
allowRegexStr = "^_$"
r.failureMsg = "parameter '%s' seems to be unused, consider removing or renaming it as _"
} else {
// Arguments = [{}]
options := args[0].(map[string]any)
// Arguments = [{allowRegex="^_"}]

if allowRegexParam, ok := options["allowRegex"]; ok {
allowRegexStr, ok = allowRegexParam.(string)
if !ok {
panic(fmt.Errorf("error configuring %s rule: allowRegex is not string but [%T]", r.Name(), allowRegexParam))
}
}
return
}
// Arguments = [{}]
options := args[0].(map[string]any)

allowRegexParam, ok := options["allowRegex"]
if !ok {
return
}
// Arguments = [{allowRegex="^_"}]
allowRegexStr, ok := allowRegexParam.(string)
if !ok {
panic(fmt.Errorf("error configuring %s rule: allowRegex is not string but [%T]", r.Name(), allowRegexParam))
}
var err error
r.allowRegex, err = regexp.Compile(allowRegexStr)
if err != nil {
panic(fmt.Errorf("error configuring %s rule: allowRegex is not valid regex [%s]: %v", r.Name(), allowRegexStr, err))
}
if r.failureMsg == "" {
r.failureMsg = "parameter '%s' seems to be unused, consider removing or renaming it to match " + r.allowRegex.String()
}
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.
Expand Down
33 changes: 16 additions & 17 deletions rule/unused_receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,30 +21,29 @@ type UnusedReceiverRule struct {
func (r *UnusedReceiverRule) configure(args lint.Arguments) {
// 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 allowRegexStr string
r.allowRegex = allowBlankIdentifierRegex
r.failureMsg = "method receiver '%s' is not referenced in method's body, consider removing or renaming it as _"
if len(args) == 0 {
allowRegexStr = "^_$"
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]any)
// Arguments = [{allowRegex="^_"}]

if allowRegexParam, ok := options["allowRegex"]; ok {
allowRegexStr, ok = allowRegexParam.(string)
if !ok {
panic(fmt.Errorf("error configuring [unused-receiver] rule: allowRegex is not string but [%T]", allowRegexParam))
}
}
return
}
// Arguments = [{}]
options := args[0].(map[string]any)

allowRegexParam, ok := options["allowRegex"]
if !ok {
return
}
// Arguments = [{allowRegex="^_"}]
allowRegexStr, ok := allowRegexParam.(string)
if !ok {
panic(fmt.Errorf("error configuring [unused-receiver] rule: allowRegex is not string but [%T]", allowRegexParam))
}
var err error
r.allowRegex, err = regexp.Compile(allowRegexStr)
if err != nil {
panic(fmt.Errorf("error configuring [unused-receiver] rule: allowRegex is not valid regex [%s]: %v", allowRegexStr, 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()
}
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.
Expand Down
4 changes: 4 additions & 0 deletions test/unused_param_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ import (

func TestUnusedParam(t *testing.T) {
testRule(t, "unused_param", &rule.UnusedParamRule{})
testRule(t, "unused_param", &rule.UnusedParamRule{}, &lint.RuleConfig{Arguments: []any{}})
testRule(t, "unused_param", &rule.UnusedParamRule{}, &lint.RuleConfig{Arguments: []any{
map[string]any{"a": "^xxx"},
}})
testRule(t, "unused_param_custom_regex", &rule.UnusedParamRule{}, &lint.RuleConfig{Arguments: []any{
map[string]any{"allowRegex": "^xxx"},
}})
Expand Down
11 changes: 11 additions & 0 deletions test/unused_receiver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,18 @@ import (

func TestUnusedReceiver(t *testing.T) {
testRule(t, "unused_receiver", &rule.UnusedReceiverRule{})
testRule(t, "unused_receiver", &rule.UnusedReceiverRule{}, &lint.RuleConfig{Arguments: []any{}})
testRule(t, "unused_receiver", &rule.UnusedReceiverRule{}, &lint.RuleConfig{Arguments: []any{
map[string]any{"a": "^xxx"},
}})
testRule(t, "unused_receiver_custom_regex", &rule.UnusedReceiverRule{}, &lint.RuleConfig{Arguments: []any{
map[string]any{"allowRegex": "^xxx"},
}})
}

func BenchmarkUnusedReceiver(b *testing.B) {
var t *testing.T
for i := 0; i <= b.N; i++ {
testRule(t, "unused_receiver", &rule.UnusedReceiverRule{})
}
}

0 comments on commit 09fb350

Please sign in to comment.