From ccdad6d71ed15d52caa53a52cc8d3adeace76511 Mon Sep 17 00:00:00 2001
From: chavacava <salvador.cavadini@gmail.com>
Date: Sat, 7 Dec 2024 12:37:34 +0100
Subject: [PATCH] fix(rule/modifies-value-receiver): miss modification by ++/--

---
 rule/modifies_value_receiver.go     | 56 ++++++++++++++++-------------
 testdata/modifies_value_receiver.go | 19 ++++++++++
 2 files changed, 50 insertions(+), 25 deletions(-)

diff --git a/rule/modifies_value_receiver.go b/rule/modifies_value_receiver.go
index d811c486b..a33a79ac2 100644
--- a/rule/modifies_value_receiver.go
+++ b/rule/modifies_value_receiver.go
@@ -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
 		}
@@ -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)
 }
diff --git a/testdata/modifies_value_receiver.go b/testdata/modifies_value_receiver.go
index e6a716606..11757c9b7 100644
--- a/testdata/modifies_value_receiver.go
+++ b/testdata/modifies_value_receiver.go
@@ -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/
+}