Skip to content

feat(go): hermetic golangci-lint linter aspect#901

Draft
b3cramer wants to merge 16 commits into
mainfrom
feat/golangci-lint-hermetic-aspect
Draft

feat(go): hermetic golangci-lint linter aspect#901
b3cramer wants to merge 16 commits into
mainfrom
feat/golangci-lint-hermetic-aspect

Conversation

@b3cramer

@b3cramer b3cramer commented Jun 8, 2026

Copy link
Copy Markdown
Member

Summary

Adds golangci-lint as a first-class rules_lint linter aspect for Go — the first Go linter in the ruleset (previously Go had only formatters). It runs the real golangci-lint binary as a hermetic, per-package, remote-cache-friendly Bazel action.

The hard part: golangci-lint is type-aware and loads packages through go/packages, which normally shells out to go list/go build — impossible in a hermetic sandbox. rules_go's stock gopackagesdriver doesn't help (it shells out to bazel query/bazel build at runtime). The solution here is a static, file-reading GOPACKAGESDRIVER: a rules_lint aspect reuses rules_go's go_pkg_info_aspect to pre-compute each package's metadata as declared action inputs, and a vendored fork of rules_go's driver read-half feeds golangci-lint from those staged files — no bazel-in-bazel, no network, no shared module cache.

What's included

  • Static driver (lint/private/gopackagesdriver_static/): vendored rules_go read-half (verbatim, with provenance headers) + a new main.go that injects stdlib ExportFile from precompiled .a archives, takes roots from GOPACKAGESDRIVER_ROOTS, and resolves go_deps imports by PkgPath. One local modification to packageregistry.go (clearly marked) for go_deps with empty Imports maps.
  • The aspect (lint/golangci_lint.bzl, lint_golangci_lint_aspect): visits go_library/go_binary/go_test; lints only the target package and consumes all dependencies (stdlib + external) as export data only; normalizes golangci-lint's SARIF URIs to workspace-relative paths.
  • Provisioning: golangci-lint v2.12.2 via @multitool, exposed as //lint:golangci_lint_bin.
  • Example + tests (examples/go/): wires the aspect via --config=lint; lint_tests assert a real govet finding on //src:hello (exit 1) and a clean case on //src:greeting (exit 0).
  • Docs: README supported-tools row.

Design decisions worth a reviewer's eye

  • Export-only dependencies — correct linter semantics (deps are compiled artifacts, not lint targets) and avoids false typecheck errors from dependency source that imports outside the Bazel graph.
  • GOMAXPROCS=1 mitigates a known x/tools gcimporter data race when decoding export data; documented as a pragmatic mitigation, not a hard guarantee.
  • Per-package action granularity gives incremental, remote-cacheable linting — editing one package does not invalidate another's lint result (verified).
  • Deferred: golangci-lint --fix patch mode (mutates in place rather than emitting a diff).

Test Plan

  • bazel test //lint/private/gopackagesdriver_static:gopackagesdriver_static_test (driver protocol unit test)
  • cd examples/go && bazel test //test:allgolangci_lint_finds_issue (exit 1) + golangci_lint_clean (exit 0)
  • Per-package cache reuse verified (changing one package leaves another's report byte-identical)
  • CI Linux (x86_64/arm64) — only verified locally on macOS arm64; the multitool lockfile includes Linux binaries but the Linux execution path is exercised by CI

🤖 Generated with Claude Code

b3cramer and others added 13 commits June 3, 2026 16:41
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copies the five JSON read + response-assembly source files from
@io_bazel_rules_go //go/tools/gopackagesdriver into
lint/private/gopackagesdriver_static/ and adds a go_library/go_binary
BUILD target. Adds golang.org/x/tools as a direct go.mod dep and exposes
org_golang_x_tools via MODULE.bazel use_repo. main.go is intentionally
absent (Task 2 adds it TDD).
Runs golangci-lint per Go package via a static GOPACKAGESDRIVER fed by
rules_go's go_pkg_info_aspect metadata. Dependencies (stdlib + external)
are consumed as export data only; only the target package is linted.
Stdlib ExportFile is injected from GoStdLib precompiled archives; the
driver resolves non-stdlib imports by PkgPath for go_deps with empty
Imports maps. SARIF URIs are normalized to workspace-relative paths.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
golangci-lint resolves a relative --output.sarif.path against the real
workspace (not the sandbox execroot), failing under sandboxing. Anchor it
at an absolute $PWD path and copy to the declared Bazel output. Adds the
example's .golangci.yml, tools/lint/linters.bzl, and --config=lint wiring;
//src:hello reports a real govet printf finding.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
golangci_lint_finds_issue asserts //src:hello exits 1 (govet printf), and
golangci_lint_clean asserts the clean //src:greeting library exits 0 (no
false positives). Verified per-package cache reuse: editing one package does
not invalidate another package's lint result.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Fix SARIF normalize docstring/comment to match the /execroot/<repo>/ logic
  the code actually uses; harden the no-execroot fallback to preserve '..'
  segments instead of silently dropping them.
- Document GOMAXPROCS=1 residual-race risk inline and in spike-findings.
- Record export-only deps, packageregistry modification, os.Exit(0), and SARIF
  path workarounds in the spike-findings note.
- Bump examples/go rules_go pin 0.52.0 -> 0.60.0 to match MVS resolution.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@aspect-workflows

aspect-workflows Bot commented Jun 8, 2026

Copy link
Copy Markdown

✨ Aspect Workflows Tasks

📅 Mon Jun 8 21:10:35 UTC 2026

✅ 3 successful tasks

  • ✅ test (test-root-bazel-7) · ⏱ 50s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (4/4 passed · 4 cached)
  • ✅ test (test-root-bazel-8) · ⏱ 3m 52s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (4/4 passed)
  • ✅ test (test-root-bazel-9) · ⏱ 4m 5s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (4/4 passed)

⏱ Last updated Mon Jun 8 21:14:42 UTC 2026 · 📊 GitHub API quota 1,875/15,000 (13% used, resets in 47m)
🚀 Powered by Aspect CLI (v2026.22.44)  |  Aspect Build · X · LinkedIn · YouTube

b3cramer and others added 3 commits June 8, 2026 14:23
The bzl_library macro generates a //lint:golangci_lint.doc_extract target that
requires bzl_library coverage for every loaded module, including rules_go's
go/private:providers.bzl and gopackagesdriver:aspect.bzl, which rules_go does
not expose. That failed analysis aborted 'aspect test //...'. Comment it out,
matching the existing ktlint/spotbugs/clang_tidy/cppcheck precedent.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The integration test asserted hello.go appears immediately after 'gofmt -w',
relying on it being the alphabetically-first Go file. The clean lint-test
library greeting.go (added in this branch) sorts before it, so update the
expected command to list both example files in order.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the design/plan/spike notes out of the tree (kept outside the repo);
drop the now-dangling reference from the golangci_lint.bzl docstring.
Comment thread lint/golangci_lint.bzl
# emitted a relative path) is returned as-is apart from stripping a leading
# "./". Empty input (no findings / empty file) yields a minimal valid empty
# SARIF document.
_SARIF_NORMALIZE_SCRIPT = """\

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be done in //tools/sarif to avoid a dependency on a non-hermetic system python?

// the upstream driver defined it; the static driver never receives this label
// as a root (stdlib packages are pulled in transitively by the import walk),
// but the symbol must exist for the vendored registry to compile.
const RulesGoStdlibLabel = "@io_bazel_rules_go//:stdlib"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we hardcoding repo names in here? :/

@b3cramer b3cramer marked this pull request as draft June 10, 2026 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants