mirror of
				https://github.com/mgechev/revive.git
				synced 2025-10-30 23:37:49 +02:00 
			
		
		
		
	feature: new rule redundant-assignment
				
					
				
			This commit is contained in:
		| @@ -576,6 +576,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | ||||
| | [`range`](./RULES_DESCRIPTIONS.md#range)               |  n/a   | Prevents redundant variables when iterating over a collection.   |   yes    |  no   | | ||||
| | [`receiver-naming`](./RULES_DESCRIPTIONS.md#receiver-naming)     |  map (optional)   | Conventions around the naming of receivers.                      |   yes    |  no   | | ||||
| | [`redefines-builtin-id`](./RULES_DESCRIPTIONS.md#redefines-builtin-id)|  n/a   | Warns on redefinitions of builtin identifiers                    |    no    |  no   | | ||||
| | [`redundant-assignment`](./RULES_DESCRIPTIONS.md#redundant-assignment) | n/a   | Warns about redundant assignments of range variables |   no    |  no   | | ||||
| | [`redundant-build-tag`](./RULES_DESCRIPTIONS.md#redundant-build-tag) | n/a   | Warns about redundant `// +build` comment lines |   no    |  no   | | ||||
| | [`redundant-import-alias`](./RULES_DESCRIPTIONS.md#redundant-import-alias)          |  n/a   |  Warns on import aliases matching the imported package name |    no    |  no   | | ||||
| | [`redundant-test-main-exit`](./RULES_DESCRIPTIONS.md#redundant-test-main-exit) |  n/a   | Suggests removing `Exit` call in `TestMain` function for test files |    no    |  no   | | ||||
|   | ||||
| @@ -72,6 +72,7 @@ List of all available rules. | ||||
| - [range](#range) | ||||
| - [receiver-naming](#receiver-naming) | ||||
| - [redefines-builtin-id](#redefines-builtin-id) | ||||
| - [redundant-assignment](#redundant-assignment) | ||||
| - [redundant-build-tag](#redundant-build-tag) | ||||
| - [redundant-import-alias](#redundant-import-alias) | ||||
| - [redundant-test-main-exit](#redundant-test-main-exit) | ||||
| @@ -1201,6 +1202,16 @@ Even if possible, redefining these built in names can lead to bugs very difficul | ||||
|  | ||||
| _Configuration_: N/A | ||||
|  | ||||
| ## redundant-assignment | ||||
|  | ||||
| _Description_: Detects and warns about unnecessary reassignments of  | ||||
| range loop variables to new variables within the loop body. | ||||
| Prior to Go 1.22, range variables were instantiated once per loop and reused in each iteration. | ||||
| A common idiom to create a per-iteration copy was to reassign the variable inside the block (e.g., `a := a`) | ||||
| Since Go 1.22, range variables are per-iteration, making this pattern obsolete. | ||||
|  | ||||
| _Configuration_: N/A | ||||
|  | ||||
| ## redundant-build-tag | ||||
|  | ||||
| _Description_: This rule warns about redundant build tag comments `// +build` when `//go:build` is present. | ||||
|   | ||||
| @@ -116,6 +116,7 @@ var allRules = append([]lint.Rule{ | ||||
| 	&rule.InefficientMapLookupRule{}, | ||||
| 	&rule.ForbiddenCallInWgGoRule{}, | ||||
| 	&rule.UnnecessaryIfRule{}, | ||||
| 	&rule.RedundantAssignmentRule{}, | ||||
| }, defaultRules...) | ||||
|  | ||||
| // allFormatters is a list of all available formatters to output the linting results. | ||||
|   | ||||
| @@ -118,6 +118,16 @@ func IsPkgDotName(expr ast.Expr, pkg, name string) bool { | ||||
| 	return ok && IsIdent(sel.X, pkg) && IsIdent(sel.Sel, name) | ||||
| } | ||||
|  | ||||
| // GetIdentName returns the name of the identifier if the given expression is an identifier, false otherwise. | ||||
| func GetIdentName(expr ast.Expr) (string, bool) { | ||||
| 	id, ok := expr.(*ast.Ident) | ||||
| 	if !ok { | ||||
| 		return "", false | ||||
| 	} | ||||
|  | ||||
| 	return id.Name, true | ||||
| } | ||||
|  | ||||
| // PickNodes yields a list of nodes by picking them from a sub-ast with root node n. | ||||
| // Nodes are selected by applying the selector function. | ||||
| func PickNodes(n ast.Node, selector func(n ast.Node) bool) []ast.Node { | ||||
|   | ||||
							
								
								
									
										85
									
								
								rule/redundant_assignment.go
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										85
									
								
								rule/redundant_assignment.go
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,85 @@ | ||||
| package rule | ||||
|  | ||||
| import ( | ||||
| 	"fmt" | ||||
| 	"go/ast" | ||||
| 	"go/token" | ||||
|  | ||||
| 	"github.com/mgechev/revive/internal/astutils" | ||||
| 	"github.com/mgechev/revive/lint" | ||||
| ) | ||||
|  | ||||
| // RedundantAssignmentRule detects unnecessary "self-assignments" of range variables. | ||||
| type RedundantAssignmentRule struct{} | ||||
|  | ||||
| // Apply applies the rule to given file. | ||||
| func (*RedundantAssignmentRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { | ||||
| 	if !file.Pkg.IsAtLeastGoVersion(lint.Go122) { | ||||
| 		return nil | ||||
| 	} | ||||
|  | ||||
| 	var failures []lint.Failure | ||||
|  | ||||
| 	onFailure := func(failure lint.Failure) { | ||||
| 		failures = append(failures, failure) | ||||
| 	} | ||||
|  | ||||
| 	w := &lintRedundantAssignmentRule{ | ||||
| 		onFailure: onFailure, | ||||
| 	} | ||||
|  | ||||
| 	ast.Walk(w, file.AST) | ||||
|  | ||||
| 	return failures | ||||
| } | ||||
|  | ||||
| // Name returns the rule name. | ||||
| func (*RedundantAssignmentRule) Name() string { | ||||
| 	return "redundant-assignment" | ||||
| } | ||||
|  | ||||
| type lintRedundantAssignmentRule struct { | ||||
| 	onFailure func(lint.Failure) | ||||
| } | ||||
|  | ||||
| func (w *lintRedundantAssignmentRule) Visit(node ast.Node) ast.Visitor { | ||||
| 	// Only interested in  range statements | ||||
| 	rangeStmt, ok := node.(*ast.RangeStmt) | ||||
| 	if !ok { | ||||
| 		return w // not a range statement | ||||
| 	} | ||||
|  | ||||
| 	body := rangeStmt.Body | ||||
| 	if body == nil || len(body.List) == 0 { | ||||
| 		return nil // empty body | ||||
| 	} | ||||
|  | ||||
| 	stmt := body.List[0] | ||||
|  | ||||
| 	assignStmt, ok := stmt.(*ast.AssignStmt) | ||||
| 	if !ok || assignStmt.Tok != token.DEFINE || len(assignStmt.Lhs) != 1 { | ||||
| 		return w // not an assignment statement of the form x := something | ||||
| 	} | ||||
|  | ||||
| 	lhsStr, _ := astutils.GetIdentName(assignStmt.Lhs[0]) | ||||
| 	rhsStr, _ := astutils.GetIdentName(assignStmt.Rhs[0]) | ||||
| 	if lhsStr != rhsStr { | ||||
| 		return w // not an assignment of the form x := x | ||||
| 	} | ||||
|  | ||||
| 	keyStr, _ := astutils.GetIdentName(rangeStmt.Key) | ||||
| 	valueStr, _ := astutils.GetIdentName(rangeStmt.Value) | ||||
|  | ||||
| 	if lhsStr != keyStr && lhsStr != valueStr { | ||||
| 		return w // the assigned variable is not from the range statement | ||||
| 	} | ||||
|  | ||||
| 	w.onFailure(lint.Failure{ | ||||
| 		Confidence: 1, | ||||
| 		Node:       assignStmt, | ||||
| 		Category:   lint.FailureCategoryOptimization, | ||||
| 		Failure:    fmt.Sprintf("redundant assignment of range variable %q, use it directly", lhsStr), | ||||
| 	}) | ||||
|  | ||||
| 	return w | ||||
| } | ||||
							
								
								
									
										13
									
								
								test/redundant_assignment_test.go
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										13
									
								
								test/redundant_assignment_test.go
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,13 @@ | ||||
| package test | ||||
|  | ||||
| import ( | ||||
| 	"testing" | ||||
|  | ||||
| 	"github.com/mgechev/revive/lint" | ||||
| 	"github.com/mgechev/revive/rule" | ||||
| ) | ||||
|  | ||||
| func TestRedundantAssignment(t *testing.T) { | ||||
| 	testRule(t, "redundant_assignment", &rule.RedundantAssignmentRule{}, &lint.RuleConfig{}) | ||||
| 	testRule(t, "go1.22/redundant_assignment", &rule.RedundantAssignmentRule{}, &lint.RuleConfig{}) | ||||
| } | ||||
							
								
								
									
										41
									
								
								testdata/go1.22/redundant_assignment.go
									
									
									
									
										vendored
									
									
										Normal file
									
								
							
							
						
						
									
										41
									
								
								testdata/go1.22/redundant_assignment.go
									
									
									
									
										vendored
									
									
										Normal file
									
								
							| @@ -0,0 +1,41 @@ | ||||
| package fixtures | ||||
|  | ||||
| func redundantAssignment() { | ||||
| 	for a, b := range collection { | ||||
| 		a := a // MATCH /redundant assignment of range variable "a", use it directly/ | ||||
| 		something(a, b) | ||||
| 	} | ||||
|  | ||||
| 	for a, b := range collection { | ||||
| 		for a, b := range collection { | ||||
| 			a := a // MATCH /redundant assignment of range variable "a", use it directly/ | ||||
| 			something(a, b) | ||||
| 		} | ||||
| 	} | ||||
|  | ||||
| 	for a, b := range collection { | ||||
| 		b := b // MATCH /redundant assignment of range variable "b", use it directly/ | ||||
| 		something(a, b) | ||||
| 	} | ||||
|  | ||||
| 	for a := range collection { | ||||
| 		a := a // MATCH /redundant assignment of range variable "a", use it directly/ | ||||
| 		something(a, b) | ||||
| 	} | ||||
|  | ||||
| 	for _, a := range collection { | ||||
| 		a := a // MATCH /redundant assignment of range variable "a", use it directly/ | ||||
| 		something(a, b) | ||||
| 	} | ||||
|  | ||||
| 	// should not report | ||||
| 	for range collection { | ||||
| 		a := a + b | ||||
| 		something(a, b) | ||||
| 	} | ||||
|  | ||||
| 	for a, b := range collection { | ||||
| 		a := a + b | ||||
| 		something(a, b) | ||||
| 	} | ||||
| } | ||||
							
								
								
									
										40
									
								
								testdata/redundant_assignment.go
									
									
									
									
										vendored
									
									
										Normal file
									
								
							
							
						
						
									
										40
									
								
								testdata/redundant_assignment.go
									
									
									
									
										vendored
									
									
										Normal file
									
								
							| @@ -0,0 +1,40 @@ | ||||
| package fixtures | ||||
|  | ||||
| func redundantAssignment() { | ||||
| 	for a, b := range collection { | ||||
| 		a := a | ||||
| 		something(a, b) | ||||
| 	} | ||||
|  | ||||
| 	for a, b := range collection { | ||||
| 		for a, b := range collection { | ||||
| 			a := a | ||||
| 			something(a, b) | ||||
| 		} | ||||
| 	} | ||||
|  | ||||
| 	for a, b := range collection { | ||||
| 		b := b | ||||
| 		something(a, b) | ||||
| 	} | ||||
|  | ||||
| 	for a := range collection { | ||||
| 		a := a | ||||
| 		something(a, b) | ||||
| 	} | ||||
|  | ||||
| 	for _, a := range collection { | ||||
| 		a := a | ||||
| 		something(a, b) | ||||
| 	} | ||||
|  | ||||
| 	for range collection { | ||||
| 		a := a + b | ||||
| 		something(a, b) | ||||
| 	} | ||||
|  | ||||
| 	for a, b := range collection { | ||||
| 		a := a + b | ||||
| 		something(a, b) | ||||
| 	} | ||||
| } | ||||
		Reference in New Issue
	
	Block a user