[linter-miner] feat(linters): add mapclearloop linter — replace range-delete loops with clear(m)

cmd/linters/main.go

@@ -42,6 +42,7 @@ import (
 	"github.com/github/gh-aw/pkg/linters/lenstringzero"
 	"github.com/github/gh-aw/pkg/linters/logfatallibrary"
 	"github.com/github/gh-aw/pkg/linters/manualmutexunlock"
+	"github.com/github/gh-aw/pkg/linters/mapclearloop"
 	"github.com/github/gh-aw/pkg/linters/mapdeletecheck"
 	"github.com/github/gh-aw/pkg/linters/nilctxpassed"
 	"github.com/github/gh-aw/pkg/linters/osexitinlibrary"
@@ -93,6 +94,7 @@ func main() {
 		largefunc.Analyzer,
 		logfatallibrary.Analyzer,
 		manualmutexunlock.Analyzer,
+		mapclearloop.Analyzer,
 		mapdeletecheck.Analyzer,
 		nilctxpassed.Analyzer,
 		osexitinlibrary.Analyzer,

docs/adr/46060-add-mapclearloop-linter.md

@@ -0,0 +1,50 @@
+# ADR-46060: Add mapclearloop Linter to Detect Range-Delete Loops Replaceable with clear(m)
+
+**Date**: 2026-07-16
+**Status**: Draft
+**Deciders**: Unknown (automated PR — pelikhan)
+
+---
+
+### Context
+
+Go 1.21 introduced the built-in `clear(m)` function, which atomically clears all entries from a map in a single call. Before Go 1.21, the idiomatic way to clear a map was a range-over-map loop with a `delete` call per entry: `for k := range m { delete(m, k) }`. Codebases that have been upgraded to Go 1.21+ frequently retain the old loop pattern due to habit or incremental migration. This project maintains a suite of custom static analysis linters (in `pkg/linters/`) that enforce idiomatic Go style across the codebase. A linter for this pattern is a natural addition: it is high-signal (the pattern is unambiguous), auto-fixable, and complements the existing `mapdeletecheck` linter which targets related map/delete idioms.
+
+### Decision
+
+We will add a new `mapclearloop` analysis pass (`pkg/linters/mapclearloop/`) that detects range-over-map loops whose sole body statement is `delete(m, k)` targeting the same map and key variable from the range, and flags them as candidates for replacement with `clear(m)`. The analyzer emits a `SuggestedFix` enabling automatic rewriting. It will be registered in `cmd/linters/main.go` alongside all other linters in this project.
+
+### Alternatives Considered
+
+#### Alternative 1: Extend the existing `mapdeletecheck` linter
+
+The `mapdeletecheck` linter already handles a related idiom. Extending it to also catch the range-delete-entire-map pattern would avoid creating a new package and keep map-related lint rules in one place.
+
+This was not chosen because `mapdeletecheck` addresses a distinct class of problems (likely incorrect individual deletes, e.g., deleting while iterating with possible side effects). Merging two semantically different checks into one package would blur responsibility boundaries, complicate testing, and make the codebase harder to navigate. The project's existing convention is one package per linter.
+
+#### Alternative 2: Rely on an upstream linter (e.g., golangci-lint, staticcheck, revive)
+
+Rather than maintaining a custom analyzer, the team could wait for the broader Go tooling ecosystem to provide this check, or configure an existing tool that already detects it.
+
+This was not chosen because the project requires tight integration with its own linter infrastructure (nolint directives, filecheck for generated file skipping, the shared `astutil` helpers, and registration in the project's own linter runner). Upstream tools may not respect project-local nolint conventions or may have false positives in the project's specific patterns. Additionally, having the linter in-tree allows immediate fixes through `SuggestedFix` and ensures the check is versioned with the codebase.
+
+### Consequences
+
+#### Positive
+- Code that clears maps will be more concise and express intent clearly via `clear(m)`.
+- The linter has a high signal-to-noise ratio: it only fires when the range body is exactly one `delete` call on the same map with the same key, verified at the type level using `go/types` (e.g., shadowed `delete` identifiers are correctly excluded).
+- The `SuggestedFix` enables automatic code rewriting, reducing developer toil when fixing violations.
+- The check skips generated files via `filecheck`, avoiding noisy violations in machine-generated code.
+
+#### Negative
+- Maintainers must maintain another custom linter package indefinitely.
+- Existing code in repositories using this linter suite will gain new lint failures on upgrade and must be updated to use `clear(m)`.
+- The linter only targets Go 1.21+ idiom; it would produce confusing violations in codebases that cannot use Go 1.21 — though in practice this project already requires a modern Go version.
+
+#### Neutral
+- The linter follows the same structural conventions as all other linters in `pkg/linters/`: `Analyzer` var, `run` function, `testdata/` fixture with `.golden` file, `analysistest` test harness.
+- No new external dependencies are introduced; `golang.org/x/tools/go/analysis` is already a project dependency.
+
+---
+
+*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*

pkg/linters/mapclearloop/mapclearloop.go

@@ -0,0 +1,208 @@
+// Package mapclearloop implements a Go analysis linter that flags
+// range-over-map loops whose only body statement is delete(m, k),
+// which should be replaced by the built-in clear(m) introduced in Go 1.21.
+package mapclearloop
+
+import (
+	"go/ast"
+	"go/token"
+	"go/types"
+
+	"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"
+)
+
+// Analyzer is the map-clear-loop analysis pass.
+var Analyzer = &analysis.Analyzer{
+	Name:     "mapclearloop",
+	Doc:      "reports range-over-map loops that delete every entry and can be replaced with clear(m)",
+	URL:      "https://github.com/github/gh-aw/tree/main/pkg/linters/mapclearloop",
+	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
+	}
+
+	nodeFilter := []ast.Node{(*ast.RangeStmt)(nil)}
+
+	insp.Preorder(nodeFilter, func(n ast.Node) {
+		rangeStmt, ok := n.(*ast.RangeStmt)
+		if !ok {
+			return
+		}
+
+		pos := pass.Fset.PositionFor(rangeStmt.Pos(), false)
+		if filecheck.ShouldSkipFilename(pos.Filename, generatedFiles) {
+			return
+		}
+		if nolint.HasDirectiveForLinter(pos, noLintIndex, "mapclearloop") {
+			return
+		}
+
+		// The range expression must be a map type.
+		mapType := pass.TypesInfo.TypeOf(rangeStmt.X)
+		if mapType == nil {
+			return
+		}
+		if _, ok := mapType.Underlying().(*types.Map); !ok {
+			return
+		}
+
+		// The key variable must be present (not blank or absent).
+		keyIdent, ok := rangeStmt.Key.(*ast.Ident)
+		if !ok || keyIdent.Name == "_" {
+			return
+		}
+		keyObj := pass.TypesInfo.Defs[keyIdent]
+		if keyObj == nil {
+			keyObj = pass.TypesInfo.Uses[keyIdent]
+		}
+		if keyObj == nil {
+			return
+		}
+
+		// The value variable must be absent or blank.
+		if rangeStmt.Value != nil {
+			valueIdent, ok := rangeStmt.Value.(*ast.Ident)
+			if !ok || valueIdent.Name != "_" {
+				return
+			}
+		}
+
+		// The body must contain exactly one statement: delete(m, k).
+		if len(rangeStmt.Body.List) != 1 {
+			return
+		}
+		exprStmt, ok := rangeStmt.Body.List[0].(*ast.ExprStmt)
+		if !ok {
+			return
+		}
+		callExpr, ok := exprStmt.X.(*ast.CallExpr)
+		if !ok {
+			return
+		}
+		delIdent, ok := callExpr.Fun.(*ast.Ident)
+		if !ok || delIdent.Name != "delete" {
+			return
+		}
+		delBuiltin, ok := pass.TypesInfo.Uses[delIdent].(*types.Builtin)
+		if !ok || delBuiltin.Name() != "delete" {
+			return
+		}
+		if len(callExpr.Args) != 2 {
+			return
+		}
+
+		// First arg to delete must be the same map as the range expression.
+		if !sameObject(pass, callExpr.Args[0], rangeStmt.X) {
+			return
+		}
+
+		// Second arg to delete must be the key variable from the range.
+		delKeyIdent, ok := callExpr.Args[1].(*ast.Ident)
+		if !ok {
+			return
+		}
+		delKeyObj := pass.TypesInfo.Uses[delKeyIdent]
+		if delKeyObj == nil || delKeyObj != keyObj {
+			return
+		}
+
+		mText := astutil.NodeText(pass.Fset, rangeStmt.X)
+		if mText == "" {
+			return
+		}
+		if !builtinVisibleAtPos(pass.Pkg, rangeStmt.Pos(), "clear") {
+			return
+		}
+
+		diag := analysis.Diagnostic{
+			Pos:     rangeStmt.Pos(),
+			End:     rangeStmt.End(),
+			Message: "range-delete loop over map can be replaced with clear(" + mText + ")",
+		}
+		if !hasOverlappingComment(pass.Files, rangeStmt.Pos(), rangeStmt.End()) {
+			diag.SuggestedFixes = []analysis.SuggestedFix{{
+				Message: "Replace range-delete loop with clear",
+				TextEdits: []analysis.TextEdit{{
+					Pos:     rangeStmt.Pos(),
+					End:     rangeStmt.End(),
+					NewText: []byte("clear(" + mText + ")"),
+				}},
+			}}
+		}
+		pass.Report(diag)
+	})
+
+	return nil, nil
+}
+
+// builtinVisibleAtPos reports whether name resolves to a builtin object at pos.
+func builtinVisibleAtPos(pkg *types.Package, pos token.Pos, name string) bool {
+	if pkg == nil {
+		return false
+	}
+	scope := pkg.Scope().Innermost(pos)
+	if scope == nil {
+		return false
+	}
+	_, obj := scope.LookupParent(name, pos)
+	if obj == nil {
+		return false
+	}
+	builtin, ok := obj.(*types.Builtin)
+	return ok && builtin.Name() == name
+}
+
+// hasOverlappingComment reports whether any comment group overlaps [start, end).
+func hasOverlappingComment(files []*ast.File, start, end token.Pos) bool {
+	for _, file := range files {
+		if end <= file.Pos() || start >= file.End() {
+			continue
+		}
+		for _, group := range file.Comments {
+			if group.Pos() < end && start < group.End() {
+				return true
+			}
+		}
+	}
+	return false
+}
+
+// sameObject reports whether expr refers to the same declared object as ref.
+// ref is expected to be an *ast.Ident or *ast.SelectorExpr.
+func sameObject(pass *analysis.Pass, expr, ref ast.Expr) bool {
+	switch r := ref.(type) {
+	case *ast.Ident:
+		e, ok := expr.(*ast.Ident)
+		if !ok {
+			return false
+		}
+		return pass.TypesInfo.Uses[e] == pass.TypesInfo.Uses[r]
+	case *ast.SelectorExpr:
+		e, ok := expr.(*ast.SelectorExpr)
+		if !ok {
+			return false
+		}
+		return pass.TypesInfo.Uses[e.Sel] == pass.TypesInfo.Uses[r.Sel] &&
+			sameObject(pass, e.X, r.X)
+	default:
+		return false
+	}
+}

pkg/linters/mapclearloop/mapclearloop_test.go

@@ -0,0 +1,16 @@
+//go:build !integration
+
+package mapclearloop_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+
+	"github.com/github/gh-aw/pkg/linters/mapclearloop"
+)
+
+func TestAnalyzer(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.RunWithSuggestedFixes(t, testdata, mapclearloop.Analyzer, "mapclearloop")
+}

pkg/linters/mapclearloop/testdata/src/mapclearloop/mapclearloop.go

@@ -0,0 +1,71 @@
+package mapclearloop
+
+func bad() {
+	m := map[string]int{"a": 1, "b": 2}
+
+	for k := range m { // want `range-delete loop over map can be replaced with clear\(m\)`
+		delete(m, k)
+	}
+
+	m2 := map[int]string{1: "x"}
+	for k := range m2 { // want `range-delete loop over map can be replaced with clear\(m2\)`
+		delete(m2, k)
+	}
+
+	m3 := map[string]int{"c": 3}
+	for k, _ := range m3 { // want `range-delete loop over map can be replaced with clear\(m3\)`
+		delete(m3, k)
+	}
+
+	var k string
+	for k = range m { // want `range-delete loop over map can be replaced with clear\(m\)`
+		delete(m, k)
+	}
+	_ = k
+
+	m4 := map[string]int{"d": 4}
+	for k := range m4 { // want `range-delete loop over map can be replaced with clear\(m4\)`
+		// keep this comment in place by omitting the suggested fix
+		delete(m4, k)
+	}
+}
+
+func good() {
+	m := map[string]int{"a": 1}
+
+	// Only ranging over value – not flagged.
+	for _, v := range m {
+		_ = v
+	}
+
+	// Body has more than one statement – not flagged.
+	for k := range m {
+		delete(m, k)
+		_ = k
+	}
+
+	// Deleting into a different map – not flagged.
+	m2 := map[string]int{"b": 2}
+	for k := range m {
+		delete(m2, k)
+	}
+
+	// delete is shadowed – not flagged.
+	delete := func(_ map[string]int, _ string) {}
+	for k := range m {
+		delete(m, k)
+	}
+
+	// clear is shadowed – not flagged.
+	clear := func(_ map[string]int) {}
+	for k := range m {
+		delete(m, k)
+	}
+	clear(m)
+
+	// Ranging over a slice – not flagged.
+	s := []int{1, 2, 3}
+	for i := range s {
+		_ = i
+	}
+}