Skip to content

Commit

Permalink
adds check for receiver names length
Browse files Browse the repository at this point in the history
  • Loading branch information
chavacava committed Sep 22, 2024
1 parent 5913192 commit ff5b9bb
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) | 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 |
Expand Down
7 changes: 6 additions & 1 deletion RULES_DESCRIPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
67 changes: 62 additions & 5 deletions rule/receiver-naming.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,69 @@ 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
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 +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 {
Expand Down Expand Up @@ -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{
Expand Down
6 changes: 6 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,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"}})
}
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 ff5b9bb

Please sign in to comment.