Skip to content

Commit 1be8855

Browse files
committed
Allow to specify option with dashes
1 parent 79142d4 commit 1be8855

9 files changed

+143
-35
lines changed

rule/add_constant_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@ func TestAddConstantRule_Configure(t *testing.T) {
3232
name: "valid arguments",
3333
arguments: lint.Arguments{
3434
map[string]any{
35-
"allowfloats": "1.0,2.0",
36-
"allowints": "1,2",
37-
"allowstrs": "a,b",
38-
"maxlitcount": "3",
39-
"ignorefuncs": "fmt.Println,fmt.Printf",
35+
"allowFloats": "1.0,2.0",
36+
"allowInts": "1,2",
37+
"allowStrs": "a,b",
38+
"maxLitCount": "3",
39+
"ignoreFuncs": "fmt.Println,fmt.Printf",
4040
},
4141
},
4242
wantErr: nil,

rule/context_as_argument.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func (*ContextAsArgumentRule) getAllowTypesFromArguments(args lint.Arguments) (m
7474
return nil, fmt.Errorf("invalid argument to the context-as-argument rule. Expecting a k,v map, got %T", args[0])
7575
}
7676
for k, v := range argKV {
77-
switch strings.ToLower(k) {
77+
switch ruleOptionFromArg(k) {
7878
case "allowtypesbefore":
7979
typesBefore, ok := v.(string)
8080
if !ok {

rule/context_as_argument_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func TestContextAsArgumentRule_Configure(t *testing.T) {
1919
name: "valid arguments",
2020
arguments: lint.Arguments{
2121
map[string]any{
22-
"allowtypesbefore": "AllowedBeforeType,AllowedBeforeStruct,*AllowedBeforePtrStruct,*testing.T",
22+
"allowTypesBefore": "AllowedBeforeType,AllowedBeforeStruct,*AllowedBeforePtrStruct,*testing.T",
2323
},
2424
},
2525
wantErr: nil,

rule/enforce_repeated_arg_type_style.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package rule
33
import (
44
"fmt"
55
"go/ast"
6-
"strings"
76

87
"github.com/mgechev/revive/lint"
98
)
@@ -70,7 +69,7 @@ func (r *EnforceRepeatedArgTypeStyleRule) Configure(arguments lint.Arguments) er
7069
r.funcRetValStyle = valstyle
7170
case map[string]any: // expecting map[string]string
7271
for k, v := range funcArgStyle {
73-
switch strings.ToLower(k) {
72+
switch ruleOptionFromArg(k) {
7473
case "funcargstyle":
7574
val, ok := v.(string)
7675
if !ok {

rule/exported.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func (r *ExportedRule) Configure(arguments lint.Arguments) error {
7878
for _, flag := range arguments {
7979
switch flag := flag.(type) {
8080
case string:
81-
switch strings.ToLower(flag) {
81+
switch ruleOptionFromArg(flag) {
8282
case "checkprivatereceivers":
8383
r.disabledChecks.PrivateReceivers = false
8484
case "disablestutteringcheck":

rule/exported_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,63 +24,63 @@ func TestExportedRule_Configure(t *testing.T) {
2424
},
2525
{
2626
name: "checkPrivateReceivers",
27-
arguments: lint.Arguments{"checkprivatereceivers"},
27+
arguments: lint.Arguments{"checkPrivateReceivers"},
2828
wantErr: nil,
2929
wantDisabledChecks: disabledChecks{PrivateReceivers: false, PublicInterfaces: true},
3030
wantStuttersMsg: "stutters",
3131
},
3232
{
3333
name: "disableStutteringCheck",
34-
arguments: lint.Arguments{"disablestutteringcheck"},
34+
arguments: lint.Arguments{"disableStutteringCheck"},
3535
wantErr: nil,
3636
wantDisabledChecks: disabledChecks{PrivateReceivers: true, PublicInterfaces: true, Stuttering: true},
3737
wantStuttersMsg: "stutters",
3838
},
3939
{
4040
name: "sayRepetitiveInsteadOfStutters",
41-
arguments: lint.Arguments{"sayrepetitiveinsteadofstutters"},
41+
arguments: lint.Arguments{"sayRepetitiveInsteadOfStutters"},
4242
wantErr: nil,
4343
wantDisabledChecks: disabledChecks{PrivateReceivers: true, PublicInterfaces: true},
4444
wantStuttersMsg: "is repetitive",
4545
},
4646
{
4747
name: "checkPublicInterface",
48-
arguments: lint.Arguments{"checkpublicinterface"},
48+
arguments: lint.Arguments{"checkPublicInterface"},
4949
wantErr: nil,
5050
wantDisabledChecks: disabledChecks{PrivateReceivers: true, PublicInterfaces: false},
5151
wantStuttersMsg: "stutters",
5252
},
5353
{
5454
name: "disableChecksOnConstants",
55-
arguments: lint.Arguments{"disablechecksonconstants"},
55+
arguments: lint.Arguments{"disableChecksOnConstants"},
5656
wantErr: nil,
5757
wantDisabledChecks: disabledChecks{PrivateReceivers: true, PublicInterfaces: true, Const: true},
5858
wantStuttersMsg: "stutters",
5959
},
6060
{
6161
name: "disableChecksOnFunctions",
62-
arguments: lint.Arguments{"disablechecksonfunctions"},
62+
arguments: lint.Arguments{"disableChecksOnFunctions"},
6363
wantErr: nil,
6464
wantDisabledChecks: disabledChecks{PrivateReceivers: true, PublicInterfaces: true, Function: true},
6565
wantStuttersMsg: "stutters",
6666
},
6767
{
6868
name: "disableChecksOnMethods",
69-
arguments: lint.Arguments{"disablechecksonmethods"},
69+
arguments: lint.Arguments{"disableChecksOnMethods"},
7070
wantErr: nil,
7171
wantDisabledChecks: disabledChecks{PrivateReceivers: true, PublicInterfaces: true, Method: true},
7272
wantStuttersMsg: "stutters",
7373
},
7474
{
7575
name: "disableChecksOnTypes",
76-
arguments: lint.Arguments{"disablechecksontypes"},
76+
arguments: lint.Arguments{"disableChecksOnTypes"},
7777
wantErr: nil,
7878
wantDisabledChecks: disabledChecks{PrivateReceivers: true, PublicInterfaces: true, Type: true},
7979
wantStuttersMsg: "stutters",
8080
},
8181
{
8282
name: "disableChecksOnVariables",
83-
arguments: lint.Arguments{"disablechecksonvariables"},
83+
arguments: lint.Arguments{"disableChecksOnVariables"},
8484
wantErr: nil,
8585
wantDisabledChecks: disabledChecks{PrivateReceivers: true, PublicInterfaces: true, Var: true},
8686
wantStuttersMsg: "stutters",

rule/unused_param.go

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,25 +28,31 @@ func (r *UnusedParamRule) Configure(args lint.Arguments) error {
2828
if len(args) == 0 {
2929
return nil
3030
}
31-
// Arguments = [{}]
32-
options := args[0].(map[string]any)
3331

34-
allowRegexParam, ok := options["allowRegex"]
35-
if !ok {
32+
switch options := args[0].(type) {
33+
case map[string]any: // Arguments = [{}]
34+
for k, v := range options {
35+
switch ruleOptionFromArg(k) {
36+
case "allowregex":
37+
// Arguments = [{allowRegex="_"}]
38+
allowRegexStr, ok := v.(string)
39+
if !ok {
40+
return fmt.Errorf("error configuring %s rule: allowRegex is not string but [%T]", r.Name(), v)
41+
}
42+
var err error
43+
r.allowRegex, err = regexp.Compile(allowRegexStr)
44+
if err != nil {
45+
return fmt.Errorf("error configuring %s rule: allowRegex is not valid regex [%s]: %w", r.Name(), allowRegexStr, err)
46+
}
47+
r.failureMsg = "parameter '%s' seems to be unused, consider removing or renaming it to match " + r.allowRegex.String()
48+
default:
49+
return nil
50+
}
51+
}
52+
return nil
53+
default:
3654
return nil
3755
}
38-
// Arguments = [{allowRegex="^_"}]
39-
allowRegexStr, ok := allowRegexParam.(string)
40-
if !ok {
41-
return fmt.Errorf("error configuring %s rule: allowRegex is not string but [%T]", r.Name(), allowRegexParam)
42-
}
43-
var err error
44-
r.allowRegex, err = regexp.Compile(allowRegexStr)
45-
if err != nil {
46-
return fmt.Errorf("error configuring %s rule: allowRegex is not valid regex [%s]: %w", r.Name(), allowRegexStr, err)
47-
}
48-
r.failureMsg = "parameter '%s' seems to be unused, consider removing or renaming it to match " + r.allowRegex.String()
49-
return nil
5056
}
5157

5258
// Apply applies the rule to given file.

rule/unused_param_test.go

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
package rule
2+
3+
import (
4+
"errors"
5+
"reflect"
6+
"regexp"
7+
"testing"
8+
9+
"github.com/mgechev/revive/lint"
10+
)
11+
12+
func TestUnusedParamRule_Configure(t *testing.T) {
13+
tests := []struct {
14+
name string
15+
arguments lint.Arguments
16+
wantErr error
17+
wantAllowRegex *regexp.Regexp
18+
wantFailureMsg string
19+
}{
20+
{
21+
name: "no arguments",
22+
arguments: lint.Arguments{},
23+
wantErr: nil,
24+
wantAllowRegex: regexp.MustCompile("^_$"),
25+
wantFailureMsg: "parameter '%s' seems to be unused, consider removing or renaming it as _",
26+
},
27+
{
28+
name: "valid arguments",
29+
arguments: lint.Arguments{
30+
map[string]any{
31+
"allowRegex": "^_",
32+
},
33+
},
34+
wantErr: nil,
35+
wantAllowRegex: regexp.MustCompile("^_"),
36+
wantFailureMsg: "parameter '%s' seems to be unused, consider removing or renaming it to match ^_",
37+
},
38+
{
39+
name: "missed allowRegex value",
40+
arguments: lint.Arguments{
41+
map[string]any{
42+
"unknownKey": "123",
43+
},
44+
},
45+
wantErr: nil,
46+
wantAllowRegex: regexp.MustCompile("^_$"),
47+
wantFailureMsg: "parameter '%s' seems to be unused, consider removing or renaming it as _",
48+
},
49+
{
50+
name: "invalid allowRegex: not a string",
51+
arguments: lint.Arguments{
52+
map[string]any{
53+
"allowRegex": 123,
54+
},
55+
},
56+
wantErr: errors.New("error configuring unused-parameter rule: allowRegex is not string but [int]"),
57+
},
58+
{
59+
name: "invalid allowRegex: not a valid regex",
60+
arguments: lint.Arguments{
61+
map[string]any{
62+
"allowRegex": "[",
63+
},
64+
},
65+
wantErr: errors.New("error configuring unused-parameter rule: allowRegex is not valid regex [[]: error parsing regexp: missing closing ]: " + "`[`"),
66+
},
67+
}
68+
69+
for _, tt := range tests {
70+
t.Run(tt.name, func(t *testing.T) {
71+
var rule UnusedParamRule
72+
73+
err := rule.Configure(tt.arguments)
74+
75+
if tt.wantErr != nil {
76+
if err == nil {
77+
t.Errorf("unexpected error: got = nil, want = %v", tt.wantErr)
78+
return
79+
}
80+
if err.Error() != tt.wantErr.Error() {
81+
t.Errorf("unexpected error: got = %v, want = %v", err, tt.wantErr)
82+
}
83+
return
84+
}
85+
if err != nil {
86+
t.Errorf("unexpected error: got = %v, want = nil", err)
87+
}
88+
if !reflect.DeepEqual(rule.allowRegex, tt.wantAllowRegex) {
89+
t.Errorf("unexpected allowRegex: got = %v, want %v", rule.allowRegex, tt.wantAllowRegex)
90+
}
91+
if rule.failureMsg != tt.wantFailureMsg {
92+
t.Errorf("unexpected failureMessage: got = %v, want %v", rule.failureMsg, tt.wantFailureMsg)
93+
}
94+
})
95+
}
96+
}

rule/utils.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"go/printer"
88
"go/token"
99
"regexp"
10+
"strings"
1011

1112
"github.com/mgechev/revive/lint"
1213
)
@@ -112,6 +113,12 @@ func checkNumberOfArguments(expected int, args lint.Arguments, ruleName string)
112113
return nil
113114
}
114115

116+
// ruleOptionFromArg returns an option name from the argument. It is lowercased and without dashes.
117+
// This function is used in the Rule.Configure method.
118+
func ruleOptionFromArg(arg string) string {
119+
return strings.ToLower(strings.ReplaceAll(arg, "-", ""))
120+
}
121+
115122
var directiveCommentRE = regexp.MustCompile("^//(line |extern |export |[a-z0-9]+:[a-z0-9])") // see https://go-review.googlesource.com/c/website/+/442516/1..2/_content/doc/comment.md#494
116123

117124
func isDirectiveComment(line string) bool {

0 commit comments

Comments
 (0)