From ff5b9bb60f24aefc136043c2266ae272fef5dd88 Mon Sep 17 00:00:00 2001 From: chavacava Date: Sun, 22 Sep 2024 20:27:52 +0200 Subject: [PATCH] adds check for receiver names length --- README.md | 2 +- RULES_DESCRIPTIONS.md | 7 ++- rule/receiver-naming.go | 67 ++++++++++++++++++++++++-- test/receiver-naming_test.go | 6 +++ testdata/receiver-naming-issue-1040.go | 5 ++ 5 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 testdata/receiver-naming-issue-1040.go diff --git a/README.md b/README.md index 81e247d03..87fcc79aa 100644 --- a/README.md +++ b/README.md @@ -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) | string (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 | diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 4e2793098..be4aa3b31 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -737,7 +737,12 @@ _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) a string indicating the maximum length of the receiver name. If not set, name length is not checked. + +```toml +[rule.receiver-naming] + arguments=["max-length=2"] +``` ## redefines-builtin-id diff --git a/rule/receiver-naming.go b/rule/receiver-naming.go index d79bb9fe8..a51b35bce 100644 --- a/rule/receiver-naming.go +++ b/rule/receiver-naming.go @@ -3,16 +3,60 @@ package rule import ( "fmt" "go/ast" + "strconv" + "strings" + "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 { + if len(arguments) < 1 { + r.receiverNameMaxLength = defaultReceiverNameMaxLength + return + } + arg := arguments[0] + argStr, ok := arg.(string) + if !ok { + panic(fmt.Sprintf("Invalid argument for %s rule. Expecting an string, got %T", r.Name(), arg)) + } + + parts := strings.Split(argStr, "=") + if len(parts) != 2 { + panic(fmt.Sprintf("Invalid argument for %s rule. Expecting an string of the form 'key=value', got %s", r.Name(), argStr)) + } + + key := strings.TrimSpace(parts[0]) + value := strings.TrimSpace(parts[1]) + switch key { + case "max-length": + var err error + r.receiverNameMaxLength, err = strconv.Atoi(value) + if err != nil { + panic(fmt.Sprintf("Invalid value %s for the configuration key max-length, expected integer value: %v", value, err)) + } + default: + panic(fmt.Sprintf("Unknown configuration key %s for %s rule.", key, 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 @@ -20,7 +64,8 @@ func (*ReceiverNamingRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failu onFailure: func(failure lint.Failure) { failures = append(failures, failure) }, - typeReceiver: map[string]string{}, + typeReceiver: map[string]string{}, + receiverNameMaxLength: r.receiverNameMaxLength, } ast.Walk(walker, fileAst) @@ -34,8 +79,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 { @@ -66,6 +112,17 @@ func (w lintReceiverName) Visit(n ast.Node) ast.Visitor { }) return w } + + if w.receiverNameMaxLength > 0 && len(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{ diff --git a/test/receiver-naming_test.go b/test/receiver-naming_test.go index 1d530cb27..448c6107b 100644 --- a/test/receiver-naming_test.go +++ b/test/receiver-naming_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/mgechev/revive/internal/typeparams" + "github.com/mgechev/revive/lint" "github.com/mgechev/revive/rule" ) @@ -13,3 +14,8 @@ func TestReceiverNamingTypeParams(t *testing.T) { } testRule(t, "receiver-naming-issue-669", &rule.ReceiverNamingRule{}) } + +func TestReceiverNamingMaxLength(t *testing.T) { + testRule(t, "receiver-naming-issue-1040", &rule.ReceiverNamingRule{}, + &lint.RuleConfig{Arguments: []any{"max-length=2"}}) +} diff --git a/testdata/receiver-naming-issue-1040.go b/testdata/receiver-naming-issue-1040.go new file mode 100644 index 000000000..ab69dfa6e --- /dev/null +++ b/testdata/receiver-naming-issue-1040.go @@ -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/