Skip to content

Commit

Permalink
fix(rule/modifies-value-receiver): miss modification by ++/-- (#1175)
Browse files Browse the repository at this point in the history
  • Loading branch information
chavacava authored Dec 7, 2024
1 parent 3e1dbc0 commit a106e63
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 25 deletions.
56 changes: 31 additions & 25 deletions rule/modifies_value_receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func (r *ModifiesValRecRule) Apply(file *lint.File, _ lint.Arguments) []lint.Fai
}

receiverName := receiver.Names[0].Name
assignmentsToReceiver := r.getAssignmentsToReceiver(receiverName, funcDecl.Body)
assignmentsToReceiver := r.getReceiverModifications(receiverName, funcDecl.Body)
if len(assignmentsToReceiver) == 0 {
continue // receiver is not modified
}
Expand Down Expand Up @@ -140,38 +140,44 @@ func (r *ModifiesValRecRule) mustSkip(receiver *ast.Field, pkg *lint.Package) bo
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
}
func (r *ModifiesValRecRule) getReceiverModifications(receiverName string, funcBody *ast.BlockStmt) []ast.Node {
receiverModificationFinder := func(n ast.Node) bool {
switch node := n.(type) {
case *ast.IncDecStmt:
se, ok := node.X.(*ast.SelectorExpr)
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 {
name := r.getNameFromExpr(se.X)
return name == receiverName
case *ast.AssignStmt:
// look for assignments with the receiver in the right hand
for _, exp := range node.Lhs {
switch e := exp.(type) {
case *ast.IndexExpr: // receiver...[] = ...
continue
}
case *ast.Ident: // receiver := ...
if e.Name != receiverName {
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
}
default:
continue
}

return true
return true
}
}

return false
}

return pick(funcBody, receiverAssignmentFinder)
return pick(funcBody, receiverModificationFinder)
}
19 changes: 19 additions & 0 deletions testdata/modifies_value_receiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,22 @@ func (b JailerCommandBuilder) WithBin(bin string) JailerCommandBuilder {
b.bin = bin
return b
}

func (this data) incrementDecrement() {
this.num++ // MATCH /suspicious assignment to a by-value method receiver/
this.num-- // MATCH /suspicious assignment to a by-value method receiver/
other++
}

func (this data) assignmentOperators() {
this.num += 1 // MATCH /suspicious assignment to a by-value method receiver/
this.num -= 1 // MATCH /suspicious assignment to a by-value method receiver/
this.num *= 1 // MATCH /suspicious assignment to a by-value method receiver/
this.num /= 1 // MATCH /suspicious assignment to a by-value method receiver/
this.num %= 1 // MATCH /suspicious assignment to a by-value method receiver/
this.num &= 1 // MATCH /suspicious assignment to a by-value method receiver/
this.num ^= 1 // MATCH /suspicious assignment to a by-value method receiver/
this.num |= 1 // MATCH /suspicious assignment to a by-value method receiver/
this.num >>= 1 // MATCH /suspicious assignment to a by-value method receiver/
this.num <<= 1 // MATCH /suspicious assignment to a by-value method receiver/
}

0 comments on commit a106e63

Please sign in to comment.