From 3928dc879154924a722abfa937dbbed768b2d402 Mon Sep 17 00:00:00 2001 From: Omar Aljarrah Date: Sun, 9 Aug 2026 06:43:10 +0300 Subject: [PATCH 1/9] build: check that every //nolint names an enabled linter nolintlint reports a directive that suppresses nothing only when the linter it names is enabled: the nolint filter drops nolintlint's "unused directive" issue outright for a linter that is off, so a directive naming a disabled linter -- or one that does not exist -- suppresses nothing, fails nothing, and goes on reading as a live constraint on the code beneath it. scripts/check-nolint-linters.sh cross-checks the names in every //nolint against the set golangci-lint reports as enabled, and the gate runs it after the lint step. The enabled set is asked of golangci-lint rather than copied from .golangci.yml, so enabling or dropping a linter needs no edit there. The gate now names one golangci-lint version for both steps: the check's answer only describes the run above it if the two are the same build. //nolint:forcetypeassert on pass/validate_propids_test.go:86 was of exactly the kind the check rejects -- forcetypeassert has never been enabled here -- so it goes, keeping its justification as a plain comment. --- .github/workflows/gate.yml | 15 +++ CLAUDE.md | 7 +- pass/validate_propids_test.go | 2 +- scripts/check-nolint-linters.sh | 178 ++++++++++++++++++++++++++++++++ 4 files changed, 198 insertions(+), 4 deletions(-) create mode 100755 scripts/check-nolint-linters.sh diff --git a/.github/workflows/gate.yml b/.github/workflows/gate.yml index a840d3d2..86a58387 100644 --- a/.github/workflows/gate.yml +++ b/.github/workflows/gate.yml @@ -6,6 +6,12 @@ on: jobs: gate: runs-on: ubuntu-latest + env: + # One version drives both golangci-lint steps below. The nolint check asks + # golangci-lint which linters are enabled, so its answer only describes the + # lint step above it if the two are the same build. Naming the version also + # means a golangci-lint release cannot redden main without a commit. + GOLANGCI_LINT_VERSION: v2.12.2 steps: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 @@ -17,6 +23,15 @@ jobs: run: go vet ./... - name: lint uses: golangci/golangci-lint-action@v8.0.0 + with: + version: ${{ env.GOLANGCI_LINT_VERSION }} + # The action runs golangci-lint from a path of its own and does not put it + # on PATH, so the check installs the same version for itself. + - name: nolint directives + run: | + curl -sSfL "https://raw.githubusercontent.com/golangci/golangci-lint/${GOLANGCI_LINT_VERSION}/install.sh" | + sh -s -- -b "${RUNNER_TEMP}/bin" "${GOLANGCI_LINT_VERSION}" + PATH="${RUNNER_TEMP}/bin:${PATH}" ./scripts/check-nolint-linters.sh - name: build run: go build ./... - name: coverage diff --git a/CLAUDE.md b/CLAUDE.md index 7dc8d414..c2314003 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -210,11 +210,12 @@ Before claiming any Go work done, run and pass the same checks CI's `gate` job r (`.github/workflows/gate.yml`), in that order: ```bash -gofmt -l $(git ls-files '*.go') # must print nothing +gofmt -l $(git ls-files '*.go') # must print nothing go vet ./... -golangci-lint run # must pass clean +golangci-lint run # must pass clean +./scripts/check-nolint-linters.sh # every //nolint names a linter that is enabled go build ./... -./scripts/check-coverage.sh # go test ./... + exactly 100% statement coverage +./scripts/check-coverage.sh # go test ./... + exactly 100% statement coverage ``` **Coverage is a gate at exactly 100%, not a target.** `scripts/check-coverage.sh` counts diff --git a/pass/validate_propids_test.go b/pass/validate_propids_test.go index 65fd5b4a..cb8cf169 100644 --- a/pass/validate_propids_test.go +++ b/pass/validate_propids_test.go @@ -83,7 +83,7 @@ func TestValidate_PropIDMapKeyIsLeftToTheEncodingCheck(t *testing.T) { // tagging prop. func discriminatedDoc(prop ir.PropID) *ir.Document { doc := validDoc() - base := doc.Types["t/m"].(*ir.Model) //nolint:forcetypeassert // validDoc builds it + base := doc.Types["t/m"].(*ir.Model) // validDoc builds it base.Discriminator = &ir.Discriminator{Property: prop} return doc } diff --git a/scripts/check-nolint-linters.sh b/scripts/check-nolint-linters.sh new file mode 100755 index 00000000..5226345a --- /dev/null +++ b/scripts/check-nolint-linters.sh @@ -0,0 +1,178 @@ +#!/usr/bin/env bash +# +# check-nolint-linters.sh fails when a //nolint directive names a linter that +# golangci-lint is not running. +# +# nolintlint reports a directive that suppresses nothing, but only for a linter +# that is enabled: golangci-lint's nolint filter drops nolintlint's "unused +# directive" issue outright when the named linter is off, in shouldPassIssue +# (pkg/result/processors/nolint_filter.go), under the comment "don't expect +# disabled linters to cover their nolint statements". So a directive naming a +# disabled linter, or one that does not exist at all, suppresses nothing and +# fails nothing, while still reading as a live constraint on the code beneath it. +# +# A run can log "Found unknown linters in //nolint directives: ..." for a name it +# cannot resolve at all, but that reaches even less: the filter parses a file only +# when it has an issue in that file to filter, so a directive in a clean file is +# never read, and the warning is in any case printed by a run that exits 0. It +# also says nothing about a real linter that is merely disabled here. +# +# The enabled set is asked of golangci-lint rather than copied from +# .golangci.yml, so enabling or dropping a linter never needs an edit here. +set -euo pipefail + +repo_root="$(git -C "$(dirname "${BASH_SOURCE[0]}")" rev-parse --show-toplevel)" + +# Bound the failure output: bad directives arrive in blocks — dropping a linter +# from the config invalidates every directive naming it at once — and the first +# screenful is what gets read. +max_reported=25 + +for tool in git jq golangci-lint; do + if ! command -v "$tool" >/dev/null 2>&1; then + echo "NOLINT FAIL: $tool is not on PATH; the enabled set cannot be derived" >&2 + exit 1 + fi +done + +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT + +# Formatters are a separate list in golangci-lint v2, but `run` reports their +# findings under their own name ("File is not properly formatted (gci)"), so +# //nolint:gci is a real directive and both lists belong in the enabled set. +golangci-lint linters --json >"$work/linters.json" +golangci-lint formatters --json >"$work/formatters.json" +jq -r '.Enabled[].name' "$work/linters.json" "$work/formatters.json" | sort -u >"$work/enabled.txt" + +if [ ! -s "$work/enabled.txt" ]; then + echo "NOLINT FAIL: golangci-lint reports no enabled linters, so nothing can be checked" >&2 + exit 1 +fi + +# git grep, not a filesystem walk: tracked Go files are the same set the gate's +# gofmt step reads. The pattern only has to find candidate lines — the awk below +# applies golangci-lint's own grammar to them. Status 1 means no candidates at +# all, which is not an error; anything above it is. +set +e +git -C "$repo_root" grep -nE '//[/ ]*nolint' -- '*.go' >"$work/hits.txt" +grep_status=$? +set -e +if [ "$grep_status" -gt 1 ]; then + echo "NOLINT FAIL: git grep exited $grep_status" >&2 + exit 1 +fi + +# The awk program mirrors extractInlineRangeFromComment in +# pkg/result/processors/nolint_filter.go: strip leading '/' and spaces, require +# what is left to start with "nolint" followed by a space, a colon or the end of +# the comment, cut a trailing "// reason", split the rest on commas, and trim and +# lower-case each name. +if ! awk -v enabled_file="$work/enabled.txt" -v max="$max_reported" ' +BEGIN { + while ((getline name < enabled_file) > 0) { + enabled[name] = 1 + known++ + } + close(enabled_file) + if (known == 0) { + print "NOLINT FAIL: the enabled set came through empty" > "/dev/stderr" + fatal = 1 + exit 1 + } +} + +function trim(s) { + sub("^[ \t\r\n]+", "", s) + sub("[ \t\r\n]+$", "", s) + return s +} + +function emit(file, lineno, why) { + findings++ + if (findings <= max) printf "NOLINT FAIL: %s:%s: %s\n", file, lineno, why +} + +# check reads one directive and reports every name in it that golangci-lint is +# not running. +function check(file, lineno, cand, body, cut, n, parts, i, name) { + directives++ + if (substr(cand, 1, 7) != "nolint:") { + # A bare //nolint, or //nolint followed by prose, suppresses every enabled + # linter. It names nothing to cross-check, which also makes it the one way + # to write a suppression this check cannot see through, so it fails here + # rather than passing silently. + emit(file, lineno, "//nolint names no linter, so it suppresses every enabled one") + return + } + body = substr(cand, 8) + cut = index(body, "//") + if (cut > 0) body = substr(body, 1, cut - 1) + + n = split(body, parts, ",") + if (n == 0) { + # split of the empty string yields no fields at all, so a bare "//nolint:" + # would otherwise leave the loop below with nothing to look at. + emit(file, lineno, "//nolint names no linter, so it suppresses every enabled one") + return + } + + for (i = 1; i <= n; i++) { + name = tolower(trim(parts[i])) + checked++ + # An empty name (a stray comma) resolves to no linter, and "all" is + # golangci-lint spelling out the blanket form above. + if (name == "" || name == "all") { + emit(file, lineno, "//nolint names no linter, so it suppresses every enabled one") + } else if (!(name in enabled)) { + emit(file, lineno, sprintf("//nolint names \"%s\", which golangci-lint is not running", name)) + } + } +} + +# scan tries every "//" on the line rather than only the one opening the comment. +# Telling those apart needs a Go parser: "//" also occurs inside string literals +# and inside prose quoting a directive. Trying all of them over-reports (a +# directive spelled inside a string literal is reported although golangci-lint +# would never see it) and never under-reports, which is the direction a check +# written because another check missed something has to fail in. +function scan(file, lineno, text, pos, cand) { + # text loses at least pos + 1 characters per turn, so the loop is bounded by + # the length of the line. + while ((pos = index(text, "//")) > 0) { + cand = substr(text, pos) + text = substr(text, pos + 2) + sub("^[/ ]+", "", cand) + if (cand == "nolint" || cand ~ "^nolint[ :]") check(file, lineno, cand) + } +} + +$0 == "" { next } + +# git grep -n emits "path:line:text". No tracked path here holds a colon, so the +# first two colons split it; a line without them is a broken invariant, not a +# finding. +{ + p = index($0, ":") + rest = substr($0, p + 1) + q = index(rest, ":") + if (p == 0 || q == 0) { + printf "NOLINT FAIL: unparsable git grep line: %s\n", $0 > "/dev/stderr" + fatal = 1 + exit 1 + } + scan(substr($0, 1, p - 1), substr(rest, 1, q - 1), substr(rest, q + 1)) +} + +END { + if (fatal) exit 1 + if (findings > max) printf " ... and %d more\n", findings - max + if (findings > 0) { + printf "nolint gate failed: %d problem(s) across %d directive(s).\n", findings, directives + exit 1 + } + printf "nolint gate passed: %d directive(s), %d linter name(s), all enabled.\n", directives, checked +} +' "$work/hits.txt"; then + exit 1 +fi From 8cf562249e1b45b2bebce367ee511d63dbc617cb Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 13 Aug 2026 20:10:35 +0300 Subject: [PATCH 2/9] fix(scripts): make the nolint check answer for the repo it scans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four things could make the check report a verdict about something other than this tree, or describe a directive as doing the opposite of what it does. The enabled set was asked of golangci-lint in whatever directory the caller stood in, while the directives were read with `git -C "$repo_root" grep`. Run from anywhere holding a different config, the check reported every real directive here as naming a linter that is not running. Both list calls now run in the repo the script lives in. `jq -r '.Enabled[].name'` cannot iterate the null that golangci-lint emits when a list is empty, and jq reports only its last input's status: with both files in one call, a null linters list exited 0 and silently shrank the enabled set to the formatters, so every directive read as disabled. A null formatters list — an ordinary config with no formatters — killed the script outright with a raw jq error. Each list is now read on its own with a `// []` default, and the existing "no enabled linters" guard sits between them, where it can finally fire; before, neither it nor its awk counterpart could be reached in either case. Three of the four forms the check called blanket suppressions are not. Measured against golangci-lint 2.12.2: `//nolint:` suppresses nothing at all, a stray comma suppresses only the names beside it, and `//nolint:allfoo` does blanket — because extractInlineRangeFromComment tests HasPrefix("nolint:all") before it splits, which the check did not mirror. A directive that suppresses nothing was being reported as suppressing everything, sending a reader looking for over-suppression when the finding beneath it is live. `scan` resumed two characters past each "//", which re-entered the same run of slashes and read one `////nolint:x` as two directives, reporting it twice and inflating both counts. It now resumes past the whole run. Also refuses a tree in which a tracked path holds a colon rather than misparsing `git grep -n` output, which folded the line number into the scanned text and reported the finding at a location without one. --- scripts/check-nolint-linters.sh | 103 +++++++++++++++++++++++--------- 1 file changed, 75 insertions(+), 28 deletions(-) diff --git a/scripts/check-nolint-linters.sh b/scripts/check-nolint-linters.sh index 5226345a..48d99bd9 100755 --- a/scripts/check-nolint-linters.sh +++ b/scripts/check-nolint-linters.sh @@ -19,6 +19,9 @@ # # The enabled set is asked of golangci-lint rather than copied from # .golangci.yml, so enabling or dropping a linter never needs an edit here. +# +# scripts/verify-nolint-grammar.sh drives this script over every grammar form, +# and mutates it to prove each case bites. set -euo pipefail repo_root="$(git -C "$(dirname "${BASH_SOURCE[0]}")" rev-parse --show-toplevel)" @@ -30,7 +33,7 @@ max_reported=25 for tool in git jq golangci-lint; do if ! command -v "$tool" >/dev/null 2>&1; then - echo "NOLINT FAIL: $tool is not on PATH; the enabled set cannot be derived" >&2 + echo "NOLINT FAIL: $tool is not on PATH; the enabled set cannot be derived" exit 1 fi done @@ -38,15 +41,38 @@ done work="$(mktemp -d)" trap 'rm -rf "$work"' EXIT +# Both lists are asked from the repo being scanned, not from wherever the caller +# stood: golangci-lint discovers .golangci.yml relative to its own working +# directory, so an unanchored call answers for a config that has nothing to do +# with the directives git grep is about to read below. +# # Formatters are a separate list in golangci-lint v2, but `run` reports their # findings under their own name ("File is not properly formatted (gci)"), so # //nolint:gci is a real directive and both lists belong in the enabled set. -golangci-lint linters --json >"$work/linters.json" -golangci-lint formatters --json >"$work/formatters.json" -jq -r '.Enabled[].name' "$work/linters.json" "$work/formatters.json" | sort -u >"$work/enabled.txt" - +(cd "$repo_root" && golangci-lint linters --json) >"$work/linters.json" +(cd "$repo_root" && golangci-lint formatters --json) >"$work/formatters.json" + +# Read each list on its own, and tolerate a null .Enabled. jq reports only the +# status of its LAST input, so one call over both files exits 0 when the first +# one is null — silently shrinking the enabled set to whatever the second held, +# which is how every real directive comes to be reported as not running. The +# guard between the two reads is what that would otherwise slip past. +jq -r '(.Enabled // [])[].name' "$work/linters.json" >"$work/enabled.txt" if [ ! -s "$work/enabled.txt" ]; then - echo "NOLINT FAIL: golangci-lint reports no enabled linters, so nothing can be checked" >&2 + echo "NOLINT FAIL: golangci-lint reports no enabled linters, so nothing can be checked" + exit 1 +fi +jq -r '(.Enabled // [])[].name' "$work/formatters.json" >>"$work/enabled.txt" +sort -u -o "$work/enabled.txt" "$work/enabled.txt" + +# git grep -n emits "path:line:text", which the awk below splits on the first two +# colons. That is only correct while no path carries one, so refuse a tree where +# one does rather than misparse it: the split would silently fold the line number +# into the scanned text and report the finding at a location without one. +colon_paths="$(git -C "$repo_root" ls-files -- '*.go' | grep ':' || true)" +if [ -n "$colon_paths" ]; then + echo "NOLINT FAIL: a tracked path holds a colon, which git grep -n output cannot be split:" + echo "$colon_paths" | sed 's/^/ /' exit 1 fi @@ -59,7 +85,7 @@ git -C "$repo_root" grep -nE '//[/ ]*nolint' -- '*.go' >"$work/hits.txt" grep_status=$? set -e if [ "$grep_status" -gt 1 ]; then - echo "NOLINT FAIL: git grep exited $grep_status" >&2 + echo "NOLINT FAIL: git grep exited $grep_status" exit 1 fi @@ -76,7 +102,7 @@ BEGIN { } close(enabled_file) if (known == 0) { - print "NOLINT FAIL: the enabled set came through empty" > "/dev/stderr" + print "NOLINT FAIL: the enabled set came through empty" fatal = 1 exit 1 } @@ -93,37 +119,54 @@ function emit(file, lineno, why) { if (findings <= max) printf "NOLINT FAIL: %s:%s: %s\n", file, lineno, why } +# blanket reports a directive that suppresses every enabled linter at once. It +# names nothing to cross-check, which also makes it the one way to write a +# suppression this check cannot see through, so it fails here rather than +# passing silently. (nolintlint has require-specific, which covers the same +# ground when it is enabled; this check does not depend on that.) +function blanket(file, lineno) { + emit(file, lineno, "//nolint suppresses every enabled linter, so nothing here names what it hides") +} + # check reads one directive and reports every name in it that golangci-lint is # not running. function check(file, lineno, cand, body, cut, n, parts, i, name) { directives++ - if (substr(cand, 1, 7) != "nolint:") { - # A bare //nolint, or //nolint followed by prose, suppresses every enabled - # linter. It names nothing to cross-check, which also makes it the one way - # to write a suppression this check cannot see through, so it fails here - # rather than passing silently. - emit(file, lineno, "//nolint names no linter, so it suppresses every enabled one") + + # extractInlineRangeFromComment blankets on a directive that does not start + # "nolint:", and — testing HasPrefix before it splits — on one that starts + # "nolint:all", which takes "nolint:allfoo" with it. + if (substr(cand, 1, 7) != "nolint:" || substr(cand, 8, 3) == "all") { + blanket(file, lineno) return } + body = substr(cand, 8) cut = index(body, "//") if (cut > 0) body = substr(body, 1, cut - 1) n = split(body, parts, ",") if (n == 0) { - # split of the empty string yields no fields at all, so a bare "//nolint:" - # would otherwise leave the loop below with nothing to look at. - emit(file, lineno, "//nolint names no linter, so it suppresses every enabled one") - return + # awk splits the empty string into no fields where Go splits it into one + # empty one, so "//nolint:" would otherwise skip the loop entirely. + n = 1 + parts[1] = "" } for (i = 1; i <= n; i++) { name = tolower(trim(parts[i])) + # "all" anywhere in the list is golangci-lint spelling out the blanket + # form above, and it stops reading the rest of the list there too. + if (name == "all") { + blanket(file, lineno) + return + } checked++ - # An empty name (a stray comma) resolves to no linter, and "all" is - # golangci-lint spelling out the blanket form above. - if (name == "" || name == "all") { - emit(file, lineno, "//nolint names no linter, so it suppresses every enabled one") + if (name == "") { + # A stray comma, or nothing at all after the colon. golangci-lint + # takes it as a linter whose name is "", which no linter has, so the + # directive is inert — the finding beneath it is still reported. + emit(file, lineno, "//nolint has an empty linter name, which suppresses nothing") } else if (!(name in enabled)) { emit(file, lineno, sprintf("//nolint names \"%s\", which golangci-lint is not running", name)) } @@ -136,28 +179,32 @@ function check(file, lineno, cand, body, cut, n, parts, i, name) { # directive spelled inside a string literal is reported although golangci-lint # would never see it) and never under-reports, which is the direction a check # written because another check missed something has to fail in. -function scan(file, lineno, text, pos, cand) { +function scan(file, lineno, text, pos, cand, lead) { # text loses at least pos + 1 characters per turn, so the loop is bounded by # the length of the line. while ((pos = index(text, "//")) > 0) { cand = substr(text, pos) - text = substr(text, pos + 2) + lead = cand sub("^[/ ]+", "", cand) + # Resume past the whole run of slashes and spaces that opened this + # comment. Resuming after the first two re-enters the same run, which + # reads one "////nolint:x" as two directives and reports it twice. + text = substr(text, pos + length(lead) - length(cand)) if (cand == "nolint" || cand ~ "^nolint[ :]") check(file, lineno, cand) } } $0 == "" { next } -# git grep -n emits "path:line:text". No tracked path here holds a colon, so the -# first two colons split it; a line without them is a broken invariant, not a -# finding. +# git grep -n emits "path:line:text", and the shell above has already refused a +# tree in which a path could hold a colon, so the first two split it; a line +# without them is a broken invariant, not a finding. { p = index($0, ":") rest = substr($0, p + 1) q = index(rest, ":") if (p == 0 || q == 0) { - printf "NOLINT FAIL: unparsable git grep line: %s\n", $0 > "/dev/stderr" + printf "NOLINT FAIL: unparsable git grep line: %s\n", $0 fatal = 1 exit 1 } From 2f1bf7f4be6357601dc54ae3f749fce07f9f3862 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 13 Aug 2026 20:10:54 +0300 Subject: [PATCH 3/9] test(scripts): drive every //nolint form and mutate the reader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate script re-implements extractInlineRangeFromComment in awk, and every verdict it reaches rests on that grammar agreeing with golangci-lint's. Nothing held it to that. The tree carries four directives, all of one shape, so the committed corpus exercises almost none of the reader: a name that is merely disabled, an empty name, a blanket suppression spelled four different ways — none of them appear, and a drift in any of them would be nobody's job to notice. Twenty-four cases cover the grammar and the shapes around it that decide whether the answer describes this repo: which config the enabled set comes from, a null Enabled list, and a path git grep -n output cannot be split on. Each is followed by a mutation that must turn it red, so a case that catches nothing fails here rather than reading as coverage. Two cases pin the case-sensitivity split that is easiest to get wrong. golangci-lint tests the "all" prefix on the raw text but compares the split-out names lower-cased, so `//nolint:allfoo` blankets while `//nolint:AllFoo` is merely an unknown name, and `//nolint:ALL` blankets by the second test after failing the first. The checks that keep a case from passing vacuously are the ones that matter most: a case naming a fixture nothing built, a mutation naming a case the table does not carry, and an anchor appearing zero or many times all fail loudly. A mutation whose anchor no longer appears changes nothing, and a target that "survives" that is indistinguishable from a case that tests nothing. --- scripts/verify-nolint-grammar.sh | 383 +++++++++++++++++++++++++++++++ 1 file changed, 383 insertions(+) create mode 100755 scripts/verify-nolint-grammar.sh diff --git a/scripts/verify-nolint-grammar.sh b/scripts/verify-nolint-grammar.sh new file mode 100755 index 00000000..679b6915 --- /dev/null +++ b/scripts/verify-nolint-grammar.sh @@ -0,0 +1,383 @@ +#!/usr/bin/env bash +# +# verify-nolint-grammar.sh checks how scripts/check-nolint-linters.sh reads a +# //nolint directive. +# +# The gate script re-implements extractInlineRangeFromComment +# (pkg/result/processors/nolint_filter.go) in awk, and every verdict it reaches +# rests on that grammar agreeing with golangci-lint's. The tree carries a handful +# of directives, all of one shape, so the committed corpus exercises almost none +# of it: the forms that matter — a name that is merely disabled, an empty name, a +# blanket suppression spelled four different ways — never appear, and would be +# nobody's job to notice if the grammar drifted. +# +# It also drives the shapes around the grammar that decide whether the answer +# describes this repo at all: which config the enabled set is read from, a null +# Enabled list, and a path git grep -n cannot be split on. +# +# Each case is followed by a mutation that must break it. A case that stays green +# against a deliberately broken reader is not testing the reader — and a gate +# step nothing tests is one that can start passing everything without a sign. +# +# Usage: +# scripts/verify-nolint-grammar.sh +set -euo pipefail + +repo_root="$(git -C "$(dirname "${BASH_SOURCE[0]}")" rev-parse --show-toplevel)" +script="$repo_root/scripts/check-nolint-linters.sh" + +for tool in git jq golangci-lint; do + if ! command -v "$tool" >/dev/null 2>&1; then + printf 'NOLINT VERIFY FAIL: %s is not on PATH\n' "$tool" >&2 + exit 1 + fi +done + +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT + +failures=0 + +# fail records a failed check and keeps going, so one run reports every problem +# rather than only the first. +fail() { + printf ' FAIL: %s\n' "$1" + failures=$((failures + 1)) +} + +pass() { printf ' ok: %s\n' "$1"; } + +# The fixture enables one linter and one formatter, so a directive can name an +# enabled linter (errorlint), an enabled formatter (gofmt), a real linter that is +# off (unparam), or nothing that exists — the four verdicts the script reaches. +standard_config='version: "2" +linters: + default: none + enable: + - errorlint +formatters: + enable: + - gofmt +' + +# make_repo creates $work/ as a tracked repo carrying the given config. +make_repo() { + local name="$1" config="$2" dir="$work/$1" + mkdir -p "$dir/pk" "$dir/scripts" + printf '%s' "$config" >"$dir/.golangci.yml" + printf 'package pk\n' >"$dir/pk/a.go" + git init -q "$dir" + git -C "$dir" config user.email nolint@example.invalid + git -C "$dir" config user.name nolint + git -C "$dir" add -A + git -C "$dir" commit -qm fixture +} + +# plant rewrites the fixture's one Go file so it holds exactly the given lines. +# git grep reads the working tree, so no commit is needed between cases. +plant() { + local dir="$work/$1" + shift + { + printf 'package pk\n\n' + printf '%s\n' "$@" + } >"$dir/pk/a.go" +} + +make_repo main "$standard_config" +make_repo noformatters 'version: "2" +linters: + default: none + enable: + - errorlint +' +make_repo nolinters 'version: "2" +linters: + default: none +formatters: + enable: + - gofmt +' + +# A directory that is not the repo, holding a config that disagrees with it about +# every name the cases use. Anything reading the enabled set from the caller's +# directory answers for this file instead of the fixture's. +mkdir -p "$work/foreign" +printf 'version: "2"\nlinters:\n default: none\n enable:\n - unparam\nformatters:\n enable:\n - gci\n' \ + >"$work/foreign/.golangci.yml" + +# 30 bad directives, five past the 25 the failure listing prints. +capped_lines=() +for i in $(seq 30); do capped_lines+=("var _ = $i //nolint:bogus$i // r"); done + +# One record per case: | | | | . "@capped" plants the 30 lines above instead of one. The +# mutation sweep looks cases up by name, so each name appears once. +# +# Every "passed" case pins the directive count too: a grammar change that stops +# seeing a directive at all would otherwise read as a pass. +cases=( + 'clean|main|var _ = 1|0|nolint gate passed: 0 directive(s), 0 linter name(s), all enabled.' + 'enabled|main|var _ = 1 //nolint:errorlint // r|0|nolint gate passed: 1 directive(s), 1 linter name(s), all enabled.' + 'formatter|main|var _ = 1 //nolint:gofmt // r|0|nolint gate passed: 1 directive(s), 1 linter name(s), all enabled.' + 'disabled|main|var _ = 1 //nolint:unparam // r|1|//nolint names "unparam", which golangci-lint is not running' + 'unknown|main|var _ = 1 //nolint:notarealinter // r|1|//nolint names "notarealinter", which golangci-lint is not running' + 'bare|main|var _ = 1 //nolint // r|1|//nolint suppresses every enabled linter' + 'all|main|var _ = 1 //nolint:all // r|1|//nolint suppresses every enabled linter' + 'allprefix|main|var _ = 1 //nolint:allfoo // r|1|//nolint suppresses every enabled linter' + 'all-in-list|main|var _ = 1 //nolint:errorlint,all // r|1|//nolint suppresses every enabled linter' + # golangci-lint tests HasPrefix("nolint:all") on the raw text and compares the + # split-out names lower-cased, so the two disagree on case: "//nolint:allfoo" + # blankets, "//nolint:AllFoo" is just an unknown name, and "//nolint:ALL" + # blankets by the second test after failing the first. + 'all-uppercase|main|var _ = 1 //nolint:ALL // r|1|//nolint suppresses every enabled linter' + 'allprefix-mixedcase|main|var _ = 1 //nolint:AllFoo // r|1|//nolint names "allfoo", which golangci-lint is not running' + 'emptyname|main|var _ = 1 //nolint:|1|//nolint has an empty linter name, which suppresses nothing' + 'trailing-comma|main|var _ = 1 //nolint:errorlint,|1|//nolint has an empty linter name, which suppresses nothing' + 'mixedcase|main|var _ = 1 //nolint:ErrorLint // r|0|nolint gate passed: 1 directive(s), 1 linter name(s), all enabled.' + 'spacecolon|main|var _ = 1 //nolint: errorlint // r|0|nolint gate passed: 1 directive(s), 1 linter name(s), all enabled.' + 'leadingspace|main|var _ = 1 // nolint:errorlint // r|0|nolint gate passed: 1 directive(s), 1 linter name(s), all enabled.' + 'reasoncut|main|var _ = 1 //nolint:errorlint //notalinter|0|nolint gate passed: 1 directive(s), 1 linter name(s), all enabled.' + 'extraslashes|main|var _ = 1 ////nolint:notarealinter // r|1|nolint gate failed: 1 problem(s) across 1 directive(s).' + 'notdirective|main|var _ = 1 //nolintfoo:bar|0|nolint gate passed: 0 directive(s), 0 linter name(s), all enabled.' + 'blockcomment|main|var _ = 1 /*nolint:notarealinter*/|0|nolint gate passed: 0 directive(s), 0 linter name(s), all enabled.' + # The documented over-report: golangci-lint never sees this one, since the + # directive is inside a string literal. Telling it apart needs a Go parser, + # and over-reporting is the direction this check has to fail in. Asserted on + # the count, because the name carries the closing quote with it. + 'instring|main|var _ = "//nolint:notarealinter"|1|nolint gate failed: 1 problem(s) across 1 directive(s).' + 'capped|main|@capped|1| ... and 5 more' + 'no-formatters-config|noformatters|var _ = 1 //nolint:errorlint // r|0|nolint gate passed: 1 directive(s), 1 linter name(s), all enabled.' + 'no-linters-config|nolinters|var _ = 1 //nolint:errorlint // r|1|NOLINT FAIL: golangci-lint reports no enabled linters, so nothing can be checked' +) + +# check_case drives one record through and echoes why it did +# not hold, echoing nothing and returning 0 when it did. +check_case() { + local record="$1" candidate="$2" from_dir="${3:-}" + local name="${record%%|*}" rest="${record#*|}" + local repo="${rest%%|*}" + rest="${rest#*|}" + local planted="${rest%%|*}" + rest="${rest#*|}" + local want_status="${rest%%|*}" want="${rest#*|}" + + local dir="$work/$repo" + if [ ! -d "$dir/.git" ]; then + printf 'no fixture repo named %s was built' "$repo" + return 1 + fi + + if [ "$planted" = "@capped" ]; then + plant "$repo" "${capped_lines[@]}" + else + plant "$repo" "$planted" + fi + + cp "$candidate" "$dir/scripts/check-nolint-linters.sh" + chmod +x "$dir/scripts/check-nolint-linters.sh" + + # stdout only: what the script reports there is the contract being checked, + # and a mutant broken outright would otherwise bury the run in awk errors. + local out status=0 + out="$(cd "${from_dir:-$dir}" && "$dir/scripts/check-nolint-linters.sh" 2>/dev/null)" || status=$? + if [ "$status" -ne "$want_status" ]; then + printf 'exit %d, want %s' "$status" "$want_status" + return 1 + fi + + case "$out" in + *"$want"*) return 0 ;; + esac + printf 'stdout does not hold "%s"' "$want" + return 1 +} + +# case_record echoes the case record named . +case_record() { + local record + for record in "${cases[@]}"; do + if [ "${record%%|*}" = "$1" ]; then + printf '%s' "$record" + return 0 + fi + done + printf 'no case named %s\n' "$1" >&2 + return 1 +} + +printf 'grammar and configuration\n' +for record in "${cases[@]}"; do + if reason="$(check_case "$record" "$script")"; then + pass "${record%%|*}" + else + fail "${record%%|*}: $reason" + fi +done + +# mutate echoes the gate script with the sole occurrence of replaced by +# . Exact text, not a regex: the mutants have to read the same under BSD and +# GNU userlands, and a pattern that quietly means something else under one of +# them is indistinguishable from a case that catches nothing. +mutate() { + awk -v from="$1" -v to="$2" ' + index($0, from) { + $0 = substr($0, 1, index($0, from) - 1) to substr($0, index($0, from) + length(from)) + } + { print } + ' "$script" +} + +# Each mutation removes one decision the reader makes, and names the case that +# has to notice. An anchor holds no "|", which splits the record, and no +# backslash, which awk -v would turn into the control character it spells. +# +# | | | +mutations=( + 'no-all-prefix|allprefix|substr(cand, 8, 3) == "all"|0' + 'no-all-in-list|all-in-list|name == "all"|0' + 'no-empty-split|emptyname|n = 1|n = 0' + 'no-empty-name|trailing-comma|name == ""|0' + 'no-lowercase|mixedcase|tolower(|(' + 'no-trim|spacecolon|trim(parts[i])|parts[i]' + 'no-slash-strip|leadingspace|sub("^[/ ]+", "", cand)|sub("^[/]+", "", cand)' + 'no-reason-cut|reasoncut|if (cut > 0)|if (0)' + 'no-scan-advance|extraslashes|pos + length(lead) - length(cand)|pos + 2' + 'no-directive-anchor|notdirective|cand ~ "^nolint[ :]"|cand ~ "nolint"' + 'no-enabled-lookup|disabled|!(name in enabled)|0' + 'no-formatters-list|formatter|>>"$work/enabled.txt"|>/dev/null' + 'no-cap|capped|max_reported=25|max_reported=100' + 'no-null-guard|no-formatters-config|(.Enabled // [])[].name'"'"' "$work/formatters.json"|.Enabled[].name'"'"' "$work/formatters.json"' + 'one-jq-call|no-linters-config|"$work/linters.json" >"$work/enabled.txt"|"$work/linters.json" "$work/formatters.json" >"$work/enabled.txt"' +) + +sanity_record="$(case_record clean)" + +printf '\nmutations, each of which must turn its case red\n' +for record in "${mutations[@]}"; do + name="${record%%|*}" + rest="${record#*|}" + target="${rest%%|*}" + rest="${rest#*|}" + from="${rest%%|*}" + to="${rest#*|}" + mutant="$work/mutant-$name.sh" + + if ! target_record="$(case_record "$target")"; then + fail "$name: names a case the table does not carry" + continue + fi + + # An anchor that no longer appears mutates nothing, and a case that + # "survives" that looks exactly like a case that catches nothing. One that + # appears twice would mutate both, so the case would not say which decision + # it caught. + hits="$(awk -v from="$from" ' + { + left = $0 + while ((at = index(left, from)) > 0) { + n++ + left = substr(left, at + length(from)) + } + } + END { print n + 0 }' "$script")" + if [ "$hits" -ne 1 ]; then + fail "$name: its anchor appears $hits times in $(basename "$script"), want 1" + continue + fi + + mutate "$from" "$to" >"$mutant" + chmod +x "$mutant" + + if cmp -s "$script" "$mutant"; then + fail "$name: changed nothing despite a unique anchor" + continue + fi + + # The directive-free case reaches no decision any mutation here alters. If it + # goes red the mutant is broken outright — an awk parse error, say — and its + # effect on the target case would say nothing about the decision removed. + if ! check_case "$sanity_record" "$mutant" >/dev/null; then + fail "$name: breaks even the directive-free case, so it proves nothing" + continue + fi + + if check_case "$target_record" "$mutant" >/dev/null; then + fail "$name: '$target' still passes with $name applied, so it does not test it" + else + pass "$name turns '$target' red" + fi +done + +# The directives are read from the repo the script lives in, but golangci-lint +# discovers .golangci.yml relative to its own working directory. Unless the two +# are tied together, running the script from anywhere else answers for a config +# that has nothing to do with the tree being scanned. Checked by running the same +# case from two directories rather than by inspecting the anchor. +printf '\nthe enabled set follows the repo, not the caller\n' + +anchor_case="$(case_record enabled)" +if ! reason="$(check_case "$anchor_case" "$script" "$work/foreign")"; then + fail "the enabled case does not hold when run from another directory: $reason" +else + # Without the anchor the two runs must disagree, or this proves nothing. + loose="$work/mutant-unanchored.sh" + mutate 'cd "$repo_root" && golangci-lint linters --json' 'golangci-lint linters --json' >"$loose" + chmod +x "$loose" + if check_case "$anchor_case" "$loose" "$work/foreign" >/dev/null; then + fail "an unanchored read still holds from another directory, so this cannot detect one" + else + pass "the verdict is the same from the repo and from an unrelated directory" + fi +fi + +# git grep -n output is split on its first two colons, which a path holding one +# would silently derail — the finding would keep its file but lose its line. +printf '\na path git grep -n output cannot be split\n' + +colon_dir="$work/colon" +make_repo colon "$standard_config" +if ! printf 'package pk\n\nvar _ = 1 //nolint:notarealinter // r\n' >"$colon_dir/pk/a:b.go" 2>/dev/null; then + printf ' skip: this filesystem will not hold a path with a colon\n' +else + git -C "$colon_dir" add -A + git -C "$colon_dir" commit -qm colon + cp "$script" "$colon_dir/scripts/check-nolint-linters.sh" + chmod +x "$colon_dir/scripts/check-nolint-linters.sh" + + out="$(cd "$colon_dir" && ./scripts/check-nolint-linters.sh 2>/dev/null)" || true + case "$out" in + *"a tracked path holds a colon"*) + # Without the guard it must not refuse, or the refusal proves nothing. + guardless="$work/mutant-guardless.sh" + mutate 'if [ -n "$colon_paths" ]; then' 'if false; then' >"$guardless" + chmod +x "$guardless" + cp "$guardless" "$colon_dir/scripts/check-nolint-linters.sh" + loose_out="$(cd "$colon_dir" && ./scripts/check-nolint-linters.sh 2>/dev/null)" || true + case "$loose_out" in + *"a tracked path holds a colon"*) + fail "the tree is refused even without the guard, so the guard is not what refuses it" + ;; + *"pk/a:b.go: //nolint names"*) + # The misparse the guard exists to prevent: the finding keeps its + # file and loses its line, "pk/a:b.go" standing where "path:line" + # belongs. Seeing it here is what makes the refusal above load-bearing. + pass "a colon in a tracked path is refused, and without the guard the line number is silently lost" + ;; + *) + fail "without the guard the run neither refused nor misparsed, so this pins nothing" + ;; + esac + ;; + *) + fail "a colon in a tracked path was not refused" + ;; + esac +fi + +printf '\n' +if ((failures > 0)); then + printf '%d check(s) failed\n' "$failures" >&2 + exit 1 +fi +printf 'all checks passed\n' From 86103c4cd1f7bda981cc67150da4d4357a87f6e5 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 13 Aug 2026 20:10:54 +0300 Subject: [PATCH 4/9] build: reuse the golangci-lint the lint step puts on PATH The nolint step installed golangci-lint a second time, on the grounds that the action "runs golangci-lint from a path of its own and does not put it on PATH". It does put it on PATH: the bundle the runner executes at v8.0.0 calls core.addPath(path.dirname(binPath)) in its entrypoint, which prepends that directory to PATH for every later step in the job. Running the install block locally proved the install works; it could not observe what the action exported, which is why the claim stood. Dropping the second install makes the check ask the same build that produced the findings above it, rather than a separately downloaded copy that merely carries the same version string, and takes a curl-piped-to-sh out of the job. The version pin moves to the action's own input, where the one step that consumes it is. Adds the grammar verifier as its own step ahead of the check it drives, matching how the coverage counter sits ahead of the coverage gate. --- .github/workflows/gate.yml | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/.github/workflows/gate.yml b/.github/workflows/gate.yml index a9970618..3544541d 100644 --- a/.github/workflows/gate.yml +++ b/.github/workflows/gate.yml @@ -6,12 +6,6 @@ on: jobs: gate: runs-on: ubuntu-latest - env: - # One version drives both golangci-lint steps below. The nolint check asks - # golangci-lint which linters are enabled, so its answer only describes the - # lint step above it if the two are the same build. Naming the version also - # means a golangci-lint release cannot redden main without a commit. - GOLANGCI_LINT_VERSION: v2.12.2 steps: - uses: actions/checkout@v4 - uses: actions/setup-go@v5 @@ -24,14 +18,17 @@ jobs: - name: lint uses: golangci/golangci-lint-action@v8.0.0 with: - version: ${{ env.GOLANGCI_LINT_VERSION }} - # The action runs golangci-lint from a path of its own and does not put it - # on PATH, so the check installs the same version for itself. + # Pinned so a golangci-lint release cannot redden main without a commit, + # and so the two steps below describe the run that produced the findings + # above: they ask golangci-lint which linters it is running, which only + # answers for this step if it is the same build. + version: v2.12.2 + # The action leaves the golangci-lint it installed on PATH for later steps + # (core.addPath in its entrypoint), so these reach that same binary. + - name: nolint grammar + run: ./scripts/verify-nolint-grammar.sh - name: nolint directives - run: | - curl -sSfL "https://raw.githubusercontent.com/golangci/golangci-lint/${GOLANGCI_LINT_VERSION}/install.sh" | - sh -s -- -b "${RUNNER_TEMP}/bin" "${GOLANGCI_LINT_VERSION}" - PATH="${RUNNER_TEMP}/bin:${PATH}" ./scripts/check-nolint-linters.sh + run: ./scripts/check-nolint-linters.sh - name: build run: go build ./... - name: coverage counting From b190518fd0a0e6d730fb652ade40d9278b8020cf Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 13 Aug 2026 20:10:54 +0300 Subject: [PATCH 5/9] docs: list both nolint checks with the rest of the gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLAUDE.md and README.md each introduce their command block as the checks CI's gate job runs, so a step that is not in them makes the promise false. README.md was missing the directive check as well as the grammar verifier; its three older disagreements with the gate are separate and are filed as #415. .golangci.yml described the gap nolintlint leaves as "GitHub #306", which this change closes — leaving a reader at a resolved issue rather than at the script that now covers it. --- .golangci.yml | 3 ++- CLAUDE.md | 1 + README.md | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 26d75991..316ac012 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -32,7 +32,8 @@ linters: # allow-unused only reaches directives naming an ENABLED linter. One naming a # disabled or nonexistent linter is ignored by the nolint processor and # reported by nothing — which is how //nolint:forcetypeassert stood here - # without ever suppressing anything. That gap is GitHub #306. + # without ever suppressing anything. scripts/check-nolint-linters.sh closes + # that, cross-checking every name against the set this file leaves enabled. allow-unused: false # it no longer suppresses anything — delete it require-specific: true # bare //nolint hides findings nobody chose to accept require-explanation: true # the rationale is what a later reader re-checks against diff --git a/CLAUDE.md b/CLAUDE.md index 85cc8654..a34a3879 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -244,6 +244,7 @@ Before claiming any Go work done, run and pass the same checks CI's `gate` job r gofmt -l $(git ls-files '*.go') # must print nothing go vet ./... golangci-lint run # must pass clean +./scripts/verify-nolint-grammar.sh # how the check below reads a directive ./scripts/check-nolint-linters.sh # every //nolint names a linter that is enabled go build ./... ./scripts/verify-coverage-count.sh # how the gate below counts a profile diff --git a/README.md b/README.md index 8c2afb99..12f383f4 100644 --- a/README.md +++ b/README.md @@ -228,6 +228,8 @@ change lands: gofmt -l . # must print nothing go vet ./... golangci-lint run +./scripts/verify-nolint-grammar.sh # checks how the check below reads a directive +./scripts/check-nolint-linters.sh # every //nolint names a linter that is enabled go build ./... go test ./... ./scripts/verify-coverage-count.sh # checks how the gate below counts a profile From 84a7eda0221a83b459ad622d3fc42edd45256ddc Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 13 Aug 2026 20:14:21 +0300 Subject: [PATCH 6/9] fix(scripts): decide the enabled set by bytes, not by collation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sort -u` folds together whatever the locale calls equal, and outside C that reaches past exact bytes — names differing only in case or punctuation can collapse into one, and the name that lost would then read as a linter golangci-lint is not running. Pinned to C, matching the failure listing in check-coverage.sh. The grammar fixtures also stop inheriting the caller's git configuration. A global commit.gpgsign with no key on the runner would fail the fixture commit, and the suite would read as a broken script rather than as a fixture that was never built. --- scripts/check-nolint-linters.sh | 5 ++++- scripts/verify-nolint-grammar.sh | 7 ++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/scripts/check-nolint-linters.sh b/scripts/check-nolint-linters.sh index 48d99bd9..500bb592 100755 --- a/scripts/check-nolint-linters.sh +++ b/scripts/check-nolint-linters.sh @@ -63,7 +63,10 @@ if [ ! -s "$work/enabled.txt" ]; then exit 1 fi jq -r '(.Enabled // [])[].name' "$work/formatters.json" >>"$work/enabled.txt" -sort -u -o "$work/enabled.txt" "$work/enabled.txt" +# LC_ALL=C so -u decides "same name" by bytes. Collation elsewhere can fold names +# that differ only in case or punctuation into one, and the loser would then read +# as a linter golangci-lint is not running. +LC_ALL=C sort -u -o "$work/enabled.txt" "$work/enabled.txt" # git grep -n emits "path:line:text", which the awk below splits on the first two # colons. That is only correct while no path carries one, so refuse a tree where diff --git a/scripts/verify-nolint-grammar.sh b/scripts/verify-nolint-grammar.sh index 679b6915..d315f257 100755 --- a/scripts/verify-nolint-grammar.sh +++ b/scripts/verify-nolint-grammar.sh @@ -66,9 +66,14 @@ make_repo() { mkdir -p "$dir/pk" "$dir/scripts" printf '%s' "$config" >"$dir/.golangci.yml" printf 'package pk\n' >"$dir/pk/a.go" - git init -q "$dir" + # The fixture must not inherit the caller's git identity, signing or default + # branch: a global commit.gpgsign would fail the commit on a runner with no + # key, and the whole suite would read as a broken script rather than a + # missing fixture. + git -c init.defaultBranch=main init -q "$dir" git -C "$dir" config user.email nolint@example.invalid git -C "$dir" config user.name nolint + git -C "$dir" config commit.gpgsign false git -C "$dir" add -A git -C "$dir" commit -qm fixture } From dc44cb83ed8e714c42b1410ccd227f5a6508a7ac Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 13 Aug 2026 20:17:35 +0300 Subject: [PATCH 7/9] test(scripts): drop an unread local from the case reader check_case bound the case name and never looked at it; the repo field is what selects the fixture. --- scripts/verify-nolint-grammar.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/verify-nolint-grammar.sh b/scripts/verify-nolint-grammar.sh index d315f257..f1c1d243 100755 --- a/scripts/verify-nolint-grammar.sh +++ b/scripts/verify-nolint-grammar.sh @@ -160,7 +160,7 @@ cases=( # not hold, echoing nothing and returning 0 when it did. check_case() { local record="$1" candidate="$2" from_dir="${3:-}" - local name="${record%%|*}" rest="${record#*|}" + local rest="${record#*|}" local repo="${rest%%|*}" rest="${rest#*|}" local planted="${rest%%|*}" From d0dfb01d86fe875350a72314e612199d4de813a4 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 13 Aug 2026 20:18:57 +0300 Subject: [PATCH 8/9] docs(scripts): say which empty-name directive is wholly inert A stray comma leaves the names beside it working; only "//nolint:" on its own suppresses nothing at all. The comment claimed the latter of both. --- scripts/check-nolint-linters.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scripts/check-nolint-linters.sh b/scripts/check-nolint-linters.sh index 500bb592..94c56094 100755 --- a/scripts/check-nolint-linters.sh +++ b/scripts/check-nolint-linters.sh @@ -166,9 +166,10 @@ function check(file, lineno, cand, body, cut, n, parts, i, name) { } checked++ if (name == "") { - # A stray comma, or nothing at all after the colon. golangci-lint - # takes it as a linter whose name is "", which no linter has, so the - # directive is inert — the finding beneath it is still reported. + # A stray comma, or nothing at all after the colon. golangci-lint reads + # it as a linter named "", which none is, so this name suppresses + # nothing; where it is the only name, the whole directive is inert and + # the finding beneath it is still reported. emit(file, lineno, "//nolint has an empty linter name, which suppresses nothing") } else if (!(name in enabled)) { emit(file, lineno, sprintf("//nolint names \"%s\", which golangci-lint is not running", name)) From 87f831c473f0d1aa75d52c6bb456257d972779e1 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Thu, 13 Aug 2026 20:36:32 +0300 Subject: [PATCH 9/9] fix(scripts): read a directive the same way on a CRLF file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit go/scanner drops a trailing CR from a // comment before the nolint filter ever sees it, so on a CRLF file golangci-lint honours a bare //nolint — measured, the finding under it disappears. The grammar here was looking at "nolint\r", which matches neither anchor, and missed the directive entirely. That is an under-report, the one direction this check is not allowed to fail in, and the gate's gofmt step rejecting CRLF first does not make it right for a check meant to stand on its own. Covered by a CRLF fixture and the mutation that removes the strip. Two guards no planted directive can reach are now driven directly: the tool check, which runs before the script has read anything, and the awk-side empty check, which only fires when awk cannot read the file the shell just found non-empty. Both were reachable and neither was tested; removing either now turns its check red. The comment on the enabled-set sort claimed collation could fold two names into one. It cannot for any name golangci-lint ships: glibc separates "gocritic", "go-critic" and "GoCritic" under en_US.UTF-8. The pin stays as insurance; the comment now says which it is. Also drops the rule that skipped an empty line in the git grep output. No such line can occur, and skipping it silently disagreed with the rule below it, which treats any line that is not path:line:text as a broken invariant. --- scripts/check-nolint-linters.sh | 20 ++++++++--- scripts/verify-nolint-grammar.sh | 62 +++++++++++++++++++++++++++++--- 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/scripts/check-nolint-linters.sh b/scripts/check-nolint-linters.sh index 94c56094..c6beb019 100755 --- a/scripts/check-nolint-linters.sh +++ b/scripts/check-nolint-linters.sh @@ -63,9 +63,10 @@ if [ ! -s "$work/enabled.txt" ]; then exit 1 fi jq -r '(.Enabled // [])[].name' "$work/formatters.json" >>"$work/enabled.txt" -# LC_ALL=C so -u decides "same name" by bytes. Collation elsewhere can fold names -# that differ only in case or punctuation into one, and the loser would then read -# as a linter golangci-lint is not running. +# -u drops what the locale's collation calls equal, which is a byte comparison +# only under C. No name golangci-lint ships is affected — glibc separates +# "gocritic", "go-critic" and "GoCritic" under en_US.UTF-8 — so this is insurance +# against a name that would collide rather than a fix for one that does. LC_ALL=C sort -u -o "$work/enabled.txt" "$work/enabled.txt" # git grep -n emits "path:line:text", which the awk below splits on the first two @@ -117,6 +118,15 @@ function trim(s) { return s } +# stripcr drops the trailing CR that go/scanner drops before the nolint filter +# reads the comment. Without it a CRLF file leaves this grammar looking at +# "nolint\r", which matches neither anchor below, while golangci-lint honours the +# bare //nolint it came from. +function stripcr(s) { + sub("\r$", "", s) + return s +} + function emit(file, lineno, why) { findings++ if (findings <= max) printf "NOLINT FAIL: %s:%s: %s\n", file, lineno, why @@ -198,12 +208,12 @@ function scan(file, lineno, text, pos, cand, lead) { } } -$0 == "" { next } - # git grep -n emits "path:line:text", and the shell above has already refused a # tree in which a path could hold a colon, so the first two split it; a line # without them is a broken invariant, not a finding. { + $0 = stripcr($0) + p = index($0, ":") rest = substr($0, p + 1) q = index(rest, ":") diff --git a/scripts/verify-nolint-grammar.sh b/scripts/verify-nolint-grammar.sh index f1c1d243..ee7dd508 100755 --- a/scripts/verify-nolint-grammar.sh +++ b/scripts/verify-nolint-grammar.sh @@ -89,6 +89,19 @@ plant() { } >"$dir/pk/a.go" } +# plant_crlf is plant with CRLF line endings. go/scanner drops the trailing CR +# before the nolint filter reads the comment, so a directive there is live to +# golangci-lint. The gate's gofmt step would reject such a file, but this check +# must not depend on a step above it having run. +plant_crlf() { + local dir="$work/$1" + shift + { + printf 'package pk\r\n\r\n' + printf '%s\r\n' "$@" + } >"$dir/pk/a.go" +} + make_repo main "$standard_config" make_repo noformatters 'version: "2" linters: @@ -151,6 +164,10 @@ cases=( # and over-reporting is the direction this check has to fail in. Asserted on # the count, because the name carries the closing quote with it. 'instring|main|var _ = "//nolint:notarealinter"|1|nolint gate failed: 1 problem(s) across 1 directive(s).' + # A bare //nolint at the end of a CRLF line. go/scanner drops the CR, so + # golangci-lint honours it — measured: the finding under it disappears — and a + # grammar reading "nolint\r" would match neither anchor and miss it entirely. + 'crlf-bare|main|@crlf|1|//nolint suppresses every enabled linter' 'capped|main|@capped|1| ... and 5 more' 'no-formatters-config|noformatters|var _ = 1 //nolint:errorlint // r|0|nolint gate passed: 1 directive(s), 1 linter name(s), all enabled.' 'no-linters-config|nolinters|var _ = 1 //nolint:errorlint // r|1|NOLINT FAIL: golangci-lint reports no enabled linters, so nothing can be checked' @@ -173,11 +190,11 @@ check_case() { return 1 fi - if [ "$planted" = "@capped" ]; then - plant "$repo" "${capped_lines[@]}" - else - plant "$repo" "$planted" - fi + case "$planted" in + @capped) plant "$repo" "${capped_lines[@]}" ;; + @crlf) plant_crlf "$repo" 'var _ = 1 //nolint' ;; + *) plant "$repo" "$planted" ;; + esac cp "$candidate" "$dir/scripts/check-nolint-linters.sh" chmod +x "$dir/scripts/check-nolint-linters.sh" @@ -249,6 +266,7 @@ mutations=( 'no-reason-cut|reasoncut|if (cut > 0)|if (0)' 'no-scan-advance|extraslashes|pos + length(lead) - length(cand)|pos + 2' 'no-directive-anchor|notdirective|cand ~ "^nolint[ :]"|cand ~ "nolint"' + 'no-cr-strip|crlf-bare|$0 = stripcr($0)|$0 = $0' 'no-enabled-lookup|disabled|!(name in enabled)|0' 'no-formatters-list|formatter|>>"$work/enabled.txt"|>/dev/null' 'no-cap|capped|max_reported=25|max_reported=100' @@ -380,6 +398,40 @@ else esac fi +# Two guards no planted directive can reach. The first runs before the script has +# read anything; the second only fires when awk cannot read the file the shell +# just found non-empty, so no config produces it. +printf '\nguards no planted directive reaches\n' + +# A PATH holding everything the script reaches before the tool loop, and jq only +# missing: bash for the shebang's `env bash`, dirname and git to find the repo. +mkdir -p "$work/bin" +for tool in bash dirname git golangci-lint; do + ln -sf "$(command -v "$tool")" "$work/bin/$tool" +done +out="$(PATH="$work/bin" "$work/main/scripts/check-nolint-linters.sh" 2>/dev/null)" || true +case "$out" in +*"NOLINT FAIL: jq is not on PATH"*) pass "a missing tool is named rather than crashed on" ;; +*) fail "a PATH without jq did not name it: $out" ;; +esac + +# The shell guard proves the file is non-empty, so the awk-side check can only +# fire on a file awk cannot read. Pointed at one that does not exist. +unreadable="$work/mutant-unreadable.sh" +mutate '-v enabled_file="$work/enabled.txt"' '-v enabled_file="$work/absent.txt"' >"$unreadable" +chmod +x "$unreadable" +cp "$unreadable" "$work/main/scripts/check-nolint-linters.sh" +plant main 'var _ = 1' +out="$(cd "$work/main" && ./scripts/check-nolint-linters.sh 2>/dev/null)" || true +case "$out" in +*"NOLINT FAIL: the enabled set came through empty"*) + pass "an unreadable enabled set is refused, not read as nothing enabled" + ;; +*) + fail "an unreadable enabled set did not fire the awk guard: $out" + ;; +esac + printf '\n' if ((failures > 0)); then printf '%d check(s) failed\n' "$failures" >&2