mirror of
https://github.com/mgechev/revive.git
synced 2025-03-17 20:57:58 +02:00
* fix #754 [rule.unhandled-error] change arguments to regexp * Update unhandled-error-w-ignorelist.go * Update unhandled-error_test.go * Update unhandled-error-w-ignorelist.go * adds config backward compatibility test * fix #754 [rule.unhandled-error] change arguments to regexp * fix #754 [rule.unhandled-error] change arguments to regexp * fix #754 [rule.unhandled-error] change arguments to regexp Co-authored-by: chavacava <salvadorcavadini+github@gmail.com>
This commit is contained in:
parent
5f26378cc2
commit
7b1458a9cb
@ -670,13 +670,13 @@ _Configuration_: N/A
|
||||
|
||||
_Description_: This rule warns when errors returned by a function are not explicitly handled on the caller side.
|
||||
|
||||
_Configuration_: function names to ignore
|
||||
_Configuration_: function names regexp patterns to ignore
|
||||
|
||||
Example:
|
||||
|
||||
```toml
|
||||
[unhandled-error]
|
||||
arguments =["fmt.Printf", "myFunction"]
|
||||
arguments =["os\.(Create|WriteFile|Chmod)", "fmt\.Print", "myFunction", "net\..*", "bytes\.Buffer\.Write"]
|
||||
```
|
||||
## unnecessary-stmt
|
||||
|
||||
|
@ -4,6 +4,8 @@ import (
|
||||
"fmt"
|
||||
"go/ast"
|
||||
"go/types"
|
||||
"regexp"
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
"github.com/mgechev/revive/lint"
|
||||
@ -11,24 +13,30 @@ import (
|
||||
|
||||
// UnhandledErrorRule lints given else constructs.
|
||||
type UnhandledErrorRule struct {
|
||||
ignoreList ignoreListType
|
||||
ignoreList []*regexp.Regexp
|
||||
sync.Mutex
|
||||
}
|
||||
|
||||
type ignoreListType map[string]struct{}
|
||||
|
||||
func (r *UnhandledErrorRule) configure(arguments lint.Arguments) {
|
||||
r.Lock()
|
||||
if r.ignoreList == nil {
|
||||
r.ignoreList = make(ignoreListType, len(arguments))
|
||||
|
||||
for _, arg := range arguments {
|
||||
argStr, ok := arg.(string)
|
||||
if !ok {
|
||||
panic(fmt.Sprintf("Invalid argument to the unhandled-error rule. Expecting a string, got %T", arg))
|
||||
}
|
||||
|
||||
r.ignoreList[argStr] = struct{}{}
|
||||
argStr = strings.Trim(argStr, " ")
|
||||
if argStr == "" {
|
||||
panic("Invalid argument to the unhandled-error rule, expected regular expression must not be empty.")
|
||||
}
|
||||
|
||||
exp, err := regexp.Compile(argStr)
|
||||
if err != nil {
|
||||
panic(fmt.Sprintf("Invalid argument to the unhandled-error rule: regexp %q does not compile: %v", argStr, err))
|
||||
}
|
||||
|
||||
r.ignoreList = append(r.ignoreList, exp)
|
||||
}
|
||||
}
|
||||
r.Unlock()
|
||||
@ -60,7 +68,7 @@ func (*UnhandledErrorRule) Name() string {
|
||||
}
|
||||
|
||||
type lintUnhandledErrors struct {
|
||||
ignoreList ignoreListType
|
||||
ignoreList []*regexp.Regexp
|
||||
pkg *lint.Package
|
||||
onFailure func(lint.Failure)
|
||||
}
|
||||
@ -102,8 +110,8 @@ func (w *lintUnhandledErrors) Visit(node ast.Node) ast.Visitor {
|
||||
}
|
||||
|
||||
func (w *lintUnhandledErrors) addFailure(n *ast.CallExpr) {
|
||||
funcName := gofmt(n.Fun)
|
||||
if _, mustIgnore := w.ignoreList[funcName]; mustIgnore {
|
||||
name := w.funcName(n)
|
||||
if w.isIgnoredFunc(name) {
|
||||
return
|
||||
}
|
||||
|
||||
@ -111,10 +119,34 @@ func (w *lintUnhandledErrors) addFailure(n *ast.CallExpr) {
|
||||
Category: "bad practice",
|
||||
Confidence: 1,
|
||||
Node: n,
|
||||
Failure: fmt.Sprintf("Unhandled error in call to function %v", funcName),
|
||||
Failure: fmt.Sprintf("Unhandled error in call to function %v", gofmt(n.Fun)),
|
||||
})
|
||||
}
|
||||
|
||||
func (w *lintUnhandledErrors) funcName(call *ast.CallExpr) string {
|
||||
fn, ok := w.getFunc(call)
|
||||
if !ok {
|
||||
return gofmt(call.Fun)
|
||||
}
|
||||
|
||||
name := fn.FullName()
|
||||
name = strings.Replace(name, "(", "", -1)
|
||||
name = strings.Replace(name, ")", "", -1)
|
||||
name = strings.Replace(name, "*", "", -1)
|
||||
|
||||
return name
|
||||
}
|
||||
|
||||
func (w *lintUnhandledErrors) isIgnoredFunc(funcName string) bool {
|
||||
for _, pattern := range w.ignoreList {
|
||||
if len(pattern.FindString(funcName)) == len(funcName) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
func (*lintUnhandledErrors) isTypeError(t *types.Named) bool {
|
||||
const errorTypeName = "_.error"
|
||||
|
||||
@ -130,3 +162,17 @@ func (w *lintUnhandledErrors) returnsAnError(tt *types.Tuple) bool {
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func (w *lintUnhandledErrors) getFunc(call *ast.CallExpr) (*types.Func, bool) {
|
||||
sel, ok := call.Fun.(*ast.SelectorExpr)
|
||||
if !ok {
|
||||
return nil, false
|
||||
}
|
||||
|
||||
fn, ok := w.pkg.TypesInfo().ObjectOf(sel.Sel).(*types.Func)
|
||||
if !ok {
|
||||
return nil, false
|
||||
}
|
||||
|
||||
return fn, true
|
||||
}
|
||||
|
@ -11,8 +11,15 @@ func TestUnhandledError(t *testing.T) {
|
||||
testRule(t, "unhandled-error", &rule.UnhandledErrorRule{})
|
||||
}
|
||||
|
||||
func TestUnhandledErrorWithBlacklist(t *testing.T) {
|
||||
args := []interface{}{"os.Chdir", "unhandledError1"}
|
||||
func TestUnhandledErrorWithIgnoreList(t *testing.T) {
|
||||
args := []interface{}{
|
||||
`unhandledError1`,
|
||||
`fmt\.Print`,
|
||||
`os\.(Create|WriteFile|Chmod)`,
|
||||
`net\..*`,
|
||||
`bytes\.Buffer\.Write`,
|
||||
`fixtures\.unhandledErrorStruct2\.reterr`,
|
||||
}
|
||||
|
||||
testRule(t, "unhandled-error-w-ignorelist", &rule.UnhandledErrorRule{}, &lint.RuleConfig{Arguments: args})
|
||||
}
|
||||
|
79
testdata/unhandled-error-w-ignorelist.go
vendored
79
testdata/unhandled-error-w-ignorelist.go
vendored
@ -1,7 +1,10 @@
|
||||
package fixtures
|
||||
|
||||
import (
|
||||
b "bytes"
|
||||
"fmt"
|
||||
"io/ioutil"
|
||||
"net"
|
||||
"os"
|
||||
)
|
||||
|
||||
@ -9,11 +12,79 @@ func unhandledError1(a int) (int, error) {
|
||||
return a, nil
|
||||
}
|
||||
|
||||
func prefixunhandledError1suffix(a int) (int, error) {
|
||||
return a, nil
|
||||
}
|
||||
|
||||
func unhandledError2() error {
|
||||
// unhandledError1
|
||||
_, err := unhandledError1(1)
|
||||
unhandledError1(1)
|
||||
fmt.Fprintf(nil, "") // MATCH /Unhandled error in call to function fmt.Fprintf/
|
||||
os.Chdir("..")
|
||||
_ = os.Chdir("..")
|
||||
unhandledError1(1) // ignore
|
||||
prefixunhandledError1suffix(1) // MATCH /Unhandled error in call to function prefixunhandledError1suffix/
|
||||
return err
|
||||
}
|
||||
|
||||
func testCase1() {
|
||||
// fmt\.Print
|
||||
fmt.Print(nil) // ignore
|
||||
fmt.Println("") // MATCH /Unhandled error in call to function fmt.Println/
|
||||
fmt.Printf("%d", 100) // MATCH /Unhandled error in call to function fmt.Printf/
|
||||
fmt.Fprintf(nil, "") // MATCH /Unhandled error in call to function fmt.Fprintf/
|
||||
}
|
||||
|
||||
func testCase2() {
|
||||
// os\.(Create|WriteFile|Chmod)
|
||||
os.Create("test") // ignore
|
||||
os.Chmod("test_file", os.ModeAppend) // ignore
|
||||
os.WriteFile("test_file", []byte("some data"), os.ModeAppend) // ignore
|
||||
|
||||
ioutil.WriteFile("test_file", []byte("some data"), os.ModeAppend) // MATCH /Unhandled error in call to function ioutil.WriteFile/
|
||||
|
||||
_ = os.Chdir("..")
|
||||
os.Chdir("..") // MATCH /Unhandled error in call to function os.Chdir/
|
||||
}
|
||||
|
||||
func testCase3() {
|
||||
// net\..*
|
||||
net.Dial("tcp", "127.0.0.1") // ignore
|
||||
net.ResolveTCPAddr("tcp4", "localhost:8080") // ignore
|
||||
}
|
||||
|
||||
func testCase4() {
|
||||
// bytes\.Buffer\.Write
|
||||
b1 := b.Buffer{}
|
||||
b2 := &b.Buffer{}
|
||||
b1.Write(nil) // ignore
|
||||
b2.Write(nil) // ignore
|
||||
|
||||
b2.Read([]byte("bytes")) // MATCH /Unhandled error in call to function b2.Read/
|
||||
}
|
||||
|
||||
type unhandledErrorStruct1 struct {
|
||||
}
|
||||
|
||||
func (s unhandledErrorStruct1) reterr() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
type unhandledErrorStruct2 struct {
|
||||
}
|
||||
|
||||
func (s unhandledErrorStruct2) reterr() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *unhandledErrorStruct2) reterr1() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
func testCase5() {
|
||||
// fixtures\.unhandledErrorStruct2\.reterr
|
||||
s1 := unhandledErrorStruct1{}
|
||||
_ = s1.reterr()
|
||||
s1.reterr() // MATCH /Unhandled error in call to function s1.reterr/
|
||||
|
||||
s2 := unhandledErrorStruct2{}
|
||||
s2.reterr() // ignore
|
||||
s2.reterr1() // MATCH /Unhandled error in call to function s2.reterr1/
|
||||
}
|
||||
|
Loading…
x
Reference in New Issue
Block a user