Skip to content

refactor: SSA-based pattern registry architecture#14

Closed
mpyw wants to merge 50 commits intomainfrom
refactor/ssa-pattern-registry
Closed

refactor: SSA-based pattern registry architecture#14
mpyw wants to merge 50 commits intomainfrom
refactor/ssa-pattern-registry

Conversation

@mpyw
Copy link
Owner

@mpyw mpyw commented Dec 22, 2025

Summary

This PR introduces the foundation for a new SSA-based pattern registry architecture (Issue #12).

Changes

  • internal/ssa/: SSA program building and value tracing utilities
  • internal/patterns/: Pattern interfaces and implementations
  • internal/registry/: Declarative API registration system
  • internal/checker/: Unified checker (not yet integrated)

Architecture Overview

┌─────────────────────────────────────────────────────────────┐
│                      analyzer.go                            │
│  (orchestration, flags, run function)                       │
└─────────────────────────────────────────────────────────────┘
                              │
                              ▼
┌─────────────────────────────────────────────────────────────┐
│                   internal/checker/                         │
│  Unified checker that ties everything together              │
│  - Dispatches to patterns based on AST node type            │
│  - Handles ignore directives                                │
└─────────────────────────────────────────────────────────────┘
                              │
              ┌───────────────┼───────────────┐
              ▼               ▼               ▼
┌──────────────────┐ ┌─────────────┐ ┌─────────────────┐
│ internal/patterns│ │internal/    │ │ internal/ssa/   │
│                  │ │registry/    │ │                 │
│ - ClosureCaptures│ │ - API def   │ │ - Program build │
│ - CallbackReceive│ │ - Match     │ │ - Value tracing │
│ - ShouldCallDeriv│ │ - Register  │ │ - Closure find  │
│ - GoStmt patterns│ │             │ │                 │
└──────────────────┘ └─────────────┘ └─────────────────┘

Pattern Classification

Pattern Description Used By
ClosureCapturesCtx Closure should capture outer ctx errgroup.Go, waitgroup.Go, conc.Pool.Go
CallbackReceivesCtx API passes ctx to callback gotask.DoAllFnsSettled, ants.PoolWithFuncGeneric
ShouldCallDeriver Callback must call deriver function goroutine-derive, gotask tasks

Status

⚠️ This PR adds the new architecture but does NOT integrate it yet.

The old internal/checkers/ remains in use. Integration will be done in follow-up commits after verification.

Why SSA?

Current AST-based approach has limitations:

  • Can't trace values through Phi nodes (control flow merges)
  • Can't trace factory function returns properly
  • Can't track closures passed through channels or stored in fields

SSA enables proper dataflow analysis for these cases.

Test plan

  • All existing tests pass (new code is additive)
  • golangci-lint passes
  • Integration into analyzer.go (future commits)
  • Verify identical output with old checkers
  • Delete old checkers after verification

🤖 Generated with Claude Code

mpyw and others added 4 commits December 22, 2025 16:08
Add internal/ssa package providing SSA program building and value tracing
utilities for the new pattern-based architecture.

Key components:
- builder.go: Wraps buildssa.Analyzer for SSA program construction
- tracer.go: Provides value tracing through Phi nodes, Store instructions,
  closures, and MakeClosure operations

The tracer supports:
- Finding closures from SSA values (MakeClosure, Phi, Call returns, etc.)
- Checking if closures capture context variables via FreeVars
- Tracing through factory function returns
- Store/address matching for field and index access

Based on patterns from zerologlintctx with improvements for goroutinectx.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add internal/patterns package defining the pattern abstraction layer
for context propagation checking.

Pattern interface:
- Pattern: For CallExpr-based checks (errgroup, waitgroup, etc.)
- GoStmtPattern: For go statement checks

Implemented patterns:
- ClosureCapturesCtx: Checks closure captures outer context
  Used by: errgroup.Group.Go, sync.WaitGroup.Go, conc.Pool.Go
- CallbackReceivesCtx: Checks callback receives context as parameter
  Used by: gotask.DoAllFnsSettled, ants.PoolWithFuncGeneric
- ShouldCallDeriver: Checks callback calls deriver function
  Used by: goroutine-derive, gotask task functions
- GoStmtCapturesCtx: Checks go statement captures context
- GoStmtCallsDeriver: Checks go statement calls deriver

CheckContext provides:
- analysis.Pass for type information
- SSA Program for dataflow analysis
- Tracer for value tracing

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add internal/registry package for declarative API registration
with pattern association.

API struct defines:
- Pkg: Package path (e.g., "golang.org/x/sync/errgroup")
- Type: Receiver type name (empty for package functions)
- Name: Method or function name
- Kind: KindMethod or KindFunc
- CallbackArgIdx: Index of callback argument

Registry provides:
- Register(pattern, ...apis): Associate pattern with multiple APIs
- Match(pass, call): Find matching API and extract callback argument
- Entries(): List all registered API/pattern pairs

FullName() returns human-readable API names:
- Methods: "errgroup.Group.Go"
- Functions: "gotask.Do"

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add internal/checker package providing the unified checker that
ties together SSA analysis, patterns, and registry.

Checker struct orchestrates:
- Registry for API matching
- GoStmtPatterns for go statement checking
- SSA Program for dataflow analysis
- Ignore directive handling

Run() method:
- Builds function scopes with context parameters
- Uses inspector.WithStack for AST traversal
- Dispatches to patterns based on node type

apis.go provides default API registrations:
- errgroup.Group.Go, TryGo
- sync.WaitGroup.Go
- sourcegraph/conc pool APIs (Pool, WaitGroup, ContextPool, etc.)
- gotask Do*, DoAsync functions

This checker is not yet integrated into analyzer.go.
The old checkers remain in use until integration is complete.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 22, 2025

実装メモ

SSA Tracer について

internal/ssa/tracer.go は zerologlintctx の tracing.go を参考に作成。主な改善点:

  1. FreeVar による context キャプチャ検出

    • ClosureUsesContext は closure の FreeVars をチェック
    • 名前と型の両方でマッチング(型の同一性だけでは不十分なケースがある)
  2. Factory 関数の戻り値トレース

    • go makeWorker(ctx)() パターンに対応
    • traceCallReturn で callee の return 文を解析
  3. Store 命令のトレース

    • struct フィールドや配列要素への代入をトレース
    • findStoredValue / addressesMatch で実装

Pattern インターフェース設計

type Pattern interface {
    Name() string
    Check(cctx *CheckContext, call *ast.CallExpr, callbackArg ast.Expr) bool
    Message(apiName string, ctxName string) string
}

type GoStmtPattern interface {
    Name() string
    CheckGoStmt(cctx *CheckContext, stmt *ast.GoStmt) bool
    Message(ctxName string) string
}

CallExpr と GoStmt で別インターフェースにした理由:

  • go func(){}() は GoStmt(statement)
  • g.Go(func(){}) は CallExpr(expression)
  • AST 構造が根本的に異なる

Registry の API マッチング

type API struct {
    Pkg            string  // "golang.org/x/sync/errgroup"
    Type           string  // "Group" (empty for package functions)
    Name           string  // "Go"
    Kind           APIKind // KindMethod or KindFunc
    CallbackArgIdx int     // 0 (callback is first argument)
}

strings.HasPrefix(pkg.Path(), api.Pkg) でマッチングしているのは、
バージョン付きパス(e.g., github.com/foo/bar/v2)に対応するため。

次のステップ

  1. analyzer.go への統合

    • buildssa.Analyzer を Requires に追加
    • 新 checker を呼び出すコード追加
    • 旧 checkers と並行実行して出力比較
  2. 出力の互換性確認

    • メッセージフォーマットは既存と同一になるよう調整済み
    • 例: errgroup.Group.Go() closure should use context "ctx"
  3. 旧コード削除

    • internal/checkers/ 全体を削除
    • internal/context/ のうち新 checker で不要な部分を削除

@mpyw
Copy link
Owner Author

mpyw commented Dec 22, 2025

既知の制限事項と今後の改善

現時点の制限

  1. チャネル経由のクロージャ受け渡し

    • ch <- func() { ... }; go (<-ch)() は追えない
    • SSA でも channel の値追跡は困難
  2. interface{} 経由の関数

    • var fn interface{} = func() { ... }; go fn.(func())() は型アサーション後の追跡が困難
  3. 外部パッケージの関数本体

    • 依存パッケージの関数内部は SSA で見えない
    • -external-spawner フラグで対応予定

SSA で解決できる可能性があるもの

  1. 変数再代入

    fn := func() { use(ctx) }
    fn = func() { /* no ctx */ }  // 再代入
    go fn()  // 現在: OK判定、SSA: NG検出可能
    • Phi ノードを辿ることで最終的な値を追跡可能
  2. 複数分岐からのクロージャ

    var fn func()
    if cond {
        fn = func() { use(ctx) }
    } else {
        fn = func() { /* no ctx */ }
    }
    go fn()
    • AllPhiEdgesSatisfy で全分岐をチェック可能
  3. Factory 関数の戻り値

    func makeWorker(ctx context.Context) func() {
        return func() { use(ctx) }
    }
    go makeWorker(ctx)()  // 現在: 追跡困難、SSA: 追跡可能
    • traceCallReturn で return 文を解析

テスト戦略

統合時は以下の順序で進める予定:

  1. 新旧両方のチェッカーを並行実行
  2. 出力が完全一致することを確認
  3. 一致しない場合は新チェッカーを修正
  4. 一致確認後、旧チェッカーを削除
  5. LIMITATION テストケースで新たに検出できるものがあれば BAD に変更

@codecov
Copy link

codecov bot commented Dec 22, 2025

Codecov Report

❌ Patch coverage is 81.77474% with 267 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.73%. Comparing base (435330d) to head (704a50c).

Files with missing lines Patch % Lines
internal/context/composite.go 63.06% 21 Missing and 20 partials ⚠️
internal/spawnerlabel/checker.go 73.80% 18 Missing and 15 partials ⚠️
.../patterns/callback_calls_deriver_or_ctx_derived.go 69.31% 19 Missing and 8 partials ⚠️
internal/ssa/builder.go 49.05% 21 Missing and 6 partials ⚠️
internal/patterns/gostmt.go 80.80% 16 Missing and 8 partials ⚠️
internal/patterns/callback_calls_deriver.go 80.00% 16 Missing and 7 partials ⚠️
internal/context/factory.go 77.94% 10 Missing and 5 partials ⚠️
internal/context/assignment.go 79.66% 6 Missing and 6 partials ⚠️
internal/runner.go 93.15% 7 Missing and 3 partials ⚠️
internal/context/capture.go 84.31% 4 Missing and 4 partials ⚠️
... and 9 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #14      +/-   ##
==========================================
+ Coverage   81.29%   82.73%   +1.43%     
==========================================
  Files          19       26       +7     
  Lines        1358     1772     +414     
==========================================
+ Hits         1104     1466     +362     
- Misses        137      194      +57     
+ Partials      117      112       -5     
Flag Coverage Δ
unittests 82.73% <81.77%> (+1.43%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

mpyw and others added 2 commits December 22, 2025 16:16
Add buildssa.Analyzer to Requires list to enable SSA-based analysis.
The SSA program is built but currently unused while we work on
achieving detection parity with AST-based checkers.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Previously, patterns returned true (OK) when SSA analysis couldn't find
the enclosing function or closure, causing false negatives. Now they
always fall back to AST-based checking for reliable detection.

Changes:
- ClosureCapturesCtx: Fall back to AST when SSA fails at any stage
- GoStmtCapturesCtx: Fall back to AST when SSA fails at any stage
- CallbackReceivesCtx: Remove unused SSA code (was already AST-based)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 22, 2025

Implementation Status Update

Current State (Phase 1: Foundation)

The SSA infrastructure is now built and integrated:

  1. internal/ssa/ - SSA program building and value tracing
  2. internal/patterns/ - Pattern interfaces with SSA + AST hybrid checking
  3. internal/registry/ - Declarative API registration system
  4. internal/checker/ - Unified checker (ready but not yet integrated)
  5. analyzer.go - Added buildssa.Analyzer dependency

What Works Now

  • All existing tests pass ✅
  • SSA program is built for each pass
  • Old AST-based checkers continue to work
  • New patterns have AST fallback for basic cases (FuncLit)

Why Full Migration is Deferred

The old AST-based checkers in internal/context/checker.go have comprehensive logic for:

  • Variable assignment tracing (g.Go(fn) where fn := func(){})
  • Call result tracing (g.Go(makeWorker()))
  • Struct field tracing (g.Go(holder.task))
  • Slice/map index tracing (g.Go(tasks[0]))

The new SSA patterns' AST fallback only handles direct FuncLit cases. Full migration requires either:

  1. Enhancing findSSAValue to find SSA values for all these patterns
  2. Or copying the comprehensive AST logic to patterns

Next Steps (Phase 2)

Option A: SSA Enhancement

  • Improve Program.findSSAValue() to handle all expression types
  • SSA tracer then handles value flow automatically

Option B: AST Delegation

  • Add optional context.CheckContext to patterns.CheckContext
  • Delegate to comprehensive AST checking when SSA fails

Option A is cleaner architecturally, Option B is faster to implement.

Files Changed

internal/ssa/builder.go       # SSA program building
internal/ssa/tracer.go        # Value tracing utilities
internal/patterns/pattern.go   # Pattern interface + CheckContext
internal/patterns/closure_captures_ctx.go  # ClosureCapturesCtx
internal/patterns/callback_receives_ctx.go # CallbackReceivesCtx  
internal/patterns/should_call_deriver.go   # ShouldCallDeriver
internal/patterns/gostmt.go    # GoStmt patterns
internal/registry/api.go       # API definition
internal/registry/registry.go  # API registration
internal/checker/checker.go    # Unified checker
internal/checker/apis.go       # Default API registrations
analyzer.go                    # Added buildssa dependency

Improvements to SSA infrastructure for future pattern integration:

1. findSSAValue now handles multiple expression types:
   - FuncLit: finds MakeClosure instructions
   - Ident: traces variable declarations to find assigned values
   - CallExpr: finds SSA Call instructions
   - SelectorExpr: finds FieldAddr/Field instructions
   - IndexExpr: finds IndexAddr/Index/Lookup instructions

2. GetContextVars now includes FreeVars:
   - Captures context variables from enclosing scopes
   - Essential for nested closure analysis

3. findEnclosingFunc uses syntax range:
   - Uses full AST syntax range for position checking
   - Handles cases where GoStmt position precedes FuncLit position

Note: These improvements prepare the infrastructure for future migration
to SSA-based patterns. Current analyzer still uses AST-based checkers.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 22, 2025

Latest Update: SSA Infrastructure Enhancement

Added improvements to the SSA infrastructure to prepare for future pattern-based migration:

Changes in Latest Commit

1. Enhanced findSSAValue (pattern.go)

  • Now handles: FuncLit, Ident, CallExpr, SelectorExpr, IndexExpr
  • Maps AST expressions to corresponding SSA values
  • Enables tracing through variable assignments and function calls

2. Enhanced GetContextVars (tracer.go)

  • Now includes FreeVars (captured variables) in addition to parameters
  • Essential for nested closure analysis where ctx is captured, not passed

3. Fixed findEnclosingFunc (builder.go)

  • Uses full syntax range for position checking
  • Correctly handles GoStmt where go keyword precedes func keyword

Current State

  • All tests pass ✅
  • SSA infrastructure is built and ready
  • Old AST-based checkers still in use
  • Migration to SSA-based patterns is prepared but not activated

Why Migration is Not Activated Yet

After extensive testing, the SSA-based patterns have fundamental differences from AST-based checking:

  • AST directly inspects source structure
  • SSA represents a different view (single static assignment)
  • Many edge cases require additional work to achieve parity

Recommendation

This PR provides solid SSA infrastructure. The actual migration should be:

  1. Done incrementally (one checker at a time)
  2. Thoroughly tested for each migrated checker
  3. Possibly in a follow-up PR to keep changes focused

mpyw and others added 2 commits December 22, 2025 17:55
This commit switches context capture detection from AST-based to SSA-based
analysis using FreeVars. Key improvements:

- Add ClosureCapturesContext to check FreeVars for context/carrier types
- Add FindFuncLit to locate SSA function for FuncLit AST nodes
- Fix isContextType to unwrap pointer types (SSA uses *context.Context)
- Support carrier types in SSA-based detection

Behavioral changes:
- Context used in nested closures (defer, IIFE) is now correctly detected
- Former LIMITATION cases now pass: ctx in deferred closures, recovery
  closures, and IIFE are properly recognized as using context

Test updates:
- Update test cases from [LIMITATION]/[BAD] to [GOOD] for nested closure ctx
- Update deriver test cases to [PARTIAL] where ctx capture works but
  deriver calls in nested closures are not yet traced

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit adds SSA-based deriver function detection that:
- Traverses into immediately-invoked function expressions (IIFE)
- Distinguishes between regular calls and defer statements
- Reports a special error message when deriver is only in defer

Key changes:
- Add CheckDeriverCalls to SSA tracer with DeriverResult struct
- Update GoStmtPattern interface to return GoStmtResult with DeferOnly flag
- Add DeferMessage method for defer-specific error messages
- Former PARTIAL test cases now pass as GOOD (SSA detects IIFE correctly)

Behavioral changes:
- Deriver calls in IIFE are now correctly detected
- Deriver calls only in defer will report: "goroutine calls X in defer,
  but it should be called at goroutine start"

TODO: Add test case for defer-only scenario

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 22, 2025

🔄 セッション引き継ぎメモ

完了した作業

  1. SSA FreeVars ベースの context capture 検出

    • ClosureCapturesContext で FreeVars をチェック
    • ネストされたクロージャ(defer, IIFE)の ctx 捕捉を正しく検出
    • Carrier 型もサポート
  2. SSA ベースの deriver 検出

    • CheckDeriverCalls で SSA Call 命令をトレース
    • IIFE 内の deriver 呼び出しを検出
    • defer 内の deriver 呼び出しを区別
  3. defer 専用エラーメッセージ

    • GoStmtResult.DeferOnly フラグを追加
    • DeferMessage() メソッドを追加
    • メッセージ: "goroutine calls X in defer, but it should be called at goroutine start"

残タスク

  1. defer-only テストケース追加

    • testdata/src/goroutinederive/ に defer でのみ deriver を呼ぶケースを追加
    • // want で defer 専用メッセージを期待
    • metatest JSON も追加
  2. (任意)defer 内の IIFE 確認

    • 現在 defer 内で IIFE を呼ぶケースは inDefer=true で処理
    • 必要であればテストケース追加

関連ファイル

  • internal/ssa/tracer.go - SSA ベースの検出ロジック
  • internal/patterns/gostmt.go - GoStmtPattern インターフェース
  • internal/checker/checker.go - 呼び出し元の更新

@mpyw
Copy link
Owner Author

mpyw commented Dec 22, 2025

defer-only テストケース追加完了

追加内容

テストケース (testdata/src/goroutinederive/goroutinederive.go)

  • badDeriverOnlyInDefer: defer でのみ deriver を呼ぶ → 専用エラーメッセージ
  • badDeriverInDeferIIFE: defer 内の IIFE で deriver を呼ぶ → 同上
  • goodDeriverAtStartWithDefer: 先頭で deriver を呼び、defer も存在 → OK

メタテストJSON (3ファイル)

  • testdata/metatest/tests/badDeriverOnlyInDefer.json
  • testdata/metatest/tests/badDeriverInDeferIIFE.json
  • testdata/metatest/tests/goodDeriverAtStartWithDefer.json

defer 専用メッセージ

goroutine calls github.com/my-example-app/telemetry/apm.NewGoroutineContext in defer, but it should be called at goroutine start

./test_all.sh 全パス確認済み。

- 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>
@mpyw
Copy link
Owner Author

mpyw commented Dec 22, 2025

Session 7: gotask Migration Complete

Summary

gotask checker を unified registry pattern に移行完了。全テスト通過。

Changes

New Pattern: ArgIsDeriverCall

DoAsync 用の新しいパターン。ctx 引数が deriver 呼び出しの結果かどうかをチェック(コールバック内に deriver があるかではなく)。

// BAD: ctx をそのまま渡している
task.DoAsync(ctx, errChan)

// GOOD: deriver で wrap している
task.DoAsync(apm.NewGoroutineContext(ctx), errChan)

API Registration Design

  • DoAll/DoAllSettled/DoRace: NewTask 経由で Task 引数のコールバック body をトレース
  • DoAllFns/DoRaceFns: variadic API として各 fn 引数の body をチェック
  • Task.DoAsync/CancelableTask.DoAsync: ctx 引数が deriver 呼び出しかチェック
  • NewTask: スタンドアロンでは登録しない(DoAll 経由でトレース)

Message Format

  • Position info: gotask.DoAll() 2nd argument should call goroutine deriver
  • Variadic expansion: gotask.DoAllFnsSettled() variadic argument should call goroutine deriver
  • Method chain: (*gotask.Task).DoAsync() 1st argument should call goroutine deriver

Slice Detection

variadic expansion (tasks...) は slice 変数の中身をトレースできないため、常にエラー報告。

Removed Legacy Checkers

  • internal/checkers/checker.go
  • internal/checkers/errgroup/checker.go
  • internal/checkers/waitgroup/checker.go
  • internal/checkers/goroutine/checker.go
  • internal/checkers/goroutinederive/checker.go
  • internal/checkers/gotask/checker.go

Remaining Work

  • spawner checker の unified checker 移行(現在は legacy path)
  • spawnerlabel checker は独立しているため移行不要

Files Changed

  • 18 files changed, 644 insertions(+), 962 deletions(-)

mpyw and others added 2 commits December 22, 2025 19:10
- Create util.go with shared utilities:
  - funcLitHasContextParam, funcLitUsesContext
  - extractCallFunc, argUsesContext
  - findFuncLitAssignment, findFuncLitInAssignment
  - factoryReturnsContextUsingFunc, returnedValueUsesContext

- Prefix pattern-specific helpers to avoid name collisions:
  - closure_captures_ctx.go: closureCheckFromSSA, closureCheckFactoryCall, etc.
  - gostmt.go: goStmtCheckFromSSA, goStmtCheckHigherOrder, goStmtFindFuncDecl, etc.
  - should_call_deriver.go: deriverFindFuncLitAssignmentBefore, etc.
  - arg_is_deriver_call.go: argDeriverSplitAPIName

- Remove duplicate implementations, reducing code by ~230 lines

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove duplicate isContextType implementations (patterns, ssa)
  → Use single typeutil.IsContextType everywhere
- Unify funcTypeHasContextParam for FuncLit and FuncDecl
  → Base function accepts *ast.FuncType
- Consolidate findFuncLitAssignment variants
  → Single function with optional beforePos parameter
- Unify factory returns context check logic
  → blockReturnsContextUsingFunc as common base

Result: -95 lines of duplicated code

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 22, 2025

重複コード統合 (e81a031)

前回のプレフィックス追加に加えて、実際の重複コードを統合しました。

統合内容

統合前 統合後 削減
patterns/pattern.go:isContextType
ssa/tracer.go:isContextType
typeutil.IsContextType 2関数削除
funcLitHasContextParam
goStmtFuncDeclHasContextParam
funcTypeHasContextParam(*ast.FuncType) ロジック統一
findFuncLitAssignment
deriverFindFuncLitAssignmentBefore
findFuncLitAssignment(cctx, v, beforePos) 1関数削除
factoryReturnsContextUsingFunc
goStmtFactoryDeclReturnsCtxFunc
blockReturnsContextUsingFunc(*ast.BlockStmt, exclude) ロジック統一

結果

  • -95行 の重複コード削減
  • isContextType がプロジェクト全体で1箇所に統一
  • beforePos パラメータで柔軟性を持たせつつ統合

…ntext package

- Integrate spawner checker into internal/checker
  - Uses SSA-based ClosureCapturesCtx pattern
  - Removed internal/checkers/spawner package
- Remove internal/context package (now dead code)
  - All 4 files: context.go, checker.go, finder.go, scope.go
  - AST-based context checking fully replaced by SSA

This completes the SSA unification - all checkers now use SSA-based
analysis through the patterns package.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 22, 2025

SSA統合完了 (dd85fea)

Phase 1: spawner の unified checker 移行

  • internal/checker/checker.go に spawner チェックを統合
  • SSA-based ClosureCapturesCtx パターンを使用
  • internal/checkers/spawner/ パッケージ削除

Phase 2: AST-based context パッケージ削除

全てデッドコードになったため削除:

  • internal/context/context.go - CheckContext 構造体
  • internal/context/checker.go - CheckClosureUsesContext 等
  • internal/context/finder.go - FindFuncLitAssignment 等
  • internal/context/scope.go - Scope, FindScope

結果

  • -809行 削除
  • 全チェッカーが SSA-based patterns パッケージを使用
  • AST/SSA 2系統 → SSA 1系統に統合完了

mpyw and others added 4 commits December 22, 2025 19:36
- Convert package-level functions to CheckContext methods:
  - funcTypeHasContextParam, funcLitHasContextParam
  - FuncLitUsesContext, extractCallFunc, argUsesContext
  - FindFuncLitAssignment, findFuncLitInAssignment
  - blockReturnsContextUsingFunc, factoryReturnsContextUsingFunc
  - returnedValueUsesContext

- Reorganize files:
  - context.go: CheckContext and all its methods
  - patterns.go: Pattern and GoStmtPattern interfaces only

- Update all call sites in pattern implementations

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create internal/context/context.go with CheckContext and methods
- Export all methods used by patterns package
- Add type alias in patterns.go for backward compatibility
- Update all pattern implementations to use exported method names

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace all patterns.CheckContext usages with context.CheckContext
to eliminate the type alias and use direct imports consistently.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move internal/checker/* to internal/ (package internal)
- Move internal/checkers/spawnerlabel/ to internal/spawnerlabel/
- Remove redundant checker/checkers directory nesting
- Update imports in top-level analyzer.go

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 22, 2025

Session 7: Package Reorganization

Changes

  • Phase 3 完了: Pattern functions を CheckContext methods に変換
  • CheckContext を internal/context パッケージに移動: メソッドを全て exported に
  • Type alias 削除: type CheckContext = context.CheckContext → 直接 import に変更
  • パッケージ構造整理:
    • internal/checker/internal/ (root level, package internal)
    • internal/checkers/spawnerlabel/internal/spawnerlabel/

Current Structure

internal/
├── analyzer.go        # Unified SSA-based checker
├── apis.go            # API registration functions
├── context/           # CheckContext and methods
├── directives/        # Directive parsing
├── patterns/          # Pattern interfaces and implementations
├── registry/          # API registry
├── spawnerlabel/      # Spawnerlabel checker
├── ssa/               # SSA utilities
└── typeutil/          # Type utilities

All tests passing ✅

@mpyw
Copy link
Owner Author

mpyw commented Dec 22, 2025

Package-level Unexported Functions

analyzer.go (root)

  • init()
  • run()
  • buildSkipFiles()
  • buildIgnoreMaps()
  • runASTChecks()
  • buildEnabledCheckers()
  • reportUnusedIgnores()

cmd/goroutinectx/main.go

  • main()

internal/analyzer.go

  • getCallReportPos()
  • formatVariadicMessage()
  • ordinal()
  • buildFuncScopes()
  • findContextScope()
  • findEnclosingScope()

internal/directives/deriver/directive.go

  • parseFunc()
  • collectCalledFuncs()
  • extractFunc()
  • groupSatisfied()
  • specSatisfied()
  • matchesSpec()
  • matchesMethod()

internal/directives/ignore/directive.go

  • parseIgnoreComment()

internal/directives/spawner/directive.go

  • matchesSpec()
  • parseExternal()
  • parseFunc()
  • buildSpawnersForFile()
  • isSpawnerComment()

internal/patterns/arg_is_deriver_call.go

  • argDeriverSplitAPIName()

internal/patterns/closure_captures_ctx.go

  • closureCheckFromSSA()
  • closureCheckFromAST()
  • closureCheckSelectorFunc()
  • closureFindStructFieldFuncLit()
  • closureFindFieldInAssignment()
  • closureCheckIndexFunc()
  • closureFindIndexedFuncLit()
  • closureFindFuncLitAtIndex()
  • closureFindFuncLitByLiteral()
  • closureCheckFactoryCall()

internal/patterns/gostmt.go

  • goStmtCheckFromSSA()
  • goStmtCheckFromAST()
  • goStmtCheckHigherOrder()
  • goStmtCheckIdentFactory()
  • goStmtFindFuncDecl()
  • goStmtFuncDeclHasContextParam()
  • goStmtFactoryDeclReturnsCtxFunc()

internal/patterns/should_call_deriver.go

  • deriverFindCallExprAssignmentBefore()

internal/registry/api.go

  • shortPkgName()

internal/spawnerlabel/detector.go

  • isSpawnCall()
  • isKnownSpawnMethod()
  • isErrgroupSpawnCall()
  • isWaitgroupSpawnCall()
  • isGotaskSpawnCall()
  • isGotaskPackageCall()
  • isGotaskTaskType()
  • isSpawnerMarkedCall()
  • hasFuncArg()
  • hasFuncArgAtIndex()
  • hasFuncParams()

internal/typeutil/typeutil.go

  • isNamedTypeFromType()
  • unwrapPointer()

Total: 54 unexported functions

@mpyw
Copy link
Owner Author

mpyw commented Dec 22, 2025

統合可能な重複関数

1. FuncSpec + parseFunc + matchesSpec (deriver ↔ spawner)

完全重複:

  • FuncSpec struct - 両方で同一定義
  • parseFunc() - ほぼ同一(unicode.IsUpper vs 直接比較の差のみ)

類似:

  • matchesSpec() - deriver は matchesMethod() を呼ぶ、spawner はインライン実装

統合案: internal/funcspec パッケージを作成して共通化

// internal/funcspec/funcspec.go
type Spec struct {
    PkgPath  string
    TypeName string
    FuncName string
}

func Parse(s string) Spec
func (s Spec) Matches(fn *types.Func) bool

2. extractFunc / ExtractCallFunc / GetFuncFromCall

3箇所で類似実装:

Location Function
internal/directives/deriver/directive.go extractFunc()
internal/context/context.go ExtractCallFunc()
internal/directives/spawner/directive.go GetFuncFromCall()

全て *ast.CallExpr*types.Func の変換。

統合案: CheckContext.ExtractCallFunc() に統一、他は削除してこれを使う


優先度

  1. FuncSpec 統合 - 明確な重複、効果大
  2. ExtractCallFunc 統合 - 3箇所 → 1箇所

統合する?

- Remove CallbackReceivesCtx (dead code, ants support deferred)
- Fix HasPrefix bug: conc/pool was incorrectly matching conc
- Add MatchPkg to typeutil for proper version suffix handling (/v2, /v3)
- Add conc test fixtures to verify generic type support (ResultPool[T], etc.)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 22, 2025

追加変更 (96dc772)

削除

  • CallbackReceivesCtx パターン削除(デッドコード)
    • ants サポートは費用対効果が低いため見送り(ジェネリクス型パラメータ判定が複雑)

バグ修正

  • パッケージマッチングの HasPrefix バグを修正
    • 問題: conc/poolconc に誤マッチ
    • 原因: strings.HasPrefix("github.com/sourcegraph/conc/pool", "github.com/sourcegraph/conc") = true
    • 修正: typeutil.MatchPkg() で厳密一致 + バージョンサフィックス (/v2, /v3) のみ許可

追加

  • conc テストケース追加
    • ジェネリクス型 (ResultPool[T], ResultContextPool[T], ResultErrorPool[T]) の動作確認
    • 8種類の Pool API をカバー

Rewrites spawnerlabel checker to use SSA analysis for detecting spawn
calls in nested closures, IIFEs, and higher-order function returns.

Changes:
- Add SSA Program to spawnerlabel Checker struct
- Implement findSpawnCallSSA() for SSA-based traversal
- Add checkReturnedFuncForSpawn() for higher-order return tracking
- Add FindFuncDecl() to internal/ssa for FuncDecl lookup
- Fix generic function handling using fn.Origin()
- Fix variadic function argument handling (slice of functions)
- Move SSA build to main run() function for shared use

Resolved limitations:
- Nested FuncLit spawn now detected (badNestedFuncLitSpawn)
- IIFE spawn now detected (badIIFEWithSpawn)
- Defer IIFE spawn now detected (badDeferSpawnNestedScope)

New limitation:
- Unreachable code after panic not detected (SSA eliminates it)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 22, 2025

SSA-based spawnerlabel Implementation

This commit implements SSA-based spawn detection for the spawnerlabel checker, resolving several previous limitations.

Key Changes

Core Implementation:

  • Added SSA Program to spawnerlabel Checker struct
  • Implemented findSpawnCallSSA() for SSA-based traversal handling:
    • Regular calls (*ssa.Call)
    • Deferred calls (*ssa.Defer)
    • Go statements (*ssa.Go)
    • Closures/MakeClosure (*ssa.MakeClosure)
    • IIFEs (Immediately Invoked Function Expressions)
  • Added checkReturnedFuncForSpawn() for higher-order function return tracking
  • Added FindFuncDecl() to internal/ssa/builder.go

Bug Fixes:

  • Fixed generic function handling using fn.Origin() for instantiated generics
  • Fixed variadic function argument handling (detecting []func(...) slices)

Resolved Limitations

Before After Description
limitationNestedFuncLitSpawn badNestedFuncLitSpawn SSA traverses nested closures
limitationIIFEWithSpawn badIIFEWithSpawn SSA traverses IIFEs
goodDeferSpawnNestedScope badDeferSpawnNestedScope SSA traverses deferred IIFEs

New Limitation

  • limitationPanicBeforeSpawn: SSA eliminates unreachable code after panic(), so spawn calls after panic are not detected

Test Results

All tests pass including metatest validation.

- Add `-conc` analyzer flag to enable/disable conc checker
- Add support for conc/iter APIs: ForEach, ForEachIdx, Map, MapErr
- Add support for iter.Iterator and iter.Mapper types
- Add support for conc/stream.Stream.Go
- Remove dead AST fallback code from spawnerlabel checker
- Update documentation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 22, 2025

進捗と引き継ぎ

完了したこと

  • ✅ SSA ベースの spawnerlabel 実装
  • -conc フラグ追加 (conc checker の有効/無効切り替え)
  • ✅ conc/iter API 対応 (ForEach, ForEachIdx, Map, MapErr)
  • ✅ conc/stream API 対応 (Stream.Go)
  • ✅ dead code 削除 (AST フォールバック)

次のタスク: Registry リファクタリング

現在の問題:

  • 1つの API に対して 1つの Pattern しか登録できない
  • ctx キャプチャと deriver チェックが混在した Pattern になっている

提案する設計変更:

// 現状
reg.Register(pattern, api1, api2, ...)  // 1 Pattern → N APIs

// 変更後
reg.Register(api, patterns...)  // 1 API → N Patterns

これにより:

  • ClosureCapturesCtx (ctx キャプチャチェック)
  • ClosureCallsDeriver (deriver 呼び出しチェック)

を独立した概念として分離し、API ごとに必要なチェックを組み合わせられる。

例:

  • errgroup.Group.Go: [ClosureCapturesCtx] (通常時)
  • errgroup.Group.Go: [ClosureCapturesCtx, ClosureCallsDeriver] (-goroutine-deriver 設定時)
  • gotask.DoAllFns: [CallbackCallsDeriver] (ctx はパラメータなのでキャプチャ不要)

変更対象ファイル

  • internal/registry/registry.go - Entry 構造体を Patterns []Pattern に変更
  • internal/patterns/ - ClosureCallsDeriver パターンを追加
  • internal/apis.go - API 登録方法を変更
  • internal/runner.go - 複数 Pattern のチェックロジックを追加

Refactor the Pattern interface to use a cleaner signature:
- Pattern.Check(cctx, callbackArg, taskArg) instead of 5 arguments

Introduce clear type hierarchy with *Config suffix for static configs:
- TaskConstructorConfig: defines how tasks are constructed (NewTask etc.)
- TaskArgumentConfig: combines Constructor + Idx (argument position)
- TaskArgument: combines Call (dynamic) + Config (static)

This separates static API-level config from dynamic per-call info:
- Static: TaskArgumentConfig (from registry.API definition)
- Dynamic: TaskArgument.Call (the actual call expression)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 23, 2025

Pattern Interface Refactoring Design

Problem

Current Pattern.Check(cctx, callbackArg, taskArg) has semantic issues:

  • For errgroup.Go: callbackArg = closure, taskArg = nil ✓
  • For task.DoAsync: callbackArg = ctx (not a callback!), taskArg = {Call, Config}

The name callbackArg containing ctx is misleading.

Solution: Wrapper Pattern with Separated Interfaces

Split into purpose-specific interfaces:

// For errgroup.Go, DoAllFns - checks the callback argument directly
type CallArgPattern interface {
    Name() string
    Check(cctx *context.CheckContext, arg ast.Expr) bool
    Message(apiName, ctxName string) string
}

// For task.DoAsync - traces receiver back to constructor
type TaskSourcePattern interface {
    Name() string
    Check(tcctx *TaskCheckContext, call *ast.CallExpr) bool
    Message(apiName, ctxName string) string
}

TaskCheckContext embeds CheckContext + constructor config:

type TaskCheckContext struct {
    *context.CheckContext
    Constructor *TaskConstructorConfig  // How to trace back (e.g., NewTask)
}

Key Simplifications

  1. Remove TaskReceiverIdx (-1 magic number): Task-source patterns always use receiver (standard library design)
  2. Remove TaskArgumentConfig: Only TaskConstructorConfig needed
  3. Remove TaskArgument: Runner builds TaskCheckContext instead

Runner Wrapper

type PatternRunner interface {
    Run(cctx *CheckContext, call *ast.CallExpr, api *API) (ok bool, msg string)
}

// Wraps CallArgPattern
type callArgRunner struct { pattern CallArgPattern }

// Wraps TaskSourcePattern  
type taskSourceRunner struct { pattern TaskSourcePattern }

This eliminates type assertions while maintaining type safety.

mpyw and others added 3 commits December 23, 2025 09:14
…askSourcePattern

Separated pattern interfaces by use case for cleaner semantics:

- CallArgPattern: for patterns that check callback arguments directly
  (errgroup.Go, DoAllFns, DoAll with task tracing)
- TaskSourcePattern: for patterns that trace task receivers back to constructors
  (task.DoAsync where task is always the method receiver)
- GoStmtPattern.CheckGoStmt renamed to Check for consistency

Key changes:
- Removed TaskArgumentConfig, TaskArgument, TaskReceiverIdx (simplified)
- Added TaskCheckContext embedding CheckContext + Constructor
- Registry now has separate RegisterCallArg and RegisterTaskSource methods
- API.TaskArgConfig replaced with API.TaskConstructor (simpler)
- MatchFunc now returns *API instead of *Entry

This fixes the semantic issue where `callbackArg` contained `ctx` for DoAsync,
which was misleading. Now DoAsync uses TaskSourcePattern with proper semantics.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove API struct and APIKind enum from registry package
- Use funcspec.Spec directly in CallArgEntry and TaskSourceEntry
- Add FullName() method to funcspec.Spec for message formatting
- Update Spec.Matches() to use typeutil.MatchPkg for version suffix support
- Simplify registry matching logic using ExtractFunc + Spec.Matches
- Add FuncMatch struct for spawnerlabel detection
- Add CheckerName() method to all pattern interfaces
- Consolidate SSA helpers (ExtractCalledFunc, ExtractIIFE, HasFuncArgs)
- Add ShortPkgName helper to typeutil package
- Merge detector.go into checker.go in spawnerlabel package

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 23, 2025

API Struct Refactoring Complete

Replaced registry.API struct with funcspec.Spec for a cleaner, more unified design.

Key Changes

Removed:

  • registry.API struct and APIKind enum
  • Duplicate SSA helper functions
  • internal/spawnerlabel/detector.go (merged into checker.go)

Added:

  • funcspec.Spec.FullName() for message formatting
  • funcspec.Spec.Matches() now uses MatchPkg for v2/v3 version suffix support
  • registry.FuncMatch struct for spawnerlabel detection
  • CheckerName() method to all pattern interfaces
  • Shared SSA helpers in internal/ssa/helpers.go
  • typeutil.ShortPkgName() helper

Simplified:

  • Entry structs now embed funcspec.Spec directly instead of API
  • Registry matching uses ExtractFunc + Spec.Matches (removed matchAPI, isMethodCall, isFuncCall, matchFuncToAPI)
  • Registration in apis.go is now more straightforward

Stats

15 files changed, 371 insertions(+), 731 deletions(-)

Net reduction of ~360 lines while maintaining all functionality.

…creation

Each Register function now creates its own patterns internally,
removing the need to pass pattern instances from the caller.

Before:
  errgroupPatterns := []patterns.CallArgPattern{...}
  internal.RegisterErrgroupAPIs(reg, enabled, errgroupPatterns)

After:
  internal.RegisterErrgroupAPIs(reg, enabled)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 23, 2025

Remaining Refactoring Tasks

1. Split internal/context/context.go (855 lines)

Split into multiple files by responsibility (same package, same struct):

  • context.go - CheckContext struct and basic helpers (VarOf, FileOf, FuncOf, FuncDeclOf)
  • capture.go - Context capture detection (FuncLitCapturesContext, FuncLitUsesContext, nodeReferencesContext)
  • factory.go - Factory/return analysis (FactoryReturnsContextUsingFunc, BlockReturnsContextUsingFunc)
  • assignment.go - Assignment tracking (FuncLitAssignedTo, CallExprAssignedTo, funcLitInAssignment)
  • composite.go - Struct/index access (SelectorExprCapturesContext, IndexExprCapturesContext)

2. Clean up analyzer.go / internal/analyzer.go split

Clarify responsibility boundaries between top-level and internal analyzer.

3. Check for duplicate code in patterns

isTaskConstructorCall may be duplicated across pattern files.

Split the 855-line context.go into 5 focused files:
- context.go (79 lines): Struct + core helpers
- capture.go (196 lines): Context capture detection
- assignment.go (137 lines): Assignment tracking
- factory.go (210 lines): Factory/return analysis
- composite.go (260 lines): Composite type access

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 23, 2025

context.go ファイル分割完了

855行あった internal/context/context.go を責務ごとに5ファイルに分割:

File Lines Purpose
context.go 79 Struct + core helpers
capture.go 196 Context capture detection
assignment.go 137 Assignment tracking
factory.go 210 Factory/return analysis
composite.go 260 Composite type access

Next: analyzer.go + internal/analyzer.go の責務整理

…runASTChecks

- Rename internal/analyzer.go to internal/runner.go for clarity
- Move contextScope to internal/context/scope.go as exported Scope type
- Inline runASTChecks into analyzer.go's run function
- Remove duplicate buildFuncScopes, findContextScope, findEnclosingScope

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 23, 2025

Session 7: Analyzer File Cleanup

Changes Made

  1. Renamed internal/analyzer.gointernal/runner.go

    • Eliminates confusion between root analyzer.go and internal/analyzer.go
    • Better reflects its purpose as the "runner" for AST checks
  2. Created internal/context/scope.go

    • Moved contextScope struct to context.Scope
    • Moved buildFuncScopescontext.BuildFuncScopes
    • Moved findEnclosingScopecontext.FindEnclosingScope
    • Context scope detection now properly belongs in the context package
  3. Inlined runASTChecks in analyzer.go

    • Was a thin wrapper function that just created and ran the Runner
    • Inlining makes the flow clearer in the main run function

File Structure After

analyzer.go              # Main analyzer (flags, configuration, orchestration)
internal/
  runner.go              # Runner struct and check execution logic
  context/
    context.go           # CheckContext core (79 lines)
    capture.go           # Context capture detection (196 lines)
    assignment.go        # Assignment tracking (137 lines)
    factory.go           # Factory/return analysis (210 lines)
    composite.go         # Composite type access (260 lines)
    scope.go             # Scope detection (NEW, 89 lines)

Remaining TODOs

  • None for file organization

…all APIs

- Create deriver.Matcher once in analyzer.go, pass to internal functions
- Rename matcher to derivers for readability (derivers.SatisfiesAnyGroup)
- Remove enabled bool parameters from internal register functions
- Apply goroutine-deriver check to errgroup/waitgroup/conc APIs
  (these spawn goroutines, so deriver check is appropriate)
- Add buildCallArgPatterns helper for consistent pattern creation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 23, 2025

Session 7 (continued): API Registration Cleanup

Changes Made

  1. Absorbed enabled flags in analyzer.go

    • Before: internal.RegisterErrgroupAPIs(reg, enabled)
    • After: if enableErrgroup { internal.RegisterErrgroupAPIs(reg, derivers) }
  2. Applied goroutine-deriver to all goroutine-spawning APIs

    • errgroup.Group.Go/TryGo
    • sync.WaitGroup.Go
    • conc Pool/WaitGroup/Stream/Iterator APIs
    • gotask (already had this)

    These all spawn goroutines, so when -goroutine-deriver is set, they should all require the deriver call.

  3. Naming: matcherderivers

    • More readable: derivers.SatisfiesAnyGroup()

File Changes

analyzer.go:
  - buildRegistry() creates `derivers *deriver.Matcher` once
  - Calls internal functions conditionally based on enable flags
  
internal/apis.go:
  - All Register functions now accept `derivers *deriver.Matcher`
  - buildCallArgPatterns() helper adds ClosureCapturesCtx + CallbackCallsDeriver

mpyw and others added 2 commits December 23, 2025 10:36
- Merge RegisterGoroutinePattern and RegisterGoroutineDerivePattern
- Now consistent with other Register*APIs functions

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 23, 2025

Coverage Analysis: High Priority Improvements

Current overall coverage: 86.0%

Top 5 Priorities (based on detection accuracy impact)

# Function Coverage Why Important
1 gostmt.checkHigherOrder 50.0% Detects go fn()() patterns - common in real code
2 findConstructorCall 42.9% Core of gotask Task.DoAsync pattern detection
3 gostmt.checkFromAST 60.0% Fallback path when SSA unavailable
4 FactoryCallReturnsContextUsingFunc 66.7% Factory patterns like g.Go(makeWorker(ctx))
5 identIsDeriverCall 62.5% Variable reference deriver calls like fn := deriver; go fn()

Rationale

All 5 are core detection logic paths that directly affect false negatives. Low coverage here means potential missed detections in real codebases.

Excluded from priority:

  • 0% Name(), Message() getters - don't affect detection accuracy
  • High coverage (90%+) functions - diminishing returns

mpyw and others added 4 commits December 23, 2025 11:02
- Add variable factory patterns with context parameters
- Add index expression tests for slice function access
- Add pointer dereference DoAsync tests for gotask
- Improve coverage for checkFromAST (60% → 86.7%)
- Improve coverage for findConstructorCall (42.9% → 57.1%)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Add tests for:
- Map index expression with string key (covers token.STRING)
- Struct field selector patterns
- Triple higher-order factory chains (go fn()()())
- Package-level factory functions
- Factory from parameter limitation
- Returned value from external func

These tests improve coverage for:
- IndexExprCapturesContext
- SelectorExprCapturesContext
- funcLitOfLiteralKey
- FactoryCallReturnsContextUsingFunc
- IdentFactoryReturnsContextUsingFunc

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add tests for:
- IIFE containing spawn call (SSA traces into IIFE)
- Nested IIFE containing spawn call

These tests verify that the spawnerlabel checker correctly detects
spawn calls inside immediately invoked function expressions.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@mpyw
Copy link
Owner Author

mpyw commented Dec 23, 2025

Closing to reorganize commits into a cleaner PR

@mpyw mpyw closed this Dec 23, 2025
@mpyw mpyw deleted the refactor/ssa-pattern-registry branch December 24, 2025 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant