Skip to content

Commit 85f2be4

Browse files
mpywclaude
andcommitted
feat(gotask): migrate gotask checker to unified registry pattern
- Add ArgIsDeriverCall pattern for DoAsync ctx argument checking - Register DoAll/DoAllSettled/DoRace with ShouldCallDeriver (trace through NewTask) - Register DoAllFns/DoAllFnsSettled/DoRaceFns as variadic APIs - Register Task.DoAsync/CancelableTask.DoAsync with ArgIsDeriverCall - Add Variadic flag to registry.API for variable callback arguments - Add MatchesFunc() to deriver.Matcher for direct function matching - Enhance ShouldCallDeriver with NewTask tracing and slice detection - Add variadic expansion detection (tasks...) with "variadic argument" message - Add ordinal formatting (2nd, 3rd) for argument position messages - Fix report position for method chains using selector position - Remove legacy checkers: errgroup, waitgroup, goroutine, goroutinederive, gotask - Add defer-only test cases for goroutinederive 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent f00076e commit 85f2be4

File tree

18 files changed

+644
-962
lines changed

18 files changed

+644
-962
lines changed

analyzer.go

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

1515
"github.com/mpyw/goroutinectx/internal/checker"
16-
"github.com/mpyw/goroutinectx/internal/checkers/gotask"
1716
"github.com/mpyw/goroutinectx/internal/checkers/spawner"
1817
"github.com/mpyw/goroutinectx/internal/checkers/spawnerlabel"
1918
"github.com/mpyw/goroutinectx/internal/context"
@@ -156,6 +155,14 @@ func runASTChecks(
156155
// Register errgroup/waitgroup/conc APIs with ClosureCapturesCtx pattern
157156
checker.RegisterDefaultAPIs(reg, enableErrgroup, enableWaitgroup)
158157

158+
// Register gotask APIs with ShouldCallDeriver and ArgIsDeriverCall patterns
159+
if goroutineDeriver != "" && enableGotask {
160+
matcher := deriver.NewMatcher(goroutineDeriver)
161+
deriverPattern := &patterns.ShouldCallDeriver{Matcher: matcher}
162+
doAsyncPattern := &patterns.ArgIsDeriverCall{Matcher: matcher}
163+
checker.RegisterGotaskAPIs(reg, deriverPattern, doAsyncPattern)
164+
}
165+
159166
// Build GoStmt patterns
160167
var goPatterns []patterns.GoStmtPattern
161168

@@ -173,6 +180,8 @@ func runASTChecks(
173180
"GoStmtCapturesCtx": ignore.Goroutine,
174181
"GoStmtCallsDeriver": ignore.GoroutineDerive,
175182
"ClosureCapturesCtx": ignore.Errgroup, // errgroup/waitgroup use this
183+
"ShouldCallDeriver": ignore.Gotask, // gotask uses this
184+
"ArgIsDeriverCall": ignore.Gotask, // gotask DoAsync uses this
176185
}
177186

178187
// Create and run unified checker
@@ -187,7 +196,7 @@ func runASTChecks(
187196
)
188197
unifiedChecker.Run(pass, insp)
189198

190-
// Run remaining checkers that aren't migrated yet (spawner, gotask)
199+
// Run remaining checkers that aren't migrated yet (spawner only)
191200
runLegacyCheckers(pass, insp, ignoreMaps, carriers, spawners, skipFiles)
192201
}
193202

