Skip to content

[refactor] Consolidate duplicated linter AST helpers into internal/astutilΒ #43644

Description

@github-actions

πŸ”§ Semantic Function Clustering Analysis

Analysis of repository: github/gh-aw β€” non-test .go files under pkg/

Executive Summary

I built a cross-file index of every top-level function/method declaration across 1,009 non-test .go files and clustered them by name and signature to surface duplicates and misplaced functions.

Headline: the codebase is already well-organized. The large packages follow a clean one-feature-per-file convention (pkg/cli, 339 files, has zero within-package free-function name collisions; pkg/workflow, 429 files, has none either once build variants are excluded). Nearly every apparent "duplicate" is a legitimate pattern, not a defect:

  • _wasm.go build-tag variants β€” e.g. RunGH, ExecGH, findGitRoot, validateDockerImage each appear twice in pkg/workflow only because there is a real impl + a //go:build wasm stub. Correct Go idiom.
  • Intentional delegation shims β€” SanitizeName, FindClosestMatches, compileSchema appear in pkg/workflow/pkg/parser as thin wrappers that already delegate to a single source of truth (pkg/stringutil, parser.CompileSchema). Consolidation has already happened.
  • Standard Go patterns β€” run (42Γ—, cobra commands), init (5Γ—), New (3Γ—).

Exactly one genuine, high-confidence consolidation opportunity survived verification, plus a few minor naming-collision notes.

Priority 1 β€” Duplicated AST helpers across linter analyzers (high confidence)

The pkg/linters/* analyzer packages copy-paste several small AST/type helpers instead of sharing them. This matters because the shared home already exists β€” pkg/linters/internal/astutil β€” and is already imported by 41 of 42 analyzer packages. These helpers are simply stragglers that were never migrated.

Verified duplicates (bodies compared, not just names):

Helper Copies Locations Similarity
enclosingFuncType(node ast.Node) *ast.FuncType 3 execcommandwithoutcontext, httpnoctx, timesleepnocontext byte-identical
contextContextType(pass *analysis.Pass) types.Type 3 execcommandwithoutcontext, timesleepnocontext, httpnoctx identical logic (httpnoctx uses early-continue)
contextParamName(pass, *ast.FuncType) (string, bool) 3 execcommandwithoutcontext, timesleepnocontext, ctxbackground identical (ctxbackground calls a local contextType)
calledOSFunc(pass, *ast.CallExpr) (*types.Func, bool) 2 osgetenvlibrary, ossetenvlibrary byte-identical
flipOp(op token.Token) token.Token 2 lenstringzero, stringsindexcontains byte-identical
Representative diff β€” enclosingFuncType is copied verbatim
// pkg/linters/execcommandwithoutcontext/execcommandwithoutcontext.go:141
// pkg/linters/httpnoctx/httpnoctx.go:164
// pkg/linters/timesleepnocontext/timesleepnocontext.go:145
func enclosingFuncType(node ast.Node) *ast.FuncType {
	switch fn := node.(type) {
	case *ast.FuncDecl:
		return fn.Type
	case *ast.FuncLit:
		return fn.Type
	default:
		return nil
	}
}

Recommendation: promote these five helpers into pkg/linters/internal/astutil (which currently exports IsLocalObject, IsFmtErrorf, Inspector, NodeText, etc.) and delete the per-package copies. contextContextType/contextParamName should be reconciled onto one spelling (ctxbackground uses contextType; the others use contextContextType).

Impact: removes ~5 copy-paste clusters (~10 redundant definitions), single source of truth for context/AST detection, and consistency with the pattern already used by 41 analyzers. Effort: ~1–2 hours; mechanical, covered by existing linter tests.

Priority 2 β€” Same-name / different-semantics collisions (minor, documentation)

These are not duplicates β€” same name, different signature and behavior across packages β€” but they invite confusion when reading code:

Name pkg/parser pkg/workflow / pkg/cli
extractToolsFromFrontmatter returns (string, error) returns map[string]any
resolveImportInputPath returns (string, bool) returns (any, bool)
hasCopilotRequestsWritePermission β€” cli takes map[string]any; workflow takes *WorkflowData (same intent, two layers)

Recommendation: rename to disambiguate by return contract (e.g. extractToolsString vs extractToolsMap), or add doc comments cross-referencing the sibling. Low priority β€” no functional issue. The hasCopilotRequestsWritePermission pair is worth checking for shared permission logic that could live in one place.

What was checked and found clean

  • Outlier functions in the wrong file: none material. File names in pkg/cli/pkg/workflow map cleanly to purpose (git_helpers.go, docker_validation.go, schema_utils.go, permissions_operations.go, ...).
  • Scattered utility functions: already centralized β€” pkg/stringutil, pkg/fileutil, pkg/sliceutil, pkg/setutil, pkg/jsonutil, pkg/timeutil, etc. exist and are used via delegation shims.
  • Cross-package free-function duplicates: all resolved to build variants or intentional shims.

Implementation checklist

  • Move enclosingFuncType, contextContextType, contextParamName, calledOSFunc, flipOp into pkg/linters/internal/astutil
  • Reconcile contextType vs contextContextType naming in ctxbackground
  • Delete the per-analyzer copies and update call sites
  • Run linter analyzer tests to confirm no behavior change
  • (Optional) Disambiguate the Priority-2 name collisions

Analysis metadata

  • Non-test .go files analyzed: 1,009 (pkg/, excluding *_test.go and testdata/)
  • Detection method: cross-file function/method name+signature index; duplicate bodies verified by direct comparison; build-tag (_wasm.go) and delegation-shim false positives filtered out
  • Confirmed consolidation clusters: 5 (all in pkg/linters/*)
  • Naming-collision notes: 3
  • Analysis date: 2026-07-06

Note: analysis focused on the tractable, high-signal axis (function-name/signature clustering across the full tree). Byte-level near-duplicate detection inside function bodies was not exhaustively run across all 1,009 files; the findings above are the clusters that survived manual verification.

Generated by πŸ”§ Semantic Function Refactoring Β· 214.5 AIC Β· βŒ– 15.1 AIC Β· ⊞ 9.2K Β· β—·

  • expires on Jul 7, 2026, 4:13 PM UTC-08:00

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions