Skip to content

Commit

Permalink
adds check for receiver names length (#1048)
Browse files Browse the repository at this point in the history
Co-authored-by: chavacava <salvador.cavadini@gmail.com>
Co-authored-by: yuta nishiyama <57400690+ytnsym@users.noreply.github.com>
  • Loading branch information
3 people authored Sep 23, 2024
1 parent a65fb8d commit 53a111d
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 7 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
| [`var-naming`](./RULES_DESCRIPTIONS.md#var-naming) | allowlist & blocklist of initialisms | Naming rules. | yes | no |
| [`package-comments`](./RULES_DESCRIPTIONS.md#package-comments) | n/a | Package commenting conventions. | yes | no |
| [`range`](./RULES_DESCRIPTIONS.md#range) | n/a | Prevents redundant variables when iterating over a collection. | yes | no |
| [`receiver-naming`](./RULES_DESCRIPTIONS.md#receiver-naming) | n/a | Conventions around the naming of receivers. | yes | no |
| [`receiver-naming`](./RULES_DESCRIPTIONS.md#receiver-naming) | map (optional) | Conventions around the naming of receivers. | yes | no |
| [`indent-error-flow`](./RULES_DESCRIPTIONS.md#indent-error-flow) | []string | Prevents redundant else statements. | yes | no |
| [`argument-limit`](./RULES_DESCRIPTIONS.md#argument-limit) | int (defaults to 8) | Specifies the maximum number of arguments a function can receive | no | no |
| [`cyclomatic`](./RULES_DESCRIPTIONS.md#cyclomatic) | int (defaults to 10) | Sets restriction for maximum Cyclomatic complexity. | no | no |
Expand Down
9 changes: 8 additions & 1 deletion RULES_DESCRIPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,14 @@ _Configuration_: N/A
_Description_: By convention, receiver names in a method should reflect their identity. For example, if the receiver is of type `Parts`, `p` is an adequate name for it. Contrary to other languages, it is not idiomatic to name receivers as `this` or `self`.
_Configuration_: N/A
_Configuration_: (optional) list of key-value-pair-map (`[]map[string]any`).
- `maxLength` : (int) max length of receiver name
```toml
[rule.receiver-naming]
arguments = [{maxLength=2}]
```
## redefines-builtin-id
Expand Down
62 changes: 57 additions & 5 deletions rule/receiver-naming.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,64 @@ package rule
import (
"fmt"
"go/ast"
"sync"

"github.com/mgechev/revive/internal/typeparams"
"github.com/mgechev/revive/lint"
)

// ReceiverNamingRule lints given else constructs.
type ReceiverNamingRule struct{}
type ReceiverNamingRule struct {
receiverNameMaxLength int
sync.Mutex
}

const defaultReceiverNameMaxLength = -1 // thus will not check

func (r *ReceiverNamingRule) configure(arguments lint.Arguments) {
r.Lock()
defer r.Unlock()
if r.receiverNameMaxLength != 0 {
return
}

r.receiverNameMaxLength = defaultReceiverNameMaxLength
if len(arguments) < 1 {
return
}

args, ok := arguments[0].(map[string]any)
if !ok {
panic(fmt.Sprintf("Unable to get arguments for rule %s. Expected object of key-value-pairs.", r.Name()))
}

for k, v := range args {
switch k {
case "maxLength":
value, ok := v.(int64)
if !ok {
panic(fmt.Sprintf("Invalid value %v for argument %s of rule %s, expected integer value got %T", v, k, r.Name(), v))
}
r.receiverNameMaxLength = int(value)
default:
panic(fmt.Sprintf("Unknown argument %s for %s rule.", k, r.Name()))
}
}
}

// Apply applies the rule to given file.
func (*ReceiverNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
func (r *ReceiverNamingRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure {
r.configure(args)

var failures []lint.Failure

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

ast.Walk(walker, fileAst)
Expand All @@ -34,8 +74,9 @@ func (*ReceiverNamingRule) Name() string {
}

type lintReceiverName struct {
onFailure func(lint.Failure)
typeReceiver map[string]string
onFailure func(lint.Failure)
typeReceiver map[string]string
receiverNameMaxLength int
}

func (w lintReceiverName) Visit(n ast.Node) ast.Visitor {
Expand Down Expand Up @@ -66,6 +107,17 @@ func (w lintReceiverName) Visit(n ast.Node) ast.Visitor {
})
return w
}

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
}

recv := typeparams.ReceiverType(fn)
if prev, ok := w.typeReceiver[recv]; ok && prev != name {
w.onFailure(lint.Failure{
Expand Down
9 changes: 9 additions & 0 deletions test/receiver-naming_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"testing"

"github.com/mgechev/revive/internal/typeparams"
"github.com/mgechev/revive/lint"
"github.com/mgechev/revive/rule"
)

Expand All @@ -13,3 +14,11 @@ func TestReceiverNamingTypeParams(t *testing.T) {
}
testRule(t, "receiver-naming-issue-669", &rule.ReceiverNamingRule{})
}

func TestReceiverNamingMaxLength(t *testing.T) {
args := []any{map[string]any{
"maxLength": int64(2),
}}
testRule(t, "receiver-naming-issue-1040", &rule.ReceiverNamingRule{},
&lint.RuleConfig{Arguments: args})
}
5 changes: 5 additions & 0 deletions testdata/receiver-naming-issue-1040.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package fixtures

func (o *o) f1() {}
func (tw *tw) f2() {}
func (thr *thr) f3() {} // MATCH /receiver name thr is longer than 2 characters/

0 comments on commit 53a111d

Please # to comment.