Skip to content

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

@github-actions

Description

@github-actions

🐭 Go Fan daily module review.

Module Overview

golang.org/x/tools is the Go team's supplementary tooling collection. gh-aw uses only its static-analysis sub-packages to build a suite of 19 custom go/analysis linters in pkg/linters/ that enforce project conventions (no context.Background() when a ctx param exists, no oversized functions, no panics in library code, unchecked type assertions, raw logging in libraries, etc.).

Version: v0.45.0 — pushed 2026-05-30, one of the most actively maintained deps in the tree. 🚀

Current Usage in gh-aw

  • Files: 21 non-test source files + 22 test files
  • Sub-packages: go/analysis (21 refs), go/analysis/passes/inspect (18), go/ast/inspector (18), go/analysis/analysistest (19, test-only), go/analysis/multichecker (driver)
  • Key APIs used: analysis.Analyzer, pass.Report/Reportf, inspect.Analyzer, inspector.Preorder / WithStack, analysistest.Run, multichecker.Main

Traversal patterns:

  • 14 analyzers use insp.Preorder(filter, func(n ast.Node){...})
  • 3 use insp.WithStack(...) with manual slices.Backward(stack) ancestor scans
  • 4 nest a raw ast.Inspect(fn.Body, ...) inside a Preorder walk
  • All 19 repeat an identical defensive comma-ok assertion to fetch the *inspector.Inspector

Research Findings

Confirmed against pkg.go.dev for v0.45.0 — the static-analysis API has grown a lot, and gh-aw is on a version new enough to use all of it:

Recent Updates

  • inspector.Cursor + Inspector.Root()/At() (v0.34.0) — an immutable cursor offering Enclosing(types...), Preorder, Inspect, Parent, Children, Contains, FindNode. The docs explicitly call it recommended over Inspector methods where possible.
  • inspector.All[N](in) iter.Seq[N] and PreorderSeq(...) (v0.26.0) — typed range-over-func iterators that eliminate per-callback type assertions.
  • Diagnostic.URL / Category / Related — richer diagnostics that editors/gopls can deep-link.

Best Practices

  • When inspect.Analyzer is in Requires, the driver guarantees ResultOf[inspect.Analyzer] is a *inspector.Inspector; the conventional idiom (used by all stdlib passes) is a direct assertion, not comma-ok.
  • Analyzers that emit SuggestedFixes should be tested with analysistest.RunWithSuggestedFixes against .golden files.

Improvement Opportunities

🏃 Quick Wins

  • Drop ~76 lines of boilerplate. Every one of the 19 analyzers repeats:
insp, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
if !ok { return nil, fmt.Errorf("inspect analyzer result has unexpected type %T", ...) }

Since inspect.Analyzer is in every Requires, the assertion cannot fail. Use the idiomatic direct form insp := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector), or extract a one-line helper in pkg/linters/internal.

✨ Feature Opportunities

  • Adopt the Cursor API (v0.34.0) to replace insp.WithStack + manual slices.Backward(stack) ancestor scans with cur.Enclosing((*ast.FuncDecl)(nil)) in ctxbackground, panic-in-library-code, regexpcompileinfunction — removing all push/pop/stack bookkeeping:
for cur := range insp.Root().Preorder((*ast.CallExpr)(nil)) {
    call := cur.Node().(*ast.CallExpr)
    for encl := range cur.Enclosing((*ast.FuncDecl)(nil)) {
        fn := encl.Node().(*ast.FuncDecl)
        // ...
    }
}
  • Use typed iterators inspector.All[*ast.CallExpr](insp) / PreorderSeq (v0.26.0) in the 14 Preorder analyzers to drop the per-callback n.(*ast.X) assertion and read as clean Go 1.23+ range loops.
  • Unify nested walks — the 4 ast.Inspect(fn.Body, ...) blocks could move to Cursor.Inspect/Cursor.Preorder under a single cursor traversal.

📐 Best Practice Alignment

  • Untested suggested fix (concrete gap). ctxbackground is the only analyzer emitting SuggestedFixes, but its test uses plain analysistest.Run, so the fix output is never verified. Add a testdata/src/ctxbackground/ctxbackground.go.golden file and switch to analysistest.RunWithSuggestedFixes.
  • Optionally set Diagnostic.URL/Category (the suite already populates Analyzer.URL) to deep-link diagnostics.

🔧 General

The suite is genuinely well-built and idiomatic: per-analyzer packages, correct Requires: inspect.Analyzer, populated Name/Doc/URL, analyzer flags for thresholds (-max-lines, -max-params), full analysistest coverage, a shared nolint directive index, and a README-derived spec_test.go. The items above are modernization/polish — not defects.

Recommendations

  1. Verify the suggested fix — add the .golden file + RunWithSuggestedFixes for ctxbackground (correctness; small, high value).
  2. Remove the comma-ok boilerplate across 19 files (readability; mechanical).
  3. Pilot the Cursor API in one WithStack analyzer, then roll out if it reads cleaner.
  4. Optionally adopt typed All/PreorderSeq iterators incrementally.

Next Steps

  • Add golden-file test for ctxbackground suggested fix
  • Introduce an internal/inspect helper (or direct assertion) and update analyzers
  • Prototype an Enclosing-based rewrite of one WithStack analyzer

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

References: §26747042425

Generated by 🐹 Go Fan · opus48 2.5M ·

  • expires on Jun 2, 2026, 9:49 AM UTC

Metadata

Metadata

Labels

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