Overview
seenmapbool (pkg/linters/seenmapbool/seenmapbool.go) under-detects map[string]bool-as-set variables when they are declared via := from anything other than a literal make(map[string]bool, ...) call or a map[string]bool{...} composite literal. The additional AST-shape check is redundant with (and more restrictive than) the type-system check already being performed, so it silently drops legitimate candidates.
Evidence
In collectSeenMapCandidates (pkg/linters/seenmapbool/seenmapbool.go:99-118), the := branch gates candidacy on both:
if isMapStringBool(pass.TypesInfo.TypeOf(ident)) && isMapStringBoolExpr(stmt.Rhs[i]) {
candidates[obj] = ident
}isMapStringBool (seenmapbool.go:183-197) already resolves the declared type of the identifier via pass.TypesInfo — this is authoritative and alias-safe (a map[string]bool typed variable is a map[string]bool typed variable no matter how the RHS is spelled). isMapStringBoolExpr (seenmapbool.go:199-216) then re-derives the same fact from the AST shape of the RHS, accepting only make(map[string]bool, ...) calls and map[string]bool{...} composite literals.
This second check is not just redundant — it actively narrows detection. Any := declaration whose RHS is a map[string]bool-typed value that isn't a literal/make() is silently excluded from candidates, even though it is exactly the same runtime type and can be used as a pure true-sentinel set:
func dedupe(items []string) []string {
seen := getSeenMap() // returns map[string]bool — NOT reported
var out []string
for _, it := range items {
if !seen[it] {
seen[it] = true
out = append(out, it)
}
}
return out
}
func copyPattern(other map[string]bool) {
seen := other // plain identifier copy — NOT reported
seen["x"] = true
}Both are indistinguishable from the BadSetBool/BadSetBoolLiteral cases already covered by testdata/src/seenmapbool/seenmapbool.go:3-20, except for the RHS syntax — and neither is exercised by the existing test file, confirming the gap is untested as well as unfixed.
By contrast, the var / DeclStmt branch (seenmapbool.go:119-141) does not apply this extra RHS-shape filter — it accepts any var seen map[string]bool purely on declared type. The := and var branches are inconsistent with each other for no principled reason; the var branch's simpler, type-only approach is the correct one.
Impact
False negatives: legitimate map[string]bool-as-set patterns assigned via a helper function, a copied variable, a type-asserted value, or any other non-literal expression silently escape detection, undermining the linter's stated purpose (catching all avoidable bool-per-entry allocations). No production sites were found with a targeted grep for ) map[string]bool return signatures in pkg/ (only one hit, pkg/cli/runner_guard_activation_gate.go:110, whose result is consumed only in tests) — so this is currently a latent correctness/coverage gap rather than one with an open prod false negative today, but it will silently miss any future case introduced this way.
Recommendation
Drop the isMapStringBoolExpr(stmt.Rhs[i]) conjunct in the := branch and rely solely on isMapStringBool(pass.TypesInfo.TypeOf(ident)), matching the already-correct var/DeclStmt branch. isMapStringBoolExpr and isMapStringBoolTypeExpr then become dead code and can be deleted.
Before / after intent
- Before:
seen := getSeenMap() (RHS not a literal/make) → no diagnostic, even when later used as a pure set.
- After: same type-only gate as the
var branch → candidate collected, and (as today) excluded again by findNonSetMaps if any non-true value is ever assigned.
Validation checklist
Effort
Small — a one-line gate simplification plus two new testdata cases; no SuggestedFix/autofix involved (this linter is diagnostic-only).
Generated by 🤖 Sergo - Serena Go Expert · agent · 190.6 AIC · ⌖ 32.2 AIC · ⊞ 6K · ◷
Overview
seenmapbool(pkg/linters/seenmapbool/seenmapbool.go) under-detectsmap[string]bool-as-set variables when they are declared via:=from anything other than a literalmake(map[string]bool, ...)call or amap[string]bool{...}composite literal. The additional AST-shape check is redundant with (and more restrictive than) the type-system check already being performed, so it silently drops legitimate candidates.Evidence
In
collectSeenMapCandidates(pkg/linters/seenmapbool/seenmapbool.go:99-118), the:=branch gates candidacy on both:isMapStringBool(seenmapbool.go:183-197) already resolves the declared type of the identifier viapass.TypesInfo— this is authoritative and alias-safe (amap[string]booltyped variable is amap[string]booltyped variable no matter how the RHS is spelled).isMapStringBoolExpr(seenmapbool.go:199-216) then re-derives the same fact from the AST shape of the RHS, accepting onlymake(map[string]bool, ...)calls andmap[string]bool{...}composite literals.This second check is not just redundant — it actively narrows detection. Any
:=declaration whose RHS is amap[string]bool-typed value that isn't a literal/make()is silently excluded fromcandidates, even though it is exactly the same runtime type and can be used as a puretrue-sentinel set:Both are indistinguishable from the
BadSetBool/BadSetBoolLiteralcases already covered bytestdata/src/seenmapbool/seenmapbool.go:3-20, except for the RHS syntax — and neither is exercised by the existing test file, confirming the gap is untested as well as unfixed.By contrast, the
var/DeclStmtbranch (seenmapbool.go:119-141) does not apply this extra RHS-shape filter — it accepts anyvar seen map[string]boolpurely on declared type. The:=andvarbranches are inconsistent with each other for no principled reason; thevarbranch's simpler, type-only approach is the correct one.Impact
False negatives: legitimate
map[string]bool-as-set patterns assigned via a helper function, a copied variable, a type-asserted value, or any other non-literal expression silently escape detection, undermining the linter's stated purpose (catching all avoidable bool-per-entry allocations). No production sites were found with a targeted grep for) map[string]boolreturn signatures inpkg/(only one hit,pkg/cli/runner_guard_activation_gate.go:110, whose result is consumed only in tests) — so this is currently a latent correctness/coverage gap rather than one with an open prod false negative today, but it will silently miss any future case introduced this way.Recommendation
Drop the
isMapStringBoolExpr(stmt.Rhs[i])conjunct in the:=branch and rely solely onisMapStringBool(pass.TypesInfo.TypeOf(ident)), matching the already-correctvar/DeclStmtbranch.isMapStringBoolExprandisMapStringBoolTypeExprthen become dead code and can be deleted.Before / after intent
seen := getSeenMap()(RHS not a literal/make) → no diagnostic, even when later used as a pure set.varbranch → candidate collected, and (as today) excluded again byfindNonSetMapsif any non-truevalue is ever assigned.Validation checklist
seen := getSeenMap()(helper returningmap[string]bool, used as pure set) → expect diagnostic.seen := other(identifier copy of amap[string]boolparam/var), used as pure set → expect diagnostic.BadSetBool/BadSetBoolLiteral/GoodBoolMapWithFalse/closure cases intestdata/src/seenmapbool/seenmapbool.gostill pass unchanged.isMapStringBoolExpr/isMapStringBoolTypeExprhelpers (or keep only if still referenced elsewhere).Effort
Small — a one-line gate simplification plus two new testdata cases; no SuggestedFix/autofix involved (this linter is diagnostic-only).