seenmapbool: duplicate diagnostics for set-maps declared inside function literals (double AST traversal)

[github-actions]

Summary

seenmapbool reports the same map[string]bool declaration two or more times when that declaration lives inside a function literal (closure). The linter's node filter visits both *ast.FuncDecl and *ast.FuncLit, while inspectBody independently recurses into nested closures via ast.Inspect — so a set-map declared in a closure is collected and reported once by the enclosing function's pass and again by the closure's own pass.

Location

pkg/linters/seenmapbool/seenmapbool.go

• run node filter: lines 34-37 register both (*ast.FuncDecl)(nil) and (*ast.FuncLit)(nil).
• inspectBody first pass: ast.Inspect(body, ...) at line 70 descends into nested *ast.FuncLit bodies (the closure returns true/keeps walking), so it collects candidate maps declared inside nested closures.
• Report site: lines 161-173 pass.ReportRangef(declNode, ...).

Mechanism

For a set-map declared in a closure nested in a function, insp.Preorder invokes the callback once for the enclosing FuncDecl and once for the nested FuncLit:

1. FuncDecl visit → inspectBody(funcDecl.Body) → its ast.Inspect walks the whole body, descending into the closure, collects seen as a candidate, and reports it.
2. FuncLit visit → inspectBody(funcLit.Body) → collects seen again and reports it a second time at the identical position.

A closure nested two levels deep is reported three times, etc. go/analysis does not de-duplicate diagnostics by position, so the user sees the warning N times.

Reproduction

package p

func Dedup(xs []string) []string {
	unique := func(in []string) []string {
		seen := make(map[string]bool) // reported TWICE
		var out []string
		for _, x := range in {
			if !seen[x] {
				seen[x] = true
				out = append(out, x)
			}
		}
		return out
	}
	return unique(xs)
}

seenmapbool emits the map[string]bool "seen" used as a set diagnostic twice at the seen := line.

Contrast with the correct sibling

pkg/linters/largefunc/largefunc.go uses the same {FuncDecl, FuncLit} node filter but does not recurse — each Preorder visit measures only that node's own Lbrace..Rbrace span (lines 70-73). Because it never re-walks nested closures, every function/closure is reported exactly once. seenmapbool is the odd one out: it recurses and gets re-visited.

Test coverage gap

pkg/linters/seenmapbool/testdata/src/seenmapbool/seenmapbool.go only exercises set-maps in top-level FuncDecls (BadSetBool, BadSetBoolLiteral) — no closure case — so analysistest (which counts // want matches) never observes the duplicate. A closure case must be added to catch this and any regression.

Impact

• Duplicate / noisy diagnostics whenever a seen-style set-map lives in a closure (a very common Go idiom for local dedup helpers).
• A correctness blocker for any future CI enforcement that counts violations: a single offending closure inflates the count by 2-3x.

Recommended fix (directional)

Process each map declaration in exactly one scope. Either:

• Make inspectBody's traversals stop at nested *ast.FuncLit boundaries (return false for nested FuncLit nodes) so each closure is handled solely by its own Preorder visit — but keep the write-disqualification pass able to observe a non-true write that happens inside a nested closure (else a closure write of false would be missed, turning the double-report into a false positive); or
• Keep the recursive single pass rooted only at the nearest enclosing function and de-duplicate reports by declaration position.

Add a closure-scoped testdata case (set-map inside a func() {...}) asserting a single diagnostic.

Effort

Small — node-filter/recursion scoping change plus one testdata function. Behavior is fully covered by analysistest.

Generated by 🤖 Sergo - Serena Go Expert · 357.9 AIC · ⌖ 10.9 AIC · ⊞ 5.8K · ◷

• expires on Jun 28, 2026, 9:42 PM UTC-08:00

[github-actions]

This issue was automatically closed because it expired on 2026-06-29T05:42:43.665Z.

Closed by Workflow