diff --git a/staticcheck/sa1006/sa1006.go b/staticcheck/sa1006/sa1006.go index c613808dd..533f773fe 100644 --- a/staticcheck/sa1006/sa1006.go +++ b/staticcheck/sa1006/sa1006.go @@ -4,6 +4,7 @@ import ( "fmt" "go/ast" "go/types" + "strconv" "honnef.co/go/tools/analysis/code" "honnef.co/go/tools/analysis/edit" @@ -74,6 +75,38 @@ var query1 = pattern.MustParse(` var query2 = pattern.MustParse(`(CallExpr (Symbol "fmt.Fprintf") _:format:[])`) func run(pass *analysis.Pass) (any, error) { + importName := func(node ast.Node, path string) (string, bool) { + for _, file := range pass.Files { + if node.Pos() < file.Pos() || node.Pos() > file.End() { + continue + } + for _, spec := range file.Imports { + pkgPath, err := strconv.Unquote(spec.Path.Value) + if err != nil || pkgPath != path { + continue + } + var pkgName *types.PkgName + if spec.Name != nil { + pkgName, _ = pass.TypesInfo.Defs[spec.Name].(*types.PkgName) + } else { + pkgName, _ = pass.TypesInfo.Implicits[spec].(*types.PkgName) + } + if pkgName == nil || pkgName.Name() == "." || pkgName.Name() == "_" { + return "", false + } + scope := pass.TypesInfo.Scopes[file].Innermost(node.Pos()) + if scope != nil { + _, obj := scope.LookupParent(pkgName.Name(), node.Pos()) + if obj == pkgName { + return pkgName.Name(), true + } + } + return "", false + } + } + return "", false + } + for node, m := range code.Matches(pass, query1, query2) { call := node.(*ast.CallExpr) name, ok := m.State["name"].(string) @@ -96,9 +129,14 @@ func run(pass *analysis.Pass) (any, error) { } var alt string + var fixes []analysis.SuggestedFix if name == "fmt.Errorf" { - // The alternative to fmt.Errorf isn't fmt.Error but errors.New - alt = "errors.New" + // The alternative to fmt.Errorf is errors.New, but we cannot + // suggest it unless errors is already imported. + if errorsName, ok := importName(node, "errors"); ok { + alt = errorsName + ".New" + fixes = append(fixes, edit.Fix(fmt.Sprintf("Use %s instead of %s", alt, name), edit.ReplaceWithString(call.Fun, alt))) + } } else { // This can be either a function call like log.Printf or a method call with an // arbitrarily complex selector, such as foo.bar[0].Printf. In either case, @@ -106,10 +144,14 @@ func run(pass *analysis.Pass) (any, error) { // expression. alt = report.Render(pass, call.Fun) alt = alt[:len(alt)-1] + fixes = append(fixes, edit.Fix(fmt.Sprintf("Use %s instead of %s", alt, name), edit.ReplaceWithString(call.Fun, alt))) + } + var opts []report.Option + if len(fixes) > 0 { + opts = append(opts, report.Fixes(fixes...)) } report.Report(pass, call, - "printf-style function with dynamic format string and no further arguments should use print-style function instead", - report.Fixes(edit.Fix(fmt.Sprintf("Use %s instead of %s", alt, name), edit.ReplaceWithString(call.Fun, alt)))) + "printf-style function with dynamic format string and no further arguments should use print-style function instead", opts...) } return nil, nil } diff --git a/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintf/CheckUnsafePrintf.go.golden b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintf/CheckUnsafePrintf.go.golden index a36a6fa64..33d2782a0 100644 --- a/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintf/CheckUnsafePrintf.go.golden +++ b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintf/CheckUnsafePrintf.go.golden @@ -44,7 +44,7 @@ func fn(s string) { tb.Fatal(s) //@ diag(`should use print-style function`) tb.Skip(s) //@ diag(`should use print-style function`) - errors.New(s) //@ diag(`should use print-style function`) + fmt.Errorf(s) //@ diag(`should use print-style function`) var nested struct { l log.Logger diff --git a/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/alias.go b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/alias.go new file mode 100644 index 000000000..d09b78f22 --- /dev/null +++ b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/alias.go @@ -0,0 +1,12 @@ +package pkg + +import ( + err "errors" + "fmt" +) + +var _ = err.New + +func alias(s string) { + _ = fmt.Errorf(s) //@ diag(`should use print-style function`) +} diff --git a/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/alias.go.golden b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/alias.go.golden new file mode 100644 index 000000000..08d4b7594 --- /dev/null +++ b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/alias.go.golden @@ -0,0 +1,12 @@ +package pkg + +import ( + err "errors" + "fmt" +) + +var _ = err.New + +func alias(s string) { + _ = err.New(s) //@ diag(`should use print-style function`) +} diff --git a/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/import.go b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/import.go new file mode 100644 index 000000000..e13b79df7 --- /dev/null +++ b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/import.go @@ -0,0 +1,5 @@ +package pkg + +import errpkg "errors" + +var _ = errpkg.New diff --git a/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/import.go.golden b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/import.go.golden new file mode 100644 index 000000000..e13b79df7 --- /dev/null +++ b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/import.go.golden @@ -0,0 +1,5 @@ +package pkg + +import errpkg "errors" + +var _ = errpkg.New diff --git a/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/other.go b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/other.go new file mode 100644 index 000000000..bb3800b4b --- /dev/null +++ b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/other.go @@ -0,0 +1,7 @@ +package pkg + +import "fmt" + +func other(s string) { + _ = fmt.Errorf(s) //@ diag(`should use print-style function`) +} diff --git a/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/other.go.golden b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/other.go.golden new file mode 100644 index 000000000..bb3800b4b --- /dev/null +++ b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/other.go.golden @@ -0,0 +1,7 @@ +package pkg + +import "fmt" + +func other(s string) { + _ = fmt.Errorf(s) //@ diag(`should use print-style function`) +} diff --git a/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/shadow.go b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/shadow.go new file mode 100644 index 000000000..ed2fa91a5 --- /dev/null +++ b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/shadow.go @@ -0,0 +1,14 @@ +package pkg + +import ( + errpkg "errors" + "fmt" +) + +var _ = errpkg.New + +func shadow(s string) { + errpkg := 1 + _ = errpkg + _ = fmt.Errorf(s) //@ diag(`should use print-style function`) +} diff --git a/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/shadow.go.golden b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/shadow.go.golden new file mode 100644 index 000000000..ed2fa91a5 --- /dev/null +++ b/staticcheck/sa1006/testdata/go1.0/CheckUnsafePrintfScope/shadow.go.golden @@ -0,0 +1,14 @@ +package pkg + +import ( + errpkg "errors" + "fmt" +) + +var _ = errpkg.New + +func shadow(s string) { + errpkg := 1 + _ = errpkg + _ = fmt.Errorf(s) //@ diag(`should use print-style function`) +}