From ce7f0669d3d6401ec105bc2603912e32869b44e4 Mon Sep 17 00:00:00 2001 From: hulk Date: Wed, 6 Jul 2022 03:51:50 +0800 Subject: [PATCH] Allow to customize user functions in rule `error-strings` (#703) * Allow to customize user functions in rule `error-strings` * Rollback the Available Rules table format in README * adds memoization of the rule's configuration Co-authored-by: chavacava --- README.md | 5 ++- rule/error-strings.go | 43 ++++++++++++++++--- test/error-strings-custom-functions_test.go | 15 +++++++ .../error-strings-with-custom-functions.go | 12 ++++++ 4 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 test/error-strings-custom-functions_test.go create mode 100644 testdata/error-strings-with-custom-functions.go diff --git a/README.md b/README.md index 49f17304d..94bd4daad 100644 --- a/README.md +++ b/README.md @@ -350,6 +350,8 @@ enableAllRules = true Arguments = [7] [rule.function-result-limit] Arguments = [3] +[rule.error-strings] + Arguments = ["mypackage.Error"] ``` ### Default Configuration @@ -410,7 +412,6 @@ warningCode = 0 ## Available Rules List of all available rules. The rules ported from `golint` are left unchanged and indicated in the `golint` column. - | Name | Config | Description | `golint` | Typed | | --------------------- | :----: | :--------------------------------------------------------------- | :------: | :---: | | [`context-keys-type`](./RULES_DESCRIPTIONS.md#context-key-types) | n/a | Disallows the usage of basic types in `context.WithValue`. | yes | yes | @@ -423,7 +424,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a | [`context-as-argument`](./RULES_DESCRIPTIONS.md#context-as-argument) | n/a | `context.Context` should be the first argument of a function. | yes | no | | [`dot-imports`](./RULES_DESCRIPTIONS.md#dot-imports) | n/a | Forbids `.` imports. | yes | no | | [`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-strings`](./RULES_DESCRIPTIONS.md#error-strings) | []string | 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) | []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 | diff --git a/rule/error-strings.go b/rule/error-strings.go index 9e405c29b..81ebda540 100644 --- a/rule/error-strings.go +++ b/rule/error-strings.go @@ -4,6 +4,8 @@ import ( "go/ast" "go/token" "strconv" + "strings" + "sync" "unicode" "unicode/utf8" @@ -11,13 +13,20 @@ import ( ) // ErrorStringsRule lints given else constructs. -type ErrorStringsRule struct{} +type ErrorStringsRule struct { + errorFunctions map[string]map[string]struct{} + sync.Mutex +} -// Apply applies the rule to given file. -func (*ErrorStringsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure { - var failures []lint.Failure +func (r *ErrorStringsRule) configure(arguments lint.Arguments) { + r.Lock() + defer r.Unlock() - errorFunctions := map[string]map[string]struct{}{ + if r.errorFunctions != nil { + return + } + + r.errorFunctions = map[string]map[string]struct{}{ "fmt": { "Errorf": {}, }, @@ -31,11 +40,33 @@ func (*ErrorStringsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure }, } + var invalidCustomFunctions []string + for _, argument := range arguments { + if functionName, ok := argument.(string); ok { + fields := strings.Split(strings.TrimSpace(functionName), ".") + if len(fields) != 2 || len(fields[0]) == 0 || len(fields[1]) == 0 { + invalidCustomFunctions = append(invalidCustomFunctions, functionName) + continue + } + r.errorFunctions[fields[0]] = map[string]struct{}{fields[1]: {}} + } + } + if len(invalidCustomFunctions) != 0 { + panic("found invalid custom function: " + strings.Join(invalidCustomFunctions, ",")) + } +} + +// Apply applies the rule to given file. +func (r *ErrorStringsRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure { + var failures []lint.Failure + + r.configure(arguments) + fileAst := file.AST walker := lintErrorStrings{ file: file, fileAst: fileAst, - errorFunctions: errorFunctions, + errorFunctions: r.errorFunctions, onFailure: func(failure lint.Failure) { failures = append(failures, failure) }, diff --git a/test/error-strings-custom-functions_test.go b/test/error-strings-custom-functions_test.go new file mode 100644 index 000000000..afd183359 --- /dev/null +++ b/test/error-strings-custom-functions_test.go @@ -0,0 +1,15 @@ +package test + +import ( + "testing" + + "github.com/mgechev/revive/lint" + "github.com/mgechev/revive/rule" +) + +func TestErrorStringsWithCustomFunctions(t *testing.T) { + args := []interface{}{"pkgErrors.Wrap"} + testRule(t, "error-strings-with-custom-functions", &rule.ErrorStringsRule{}, &lint.RuleConfig{ + Arguments: args, + }) +} diff --git a/testdata/error-strings-with-custom-functions.go b/testdata/error-strings-with-custom-functions.go new file mode 100644 index 000000000..ec8a4055e --- /dev/null +++ b/testdata/error-strings-with-custom-functions.go @@ -0,0 +1,12 @@ +package fixtures + +import ( + pkgErrors "github.com/pkg/errors" +) + +// Check for the error strings themselves. + +func errorsStrings(x int) error { + var err error + return pkgErrors.Wrap(err, "This %d is too low") // MATCH /error strings should not be capitalized or end with punctuation or a newline/ +}