@@ -200,10 +209,8 @@ func runLegacyCheckers(
200209
spawners *spawnerdir.Map,
201210
skipFiles map[string]bool,
202211
) {
203-
// Only spawner and gotask need the legacy path
204-
spawnerEnabled := enableSpawner && spawners.Len() > 0
205-
gotaskEnabled := goroutineDeriver != "" && enableGotask
206-
if !spawnerEnabled && !gotaskEnabled {
212+
// Only spawner needs the legacy path
213+
if !enableSpawner || spawners.Len() == 0 {
207214
return
208215
}
209216

@@ -241,14 +248,7 @@ func runLegacyCheckers(
241248
}
242249

243250
if call, ok := n.(*ast.CallExpr); ok {
244-
// Spawner checker
245-
if enableSpawner && spawners.Len() > 0 {
246-
spawner.New(spawners).CheckCall(cctx, call)
247-
}
248-
// Gotask checker
249-
if goroutineDeriver != "" && enableGotask {
250-
gotask.New(goroutineDeriver).CheckCall(cctx, call)
251-
}
251+
spawner.New(spawners).CheckCall(cctx, call)
252252
}
253253

254254
return true

internal/checker/apis.go

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -113,62 +113,80 @@ func RegisterDefaultAPIs(reg *registry.Registry, enableErrgroup, enableWaitgroup
113113
}
114114

115115
// RegisterGotaskAPIs registers gotask APIs with deriver pattern.
116-
func RegisterGotaskAPIs(reg *registry.Registry, deriverPattern *patterns.ShouldCallDeriver) {
117-
// gotask Do* functions - task callbacks should call deriver
116+
func RegisterGotaskAPIs(reg *registry.Registry, deriverPattern *patterns.ShouldCallDeriver, doAsyncPattern *patterns.ArgIsDeriverCall) {
117+
// DoAll, DoAllSettled, DoRace - variadic Task arguments
118+
// Each Task arg is traced through NewTask to check the callback body
118119
reg.Register(deriverPattern,
119120
registry.API{
120121
Pkg: "github.com/siketyan/gotask",
121122
Type: "",
122-
Name: "Do",
123+
Name: "DoAll",
123124
Kind: registry.KindFunc,
124-
CallbackArgIdx: 1, // tasks start at index 1
125+
CallbackArgIdx: 1, // Tasks start at index 1 (after ctx)
126+
Variadic: true,
125127
},
126128
registry.API{
127129
Pkg: "github.com/siketyan/gotask",
128130
Type: "",
129-
Name: "DoAll",
131+
Name: "DoAllSettled",
130132
Kind: registry.KindFunc,
131133
CallbackArgIdx: 1,
134+
Variadic: true,
132135
},
133136
registry.API{
134137
Pkg: "github.com/siketyan/gotask",
135138
Type: "",
136-
Name: "DoAllSettled",
139+
Name: "DoRace",
137140
Kind: registry.KindFunc,
138141
CallbackArgIdx: 1,
142+
Variadic: true,
139143
},
140-
// DoAllFns, DoAllFnsSettled - callback receives ctx
144+
)
145+
146+
// DoAllFns, DoAllFnsSettled, DoRaceFns - variadic functions
147+
// Each fn argument should call deriver in its body
148+
reg.Register(deriverPattern,
141149
registry.API{
142150
Pkg: "github.com/siketyan/gotask",
143151
Type: "",
144152
Name: "DoAllFns",
145153
Kind: registry.KindFunc,
146-
CallbackArgIdx: 1,
154+
CallbackArgIdx: 1, // fns start at index 1
155+
Variadic: true,
147156
},
148157
registry.API{
149158
Pkg: "github.com/siketyan/gotask",
150159
Type: "",
151160
Name: "DoAllFnsSettled",
152161
Kind: registry.KindFunc,
153162
CallbackArgIdx: 1,
163+
Variadic: true,
164+
},
165+
registry.API{
166+
Pkg: "github.com/siketyan/gotask",
167+
Type: "",
168+
Name: "DoRaceFns",
169+
Kind: registry.KindFunc,
170+
CallbackArgIdx: 1,
171+
Variadic: true,
154172
},
155173
)
156174

157-
// Task/CancelableTask.DoAsync - ctx should be derived
158-
reg.Register(deriverPattern,
175+
// Task.DoAsync, CancelableTask.DoAsync - ctx arg should BE a deriver call
176+
reg.Register(doAsyncPattern,
159177
registry.API{
160178
Pkg: "github.com/siketyan/gotask",
161179
Type: "Task",
162180
Name: "DoAsync",
163181
Kind: registry.KindMethod,
164-
CallbackArgIdx: -1, // ctx is in call args, not callback
182+
CallbackArgIdx: 0, // ctx is first argument
165183
},
166184
registry.API{
167185
Pkg: "github.com/siketyan/gotask",
168186
Type: "CancelableTask",
169187
Name: "DoAsync",
170188
Kind: registry.KindMethod,
171-
CallbackArgIdx: -1,
189+
CallbackArgIdx: 0,
172190
},
173191
)
174192
}

internal/checker/checker.go

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
package checker
33

44
import (
5+
"fmt"
56
"go/ast"
67
"go/token"
78

@@ -137,10 +138,86 @@ func (c *Checker) checkCallExpr(cctx *patterns.CheckContext, call *ast.CallExpr,
137138
return
138139
}
139140

141+
// Handle variadic APIs (e.g., DoAllFns(ctx, fn1, fn2, ...))
142+
if entry.API.Variadic {
143+
c.checkVariadicCallExpr(cctx, call, entry, scope)
144+
return
145+
}
146+
140147
if !entry.Pattern.Check(cctx, call, callbackArg) {
141148
msg := entry.Pattern.Message(entry.API.FullName(), scope.ctxName())
142-
cctx.Pass.Reportf(call.Pos(), "%s", msg)
149+
// Report at method selector position for chained calls
150+
reportPos := getCallReportPos(call)
151+
cctx.Pass.Reportf(reportPos, "%s", msg)
152+
}
153+
}
154+
155+
// getCallReportPos returns the best position to report for a call expression.
156+
// For method calls, this is the selector (method name) position.
157+
// For other calls, this is the call position.
158+
func getCallReportPos(call *ast.CallExpr) token.Pos {
159+
if sel, ok := call.Fun.(*ast.SelectorExpr); ok {
160+
return sel.Sel.Pos()
161+
}
162+
return call.Pos()
163+
}
164+
165+
// checkVariadicCallExpr checks each callback argument in a variadic API call.
166+
func (c *Checker) checkVariadicCallExpr(
167+
cctx *patterns.CheckContext,
168+
call *ast.CallExpr,
169+
entry *registry.Entry,
170+
scope *contextScope,
171+
) {
172+
startIdx := entry.API.CallbackArgIdx
173+
if startIdx < 0 || startIdx >= len(call.Args) {
174+
return
175+
}
176+
177+
// Check if this is a variadic expansion (e.g., DoAllFns(ctx, slice...))
178+
isVariadicExpansion := call.Ellipsis.IsValid()
179+
180+
for i := startIdx; i < len(call.Args); i++ {
181+
arg := call.Args[i]
182+
if !entry.Pattern.Check(cctx, call, arg) {
183+
var msg string
184+
if isVariadicExpansion {
185+
// For variadic expansion, we can't determine the position
186+
msg = entry.API.FullName() + "() variadic argument should call goroutine deriver"
187+
} else {
188+
// Create message with argument position (1-based for human readability)
189+
argNum := i + 1
190+
msg = formatVariadicMessage(entry.API.FullName(), argNum)
191+
}
192+
// Report at the call position (where the // want comment is)
193+
cctx.Pass.Reportf(call.Pos(), "%s", msg)
194+
}
195+
}
196+
}
197+
198+
// formatVariadicMessage formats a diagnostic message with argument position.
199+
func formatVariadicMessage(apiName string, argNum int) string {
200+
return apiName + "() " + ordinal(argNum) + " argument should call goroutine deriver"
201+
}
202+
203+
// ordinal returns the ordinal form of a number (1st, 2nd, 3rd, 4th, etc.)
204+
func ordinal(n int) string {
205+
suffix := "th"
206+
switch n % 10 {
207+
case 1:
208+
if n%100 != 11 {
209+
suffix = "st"
210+
}
211+
case 2:
212+
if n%100 != 12 {
213+
suffix = "nd"
214+
}
215+
case 3:
216+
if n%100 != 13 {
217+
suffix = "rd"
218+
}
143219
}
220+
return fmt.Sprintf("%d%s", n, suffix)
144221
}
145222

146223
// shouldIgnore checks if the position should be ignored for the given checker.

internal/checkers/checker.go

Lines changed: 0 additions & 18 deletions
This file was deleted.

internal/checkers/errgroup/checker.go

Lines changed: 0 additions & 48 deletions
This file was deleted.

0 commit comments

Comments
 (0)