Add setboolcheck linter: flag map[T]bool used as sets

[getvictor]

Motivation: add a check for a common issue I see humans and AI agents making, so that we don't have to waste time on it in code reviews.
Resolves #42635

Note: This lint check has been mostly AI generated. I don't think it needs a thorough review because it is not production code and not even test code. Any issues will be obvious from usage by contributors.

Add a custom go/analysis analyzer that detects map[T]bool variables
used as sets (where only the literal true is ever assigned) and
suggests using map[T]struct{} instead, which is the idiomatic Go
approach for sets — zero memory for values and unambiguous semantics.

The analyzer minimizes false positives by:

• Only flagging when ALL indexed assignments use the literal true
• Skipping variables initialized from function calls (unknown source)
• Skipping variables reassigned from unknown sources
• Skipping function parameters and exported package-level variables
• Skipping range loop variables

Integrated as an incremental linter (new/changed code only) to avoid
breaking existing code.

Running this check on our whole codebase flags valid cases:

     cmd/fleet/serve.go:306:2: map[string]bool used as a set; consider map[string]struct{} instead (setboolcheck)
        allowedHostIdentifiers := map[string]bool{                                                                                                                           
        ^                                                                                                                                                                    
     cmd/fleetctl/fleetctl/generate_gitops.go:189:3: map[string]bool used as a set; consider map[string]struct{} instead (setboolcheck)                                      
                handled := make(map[string]bool, len(renames)*2)                                                                                                             
                ^                                                                                                                                                            
     cmd/fleetctl/fleetctl/generate_gitops.go:1593:2: map[uint]bool used as a set; consider map[uint]struct{} instead (setboolcheck)
        m := make(map[uint]bool, len(ids))

Summary by CodeRabbit

• Chores
  • Added a new code analyzer to detect maps used as boolean sets and recommend more efficient alternatives for better performance.

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

_{Tip: disable this comment in your organization's Code Review settings.}

[Copilot]

Pull request overview

Adds a new custom go/analysis-based linter (“setboolcheck”) and wires it into the repo’s incremental golangci-lint setup to detect map[T]bool used as set-like maps (only assigning literal true) and suggest map[T]struct{} instead.

Changes:

• Implement setboolcheck analyzer with heuristics to reduce false positives (tainting/skip rules).
• Add analysistest coverage and testdata examples for flagged vs non-flagged patterns.
• Integrate the analyzer as a golangci-lint module plugin and enable it in .golangci-incremental.yml.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tools/ci/setboolcheck/setboolcheck.go	New analyzer implementation and reporting logic
tools/ci/setboolcheck/setboolcheck_test.go	Analysistest runner for the analyzer
tools/ci/setboolcheck/testdata/src/example/example.go	Test corpus for expected diagnostics
tools/ci/setboolcheck/cmd/gclplugin/plugin.go	Golangci-lint module plugin entry point
tools/ci/setboolcheck/go.mod	New module for the plugin/analyzer
tools/ci/setboolcheck/go.sum	Module dependency checksums
.golangci-incremental.yml	Enable/configure setboolcheck for incremental linting
.custom-gcl.yml	Register setboolcheck as a golangci-lint module plugin (local path)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.67%. Comparing base (04471ce) to head (376c93b).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #42631      +/-   ##
==========================================
- Coverage   66.67%   66.67%   -0.01%     
==========================================
  Files        2535     2535              
  Lines      203320   203320              
  Branches     9115     9115              
==========================================
- Hits       135571   135554      -17     
- Misses      55484    55496      +12     
- Partials    12265    12270       +5     

Flag	Coverage Δ
backend	68.50% <ø> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[getvictor]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

Walkthrough

This pull request introduces a new custom golangci-lint plugin called setboolcheck to the Fleet project. The plugin analyzes Go code to detect instances where map[T]bool is used as a set (when only true values are stored) and suggests using map[T]struct{} instead for better memory efficiency. The change includes plugin configuration files, the analyzer implementation, a golangci-lint module plugin entry point, a Go module definition, comprehensive tests, and test fixtures demonstrating various detection scenarios and edge cases.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 51.61% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	⚠️ Warning	The pull request description is largely incomplete, missing most required sections from the template such as changes files, testing, database migrations, and other critical checklists.	Add a changes file entry, document testing approach, address each template checklist item, and clarify why non-applicable items are intentionally omitted rather than deleting them.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Add setboolcheck linter: flag map[T]bool used as sets' accurately describes the main change—introducing a new linter that detects map[T]bool used as sets.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/go-set-lint-rule-XOdT7

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tools/ci/setboolcheck/setboolcheck.go`:
- Around line 173-176: Replace plain identifier name checks with resolved
objects: instead of comparing ident.Name == "make" or "true", call
pass.TypesInfo.ObjectOf(ident) and compare that object to the universe
declarations via types.Universe.Lookup("make") and types.Universe.Lookup("true")
(and any other predeclared names) so shadowed locals don't mislead the logic;
update the checks around e.Fun / ident and the spots that set info.tainted and
preserve allTrue to use these object comparisons (use pass and
types.Universe.Lookup) rather than string matching.
- Around line 63-65: Remove the token check that only calls registerRangeVarDecl
for token.DEFINE in the ast.RangeStmt handling so registerRangeVarDecl(pass,
vars, node) runs for all range statements (i.e., drop the "if node.Tok ==
token.DEFINE" guard); this ensures assignments like "for _, v = range" mark the
reused map[K]bool variable as tainted. After changing the branch in the visitor
handling ast.RangeStmt, add a regression test alongside the existing
boolMapFromRangeValue test that exercises "for _, v = range" (reusing a
previously declared map[K]bool) to confirm the variable is treated as tainted.
- Around line 70-75: Phase 2 misses map escapes: when a tracked map identifier
is copied to another identifier or passed into a function it must be marked
tainted; update the Preorder traversal that currently visits
assignFilter/(*ast.AssignStmt)(nil) and calls checkAssignment to also detect
escapes by (1) inspecting assignment RHS identifiers in the same visit (e.g., in
the visitor for AssignStmt used with insp.Preorder) and marking the target
identifier tainted in your vars map when the RHS refers to a tracked map, and
(2) adding a visitor for ast.CallExpr that examines each argument and taints the
corresponding tracked map if an argument is an identifier referring to a tracked
map; use the existing symbols (insp.Preorder, checkAssignment, vars, pass) to
locate where to add these checks so any aliasing or passing of a tracked map
will flip it to tainted instead of leaving the original unmarked.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5f01473c-6ffa-455f-920e-f6dbe585146e

📥 Commits

Reviewing files that changed from the base of the PR and between 04471ce and b672747.

⛔ Files ignored due to path filters (1)

• tools/ci/setboolcheck/go.sum is excluded by !**/*.sum

📒 Files selected for processing (7)

• .custom-gcl.yml
• .golangci-incremental.yml
• tools/ci/setboolcheck/cmd/gclplugin/plugin.go
• tools/ci/setboolcheck/go.mod
• tools/ci/setboolcheck/setboolcheck.go
• tools/ci/setboolcheck/setboolcheck_test.go
• tools/ci/setboolcheck/testdata/src/example/example.go

[getvictor]

@sgress454 can you review this one since you've been using Claude Code quite a bit?

I'd like to start creating more guardrails for AI agents, and this is an example of one. I think whenever we see an AI agent doing something incorrectly, we should think how we can put in a deterministic guardrail for that case. Putting things in CLAUDE.md is easy and mostly works, but it is not reliable and clutters up the context.