From 3249a5ef06e41c4a370fdf1172826b0cd06243cb Mon Sep 17 00:00:00 2001
From: Denis Voytyuk <5462781+denisvmedia@users.noreply.github.com>
Date: Mon, 23 Sep 2024 13:48:49 +0200
Subject: [PATCH] fix: enforce-repeated-arg-type-style rule finds false
 positives in case of invalid types (#1046)

* fix: enforce-repeated-arg-type-style rule finds false positives in case of invalid types

* Fix oversight issue
---
 RULES_DESCRIPTIONS.md                   |  3 +++
 rule/enforce-repeated-arg-type-style.go | 14 ++++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md
index 9b18df6d8..ecd304c0f 100644
--- a/RULES_DESCRIPTIONS.md
+++ b/RULES_DESCRIPTIONS.md
@@ -389,6 +389,9 @@ 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 067082b1b..89e8c050e 100644
--- a/rule/enforce-repeated-arg-type-style.go
+++ b/rule/enforce-repeated-arg-type-style.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"go/ast"
 	"go/types"
+	"strings"
 	"sync"
 
 	"github.com/mgechev/revive/lint"
@@ -134,7 +135,8 @@ 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 {
-						if types.Identical(typesInfo.Types[field.Type].Type, typesInfo.Types[prevType].Type) {
+						// 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) {
 							failures = append(failures, lint.Failure{
 								Confidence: 1,
 								Node:       field,
@@ -166,7 +168,8 @@ 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 {
-						if field.Names != nil && types.Identical(typesInfo.Types[field.Type].Type, typesInfo.Types[prevType].Type) {
+						// 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) {
 							failures = append(failures, lint.Failure{
 								Confidence: 1,
 								Node:       field,
@@ -189,3 +192,10 @@ 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")
+}