Skip to content

Commit ce7f066

Browse files
git-hulkchavacava
andauthored
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 <[email protected]>
1 parent 5caa8cf commit ce7f066

File tree

4 files changed

+67
-8
lines changed

4 files changed

+67
-8
lines changed

README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,8 @@ enableAllRules = true
350350
Arguments = [7]
351351
[rule.function-result-limit]
352352
Arguments = [3]
353+
[rule.error-strings]
354+
Arguments = ["mypackage.Error"]
353355
```
354356

355357
### Default Configuration
@@ -410,7 +412,6 @@ warningCode = 0
410412
## Available Rules
411413

412414
List of all available rules. The rules ported from `golint` are left unchanged and indicated in the `golint` column.
413-
414415
| Name | Config | Description | `golint` | Typed |
415416
| --------------------- | :----: | :--------------------------------------------------------------- | :------: | :---: |
416417
| [`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
423424
| [`context-as-argument`](./RULES_DESCRIPTIONS.md#context-as-argument) | n/a | `context.Context` should be the first argument of a function. | yes | no |
424425
| [`dot-imports`](./RULES_DESCRIPTIONS.md#dot-imports) | n/a | Forbids `.` imports. | yes | no |
425426
| [`error-return`](./RULES_DESCRIPTIONS.md#error-return) | n/a | The error return parameter should be last. | yes | no |
426-
| [`error-strings`](./RULES_DESCRIPTIONS.md#error-strings) | n/a | Conventions around error strings. | yes | no |
427+
| [`error-strings`](./RULES_DESCRIPTIONS.md#error-strings) | []string | Conventions around error strings. | yes | no |
427428
| [`error-naming`](./RULES_DESCRIPTIONS.md#error-naming) | n/a | Naming of error variables. | yes | no |
428429
| [`exported`](./RULES_DESCRIPTIONS.md#exported) | []string | Naming and commenting conventions on exported symbols. | yes | no |
429430
| [`if-return`](./RULES_DESCRIPTIONS.md#if-return) | n/a | Redundant if when returning an error. | no | no |

rule/error-strings.go

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,29 @@ import (
44
"go/ast"
55
"go/token"
66
"strconv"
7+
"strings"
8+
"sync"
79
"unicode"
810
"unicode/utf8"
911

1012
"github.com/mgechev/revive/lint"
1113
)
1214

1315
// ErrorStringsRule lints given else constructs.
14-
type ErrorStringsRule struct{}
16+
type ErrorStringsRule struct {
17+
errorFunctions map[string]map[string]struct{}
18+
sync.Mutex
19+
}
1520

16-
// Apply applies the rule to given file.
17-
func (*ErrorStringsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
18-
var failures []lint.Failure
21+
func (r *ErrorStringsRule) configure(arguments lint.Arguments) {
22+
r.Lock()
23+
defer r.Unlock()
1924

20-
errorFunctions := map[string]map[string]struct{}{
25+
if r.errorFunctions != nil {
26+
return
27+
}
28+
29+
r.errorFunctions = map[string]map[string]struct{}{
2130
"fmt": {
2231
"Errorf": {},
2332
},
@@ -31,11 +40,33 @@ func (*ErrorStringsRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure
3140
},
3241
}
3342

43+
var invalidCustomFunctions []string
44+
for _, argument := range arguments {
45+
if functionName, ok := argument.(string); ok {
46+
fields := strings.Split(strings.TrimSpace(functionName), ".")
47+
if len(fields) != 2 || len(fields[0]) == 0 || len(fields[1]) == 0 {
48+
invalidCustomFunctions = append(invalidCustomFunctions, functionName)
49+
continue
50+
}
51+
r.errorFunctions[fields[0]] = map[string]struct{}{fields[1]: {}}
52+
}
53+
}
54+
if len(invalidCustomFunctions) != 0 {
55+
panic("found invalid custom function: " + strings.Join(invalidCustomFunctions, ","))
56+
}
57+
}
58+
59+
// Apply applies the rule to given file.
60+
func (r *ErrorStringsRule) Apply(file *lint.File, arguments lint.Arguments) []lint.Failure {
61+
var failures []lint.Failure
62+
63+
r.configure(arguments)
64+
3465
fileAst := file.AST
3566
walker := lintErrorStrings{
3667
file: file,
3768
fileAst: fileAst,
38-
errorFunctions: errorFunctions,
69+
errorFunctions: r.errorFunctions,
3970
onFailure: func(failure lint.Failure) {
4071
failures = append(failures, failure)
4172
},
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/mgechev/revive/lint"
7+
"github.com/mgechev/revive/rule"
8+
)
9+
10+
func TestErrorStringsWithCustomFunctions(t *testing.T) {
11+
args := []interface{}{"pkgErrors.Wrap"}
12+
testRule(t, "error-strings-with-custom-functions", &rule.ErrorStringsRule{}, &lint.RuleConfig{
13+
Arguments: args,
14+
})
15+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package fixtures
2+
3+
import (
4+
pkgErrors "github.com/pkg/errors"
5+
)
6+
7+
// Check for the error strings themselves.
8+
9+
func errorsStrings(x int) error {
10+
var err error
11+
return pkgErrors.Wrap(err, "This %d is too low") // MATCH /error strings should not be capitalized or end with punctuation or a newline/
12+
}

0 commit comments

Comments
 (0)