fix(seenmapbool): treat non-true values in declaration-site composite literals as non-set maps - #51514
Conversation
…es as non-set maps Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Great work on this fix! 🎯 This PR correctly addresses the issue where the seenmapbool linter was misflagging map[string]bool composite literals with mixed boolean values. The changes are focused, well-tested with both := and var forms covered, and the description clearly explains the bug and solution. The implementation properly mirrors existing false-detection logic at the initialization site, and test coverage ensures both good and bad cases are caught. Ready for review!
|
PR Triage
Small draft lint-tool fix, low urgency, still in progress.
|
There was a problem hiding this comment.
Pull request overview
Fixes seenmapbool false positives for maps initialized with meaningful false values.
Changes:
- Inspects
varand assignment initializers. - Marks mixed-value composite literals as non-set maps.
- Adds regression cases for
:=andvardeclarations.
Show a summary per file
| File | Description |
|---|---|
pkg/linters/seenmapbool/seenmapbool.go |
Adds declaration-site literal analysis. |
pkg/linters/seenmapbool/testdata/src/seenmapbool/seenmapbool.go |
Adds mixed-value literal test cases. |
Review details
Tip
Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
| lit, ok := value.(*ast.CompositeLit) | ||
| if !ok { | ||
| return | ||
| } |
| if i < len(valSpec.Values) { | ||
| markIfNonSetLiteral(pass, name, valSpec.Values[i], candidates, nonSetMaps) |
|
🎉 This pull request is included in a new release. Release: |
seenmapbooldecided whether amap[string]boolwas a puretrue-sentinel set by scanning only for laterm[k] = vstatements, never inspecting the composite literal supplied at the declaration. A map populated in one shot with mixed values was therefore misflagged, suggesting amap[string]struct{}conversion that would drop realfalseentries.Changes
findNonSetMapsnow also visits initializers, not justIndexExprassignments:*ast.ValueSpecvalues, covering thevar flags = map[string]bool{...}branch.*ast.AssignStmtRHS when the LHS is a plain identifier, covering the:=branch.markIfNonSetLiteralhelper — for a candidate whose initializer is an*ast.CompositeLit, marks it non-set if anyKeyValueExpr.ValuefailsisBoolTrue, mirroring how a laterm[k] = falseis handled.make(...), calls, map copies) return early, so existing candidate collection and diagnostics are untouched.Tests
Added two testdata cases expecting no diagnostic:
GoodBoolMapLiteralWithFalse(:=form) andGoodBoolMapVarLiteralWithFalse(varform). ExistingBadSetBool*,GoodBoolMapWithFalse, closure, and//nolintcases are unchanged.