diff --git a/checkers/rules/rules.go b/checkers/rules/rules.go index 37881da3b..454bcae4f 100644 --- a/checkers/rules/rules.go +++ b/checkers/rules/rules.go @@ -717,3 +717,18 @@ func emptyDecl(m dsl.Matcher) { m.Match(`const()`).Report(`empty const() block`) m.Match(`type()`).Report(`empty type() block`) } + +//doc:summary Detects bad usage of fmt.Errorf +//doc:tags diagnostic experimental opinionated +//doc:before fmt.Errorf(xs) +//doc:after fmt.Errorf("%s", xs) +func undefinedFormatting(m dsl.Matcher) { + m.Match(`fmt.Errorf($f)`). + Where(!m["f"].Const). + Suggest("errors.New($f)"). + Report(`use errors.New($f) or fmt.Errorf("%s", $f) instead`) + + m.Match(`fmt.Errorf($f($*args))`). + Suggest("errors.New($f($*args))"). + Report(`use errors.New($f($*args)) or fmt.Errorf("%s",$f($*args)) instead`) +} diff --git a/checkers/rulesdata/rulesdata.go b/checkers/rulesdata/rulesdata.go index cbf13db62..ec1eb7034 100644 --- a/checkers/rulesdata/rulesdata.go +++ b/checkers/rulesdata/rulesdata.go @@ -2678,6 +2678,49 @@ var PrecompiledRules = &ir.File{ }, }, }, + ir.RuleGroup{ + Line: 717, + Name: "undefinedFormatting", + MatcherName: "m", + DocTags: []string{ + "diagnostic", + "experimental", + }, + DocSummary: "Detects bad usage of fmt.Errorf", + DocBefore: "fmt.Errorf(xs)", + DocAfter: "fmt.Errorf(\"%s\", xs)", + Rules: []ir.Rule{ + ir.Rule{ + Line: 718, + SyntaxPatterns: []ir.PatternString{ + ir.PatternString{Line: 718, Value: "fmt.Errorf($f)"}, + }, + ReportTemplate: "use errors.New($f) or fmt.Errorf(\"%s\", $f) instead", + SuggestTemplate: "errors.New($f)", + WhereExpr: ir.FilterExpr{ + Line: 719, + Op: ir.FilterNotOp, + Src: "!m[\"f\"].Const", + Args: []ir.FilterExpr{ + ir.FilterExpr{ + Line: 719, + Op: ir.FilterVarConstOp, + Src: "m[\"f\"].Const", + Value: "f", + }, + }, + }, + }, + ir.Rule{ + Line: 723, + SyntaxPatterns: []ir.PatternString{ + ir.PatternString{Line: 723, Value: "fmt.Errorf($f($*args))"}, + }, + ReportTemplate: "use errors.New($f($*args)) or fmt.Errorf(\"%s\",$f($*args)) instead", + SuggestTemplate: "errors.New($f($*args))", + }, + }, + }, }, } diff --git a/checkers/testdata/undefinedFormatting/negative_tests.go b/checkers/testdata/undefinedFormatting/negative_tests.go new file mode 100644 index 000000000..7ffdfced0 --- /dev/null +++ b/checkers/testdata/undefinedFormatting/negative_tests.go @@ -0,0 +1,15 @@ +package checker_test + +import ( + "fmt" +) + +var foo = "bar" + +func fmtFormatting() { + fmt.Errorf("%s", foo) + fmt.Errorf("123") + fmt.Errorf("%s", fooError()) +} + +func fooError() string { return "foo" } diff --git a/checkers/testdata/undefinedFormatting/positive_tests.go b/checkers/testdata/undefinedFormatting/positive_tests.go new file mode 100644 index 000000000..5e80c5375 --- /dev/null +++ b/checkers/testdata/undefinedFormatting/positive_tests.go @@ -0,0 +1,19 @@ +package checker_test + +import ( + "fmt" +) + +func fmtUndefinedFormatting() { + foo := "foo happend" + fooFunc := func(k string) (string, string) { return "", "123" } + var barFunc = func() string { return "123" } + /*! use errors.New(foo) or fmt.Errorf("%s", foo) instead */ + fmt.Errorf(foo) + + /*! use errors.New(fooFunc("123")) or fmt.Errorf("%s", fooFunc("123")) instead */ + fmt.Errorf(fooFunc("123")) + + /*! use errors.New(barFunc()) or fmt.Errorf("%s", barFunc()) instead */ + fmt.Errorf(barFunc()) +} diff --git a/framework/lintmain/internal/hotload/hotload.go b/framework/lintmain/internal/hotload/hotload.go index 324d7e78f..5e5567a32 100644 --- a/framework/lintmain/internal/hotload/hotload.go +++ b/framework/lintmain/internal/hotload/hotload.go @@ -7,7 +7,7 @@ import ( "github.com/go-critic/go-critic/framework/linter" ) -// CheckersFromDylib loads checkers provided by a dynamic lybrary found under path. +// CheckersFromDylib loads checkers provided by a dynamic library found under path. // // The returned info slice must be re-assigned to the original info slice, // since there will be new entries there.