From 3a9341b57bfe32728b95e7d6d7a18fe2ee2a8933 Mon Sep 17 00:00:00 2001 From: closer-claudio Date: Tue, 10 Nov 2026 00:33:37 -0800 Subject: [PATCH] lint go: every lane enters each command that is a module of its own The Go lanes ran at the repository root, and golangci-lint, vet, the tidiness check, go-licenses, NilAway, deadcode and govulncheck all stop at a nested go.mod: a command that is a module of its own, the shape build go builds from inside (cmd//go.mod), was never linted or scanned by the shared lanes. limen linted its own cmd/limen-lint-go through five lines in its root justfile, and any other repository with such a command linted it not at all. The list is one statement, the root then each cmd/ carrying a go.mod, built into an array; a tool module under .limen/tools is not project code and is not on it. lib.just holds it as go_modules_init for the commands handed to _per-platform, and a recipe body writes it out, since the shell lane lints a body with its interpolations opaque and must see the assignment the loop reads; a test holds the copies to the variable. Every lint lane, the vulnerability scan and the fix lane (the fixers, the formatter's convergence loop, tidiness) loop over it from inside each module, and _golangci-config renders one configuration per module, the baseline with that module's own .lint-go.yaml when it has one (the root's at golangci.yml as before, a command's beside it by name), so a command's carve-outs are its own. NilAway reads its flags and its mode per module. A lane names the module it enters, so a finding's relative path is locatable. _per-platform exports a leg's environment instead of prefixing it: a prefix binds to a simple command alone, and a leg's command is now a loop; the recipe is its own process, so nothing outlives it. The two one-line recipes, the formatter's check and deadcode, become bash blocks: the loop is bash, and a plain recipe line runs under sh. Found on the way: the mod lanes ended on the loop over the isolated tool modules, whose body was `[ -f ] && go ...`, so a repository with no such module ended the recipe on 1; `continue` instead. limen's root justfile loses lint-lint-go; its tests of the nested module stay, since the test lane still runs the root alone. book/linting-go.md states the rule and drops the "never enters" clause; book/errors-go.md's clause narrows to the modules the lane still does not enter. Tests, black-box through just on a fixture with a root module and a command module: a malformed build constraint in the command fails `lint go vet` naming cmd/nested; a sound command passes vet and mod. limen's own `just lint` now lints cmd/limen-lint-go through the shared lanes, on its own overlay. Closes https://github.com/farcloser/limen/issues/505 Co-Authored-By: Claude Fable 5.1 Signed-off-by: closer-claudio --- .justfile | 18 ++---- .limen/just/fix-go.just | 49 ++++++++++------ .limen/just/lib.just | 29 ++++++++- .limen/just/lint-go.just | 111 ++++++++++++++++++++++------------- .limen/just/security-go.just | 10 ++-- book/errors-go.md | 6 +- book/linting-go.md | 14 ++++- lint_go_test.go | 93 +++++++++++++++++++++++++++++ modules_test.go | 65 ++++++++++++++++++++ 9 files changed, 313 insertions(+), 82 deletions(-) create mode 100644 lint_go_test.go create mode 100644 modules_test.go diff --git a/.justfile b/.justfile index 6fc2be70..87091b38 100644 --- a/.justfile +++ b/.justfile @@ -12,27 +12,21 @@ export LIMEN_LINT_GO_BIN := LIMEN_BUILD / 'tools/limen-lint-go' default: @just --list -lint: build-lint-go do::lint::default do::lint::go::default do::lint::go::deadcode lint-lint-go +lint: build-lint-go do::lint::default do::lint::go::default do::lint::go::deadcode fix: build-lint-go do::fix::default do::fix::go::default test: do::test::go::default test-lint-go security: do::security::default # limen-lint-go is the second binary of limen's release (see .release-go.yaml), # a nested module so its YAML parser never enters limen's own go.mod. The -# shared Go lanes stop at the root module, so this repository builds, lints -# and tests it itself: the lint runs on the baseline rendered for that module -# with its own .lint-go.yaml, using the golangci-lint the lint go lane built, -# and the tests through the gotestsum the test go lane built. +# shared Go lint lanes enter it as they enter every command that is a module +# of its own, on the baseline rendered with its own .lint-go.yaml; this +# repository builds it first, since the lanes render through it, and runs its +# tests itself, through the gotestsum the test go lane built, which stops at +# the root module. build-lint-go: mkdir -p "$LIMEN_BUILD/tools" && GOOS='' GOARCH='' go -C cmd/limen-lint-go build -o "$LIMEN_BUILD/tools/" . -lint-lint-go: - go -C cmd/limen-lint-go mod tidy -diff - go -C cmd/limen-lint-go vet ./... - "$LIMEN_BUILD/tools/limen-lint-go" -C cmd/limen-lint-go render -o "$LIMEN_BUILD/golangci-lint-go.yml" - cd cmd/limen-lint-go && "$LIMEN_BUILD/tools/golangci-lint" run -c "$LIMEN_BUILD/golangci-lint-go.yml" ./... - cd cmd/limen-lint-go && "$LIMEN_BUILD/tools/golangci-lint" fmt --diff -c "$LIMEN_BUILD/golangci-lint-go.yml" - test-lint-go: cd cmd/limen-lint-go && "$LIMEN_BUILD/tools/gotestsum" -- -count=1 -timeout "${TEST_GO_TIMEOUT:-10m}" ./... diff --git a/.limen/just/fix-go.just b/.limen/just/fix-go.just index c628d9a4..7e79aba7 100644 --- a/.limen/just/fix-go.just +++ b/.limen/just/fix-go.just @@ -39,36 +39,47 @@ go_fixers := "wsl_v5,godot,tagalign,nolintlint" # runs and no other, since `go vet` honours neither the rendered lint # configuration nor `//nolint` and a fix must not fail on a finding lint # never reports (book/linting-go.md). -code: (_banner "fix go" "go") (_go-tool "golangci-lint" "github.com/golangci/golangci-lint/v2/cmd/golangci-lint") _golangci-config (_per-platform '"$LIMEN_BUILD/tools/golangci-lint" run --fix --issues-exit-code 0 --enable-only ' + go_fixers + ' -c "$LIMEN_BUILD/golangci.yml"') +code: (_banner "fix go" "go") (_go-tool "golangci-lint" "github.com/golangci/golangci-lint/v2/cmd/golangci-lint") _golangci-config (_per-platform go_modules_init + '; for module in "${modules[@]}"; do (cd "$module" && "$LIMEN_BUILD/tools/golangci-lint" run --fix --issues-exit-code 0 --enable-only ' + go_fixers + ' -c ' + golangci_config + ' ./...); done') #!/usr/bin/env bash set -euo pipefail - max=5 - pass=0 - # Check first: an already-formatted tree costs one --diff and no rewrite. - while ! "$LIMEN_BUILD/tools/golangci-lint" fmt --diff -c "$LIMEN_BUILD/golangci.yml" >/dev/null 2>&1; do - pass=$((pass + 1)) - if [ "${pass}" -gt "${max}" ]; then - echo "formatters did not converge after ${max} passes — they are fighting each other." >&2 - echo "The residual diff below reverses on the next pass; fix the formatter config." >&2 - "$LIMEN_BUILD/tools/golangci-lint" fmt --diff -c "$LIMEN_BUILD/golangci.yml" >&2 || true + # Every module the lint lane enters (go_modules_init in lib.just), each on its + # own rendered configuration, as `lint go` checks it. + modules=(.); for d in cmd/*/; do [ -f "${d}go.mod" ] || continue; modules+=("${d%/}"); done + for module in "${modules[@]}"; do + config={{ golangci_config }} + max=5 + pass=0 + # Check first: an already-formatted tree costs one --diff and no rewrite. + while ! (cd "$module" && "$LIMEN_BUILD/tools/golangci-lint" fmt --diff -c "$config" >/dev/null 2>&1); do + pass=$((pass + 1)) + if [ "${pass}" -gt "${max}" ]; then + echo "formatters did not converge after ${max} passes in ${module} — they are fighting each other." >&2 + echo "The residual diff below reverses on the next pass; fix the formatter config." >&2 + (cd "$module" && "$LIMEN_BUILD/tools/golangci-lint" fmt --diff -c "$config" >&2) || true + exit 1 + fi + (cd "$module" && "$LIMEN_BUILD/tools/golangci-lint" fmt -c "$config") + done + if ! go -C "$module" vet -asmdecl -buildtag ./...; then + echo "the fixers left ${module} not compiling: repair what they broke above, then run again." >&2 exit 1 fi - "$LIMEN_BUILD/tools/golangci-lint" fmt -c "$LIMEN_BUILD/golangci.yml" done - if ! go vet -asmdecl -buildtag ./...; then - echo "the fixers left a tree that does not compile: repair what they broke above, then run again." >&2 - exit 1 - fi -# Tidy go.mod/go.sum, for the module, for .limen/tools/ (the analyzers' module) and -# for each isolated tool module (.limen/tools//go.mod, see _go-tool). +# Tidy go.mod/go.sum, for every module (go_modules_init in lib.just), for +# .limen/tools/ (the analyzers' module) and for each isolated tool module +# (.limen/tools//go.mod, see _go-tool). mod: (_banner "fix go" "mod") #!/usr/bin/env bash set -euo pipefail - go mod tidy + modules=(.); for d in cmd/*/; do [ -f "${d}go.mod" ] || continue; modules+=("${d%/}"); done + for module in "${modules[@]}"; do + go -C "$module" mod tidy + done go -C .limen/tools mod tidy for m in .limen/tools/*/go.mod; do - [ -f "$m" ] && go -C "${m%/go.mod}" mod tidy + [ -f "$m" ] || continue + go -C "${m%/go.mod}" mod tidy done # Update every dependency to its latest version, then tidy. diff --git a/.limen/just/lib.just b/.limen/just/lib.just index fba1ca0e..f22c37c5 100644 --- a/.limen/just/lib.just +++ b/.limen/just/lib.just @@ -50,10 +50,27 @@ _per-platform +cmd: platforms=(darwin/arm64 linux/amd64 linux/arm64 windows/amd64 windows/arm64) for platform in "${platforms[@]}"; do echo "→ GOOS=${platform%/*} GOARCH=${platform#*/}" - CGO_ENABLED=0 GOOS="${platform%/*}" GOARCH="${platform#*/}" {{ cmd }} + # Exported, not prefixed: a prefix binds to a simple command alone, + # and a leg's command may be a loop over modules. The recipe is its + # own process, so nothing outlives the loop. + export CGO_ENABLED=0 GOOS="${platform%/*}" GOARCH="${platform#*/}" + {{ cmd }} done fi +# The Go modules the Go lanes analyze, built into the array `modules` by this +# statement: the root, then every command that is a module of its own +# (cmd//go.mod, the shape build-go.just builds from inside). A tool +# module under .limen/tools/ is not project code and is not on it. For a +# command a lane hands _per-platform; a recipe body writes the statement +# out instead, since the shell lane lints a body with its interpolations +# opaque and must see the assignment the loop reads. +go_modules_init := 'modules=(.); for d in cmd/*/; do [ -f "${d}go.mod" ] || continue; modules+=("${d%/}"); done' + +# The rendered golangci configuration of the module `$module` names in such a +# loop: the root's at golangci.yml, as before; a command's beside it by name. +golangci_config := '"$LIMEN_BUILD/golangci$([ "$module" = . ] || echo "-${module##*/}").yml"' + # The native build of one .limen/tools/go.mod tool into .build/tools/, shared by # _go-tool and the tools recipes (`{{ go_tool_build }} `). GOOS/GOARCH # cleared: a per-platform leg would cross-compile the tool itself (exec format @@ -172,5 +189,13 @@ _vulndb: # LIMEN_LINT_GO_BIN is for the limen repository itself, which builds it from # its own tree (same idea as LIMEN_BIN in lint.just). _golangci-config: + #!/usr/bin/env bash + set -euo pipefail ${LIMEN_LINT_GO_BIN:-limen-lint-go} check "$LIMEN_BUILD/tools/golangci-lint" - ${LIMEN_LINT_GO_BIN:-limen-lint-go} render -o "$LIMEN_BUILD/golangci.yml" + # One configuration per module: the baseline with that module's own + # .lint-go.yaml, so a command that is a module of its own carries its + # own carve-outs. + modules=(.); for d in cmd/*/; do [ -f "${d}go.mod" ] || continue; modules+=("${d%/}"); done + for module in "${modules[@]}"; do + ${LIMEN_LINT_GO_BIN:-limen-lint-go} -C "$module" render -o {{ golangci_config }} + done diff --git a/.limen/just/lint-go.just b/.limen/just/lint-go.just index ff56b9c1..00650205 100644 --- a/.limen/just/lint-go.just +++ b/.limen/just/lint-go.just @@ -20,12 +20,21 @@ default: code vet mod licenses nilaway # .golangci.yml), then once per supported platform (see _per-platform: # platform-specific files are only analyzed for the GOOS/GOARCH that builds # them). Built with the toolchain it analyzes, it cannot skew from it — the -# reason the prebuilt binary and its lockstep check are gone. -# The formatter is build-graph-independent and runs once. The last step -# enforces the suppression doctrine (book/linting-go.md, "silencing a -# finding"): a finding is silenced by its rule, never by its linter. -code: (_banner "lint go" "go") (_go-tool "golangci-lint" "github.com/golangci/golangci-lint/v2/cmd/golangci-lint") _golangci-config (_per-platform '"$LIMEN_BUILD/tools/golangci-lint" run -c "$LIMEN_BUILD/golangci.yml"') _suppression-check - "$LIMEN_BUILD/tools/golangci-lint" fmt --diff -c "$LIMEN_BUILD/golangci.yml" +# reason the prebuilt binary and its lockstep check are gone. Every module +# (see go_modules_init in lib.just: the root and each command that is one of its +# own), from inside it, on its own rendered configuration: golangci stops at +# a nested go.mod, so a module not entered is a module not linted. +# The formatter is build-graph-independent and runs once per module. The +# last step enforces the suppression doctrine (book/linting-go.md, +# "silencing a finding"): a finding is silenced by its rule, never by its +# linter. +code: (_banner "lint go" "go") (_go-tool "golangci-lint" "github.com/golangci/golangci-lint/v2/cmd/golangci-lint") _golangci-config (_per-platform go_modules_init + '; for module in "${modules[@]}"; do [ "$module" = . ] || echo "→ $module"; (cd "$module" && "$LIMEN_BUILD/tools/golangci-lint" run -c ' + golangci_config + ' ./...); done') _suppression-check + #!/usr/bin/env bash + set -euo pipefail + modules=(.); for d in cmd/*/; do [ -f "${d}go.mod" ] || continue; modules+=("${d%/}"); done + for module in "${modules[@]}"; do + (cd "$module" && "$LIMEN_BUILD/tools/golangci-lint" fmt --diff -c {{ golangci_config }}) + done # Polices the shape the book requires (linting-go.md, "silencing a # finding"): nolintlint checks a directive's form, not which linter it names, @@ -95,23 +104,30 @@ _suppression-check: # neither the rendered configuration nor //nolint, so any analyzer that overlaps with # golangci's govet would re-report findings a project deliberately silenced, # and the same exception would then have to be written twice. Every -# supported platform, like the rest (see _per-platform in lib.just); vet -# type-checks without an assembler or C toolchain, so each leg is a few -# seconds. Note `go test` is not a backstop here: the vet subset it runs -# excludes both analyzers. -vet: (_banner "lint go" "vet") (_per-platform "go vet -asmdecl -buildtag ./...") +# supported platform, like the rest (see _per-platform in lib.just), and +# every module (go_modules_init); vet type-checks without an assembler or C +# toolchain, so each leg is a few seconds. Note `go test` is not a backstop +# here: the vet subset it runs excludes both analyzers. +vet: (_banner "lint go" "vet") (_per-platform go_modules_init + '; for module in "${modules[@]}"; do [ "$module" = . ] || echo "→ $module"; go -C "$module" vet -asmdecl -buildtag ./...; done') -# go.mod/go.sum tidiness, for the module and for .limen/tools/ (the Go-built tools' -# own module — see _go-tool in lib.just). Fails printing the diff; `just do fix -# go mod` repairs. +# go.mod/go.sum tidiness, for every module (go_modules_init) and for .limen/tools/ +# (the Go-built tools' own module — see _go-tool in lib.just). Fails printing +# the diff; `just do fix go mod` repairs. mod: (_banner "lint go" "mod") #!/usr/bin/env bash set -euo pipefail - go mod tidy -diff + modules=(.); for d in cmd/*/; do [ -f "${d}go.mod" ] || continue; modules+=("${d%/}"); done + for module in "${modules[@]}"; do + [ "$module" = . ] || echo "→ $module" + go -C "$module" mod tidy -diff + done go -C .limen/tools mod tidy -diff # The isolated tool modules too (.limen/tools//go.mod, see _go-tool). + # `continue`, not `&&`: the loop's status is the recipe's, and a + # repository with no isolated tool module would otherwise end on 1. for m in .limen/tools/*/go.mod; do - [ -f "$m" ] && go -C "${m%/go.mod}" mod tidy -diff + [ -f "$m" ] || continue + go -C "${m%/go.mod}" mod tidy -diff done # NilAway (go.uber.org/nilaway), Uber's nil-panic analyzer: not a golangci-lint @@ -127,29 +143,38 @@ mod: (_banner "lint go" "mod") nilaway: (_banner "lint go" "nilaway") (_go-tool "nilaway" "go.uber.org/nilaway/cmd/nilaway") #!/usr/bin/env bash set -euo pipefail - flags=$(${LIMEN_LINT_GO_BIN:-limen-lint-go} flags nilaway) - mode=$(${LIMEN_LINT_GO_BIN:-limen-lint-go} mode nilaway) - fail=0 - # The platform loop of _per-platform, inline: a leg's failure must not - # stop the others, and the verdict depends on the mode. - if [ "{{ go_cgo }}" = "1" ]; then - echo "CGO_ENABLED=1: cross-platform runs are impossible (foreign-platform package" >&2 - echo "loading needs that platform's C toolchain) — native platform only." >&2 - # shellcheck disable=SC2086 # the flags are a list by contract - "$LIMEN_BUILD/tools/nilaway" $flags ./... || fail=1 - else - platforms=(darwin/arm64 linux/amd64 linux/arm64 windows/amd64 windows/arm64) - for platform in "${platforms[@]}"; do - echo "→ GOOS=${platform%/*} GOARCH=${platform#*/}" + verdict=0 + # Every module (go_modules_init), each with the flags and the mode of its own + # overlay; the platform loop of _per-platform, inline: a leg's failure + # must not stop the others, and the verdict depends on the mode. + modules=(.); for d in cmd/*/; do [ -f "${d}go.mod" ] || continue; modules+=("${d%/}"); done + for module in "${modules[@]}"; do + flags=$(${LIMEN_LINT_GO_BIN:-limen-lint-go} -C "$module" flags nilaway) + mode=$(${LIMEN_LINT_GO_BIN:-limen-lint-go} -C "$module" mode nilaway) + fail=0 + [ "$module" = . ] || echo "→ $module" + if [ "{{ go_cgo }}" = "1" ]; then + echo "CGO_ENABLED=1: cross-platform runs are impossible (foreign-platform package" >&2 + echo "loading needs that platform's C toolchain) — native platform only." >&2 # shellcheck disable=SC2086 # the flags are a list by contract - CGO_ENABLED=0 GOOS="${platform%/*}" GOARCH="${platform#*/}" "$LIMEN_BUILD/tools/nilaway" $flags ./... || fail=1 - done - fi - if [ "$fail" = 1 ] && [ "$mode" != blocking ]; then - echo "nilaway: findings above are informational for this project (nilaway.blocking is false in .lint-go.yaml)" >&2 - exit 0 - fi - exit "$fail" + (cd "$module" && "$LIMEN_BUILD/tools/nilaway" $flags ./...) || fail=1 + else + platforms=(darwin/arm64 linux/amd64 linux/arm64 windows/amd64 windows/arm64) + for platform in "${platforms[@]}"; do + echo "→ GOOS=${platform%/*} GOARCH=${platform#*/}" + # shellcheck disable=SC2086 # the flags are a list by contract + (cd "$module" && CGO_ENABLED=0 GOOS="${platform%/*}" GOARCH="${platform#*/}" "$LIMEN_BUILD/tools/nilaway" $flags ./...) || fail=1 + done + fi + if [ "$fail" = 1 ]; then + if [ "$mode" = blocking ]; then + verdict=1 + else + echo "nilaway: findings above are informational for ${module} (nilaway.blocking is false in its .lint-go.yaml)" >&2 + fi + fi + done + exit "$verdict" # Dependency license compliance, once per supported platform (see _per-platform in # lib.just: each platform pulls its own dependencies). The allowed licenses @@ -167,11 +192,17 @@ nilaway: (_banner "lint go" "nilaway") (_go-tool "nilaway" "go.uber.org/nilaway/ # graph, the repository's own LICENSE is the `license` rule's, and go-licenses # classifies a license by its text, so the Closed-source notice limen writes # failed every package of such a module. -licenses: (_banner "lint go" "licenses") (_go-tool "go-licenses" "github.com/forkcloser/go-licenses/cmd/go-licenses") (_per-platform 'GOROOT="$(go env GOROOT)" "$LIMEN_BUILD/tools/go-licenses" check --include_tests --ignore "$(go list -m)" $(${LIMEN_LINT_GO_BIN:-limen-lint-go} flags licenses) ./...') +licenses: (_banner "lint go" "licenses") (_go-tool "go-licenses" "github.com/forkcloser/go-licenses/cmd/go-licenses") (_per-platform go_modules_init + '; for module in "${modules[@]}"; do [ "$module" = . ] || echo "→ $module"; (cd "$module" && GOROOT="$(go env GOROOT)" "$LIMEN_BUILD/tools/go-licenses" check --include_tests --ignore "$(go list -m)" $(${LIMEN_LINT_GO_BIN:-limen-lint-go} -C . flags licenses) ./...); done') # Unreachable functions, by whole-program call-graph analysis (deeper than the # per-package `unused` linter in golangci). -test keeps library-only repos # analyzable (test binaries serve as roots) and stops dead code from hiding # behind test-only callers. Informational: native only, not in the default set. deadcode: (_banner "lint go" "deadcode") (_go-tool "deadcode" "golang.org/x/tools/cmd/deadcode") - "$LIMEN_BUILD/tools/deadcode" -test ./... + #!/usr/bin/env bash + set -euo pipefail + modules=(.); for d in cmd/*/; do [ -f "${d}go.mod" ] || continue; modules+=("${d%/}"); done + for module in "${modules[@]}"; do + [ "$module" = . ] || echo "→ $module" + (cd "$module" && "$LIMEN_BUILD/tools/deadcode" -test ./...) + done diff --git a/.limen/just/security-go.just b/.limen/just/security-go.just index e357d576..8648cc12 100644 --- a/.limen/just/security-go.just +++ b/.limen/just/security-go.just @@ -12,11 +12,13 @@ import 'lib.just' default: vuln -# Known-vulnerability scan of the module's dependency graph against the Go +# Known-vulnerability scan of each module's dependency graph against the Go # vulnerability database (fetched once, see _vulndb in lib.just), once per # supported platform (see _per-platform in lib.just: call paths and -# dependencies differ by platform). Reports what the module reaches, not what -# it merely imports: a finding is a call path to cut or a bump to take. No +# dependencies differ by platform) and for every module the lint lanes enter +# (go_modules_init: the root and each command that is a module of its own), +# since govulncheck stops at a nested go.mod. Reports what a module reaches, +# not what it merely imports: a finding is a call path to cut or a bump to take. No # go.mod at all: says so and passes, so `just do security` is safe in every # repository — the guard is a recipe of its own because a dependency cannot # short-circuit the ones after it, and the tool build would fail first. @@ -29,4 +31,4 @@ vuln: (_banner "security go" "vuln") fi just do security go _vuln -_vuln: (_go-tool "govulncheck" "golang.org/x/vuln/cmd/govulncheck") _vulndb (_per-platform '"$LIMEN_BUILD/tools/govulncheck" -db "$(cat "$LIMEN_BUILD/vulndb.url")" ./...') +_vuln: (_go-tool "govulncheck" "golang.org/x/vuln/cmd/govulncheck") _vulndb (_per-platform go_modules_init + '; for module in "${modules[@]}"; do [ "$module" = . ] || echo "→ $module"; (cd "$module" && "$LIMEN_BUILD/tools/govulncheck" -db "$(cat "$LIMEN_BUILD/vulndb.url")" ./...); done') diff --git a/book/errors-go.md b/book/errors-go.md index b5bceb60..d7d209cb 100644 --- a/book/errors-go.md +++ b/book/errors-go.md @@ -54,8 +54,10 @@ unwraps to the old one (`ErrCorrupt` in erofs matches `ErrInvalid` too), so ever reverse, moving faults out from under a sentinel consumers test, is. **Where the lint does not reach, the rule still does.** golangci-lint skips generated -files and the lint lane does not enter a nested module, so a bare `fmt.Errorf` there is -never reported: the binding's errors above were 59 such sites. The fix goes in the +files, and the lint lane enters a nested module only when it is a command under `cmd/` +([linting Go](./linting-go.md), "Every module, from inside it"), so a bare `fmt.Errorf` in a +generated file or in a module elsewhere is never reported: the binding's errors above were +59 such sites. The fix goes in the template or the generator, then `just bindings`, and the nested tool gets the same sentinels by hand. A dynamic error is right in one place only: a test fixture, where the error is the fake's own and compared by identity; err113 stays on in tests in the diff --git a/book/linting-go.md b/book/linting-go.md index bb43b60d..225e2a4c 100644 --- a/book/linting-go.md +++ b/book/linting-go.md @@ -76,6 +76,15 @@ recipe asks `limen-lint-go disabled revive` for the rules the rendered configura and fails on a directive naming one: remove it, or move the finding to the linter that owns the check now. +**Every module, from inside it.** Each Go lint lane runs on the root module and on every +command that is a module of its own (`cmd//go.mod`, the shape `build go` builds from +inside), because golangci-lint, vet and the analyzers stop at a nested `go.mod`: a module +not entered is a module not linted. The configuration is rendered per module, the baseline +with that module's own `.lint-go.yaml` when it has one, so a command's carve-outs are its +own; the tidiness, license and NilAway lanes, the vulnerability scan of `just security` and +`just do fix go` follow the same list. A tool module under `.limen/tools/` is not project +code and is not on it; the test lane still runs the root module alone. + **What limen enforces** (the `lintgo` rule, Go modules only): `.limen/lint-go.yaml` is content-pinned, created when missing and overwritten when drifted; a missing root `.lint-go.yaml` is seeded; a root golangci-lint configuration fails the check and is an advisory on fix, since @@ -103,9 +112,8 @@ One warning is not a verdict: golangci-lint's "Skipped 0 issues by rules" for a *formatter* exclusion (gci, gofumpt, golines, goimports on a generated file) says only that the lint pass found nothing there, since formatters report through the separate format pass. The exclusion is live when removing it makes `just do lint go` rewrite the -file; a generated file in a nested module, which the lint pass never enters, shows the -same warning for the same reason. Test by removal before calling one dead (seen on -go-graphviz's `bind.go` and `nori.pb.go`). +file. Test by removal before calling one dead (seen on go-graphviz's `bind.go` and +`nori.pb.go`). A dead silence is reported by nolintlint for a linter that runs, and by `just do lint go` for a disabled linter, a disabled revive rule, and a `#nosec` in a test file, which gosec diff --git a/lint_go_test.go b/lint_go_test.go new file mode 100644 index 00000000..e0360831 --- /dev/null +++ b/lint_go_test.go @@ -0,0 +1,93 @@ +package limen_test + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// goModule is a module of one package, its go directive the one this +// repository's lanes run under. +func goModule(t *testing.T, dir, module, source string) { + t.Helper() + + if err := os.MkdirAll(dir, 0o750); err != nil { + t.Fatal(err) + } + + for name, content := range map[string]string{ + "go.mod": "module " + module + "\n\ngo 1.26.0\n", + "main.go": source, + } { + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } +} + +// lintRepo makes a repository carrying this checkout's .limen/just/ recipes, a +// root module and a command that is a module of its own under cmd/, whose +// main.go is source, and returns its path. +func lintRepo(t *testing.T, source string) string { + t.Helper() + + dir := t.TempDir() + if err := os.CopyFS(filepath.Join(dir, ".limen", "just"), os.DirFS(filepath.Join(".limen", "just"))); err != nil { + t.Fatal(err) + } + + if err := os.WriteFile( + filepath.Join(dir, ".justfile"), + []byte("import '.limen/just/main.just'\n"), + 0o600, + ); err != nil { + t.Fatal(err) + } + + // The tools module every repository carries, empty: the mod lane tidies it. + if err := os.MkdirAll(filepath.Join(dir, ".limen", "tools"), 0o750); err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(filepath.Join(dir, ".limen", "tools", "go.mod"), + []byte("module example.com/root/.limen/tools\n\ngo 1.26.0\n"), 0o600); err != nil { + t.Fatal(err) + } + + goModule(t, dir, "example.com/root", "package main\n\nfunc main() {}\n") + goModule(t, filepath.Join(dir, "cmd", "nested"), "example.com/root/cmd/nested", source) + + return dir +} + +// TestLintGoVetEntersNestedModule: `lint go vet` runs in a command that is a +// module of its own, not only in the root module, so a finding there fails +// the lane and names the module. +func TestLintGoVetEntersNestedModule(t *testing.T) { + t.Parallel() + + // A build constraint vet's buildtag analyzer refuses: `linux &&` with no + // right-hand term. + broken := "//go:build linux &&\n\npackage main\n\nfunc main() {}\n" + + out, err := command(t, lintRepo(t, broken), nil, "just", "do", "lint", "go", "vet").CombinedOutput() + if err == nil { + t.Fatalf("a malformed build constraint in cmd/nested passed lint go vet:\n%s", out) + } + + if !strings.Contains(string(out), "nested") { + t.Errorf("the failure does not name the nested module:\n%s", out) + } +} + +// TestLintGoVetAndModPassNestedModule: a sound nested module passes the vet +// and tidiness lanes, so entering it reports nothing of its own. +func TestLintGoVetAndModPassNestedModule(t *testing.T) { + t.Parallel() + + dir := lintRepo(t, "package main\n\nfunc main() {}\n") + + run(t, dir, nil, "just", "do", "lint", "go", "vet") + run(t, dir, nil, "just", "do", "lint", "go", "mod") +} diff --git a/modules_test.go b/modules_test.go new file mode 100644 index 00000000..6e27de70 --- /dev/null +++ b/modules_test.go @@ -0,0 +1,65 @@ +package limen_test + +import ( + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// TestGoModuleStatementCopiesAgree: the statement that lists the Go modules a +// lane enters is lib.just's go_modules_init for the commands handed to +// _per-platform and written out in every recipe body, since the shell lane +// lints a body with its interpolations opaque. The copies must be the one +// statement. +func TestGoModuleStatementCopiesAgree(t *testing.T) { + t.Parallel() + + lib, err := os.ReadFile(filepath.Join(".limen", "just", "lib.just")) + if err != nil { + t.Fatal(err) + } + + match := regexp.MustCompile(`(?m)^go_modules_init := '(.*)'$`).FindStringSubmatch(string(lib)) + if match == nil { + t.Fatal("lib.just has no go_modules_init") + } + + want := match[1] + copies := 0 + + files, err := filepath.Glob(filepath.Join(".limen", "just", "*.just")) + if err != nil { + t.Fatal(err) + } + + for _, file := range files { + data, err := os.ReadFile(file) + if err != nil { + t.Fatal(err) + } + + for i, line := range strings.Split(string(data), "\n") { + if !strings.Contains(line, "modules=(.)") || strings.Contains(line, "go_modules_init :=") { + continue + } + + copies++ + + if got := strings.TrimSpace(line); got != want { + t.Errorf( + "%s:%d writes the module statement as\n %s\nlib.just's go_modules_init is\n %s", + file, + i+1, + got, + want, + ) + } + } + } + + if copies == 0 { + t.Error("no recipe body writes the module statement out") + } +}