Skip to content

chore(go): lint every Go module with gofumpt, golangci-lint, and go fix - #1425

Merged
jdx merged 1 commit into
jdx:mainfrom
sahidvelji:chore/go-lint-config
Sep 14, 2026
Merged

jdx merged 1 commit into
jdx:mainfrom
sahidvelji:chore/go-lint-config

Conversation

@sahidvelji

@sahidvelji sahidvelji commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

What changed

All three Go modules — go, integrations/cobra, benches/go — are now linted by one shared config. integrations/cobra was previously linted by nothing: lint:go hardcoded gofmt -l go benches/go and ran go vet in two named directories. The replacements are hk builtins keyed on workspace_indicator = "go.mod" so hk resolves each module itself: gofumpt, golangci-lint, go fix, and go mod tidy -diff. mise run lint already runs hk check --all, so CI needs no workflow change.

The new linters turned up four dead functions, five staticcheck rewrites in argv, two unchecked errors in the cobra test, and one go fix modernization.

Generated Go changes shape

usage generate go now emits type X struct{} for a command with no flags, args, or subcommands, rather than:

type McpCmd struct {
}

Adopters who regenerate will see this in their diff. gofumpt skips generated files when walking a directory but formats them when given explicit paths, so emitting what the formatter wants is what keeps generated output clean without a post-pass; gen-shadow now passes explicit paths for the same reason. The kong shadow emitter in xtask got the same fix.

benches/go is no longer special-cased

It is the only module with third-party dependencies, and lint:go and test:go both probed go mod download so they could skip it when those cannot be fetched. Both probes are gone. The test:go one existed so that an offline machine "should still get the suite that matters", but the go module needs expr fetched too, so that suite fails first on a cold cache. tasks/perf-go.sh keeps its equivalent, since it labels missing benchmark rows in the report rather than skipping quietly.

Worth a reviewer's attention

  • Both tools are pinned, so a tool release cannot change what the check means without a deliberate bump.
  • The golangci-lint step passes --allow-serial-runners: it takes a global lock and hk runs it once per module, so otherwise the second instance fails instead of waiting.
  • Builtins.go_vet is deliberately absent. govet is in golangci-lint's standard set and runs the same passes.

mise run lint, mise run test:go including the full conformance corpus, and usage-lib's tests pass, and gen-go, gen-shadow, and render produce no diff. Three shell-completion tests fail locally on macOS without the system bash-completion that CI installs, identically on an unmodified checkout.


AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5; version: 2.1.236.

`lint:go` hardcoded `gofmt -l go benches/go` and ran `go vet` in two named
directories, so `integrations/cobra` was linted by nothing. Replace it with hk
builtins keyed on `workspace_indicator = "go.mod"` so hk resolves each module
itself: gofumpt for formatting, golangci-lint, `go fix`, and `go mod tidy -diff`.

`usage generate go` and the kong shadow emitter now write `type X struct{}` for an
empty struct rather than `struct {\n}`. gofumpt skips generated files when walking
a directory but formats them when given explicit paths, so emitting what the
formatter wants is what keeps generated output clean without a post-pass.

Drop the `benches/go` carve-out from `lint:go` and `test:go`. Its stated purpose
was that a machine without a proxy should still get the suite that matters, but
the `go` module needs `expr` fetched too, so on a cold cache that suite fails
before the probe is reached.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Advanced

Run ID: 0a7cd841-c53b-4fcb-b6c1-18b862b02723

📥 Commits

Reviewing files that changed from the base of the PR and between 591f9bc and 97a5c12.

⛔ Files ignored due to path filters (3)
  • hk.pkl is excluded by !**/*.pkl
  • lib/src/go/snapshots/usage__go__tests__unknown_flags_are_inherited_and_overridable.snap is excluded by !**/*.snap
  • mise.lock is excluded by !**/*.lock
📒 Files selected for processing (28)
  • .golangci.yml
  • benches/go/cmd/sweep/main.go
  • benches/go/mise-kong/kong.go
  • benches/go/mise/tables.go
  • go/argv/complete.go
  • go/argv/complete_test.go
  • go/argv/page.go
  • go/argv/page_long.go
  • go/argv/parser.go
  • go/argv/parser_test.go
  • go/argv/post.go
  • go/argv/post_test.go
  • go/argv/relationships.go
  • go/argv/relationships_test.go
  • go/argv/render_test.go
  • go/argv/request_test.go
  • go/argv/scope.go
  • go/conformance/complete_test.go
  • go/conformance/conformance_test.go
  • go/internal/shadow/mise/meta_test.go
  • go/internal/shadow/mise/tables.go
  • go/internal/spec/spec.go
  • go/internal/spec/spec_test.go
  • integrations/cobra/cobra_usage.go
  • integrations/cobra/cobra_usage_test.go
  • lib/src/go/structs.rs
  • mise.toml
  • xtask/src/go/kong.rs

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

Changes

Go maintenance and tooling

Layer / File(s) Summary
Compact empty structs in generated Go
lib/src/go/structs.rs, xtask/src/go/kong.rs, benches/go/..., go/internal/shadow/mise/tables.go
Generators and generated command tables now use struct{} for empty structs. The benchmark warm-up loop uses for range warm.
Argument parsing and rendering cleanup
go/argv/complete.go, go/argv/page.go, go/argv/page_long.go, go/argv/parser.go, go/argv/post.go, go/argv/relationships.go, go/argv/scope.go
Equivalent boolean expressions and dispatch logic were rewritten. Unused page helpers were removed. Several declarations and composite literals were reformatted.
Argument handling test fixture updates
go/argv/*_test.go
Test fixtures and case tables were expanded into multiline literals without changing inputs, assertions, or expected values.
Tooling, integration, and supporting Go updates
.golangci.yml, mise.toml, integrations/cobra/*, go/conformance/*, go/internal/spec/*, go/internal/shadow/mise/meta_test.go
Go lint configuration and tool dependencies were added. Go test and formatting tasks were updated. Cobra test errors are now checked, and unused test helpers were removed.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Other

Suggested reviewers: jdx

Merge Risk: ⚪ Minimal · up to 97a5c

The reviewed changes preserve the described behavior or update development tooling; no merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 76.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 59 functions across 24 files. (4 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: expanding Go linting with gofumpt, golangci-lint, and go fix across the Go modules.
Full details: Docstring Coverage

Explanation

Docstring coverage is 76.27% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 59 functions across 24 files. (4 skipped: 2 unsupported, 2 too large.)

  • Fix all pre-merge checks with AI

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sahidvelji
sahidvelji marked this pull request as ready for review September 14, 2026 01:27
@greptile-apps

greptile-apps Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge with no actionable correctness, security, or repository-rule issues identified.

Summary

  • Adds shared golangci-lint configuration and pins gofumpt and golangci-lint.
  • Replaces bespoke Go lint tasks with workspace-aware hk builtins.
  • Makes generated empty structs gofumpt-clean and formats generated shadow files explicitly.
  • Applies equivalent static-analysis rewrites, removes unused helpers, and checks Cobra test errors.

Reviews (1) · Last reviewed commit: "chore(go): lint every Go module with gof..."

@jdx
jdx merged commit 54007f8 into jdx:main Sep 14, 2026
11 checks passed
This was referenced Sep 14, 2026
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.

2 participants