Skip to content

[go-fan] Go Module Review: golang.org/x/toolsΒ #33498

@github-actions

Description

@github-actions

🐹 Go Fan Report: golang.org/x/tools

Daily Go-dependency deep-dive. This module is the foundation of gh-aw's custom linter framework under pkg/linters/ β€” and is currently the most-recently-pushed direct dependency in go.mod (pushed_at 2026-05-19T17:00:38Z, version v0.45.0). The review surfaces a few small inconsistencies in the analyzer suite, a brand-new published modernize package that could clean up the codebase in one pass, and several opportunities to emit SuggestedFixes so gopls and go vet -fix can apply fixes automatically.

Summary

  • Files importing the module: 21
  • Subpackages used: go/analysis, go/analysis/analysistest, go/analysis/multichecker, go/analysis/passes/inspect, go/ast/inspector
  • Analyzers wired into cmd/linters/main.go: 8
  • Status: βœ… Mature, idiomatic usage overall. A few drift-to-best-practice opportunities and one new upstream feature worth adopting.

Current Usage in gh-aw

Every custom linter in pkg/linters/ is built on golang.org/x/tools/go/analysis. The shape is consistent across 7 of 8 analyzers:

var Analyzer = &analysis.Analyzer{
    Name:     "<name>",
    Doc:      "...",
    URL:      "https://github.com/github/gh-aw/tree/main/pkg/linters/<name>",
    Requires: []*analysis.Analyzer{inspect.Analyzer},
    Run:      run,
}

Then run grabs pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) and uses insp.Preorder(nodeFilter, ...). Test files use analysistest.Run(t, analysistest.TestData(), Analyzer, "a") against testdata packages.

Analyzer roster
Analyzer What it flags
ctxbackground context.Background() inside funcs that already receive a ctx context.Context
errormessage non-actionable error messages in changed files
errstringmatch strings.Contains(err.Error(), "...") substring matching
excessivefuncparams > 8 positional params (configurable)
largefunc > 60-line function bodies (configurable)
osexitinlibrary os.Exit inside pkg/
rawloginlib std-library log.* inside pkg/
ssljson validates .github/skills/*/ssl.json

Research Findings

Recent upstream activity

  • go/analysis/passes/modernize β€” a brand-new published suite of 25+ analyzers (commit cd58e99, 2026-05-05) that mechanically modernize older Go code with safe, auto-appliable SuggestedFixes: slices.Concat, slices.Clone, min/max, for i := range N, typed atomic.Int32, strings.Cut/CutPrefix, omitzero, errors.As type-param form, and more. Distributed as a CLI: go run golang.org/x/tools/go/analysis/passes/modernize/cmd/modernize@latest -fix ./....
  • inspector.Cursor API (2025-12 β†’ 2026-01) β€” Cursor.Valid, Cursor.ParentEdge{Kind,Index}, FindByPos improvements. Lets analyzers cheaply ask "what AST edge connects me to my parent?" without manually maintaining a stack.

Best practices observed in the maintainers' own analyzers

  • One shared inspector.Inspector per driver via inspect.Analyzer β€” already followed everywhere except ssljson.
  • Filter at the []ast.Node level β€” already followed.
  • Emit SuggestedFixes whenever the fix is mechanical β€” not followed by any gh-aw analyzer.
  • Use pass.ReportRangef(node, ...) for range diagnostics β€” not followed; everyone uses Reportf(node.Pos(), ...).
  • URL: field pointing to rule docs β€” followed (idiomatic).

Improvement Opportunities

πŸƒ Quick Wins

  • Fix ctxbackground body walk (pkg/linters/ctxbackground/ctxbackground.go:43) β€” it requires inspect.Analyzer but then uses raw ast.Inspect(fn.Body, ...). Use insp.WithStack([]ast.Node{(*ast.CallExpr)(nil)}, ...) and check the enclosing *ast.FuncDecl from the stack β€” one traversal instead of two.
  • Add _test.go filtering to ctxbackground β€” every other analyzer skips test files; ctxbackground does not. Likely an oversight: test helpers commonly accept ctx context.Context and also legitimately call context.Background() for goroutines that outlive the test.
  • Lift the duplicated _test.go check into pkg/linters/internal/ β€” strings.HasSuffix(filename, "_test.go") appears in 5 different analyzers. One helper, one consistent rule.
  • Swap pass.Reportf(node.Pos(), ...) for pass.ReportRangef(node, ...) in largefunc, excessivefuncparams, errormessage, errstringmatch, ctxbackground, osexitinlibrary, rawloginlib β€” editors and gopls get a precise underlined range instead of a single-character marker.

✨ Feature Opportunities

  • Emit SuggestedFix from ctxbackground β€” the fix is mechanical: replace context.Background() with the in-scope ctx parameter name. gopls can then auto-apply it.
  • Emit SuggestedFix from errstringmatch for common sentinels β€” strings.Contains(err.Error(), "context canceled") β†’ errors.Is(err, context.Canceled). Same shape for context.DeadlineExceeded, io.EOF, os.ErrNotExist, fs.ErrNotExist.
  • Run modernize once β€” go run golang.org/x/tools/go/analysis/passes/modernize/cmd/modernize@latest -fix ./.... One-shot cleanup, not a CI gate.
  • Adopt the inspector.Cursor API in errormessage for cheap ancestor lookups ("am I inside a return statement?").

πŸ“ Best Practice Alignment

  • Reconsider the ssljson "anchor package" trick (pkg/linters/ssljson/ssljson.go:207) β€” it's a workaround for multichecker running each analyzer once per package. ssljson is fundamentally a file-system validator, not a Go-source analyzer, so a separate cmd/ssljson binary using singlechecker.Main (or a stand-alone go test) is a cleaner long-term shape. At minimum, document the anchor-package trick in the Doc: field.
  • Cache the error interface lookup in internal/nolint.ImplementsError (pkg/linters/internal/nolint/nolint.go:60) β€” types.Universe.Lookup("error") runs on every call. sync.Once-initialized package var is trivially correct.

πŸ”§ General Improvements

  • Add direct unit tests for pkg/linters/internal/nolint β€” currently only exercised indirectly via the analyzer end-to-end tests.
  • Add a CI smoke test that runs the linters binary in vet-mode β€” confirms the multichecker.Main binding builds and at least one analyzer fires.

Recommendations (prioritized)

  1. ctxbackground fix-ups β€” body-walk consolidation + _test.go skip + SuggestedFix. Single PR, small diff, immediate correctness/UX improvement.
  2. Run modernize -fix once β€” separate PR, mechanical, easy to review.
  3. errstringmatch sentinel SuggestedFixes β€” meaningful gopls UX upgrade, scoped to the well-known sentinels.
  4. Lift _test.go and pkg/ skip helpers into internal/ β€” DRY cleanup; pairs naturally with rejig docsΒ #1.
  5. Swap to ReportRangef β€” drive-by polish, can ride any of the above PRs.
  6. Reshape ssljson (longer-term) β€” orthogonal to go/analysis; tackle when the SSL rules grow.

Next Steps

References:


Generated by Go Fan 🐹
Module summary: scratchpad/mods/golang-x-tools.md

Generated by 🐹 Go Fan Β· ● 9.9M Β· β—·

  • expires on May 21, 2026, 8:59 AM UTC

Metadata

Metadata

Labels

automationcookieIssue Monster Loves Cookies!dependenciesPull requests that update a dependency filego-fan

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions