fix(sprintfint): make suggested fix self-contained — add strconv import, remove orphaned fmt import

pkg/linters/sprintfint/sprintfint.go

@@ -16,10 +16,15 @@ import (
 	"github.com/github/gh-aw/pkg/linters/internal/nolint"
 )
 
+const (
+	strconvPkg = "strconv"
+	fmtPkg     = "fmt"
+)
+
 // Analyzer is the sprintfint analysis pass.
 var Analyzer = &analysis.Analyzer{
 	Name:     "sprintfint",
-	Doc:      "reports fmt.Sprintf(\"%d\", x) calls where x is a single int value; use strconv.Itoa(x) instead (suggested fixes may require goimports to add/remove imports)",
+	Doc:      `reports fmt.Sprintf("%d", x) calls where x is a single int value; use strconv.Itoa(x) instead`,
 	URL:      "https://github.com/github/gh-aw/tree/main/pkg/linters/sprintfint",
 	Requires: []*analysis.Analyzer{inspect.Analyzer},
 	Run:      run,
@@ -32,6 +37,11 @@ func run(pass *analysis.Pass) (any, error) {
 	}
 	noLintLinesByFile := nolint.BuildLineIndex(pass, "sprintfint")
 
+	// 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)
+
 	nodeFilter := []ast.Node{
 		(*ast.CallExpr)(nil),
 	}
@@ -82,28 +92,265 @@ func run(pass *analysis.Pass) (any, error) {
 			Pos:            call.Pos(),
 			End:            call.End(),
 			Message:        `use strconv.Itoa(x) instead of fmt.Sprintf("%d", x)`,
-			SuggestedFixes: buildItoaFix(pass, call, arg),
+			SuggestedFixes: buildItoaFix(pass, call, arg, seenImportFiles),
 		})
 	})
 
 	return nil, nil
 }
 
