Repository: mgechev/revive (go)

Issue:
The `max-public-structs` rule starts finding many false positives in version `1.5.0`
**Describe the bug**
The `max-public-structs` rule starts finding many false positives in version `1.5.0`.
Version `1.4.0`, with the same configuration, doesn't find any findings.
I will demonstrate this using the repository https://github.com/influxdata/telegraf (over 650k lines) as an example.

**To Reproduce**
Steps to reproduce the behavior:
1. I updated revive `go install github.com/mgechev/revive@latest`
2. I cloned repo and switched to particular branch
```
git clone https://github.com/influxdata/telegraf.git
cd telegraf
git checkout v1.32.2
```
3. I run it with the following flags & configuration file:
```shell
# flags
revive -config revive.toml ./... | sort
```
```toml
#revive.toml
[rule.max-public-structs]
  arguments = [14]
```
4. There are a `632` false positives.

**Expected behavior**
I would like there to be no findings for the given repo, branch and configuration - just as it was for the previous version of `revive` (`1.4.0`)."

**Logs**
All findings attached: 
[revive_1.5.0_findings_for_telegraf.txt](https://github.com/user-attachments/files/17686531/revive_1.5.0_findings_for_telegraf.txt)

**Desktop (please complete the following information):**
 - OS: Ubuntu 23:10
 - Version of Go:
```shell
$ go version 
go version go1.23.3 linux/amd64
```


Pull request description:
fix: fixes #1103 by fixing the processing of max-public-structs arguments
Closes #1103 

Patch:
diff --git a/README.md b/README.md
index 7b1e8fe..e40bf97 100644
--- a/README.md
+++ b/README.md
@@ -702,7 +702,7 @@ func LintMyFile(file string) {
  		// You can use as many revivelib.Include or revivelib.Exclude as required
  	)
   	if err != nil {
-  	 	panic("Shouldn't have failed: " + err.Error)
+  	 	panic("Shouldn't have failed: " + err.Error())
   	}
 
   	// Now let's return the formatted errors
diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md
index fcd4e68..f741224 100644
--- a/RULES_DESCRIPTIONS.md
+++ b/RULES_DESCRIPTIONS.md
@@ -756,6 +756,8 @@ _Description_: Range variables in a loop are reused at each iteration. This rule
 
 _Configuration_: N/A
 
+_Note_: This rule is irrelevant for Go 1.22+.
+
 ## range-val-in-closure
 
 _Description_: Range variables in a loop are reused at each iteration; therefore a goroutine created in a loop will point to the range variable with from the upper scope. This way, the goroutine could use the variable with an undesired value.
@@ -763,6 +765,8 @@ This rule warns when a range value (or index) is used inside a closure
 
 _Configuration_: N/A
 
+_Note_: This rule is irrelevant for Go 1.22+.
+
 ## range
 
 _Description_: This rule suggests a shorter way of writing ranges that do not use the second value.
@@ -943,7 +947,7 @@ _Configuration_: Supports arguments with single of `map[string]any` with option
 
 ```toml
 [rule.unused-parameter]
-    Arguments = [{ allowRegex = "^_" }]
+    arguments = [{ allowRegex = "^_" }]
 ```
 
 allows any names started with `_`, not just `_` itself:
@@ -960,7 +964,7 @@ _Configuration_: Supports arguments with single of `map[string]any` with option
 
 ```toml
 [rule.unused-receiver]
-    Arguments = [{ allowRegex = "^_" }]
+    arguments = [{ allowRegex = "^_" }]
 ```
 
 allows any names started with `_`, not just `_` itself:
diff --git a/lint/linter.go b/lint/linter.go
index b42b397..b777f92 100644
--- a/lint/linter.go
+++ b/lint/linter.go
@@ -184,7 +184,9 @@ func detectGoMod(dir string) (rootDir string, ver *goversion.Version, err error)
 func retrieveModFile(dir string) (string, error) {
 	const lookingForFile = "go.mod"
 	for {
-		if dir == "." || dir == "/" {
+		// filepath.Dir returns 'C:\' on Windows, and '/' on Unix
+		isRootDir := (dir == filepath.VolumeName(dir)+string(filepath.Separator))
+		if dir == "." || isRootDir {
 			return "", fmt.Errorf("did not found %q file", lookingForFile)
 		}
 
diff --git a/rule/max_public_structs.go b/rule/max_public_structs.go
index 70840e7..8e42d82 100644
--- a/rule/max_public_structs.go
+++ b/rule/max_public_structs.go
@@ -1,6 +1,7 @@
 package rule
 
 import (
+	"fmt"
 	"go/ast"
 	"strings"
 	"sync"
@@ -19,7 +20,7 @@ const defaultMaxPublicStructs = 5
 func (r *MaxPublicStructsRule) configure(arguments lint.Arguments) {
 	r.Lock()
 	defer r.Unlock()
-	if r.max == 0 {
+	if r.max != 0 {
 		return // already configured
 	}
 
@@ -43,6 +44,10 @@ func (r *MaxPublicStructsRule) Apply(file *lint.File, arguments lint.Arguments)
 
 	var failures []lint.Failure
 
+	if r.max < 1 {
+		return failures
+	}
+
 	fileAst := file.AST
 
 	walker := &lintMaxPublicStructs{
@@ -56,7 +61,7 @@ func (r *MaxPublicStructsRule) Apply(file *lint.File, arguments lint.Arguments)
 
 	if walker.current > r.max {
 		walker.onFailure(lint.Failure{
-			Failure:    "you have exceeded the maximum number of public struct declarations",
+			Failure:    fmt.Sprintf("you have exceeded the maximum number (%d) of public struct declarations", r.max),
 			Confidence: 1,
 			Node:       fileAst,
 			Category:   "style",
diff --git a/rule/modifies_value_receiver.go b/rule/modifies_value_receiver.go
index e9e64b9..2f92991 100644
--- a/rule/modifies_value_receiver.go
+++ b/rule/modifies_value_receiver.go
@@ -2,6 +2,7 @@ package rule
 
 import (
 	"go/ast"
+	"go/token"
 	"strings"
 
 	"github.com/mgechev/revive/lint"
@@ -60,14 +61,14 @@ func (w lintModifiesValRecRule) Visit(node ast.Node) ast.Visitor {
 			return nil // skip, anonymous receiver
 		}
 
-		fselect := func(n ast.Node) bool {
+		receiverAssignmentFinder := func(n ast.Node) bool {
 			// look for assignments with the receiver in the right hand
-			asgmt, ok := n.(*ast.AssignStmt)
+			assignment, ok := n.(*ast.AssignStmt)
 			if !ok {
 				return false
 			}
 
-			for _, exp := range asgmt.Lhs {
+			for _, exp := range assignment.Lhs {
 				switch e := exp.(type) {
 				case *ast.IndexExpr: // receiver...[] = ...
 					continue
@@ -92,7 +93,15 @@ func (w lintModifiesValRecRule) Visit(node ast.Node) ast.Visitor {
 			return false
 		}
 
-		assignmentsToReceiver := pick(n.Body, fselect)
+		assignmentsToReceiver := pick(n.Body, receiverAssignmentFinder)
+		if len(assignmentsToReceiver) == 0 {
+			return nil // receiver is not modified
+		}
+
+		methodReturnsReceiver := len(w.findReturnReceiverStatements(receiverName, n.Body)) > 0
+		if methodReturnsReceiver {
+			return nil // modification seems legit (see issue #1066)
+		}
 
 		for _, assignment := range assignmentsToReceiver {
 			w.onFailure(lint.Failure{
@@ -127,3 +136,44 @@ func (lintModifiesValRecRule) getNameFromExpr(ie ast.Expr) string {
 
 	return ident.Name
 }
+
+func (w lintModifiesValRecRule) 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)
+		if !ok {
+			return false
+		}
+
+		for _, exp := range returnStatement.Results {
+			switch e := exp.(type) {
+			case *ast.SelectorExpr: // receiver.field = ...
+				name := w.getNameFromExpr(e.X)
+				if name == "" || name != receiverName {
+					continue
+				}
+			case *ast.Ident: // receiver := ...
+				if e.Name != receiverName {
+					continue
+				}
+			case *ast.UnaryExpr:
+				if e.Op != token.AND {
+					continue
+				}
+				name := w.getNameFromExpr(e.X)
+				if name == "" || name != receiverName {
+					continue
+				}
+
+			default:
+				continue
+			}
+
+			return true
+		}
+
+		return false
+	}
+
+	return pick(target, finder)
+}


Test patch:
diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
index a897ef7..5800b39 100644
--- a/.github/workflows/test.yaml
+++ b/.github/workflows/test.yaml
@@ -5,8 +5,8 @@ on:
     types: [opened, edited, synchronize, reopened]
 
 jobs:
-  test:
-    name: Test
+  tests-on-unix:
+    name: Tests on Unix
     runs-on: ubuntu-latest
     strategy:
       fail-fast: false
@@ -23,3 +23,16 @@ jobs:
           go-version: ${{ matrix.go-version }}
       - name: Run tests
         run: go test -race -shuffle=on ./...
+
+  tests-on-windows:
+    name: Tests on Windows
+    runs-on: windows-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+      - name: Set up Go
+        uses: actions/setup-go@v5
+        with:
+          go-version: stable
+      - name: Run tests
+        run: go test -race -shuffle=on ./...
diff --git a/cli/main_test.go b/cli/main_test.go
index c570efd..fd3a5ba 100644
--- a/cli/main_test.go
+++ b/cli/main_test.go
@@ -2,6 +2,7 @@ package cli
 
 import (
 	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/mitchellh/go-homedir"
@@ -22,19 +23,19 @@ func TestXDGConfigDirIsPreferredFirst(t *testing.T) {
 		AppFs = afero.NewMemMapFs()
 	})
 
-	xdgDirPath := "/tmp-iofs/xdg/config"
-	homeDirPath := "/tmp-iofs/home/tester"
+	xdgDirPath := filepath.FromSlash("/tmp-iofs/xdg/config")
+	homeDirPath := filepath.FromSlash("/tmp-iofs/home/tester")
 	AppFs.MkdirAll(xdgDirPath, 0755)
 	AppFs.MkdirAll(homeDirPath, 0755)
 
-	afero.WriteFile(AppFs, xdgDirPath+"/revive.toml", []byte("\n"), 0644)
+	afero.WriteFile(AppFs, filepath.Join(xdgDirPath, "revive.toml"), []byte("\n"), 0644)
 	t.Setenv("XDG_CONFIG_HOME", xdgDirPath)
 
-	afero.WriteFile(AppFs, homeDirPath+"/revive.toml", []byte("\n"), 0644)
+	afero.WriteFile(AppFs, filepath.Join(homeDirPath, "revive.toml"), []byte("\n"), 0644)
 	t.Setenv("HOME", homeDirPath)
 
 	got := buildDefaultConfigPath()
-	want := xdgDirPath + "/revive.toml"
+	want := filepath.Join(xdgDirPath, "revive.toml")
 
 	if got != want {
 		t.Errorf("got %q, wanted %q", got, want)
@@ -43,14 +44,14 @@ func TestXDGConfigDirIsPreferredFirst(t *testing.T) {
 
 func TestHomeConfigDir(t *testing.T) {
 	t.Cleanup(func() { AppFs = afero.NewMemMapFs() })
-	homeDirPath := "/tmp-iofs/home/tester"
+	homeDirPath := filepath.FromSlash("/tmp-iofs/home/tester")
 	AppFs.MkdirAll(homeDirPath, 0755)
 
-	afero.WriteFile(AppFs, homeDirPath+"/revive.toml", []byte("\n"), 0644)
+	afero.WriteFile(AppFs, filepath.Join(homeDirPath, "revive.toml"), []byte("\n"), 0644)
 	t.Setenv("HOME", homeDirPath)
 
 	got := buildDefaultConfigPath()
-	want := homeDirPath + "/revive.toml"
+	want := filepath.Join(homeDirPath, "revive.toml")
 
 	if got != want {
 		t.Errorf("got %q, wanted %q", got, want)
@@ -59,14 +60,14 @@ func TestHomeConfigDir(t *testing.T) {
 
 func TestXDGConfigDir(t *testing.T) {
 	t.Cleanup(func() { AppFs = afero.NewMemMapFs() })
-	xdgDirPath := "/tmp-iofs/xdg/config"
+	xdgDirPath := filepath.FromSlash("/tmp-iofs/xdg/config")
 	AppFs.MkdirAll(xdgDirPath, 0755)
 
-	afero.WriteFile(AppFs, xdgDirPath+"/revive.toml", []byte("\n"), 0644)
+	afero.WriteFile(AppFs, filepath.Join(xdgDirPath, "revive.toml"), []byte("\n"), 0644)
 	t.Setenv("XDG_CONFIG_HOME", xdgDirPath)
 
 	got := buildDefaultConfigPath()
-	want := xdgDirPath + "/revive.toml"
+	want := filepath.Join(xdgDirPath, "revive.toml")
 
 	if got != want {
 		t.Errorf("got %q, wanted %q", got, want)
@@ -75,7 +76,7 @@ func TestXDGConfigDir(t *testing.T) {
 
 func TestXDGConfigDirNoFile(t *testing.T) {
 	t.Cleanup(func() { AppFs = afero.NewMemMapFs() })
-	xdgDirPath := "/tmp-iofs/xdg/config"
+	xdgDirPath := filepath.FromSlash("/tmp-iofs/xdg/config")
 	t.Setenv("XDG_CONFIG_HOME", xdgDirPath)
 
 	got := buildDefaultConfigPath()
diff --git a/lint/linter_test.go b/lint/linter_test.go
index 4c1f95d..a7bb1d2 100644
--- a/lint/linter_test.go
+++ b/lint/linter_test.go
@@ -1,6 +1,44 @@
 package lint
 
-import "testing"
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestRetrieveModFile(t *testing.T) {
+	t.Run("go.mod file exists", func(t *testing.T) {
+		nestedDir := filepath.Join(t.TempDir(), "nested", "dir", "structure")
+		err := os.MkdirAll(nestedDir, 0o755)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		modFilePath := filepath.Join(nestedDir, "go.mod")
+		err = os.WriteFile(modFilePath, []byte("module example.com/test"), 0o644)
+		if err != nil {
+			t.Fatal(err)
+		}
+		foundPath, err := retrieveModFile(nestedDir)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if foundPath != modFilePath {
+			t.Fatalf("expected %q, got %q", modFilePath, foundPath)
+		}
+	})
+
+	t.Run("go.mod file does not exist", func(t *testing.T) {
+		_, err := retrieveModFile(t.TempDir())
+		if err == nil {
+			t.Fatalf("expected error, got nil")
+		}
+		expectedErrMsg := `did not found "go.mod" file`
+		if err.Error() != expectedErrMsg {
+			t.Fatalf("expected error message %q, got %q", expectedErrMsg, err.Error())
+		}
+	})
+}
 
 // TestIsGenerated tests isGenerated function.
 func TestIsGenerated(t *testing.T) { //revive:disable-line:exported
diff --git a/test/max_public_structs_test.go b/test/max_public_structs_test.go
index 22c7455..55718b0 100644
--- a/test/max_public_structs_test.go
+++ b/test/max_public_structs_test.go
@@ -12,3 +12,7 @@ func TestMaxPublicStructs(t *testing.T) {
 		Arguments: []any{int64(1)},
 	})
 }
+
+func TestMaxPublicStructsDefaultConfig(t *testing.T) {
+	testRule(t, "max_public_structs_ok", &rule.MaxPublicStructsRule{}, &lint.RuleConfig{})
+}
diff --git a/testdata/max_public_structs.go b/testdata/max_public_structs.go
index abdd3b7..9188c5f 100644
--- a/testdata/max_public_structs.go
+++ b/testdata/max_public_structs.go
@@ -1,5 +1,5 @@
 // Package pkg ...
-package pkg // MATCH /you have exceeded the maximum number of public struct declarations/
+package pkg // MATCH /you have exceeded the maximum number (1) of public struct declarations/
 
 type Foo struct {
 }
diff --git a/testdata/max_public_structs_ok.go b/testdata/max_public_structs_ok.go
new file mode 100644
index 0000000..5aaac52
--- /dev/null
+++ b/testdata/max_public_structs_ok.go
@@ -0,0 +1,11 @@
+// Package pkg ...
+package pkg
+
+type Foo struct {
+}
+
+type Bar struct {
+}
+
+type Baz struct {
+}
diff --git a/testdata/modifies_value_receiver.go b/testdata/modifies_value_receiver.go
index b5ae877..e6a7166 100644
--- a/testdata/modifies_value_receiver.go
+++ b/testdata/modifies_value_receiver.go
@@ -12,3 +12,19 @@ func (this data) vmethod() {
 	this.items = make(map[string]bool) // MATCH /suspicious assignment to a by-value method receiver/
 	this.items["vmethod"] = true
 }
+
+func (a A) Foo() *A {
+	a.whatever = true
+	return &a
+}
+
+func (a A) Clone() (*A, error) {
+	a.whatever = true
+	return &a, nil
+}
+
+// WithBin will set the specific bin path to the builder.
+func (b JailerCommandBuilder) WithBin(bin string) JailerCommandBuilder {
+	b.bin = bin
+	return b
+}