mirror of
				https://github.com/mgechev/revive.git
				synced 2025-10-30 23:37:49 +02:00 
			
		
		
		
	adds [allowRegex] parameter for unused-parameter and unused-receiver rules (#858)
				
					
				
			This commit is contained in:
		| @@ -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 | ||||
|  | ||||
|   | ||||
| @@ -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), | ||||
| 					}) | ||||
| 				} | ||||
| 			} | ||||
|   | ||||
| @@ -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 | ||||
|   | ||||
| @@ -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) { | ||||
|   | ||||
| @@ -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"}, | ||||
| 	}}) | ||||
| } | ||||
|   | ||||
							
								
								
									
										12
									
								
								testdata/unused-param-custom-regex.go
									
									
									
									
										vendored
									
									
										Normal file
									
								
							
							
						
						
									
										12
									
								
								testdata/unused-param-custom-regex.go
									
									
									
									
										vendored
									
									
										Normal file
									
								
							| @@ -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/ | ||||
| } | ||||
							
								
								
									
										12
									
								
								testdata/unused-receiver-custom-regex.go
									
									
									
									
										vendored
									
									
										Normal file
									
								
							
							
						
						
									
										12
									
								
								testdata/unused-receiver-custom-regex.go
									
									
									
									
										vendored
									
									
										Normal file
									
								
							| @@ -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/ | ||||
| } | ||||
		Reference in New Issue
	
	Block a user