Skip to content

Commit

Permalink
Exported config (#565)
Browse files Browse the repository at this point in the history
* working version

* adds flag for replacing "stutters"
  • Loading branch information
chavacava authored Aug 26, 2021
1 parent 406b1ce commit 18cdb55
Show file tree
Hide file tree
Showing 8 changed files with 116 additions and 5 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
| [`error-return`](./RULES_DESCRIPTIONS.md#error-return) | n/a | The error return parameter should be last. | yes | no |
| [`error-strings`](./RULES_DESCRIPTIONS.md#error-strings) | n/a | Conventions around error strings. | yes | no |
| [`error-naming`](./RULES_DESCRIPTIONS.md#error-naming) | n/a | Naming of error variables. | yes | no |
| [`exported`](./RULES_DESCRIPTIONS.md#exported) | n/a | Naming and commenting conventions on exported symbols. | yes | no |
| [`exported`](./RULES_DESCRIPTIONS.md#exported) | []string | Naming and commenting conventions on exported symbols. | yes | no |
| [`if-return`](./RULES_DESCRIPTIONS.md#if-return) | n/a | Redundant if when returning an error. | no | no |
| [`increment-decrement`](./RULES_DESCRIPTIONS.md#increment-decrement) | n/a | Use `i++` and `i--` instead of `i += 1` and `i -= 1`. | yes | no |
| [`var-naming`](./RULES_DESCRIPTIONS.md#var-naming) | whitelist & blacklist of initialisms | Naming rules. | yes | no |
Expand Down
15 changes: 14 additions & 1 deletion RULES_DESCRIPTIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,20 @@ _Description_: Exported function and methods should have comments. This warns on

More information [here](https://github.com/golang/go/wiki/CodeReviewComments#doc-comments)

_Configuration_: N/A
_Configuration_: ([]string) rule flags.
Please notice that without configuration, the default behavior of the rule is that of its `golint` counterpart.
Available flags are:

* _checkPrivateReceivers_ enables checking public methods of private types
* _disableStutteringCheck_ disables checking for method names that stutter with the package name (i.e. avoid failure messages of the form _type name will be used as x.XY by other packages, and that stutters; consider calling this Y_)
* _sayRepetitiveInsteadOfStutters_ replaces the use of the term _stutters_ by _repetitive_ in failure messages

Example:

```toml
[rule.exported]
arguments =["checkPrivateReceivers","disableStutteringCheck"]
```

## file-header

Expand Down
49 changes: 46 additions & 3 deletions rule/exported.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,20 @@ import (
type ExportedRule struct{}

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

if isTest(file) {
return failures
}

checkPrivateReceivers, disableStutteringCheck, sayRepetitiveInsteadOfStutters := r.getConf(args)

stuttersMsg := "stutters"
if sayRepetitiveInsteadOfStutters {
stuttersMsg = "is repetitive"
}

fileAst := file.AST
walker := lintExported{
file: file,
Expand All @@ -30,6 +37,9 @@ func (r *ExportedRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
failures = append(failures, failure)
},
genDeclMissingComments: make(map[*ast.GenDecl]bool),
checkPrivateReceivers: checkPrivateReceivers,
disableStutteringCheck: disableStutteringCheck,
stuttersMsg: stuttersMsg,
}

ast.Walk(&walker, fileAst)
Expand All @@ -42,12 +52,41 @@ func (r *ExportedRule) Name() string {
return "exported"
}

func (r *ExportedRule) getConf(args lint.Arguments) (checkPrivateReceivers bool, disableStutteringCheck bool, sayRepetitiveInsteadOfStutters bool) {
// if any, we expect a slice of strings as configuration
if len(args) < 1 {
return
}
for _, flag := range args {
flagStr, ok := flag.(string)
if !ok {
panic(fmt.Sprintf("Invalid argument for the %s rule: expecting a string, got %T", r.Name(), flag))
}

switch flagStr {
case "checkPrivateReceivers":
checkPrivateReceivers = true
case "disableStutteringCheck":
disableStutteringCheck = true
case "sayRepetitiveInsteadOfStutters":
sayRepetitiveInsteadOfStutters = true
default:
panic(fmt.Sprintf("Unknown configuration flag %s for %s rule", flagStr, r.Name()))
}
}

return
}

type lintExported struct {
file *lint.File
fileAst *ast.File
lastGen *ast.GenDecl
genDeclMissingComments map[*ast.GenDecl]bool
onFailure func(lint.Failure)
checkPrivateReceivers bool
disableStutteringCheck bool
stuttersMsg string
}

func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) {
Expand All @@ -61,7 +100,7 @@ func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) {
// method
kind = "method"
recv := receiverType(fn)
if !ast.IsExported(recv) {
if !ast.IsExported(recv) && !w.checkPrivateReceivers {
// receiver is unexported
return
}
Expand Down Expand Up @@ -98,6 +137,10 @@ func (w *lintExported) lintFuncDoc(fn *ast.FuncDecl) {
}

func (w *lintExported) checkStutter(id *ast.Ident, thing string) {
if w.disableStutteringCheck {
return
}

pkg, name := w.fileAst.Name.Name, id.Name
if !ast.IsExported(name) {
// unexported name
Expand All @@ -122,7 +165,7 @@ func (w *lintExported) checkStutter(id *ast.Ident, thing string) {
Node: id,
Confidence: 0.8,
Category: "naming",
Failure: fmt.Sprintf("%s name will be used as %s.%s by other packages, and that stutters; consider calling this %s", thing, pkg, name, rem),
Failure: fmt.Sprintf("%s name will be used as %s.%s by other packages, and that %s; consider calling this %s", thing, pkg, name, w.stuttersMsg, rem),
})
}
}
Expand Down
26 changes: 26 additions & 0 deletions test/exported_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package test

import (
"testing"

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

func TestExportedWithDisableStutteringCheck(t *testing.T) {
args := []interface{}{"disableStutteringCheck"}

testRule(t, "exported-issue-555", &rule.ExportedRule{}, &lint.RuleConfig{Arguments: args})
}

func TestExportedWithChecksOnMethodsOfPrivateTypes(t *testing.T) {
args := []interface{}{"checkPrivateReceivers"}

testRule(t, "exported-issue-552", &rule.ExportedRule{}, &lint.RuleConfig{Arguments: args})
}

func TestExportedReplacingStuttersByRepetitive(t *testing.T) {
args := []interface{}{"sayRepetitiveInsteadOfStutters"}

testRule(t, "exported-issue-519", &rule.ExportedRule{}, &lint.RuleConfig{Arguments: args})
}
7 changes: 7 additions & 0 deletions testdata/exported-issue-519.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Package golint comment
package golint

// Test case for the configuration option tp replace the word "stutters" by "repetitive" failure messages

// GolintRepetitive is a dummy function
func GolintRepetitive() {} // MATCH /func name will be used as golint.GolintRepetitive by other packages, and that is repetitive; consider calling this Repetitive/
10 changes: 10 additions & 0 deletions testdata/exported-issue-552.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// Package golint comment
package golint

// Test cases for enabling checks of exported methods of private types in exported rule
type private struct {
}

// MATCH /comment on exported method private.Method should be of the form "Method ..."/
func (p *private) Method() {
}
7 changes: 7 additions & 0 deletions testdata/exported-issue-555.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Package golint comment
package golint

// Test case for disabling stuttering check in exported rule

// GolintSomething is a dummy function
func GolintSomething() {}
5 changes: 5 additions & 0 deletions testdata/golint/exported.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ package golint

import "net/http"

// GolintFoo is a dummy function
func GolintFoo() {} // MATCH /func name will be used as golint.GolintFoo by other packages, and that stutters; consider calling this Foo/

type (
// O is a shortcut (alias) for map[string]interface{}, e.g. a JSON object.
O = map[string]interface{}
Expand Down Expand Up @@ -53,3 +56,5 @@ func ServeHTTP(w http.ResponseWriter, r *http.Request) {} // MATC
func Read(p []byte) (n int, err error) { return 0, nil } // MATCH /exported function Read should have comment or be unexported/
func Write(p []byte) (n int, err error) { return 0, nil } // MATCH /exported function Write should have comment or be unexported/
func Unwrap(err error) error { return nil } // MATCH /exported function Unwrap should have comment or be unexported/

// The following cases are tests for issue 555

0 comments on commit 18cdb55

Please sign in to comment.