Repository: mgechev/revive
Pull request #1165 (2024-12-04 11:52:08)

 refactor (rule/modifies-value-receiver): replace AST walker by iteration over declarations
Simplifies modifies-value-receiver rule by replacing AST walker by an iteration over global declarations



Patch:
diff --git a/rule/argument_limit.go b/rule/argument_limit.go
index ce831e5..b44e1fd 100644
--- a/rule/argument_limit.go
+++ b/rule/argument_limit.go
@@ -35,50 +35,33 @@ func (r *ArgumentsLimitRule) Apply(file *lint.File, arguments lint.Arguments) []
 	r.configureOnce.Do(func() { r.configure(arguments) })
 
 	var failures []lint.Failure
-	onFailure := func(failure lint.Failure) {
-		failures = append(failures, failure)
-	}
-
-	walker := lintArgsNum{
-		max:       r.max,
-		onFailure: onFailure,
-	}
-
-	ast.Walk(walker, file.AST)
-
-	return failures
-}
-
-// Name returns the rule name.
-func (*ArgumentsLimitRule) Name() string {
-	return "argument-limit"
-}
 
-type lintArgsNum struct {
-	max       int
-	onFailure func(lint.Failure)
-}
+	for _, decl := range file.AST.Decls {
+		funcDecl, ok := decl.(*ast.FuncDecl)
+		if !ok {
+			continue
+		}
 
-func (w lintArgsNum) Visit(n ast.Node) ast.Visitor {
-	node, ok := n.(*ast.FuncDecl)
-	if !ok {
-		return w
-	}
+		numParams := 0
+		for _, l := range funcDecl.Type.Params.List {
+			numParams += len(l.Names)
+		}
 
-	num := 0
-	for _, l := range node.Type.Params.List {
-		for range l.Names {
-			num++
+		if numParams <= r.max {
+			continue
 		}
-	}
 
-	if num > w.max {
-		w.onFailure(lint.Failure{
+		failures = append(failures, lint.Failure{
 			Confidence: 1,
-			Failure:    fmt.Sprintf("maximum number of arguments per function exceeded; max %d but got %d", w.max, num),
-			Node:       node.Type,
+			Failure:    fmt.Sprintf("maximum number of arguments per function exceeded; max %d but got %d", r.max, numParams),
+			Node:       funcDecl.Type,
 		})
 	}
 
-	return nil // skip visiting the body of the function
+	return failures
+}
+
+// Name returns the rule name.
+func (*ArgumentsLimitRule) Name() string {
+	return "argument-limit"
 }
diff --git a/rule/confusing_results.go b/rule/confusing_results.go
index 1b79ada..c1804d1 100644
--- a/rule/confusing_results.go
+++ b/rule/confusing_results.go
@@ -13,54 +13,43 @@ type ConfusingResultsRule struct{}
 func (*ConfusingResultsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
 
-	fileAst := file.AST
-	walker := lintConfusingResults{
-		onFailure: func(failure lint.Failure) {
-			failures = append(failures, failure)
-		},
-	}
+	for _, decl := range file.AST.Decls {
+		funcDecl, ok := decl.(*ast.FuncDecl)
 
-	ast.Walk(walker, fileAst)
+		isFunctionWithMoreThanOneResult := ok && funcDecl.Type.Results != nil && len(funcDecl.Type.Results.List) > 1
+		if !isFunctionWithMoreThanOneResult {
+			continue
+		}
 
-	return failures
-}
+		resultsAreNamed := len(funcDecl.Type.Results.List[0].Names) > 0
+		if resultsAreNamed {
+			continue
+		}
 
-// Name returns the rule name.
-func (*ConfusingResultsRule) Name() string {
-	return "confusing-results"
-}
+		lastType := ""
+		for _, result := range funcDecl.Type.Results.List {
 
-type lintConfusingResults struct {
-	onFailure func(lint.Failure)
-}
+			resultTypeName := gofmt(result.Type)
 
-func (w lintConfusingResults) Visit(n ast.Node) ast.Visitor {
-	fn, ok := n.(*ast.FuncDecl)
-	if !ok || fn.Type.Results == nil || len(fn.Type.Results.List) < 2 {
-		return w
-	}
-	lastType := ""
-	for _, result := range fn.Type.Results.List {
-		if len(result.Names) > 0 {
-			return w
-		}
+			if resultTypeName == lastType {
+				failures = append(failures, lint.Failure{
+					Node:       result,
+					Confidence: 1,
+					Category:   "naming",
+					Failure:    "unnamed results of the same type may be confusing, consider using named results",
+				})
 
-		t, ok := result.Type.(*ast.Ident)
-		if !ok {
-			return w
-		}
+				break
+			}
 
-		if t.Name == lastType {
-			w.onFailure(lint.Failure{
-				Node:       n,
-				Confidence: 1,
-				Category:   "naming",
-				Failure:    "unnamed results of the same type may be confusing, consider using named results",
-			})
-			break
+			lastType = resultTypeName
 		}
-		lastType = t.Name
 	}
 
-	return w
+	return failures
+}
+
+// Name returns the rule name.
+func (*ConfusingResultsRule) Name() string {
+	return "confusing-results"
 }
diff --git a/rule/context_as_argument.go b/rule/context_as_argument.go
index 06dc0f2..f2f9aee 100644
--- a/rule/context_as_argument.go
+++ b/rule/context_as_argument.go
@@ -11,7 +11,7 @@ import (
 
 // ContextAsArgumentRule suggests that `context.Context` should be the first argument of a function.
 type ContextAsArgumentRule struct {
-	allowTypesLUT map[string]struct{}
+	allowTypes map[string]struct{}
 
 	configureOnce sync.Once
 }
@@ -21,20 +21,37 @@ func (r *ContextAsArgumentRule) Apply(file *lint.File, args lint.Arguments) []li
 	r.configureOnce.Do(func() { r.configure(args) })
 
 	var failures []lint.Failure
-	walker := lintContextArguments{
-		allowTypesLUT: r.allowTypesLUT,
-		onFailure: func(failure lint.Failure) {
-			failures = append(failures, failure)
-		},
-	}
+	for _, decl := range file.AST.Decls {
+		fn, ok := decl.(*ast.FuncDecl)
+		if !ok || len(fn.Type.Params.List) <= 1 {
+			continue // not a function or a function with less than 2 parameters
+		}
 
-	ast.Walk(walker, file.AST)
+		fnArgs := fn.Type.Params.List
+
+		// A context.Context should be the first parameter of a function.
+		// Flag any that show up after the first.
+		isCtxStillAllowed := true
+		for _, arg := range fnArgs {
+			argIsCtx := isPkgDot(arg.Type, "context", "Context")
+			if argIsCtx && !isCtxStillAllowed {
+				failures = append(failures, lint.Failure{
+					Node:       arg,
+					Category:   "arg-order",
+					Failure:    "context.Context should be the first parameter of a function",
+					Confidence: 0.9,
+				})
+
+				break // only flag one
+			}
 
-	return failures
-}
+			typeName := gofmt(arg.Type)
+			// a parameter of type context.Context is still allowed if the current arg type is in the allow types LookUpTable
+			_, isCtxStillAllowed = r.allowTypes[typeName]
+		}
+	}
 
-func (r *ContextAsArgumentRule) configure(arguments lint.Arguments) {
-	r.allowTypesLUT = getAllowTypesFromArguments(arguments)
+	return failures
 }
 
 // Name returns the rule name.
@@ -42,43 +59,11 @@ func (*ContextAsArgumentRule) Name() string {
 	return "context-as-argument"
 }
 
-type lintContextArguments struct {
-	allowTypesLUT map[string]struct{}
-	onFailure     func(lint.Failure)
-}
-
-func (w lintContextArguments) Visit(n ast.Node) ast.Visitor {
-	fn, ok := n.(*ast.FuncDecl)
-	if !ok || len(fn.Type.Params.List) <= 1 {
-		return w
-	}
-
-	fnArgs := fn.Type.Params.List
-
-	// A context.Context should be the first parameter of a function.
-	// Flag any that show up after the first.
-	isCtxStillAllowed := true
-	for _, arg := range fnArgs {
-		argIsCtx := isPkgDot(arg.Type, "context", "Context")
-		if argIsCtx && !isCtxStillAllowed {
-			w.onFailure(lint.Failure{
-				Node:       arg,
-				Category:   "arg-order",
-				Failure:    "context.Context should be the first parameter of a function",
-				Confidence: 0.9,
-			})
-			break // only flag one
-		}
-
-		typeName := gofmt(arg.Type)
-		// a parameter of type context.Context is still allowed if the current arg type is in the LUT
-		_, isCtxStillAllowed = w.allowTypesLUT[typeName]
-	}
-
-	return nil // avoid visiting the function body
+func (r *ContextAsArgumentRule) configure(arguments lint.Arguments) {
+	r.allowTypes = r.getAllowTypesFromArguments(arguments)
 }
 
-func getAllowTypesFromArguments(args lint.Arguments) map[string]struct{} {
+func (r *ContextAsArgumentRule) getAllowTypesFromArguments(args lint.Arguments) map[string]struct{} {
 	allowTypesBefore := []string{}
 	if len(args) >= 1 {
 		argKV, ok := args[0].(map[string]any)
diff --git a/rule/cyclomatic.go b/rule/cyclomatic.go
index 60f8441..7f42a1c 100644
--- a/rule/cyclomatic.go
+++ b/rule/cyclomatic.go
@@ -38,53 +38,30 @@ func (r *CyclomaticRule) Apply(file *lint.File, arguments lint.Arguments) []lint
 	r.configureOnce.Do(func() { r.configure(arguments) })
 
 	var failures []lint.Failure
-	fileAst := file.AST
-
-	walker := lintCyclomatic{
-		file:       file,
-		complexity: r.maxComplexity,
-		onFailure: func(failure lint.Failure) {
-			failures = append(failures, failure)
-		},
-	}
-
-	ast.Walk(walker, fileAst)
-
-	return failures
-}
-
-// Name returns the rule name.
-func (*CyclomaticRule) Name() string {
-	return "cyclomatic"
-}
-
-type lintCyclomatic struct {
-	file       *lint.File
-	complexity int
-	onFailure  func(lint.Failure)
-}
-
-func (w lintCyclomatic) Visit(_ ast.Node) ast.Visitor {
-	f := w.file
-	for _, decl := range f.AST.Decls {
+	for _, decl := range file.AST.Decls {
 		fn, ok := decl.(*ast.FuncDecl)
 		if !ok {
 			continue
 		}
 
 		c := complexity(fn)
-		if c > w.complexity {
-			w.onFailure(lint.Failure{
+		if c > r.maxComplexity {
+			failures = append(failures, lint.Failure{
 				Confidence: 1,
 				Category:   "maintenance",
 				Failure: fmt.Sprintf("function %s has cyclomatic complexity %d (> max enabled %d)",
-					funcName(fn), c, w.complexity),
+					funcName(fn), c, r.maxComplexity),
 				Node: fn,
 			})
 		}
 	}
 
-	return nil
+	return failures
+}
+
+// Name returns the rule name.
+func (*CyclomaticRule) Name() string {
+	return "cyclomatic"
 }
 
 // funcName returns the name representation of a function or method:
diff --git a/rule/error_return.go b/rule/error_return.go
index f26d807..c5d3f46 100644
--- a/rule/error_return.go
+++ b/rule/error_return.go
@@ -13,16 +13,34 @@ type ErrorReturnRule struct{}
 func (*ErrorReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
 
-	fileAst := file.AST
-	walker := lintErrorReturn{
-		file:    file,
-		fileAst: fileAst,
-		onFailure: func(failure lint.Failure) {
-			failures = append(failures, failure)
-		},
-	}
+	for _, decl := range file.AST.Decls {
+		funcDecl, ok := decl.(*ast.FuncDecl)
+		isFunctionWithMoreThanOneResult := ok && funcDecl.Type.Results != nil && len(funcDecl.Type.Results.List) > 1
+		if !isFunctionWithMoreThanOneResult {
+			continue
+		}
+
+		funcResults := funcDecl.Type.Results.List
+		isLastResultError := isIdent(funcResults[len(funcResults)-1].Type, "error")
+		if isLastResultError {
+			continue
+		}
 
-	ast.Walk(walker, fileAst)
+		// An error return parameter should be the last parameter.
+		// Flag any error parameters found before the last.
+		for _, r := range funcResults[:len(funcResults)-1] {
+			if isIdent(r.Type, "error") {
+				failures = append(failures, lint.Failure{
+					Category:   "style",
+					Confidence: 0.9,
+					Node:       funcDecl,
+					Failure:    "error should be the last type when returning multiple items",
+				})
+
+				break // only flag one
+			}
+		}
+	}
 
 	return failures
 }
@@ -31,37 +49,3 @@ func (*ErrorReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure
 func (*ErrorReturnRule) Name() string {
 	return "error-return"
 }
-
-type lintErrorReturn struct {
-	file      *lint.File
-	fileAst   *ast.File
-	onFailure func(lint.Failure)
-}
-
-func (w lintErrorReturn) Visit(n ast.Node) ast.Visitor {
-	fn, ok := n.(*ast.FuncDecl)
-	if !ok || fn.Type.Results == nil {
-		return w
-	}
-	ret := fn.Type.Results.List
-	if len(ret) <= 1 {
-		return w
-	}
-	if isIdent(ret[len(ret)-1].Type, "error") {
-		return nil
-	}
-	// An error return parameter should be the last parameter.
-	// Flag any error parameters found before the last.
-	for _, r := range ret[:len(ret)-1] {
-		if isIdent(r.Type, "error") {
-			w.onFailure(lint.Failure{
-				Category:   "arg-order",
-				Confidence: 0.9,
-				Node:       fn,
-				Failure:    "error should be the last type when returning multiple items",
-			})
-			break // only flag one
-		}
-	}
-	return w
-}
diff --git a/rule/flag_param.go b/rule/flag_param.go
index 8156121..0a67f33 100644
--- a/rule/flag_param.go
+++ b/rule/flag_param.go
@@ -13,58 +13,48 @@ type FlagParamRule struct{}
 // Apply applies the rule to given file.
 func (*FlagParamRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
-
 	onFailure := func(failure lint.Failure) {
 		failures = append(failures, failure)
 	}
 
-	w := lintFlagParamRule{onFailure: onFailure}
-	ast.Walk(w, file.AST)
-	return failures
-}
-
-// Name returns the rule name.
-func (*FlagParamRule) Name() string {
-	return "flag-parameter"
-}
-
-type lintFlagParamRule struct {
-	onFailure func(lint.Failure)
-}
-
-func (w lintFlagParamRule) Visit(node ast.Node) ast.Visitor {
-	fd, ok := node.(*ast.FuncDecl)
-	if !ok {
-		return w
-	}
-
-	if fd.Body == nil {
-		return nil // skip whole function declaration
-	}
+	for _, decl := range file.AST.Decls {
+		fd, ok := decl.(*ast.FuncDecl)
+		isFuncWithNonEmptyBody := ok && fd.Body != nil
+		if !isFuncWithNonEmptyBody {
+			continue
+		}
 
-	for _, p := range fd.Type.Params.List {
-		t := p.Type
+		boolParams := map[string]struct{}{}
+		for _, param := range fd.Type.Params.List {
+			if !isIdent(param.Type, "bool") {
+				continue
+			}
 
-		id, ok := t.(*ast.Ident)
-		if !ok {
-			continue
+			for _, paramIdent := range param.Names {
+				boolParams[paramIdent.Name] = struct{}{}
+			}
 		}
 
-		if id.Name != "bool" {
+		if len(boolParams) == 0 {
 			continue
 		}
 
-		cv := conditionVisitor{p.Names, fd, w}
+		cv := conditionVisitor{boolParams, fd, onFailure}
 		ast.Walk(cv, fd.Body)
 	}
 
-	return w
+	return failures
+}
+
+// Name returns the rule name.
+func (*FlagParamRule) Name() string {
+	return "flag-parameter"
 }
 
 type conditionVisitor struct {
-	ids    []*ast.Ident
-	fd     *ast.FuncDecl
-	linter lintFlagParamRule
+	idents    map[string]struct{}
+	fd        *ast.FuncDecl
+	onFailure func(lint.Failure)
 }
 
 func (w conditionVisitor) Visit(node ast.Node) ast.Visitor {
@@ -73,28 +63,22 @@ func (w conditionVisitor) Visit(node ast.Node) ast.Visitor {
 		return w
 	}
 
-	fselect := func(n ast.Node) bool {
+	findUsesOfIdents := func(n ast.Node) bool {
 		ident, ok := n.(*ast.Ident)
 		if !ok {
 			return false
 		}
 
-		for _, id := range w.ids {
-			if ident.Name == id.Name {
-				return true
-			}
-		}
-
-		return false
+		return w.idents[ident.Name] == struct{}{}
 	}
 
-	uses := pick(ifStmt.Cond, fselect)
+	uses := pick(ifStmt.Cond, findUsesOfIdents)
 
 	if len(uses) < 1 {
 		return w
 	}
 
-	w.linter.onFailure(lint.Failure{
+	w.onFailure(lint.Failure{
 		Confidence: 1,
 		Node:       w.fd.Type.Params,
 		Category:   "bad practice",
diff --git a/rule/function_length.go b/rule/function_length.go
index c58cd4c..6f5a59e 100644
--- a/rule/function_length.go
+++ b/rule/function_length.go
@@ -28,17 +28,40 @@ func (r *FunctionLength) Apply(file *lint.File, arguments lint.Arguments) []lint
 	r.configureOnce.Do(func() { r.configure(arguments) })
 
 	var failures []lint.Failure
+	for _, decl := range file.AST.Decls {
+		funcDecl, ok := decl.(*ast.FuncDecl)
+		if !ok {
+			continue
+		}
 
-	walker := lintFuncLength{
-		file:     file,
-		maxStmt:  r.maxStmt,
-		maxLines: r.maxLines,
-		onFailure: func(failure lint.Failure) {
-			failures = append(failures, failure)
-		},
-	}
+		body := funcDecl.Body
+		emptyBody := body == nil || len(body.List) == 0
+		if emptyBody {
+			return nil
+		}
+
+		if r.maxStmt > 0 {
+			stmtCount := r.countStmts(body.List)
+			if stmtCount > r.maxStmt {
+				failures = append(failures, lint.Failure{
+					Confidence: 1,
+					Failure:    fmt.Sprintf("maximum number of statements per function exceeded; max %d but got %d", r.maxStmt, stmtCount),
+					Node:       funcDecl,
+				})
+			}
+		}
 
-	ast.Walk(walker, file.AST)
+		if r.maxLines > 0 {
+			lineCount := r.countLines(body, file)
+			if lineCount > r.maxLines {
+				failures = append(failures, lint.Failure{
+					Confidence: 1,
+					Failure:    fmt.Sprintf("maximum number of lines per function exceeded; max %d but got %d", r.maxLines, lineCount),
+					Node:       funcDecl,
+				})
+			}
+		}
+	}
 
 	return failures
 }
@@ -80,79 +103,35 @@ func (*FunctionLength) parseArguments(arguments lint.Arguments) (maxStmt, maxLin
 	return maxStmt, maxLines
 }
 
-type lintFuncLength struct {
-	file      *lint.File
-	maxStmt   int
-	maxLines  int
-	onFailure func(lint.Failure)
-}
-
-func (w lintFuncLength) Visit(n ast.Node) ast.Visitor {
-	node, ok := n.(*ast.FuncDecl)
-	if !ok {
-		return w
-	}
-
-	body := node.Body
-	emptyBody := body == nil || len(node.Body.List) == 0
-	if emptyBody {
-		return nil
-	}
-
-	if w.maxStmt > 0 {
-		stmtCount := w.countStmts(node.Body.List)
-		if stmtCount > w.maxStmt {
-			w.onFailure(lint.Failure{
-				Confidence: 1,
-				Failure:    fmt.Sprintf("maximum number of statements per function exceeded; max %d but got %d", w.maxStmt, stmtCount),
-				Node:       node,
-			})
-		}
-	}
-
-	if w.maxLines > 0 {
-		lineCount := w.countLines(node.Body)
-		if lineCount > w.maxLines {
-			w.onFailure(lint.Failure{
-				Confidence: 1,
-				Failure:    fmt.Sprintf("maximum number of lines per function exceeded; max %d but got %d", w.maxLines, lineCount),
-				Node:       node,
-			})
-		}
-	}
-
-	return nil
-}
-
-func (w lintFuncLength) countLines(b *ast.BlockStmt) int {
-	return w.file.ToPosition(b.End()).Line - w.file.ToPosition(b.Pos()).Line - 1
+func (*FunctionLength) countLines(b *ast.BlockStmt, file *lint.File) int {
+	return file.ToPosition(b.End()).Line - file.ToPosition(b.Pos()).Line - 1
 }
 
-func (w lintFuncLength) countStmts(b []ast.Stmt) int {
+func (r *FunctionLength) countStmts(b []ast.Stmt) int {
 	count := 0
 	for _, s := range b {
 		switch stmt := s.(type) {
 		case *ast.BlockStmt:
-			count += w.countStmts(stmt.List)
+			count += r.countStmts(stmt.List)
 		case *ast.IfStmt:
-			count += 1 + w.countBodyListStmts(stmt)
+			count += 1 + r.countBodyListStmts(stmt)
 			if stmt.Else != nil {
 				elseBody, ok := stmt.Else.(*ast.BlockStmt)
 				if ok {
-					count += w.countStmts(elseBody.List)
+					count += r.countStmts(elseBody.List)
 				}
 			}
 		case *ast.ForStmt, *ast.RangeStmt,
 			*ast.SwitchStmt, *ast.TypeSwitchStmt, *ast.SelectStmt:
-			count += 1 + w.countBodyListStmts(stmt)
+			count += 1 + r.countBodyListStmts(stmt)
 		case *ast.CaseClause:
-			count += w.countStmts(stmt.Body)
+			count += r.countStmts(stmt.Body)
 		case *ast.AssignStmt:
-			count += 1 + w.countFuncLitStmts(stmt.Rhs[0])
+			count += 1 + r.countFuncLitStmts(stmt.Rhs[0])
 		case *ast.GoStmt:
-			count += 1 + w.countFuncLitStmts(stmt.Call.Fun)
+			count += 1 + r.countFuncLitStmts(stmt.Call.Fun)
 		case *ast.DeferStmt:
-			count += 1 + w.countFuncLitStmts(stmt.Call.Fun)
+			count += 1 + r.countFuncLitStmts(stmt.Call.Fun)
 		default:
 			count++
 		}
@@ -161,14 +140,15 @@ func (w lintFuncLength) countStmts(b []ast.Stmt) int {
 	return count
 }
 
-func (w lintFuncLength) countFuncLitStmts(stmt ast.Expr) int {
+func (r *FunctionLength) countFuncLitStmts(stmt ast.Expr) int {
 	if block, ok := stmt.(*ast.FuncLit); ok {
-		return w.countStmts(block.Body.List)
+		return r.countStmts(block.Body.List)
 	}
+
 	return 0
 }
 
-func (w lintFuncLength) countBodyListStmts(t any) int {
+func (r *FunctionLength) countBodyListStmts(t any) int {
 	i := reflect.ValueOf(t).Elem().FieldByName(`Body`).Elem().FieldByName(`List`).Interface()
-	return w.countStmts(i.([]ast.Stmt))
+	return r.countStmts(i.([]ast.Stmt))
 }
diff --git a/rule/modifies_value_receiver.go b/rule/modifies_value_receiver.go
index 2f92991..d811c48 100644
--- a/rule/modifies_value_receiver.go
+++ b/rule/modifies_value_receiver.go
@@ -12,99 +12,35 @@ import (
 type ModifiesValRecRule struct{}
 
 // Apply applies the rule to given file.
-func (*ModifiesValRecRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
+func (r *ModifiesValRecRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
 	var failures []lint.Failure
 
-	onFailure := func(failure lint.Failure) {
-		failures = append(failures, failure)
-	}
-
-	w := lintModifiesValRecRule{file: file, onFailure: onFailure}
 	file.Pkg.TypeCheck()
-	ast.Walk(w, file.AST)
-
-	return failures
-}
-
-// Name returns the rule name.
-func (*ModifiesValRecRule) Name() string {
-	return "modifies-value-receiver"
-}
-
-type lintModifiesValRecRule struct {
-	file      *lint.File
-	onFailure func(lint.Failure)
-}
-
-func (w lintModifiesValRecRule) Visit(node ast.Node) ast.Visitor {
-	switch n := node.(type) {
-	case *ast.FuncDecl:
-		if n.Recv == nil {
-			return nil // skip, not a method
-		}
-
-		receiver := n.Recv.List[0]
-		if _, ok := receiver.Type.(*ast.StarExpr); ok {
-			return nil // skip, method with pointer receiver
+	for _, decl := range file.AST.Decls {
+		funcDecl, ok := decl.(*ast.FuncDecl)
+		isAMethod := ok && funcDecl.Recv != nil
+		if !isAMethod {
+			continue // skip, not a method
 		}
 
-		if w.skipType(receiver.Type) {
-			return nil // skip, receiver is a map or array
-		}
-
-		if len(receiver.Names) < 1 {
-			return nil // skip, anonymous receiver
+		receiver := funcDecl.Recv.List[0]
+		if r.mustSkip(receiver, file.Pkg) {
+			continue
 		}
 
 		receiverName := receiver.Names[0].Name
-		if receiverName == "_" {
-			return nil // skip, anonymous receiver
-		}
-
-		receiverAssignmentFinder := func(n ast.Node) bool {
-			// look for assignments with the receiver in the right hand
-			assignment, ok := n.(*ast.AssignStmt)
-			if !ok {
-				return false
-			}
-
-			for _, exp := range assignment.Lhs {
-				switch e := exp.(type) {
-				case *ast.IndexExpr: // receiver...[] = ...
-					continue
-				case *ast.StarExpr: // *receiver = ...
-					continue
-				case *ast.SelectorExpr: // receiver.field = ...
-					name := w.getNameFromExpr(e.X)
-					if name == "" || name != receiverName {
-						continue
-					}
-				case *ast.Ident: // receiver := ...
-					if e.Name != receiverName {
-						continue
-					}
-				default:
-					continue
-				}
-
-				return true
-			}
-
-			return false
-		}
-
-		assignmentsToReceiver := pick(n.Body, receiverAssignmentFinder)
+		assignmentsToReceiver := r.getAssignmentsToReceiver(receiverName, funcDecl.Body)
 		if len(assignmentsToReceiver) == 0 {
-			return nil // receiver is not modified
+			continue // receiver is not modified
 		}
 
-		methodReturnsReceiver := len(w.findReturnReceiverStatements(receiverName, n.Body)) > 0
+		methodReturnsReceiver := len(r.findReturnReceiverStatements(receiverName, funcDecl.Body)) > 0
 		if methodReturnsReceiver {
-			return nil // modification seems legit (see issue #1066)
+			continue // modification seems legit (see issue #1066)
 		}
 
 		for _, assignment := range assignmentsToReceiver {
-			w.onFailure(lint.Failure{
+			failures = append(failures, lint.Failure{
 				Node:       assignment,
 				Confidence: 1,
 				Failure:    "suspicious assignment to a by-value method receiver",
@@ -112,11 +48,16 @@ func (w lintModifiesValRecRule) Visit(node ast.Node) ast.Visitor {
 		}
 	}
 
-	return w
+	return failures
+}
+
+// Name returns the rule name.
+func (*ModifiesValRecRule) Name() string {
+	return "modifies-value-receiver"
 }
 
-func (w lintModifiesValRecRule) skipType(t ast.Expr) bool {
-	rt := w.file.Pkg.TypeOf(t)
+func (r *ModifiesValRecRule) skipType(t ast.Expr, pkg *lint.Package) bool {
+	rt := pkg.TypeOf(t)
 	if rt == nil {
 		return false
 	}
@@ -128,7 +69,7 @@ func (w lintModifiesValRecRule) skipType(t ast.Expr) bool {
 	return strings.HasPrefix(rtName, "[]") || strings.HasPrefix(rtName, "map[")
 }
 
-func (lintModifiesValRecRule) getNameFromExpr(ie ast.Expr) string {
+func (*ModifiesValRecRule) getNameFromExpr(ie ast.Expr) string {
 	ident, ok := ie.(*ast.Ident)
 	if !ok {
 		return ""
@@ -137,7 +78,7 @@ func (lintModifiesValRecRule) getNameFromExpr(ie ast.Expr) string {
 	return ident.Name
 }
 
-func (w lintModifiesValRecRule) findReturnReceiverStatements(receiverName string, target ast.Node) []ast.Node {
+func (r *ModifiesValRecRule) findReturnReceiverStatements(receiverName string, target ast.Node) []ast.Node {
 	finder := func(n ast.Node) bool {
 		// look for returns with the receiver as value
 		returnStatement, ok := n.(*ast.ReturnStmt)
@@ -148,7 +89,7 @@ func (w lintModifiesValRecRule) findReturnReceiverStatements(receiverName string
 		for _, exp := range returnStatement.Results {
 			switch e := exp.(type) {
 			case *ast.SelectorExpr: // receiver.field = ...
-				name := w.getNameFromExpr(e.X)
+				name := r.getNameFromExpr(e.X)
 				if name == "" || name != receiverName {
 					continue
 				}
@@ -160,7 +101,7 @@ func (w lintModifiesValRecRule) findReturnReceiverStatements(receiverName string
 				if e.Op != token.AND {
 					continue
 				}
-				name := w.getNameFromExpr(e.X)
+				name := r.getNameFromExpr(e.X)
 				if name == "" || name != receiverName {
 					continue
 				}
@@ -177,3 +118,60 @@ func (w lintModifiesValRecRule) findReturnReceiverStatements(receiverName string
 
 	return pick(target, finder)
 }
+
+func (r *ModifiesValRecRule) mustSkip(receiver *ast.Field, pkg *lint.Package) bool {
+	if _, ok := receiver.Type.(*ast.StarExpr); ok {
+		return true // skip, method with pointer receiver
+	}
+
+	if len(receiver.Names) < 1 {
+		return true // skip, anonymous receiver
+	}
+
+	receiverName := receiver.Names[0].Name
+	if receiverName == "_" {
+		return true // skip, anonymous receiver
+	}
+
+	if r.skipType(receiver.Type, pkg) {
+		return true // skip, receiver is a map or array
+	}
+
+	return false
+}
+
+func (r *ModifiesValRecRule) getAssignmentsToReceiver(receiverName string, funcBody *ast.BlockStmt) []ast.Node {
+	receiverAssignmentFinder := func(n ast.Node) bool {
+		// look for assignments with the receiver in the right hand
+		assignment, ok := n.(*ast.AssignStmt)
+		if !ok {
+			return false
+		}
+
+		for _, exp := range assignment.Lhs {
+			switch e := exp.(type) {
+			case *ast.IndexExpr: // receiver...[] = ...
+				continue
+			case *ast.StarExpr: // *receiver = ...
+				continue
+			case *ast.SelectorExpr: // receiver.field = ...
+				name := r.getNameFromExpr(e.X)
+				if name == "" || name != receiverName {
+					continue
+				}
+			case *ast.Ident: // receiver := ...
+				if e.Name != receiverName {
+					continue
+				}
+			default:
+				continue
+			}
+
+			return true
+		}
+
+		return false
+	}
+
+	return pick(funcBody, receiverAssignmentFinder)
+}


Test patch:
diff --git a/test/argument_limit_test.go b/test/argument_limit_test.go
index cbe6be2..b89b0ec 100644
--- a/test/argument_limit_test.go
+++ b/test/argument_limit_test.go
@@ -7,12 +7,19 @@ import (
 	"github.com/mgechev/revive/rule"
 )
 
-func TestArgumentLimitDefault(t *testing.T) {
+func TestArgumentsLimitDefault(t *testing.T) {
 	testRule(t, "argument_limit_default", &rule.ArgumentsLimitRule{}, &lint.RuleConfig{})
 }
 
-func TestArgumentLimit(t *testing.T) {
+func TestArgumentsLimit(t *testing.T) {
 	testRule(t, "argument_limit", &rule.ArgumentsLimitRule{}, &lint.RuleConfig{
 		Arguments: []any{int64(3)},
 	})
 }
+
+func BenchmarkArgumentsLimit(b *testing.B) {
+	var t *testing.T
+	for i := 0; i <= b.N; i++ {
+		testRule(t, "argument_limit_default", &rule.ArgumentsLimitRule{}, &lint.RuleConfig{})
+	}
+}
diff --git a/testdata/confusing_results.go b/testdata/confusing_results.go
index fc47abe..6f45414 100644
--- a/testdata/confusing_results.go
+++ b/testdata/confusing_results.go
@@ -15,6 +15,14 @@ func GetTaz(a string, b int) string {
 
 }
 
-func (t *t) GetTaz(a int, b int)  {
+func (t *t) GetTaz(a int, b int) {
 
 }
+
+func namedResults() (a string, b string) {
+	return "nil", "nil"
+}
+
+func pointerResults() (*string, *string) { // MATCH /unnamed results of the same type may be confusing, consider using named results/
+	return nil, nil
+}