lint: enable govet (all) and a set of correctness linters#420
Open
randomizedcoder wants to merge 2 commits into
Open
lint: enable govet (all) and a set of correctness linters#420randomizedcoder wants to merge 2 commits into
randomizedcoder wants to merge 2 commits into
Conversation
gocritic's appendAssign check flags append(cert.DNSNames, cert.EmailAddresses...): appending to cert.DNSNames mutates its backing array when it has spare capacity, which could surprise a caller still holding the certificate. slices.Concat always allocates a fresh slice, which is the intent here. No functional change; slices is already imported. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: randomizedcoder dave.seddon.ca@gmail.com <dave.seddon.ca@gmail.com>
Mirrors prometheus/blackbox_exporter#1631. Enables copyloopvar, durationcheck, gocritic, makezero, unconvert, and wastedassign, plus govet enable-all (minus fieldalignment and shadow, which are noisy / idiomatic in this codebase). These are low-noise correctness checks; after the preceding appendAssign fix there are no findings on the tree. golangci-lint run ./... is clean, and go build / go test ./... pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: randomizedcoder dave.seddon.ca@gmail.com <dave.seddon.ca@gmail.com>
randomizedcoder
marked this pull request as ready for review
July 20, 2026 21:27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi 👋 — over in blackbox_exporter we recently landed prometheus/blackbox_exporter#1631 to enable a few more linters, and it occurred to us the same low-noise additions might help here too, so we thought we would try to help by proposing them for exporter-toolkit as well.
Why
The current
.golangci.ymlrunsmisspell,modernize,revive(plus the golangci-lint v2 defaults).govetruns only its default analyzer subset, so a few correctness checks (composites,unreachable,copylocks, etc.) are not enforced in CI.What
Enable
govetwithenable-all(minusfieldalignment, which is churn-heavy, andshadow, which is idiomatic and used throughout this codebase) plus a curated, low-noise correctness set:copyloopvar,durationcheck,gocritic,makezero,unconvert,wastedassign.These surfaced exactly one finding —
gocritic'sappendAssignonappend(cert.DNSNames, cert.EmailAddresses...)inVerifyPeerCertificate, where appending tocert.DNSNamescan mutate its backing array. That is fixed in the first commit withslices.Concat; after it,golangci-lint run ./...is clean andgo build/go test ./...pass.No golangci-lint version bump is needed — every linter above is available in the pinned version (
Makefile.commonuntouched).Happy to trim or extend the set to your preference. As a follow-up we would also like to propose adding
ReadHeaderTimeoutto thehttp.Serverinbootstrap/(a Slowloris hardening, matching prometheus/blackbox_exporter#1626).🤖 Generated with Claude Code