-// buildItoaFix returns a SuggestedFix rewriting
-// fmt.Sprintf("%d", x) → strconv.Itoa(x).
-func buildItoaFix(pass *analysis.Pass, call *ast.CallExpr, arg ast.Expr) []analysis.SuggestedFix {
+// buildItoaFix returns a SuggestedFix rewriting fmt.Sprintf("%d", x) →
+// strconv.Itoa(x). It also emits import TextEdits to add "strconv" and, when
+// the flagged call is the file's only "fmt" reference, to remove the now-
+// unused "fmt" import so the resulting code compiles without a goimports pass.
+func buildItoaFix(pass *analysis.Pass, call *ast.CallExpr, arg ast.Expr, seenImportFiles map[token.Pos]bool) []analysis.SuggestedFix {
 	argText := astutil.NodeText(pass.Fset, arg)
 	if argText == "" {
 		return nil
 	}
+
+	edits := []analysis.TextEdit{{
+		Pos:     call.Pos(),
+		End:     call.End(),
+		NewText: []byte("strconv.Itoa(" + argText + ")"),
+	}}
+
+	// Find the file that contains this call so we can inspect its imports.
+	var file *ast.File
+	for _, f := range pass.Files {
+		if f.Pos() <= call.Pos() && call.Pos() <= f.End() {
+			file = f
+			break
+		}
+	}
+	if file != nil {
+		edits = append(edits, buildImportEdits(pass, file, seenImportFiles)...)
+	}
+
 	return []analysis.SuggestedFix{{
-		Message: "Replace fmt.Sprintf with strconv.Itoa",
-		TextEdits: []analysis.TextEdit{
-			{
-				Pos:     call.Pos(),
-				End:     call.End(),
-				NewText: []byte("strconv.Itoa(" + argText + ")"),
-			},
-		},
+		Message:   "Replace fmt.Sprintf with strconv.Itoa",
+		TextEdits: edits,
 	}}
 }
+
+// buildImportEdits returns TextEdits that add "strconv" to file and, when the
+// flagged call is the file's only "fmt" reference, also remove the now-unused
+// "fmt" import. seenImportFiles prevents duplicate overlapping edits in files
+// with multiple violations.
+func buildImportEdits(pass *analysis.Pass, file *ast.File, seenImportFiles map[token.Pos]bool) []analysis.TextEdit {
+	if seenImportFiles[file.Pos()] {
+		return nil
+	}
+
+	strconvImported := false
+	fmtImported := false
+	for _, imp := range file.Imports {
+		switch imp.Path.Value {
+		case `"` + strconvPkg + `"`:
+			strconvImported = true
+		case `"` + fmtPkg + `"`:
+			fmtImported = true
+		}
+	}
+
+	// If the flagged call is the only "fmt" reference in the file the "fmt"
+	// import will become unused after the fix and must be removed.
+	orphanFmt := fmtImported && countPkgUsesInFile(pass, file, fmtPkg) == 1
+
+	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
+}
+
+// countPkgUsesInFile returns the number of times the package at pkgPath is
+// referenced as a selector base within file (e.g. each "fmt.X" call counts
+// as one use of the "fmt" package).
+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
+}
+
+// addStrconvRemoveFmtEdits returns the TextEdits that simultaneously add
+// "strconv" and remove "fmt" from file's import section. Three structural
+// cases are handled:
+//   - single ungrouped import "fmt"          → replaced with import "strconv"
+//   - grouped import block with only "fmt"   → replaced with import "strconv"
+//   - grouped block with "fmt" + others      → insert "strconv", delete "fmt" line
+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 && imp.Path.Value == `"`+fmtPkg+`"` {
+				fmtSpec = imp
+				fmtDecl = genDecl
+				break
+			}
+		}
+		if fmtDecl != nil {
+			break
+		}
+	}
+	if fmtDecl == nil {
+		return nil
+	}
+
+	// Single ungrouped import or grouped block with only "fmt":
+	// replace the entire declaration with import "strconv".
+	if !fmtDecl.Lparen.IsValid() || len(fmtDecl.Specs) == 1 {
+		return []analysis.TextEdit{{
+			Pos:     fmtDecl.Pos(),
+			End:     fmtDecl.End(),
+			NewText: []byte(`import "` + strconvPkg + `"`),
+		}}
+	}
+
+	// Grouped block with "fmt" alongside other packages: insert "strconv"
+	// before the closing paren and delete the entire "fmt" spec line.
+	lineStart, lineEnd := importSpecLineRange(fset, fmtSpec)
+	return []analysis.TextEdit{
+		{
+			Pos:     fmtDecl.Rparen,
+			End:     fmtDecl.Rparen,
+			NewText: []byte("\t\"" + strconvPkg + "\"\n"),
+		},
+		{
+			Pos:     lineStart,
+			End:     lineEnd,
+			NewText: nil,
+		},
+	}
+}
+
+// addImportEdit returns a TextEdit that inserts an import for pkg into file,
+// choosing the least-invasive insertion point: append to an existing grouped
+// block, convert a single non-grouped import to a grouped block, or insert a
+// standalone declaration after the package name.
+func addImportEdit(pass *analysis.Pass, file *ast.File, pkg string) (analysis.TextEdit, bool) {
+	// Append to an existing grouped import block.
+	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
+	}
+
+	// Convert a single non-grouped import into a grouped block.
+	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
+		}
+	}
+
+	// No existing import block; insert a standalone import after the package name.
+	return analysis.TextEdit{
+		Pos:     file.Name.End(),
+		End:     file.Name.End(),
+		NewText: []byte("\n\nimport \"" + pkg + "\""),
+	}, true
+}
+
+// removeImportEdit returns a TextEdit that removes the import of pkg from
+// file's import section. For an ungrouped or sole-spec grouped declaration the
+// entire decl is removed; for a multi-spec grouped block only the spec line is
+// deleted using line-boundary positions from fset to handle any indentation.
+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 || imp.Path.Value != `"`+pkg+`"` {
+				continue
+			}
+			// Ungrouped or single-spec grouped: remove the entire declaration.
+			if !genDecl.Lparen.IsValid() || len(genDecl.Specs) == 1 {
+				return analysis.TextEdit{
+					Pos:     genDecl.Pos(),
+					End:     genDecl.End(),
+					NewText: nil,
+				}, true
+			}
+			// Multi-spec grouped: remove just this spec's line.
+			lineStart, lineEnd := importSpecLineRange(fset, imp)
+			return analysis.TextEdit{
+				Pos:     lineStart,
+				End:     lineEnd,
+				NewText: nil,
+			}, true
+		}
+	}
+	return analysis.TextEdit{}, false
+}
+
+// importSpecLineRange returns the [start, end) byte range that covers the
+// entire source line of spec — including any leading whitespace and the
+// trailing newline. It uses the token.File's line table so it works correctly
+// regardless of indentation style.
+func importSpecLineRange(fset *token.FileSet, spec *ast.ImportSpec) (token.Pos, token.Pos) {
+	tokFile := fset.File(spec.Pos())
+	if tokFile == nil {
+		// Unreachable in practice; fall back to simple single-char arithmetic.
+		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)
+	}
+	// Last line has no following newline — extend past the spec token end.
+	return lineStart, spec.End() + 1
+}

pkg/linters/sprintfint/testdata/src/sprintfint/singleuse.go

@@ -0,0 +1,12 @@
+// Package sprintfint contains a single-use test fixture for the sprintfint
+// analyzer: the file imports only "fmt" and uses fmt.Sprintf("%d", n) exactly
+// once, exercising the strconv-add and fmt-removal paths in the suggested fix.
+package sprintfint
+
+import "fmt"
+
+// singleUseFmt is the only "fmt" reference in this file, so the fix must add
+// "strconv" and remove the now-unused "fmt" import.
+func singleUseFmt(n int) string {
+	return fmt.Sprintf("%d", n) // want `use strconv\.Itoa\(x\) instead of fmt\.Sprintf\("%d", x\)`
+}

pkg/linters/sprintfint/testdata/src/sprintfint/singleuse.go.golden

@@ -0,0 +1,12 @@
+// Package sprintfint contains a single-use test fixture for the sprintfint
+// analyzer: the file imports only "fmt" and uses fmt.Sprintf("%d", n) exactly
+// once, exercising the strconv-add and fmt-removal paths in the suggested fix.
+package sprintfint
+
+import "strconv"
+
+// singleUseFmt is the only "fmt" reference in this file, so the fix must add
+// "strconv" and remove the now-unused "fmt" import.
+func singleUseFmt(n int) string {
+	return strconv.Itoa(n) // want `use strconv\.Itoa\(x\) instead of fmt\.Sprintf\("%d", x\)`
+}