Skip to content

Commit

Permalink
fix: error-strings custom funcs overwrites defaults (#1249)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexandear authored Feb 26, 2025
1 parent 9177f50 commit 67d0a61
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 7 deletions.
19 changes: 12 additions & 7 deletions rule/error_strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,19 @@ func (r *ErrorStringsRule) Configure(arguments lint.Arguments) error {

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]: {}}
pkgFunction, ok := argument.(string)
if !ok {
continue
}
pkg, function, ok := strings.Cut(strings.TrimSpace(pkgFunction), ".")
if !ok || pkg == "" || function == "" {
invalidCustomFunctions = append(invalidCustomFunctions, pkgFunction)
continue
}
if _, ok := r.errorFunctions[pkg]; !ok {
r.errorFunctions[pkg] = map[string]struct{}{}
}
r.errorFunctions[pkg][function] = struct{}{}
}
if len(invalidCustomFunctions) != 0 {
return fmt.Errorf("found invalid custom function: %s", strings.Join(invalidCustomFunctions, ","))
Expand Down
68 changes: 68 additions & 0 deletions rule/error_strings_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package rule_test

import (
"errors"
"testing"

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

func TestErrorStringsRule_Configure(t *testing.T) {
tests := []struct {
name string
arguments lint.Arguments
wantErr error
}{
{
name: "Default configuration",
arguments: lint.Arguments{},
},
{
name: "Valid custom functions",
arguments: lint.Arguments{"mypkg.MyErrorFunc", "errors.New"},
},
{
name: "Argument not a string",
arguments: lint.Arguments{123},
},
{
name: "Invalid package",
arguments: lint.Arguments{".MyErrorFunc"},
wantErr: errors.New("found invalid custom function: .MyErrorFunc"),
},
{
name: "Invalid function",
arguments: lint.Arguments{"errors."},
wantErr: errors.New("found invalid custom function: errors."),
},
{
name: "Invalid custom function",
arguments: lint.Arguments{"invalidFunction"},
wantErr: errors.New("found invalid custom function: invalidFunction"),
},
{
name: "Mixed valid and invalid custom functions",
arguments: lint.Arguments{"mypkg.MyErrorFunc", "invalidFunction", "invalidFunction2"},
wantErr: errors.New("found invalid custom function: invalidFunction,invalidFunction2"),
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var r rule.ErrorStringsRule

err := r.Configure(tt.arguments)

if tt.wantErr == nil {
if err != nil {
t.Errorf("Configure() unexpected non-nil error %q", err)
}
return
}
if err == nil || err.Error() != tt.wantErr.Error() {
t.Errorf("Configure() unexpected error: got %q, want %q", err, tt.wantErr)
}
})
}
}
7 changes: 7 additions & 0 deletions test/error_strings_custom_functions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,10 @@ func TestErrorStringsWithCustomFunctions(t *testing.T) {
Arguments: args,
})
}

func TestErrorStringsIssue1243(t *testing.T) {
args := []any{"errors.Wrap"}
testRule(t, "error_strings_issue_1243", &rule.ErrorStringsRule{}, &lint.RuleConfig{
Arguments: args,
})
}
11 changes: 11 additions & 0 deletions testdata/error_strings_issue_1243.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package fixtures

import (
"errors"
"fmt"
)

func issue1243() {
err := errors.New("An error occurred!") // MATCH /error strings should not be capitalized or end with punctuation or a newline/
fmt.Println(err)
}

0 comments on commit 67d0a61

Please sign in to comment.