Repository: mgechev/revive (go)

Issue:
string-format rule not working on method called when obj is inside an struct.
**Describe the bug**
The string-format rule search only for function calls and methods, but if the object is
inside a structure, the method call is not found by the rule.
This is a typical pattern when we want a generic Logger interface and different implementations. In this case `Logger` is a port and `MyLogger` would be an adapter from a project with hexagonal architecture. This implementation is injected in main to a service.

**To Reproduce**
Steps to reproduce the behavior:
1. Create go file with the following code:
```golang
package main

import "fmt"

type Logger interface {
	Error(msg string, keysAndValues ...any)
}
type MyLogger struct {
}

func (m *MyLogger) Error(msg string, _ ...any) {
	fmt.Println(msg)
}

type Service struct {
	logger Logger
}

func main() {
	s := Service{
		logger: &MyLogger{},
	}
	s.logger.Error("Test")
}
```
2. I run it with the following flags & configuration file:

```shell
revive main.go
```

_config_
```toml
[rule.string-format]
  arguments = [
    ["logger.Error", "/^([^A-Z]|$)/",  "must not start with a capital letter"]]
```

**Expected behavior**
I spect it to fail with 

`./tests/main.go:23:17: must not start with a capital letter`

as the logger.Error call start with a capital letter

**Desktop (please complete the following information):**
 - OS: Gentoo
 - Version of Go: 1.20

