Repository: mgechev/revive (go)

Issue:
The `enforce-repeated-arg-type-style` rule finds some false positives.
**Describe the bug**
The `enforce-repeated-arg-type-style` rule finds some false positives. 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.31.3
```
3. I run it with the following flags & configuration file:
```shell
# flags
revive -config revive.toml ./... | sort
```
```toml
#revive.toml
[rule.enforce-repeated-arg-type-style]
  arguments = ["short"]
```
4. Some of the findings are false positives.

**Expected behavior**
I would like only the findings that are not false positives to be shown.

**Logs**
Some examples:

`plugins/inputs/webhooks/artifactory/artifactory_webhook.go:23:87: repeated argument type can be omitted`:
![image](https://github.com/user-attachments/assets/43d3fd8c-c391-4fb3-ae89-8567ccac2543)

`plugins/outputs/bigquery/bigquery.go:244:45: repeated argument type can be omitted`:
![image](https://github.com/user-attachments/assets/b4d35730-50dc-48ce-9a1e-5dcb565a3ec2)

`plugins/outputs/mqtt/mqtt_test.go:803:59: repeated argument type can be omitted`:
![image](https://github.com/user-attachments/assets/b1bfd5ff-ef49-4aea-9fe4-45307a044e7e)


**Desktop (please complete the following information):**
 - OS: Ubuntu 23:10
 - Version of Go:
```shell
$ go version 
go version go1.23.0 linux/amd64
```


Pull request description:
fix #1032 by comparing string representations of types
Closes #1032 by comparing string representations of types (and not relying on actual type information)

Failure messages were extended to provide the name of the type that can be omitted.
Failure position information were fixed to point to the type that can be omitted.

This new implementation of the rule spots more failures than the current implementation and it seems to not generate false positives.
The attached files contain the result of linting the `telegraf` codebase (cf #1032) with the current implementation and with the implementation proposed by this PR.
[pr-result.txt](https://github.com/user-attachments/files/17103498/pr-result.txt)
[head-result.txt](https://github.com/user-attachments/files/17103499/head-result.txt)



Patch:
diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md
index ecd304c..9b18df6 100644
--- a/RULES_DESCRIPTIONS.md
+++ b/RULES_DESCRIPTIONS.md
@@ -389,9 +389,6 @@ declaration. The 'short' style encourages omitting repeated types for concisenes
 whereas the 'full' style mandates explicitly stating the type for each argument
 and return value, even if they are repeated, promoting clarity.
 
-_IMPORTANT_: When `short` style is used, the rule will not flag the arguments that use
-imported types. This is because the rule cannot efficiently determine the imported type.
-
 _Configuration (1)_: (string) as a single string, it configures both argument
 and return value styles. Accepts 'any', 'short', or 'full' (default: 'any').
 
diff --git a/rule/enforce-repeated-arg-type-style.go b/rule/enforce-repeated-arg-type-style.go
index 89e8c05..a435ee1 100644
--- a/rule/enforce-repeated-arg-type-style.go
+++ b/rule/enforce-repeated-arg-type-style.go
@@ -3,8 +3,6 @@ package rule
 import (
 	"fmt"
 	"go/ast"
-	"go/types"
-	"strings"
 	"sync"
 
 	"github.com/mgechev/revive/lint"
@@ -105,13 +103,6 @@ func (r *EnforceRepeatedArgTypeStyleRule) Apply(file *lint.File, arguments lint.
 
 	var failures []lint.Failure
 
-	err := file.Pkg.TypeCheck()
-	if err != nil {
-		// the file has other issues
-		return nil
-	}
-	typesInfo := file.Pkg.TypesInfo()
-
 	astFile := file.AST
 	ast.Inspect(astFile, func(n ast.Node) bool {
 		switch fn := n.(type) {
@@ -135,13 +126,14 @@ func (r *EnforceRepeatedArgTypeStyleRule) Apply(file *lint.File, arguments lint.
 				var prevType ast.Expr
 				if fn.Type.Params != nil {
 					for _, field := range fn.Type.Params.List {
-						// TODO: For invalid types we could have compared raw import names (import package alias + selector), but will it work properly in all the cases?
-						if !r.isInvalidType(typesInfo.Types[field.Type].Type) && types.Identical(typesInfo.Types[field.Type].Type, typesInfo.Types[prevType].Type) {
+						prevTypeStr := gofmt(prevType)
+						currentTypeStr := gofmt(field.Type)
+						if currentTypeStr == prevTypeStr {
 							failures = append(failures, lint.Failure{
 								Confidence: 1,
-								Node:       field,
+								Node:       prevType,
 								Category:   "style",
-								Failure:    "repeated argument type can be omitted",
+								Failure:    fmt.Sprintf("repeated argument type %q can be omitted", prevTypeStr),
 							})
 						}
 						prevType = field.Type
@@ -168,13 +160,14 @@ func (r *EnforceRepeatedArgTypeStyleRule) Apply(file *lint.File, arguments lint.
 				var prevType ast.Expr
 				if fn.Type.Results != nil {
 					for _, field := range fn.Type.Results.List {
-						// TODO: For invalid types we could have compared raw import names (import package alias + selector), but will it work properly in all the cases?
-						if !r.isInvalidType(typesInfo.Types[field.Type].Type) && field.Names != nil && types.Identical(typesInfo.Types[field.Type].Type, typesInfo.Types[prevType].Type) {
+						prevTypeStr := gofmt(prevType)
+						currentTypeStr := gofmt(field.Type)
+						if field.Names != nil && currentTypeStr == prevTypeStr {
 							failures = append(failures, lint.Failure{
 								Confidence: 1,
-								Node:       field,
+								Node:       prevType,
 								Category:   "style",
-								Failure:    "repeated return type can be omitted",
+								Failure:    fmt.Sprintf("repeated return type %q can be omitted", prevTypeStr),
 							})
 						}
 						prevType = field.Type
@@ -192,10 +185,3 @@ func (r *EnforceRepeatedArgTypeStyleRule) Apply(file *lint.File, arguments lint.
 func (*EnforceRepeatedArgTypeStyleRule) Name() string {
 	return "enforce-repeated-arg-type-style"
 }
-
-// Invalid types are imported from other packages, and we can't compare them.
-// Note, we check the string suffix to cover all the cases: non-pointer, pointer, double pointer, etc.
-// See: https://github.com/mgechev/revive/issues/1032
-func (*EnforceRepeatedArgTypeStyleRule) isInvalidType(t types.Type) bool {
-	return strings.HasSuffix(t.String(), "invalid type")
-}


Test patch:
diff --git a/testdata/enforce-repeated-arg-type-style-mixed-full-short.go b/testdata/enforce-repeated-arg-type-style-mixed-full-short.go
index f805892..8ad6007 100644
--- a/testdata/enforce-repeated-arg-type-style-mixed-full-short.go
+++ b/testdata/enforce-repeated-arg-type-style-mixed-full-short.go
@@ -2,5 +2,5 @@ package fixtures
 
 func compliantFunc(a int, b int, c string) (x, y int, z string) // Must not match - compliant with rule
 
-func nonCompliantFunc1(a int, b int, c string) (x int, y int, z string) { panic("implement me") } // MATCH /repeated return type can be omitted/
+func nonCompliantFunc1(a int, b int, c string) (x int, y int, z string) { panic("implement me") } // MATCH /repeated return type "int" can be omitted/
 func nonCompliantFunc2(a, b int, c string) (x, y int, z string)         { panic("implement me") } // MATCH /argument types should not be omitted/
diff --git a/testdata/enforce-repeated-arg-type-style-mixed-short-full.go b/testdata/enforce-repeated-arg-type-style-mixed-short-full.go
index 41ff89d..12748a0 100644
--- a/testdata/enforce-repeated-arg-type-style-mixed-short-full.go
+++ b/testdata/enforce-repeated-arg-type-style-mixed-short-full.go
@@ -3,4 +3,4 @@ package fixtures
 func compliantFunc(a, b int, c string) (x int, y int, z string) // Must not match - compliant with rule
 
 func nonCompliantFunc1(a, b int, c string) (x, y int, z string)         { panic("implement me") } // MATCH /return types should not be omitted/
-func nonCompliantFunc2(a int, b int, c string) (x int, y int, z string) { panic("implement me") } // MATCH /repeated argument type can be omitted/
+func nonCompliantFunc2(a int, b int, c string) (x int, y int, z string) { panic("implement me") } // MATCH /repeated argument type "int" can be omitted/
diff --git a/testdata/enforce-repeated-arg-type-style-short-args.go b/testdata/enforce-repeated-arg-type-style-short-args.go
index 352f90a..9503bce 100644
--- a/testdata/enforce-repeated-arg-type-style-short-args.go
+++ b/testdata/enforce-repeated-arg-type-style-short-args.go
@@ -2,15 +2,15 @@ package fixtures
 
 func compliantFunc(a, b int, c string) {} // Must not match - compliant with rule
 
-func nonCompliantFunc1(a int, b int, c string) {} // MATCH /repeated argument type can be omitted/
-func nonCompliantFunc2(a int, b, c int)        {} // MATCH /repeated argument type can be omitted/
+func nonCompliantFunc1(a int, b int, c string) {} // MATCH /repeated argument type "int" can be omitted/
+func nonCompliantFunc2(a int, b, c int)        {} // MATCH /repeated argument type "int" can be omitted/
 
 type myStruct struct{}
 
 func (m myStruct) compliantMethod(a, b int, c string) {} // Must not match - compliant with rule
 
-func (m myStruct) nonCompliantMethod1(a int, b int, c string) {} // MATCH /repeated argument type can be omitted/
-func (m myStruct) nonCompliantMethod2(a int, b, c int)        {} // MATCH /repeated argument type can be omitted/
+func (m myStruct) nonCompliantMethod1(a int, b int, c string) {} // MATCH /repeated argument type "int" can be omitted/
+func (m myStruct) nonCompliantMethod2(a int, b, c int)        {} // MATCH /repeated argument type "int" can be omitted/
 
 func variadicFunction(a int, b ...int) {} // Must not match - variadic parameters are a special case
 
@@ -18,4 +18,4 @@ func singleArgFunction(a int) {} // Must not match - only one argument
 
 func multiTypeArgs(a int, b string, c float64) {} // Must not match - different types for each argument
 
-func mixedCompliance(a, b int, c int, d string) {} // MATCH /repeated argument type can be omitted/ - 'c int' could be combined with 'a, b int'
+func mixedCompliance(a, b int, c int, d string) {} // MATCH /repeated argument type "int" can be omitted/ - 'c int' could be combined with 'a, b int'
diff --git a/testdata/enforce-repeated-arg-type-style-short-return.go b/testdata/enforce-repeated-arg-type-style-short-return.go
index 1605c40..522e6e1 100644
--- a/testdata/enforce-repeated-arg-type-style-short-return.go
+++ b/testdata/enforce-repeated-arg-type-style-short-return.go
@@ -3,18 +3,18 @@ package fixtures
 func compliantFunc() (a, b int, c string) { panic("implement me") } // Must not match - compliant with rule
 func compliantFunc2() (int, int, string)  // Must not match - compliant with rule
 
-func nonCompliantFunc1() (a int, b int, c string) { panic("implement me") } // MATCH /repeated return type can be omitted/
-func nonCompliantFunc2() (a int, b, c int)        { panic("implement me") } // MATCH /repeated return type can be omitted/
+func nonCompliantFunc1() (a int, b int, c string) { panic("implement me") } // MATCH /repeated return type "int" can be omitted/
+func nonCompliantFunc2() (a int, b, c int)        { panic("implement me") } // MATCH /repeated return type "int" can be omitted/
 
 type myStruct struct{}
 
 func (m myStruct) compliantMethod() (a, b int, c string) { panic("implement me") } // Must not match - compliant with rule
 
-func (m myStruct) nonCompliantMethod1() (a int, b int, c string) { panic("implement me") } // MATCH /repeated return type can be omitted/
-func (m myStruct) nonCompliantMethod2() (a int, b, c int)        { panic("implement me") } // MATCH /repeated return type can be omitted/
+func (m myStruct) nonCompliantMethod1() (a int, b int, c string) { panic("implement me") } // MATCH /repeated return type "int" can be omitted/
+func (m myStruct) nonCompliantMethod2() (a int, b, c int)        { panic("implement me") } // MATCH /repeated return type "int" can be omitted/
 
 func singleArgFunction() (a int) { panic("implement me") } // Must not match - only one return
 
 func multiTypeArgs() (a int, b string, c float64) { panic("implement me") } // Must not match - different types for each return
 
-func mixedCompliance() (a, b int, c int, d string) { panic("implement me") } // MATCH /repeated return type can be omitted/ - 'c int' could be combined with 'a, b int'
+func mixedCompliance() (a, b int, c int, d string) { panic("implement me") } // MATCH /repeated return type "int" can be omitted/ - 'c int' could be combined with 'a, b int'