go/revive
1
0
Fork 0
mirror of https://github.com/mgechev/revive.git synced 2025-03-17 20:57:58 +02:00
Code Issues Releases Activity

fix #1032 by comparing string representations of types (#1049)

Browse Source
This commit is contained in:
chavacava 2024-09-24 11:29:20 +02:00 committed by GitHub
parent 3249a5ef06
commit 4c3641ebc3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 22 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
RULES_DESCRIPTIONS.md
View File

@ -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').

34
rule/enforce-repeated-arg-type-style.go
View File

@ -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")
}

2
testdata/enforce-repeated-arg-type-style-mixed-full-short.go vendored
View File

@ -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/

2
testdata/enforce-repeated-arg-type-style-mixed-short-full.go vendored
View File

@ -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/

10
testdata/enforce-repeated-arg-type-style-short-args.go vendored
View File

@ -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'

10
testdata/enforce-repeated-arg-type-style-short-return.go vendored
View File

@ -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'