Skip to content

Commit

Permalink
Allow to customize user functions in rule error-strings (#703)
Browse files Browse the repository at this point in the history
* 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 <salvadorcavadini+github@gmail.com>
  • Loading branch information
git-hulk and chavacava committed Jul 5, 2022
1 parent 5caa8cf commit ce7f066
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 8 deletions.
5 changes: 3 additions & 2 deletions README.md
Expand Up @@ -350,6 +350,8 @@ enableAllRules = true
Arguments = [7]
[rule.function-result-limit]
Arguments = [3]
[rule.error-strings]
Arguments = ["mypackage.Error"]
```

### Default Configuration
Expand Down Expand Up @@ -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 |
Expand All @@ -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 |
Expand Down
43 changes: 37 additions & 6 deletions rule/error-strings.go
Expand Up @@ -4,20 +4,29 @@ import (
"go/ast"
"go/token"
"strconv"
"strings"
"sync"
"unicode"
"unicode/utf8"

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

// 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": {},
},
Expand All @@ -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)
},
Expand Down
15 changes: 15 additions & 0 deletions 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,
})
}
12 changes: 12 additions & 0 deletions 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/
}

0 comments on commit ce7f066

Please sign in to comment.