feat(plugin-go): per-package go vet/lint with serialized facts#135
feat(plugin-go): per-package go vet/lint with serialized facts#135raphaelvigee wants to merge 6 commits into
Conversation
In-sandbox build of heph-govet: proven ✅ (with a caveat)Verified end-to-end that heph can build Caveat — cold build is ~50 min (3033s, no heph cache). heph caches per-package so subsequent builds are fast, but first-run lint latency is brutal. Recommendation for productization:
Coverage as-is: unit (spec builders, report parser, gate, config wiring), provider resolution (real Infra notes for reviewers: needs an isolated |
Add a `lint` target that runs go/analysis (the `go vet` analyzer set) per package, with nogo-style serialized facts so interprocedural analyzers keep full fidelity across first-party package boundaries — something golangci-lint's whole-graph, in-memory-facts model cannot give a per-package build cache. Approach: instead of bending golangci-lint, run x/tools' unitchecker — the same per-unit driver `go vet` uses. Its cfg carries dep export data (PackageFile) for type info and dep facts (PackageVetx) in, and writes this package's facts to VetxOutput. heph's content-hash-of-inputs caching then makes a package re-lint only when its own sources, a dep's archive, or a dep's facts change. Pieces: - tools/heph-govet: the unitchecker binary (go vet analyzer set). Add any *analysis.Analyzer (golangci native linters, nilaway) + rebuild; facts flow automatically. Built as an ordinary first-party main target. - go_lint driver (`_lint` target): builds the unitchecker cfg, runs heph-govet with -json, writes lint.facts (consumed by dependents) + lint-report.json. Exits 0 even with findings so facts are always produced and cached. - go_lint_gate driver (`lint` target): consumes _lint's report and fails the build on findings. Keeps fact production (and caching) separate from enforcement. - provider: lint/_lint dispatch, facts deps wired between first-party packages (std/thirdparty contribute export-data only). Facts flow first-party-only by design, bounding the graph: std/thirdparty give type info via their build_lib archives but are not linted. Tested: unit tests for both spec builders, the report parser, and gate wiring; provider resolution tests (real `go list`); the unitchecker binary's -json / facts behavior verified by smoke test. 314 plugin-go tests pass. Remaining: building heph-govet through heph (`//tools/heph-govet:build`) exercises the x/tools thirdparty download in-sandbox; the end-to-end run test depends on that. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
heph-govet now ships the same upstream analyzers golangci-lint curates (go vet passes + staticcheck/gosimple/stylecheck) and selects among them from a `.golangci.yml`, so the lint UX stays close to stock golangci while keeping the per-package facts + caching model. golangci-lint hides its analyzers (goanalysis.Linter.getAnalyzers is unexported) and runs them through its own in-memory runner that doesn't serialize facts — so it can't be reused as a library for this. But golangci doesn't own the analyzers; it curates upstream go/analysis packages that export theirs. heph-govet imports those directly (registry.go) and parses `.golangci.yml` itself (config.go: linters default/enable/disable), avoiding any dependency on golangci-lint. - registry.go: golangci linter-name -> upstream analyzers. Extend by importing the upstream package + adding an entry. - config.go: minimal `.golangci.yml` (v2) parse + analyzer selection. Unknown linters fail loudly rather than silently dropping. - main.go: config path via HEPH_GOVET_GOLANGCI_CONFIG; run selected analyzers under unitchecker. - driver/provider: `.golangci.yml` at the workspace root is wired as a hashed `config` input and passed via env, so a config edit re-lints. Absent -> the standard set. Not supported in the per-package model (documented): formatters (gofmt etc., not analyzers), `unused` (whole-program), and `//nolint` / exclude-rules (golangci runner features) — the latter is a planned gate-side follow-up. Verified: heph-govet smoke tests (gosimple S1002 + stylecheck ST1000 fire under unitchecker with -json exit 0; 95 staticcheck analyzers wired; unknown linter fails). 14 driver_lint unit tests, prod clippy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Apply golangci-lint's two runner-level suppression features at analysis time by wrapping each selected analyzer's Pass.Report: - //nolint[:l1,l2] directives — trailing form covers its own line; own-line form also covers the next line; bare //nolint covers all linters. Matched against the golangci linter name (e.g. //nolint:gosimple suppresses S1002). - linters.exclusions.rules — per-rule linter scoping + path/text regexes. Only analyzers outside the selection's Requires-closure are wrapped, so wrapping never causes an analyzer to run both wrapped and as a dependency (double-emit). The report heph-govet writes is therefore already filtered; the gate target needs no change. Not parsed: exclusion presets and rules[].source. Verified: Go unit tests for scanNolint + suppressed (nolint scoping, exclude text/path/linter scoping); smoke tests confirm //nolint:gosimple and a text exclude-rule drop S1002 while ST1000 survives, and a mis-scoped rule does not. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a user-facing `lint-fix` target (driver `go_lint_fix`) that rewrites a package's Go sources with go/analysis suggested fixes, via `codegen=in_place`. It reuses the `_lint` analyze target's `-json` report — which already carries each diagnostic's `suggested_fixes` (byte-offset text edits) — so it runs no analysis of its own and shares `_lint`'s per-package cache. The report plus the package's own `.go` sources are the inputs; the rewritten sources are the outputs, declared `CodegenMode::InPlace` so the engine writes them back over the tracked source files on a top-level run (and reaches a fixpoint on re-run). Those sources are unstamped tracked files, so the copy-ownership in_place guard does not apply. Application is purely mechanical, split into two pure, unit-tested helpers: - report_edits: parse the report into edits grouped by source basename (the report's filename is an absolute sandbox path; a package's Go files are flat, so basenames are unique). Only the first suggested fix of each diagnostic is taken, matching the go/analysis auto-fix convention. - apply_edits: sort edits by offset, drop out-of-range or overlapping ones (earliest wins), and splice in a single pass. Offsets line up because `_lint` and `lint-fix` stage byte-identical sources. Sources are staged as writable copies (the default, non-read-only stage path), so the in-place overwrite can never touch a shared cache blob. Provider: `lint-fix` is listed for first-party packages and resolved through the `_golist` machinery like the gate. `.golangci.yml` linter selection flows in through `_lint`, so the fix target needs no config wiring. Also sets `TargetSpec.approval` on the go_lint spec builders to their default, adapting to the field added on master. Tests: report_edits/apply_edits (overlap, insertion, out-of-range, first-fix- only), the spec builder, an async parse() check asserting in-place pkg-relative outputs, and a provider resolution test over real `go list`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
626e0ee to
725ead7
Compare
…tters Two additions to close the gap between this per-package driver and golangci-lint. ## linters.settings Parse `linters.settings` and apply per-linter configuration: - govet: `enable`/`disable`/`enable-all`/`disable-all` select which vet analyzers run (over a superset that adds the off-by-default passes shadow, fieldalignment, nilness, sortslice, unusedwrite); `settings.<analyzer>.<flag>` sets analyzer flags (e.g. printf.funcs, shadow.strict). Flags are set via Analyzer.Flags.Set before unitchecker.Main — since our argv carries no analyzer flags, the values survive unitchecker's own flag parse (which only overrides flags present on the command line), so no fork is needed. - honnef staticcheck/gosimple/stylecheck: `checks` selects which checks run (patterns "all"/"none"/"-SA1000"/globs), applied by filtering the analyzer set because honnef analyzers do not self-disable on Config.Checks. `initialisms` + `dot-import-whitelist` + `http-status-code-whitelist` are honored by overriding honnef's `config.Analyzer.Run` in place to return a Config built from the yaml (merged over honnef's defaults) — no staticcheck.conf on disk. Unknown analyzer/flag names fail loudly. ## Formatters heph-govet doubles as the formatter driver in a `-format` mode (a whole-file rewrite path, entirely separate from unitchecker), so formatting reuses the one hermetic Go binary instead of paying a second ~50-min tool build. Backends: gofmt (go/format + golangci/gofmt for -s + rewrite rules), gofumpt, and goimports (FormatOnly — sort/group/remove; adding imports needs the module graph and would break the hermetic sandbox, so it is not attempted). Selection + settings come from the `formatters:` block; absent → gofmt. Two per-package targets mirror lint/lint-fix: - `format-fix` (`go_format`) rewrites sources and declares them codegen=in_place, so the engine writes them back over the tracked files; - `format` (`go_format_check`) runs `-format -check` and fails on any unformatted file, writing nothing. Tests: Go unit tests for govet resolution/flags, honnef check filtering + config override, and each formatter (reformat, -s simplify, gofumpt extra rules, goimports sort, idempotence); Rust unit tests for the format spec builders and a provider resolution test over real `go list`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Passing a package's every .go path on argv overflows the OS argv limit (ARG_MAX) for packages with thousands of files. heph-govet -format now expands `@listfile` arguments (one path per line, blank lines skipped); the go_format / go_format_check drivers write the staged file list to a sandbox response file and pass a single `@<file>` instead of one arg per source. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
Per-package Go
linttarget with nogo-style serialized facts — interprocedural analyzers (printf wrappers, lostcancel, …) keep full fidelity across first-party package boundaries, while caching per package. Linter selection + suppression via.golangci.yml.Why not golangci-lint directly
golangci-lint loads the whole graph in one process and keeps go/analysis facts in memory only — no fact-file export, incompatible with a per-package build cache. It also hides its analyzers (
goanalysis.Linter.getAnalyzersis unexported) and its runner doesn't serialize facts, so it can't be reused as a library here.But golangci doesn't own the analyzers — it curates upstream
go/analysispackages that export theirs.Approach
x/toolsunitchecker (the per-unit drivergo vetuses): dep type info viaPackageFile, dep facts viaPackageVetx, own facts out viaVetxOutput.heph-govetimports the same upstream analyzers golangci curates —go vetpasses + honnefstaticcheck/gosimple/stylecheck— and parses.golangci.ymlitself (linters.default/enable/disable,linters.exclusions.rules) to select + suppress. No dependency on golangci-lint..golangci.ymlchange.Targets
_lint(go_lint) — runs heph-govet, writeslint.facts(consumed by dependents) +lint-report.json. Exits 0 even with findings so facts always cache.lint(go_lint_gate) — consumes_lint's report, fails on findings.lint-fix(go_lint_fix) — applies the report's suggested fixes back into source viacodegen=in_place.format(go_format_check) — fails on any unformatted file (writes nothing).format-fix(go_format) — reformats sources and writes back viacodegen=in_place.Facts flow first-party-only (bounds the graph); std/thirdparty give type info via their
build_libarchives..golangci.ymlat the workspace root is a hashedconfiginput, passed viaHEPH_GOVET_GOLANGCI_CONFIG. Absent → the standard set..golangci.ymlcoveragelinters.default/enable/disable.linters.settings— govet (enable/disable/enable-all/disable-all+ per-analyzer flags likeprintf.funcs,shadow.strict); honnefstaticcheck/gosimple/stylecheck(checkspatterns;initialisms,dot-import-whitelist,http-status-code-whitelist). govet flags are set viaAnalyzer.Flags.Setbeforeunitchecker.Main(they survive its flag parse — our argv carries none); honnef knobs overrideconfig.Analyzer.Runin place;checksfilters the analyzer set.//nolint[:l],linters.exclusions.rules(linter/path/text).formatters—enable+settingsfor gofmt/gofumpt/goimports.Suppression (honored at analysis time)
//nolint[:l1,l2]— trailing covers its line; own-line also covers the next line; bare covers all. Matched against the golangci linter name (//nolint:gosimplesuppressesS1002).linters.exclusions.rules— per-rule linter scoping + path/text regexes.Implemented by wrapping each analyzer's
Pass.Report, so the report is already filtered (the gate needs no change).Fix (
lint-fix)lint-fix(go_lint_fix) rewrites a package's Go sources with go/analysis suggested fixes and writes them back over the tracked files viacodegen=in_place._lint's-jsonreport — which already carries each diagnostic'ssuggested_fixes(byte-offset text edits) — so it runs no analysis of its own and shares_lint's per-package cache. Inputs: that report + the package's own.gosources. Outputs: the rewritten sources, declaredCodegenMode::InPlace, so the engine writes them back on a top-level run (and reaches a fixpoint on re-run).report_edits(parse → edits grouped by source basename; first suggested fix per diagnostic, the go/analysis auto-fix convention) andapply_edits(sort by offset, drop out-of-range/overlapping keeping the earliest, splice in one pass). Offsets line up because_lintandlint-fixstage byte-identical sources..golangci.ymlselection flows in through_lint, so the fix target needs no config wiring.suggested_fixes.Formatters (
format/format-fix)heph-govet doubles as the formatter driver in a
-formatmode (a whole-file rewrite path, separate from unitchecker), so formatting reuses the one hermetic Go binary — no second ~50-min tool build. Backends: gofmt (go/format+ golangci/gofmt for-s+ rewrite rules), gofumpt, goimports (FormatOnly— sort/group/remove; adding imports needs the module graph and would break the hermetic sandbox, so it is not attempted).format-fixmirrorslint-fix(codegen=in_place);formatis the check gate.Not supported in the per-package model (documented)
unused(andunparam) — whole-program. Feasible via the facts model + a root fan-in target (honnef'sSerializedGraph.Mergeis exactly that combine); designed, not yet built.rules[].source(only path/text/linter parsed).Tests
go list).scanNolint+suppressedunit tests (nolint scoping, exclude text/path/linter).-jsonexit 0; 95 staticcheck analyzers wired; unknown linter fails;//nolint:gosimpleand a text exclude-rule drop S1002 while ST1000 survives; mis-scoped rule does not suppress.lint-fix:report_edits/apply_edits(overlap, insertion, out-of-range, first-fix-only), spec builder, an asyncparse()check asserting in-place pkg-relative outputs, and provider resolution over realgo list.-/glob), honnef config override (initialisms + default fallback).-ssimplify, gofumpt extra rules, goimports sort, idempotence), plus Rust spec builders + provider resolution.go test ./...green.Remaining
Building
heph-govetthrough heph (//tools/heph-govet:build) exercises the in-sandbox thirdparty download of x/tools + honnef (go.sumcommitted; host build verified). The end-to-endws.run("//:lint")andws.run("//:lint-fix")tests depend on that step.🤖 Generated with Claude Code