From 7b1458a9cb2d0b92aa351aa53e86fa16f4598f37 Mon Sep 17 00:00:00 2001 From: Buyanov Vladimir Date: Wed, 9 Nov 2022 15:13:52 +0300 Subject: [PATCH] fix #754 [rule.unhandled-error] change arguments to regexp (#757) * 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 --- RULES_DESCRIPTIONS.md | 4 +- rule/unhandled-error.go | 66 +++++++++++++++++--- test/unhandled-error_test.go | 11 +++- testdata/unhandled-error-w-ignorelist.go | 79 ++++++++++++++++++++++-- 4 files changed, 142 insertions(+), 18 deletions(-) diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md index 0494c29..986ba7a 100644 --- a/RULES_DESCRIPTIONS.md +++ b/RULES_DESCRIPTIONS.md @@ -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 diff --git a/rule/unhandled-error.go b/rule/unhandled-error.go index 6cde24b..32a5fe4 100644 --- a/rule/unhandled-error.go +++ b/rule/unhandled-error.go @@ -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 +} diff --git a/test/unhandled-error_test.go b/test/unhandled-error_test.go index a3a5cc3..f707117 100644 --- a/test/unhandled-error_test.go +++ b/test/unhandled-error_test.go @@ -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}) } diff --git a/testdata/unhandled-error-w-ignorelist.go b/testdata/unhandled-error-w-ignorelist.go index e157a83..d52ab19 100644 --- a/testdata/unhandled-error-w-ignorelist.go +++ b/testdata/unhandled-error-w-ignorelist.go @@ -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/ +}