diff --git a/README.md b/README.md index 68292afe9..9fb6e36eb 100644 --- a/README.md +++ b/README.md @@ -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 | diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 48fa41253..4ef9aefcd 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -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 diff --git a/rule/exported.go b/rule/exported.go index b68f2bacc..3dab1bbaf 100644 --- a/rule/exported.go +++ b/rule/exported.go @@ -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, @@ -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) @@ -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) { @@ -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 } @@ -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 @@ -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), }) } } diff --git a/test/exported_test.go b/test/exported_test.go new file mode 100644 index 000000000..c0c7c5831 --- /dev/null +++ b/test/exported_test.go @@ -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}) +} diff --git a/testdata/exported-issue-519.go b/testdata/exported-issue-519.go new file mode 100644 index 000000000..aa29a7e32 --- /dev/null +++ b/testdata/exported-issue-519.go @@ -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/ diff --git a/testdata/exported-issue-552.go b/testdata/exported-issue-552.go new file mode 100644 index 000000000..3b4d4de32 --- /dev/null +++ b/testdata/exported-issue-552.go @@ -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() { +} diff --git a/testdata/exported-issue-555.go b/testdata/exported-issue-555.go new file mode 100644 index 000000000..3f64eef8b --- /dev/null +++ b/testdata/exported-issue-555.go @@ -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() {} diff --git a/testdata/golint/exported.go b/testdata/golint/exported.go index f43a54575..9b93503f6 100644 --- a/testdata/golint/exported.go +++ b/testdata/golint/exported.go @@ -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{} @@ -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