[ci] exhaustive linter: switch + map composite-literal checking by trilamsr · Pull Request #63 · TraceCoreAI/tracecore

trilamsr · 2026-05-19T02:04:16Z

What this PR does

Wires the exhaustive linter
into .golangci.yml so a switch over a typed enum that omits cases
— OR a map composite literal keyed by a typed enum that omits keys —
fails CI. The forcing function was the Hint typed enum in
k8sevents (adding a Hint constant without updating every consumer
was previously invisible to Go vet, which doesn't check exhaustiveness
for string-typed enums).

Configuration:

default-signifies-exhaustive: true — an explicit default: clause
satisfies the linter. The goal is to catch unintentional
missing-case bugs, not to ban legitimate pattern-match-with-default.
check: [switch, map] — checks both switch statements AND map
composite literals. The map check is load-bearing for the original
Hint-drift motivation: future consumers may build map[Hint]X{...}
lookup tables rather than switches, and the same omission bug applies.

Audit + findings — 5 issues across switches and maps, all addressed:

Site	Kind	Status
`components/receivers/dcgm/metrics.go:491` switch over `EntityKind`	switch	Has `default:` — silenced by the setting
`components/receivers/dcgm/metrics.go:596` switch over `SampleType`	switch	Has `default:` with rationale — silenced by the setting
`components/receivers/dcgm/coexistence_test.go:126` switch over `FieldID`	switch	No `default:` — added explicit `default:` clause with "non-profiling-tier fields are accepted" comment
`components/receivers/dcgm/metrics.go:74` `fieldEmitters` (`map[FieldID]fieldEmitter`)	map	Intentionally a strict subset of pkg/dcgm FieldIDs per RFC-0005's v0 metric set — annotated `//nolint:exhaustive` with rationale
`components/receivers/dcgm/metrics_test.go:168` `allowedPlaceholders` (`map[dropGroup]string`)	map	Test-helper pinning the 2 dropGroups with no fieldEmitter today — annotated `//nolint:exhaustive` with rationale

Review history

Opened with a default-signifies-exhaustive: true enabling + the
3 switch findings. Single-pass adversarial review surfaced the
map-check gap and 2 intentional pre-existing partial-enum maps in
dcgm. Validation cycle for each finding recorded in commit 6407a2f.

Deferred to FOLLOWUPS (one finding):

The choice of default-signifies-exhaustive: true is laxer than
strict mode. Strict mode would force every defaulted switch to
carry //exhaustive:ignore with a rationale. The two metrics.go
switches legitimately want pattern-match-with-fallback semantics,
so strict mode would be ceremony today. Trigger: an
operator-visible bug traced back to a defaulted switch that should
have enumerated the missing case. Cheapest fix when triggered:
flip to strict + annotate the two metrics.go switches.

Closes the exhaustive linter wiring row in FOLLOWUPS (k8sevents
post-merge section).

Linked issue(s)

No linked issue.

Release notes

NONE

Checklist

make lint exit 0 with exhaustive + check: [switch, map] enabled
make ci exit 0 end-to-end
TDD: mutation fixture (10/11 Hint keys missing) caught only with new config; removed after
Commits are signed off

Test plan

Local make ci green
CI green on this PR
After merge: a future PR that adds a Hint (or any typed-enum) constant without updating its switch or map sites fails CI

🤖 Generated with Claude Code

