diff --git a/cmd/linters/main.go b/cmd/linters/main.go index 218ee04423d..ef212caea99 100644 --- a/cmd/linters/main.go +++ b/cmd/linters/main.go @@ -54,6 +54,7 @@ import ( "github.com/github/gh-aw/pkg/linters/regexpcompileinfunction" "github.com/github/gh-aw/pkg/linters/seenmapbool" "github.com/github/gh-aw/pkg/linters/sortslice" + "github.com/github/gh-aw/pkg/linters/sprintfbool" "github.com/github/gh-aw/pkg/linters/sprintferrdot" "github.com/github/gh-aw/pkg/linters/sprintferrorsnew" "github.com/github/gh-aw/pkg/linters/sprintfint" @@ -112,6 +113,7 @@ func main() { sortslice.Analyzer, sprintferrdot.Analyzer, sprintferrorsnew.Analyzer, + sprintfbool.Analyzer, sprintfint.Analyzer, strconvparseignorederror.Analyzer, stringreplaceminusone.Analyzer, diff --git a/docs/adr/46898-add-sprintfbool-linter.md b/docs/adr/46898-add-sprintfbool-linter.md new file mode 100644 index 00000000000..145be427ef0 --- /dev/null +++ b/docs/adr/46898-add-sprintfbool-linter.md @@ -0,0 +1,44 @@ +# ADR-46898: Add sprintfbool Linter to Flag fmt.Sprintf("%t", b) → strconv.FormatBool(b) + +**Date**: 2026-07-20 +**Status**: Draft +**Deciders**: Unknown + +--- + +### Context + +The repository maintains a suite of custom Go static analysis linters under `pkg/linters/`. A pre-existing `sprintfint` linter already flags `fmt.Sprintf("%d", n)` and recommends `strconv.Itoa(n)` for integer-to-string conversions, because `strconv` avoids the runtime reflection overhead of the `fmt` package. The same performance and clarity argument applies to boolean-to-string conversions: `fmt.Sprintf("%t", b)` uses reflection to format a `bool`, while `strconv.FormatBool(b)` is a direct, zero-allocation call. No existing linter in the suite catches this pattern, leaving a gap in consistency enforcement across the codebase. + +### Decision + +We will add a new `sprintfbool` analysis pass (`pkg/linters/sprintfbool/`) that reports `fmt.Sprintf("%t", b)` calls where `b` has the exact predeclared `bool` type, and provides a suggested fix that rewrites the call to `strconv.FormatBool(b)` while managing `fmt`/`strconv` import additions and removals automatically. The linter will be registered in `cmd/linters/main.go` alongside existing passes, will skip generated files, and will honour `//nolint:sprintfbool` suppression directives. + +### Alternatives Considered + +#### Alternative 1: Extend sprintfint to Cover Both Integer and Bool Patterns + +The `sprintfint` linter could be generalised into a broader "prefer strconv over fmt.Sprintf" pass covering both `%d`/int and `%t`/bool (and potentially other format verbs). This would reduce the number of top-level packages. It was not chosen because combining unrelated verb/type pairs in a single analyzer increases code complexity, makes the diagnostic messages harder to scope precisely, and conflicts with the single-responsibility design pattern already established by the existing linter suite (each linter addresses one specific pattern). + +#### Alternative 2: Rely on an External Linter (e.g., staticcheck S1039 or perfsprint) + +Third-party linters such as `staticcheck` or `perfsprint` can already flag some `fmt.Sprintf` patterns. Delegating to an external tool would avoid adding a new in-house package. This was not chosen because the repository uses an internal linter framework (`pkg/linters/internal/astutil`, `nolint`, `filecheck`) that provides repo-specific affordances (generated-file skipping, `//nolint` directive support, suggested-fix generation with import management). Adding an external tool dependency would require integrating and configuring a separate toolchain and would not benefit from those shared utilities. + +### Consequences + +#### Positive +- Eliminates reflection overhead from `fmt.Sprintf("%t", b)` in flagged call sites, replacing it with the zero-allocation `strconv.FormatBool`. +- Makes bool-to-string conversion intent explicit, consistent with the `sprintfint` rule and the broader repo style of preferring `strconv` for primitive conversions. +- Suggested fixes automatically manage `fmt`/`strconv` imports, reducing friction when applying the rewrite. + +#### Negative +- Adds a new custom package (`pkg/linters/sprintfbool/`) that must be maintained as the Go language, analysis framework, or internal utilities evolve. +- Developers unfamiliar with the linter suite will encounter a new diagnostic category and must learn the `strconv.FormatBool` idiom; this is a minor onboarding cost. + +#### Neutral +- The linter intentionally excludes named bool types (e.g., `type myBool bool`) to avoid false positives, since `strconv.FormatBool` requires the exact predeclared `bool` type. Callers using named bool types will not see diagnostics. +- Calls using multiple arguments (e.g., `fmt.Sprintf("%t %t", a, b)`) or different format verbs (e.g., `%v`) are also excluded, preserving `fmt.Sprintf` where it provides genuine multi-arg formatting. + +--- + +*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.* diff --git a/pkg/linters/sprintfbool/sprintfbool.go b/pkg/linters/sprintfbool/sprintfbool.go new file mode 100644 index 00000000000..bd6a6a9a68c --- /dev/null +++ b/pkg/linters/sprintfbool/sprintfbool.go @@ -0,0 +1,450 @@ +// Package sprintfbool implements a Go analysis linter that flags +// fmt.Sprintf("%t", b) calls where b is a single bool value and suggests +// using strconv.FormatBool(b) instead. +package sprintfbool + +import ( + "go/ast" + "go/token" + "go/types" + stdstrconv "strconv" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + + "github.com/github/gh-aw/pkg/linters/internal/astutil" + "github.com/github/gh-aw/pkg/linters/internal/filecheck" + "github.com/github/gh-aw/pkg/linters/internal/nolint" +) + +const ( + strconvPkg = "strconv" + fmtPkg = "fmt" +) + +type replacement struct { + argText string + qualifier string + canFix bool +} + +// Analyzer is the sprintfbool analysis pass. +var Analyzer = &analysis.Analyzer{ + Name: "sprintfbool", + Doc: `reports fmt.Sprintf("%t", b) calls where b is a single bool value; use strconv.FormatBool(b) instead`, + URL: "https://github.com/github/gh-aw/tree/main/pkg/linters/sprintfbool", + Requires: []*analysis.Analyzer{inspect.Analyzer, nolint.Analyzer, filecheck.Analyzer}, + Run: run, +} + +func run(pass *analysis.Pass) (any, error) { + insp, err := astutil.Inspector(pass) + if err != nil { + return nil, err + } + noLintIndex, err := nolint.Index(pass) + if err != nil { + return nil, err + } + generatedFiles, err := filecheck.Index(pass) + if err != nil { + return nil, err + } + + // seenImportFiles tracks files that have already received an import edit in + // this pass, preventing duplicate overlapping edits when a single file + // contains multiple flagged calls. + seenImportFiles := make(map[token.Pos]bool) + orphanFmtByFile := make(map[token.Pos]bool) + + nodeFilter := []ast.Node{ + (*ast.CallExpr)(nil), + } + + type candidate struct { + call *ast.CallExpr + arg ast.Expr + file *ast.File + } + candidates := make([]candidate, 0) + targetCallsByFile := make(map[token.Pos]int) + fixableCallsByFile := make(map[token.Pos]int) + filesByPos := make(map[token.Pos]*ast.File) + + insp.Preorder(nodeFilter, func(n ast.Node) { + call, ok := n.(*ast.CallExpr) + if !ok { + return + } + + pos := pass.Fset.PositionFor(call.Pos(), false) + if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) { + return + } + if nolint.HasDirectiveForLinter(pos, noLintIndex, "sprintfbool") { + return + } + + // Match fmt.Sprintf(format, arg) with exactly two arguments. + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "Sprintf" { + return + } + if !astutil.IsPkgSelector(pass, sel, "fmt") { + return + } + if len(call.Args) != 2 { + return + } + + // The format argument must be the string literal "%t". + formatLit, ok := call.Args[0].(*ast.BasicLit) + if !ok || formatLit.Kind != token.STRING || formatLit.Value != `"%t"` { + return + } + + // The value argument must have the exact type bool. + arg := call.Args[1] + argType := pass.TypesInfo.TypeOf(arg) + if argType == nil { + return + } + if argType != types.Typ[types.Bool] { + return + } + + file := fileForPos(pass.Files, call.Pos()) + if file != nil { + targetCallsByFile[file.Pos()]++ + filesByPos[file.Pos()] = file + } + + candidates = append(candidates, candidate{ + call: call, + arg: arg, + file: file, + }) + }) + + replacements := make([]replacement, len(candidates)) + for i, c := range candidates { + repl := replacementForCall(pass, c.call, c.arg, c.file) + replacements[i] = repl + if repl.canFix && c.file != nil { + fixableCallsByFile[c.file.Pos()]++ + } + } + + for filePos, targetCalls := range targetCallsByFile { + file := filesByPos[filePos] + if file == nil { + continue + } + fmtImported := false + for _, imp := range file.Imports { + if importSpecPathEquals(imp, fmtPkg) { + fmtImported = true + break + } + } + orphanFmtByFile[filePos] = fmtImported && + countPkgUsesInFile(pass, file, fmtPkg) == targetCalls && + fixableCallsByFile[filePos] == targetCalls + } + + for i, c := range candidates { + repl := replacements[i] + argText := repl.argText + if argText == "" { + argText = astutil.NodeText(pass.Fset, c.arg) + } + if argText == "" { + argText = "b" + } + + var fixes []analysis.SuggestedFix + if repl.canFix { + fixes = buildFormatBoolFix( + pass, + c.call, + repl.argText, + repl.qualifier, + c.file, + seenImportFiles, + orphanFmtByFile, + ) + } + + pass.Report(analysis.Diagnostic{ + Pos: c.call.Pos(), + End: c.call.End(), + Message: `use strconv.FormatBool(` + argText + `) instead of fmt.Sprintf("%t", ` + argText + `)`, + SuggestedFixes: fixes, + }) + } + + return nil, nil +} + +func buildFormatBoolFix( + pass *analysis.Pass, + call *ast.CallExpr, + argText string, + qualifier string, + file *ast.File, + seenImportFiles map[token.Pos]bool, + orphanFmtByFile map[token.Pos]bool, +) []analysis.SuggestedFix { + edits := []analysis.TextEdit{{ + Pos: call.Pos(), + End: call.End(), + NewText: []byte(qualifier + ".FormatBool(" + argText + ")"), + }} + + if file != nil { + edits = append(edits, buildImportEdits(pass, file, seenImportFiles, orphanFmtByFile)...) + } + + return []analysis.SuggestedFix{{ + Message: "Replace fmt.Sprintf with " + qualifier + ".FormatBool", + TextEdits: edits, + }} +} + +func buildImportEdits( + pass *analysis.Pass, + file *ast.File, + seenImportFiles map[token.Pos]bool, + orphanFmtByFile map[token.Pos]bool, +) []analysis.TextEdit { + if seenImportFiles[file.Pos()] { + return nil + } + + strconvImported := false + fmtImported := false + for _, imp := range file.Imports { + switch { + case importSpecPathEquals(imp, strconvPkg): + strconvImported = true + case importSpecPathEquals(imp, fmtPkg): + fmtImported = true + } + } + + orphanFmt := fmtImported && orphanFmtByFile[file.Pos()] + needStrconv := !strconvImported + needRemoveFmt := orphanFmt + + if !needStrconv && !needRemoveFmt { + return nil + } + seenImportFiles[file.Pos()] = true + + switch { + case needStrconv && needRemoveFmt: + return addStrconvRemoveFmtEdits(pass.Fset, file) + case needStrconv: + if edit, ok := addImportEdit(pass, file, strconvPkg); ok { + return []analysis.TextEdit{edit} + } + case needRemoveFmt: + if edit, ok := removeImportEdit(pass.Fset, file, fmtPkg); ok { + return []analysis.TextEdit{edit} + } + } + return nil +} + +func countPkgUsesInFile(pass *analysis.Pass, file *ast.File, pkgPath string) int { + fileStart, fileEnd := file.Pos(), file.End() + count := 0 + for ident, obj := range pass.TypesInfo.Uses { + pkgName, ok := obj.(*types.PkgName) + if !ok || pkgName.Imported() == nil || pkgName.Imported().Path() != pkgPath { + continue + } + if p := ident.Pos(); p >= fileStart && p <= fileEnd { + count++ + } + } + return count +} + +func addStrconvRemoveFmtEdits(fset *token.FileSet, file *ast.File) []analysis.TextEdit { + var fmtSpec *ast.ImportSpec + var fmtDecl *ast.GenDecl + + for _, decl := range file.Decls { + genDecl, ok := decl.(*ast.GenDecl) + if !ok || genDecl.Tok != token.IMPORT { + continue + } + for _, spec := range genDecl.Specs { + imp, ok := spec.(*ast.ImportSpec) + if ok && importSpecPathEquals(imp, fmtPkg) { + fmtSpec = imp + fmtDecl = genDecl + break + } + } + if fmtDecl != nil { + break + } + } + if fmtDecl == nil { + return nil + } + + if !fmtDecl.Lparen.IsValid() || len(fmtDecl.Specs) == 1 { + return []analysis.TextEdit{{ + Pos: fmtDecl.Pos(), + End: fmtDecl.End(), + NewText: []byte(`import "` + strconvPkg + `"`), + }} + } + + lineStart, lineEnd := importSpecLineRange(fset, fmtSpec) + return []analysis.TextEdit{ + { + Pos: lineStart, + End: lineEnd, + NewText: nil, + }, + { + Pos: fmtDecl.Rparen, + End: fmtDecl.Rparen, + NewText: []byte("\t\"" + strconvPkg + "\"\n"), + }, + } +} + +func addImportEdit(pass *analysis.Pass, file *ast.File, pkg string) (analysis.TextEdit, bool) { + for _, decl := range file.Decls { + genDecl, ok := decl.(*ast.GenDecl) + if !ok || genDecl.Tok != token.IMPORT || !genDecl.Lparen.IsValid() { + continue + } + return analysis.TextEdit{ + Pos: genDecl.Rparen, + End: genDecl.Rparen, + NewText: []byte("\t\"" + pkg + "\"\n"), + }, true + } + + if len(file.Imports) == 1 { + for _, decl := range file.Decls { + genDecl, ok := decl.(*ast.GenDecl) + if !ok || genDecl.Tok != token.IMPORT || genDecl.Lparen.IsValid() { + continue + } + specText := astutil.NodeText(pass.Fset, genDecl.Specs[0]) + if specText == "" { + continue + } + return analysis.TextEdit{ + Pos: genDecl.Pos(), + End: genDecl.End(), + NewText: []byte("import (\n\t" + specText + "\n\t\"" + pkg + "\"\n)"), + }, true + } + } + + return analysis.TextEdit{ + Pos: file.Name.End(), + End: file.Name.End(), + NewText: []byte("\n\nimport \"" + pkg + "\""), + }, true +} + +func removeImportEdit(fset *token.FileSet, file *ast.File, pkg string) (analysis.TextEdit, bool) { + for _, decl := range file.Decls { + genDecl, ok := decl.(*ast.GenDecl) + if !ok || genDecl.Tok != token.IMPORT { + continue + } + for _, spec := range genDecl.Specs { + imp, ok := spec.(*ast.ImportSpec) + if !ok || !importSpecPathEquals(imp, pkg) { + continue + } + if !genDecl.Lparen.IsValid() || len(genDecl.Specs) == 1 { + return analysis.TextEdit{ + Pos: genDecl.Pos(), + End: genDecl.End(), + NewText: nil, + }, true + } + lineStart, lineEnd := importSpecLineRange(fset, imp) + return analysis.TextEdit{ + Pos: lineStart, + End: lineEnd, + NewText: nil, + }, true + } + } + return analysis.TextEdit{}, false +} + +func importSpecLineRange(fset *token.FileSet, spec *ast.ImportSpec) (token.Pos, token.Pos) { + tokFile := fset.File(spec.Pos()) + if tokFile == nil { + return spec.Pos() - 1, spec.End() + 1 + } + line := tokFile.Line(spec.Pos()) + lineStart := tokFile.LineStart(line) + if line < tokFile.LineCount() { + return lineStart, tokFile.LineStart(line + 1) + } + return lineStart, spec.End() + 1 +} + +func fileForPos(files []*ast.File, pos token.Pos) *ast.File { + for _, file := range files { + if file.Pos() <= pos && pos <= file.End() { + return file + } + } + return nil +} + +func replacementForCall(pass *analysis.Pass, call *ast.CallExpr, arg ast.Expr, file *ast.File) replacement { + argText := astutil.NodeText(pass.Fset, arg) + if argText == "" { + return replacement{} + } + + qualifier := strconvPkg + if file != nil { + if localName, imported := astutil.ImportedAs(file, pass.TypesInfo, strconvPkg); imported { + if localName == "." || localName == "_" { + return replacement{argText: argText} + } + qualifier = localName + } + } + + if astutil.QualifierShadowed(pass.Pkg, call.Pos(), qualifier, strconvPkg) { + return replacement{argText: argText} + } + if astutil.HasOverlappingComment(pass.Files, call.Pos(), call.End()) { + return replacement{argText: argText} + } + + return replacement{ + argText: argText, + qualifier: qualifier, + canFix: true, + } +} + +func importSpecPathEquals(spec *ast.ImportSpec, pkgPath string) bool { + if spec == nil || spec.Path == nil { + return false + } + unquoted, err := stdstrconv.Unquote(spec.Path.Value) + if err != nil { + return spec.Path.Value == `"`+pkgPath+`"` || spec.Path.Value == "`"+pkgPath+"`" + } + return unquoted == pkgPath +} diff --git a/pkg/linters/sprintfbool/sprintfbool_test.go b/pkg/linters/sprintfbool/sprintfbool_test.go new file mode 100644 index 00000000000..6641c132dc2 --- /dev/null +++ b/pkg/linters/sprintfbool/sprintfbool_test.go @@ -0,0 +1,17 @@ +//go:build !integration + +// Package sprintfbool_test provides tests for the sprintfbool analyzer. +package sprintfbool_test + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" + + "github.com/github/gh-aw/pkg/linters/sprintfbool" +) + +func TestSprintfBool(t *testing.T) { + testdata := analysistest.TestData() + analysistest.RunWithSuggestedFixes(t, testdata, sprintfbool.Analyzer, "sprintfbool") +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/aliased_import.go b/pkg/linters/sprintfbool/testdata/src/sprintfbool/aliased_import.go new file mode 100644 index 00000000000..ce14495abc9 --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/aliased_import.go @@ -0,0 +1,14 @@ +package sprintfbool + +import ( + "fmt" + sc "strconv" +) + +func badAliased(b bool) string { + return fmt.Sprintf("%t", b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} + +func goodAliasedAlready(b bool) string { + return sc.FormatBool(b) +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/aliased_import.go.golden b/pkg/linters/sprintfbool/testdata/src/sprintfbool/aliased_import.go.golden new file mode 100644 index 00000000000..8d14e822baf --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/aliased_import.go.golden @@ -0,0 +1,13 @@ +package sprintfbool + +import ( + sc "strconv" +) + +func badAliased(b bool) string { + return sc.FormatBool(b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} + +func goodAliasedAlready(b bool) string { + return sc.FormatBool(b) +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/comment_overlap.go b/pkg/linters/sprintfbool/testdata/src/sprintfbool/comment_overlap.go new file mode 100644 index 00000000000..6b3de60ee78 --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/comment_overlap.go @@ -0,0 +1,7 @@ +package sprintfbool + +import "fmt" + +func badCommentOverlap(b bool) string { + return fmt.Sprintf("%t", /* rationale */ b) // want `use strconv\.FormatBool\(.*\) instead of fmt\.Sprintf\("%t", .*\)` +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/comment_overlap.go.golden b/pkg/linters/sprintfbool/testdata/src/sprintfbool/comment_overlap.go.golden new file mode 100644 index 00000000000..6b3de60ee78 --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/comment_overlap.go.golden @@ -0,0 +1,7 @@ +package sprintfbool + +import "fmt" + +func badCommentOverlap(b bool) string { + return fmt.Sprintf("%t", /* rationale */ b) // want `use strconv\.FormatBool\(.*\) instead of fmt\.Sprintf\("%t", .*\)` +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/keep_fmt.go b/pkg/linters/sprintfbool/testdata/src/sprintfbool/keep_fmt.go new file mode 100644 index 00000000000..0bfa825b20f --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/keep_fmt.go @@ -0,0 +1,11 @@ +package sprintfbool + +import "fmt" + +func badKeepFmt(b bool) string { + return fmt.Sprintf("%t", b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} + +func stillUsesFmt(b bool) string { + return fmt.Sprintf("%v", b) +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/keep_fmt.go.golden b/pkg/linters/sprintfbool/testdata/src/sprintfbool/keep_fmt.go.golden new file mode 100644 index 00000000000..dabf8d946aa --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/keep_fmt.go.golden @@ -0,0 +1,14 @@ +package sprintfbool + +import ( + "fmt" + "strconv" +) + +func badKeepFmt(b bool) string { + return strconv.FormatBool(b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} + +func stillUsesFmt(b bool) string { + return fmt.Sprintf("%v", b) +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/multiple.go b/pkg/linters/sprintfbool/testdata/src/sprintfbool/multiple.go new file mode 100644 index 00000000000..e3f0aad1dc4 --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/multiple.go @@ -0,0 +1,11 @@ +package sprintfbool + +import "fmt" + +func badOne(b bool) string { + return fmt.Sprintf("%t", b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} + +func badTwo(b bool) string { + return fmt.Sprintf("%t", b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/multiple.go.golden b/pkg/linters/sprintfbool/testdata/src/sprintfbool/multiple.go.golden new file mode 100644 index 00000000000..207849239fe --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/multiple.go.golden @@ -0,0 +1,11 @@ +package sprintfbool + +import "strconv" + +func badOne(b bool) string { + return strconv.FormatBool(b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} + +func badTwo(b bool) string { + return strconv.FormatBool(b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/raw_imports.go b/pkg/linters/sprintfbool/testdata/src/sprintfbool/raw_imports.go new file mode 100644 index 00000000000..5ce7c505de4 --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/raw_imports.go @@ -0,0 +1,14 @@ +package sprintfbool + +import ( + `fmt` + `strconv` +) + +func badRawImports(b bool) string { + return fmt.Sprintf("%t", b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} + +func goodRawImports(b bool) string { + return strconv.FormatBool(b) +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/raw_imports.go.golden b/pkg/linters/sprintfbool/testdata/src/sprintfbool/raw_imports.go.golden new file mode 100644 index 00000000000..a20a972c35e --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/raw_imports.go.golden @@ -0,0 +1,13 @@ +package sprintfbool + +import ( + "strconv" +) + +func badRawImports(b bool) string { + return strconv.FormatBool(b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} + +func goodRawImports(b bool) string { + return strconv.FormatBool(b) +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/shadow.go b/pkg/linters/sprintfbool/testdata/src/sprintfbool/shadow.go new file mode 100644 index 00000000000..38e015ca47e --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/shadow.go @@ -0,0 +1,7 @@ +package sprintfbool + +import "fmt" + +func shadowStrconv(strconv bool, b bool) string { + return fmt.Sprintf("%t", b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/shadow.go.golden b/pkg/linters/sprintfbool/testdata/src/sprintfbool/shadow.go.golden new file mode 100644 index 00000000000..38e015ca47e --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/shadow.go.golden @@ -0,0 +1,7 @@ +package sprintfbool + +import "fmt" + +func shadowStrconv(strconv bool, b bool) string { + return fmt.Sprintf("%t", b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/singleuse.go b/pkg/linters/sprintfbool/testdata/src/sprintfbool/singleuse.go new file mode 100644 index 00000000000..125dc29d429 --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/singleuse.go @@ -0,0 +1,7 @@ +package sprintfbool + +import "fmt" + +func singleUseFmt(b bool) string { + return fmt.Sprintf("%t", b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/singleuse.go.golden b/pkg/linters/sprintfbool/testdata/src/sprintfbool/singleuse.go.golden new file mode 100644 index 00000000000..00a225beb4b --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/singleuse.go.golden @@ -0,0 +1,7 @@ +package sprintfbool + +import "strconv" + +func singleUseFmt(b bool) string { + return strconv.FormatBool(b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/sprintfbool.go b/pkg/linters/sprintfbool/testdata/src/sprintfbool/sprintfbool.go new file mode 100644 index 00000000000..dc22dfa114d --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/sprintfbool.go @@ -0,0 +1,46 @@ +// Package sprintfbool is the test fixture for the sprintfbool analyzer. +package sprintfbool + +import ( + "fmt" + "strconv" +) + +type myBool bool + +// bad demonstrates the flagged pattern: fmt.Sprintf with a single "%t" verb +// and a bool argument, which should be replaced by strconv.FormatBool. +func bad(b bool) string { + return fmt.Sprintf("%t", b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} + +// badLiteral demonstrates the flagged pattern with a bool literal. +func badLiteral() string { + return fmt.Sprintf("%t", true) // want `use strconv\.FormatBool\(true\) instead of fmt\.Sprintf\("%t", true\)` +} + +// goodStrconvFormatBool is already using the preferred form — no diagnostic expected. +func goodStrconvFormatBool(b bool) string { + return strconv.FormatBool(b) +} + +// goodNamedBool uses a named bool type — not flagged because strconv.FormatBool +// requires the exact predeclared bool type. +func goodNamedBool(b myBool) string { + return fmt.Sprintf("%t", b) +} + +// goodMultipleVerbs uses more than one verb — not flagged. +func goodMultipleVerbs(a, b bool) string { + return fmt.Sprintf("%t %t", a, b) +} + +// goodOtherVerb uses a different verb — not flagged. +func goodOtherVerb(b bool) string { + return fmt.Sprintf("%v", b) +} + +// suppressed suppresses the linter directive — no diagnostic expected. +func suppressed(b bool) string { + return fmt.Sprintf("%t", b) //nolint:sprintfbool +} diff --git a/pkg/linters/sprintfbool/testdata/src/sprintfbool/sprintfbool.go.golden b/pkg/linters/sprintfbool/testdata/src/sprintfbool/sprintfbool.go.golden new file mode 100644 index 00000000000..44d92b469cc --- /dev/null +++ b/pkg/linters/sprintfbool/testdata/src/sprintfbool/sprintfbool.go.golden @@ -0,0 +1,46 @@ +// Package sprintfbool is the test fixture for the sprintfbool analyzer. +package sprintfbool + +import ( + "fmt" + "strconv" +) + +type myBool bool + +// bad demonstrates the flagged pattern: fmt.Sprintf with a single "%t" verb +// and a bool argument, which should be replaced by strconv.FormatBool. +func bad(b bool) string { + return strconv.FormatBool(b) // want `use strconv\.FormatBool\(b\) instead of fmt\.Sprintf\("%t", b\)` +} + +// badLiteral demonstrates the flagged pattern with a bool literal. +func badLiteral() string { + return strconv.FormatBool(true) // want `use strconv\.FormatBool\(true\) instead of fmt\.Sprintf\("%t", true\)` +} + +// goodStrconvFormatBool is already using the preferred form — no diagnostic expected. +func goodStrconvFormatBool(b bool) string { + return strconv.FormatBool(b) +} + +// goodNamedBool uses a named bool type — not flagged because strconv.FormatBool +// requires the exact predeclared bool type. +func goodNamedBool(b myBool) string { + return fmt.Sprintf("%t", b) +} + +// goodMultipleVerbs uses more than one verb — not flagged. +func goodMultipleVerbs(a, b bool) string { + return fmt.Sprintf("%t %t", a, b) +} + +// goodOtherVerb uses a different verb — not flagged. +func goodOtherVerb(b bool) string { + return fmt.Sprintf("%v", b) +} + +// suppressed suppresses the linter directive — no diagnostic expected. +func suppressed(b bool) string { + return fmt.Sprintf("%t", b) //nolint:sprintfbool +}