Repository navigation
ci: upgrade golangci-lint to 2.13.1, rework lint config - #59
Merged
Merged
Conversation
|
ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Migrate `gofumpt.extra-rules` to the 2.13 `extra` block -- it is a split, not a rename, so each sub-rule is named explicitly. Drop the linters now covered by others (gci/gofmt by gofumpt+goimports, copyloopvar/intrange by modernize, whitespace by wsl_v5) plus dupword and goheader, whose value does not carry for this repo. Lift both issue caps and turn off uniq-by-line so a CI run reports every finding instead of one per line. Add gomodguard_v2 with a deprecated-module blocklist and local-replace-directives, which catches a `replace => ../local` that would break the tagged goreleaser build. Azure track 1 goes in that list rather than in depguard: depguard matches raw string prefixes, so a rule naming the module root would also block the track 2 modules under sdk/, and its allow list cannot carve them back out. Extend govet with the non-default analyzers (deepequalerrors, nilness, reflectvaluecompare, sortslice, unusedwrite); `inline` is left off the list because it is enabled by default. Sync the pinned version in AGENTS.md -- 2.12.2 rejects this config outright -- and document the whole-files + --new-from-rev ratchet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
min0625
force-pushed
the
ci/golangci-lint-2.13.1
branch
from
August 28, 2026 15:46
585831d to
5cfd14c
Compare
min0625
added a commit
that referenced
this pull request
Sep 1, 2026
Re-add gci. #59 dropped it as covered by gofumpt+goimports, but those two only sort within the groups a file already has: a third-party import stranded in the stdlib block is moved out into a group of its own rather than merged into the existing third-party block. gci is what collapses it back to the canonical two groups. Replace gomodguard_v2's local-replace-directives with the gomoddirectives linter. gomodguard only sees directly imported modules, so an indirect dep's replace slipped through; gomoddirectives forbids every replace by default -- checked against a resolvable non-local one, not just a `=> ../local`. Retire depguard. Its one rule (github.com/pkg/errors) moves into the gomodguard_v2 blocklist so the recommendation text sits next to the other deprecated modules. mitchellh/mapstructure and gopkg.in/yaml (prefix match, so both majors) join it. Enable bidichk, makezero and reassign. reassign gets patterns: ".*", which is upstream's own recommendation -- the default only guards EOF and Err*, leaving os.Args and http.DefaultClient open. Tests are excluded from it: they borrow os.Stdin/os.Args and restore them afterwards. Turn on errcheck.check-type-assertions and prealloc.for-loops (modernize rewrites 3-clause loops into the range form prealloc polices, so with the default the finding only surfaces after --fix), and turn off perfsprint.concat-loop, which modernize rewrites better by reusing the variable instead of inventing one. `make fix` now runs tidy -> --fix -> tidy -> lint. --fix is not a fixpoint: its edits can trip a linter the fixing run never saw, and can add or remove imports. `golangci-lint run ./...` reports 0 issues repo-wide under this config, so the whole-files ratchet takes on no new debt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
min0625
added a commit
that referenced
this pull request
Sep 1, 2026
Re-add gci. #59 dropped it as covered by gofumpt+goimports, but those two only sort within the groups a file already has: a third-party import stranded in the stdlib block is moved out into a group of its own rather than merged into the existing third-party block. gci is what collapses it back to the canonical two groups. Replace gomodguard_v2's local-replace-directives with the gomoddirectives linter. gomodguard only sees directly imported modules, so an indirect dep's replace slipped through; gomoddirectives forbids every replace by default -- checked against a resolvable non-local one, not just a `=> ../local`. Retire depguard. Its one rule (github.com/pkg/errors) moves into the gomodguard_v2 blocklist so the recommendation text sits next to the other deprecated modules. mitchellh/mapstructure and gopkg.in/yaml (prefix match, so both majors) join it. Enable bidichk, makezero and reassign. reassign gets patterns: ".*", which is upstream's own recommendation -- the default only guards EOF and Err*, leaving os.Args and http.DefaultClient open. Tests are excluded from it: they borrow os.Stdin/os.Args and restore them afterwards. Turn on errcheck.check-type-assertions and prealloc.for-loops (modernize rewrites 3-clause loops into the range form prealloc polices, so with the default the finding only surfaces after --fix), and turn off perfsprint.concat-loop, which modernize rewrites better by reusing the variable instead of inventing one. Exclude gosec's G104. It fires on bare call statements only, never on defer/go, and honors a whitelist -- a strict subset of errcheck, which also names the offending function; with uniq-by-line: false both would print on the same line. Narrowing errcheck (exclude-functions, std-error-handling) would reopen the gap. Correct the check-rev rationale in the Makefile and AGENTS.md. #58 documented golangci-lint as only warning and exiting 0 on an unresolvable --new-from-rev, which would let CI pass having linted nothing. It does not: it warns, reports every issue in the repo, and exits 1. The guard stays, but it buys a readable error message, not safety. `make fix` now runs tidy -> --fix -> tidy -> lint. --fix is not a fixpoint: its edits can trip a linter the fixing run never saw, and can add or remove imports. The new ordering was exercised on a clean tree, so it confirms the plumbing runs, not that the second pass caught a real fixpoint miss. Also document in AGENTS.md that .golangci.yaml is not self-contained -- whole-files: true is silently inert without one of the --new* modes, so the file only works alongside the Makefile's --new-from-rev harness -- and that formatter findings are the exception to the ratchet: apply `golangci-lint fmt ./...` repo-wide rather than letting a half-formatted tree contradict its own config. `golangci-lint run ./...` reports 0 issues repo-wide under this config, so the whole-files ratchet takes on no new debt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
min0625
added a commit
that referenced
this pull request
Sep 1, 2026
Re-add gci. #59 dropped it as covered by gofumpt+goimports, but those two only sort within the groups a file already has: a third-party import stranded in the stdlib block is moved out into a group of its own rather than merged into the existing third-party block. gci is what collapses it back to the canonical two groups. Replace gomodguard_v2's local-replace-directives with the gomoddirectives linter. gomodguard only sees directly imported modules, so an indirect dep's replace slipped through; gomoddirectives forbids every replace by default -- checked against a resolvable non-local one, not just a `=> ../local`. Retire depguard. Its one rule (github.com/pkg/errors) moves into the gomodguard_v2 blocklist so the recommendation text sits next to the other deprecated modules. mitchellh/mapstructure and gopkg.in/yaml (prefix match, so both majors) join it. Enable bidichk, makezero and reassign. reassign gets patterns: ".*", which is upstream's own recommendation -- the default only guards EOF and Err*, leaving os.Args and http.DefaultClient open. Tests are excluded from it: they borrow os.Stdin/os.Args and restore them afterwards. Turn on errcheck.check-type-assertions and prealloc.for-loops (modernize rewrites 3-clause loops into the range form prealloc polices, so with the default the finding only surfaces after --fix), and turn off perfsprint.concat-loop, which modernize rewrites better by reusing the variable instead of inventing one. Exclude gosec's G104. It fires on bare call statements only, never on defer/go, and honors a whitelist -- a strict subset of errcheck, which also names the offending function; with uniq-by-line: false both would print on the same line. Narrowing errcheck (exclude-functions, std-error-handling) would reopen the gap. Correct the check-rev rationale in the Makefile and AGENTS.md. #58 documented golangci-lint as only warning and exiting 0 on an unresolvable --new-from-rev, which would let CI pass having linted nothing. It does not: it warns, reports every issue in the repo, and exits 1. The guard stays, but it buys a readable error message, not safety. `make fix` now runs tidy -> --fix -> tidy -> lint. --fix is not a fixpoint: its edits can trip a linter the fixing run never saw, and can add or remove imports. The new ordering was exercised on a clean tree, so it confirms the plumbing runs, not that the second pass caught a real fixpoint miss. Also document in AGENTS.md that .golangci.yaml is not self-contained -- whole-files: true is silently inert without one of the --new* modes, so the file only works alongside the Makefile's --new-from-rev harness -- and that formatter findings are the exception to the ratchet: apply `golangci-lint fmt ./...` repo-wide rather than letting a half-formatted tree contradict its own config. `golangci-lint run ./...` reports 0 issues repo-wide under this config, so the whole-files ratchet takes on no new debt. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.
Bumps golangci-lint
2.12.2→2.13.1and rewrites.golangci.yamlso every entry carries the reason it is there.2.13 breaking / behavior changes
gofumpt.extra-rulesis deprecated.extrais a split, not a rename, and the deprecation warning only namesgroup-params— taking it at face value would have silently droppedclothe-returns, the only thing here that clothes naked returns (nonamedreturnsis not enabled). Both are now named explicitly.balance-calls(new in 2.13) stays off: never previously enforced.dupwordnow scans string literals. The stub LLM inmain_test.goanswers every chunk withOK, so a multi-chunk expectation is literally"OK\n\nOK\n"— 3 findings. Handled withdupword.ignore: [OK]rather than excludingdupwordfrom_test.gowholesale, which would also stop it catching real duplicated words in test comments.This is a 2.13 behavior change, not fallout from the config rewrite — verified by running the new config under 2.12.2:
0 issues.Removed as no-ops or duplicates
severity.default: errorerrorwhen nothing is configured.run.timeout: 5mtimeout-minuteson the job is the outer guard.gofmt,gcigoimportsstays — it alone drops unused imports.copyloopvar,intrangemodernize(forvar,rangeint). The gofmtinterface{}→anyrewrite rule is likewise replaced by modernize'sany.whitespacewsl_v5reports the same leading-/trailing-whitespace-in-a-block cases (verified against a probe file); gofumpt rewrites them too.Tightened
max-same-issues: 0,max-issues-per-linter: 0,uniq-by-line: false— CI is a repo-wide gate, so a truncated report is a silently missed violation, and a dropped issue never gets auto-fixed either.goheadergets the template the repo already uses, with the year as a regexp (20\d{2}) so files keep the year they were written with instead of churning every January.nolintlintnow requires a specific linter and an explanation — a bare//nolintdisables everything on that line forever.depguard/gomodguard_v2split by what each can actually express (import paths vs. whole modules + version constraints + localreplacedirectives), so no import is reported twice.gomodguard_v2.local-replace-directivescatches areplace => ../localthat would break the tagged goreleaser build.mise.toml: version pin written bare (2.13.1) to matchgo/goreleaser/prek.Verification
golangci-lint config verify— clean.golangci-lint run ./...(full repo, not just new-from-rev) —0 issues.make check NEW_FROM_REV=origin/main— full prek suite passes.🤖 Generated with Claude Code