Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/master' into rules-avoid-using…
Browse files Browse the repository at this point in the history
…-panic

# Conflicts:
#	rule/receiver_naming.go
  • Loading branch information
mfederowicz committed Dec 8, 2024
2 parents 92bcf9e + 5c2aadf commit aee44cb
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 74 deletions.
21 changes: 19 additions & 2 deletions internal/astutils/ast_utils.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// Package astutils provides utility functions for working with AST nodes
package astutils

import "go/ast"
import (
"fmt"
"go/ast"
)

// FuncSignatureIs returns true if the given func decl satisfies a signature characterized
// by the given name, parameters types and return types; false otherwise.
Expand Down Expand Up @@ -45,7 +49,7 @@ func getTypeNames(fields *ast.FieldList) []string {
}

for _, field := range fields.List {
typeName := field.Type.(*ast.Ident).Name
typeName := getFieldTypeName(field.Type)
if field.Names == nil { // unnamed field
result = append(result, typeName)
continue
Expand All @@ -58,3 +62,16 @@ func getTypeNames(fields *ast.FieldList) []string {

return result
}

func getFieldTypeName(typ ast.Expr) string {
switch f := typ.(type) {
case *ast.Ident:
return f.Name
case *ast.SelectorExpr:
return f.Sel.Name + "." + getFieldTypeName(f.X)
case *ast.StarExpr:
return "*" + getFieldTypeName(f.X)
default:
panic(fmt.Sprintf("not supported type %T", typ))
}
}
1 change: 0 additions & 1 deletion rule/confusing_results.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ func (*ConfusingResultsRule) Apply(file *lint.File, _ lint.Arguments) ([]lint.Fa

lastType := ""
for _, result := range funcDecl.Type.Results.List {

resultTypeName := gofmt(result.Type)

if resultTypeName == lastType {
Expand Down
4 changes: 2 additions & 2 deletions rule/datarace.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func (r *DataRaceRule) Apply(file *lint.File, _ lint.Arguments) ([]lint.Failure,

returnIDs := map[*ast.Object]struct{}{}
if funcResults != nil {
returnIDs = r.ExtractReturnIDs(funcResults.List)
returnIDs = r.extractReturnIDs(funcResults.List)
}

onFailure := func(failure lint.Failure) {
Expand All @@ -49,7 +49,7 @@ func (*DataRaceRule) Name() string {
return "datarace"
}

func (*DataRaceRule) ExtractReturnIDs(fields []*ast.Field) map[*ast.Object]struct{} {
func (*DataRaceRule) extractReturnIDs(fields []*ast.Field) map[*ast.Object]struct{} {
r := map[*ast.Object]struct{}{}
for _, f := range fields {
for _, id := range f.Names {
Expand Down
124 changes: 55 additions & 69 deletions rule/receiver_naming.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,82 +53,68 @@ func (r *ReceiverNamingRule) Apply(file *lint.File, arguments lint.Arguments) ([
return nil, configureErr
}

typeReceiver := map[string]string{}
var failures []lint.Failure
for _, decl := range file.AST.Decls {
fn, ok := decl.(*ast.FuncDecl)
if !ok || fn.Recv == nil || len(fn.Recv.List) == 0 {
continue
}

fileAst := file.AST
walker := lintReceiverName{
onFailure: func(failure lint.Failure) {
failures = append(failures, failure)
},
typeReceiver: map[string]string{},
receiverNameMaxLength: r.receiverNameMaxLength,
}

ast.Walk(walker, fileAst)
names := fn.Recv.List[0].Names
if len(names) < 1 {
continue
}
name := names[0].Name

if name == "_" {
failures = append(failures, lint.Failure{
Node: decl,
Confidence: 1,
Category: "naming",
Failure: "receiver name should not be an underscore, omit the name if it is unused",
})
continue
}

return failures, nil
}
if name == "this" || name == "self" {
failures = append(failures, lint.Failure{
Node: decl,
Confidence: 1,
Category: "naming",
Failure: `receiver name should be a reflection of its identity; don't use generic names such as "this" or "self"`,
})
continue
}

// Name returns the rule name.
func (*ReceiverNamingRule) Name() string {
return "receiver-naming"
}
if r.receiverNameMaxLength > 0 && len([]rune(name)) > r.receiverNameMaxLength {
failures = append(failures, lint.Failure{
Node: decl,
Confidence: 1,
Category: "naming",
Failure: fmt.Sprintf("receiver name %s is longer than %d characters", name, r.receiverNameMaxLength),
})
continue
}

type lintReceiverName struct {
onFailure func(lint.Failure)
typeReceiver map[string]string
receiverNameMaxLength int
}
recv := typeparams.ReceiverType(fn)
if prev, ok := typeReceiver[recv]; ok && prev != name {
failures = append(failures, lint.Failure{
Node: decl,
Confidence: 1,
Category: "naming",
Failure: fmt.Sprintf("receiver name %s should be consistent with previous receiver name %s for %s", name, prev, recv),
})
continue
}

func (w lintReceiverName) Visit(n ast.Node) ast.Visitor {
fn, ok := n.(*ast.FuncDecl)
if !ok || fn.Recv == nil || len(fn.Recv.List) == 0 {
return w
}
names := fn.Recv.List[0].Names
if len(names) < 1 {
return w
}
name := names[0].Name
if name == "_" {
w.onFailure(lint.Failure{
Node: n,
Confidence: 1,
Category: "naming",
Failure: "receiver name should not be an underscore, omit the name if it is unused",
})
return w
}
if name == "this" || name == "self" {
w.onFailure(lint.Failure{
Node: n,
Confidence: 1,
Category: "naming",
Failure: `receiver name should be a reflection of its identity; don't use generic names such as "this" or "self"`,
})
return w
typeReceiver[recv] = name
}

if w.receiverNameMaxLength > 0 && len([]rune(name)) > w.receiverNameMaxLength {
w.onFailure(lint.Failure{
Node: n,
Confidence: 1,
Category: "naming",
Failure: fmt.Sprintf("receiver name %s is longer than %d characters", name, w.receiverNameMaxLength),
})
return w
}
return failures
}

recv := typeparams.ReceiverType(fn)
if prev, ok := w.typeReceiver[recv]; ok && prev != name {
w.onFailure(lint.Failure{
Node: n,
Confidence: 1,
Category: "naming",
Failure: fmt.Sprintf("receiver name %s should be consistent with previous receiver name %s for %s", name, prev, recv),
})
return w
}
w.typeReceiver[recv] = name
return w
// Name returns the rule name.
func (*ReceiverNamingRule) Name() string {
return "receiver-naming"
}
5 changes: 5 additions & 0 deletions testdata/golint/sort.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,8 @@ type Vv []int

func (vv Vv) Len() (result int) { return len(w) } // MATCH /exported method Vv.Len should have comment or be unexported/
func (vv Vv) Less(i int, j int) (result bool) { return w[i] < w[j] } // MATCH /exported method Vv.Less should have comment or be unexported/

// X is ...
type X []int

func (x X) Less(i *pkg.tip) (result bool) { return len(x) } // MATCH /exported method X.Less should have comment or be unexported/

0 comments on commit aee44cb

Please sign in to comment.