Repository: mgechev/revive (go)

Issue:
modifies-value-receiver ignores modification to maps and slices
**Describe the bug**
The `modifies-value-receiver` rule is coded to ignore modifications where the object being modified is `map` or `[]foo`.
This is inappropriate: modifying those values is just as wrong as other types of values.
Assignments to a part of the map or slice (i.e. indexing operations) should still be ignored.

**To Reproduce**

Steps to reproduce the behavior:
1. I updated revive `go install github.com/mgechev/revive@latest`
2. I modified the test case `modifies-value-receiver.go`, assigning to `items`:
```
package fixtures

type data struct {
	num   int
	key   *string
	items map[string]bool
}

func (this data) vmethod() {
	this.items = make(map[string]bool) // MATCH /suspicious assignment to a by-value method receiver/
	this.items["vmethod"] = true
}
```

3. I run it with the following flags & configuration file:

```shell
revive -config ./defaults.toml ./testdata/modifies-value-receiver.go 
```

```toml
# config file
[rule.modifies-value-receiver]
```

**Expected behavior**
I expect a warning on the line assigning to `this.items`.

**Desktop (please complete the following information):**
 - OS: Ubuntu 20.04
 - Version of Go 1.21.1

**Additional context**
The motivating case is here: https://github.com/prometheus/prometheus/pull/13040/files#diff-d9f058bd14a07ebec78d70aaa8d6e8e705bc09086e338b74b12666e7ac121605R1711


Pull request:
modifies-value-receiver: warn on slice or map
Assignments which index into the slice or map are already ignored on line 72, and those where the whole receiver is slice or map on line 50. We should not ignore assignments where the member variable is slice or map.

Closes #942 


Patch:
diff --git a/go.mod b/go.mod
index ec8a499..276b9fe 100644
--- a/go.mod
+++ b/go.mod
@@ -12,7 +12,7 @@ require (
 	github.com/olekukonko/tablewriter v0.0.5
 	github.com/pkg/errors v0.9.1
 	github.com/spf13/afero v1.10.0
-	golang.org/x/tools v0.15.0
+	golang.org/x/tools v0.16.0
 )
 
 replace github.com/spf13/afero => github.com/spf13/afero v1.10.0
diff --git a/go.sum b/go.sum
index 2ced0b9..d5e0eb0 100644
--- a/go.sum
+++ b/go.sum
@@ -375,6 +375,8 @@ golang.org/x/tools v0.14.0 h1:jvNa2pY0M4r62jkRQ6RwEZZyPcymeL9XZMLBbV7U2nc=
 golang.org/x/tools v0.14.0/go.mod h1:uYBEerGOWcJyEORxN+Ek8+TT266gXkNlHdJBwexUsBg=
 golang.org/x/tools v0.15.0 h1:zdAyfUGbYmuVokhzVmghFl2ZJh5QhcfebBgmVPFYA+8=
 golang.org/x/tools v0.15.0/go.mod h1:hpksKq4dtpQWS1uQ61JkdqWM3LscIS6Slf+VVkm+wQk=
+golang.org/x/tools v0.16.0 h1:GO788SKMRunPIBCXiQyo2AaexLstOrVhuAL5YwsckQM=
+golang.org/x/tools v0.16.0/go.mod h1:kYVVN6I1mBNoB1OX+noeBjbRk4IUEPa7JJ+TJMEooJ0=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
diff --git a/rule/modifies-value-receiver.go b/rule/modifies-value-receiver.go
index ae260a9..e9e64b9 100644
--- a/rule/modifies-value-receiver.go
+++ b/rule/modifies-value-receiver.go
@@ -78,11 +78,6 @@ func (w lintModifiesValRecRule) Visit(node ast.Node) ast.Visitor {
 					if name == "" || name != receiverName {
 						continue
 					}
-
-					if w.skipType(ast.Expr(e.Sel)) {
-						continue
-					}
-
 				case *ast.Ident: // receiver := ...
 					if e.Name != receiverName {
 						continue


Test patch:
diff --git a/testdata/modifies-value-receiver.go b/testdata/modifies-value-receiver.go
index e7355a4..b5ae877 100644
--- a/testdata/modifies-value-receiver.go
+++ b/testdata/modifies-value-receiver.go
@@ -9,5 +9,6 @@ type data struct {
 func (this data) vmethod() {
 	this.num = 8 // MATCH /suspicious assignment to a by-value method receiver/
 	*this.key = "v.key"
+	this.items = make(map[string]bool) // MATCH /suspicious assignment to a by-value method receiver/
 	this.items["vmethod"] = true
 }