[typist] Typist: Go type consistency analysis (duplicated types and untyped usage) #64752
Closed
Replies: 1 comment
|
This discussion was automatically closed because it expired on 2026-10-02T11:44:11.075Z.
|
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
🔤 Typist - Go Type Consistency Analysis
Analysis of repository: github/gh-aw
Executive Summary
I scanned all 1,395 non-test Go files under
pkg/(1,404 top-level type definitions) for duplicated types and weakly-typed (interface{}/any) usage. The good news first: this codebase has already largely modernized away frominterface{}— it survives almost nowhere in production logic — and the ~460 files usingmap[string]any/[]anyare doing so legitimately, to represent arbitrary user-authored YAML/JSON frontmatter and schemas, which is the textbook appropriate use of Go'sany.The real finds are narrower but concrete:
parserandworkfloweach define their own privateruntimeImportReferencestruct andextractRuntimeImportReferencesfunction with nearly identical logic (worth sharing), andcli.graderManifestEntryis a hand-maintained subset of the JSON shape thatworkflow.graderManifestEntrywrites — a schema-drift risk if the writer changes without the reader noticing. I also flagged one dead/unusedanyparameter and one documented "union type" field that could be given a proper type instead ofany.Full Analysis Report
Duplicated Type Definitions
Summary Statistics
Cluster 1:
runtimeImportReference— Exact Duplicate (Actionable)Impact: Medium-High — same struct and the same parsing function reimplemented in two packages
Locations:
pkg/parser/frontmatter_hash.go:572—type runtimeImportReference struct { path string; startLine int; endLine int }, withextractRuntimeImportReferencesatpkg/parser/frontmatter_hash.go:617pkg/workflow/runtime_import_validation.go:38—type runtimeImportReference struct { importPath string; startLine int; endLine int }, with its ownextractRuntimeImportReferencesatpkg/workflow/runtime_import_validation.go:44Both parse
{{#import ...}}/{{#runtime-import ...}}macro references out of markdown into a path + line range, with the only difference being a field name (pathvsimportPath).Recommendation:
runtimeImportReferenceandextractRuntimeImportReferencesinto a shared location both packages already depend on (e.g. an exported helper inpkg/parser, sinceworkflowalready importsparser), and delete theworkflow-local copy.Cluster 2:
graderManifestEntry— Near Duplicate / Schema-Drift Risk (Actionable)Impact: Medium — a reader struct that silently falls out of sync with the writer's JSON shape
Locations:
pkg/workflow/compiler_yaml_graders.go:86— the writer: 13 fields (ID, Name, Description, Source, Enabled, Unit, Direction, Threshold, Max, Min, Digest, Run, Inline, Config)pkg/cli/audit_report_graders.go:67— the reader: only 5 fields (ID, Name, Unit, Direction, Threshold)The CLI reader only needs a subset today, but because it's a hand-copied struct rather than a shared type, a future rename/addition in the writer (e.g. renaming
Unit) won't fail to compile incli— it'll just silently stop populating that field.Recommendation:
GraderManifestEntryinto a shared package) and haveclidecode into the same struct, ignoring fields it doesn't use, instead of maintaining a parallel partial definition.Cluster 3:
JobInfovsJobData— Partial Overlap (Lower Confidence)Impact: Low — plausibly intentional (full GH Actions job metadata vs. a trimmed audit-report view)
Locations:
pkg/cli/logs_models.go—JobInfo(22 fields, mirrors the GitHub Actions job API shape)pkg/cli/audit_report.go—JobData(5 fields:Name, Status, Conclusion, Duration, Steps)JobDatalooks like a deliberately-slimmed presentation type for the audit report rather than a duplicate, but since both live inpkg/cliit may be worth buildingJobDatafromJobInfovia a mapping function (if not already done) so the "slimming" logic lives in one place. Not flagging this as urgent — recommend a quick look rather than a scheduled refactor.Reviewed and Dismissed (false positives / non-issues)
parser.ValidationErrorvsvalidationerror.ValidationError: not a duplicate —parser.ValidationErrorintentionally embedsvalidationerror.Payloadas part of a deliberate shared-payload design (documented atpkg/parser/validation_error.go:9-14) soerrors.Asworks uniformly across packages. This is the fix for duplication, not an instance of it.Worker,fakeOS,state,myString,myBytes(pkg/linters/*/testdata/...): these are isolatedgo/analysislinter test fixtures. Each custom linter underpkg/linters/needs its own self-contained testdata package to exercise the analyzer, so repeated minimal type names across fixture packages are expected and not a maintenance burden.Create*Config/Update*Configfamilies (CreateIssuesConfig,CreateDiscussionsConfig,CreatePullRequestsConfig, etc., and theirUpdate*counterparts): these already share common fields via embeddedBaseSafeOutputConfig/UpdateEntityConfig/SafeOutputAllowedLabelsConfig. This is the correct composition pattern for a family of safe-output configs that are genuinely different per entity — no further consolidation recommended.Untyped Usages
Summary Statistics
interface{}in production (non-test) code: effectively 0 — the only two hits outside_test.go/testdata/are a code comment and ajsonschema-godoc comment; genuineinterface{}usage lives only in test files andgo/analysislinter testdata fixtures, where it's the thing being tested.map[string]anyin non-test files: 463 files (almost entirelypkg/parser,pkg/workflow,pkg/cli)[]anyin non-test files: 183 filesWhy the bulk of
anyusage is not flaggedThe overwhelming majority of
map[string]any/anyusage sits inpkg/parser(YAML/JSON frontmatter parsing, JSON Schema validation, import resolution) andpkg/cli(reading/rewriting arbitrary workflow frontmatter via codemods). This is dynamic, user-authored, schema-validated-at-runtime data — exactly the case where Go'sanyis idiomatic rather than a smell. Replacing it wholesale with structs would either duplicate the JSON Schema definitions in Go or make the parser brittle against legitimately-variable user YAML shapes. No action recommended here.Finding 1: Dead
anyparameter inLayoutEmphasisBoxpkg/console/layout_wasm.go:16func LayoutEmphasisBox(content string, color any) stringcoloris never read inside the function body, and a repo-wide search finds zero callers anywhere (only a README line documenting the signature). It's untyped and unused.func LayoutEmphasisBox(content string) string) ifcolorwas a stub for future styling, or give it a real type (e.g. a concrete color type) if WASM-build coloring is still planned.Finding 2: Union-typed field via
anyinobservabilityImportEndpointpkg/parser/import_observability.go:16-19Headersis deliberatelyanybecause the original YAML field accepts either a bare string or amap[string]string, and normalization is deferred to theworkflowpackage.pkg/parseritself (convert both accepted forms into a singlemap[string]stringonce), soHeaderscan be typed asmap[string]stringand every downstream consumer inworkflowis freed from re-checking the type. If the dual-form input must be preserved verbatim for some reason, a small named type with a documented contract would at least make the intent self-documenting at the type level.Untyped constants
Sampled ~200 top-level
constdeclarations without explicit types acrosspkg/constants,pkg/parser,pkg/cli, andpkg/workflow. Unlikeinterface{}/any, bare untyped constants are idiomatic Go (they're intentionally flexible for arithmetic/conversion) and essentially every example found here has a self-explanatory name (DefaultMaxDailyAICredits,MaxSymlinkDepth,shellcheckMaxConcurrency, etc.). No action recommended — this is not a type-safety gap in this codebase.Recommendations
Priority 1: Consolidate
runtimeImportReferenceSteps:
pkg/parser, sinceworkflowalready imports it) and export the struct + extraction function.pkg/workflow/runtime_import_validation.goto call the shared version and delete its local copy.make test-unitto confirm macro-reference extraction behavior is unchanged in both call sites.Estimated effort: 1-2 hours · Impact: Medium-High (single source of truth for import-reference parsing)
Priority 2: Share
graderManifestEntrybetween writer and readerSteps:
workflow.graderManifestEntry(or move it to a neutral shared package) and havecli/audit_report_graders.godecode into it directly.cliside still only reads the fields it needs — no behavior change, just removes the parallel definition.Estimated effort: 1 hour · Impact: Medium (removes a silent schema-drift risk between a JSON writer and reader)
Priority 3: Clean up the two concrete untyped-usage spots
Steps:
color anyparameter inLayoutEmphasisBox.observabilityImportEndpoint.Headerstomap[string]stringat parse time inpkg/parser.Estimated effort: 1-2 hours combined · Impact: Low-Medium (small clarity wins, no urgent risk)
Implementation Checklist
runtimeImportReferenceinto one shared implementationgraderManifestEntrytype betweenworkflow(writer) andcli(reader)JobInfo/JobDataoverlap (likely no action — note as reviewed)color anyparameter inpkg/console/layout_wasm.goobservabilityImportEndpoint.Headersto a concrete type inpkg/parserCreate*Config/Update*Configfamilies — already well-composedmap[string]any/[]anyusage inparser/cli— appropriate for dynamic YAML/JSON handlingAnalysis Metadata
pkg/)interface{}in production code)get_symbols_overview) + targetedgreppattern matching, with manual verification of every flagged cluster before inclusionAll reactions