From a22b8120934d7ba334a5b1a2646a860a4e4ddfc1 Mon Sep 17 00:00:00 2001 From: Gabriel Rosales Date: Fri, 7 Aug 2026 21:34:44 -0600 Subject: [PATCH 1/4] chore(lint): enable revive defaults instead of re-declaring 23 rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Declaring linters.settings.revive.rules REPLACES revive's default set, so the 23 default rule names were re-listed ahead of the uber-go additions to avoid silently dropping enforcement. golangci-lint v2.12.2 accepts enable-default-rules, which restores those defaults without the re-declaration and removes the failure mode the re-declaration created: deleting one of the 23 lines loses a rule with no signal. Verified behavior-preserving rather than assumed: - golangci-lint v2.12.2's internal defaultRules list is a verbatim copy of revive v1.15.0's, and enable-default-rules feeds that same list through normalizeConfig, so the two spellings cannot diverge at this pin. - GL_DEBUG=revive dumps the resolved lint.Config. The dumps for the two spellings differ in exactly one field — EnableDefaultRules itself: same 40 rules, each with identical Arguments, Severity, Disabled and Exclude, plus Confidence 0.8 and Severity "warning". - A throwaway probe package planting a deliberate violation of each of the 23 defaults produced an identical finding set under both configs (22 rules fired; time-naming emits nothing under golangci-lint at this pin for any construct tried, and staticcheck ST1011 covers that class). - Repo-wide runs matched exactly, issue for issue: 0/0 with the real config, 166/166 with the exclusion presets stripped (88 revive: 68 exported + 20 package-comments), 0/0 with the two var-naming exclusion rules stripped. - No run logged level=error or "cannot find rule". Pre-existing drift noticed while checking the exclusions and left alone: the two `text: "var-naming: avoid package names"` exclusion rules match nothing at revive v1.15.0, because that package-name check moved out of var-naming into the (unenabled) package-naming rule and was reworded. The findings they suppressed are no longer produced at all, under either spelling. --- .golangci.yml | 36 ++++++++---------------------------- 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 01b49766..163bc7a1 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -114,35 +114,15 @@ linters: string-format: false revive: confidence: 0 - # Declaring `rules` REPLACES revive's default set rather than extending it, - # so every rule golangci-lint enables by default is re-listed below before - # the additions. An unknown rule name only logs `level=error` and still - # exits 0, so a typo here silently disables a rule — keep names exact. + # Declaring `rules` REPLACES revive's default set rather than extending it. + # `enable-default-rules` restores the defaults, so only the additions need + # listing. This is behavior-identical to re-declaring the 23 default names: + # golangci-lint's own default list is a verbatim copy of revive's, and both + # spellings resolve to the same 40-rule set. An unknown rule name only logs + # `level=error` and still exits 0, so a typo below silently disables a rule — + # keep names exact. + enable-default-rules: true rules: - # Default set — re-declared to preserve current behavior. - - name: blank-imports - - name: context-as-argument - - name: context-keys-type - - name: dot-imports - - name: empty-block - - name: error-naming - - name: error-return - - name: error-strings - - name: errorf - - name: exported - - name: increment-decrement - - name: indent-error-flow - - name: package-comments - - name: range - - name: receiver-naming - - name: redefines-builtin-id - - name: superfluous-else - - name: time-naming - - name: unexported-return - - name: unreachable-code - - name: unused-parameter - - name: var-declaration - - name: var-naming # Additions from the uber-go style guide. Each measured 0 findings # against the tree, so they ratchet current behavior without a sweep. - name: atomic # non-atomic assignment to an atomic value From bd7baa0b18a9123cfe400bd9658f91e67bdb8ce3 Mon Sep 17 00:00:00 2001 From: Gabriel Rosales Date: Fri, 7 Aug 2026 21:42:20 -0600 Subject: [PATCH 2/4] chore(lint): drop two dead var-naming exclusions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both `text: "var-naming: avoid package names"` stanzas — for (cache|database|observability)/testing/ and trace/ — stopped matching at revive v1.15.0, when the package-name check moved out into a separate `package-naming` rule (not in the default set, not enabled here) and every message was reworded. That text exists only in v1.14.0. They were suppressing nothing: deleting them leaves the repo at 0 findings under `--uniq-by-line=false`, exactly as before. This is not a behavior change, it is removing config that had silently stopped applying. Found while proving the enable-default-rules swap in the previous commit: the exclusions match on message text, so they had to be re-verified rather than assumed, and re-verifying showed they were already inert. wiki/linting.md is updated for both commits. The revive section now presents enable-default-rules as what this repo does rather than as an alternative for new configs, and the exclusions section carries the transferable lesson: an exclusion keyed on message `text` dies silently when the linter rewords the message, in either direction, so prefer scoping by `path` and `linters`. Co-Authored-By: Claude Opus 5 (1M context) --- .golangci.yml | 10 ---------- wiki/linting.md | 24 +++++++++++++++++------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 163bc7a1..f94657a6 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -179,16 +179,6 @@ linters: - linters: - zerologlint path: logger/adapter\.go - # Intentional "testing" subpackages for test utilities (imported with aliases) - - linters: - - revive - path: "(cache|database|observability)/testing/|^testing/" - text: "var-naming: avoid package names" - # trace package intentionally named to match its domain - - linters: - - revive - path: "trace/" - text: "var-naming: avoid package names" # cmd/seal-payload is the only file set that imports BOTH go-jose/v4 (whose # package name is `jose`) and github.com/gaborage/go-bricks/jose. One of # them must be aliased away, and the go-bricks package is the dominant one diff --git a/wiki/linting.md b/wiki/linting.md index b2c15ba2..66965b86 100644 --- a/wiki/linting.md +++ b/wiki/linting.md @@ -57,9 +57,15 @@ Pick the order that moves you *toward* your existing convention, not away from i ### 2. Delete the framework-only exclusions Every entry under `linters.exclusions.rules` is a GoBricks path and means nothing in your -repo — `logger/adapter.go` (zerologlint), `(cache|database|observability)/testing/` and -`trace/` (revive var-naming), `^cmd/seal-payload/` (an importas carve-out). Delete them all -and add your own as findings justify. +repo — `logger/adapter.go` (zerologlint), `^cmd/seal-payload/` (an importas carve-out), and +a few `_test.go` scopings. Delete them all and add your own as findings justify. + +Recheck your own exclusions periodically: an exclusion that matches on message `text` stops +matching when the linter rewords the message, and it fails **silently** in either +direction. GoBricks carried two `text: "var-naming: avoid package names"` stanzas that were +dead from revive v1.15.0 onward, when that check moved to a separate rule and the wording +changed. Nothing was being suppressed, and nothing said so — deleting them changed no +findings. Prefer scoping by `path` and `linters` over matching message text. Keep the `presets` list (`comments`, `common-false-positives`, `legacy`, `std-error-handling`) — those are generic. @@ -148,9 +154,9 @@ golangci-lint run ./... 2>&1 | grep -E 'level=error|cannot find rule' ``` **`revive.rules` REPLACES the default set** when `enable-default-rules` is omitted. It does -not extend it, so declaring any rule silently drops every default you did not re-list. -GoBricks re-declares all 23 above its additions for that reason. The alternative is -`enable-default-rules: true`, which keeps the defaults without re-declaration: +not extend it, so declaring any rule silently drops every default you did not re-list — +with no warning, because a missing rule and a passing rule both report nothing. Set +`enable-default-rules: true` and list only your additions: ```yaml linters: @@ -161,7 +167,11 @@ linters: - name: early-return # additions only ``` -That is the shorter path for a new config. It cannot be combined with `enable-all-rules`. +This is what GoBricks does. It previously re-declared all 23 defaults above its additions, +which worked but meant deleting one line lost a rule silently. The two spellings are +equivalent at v2.12.2 — golangci-lint's default list is a verbatim copy of revive's, and +both resolve to the same rule set — so the flag is strictly safer. It cannot be combined +with `enable-all-rules`. Because of all three, a reading of "0 findings" is ambiguous between *no violations*, *the rule never ran*, and *another linter claimed the line*. Prove a rule fires by planting a From 21fd9340d5b7d14e231e084592fd6ef33e4aef70 Mon Sep 17 00:00:00 2001 From: Gabriel Rosales Date: Fri, 7 Aug 2026 21:50:50 -0600 Subject: [PATCH 3/4] docs(wiki): fix awkward wording in the exclusions section "a few `_test.go` scopings" -> "a few rules scoped to `_test.go`". Not a word, and the rephrasing also names what is actually being described. Co-Authored-By: Claude Opus 5 (1M context) --- wiki/linting.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wiki/linting.md b/wiki/linting.md index 66965b86..00c58285 100644 --- a/wiki/linting.md +++ b/wiki/linting.md @@ -58,7 +58,7 @@ Pick the order that moves you *toward* your existing convention, not away from i Every entry under `linters.exclusions.rules` is a GoBricks path and means nothing in your repo — `logger/adapter.go` (zerologlint), `^cmd/seal-payload/` (an importas carve-out), and -a few `_test.go` scopings. Delete them all and add your own as findings justify. +a few rules scoped to `_test.go`. Delete them all and add your own as findings justify. Recheck your own exclusions periodically: an exclusion that matches on message `text` stops matching when the linter rewords the message, and it fails **silently** in either From 0c2e531df1807755a5be2dcd49f51cb02dfaf04a Mon Sep 17 00:00:00 2001 From: Gabriel Rosales Date: Fri, 7 Aug 2026 21:56:16 -0600 Subject: [PATCH 4/4] docs(wiki): narrow two over-broad claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both were unqualified statements that read wider than the evidence behind them. "Nothing was being suppressed" sat in a paragraph advising readers to recheck their own exclusions, where it could be read as a claim about exclusions in general. Scoped to the two var-naming stanzas actually measured. "strictly safer" overstated enable-default-rules. It is safer against one specific failure — omitting a default when re-declaring by hand — and it costs the visible inventory that re-declaration gave. Now "safer against accidental omissions", which is the claim the evidence supports. Co-Authored-By: Claude Opus 5 (1M context) --- wiki/linting.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/wiki/linting.md b/wiki/linting.md index 00c58285..ebeaf492 100644 --- a/wiki/linting.md +++ b/wiki/linting.md @@ -64,8 +64,8 @@ Recheck your own exclusions periodically: an exclusion that matches on message ` matching when the linter rewords the message, and it fails **silently** in either direction. GoBricks carried two `text: "var-naming: avoid package names"` stanzas that were dead from revive v1.15.0 onward, when that check moved to a separate rule and the wording -changed. Nothing was being suppressed, and nothing said so — deleting them changed no -findings. Prefer scoping by `path` and `linters` over matching message text. +changed. Those two stanzas were suppressing nothing, and nothing said so — deleting them +changed no findings. Prefer scoping by `path` and `linters` over matching message text. Keep the `presets` list (`comments`, `common-false-positives`, `legacy`, `std-error-handling`) — those are generic. @@ -170,8 +170,8 @@ linters: This is what GoBricks does. It previously re-declared all 23 defaults above its additions, which worked but meant deleting one line lost a rule silently. The two spellings are equivalent at v2.12.2 — golangci-lint's default list is a verbatim copy of revive's, and -both resolve to the same rule set — so the flag is strictly safer. It cannot be combined -with `enable-all-rules`. +both resolve to the same rule set — so the flag is safer against accidental omissions. It +cannot be combined with `enable-all-rules`. Because of all three, a reading of "0 findings" is ambiguous between *no violations*, *the rule never ran*, and *another linter claimed the line*. Prove a rule fires by planting a