**Additional context**
MR is created with working solution (I don't know if the best)


Pull request description:
fix: support method call from structures for string-format
This MR fix issue #839. Here the called object method is inside a structure (common pattern for dependency injection). Before the rule was not running for this rules.

The only thing is the function scope is tied to the structure name of the object, not to the object name.

This MR tries to fix this. A test was added to the string-format to evaluate this.

- [x] Did you add tests?
- [x] Does your code follows the coding style of the rest of the repository?
- [x] Does the Travis build passes?

Closes #839 


Patch:
diff --git a/README.md b/README.md
index d402717..e164826 100644
--- a/README.md
+++ b/README.md
@@ -54,6 +54,8 @@ Here's how `revive` is different from `golint`:
 - [`ggz`](https://github.com/go-ggz/ggz) - An URL shortener service written in Golang
 - [`Codeac.io`](https://www.codeac.io?ref=revive) - Automated code review service integrates with GitHub, Bitbucket and GitLab (even self-hosted) and helps you fight technical debt.
 - [`DevLake`](https://github.com/apache/incubator-devlake) - Apache DevLake is an open-source dev data platform to ingest, analyze, and visualize the fragmented data from DevOps tools，which can distill insights to improve engineering productivity.
+- [`checker`](https://github.com/cinar/checker) - Checker helps validating user input through rules defined in struct tags or directly through functions.
+- [`milvus`](https://github.com/milvus-io/milvus) - A cloud-native vector database, storage for next generation AI applications.
 
 *Open a PR to add your project*.
 
@@ -406,7 +408,6 @@ warningCode = 0
 [rule.error-strings]
 [rule.error-naming]
 [rule.exported]
-[rule.if-return]
 [rule.increment-decrement]
 [rule.var-naming]
 [rule.var-declaration]
diff --git a/config/config.go b/config/config.go
index 04cd214..f6ee2a4 100644
--- a/config/config.go
+++ b/config/config.go
@@ -31,7 +31,6 @@ var defaultRules = []lint.Rule{
 	&rule.TimeNamingRule{},
 	&rule.ContextKeysType{},
 	&rule.ContextAsArgumentRule{},
-	&rule.IfReturnRule{},
 	&rule.EmptyBlockRule{},
 	&rule.SuperfluousElseRule{},
 	&rule.UnusedParamRule{},
@@ -87,6 +86,7 @@ var allRules = append([]lint.Rule{
 	&rule.UseAnyRule{},
 	&rule.DataRaceRule{},
 	&rule.CommentSpacingsRule{},
+	&rule.IfReturnRule{},
 }, defaultRules...)
 
 var allFormatters = []lint.Formatter{
diff --git a/defaults.toml b/defaults.toml
index f9e2405..d618645 100644
--- a/defaults.toml
+++ b/defaults.toml
@@ -8,23 +8,22 @@ warningCode = 0
 [rule.context-as-argument]
 [rule.context-keys-type]
 [rule.dot-imports]
+[rule.empty-block]
+[rule.error-naming]
 [rule.error-return]
 [rule.error-strings]
-[rule.error-naming]
+[rule.errorf]
 [rule.exported]
-[rule.if-return]
 [rule.increment-decrement]
-[rule.var-naming]
-[rule.var-declaration]
+[rule.indent-error-flow]
 [rule.package-comments]
 [rule.range]
 [rule.receiver-naming]
+[rule.redefines-builtin-id]
+[rule.superfluous-else]
 [rule.time-naming]
 [rule.unexported-return]
-[rule.indent-error-flow]
-[rule.errorf]
-[rule.empty-block]
-[rule.superfluous-else]
-[rule.unused-parameter]
 [rule.unreachable-code]
-[rule.redefines-builtin-id]
+[rule.unused-parameter]
+[rule.var-declaration]
+[rule.var-naming]
diff --git a/go.mod b/go.mod
index 981ee79..1bb14e9 100644
--- a/go.mod
+++ b/go.mod
@@ -3,7 +3,7 @@ module github.com/mgechev/revive
 go 1.19
 
 require (
-	github.com/BurntSushi/toml v1.2.1
+	github.com/BurntSushi/toml v1.3.2
 	github.com/chavacava/garif v0.0.0-20230531072157-8144c224b063
 	github.com/fatih/color v1.15.0
 	github.com/fatih/structtag v1.2.0
@@ -11,12 +11,12 @@ require (
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/olekukonko/tablewriter v0.0.5
 	github.com/pkg/errors v0.9.1
-	golang.org/x/tools v0.9.3
+	golang.org/x/tools v0.11.0
 )
 
 require (
 	github.com/mattn/go-colorable v0.1.13 // indirect
 	github.com/mattn/go-isatty v0.0.17 // indirect
 	github.com/mattn/go-runewidth v0.0.9 // indirect
-	golang.org/x/sys v0.8.0 // indirect
+	golang.org/x/sys v0.10.0 // indirect
 )
diff --git a/go.sum b/go.sum
index 90c82f9..5a26346 100644
--- a/go.sum
+++ b/go.sum
@@ -2,6 +2,8 @@ github.com/BurntSushi/toml v1.2.0 h1:Rt8g24XnyGTyglgET/PRUNlrUeu9F5L+7FilkXfZgs0
 github.com/BurntSushi/toml v1.2.0/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=
 github.com/BurntSushi/toml v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
 github.com/BurntSushi/toml v1.2.1/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=
+github.com/BurntSushi/toml v1.3.2 h1:o7IhLm0Msx3BaB+n3Ag7L8EVlByGnpq14C4YWiu/gL8=
+github.com/BurntSushi/toml v1.3.2/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=
 github.com/chavacava/garif v0.0.0-20220630083739-93517212f375 h1:E7LT642ysztPWE0dfz43cWOvMiF42DyTRC+eZIaO4yI=
 github.com/chavacava/garif v0.0.0-20220630083739-93517212f375/go.mod h1:4m1Rv7xfuwWPNKXlThldNuJvutYM6J95wNuuVmn55To=
 github.com/chavacava/garif v0.0.0-20221024190013-b3ef35877348 h1:cy5GCEZLUCshCGCRRUjxHrDUqkB4l5cuUt3ShEckQEo=
@@ -75,6 +77,10 @@ golang.org/x/sys v0.6.0 h1:MVltZSvRTcU2ljQOhs94SXPftV6DCNnZViHeQps87pQ=
 golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.8.0 h1:EBmGv8NaZBZTWvrbjNoL6HVt+IVy3QDQpJs7VRIw3tU=
 golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.9.0 h1:KS/R3tvhPqvJvwcKfnBHJwwthS11LRhmM5D59eEXa0s=
+golang.org/x/sys v0.9.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.10.0 h1:SqMFp9UcQJZa+pmYuAKjd9xq1f0j5rLcDIk0mj4qAsA=
+golang.org/x/sys v0.10.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/tools v0.1.12 h1:VveCTK38A2rkS8ZqFY25HIDFscX5X9OoEhJd3quQmXU=
 golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
 golang.org/x/tools v0.2.0 h1:G6AHpWxTMGY1KyEYoAQ5WTtIekUUvDNjan3ugu60JvE=
@@ -93,6 +99,10 @@ golang.org/x/tools v0.9.1 h1:8WMNJAz3zrtPmnYC7ISf5dEn3MT0gY7jBJfw27yrrLo=
 golang.org/x/tools v0.9.1/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc=
 golang.org/x/tools v0.9.3 h1:Gn1I8+64MsuTb/HpH+LmQtNas23LhUVr3rYZ0eKuaMM=
 golang.org/x/tools v0.9.3/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc=
+golang.org/x/tools v0.10.0 h1:tvDr/iQoUqNdohiYm0LmmKcBk+q86lb9EprIUFhHHGg=
+golang.org/x/tools v0.10.0/go.mod h1:UJwyiVBsOA2uwvK/e5OY3GTpDUJriEd+/YlqAwLPmyM=
+golang.org/x/tools v0.11.0 h1:EMCa6U9S2LtZXLAMoWiR/R8dAQFRqbAitmbJ2UKhoi8=
+golang.org/x/tools v0.11.0/go.mod h1:anzJrxPjNtfgiYQYirP2CPGzGLxrH2u2QBhn6Bf3qY8=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
diff --git a/rule/string-format.go b/rule/string-format.go
index 0e30ebf..3edd624 100644
--- a/rule/string-format.go
+++ b/rule/string-format.go
@@ -211,10 +211,14 @@ func (lintStringFormatRule) getCallName(call *ast.CallExpr) (callName string, ok
 	if selector, ok := call.Fun.(*ast.SelectorExpr); ok {
 		// Scoped function call
 		scope, ok := selector.X.(*ast.Ident)
-		if !ok {
-			return "", false
+		if ok {
+			return scope.Name + "." + selector.Sel.Name, true
+		}
+		// Scoped function call inside structure
+		recv, ok := selector.X.(*ast.SelectorExpr)
+		if ok {
+			return recv.Sel.Name + "." + selector.Sel.Name, true
 		}
-		return scope.Name + "." + selector.Sel.Name, true
 	}
 
 	return "", false


Test patch:
diff --git a/revivelib/core_test.go b/revivelib/core_test.go
index 44ab627..9dd1505 100644
--- a/revivelib/core_test.go
+++ b/revivelib/core_test.go
@@ -7,6 +7,7 @@ import (
 	"github.com/mgechev/revive/config"
 	"github.com/mgechev/revive/lint"
 	"github.com/mgechev/revive/revivelib"
+	"github.com/mgechev/revive/rule"
 )
 
 func TestReviveLint(t *testing.T) {
@@ -45,7 +46,6 @@ func TestReviveFormat(t *testing.T) {
 
 	// ACT
 	failures, exitCode, err := revive.Format("stylish", failuresChan)
-
 	// ASSERT
 	if err != nil {
 		t.Fatal(err)
@@ -70,8 +70,7 @@ func TestReviveFormat(t *testing.T) {
 	}
 }
 
-type mockRule struct {
-}
+type mockRule struct{}
 
 func (r *mockRule) Name() string {
 	return "mock-rule"
@@ -93,6 +92,7 @@ func getMockRevive(t *testing.T) *revivelib.Revive {
 		conf,
 		true,
 		2048,
+		revivelib.NewExtraRule(&rule.IfReturnRule{}, lint.RuleConfig{}),
 		revivelib.NewExtraRule(&mockRule{}, lint.RuleConfig{}),
 	)
 	if err != nil {
diff --git a/test/string-format_test.go b/test/string-format_test.go
index f58c10c..1d425c6 100644
--- a/test/string-format_test.go
+++ b/test/string-format_test.go
@@ -21,7 +21,11 @@ func TestStringFormat(t *testing.T) {
 			[]interface{}{
 				"s.Method3[2]",
 				"!/^[Tt][Hh]/",
-				"must not start with 'th'"}}})
+				"must not start with 'th'"},
+			[]interface{}{
+				"s.Method4", // same as before, but called from a struct
+				"!/^[Ot][Tt]/",
+				"must not start with 'ot'"}}})
 }
 
 func TestStringFormatArgumentParsing(t *testing.T) {
diff --git a/testdata/string-format.go b/testdata/string-format.go
index 69c3204..08e0fa2 100644
--- a/testdata/string-format.go
+++ b/testdata/string-format.go
@@ -18,6 +18,16 @@ func (s stringFormatMethods) Method3(a, b, c string) {
 
 }
 
+type stringFormatMethodsInjected struct{}
+
+func (s stringFormatMethodsInjected) Method4(a, b, c string) {
+
+}
+
+type container struct {
+	s stringFormatMethodsInjected
+}
+
 func stringFormat() {
 	stringFormatMethod1("This string is fine", "")
 	stringFormatMethod1("this string is not capitalized", "") // MATCH /must start with a capital letter/
@@ -27,4 +37,9 @@ func stringFormat() {
 		d: "This string is capitalized, but ends with a period."}) // MATCH /string literal doesn't match user defined regex /[^\.]$//
 	s := stringFormatMethods{}
 	s.Method3("", "", "This string starts with th") // MATCH /must not start with 'th'/
+
+	c := container{
+		s: stringFormatMethods{},
+	}
+	c.s.Method4("Other string starts with ot") // MATCH /must not start with 'ot'/
 }