Wires the `exhaustive` linter (https://github.com/nishanths/exhaustive) into `.golangci.yml` so a switch over a typed enum that omits cases fails CI. The forcing function for this work was the Hint typed enum in k8sevents — adding a Hint constant without updating every Hint switch would previously be invisible to Go vet (which doesn't check exhaustiveness for `string`-typed enums). Configuration: `default-signifies-exhaustive: true`. The goal is to catch *unintentional* missing-case bugs (e.g., new Hint constant added but a switch site forgotten), not to ban legitimate pattern-match-with-default. An explicit `default:` clause signals the author has considered the non-enumerated path. Audit + findings: Dry-run surfaced 3 issues across the repo: 1. `components/receivers/dcgm/metrics.go:491` switch over `EntityKind` — has a `default:` already. Silenced by the new setting. 2. `components/receivers/dcgm/metrics.go:596` switch over `SampleType` — has a `default:` with rationale comment ("Strings and blobs don't map to numeric data points"). Silenced by the setting. 3. `components/receivers/dcgm/coexistence_test.go:126` test-helper switch over `FieldID` — no `default:`. The switch is a pattern-match for 4 profiling-tier fields; non-matching fields are meant to continue the for-loop. Added an explicit `default:` clause with a "non-profiling-tier fields are accepted" comment that documents the intent and satisfies the linter. Verification: - `make lint` exit 0 with exhaustive enabled - `make ci` exit 0 end-to-end (kernelevents test had one transient flake on first run; passed on retry — same intermittent test seen during PRs #55 and #60) Closes the `exhaustive` linter wiring row in FOLLOWUPS. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Tri Lam <trilamsr@gmail.com>

Single-pass adversarial review of PR #63 surfaced three real findings, two of which applied. Validation cycle below. F3 (applied) — Stricter check scope: Reviewer flagged that `exhaustive` defaults to `check: [switch]` only, so map composite literals keyed by typed enums (e.g. `map[Hint]X{HintFoo: ...}`) aren't validated. Enabled `check: [switch, map]` and verified the change is load-bearing: Mutation test: introduced a `map[Hint]string{HintPodEvicted: ...}` fixture (10 of 11 Hint constants missing). With the old config: `make lint` exit 0. With `check: [switch, map]`: linter reports `missing keys in map of key type k8sevents.Hint` listing all 10. Removed fixture; re-ran clean. The stricter check also surfaced two pre-existing intentional non-exhaustive map literals in dcgm: - `fieldEmitters` (metrics.go:74) — `map[FieldID]fieldEmitter` pinning the v0 metric set, deliberately a subset of pkg/dcgm's full FieldID enum per RFC-0005. - `allowedPlaceholders` (metrics_test.go:168) — a test-helper map pinning the 2 dropGroups with no fieldEmitter today. Both received `//nolint:exhaustive` with rationale comments so the intentional partiality is documented at the call site. F2 (deferred) — `default-signifies-exhaustive: true` is laxer than strict mode. Today the 2 metrics.go pattern-match-with-fallback switches over EntityKind/SampleType legitimately want defaults, and strict mode would force `//exhaustive:ignore` annotations on each. Deferred to FOLLOWUPS with "revisit if a defaulted switch hides a real missed-enum bug" trigger. F1, F4, F5 (skipped) — see validation cycle notes: F1: pre-PR coexistence_test switch was already non-exhaustive (4 of ~20 FieldIDs, no default). Adding `default:` makes the existing fall-through behavior explicit; doesn't loosen. F4: requested lint_smoke_test would be testing whether `make ci` runs golangci-lint, which is meta-testing. Per feedback_anti_bureaucracy, declined. F5: reviewer self-resolved (NIT, SA9003 doesn't apply to switch). Verification: - TDD mutation: confirmed gate fires before fix, passes after. - `make lint` exit 0 with new annotations in place. - `make ci` exit 0 end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Tri Lam <trilamsr@gmail.com>

## What this PR does Follow-up to #64 (zizmor security-lint gate). Trims comments that didn't pass the six-months-cold-reader test. The commit was prepared on the #64 branch but pushed seconds after that PR auto-merged, so it never made it in — re-applying as a standalone follow-up. Comments trimmed: - `scripts/zizmor.sh`: dropped per-flag descriptions in the header block — `--no-progress` and `--config` don't need explainers; the WHY for `--min-severity=high` stays. Also fixed a stale error- message prefix copied from register-lint. - `.github/zizmor.yml`: 6 lines → 3. Kept the one load-bearing claim ("ignores live inline"). - `release.yml` cache-poisoning ignores: 6 lines → 2. Kept the WHY (cache keyed on go.sum, trust root M3 already validates); dropped the meta-pointer to zizmor.yml (which no longer carries the rationale) and the audit-confidence parens. - `release.yml` INPUT_TAG env comment: 4 lines → 2. No behavior change. `make ci` exit 0 (one transient kernelevents flake on first run, passed on retry — same intermittent test seen in this session's PRs #55, #60, #62, #63, #64). ## Linked issue(s) _No linked issue._ Follows up #64. ## Release notes ```release-notes NONE ``` ## Checklist - [x] `make ci` exit 0 - [x] No behavior change; pure documentation hygiene - [x] Commits are signed off 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Tri Lam <trilamsr@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

#68) ## What this PR does Two small fixes that turn this session's lessons into code instead of notes-to-self: ### 1. Deflake `TestJournaldSource_StreamsMockOutput` The 2s deadline for the mock-journalctl test was tight on loaded macOS / CI runners — subprocess spawn + fixture read + goroutine schedule could miss it. The test flaked on 5 of 5 PRs in this session that exercised the full suite (#55, #60, #62, #63, #64). Bumped the deadline 2s → 5s. The non-flake budget for "no record ever emitted" regressions stays well under the deadline; 5s just absorbs runner jitter without hiding real bugs. ### 2. Warn locally when `shellcheck` isn't on PATH `actionlint` silently skips run-block shellcheck when shellcheck isn't on PATH (macOS doesn't ship shellcheck by default). PR #62 passed `make actionlint` locally and then surfaced 4 shellcheck findings in CI — exactly the failure mode the gate is supposed to prevent. `make actionlint` now prints a `WARNING: shellcheck not on PATH; actionlint will skip run-block shellcheck.` line with `brew install` / `apt-get install` hints when the binary is missing. The underlying lint still runs; the warning just makes the gap visible to the macOS dev. ## Linked issue(s) _No linked issue._ ## Release notes ```release-notes NONE ``` ## Checklist - [x] `make ci` exit 0 (test passes deterministically with new deadline) - [x] `make actionlint` shows the warning when shellcheck is uninstalled (verified locally) - [x] No behavior change in CI (shellcheck already on Linux runners) - [x] Commits are signed off 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Tri Lam <trilamsr@gmail.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

trilamsr and others added 2 commits May 18, 2026 19:03

trilamsr changed the title ~~[ci] exhaustive linter: enable with default-signifies-exhaustive~~ [ci] exhaustive linter: switch + map composite-literal checking May 19, 2026

trilamsr merged commit dc8467c into main May 19, 2026
8 checks passed

trilamsr deleted the worktree-exhaustive-linter branch May 19, 2026 02:29

This was referenced May 19, 2026

[ci] zizmor: trim superfluous comments (#64 follow-up) #66

Merged

[ci] deflake journald mock test + warn when shellcheck missing locally #68

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ci] exhaustive linter: switch + map composite-literal checking#63

[ci] exhaustive linter: switch + map composite-literal checking#63
trilamsr merged 2 commits into
mainfrom
worktree-exhaustive-linter

trilamsr commented May 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

trilamsr commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Review history

Linked issue(s)

Release notes

Checklist

Test plan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

trilamsr commented May 19, 2026 •

edited

Loading