Skip to content

Commit a29c550

Browse files
mpywclaude
andauthored
refactor: merge v2 modular architecture into internal package (#17)
* refactor: merge v2 modular architecture into internal package This refactoring applies the successful pattern from gormreuse/zerologlintctx: create v2 with ideal architecture, then merge v2→v1. Key changes: - Add internal/check: check context module for checker implementations - Add internal/checker.go: unified checker interfaces (GoStmtChecker, CallChecker) - Add internal/checkers: individual checker implementations - goroutine.go: go statement checker - callarg.go: errgroup/waitgroup/conc/spawner checkers - gotask.go: gotask library checker - Add internal/scope: context scope detection - Reorganize directives: - internal/directives/* → internal/directive/* (singular) - Simplified carrier, ignore, spawner modules - Merge internal/deriver: deriver matcher with OR/AND semantics - Simplify internal/runner.go: use new checker interfaces - Consolidate internal/ssa: merge builder and tracer Package structure after merge: - internal/check/ - check context for patterns - internal/checkers/ - individual checker implementations - internal/deriver/ - derive function matcher - internal/directive/ - ignore, spawner, carrier directives - internal/funcspec/ - function specification parsing - internal/scope/ - context scope detection - internal/ssa/ - SSA program and tracer Removed: - internal/directives/ - replaced by internal/directive - internal/ssa/helpers.go - merged into tracer.go 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: consolidate duplicate functions and remove dead code - Remove dead code packages: internal/context, internal/patterns - Simplify internal/registry (remove unused Patterns field) - Simplify internal/apis.go (remove patterns-related code) - Rename internal/check to internal/probe for clarity - Move internal/spawnerlabel to internal/checkers/spawnerlabel - Consolidate duplicate isContextOrCarrierType functions: - Add carrier.IsCarrierType() as the canonical implementation - Remove duplicates from probe, scope, and ssa packages - Clean up internal/typeutil (remove 5 unused exported functions) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: add comprehensive package documentation - Add doc.go files to all internal packages with: - ASCII architecture diagrams - Code examples - API documentation - Fix redundant import aliases (internal "...") - Exclude doc.go files from gci formatter in golangci-lint 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: remove unused functions to improve coverage Remove dead code functions that are never called: - probe.Context.Report, FuncOf - probe.Context.FuncLitOfIdentBefore - scope.Scope.CtxName - ssa.Program.EnclosingFunc, findEnclosingFunc - spawner.FindFuncArgs Coverage improved from 84.1% to 85.8% 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor(ignore): remove unused AllCheckerNames function 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * refactor(ignore): remove unused Entry getter methods Entry fields are only accessed internally within the package. External code uses high-level APIs (ShouldIgnore, GetUnusedIgnores). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * refactor(callarg): remove unused variadic support code - Remove Variadic field from CallArgEntry (never set to true) - Remove checkVariadic, formatOrdinals functions (dead code) - Move ordinal function to gotask.go (only user) Coverage improved: 86.1% → 87.7% 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * test: improve coverage for gotask and spawner checkers Coverage improvements: - carrier.matchPkg: 28.6% → 100% - gotask.callbackCallsDeriver: 50.0% → 83.3% - callarg.checkFuncArg: 58.3% → 75.0% - gotask.identIsDeriverCall: 60.0% → 100% Bug fix: - Add *ast.ParenExpr case to findConstructorCall to handle (*taskPtr).DoAsync() New test cases: - carrier_test.go: unit tests for matchPkg and Parse functions - gotask/advanced.go: callback variable patterns, derived context variable tests - gotask/evil.go: pointer dereference limitation documentation - spawner/spawner.go: factory function patterns 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent f6beeaf commit a29c550

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+3292
-2808
lines changed

.golangci.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,6 @@ formatters:
3434
- default
3535
- prefix(github.com/mpyw/goroutinectx)
3636
custom-order: true
37+
exclusions:
38+
paths:
39+
- "doc\\.go$"

analyzer.go

Lines changed: 53 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,14 @@ import (
1313
"golang.org/x/tools/go/ast/inspector"
1414

1515
"github.com/mpyw/goroutinectx/internal"
16-
"github.com/mpyw/goroutinectx/internal/directives/carrier"
17-
"github.com/mpyw/goroutinectx/internal/directives/deriver"
18-
"github.com/mpyw/goroutinectx/internal/directives/ignore"
19-
"github.com/mpyw/goroutinectx/internal/directives/spawner"
16+
"github.com/mpyw/goroutinectx/internal/checkers"
17+
"github.com/mpyw/goroutinectx/internal/checkers/spawnerlabel"
18+
"github.com/mpyw/goroutinectx/internal/deriver"
19+
"github.com/mpyw/goroutinectx/internal/directive/carrier"
20+
"github.com/mpyw/goroutinectx/internal/directive/ignore"
21+
"github.com/mpyw/goroutinectx/internal/directive/spawner"
2022
"github.com/mpyw/goroutinectx/internal/registry"
21-
"github.com/mpyw/goroutinectx/internal/spawnerlabel"
22-
internalssa "github.com/mpyw/goroutinectx/internal/ssa"
23+
"github.com/mpyw/goroutinectx/internal/ssa"
2324
)
2425

2526
// Flags for the analyzer.
@@ -60,7 +61,7 @@ func init() {
6061
var Analyzer = &analysis.Analyzer{
6162
Name: "goroutinectx",
6263
Doc: "checks that context.Context is properly propagated to downstream calls",
63-
Requires: []*analysis.Analyzer{inspect.Analyzer, internalssa.BuildSSAAnalyzer},
64+
Requires: []*analysis.Analyzer{inspect.Analyzer, ssa.BuildSSAAnalyzer},
6465
Run: run,
6566
Flags: flag.FlagSet{},
6667
}
@@ -88,22 +89,22 @@ func run(pass *analysis.Pass) (any, error) {
8889
// Build enabled checkers map
8990
enabled := buildEnabledCheckers(spawners)
9091

91-
// Build registry
92-
reg := buildRegistry()
93-
9492
// Build SSA program
95-
ssaProg := internalssa.Build(pass)
93+
ssaProg := ssa.Build(pass)
9694

97-
// Spawner map (nil if disabled)
98-
var spawnerMap *spawner.Map
99-
if enableSpawner {
100-
spawnerMap = spawners
95+
// Build derivers matcher
96+
var derivers *deriver.Matcher
97+
if goroutineDeriver != "" {
98+
derivers = deriver.NewMatcher(goroutineDeriver)
10199
}
102100

103-
// Create and run unified checker
101+
// Build checkers
102+
goStmtCheckers, callCheckers := buildCheckers(derivers, spawners)
103+
104+
// Create and run runner
104105
runner := internal.NewRunner(
105-
reg,
106-
spawnerMap,
106+
goStmtCheckers,
107+
callCheckers,
107108
ssaProg,
108109
carriers,
109110
ignoreMaps,
@@ -113,6 +114,14 @@ func run(pass *analysis.Pass) (any, error) {
113114

114115
// Run spawnerlabel checker if enabled
115116
if enableSpawnerlabel {
117+
reg := registry.New()
118+
119+
// Register APIs for spawnerlabel detection
120+
internal.RegisterErrgroupAPIs(reg)
121+
internal.RegisterWaitgroupAPIs(reg)
122+
internal.RegisterConcAPIs(reg)
123+
internal.RegisterGotaskAPIs(reg)
124+
116125
spawnerlabelChecker := spawnerlabel.New(spawners, reg, ssaProg)
117126
spawnerlabelChecker.Check(pass, ignoreMaps, skipFiles)
118127
}
@@ -124,15 +133,12 @@ func run(pass *analysis.Pass) (any, error) {
124133
}
125134

126135
// buildSkipFiles creates a set of filenames to skip.
127-
// Generated files are always skipped.
128-
// Test files can be skipped via the driver's built-in -test flag.
129136
func buildSkipFiles(pass *analysis.Pass) map[string]bool {
130137
skipFiles := make(map[string]bool)
131138

132139
for _, file := range pass.Files {
133140
filename := pass.Fset.Position(file.Pos()).Filename
134141

135-
// Always skip generated files
136142
if ast.IsGenerated(file) {
137143
skipFiles[filename] = true
138144
}
@@ -156,34 +162,44 @@ func buildIgnoreMaps(pass *analysis.Pass, skipFiles map[string]bool) map[string]
156162
return ignoreMaps
157163
}
158164

159-
// buildRegistry creates and populates the API registry.
160-
func buildRegistry() *registry.Registry {
161-
reg := registry.New()
165+
// buildCheckers creates the checker instances.
166+
func buildCheckers(derivers *deriver.Matcher, spawners *spawner.Map) ([]internal.GoStmtChecker, []internal.CallChecker) {
167+
var goStmtCheckers []internal.GoStmtChecker
168+
var callCheckers []internal.CallChecker
162169

163-
// Create derivers matcher once if configured
164-
var derivers *deriver.Matcher
165-
if goroutineDeriver != "" {
166-
derivers = deriver.NewMatcher(goroutineDeriver)
170+
// Goroutine checkers
171+
if enableGoroutine {
172+
goStmtCheckers = append(goStmtCheckers, &checkers.Goroutine{Derivers: derivers})
167173
}
168174

169-
// Register patterns (pass derivers for deriver checking)
170-
if enableGoroutine {
171-
internal.RegisterGoroutinePatterns(reg, derivers)
175+
if derivers != nil {
176+
goStmtCheckers = append(goStmtCheckers, &checkers.GoroutineDerive{Derivers: derivers})
172177
}
178+
179+
// Call checkers
173180
if enableErrgroup {
174-
internal.RegisterErrgroupAPIs(reg, derivers)
181+
callCheckers = append(callCheckers, checkers.NewErrgroupChecker(derivers))
175182
}
183+
176184
if enableWaitgroup {
177-
internal.RegisterWaitgroupAPIs(reg, derivers)
185+
callCheckers = append(callCheckers, checkers.NewWaitgroupChecker(derivers))
178186
}
187+
179188
if enableConc {
180-
internal.RegisterConcAPIs(reg, derivers)
189+
callCheckers = append(callCheckers, checkers.NewConcChecker(derivers))
181190
}
182-
if enableGotask {
183-
internal.RegisterGotaskAPIs(reg, derivers)
191+
192+
if enableSpawner && spawners.Len() > 0 {
193+
callCheckers = append(callCheckers, checkers.NewSpawnerChecker(spawners, derivers))
194+
}
195+
196+
if enableGotask && derivers != nil {
197+
if gotaskChecker := checkers.NewGotaskChecker(derivers); gotaskChecker != nil {
198+
callCheckers = append(callCheckers, gotaskChecker)
199+
}
184200
}
185201

186-
return reg
202+
return goStmtCheckers, callCheckers
187203
}
188204

189205
// buildEnabledCheckers creates a map of which checkers are enabled.

0 commit comments

Comments
 (0)