Skip to content

fix: track conditional reassignments in errgroup/spawner checkers#23

Merged
mpyw merged 2 commits intomainfrom
fix/errgroup-conditional-reassignment
Dec 26, 2025
Merged

fix: track conditional reassignments in errgroup/spawner checkers#23
mpyw merged 2 commits intomainfrom
fix/errgroup-conditional-reassignment

Conversation

@mpyw
Copy link
Owner

@mpyw mpyw commented Dec 26, 2025

Summary

Problem

Same issue as the goroutine checker: when a variable is conditionally reassigned:

func bad(ctx context.Context) {
    g := new(errgroup.Group)
    fn := func() error {
        fmt.Println("no ctx")
        return nil
    }
    if condition {
        fn = func() error {
            _ = ctx
            return nil
        }
    }
    g.Go(fn)  // No warning - but should warn!
}

The analyzer only checked the last assignment and would incorrectly pass if the conditional branch used context while the unconditional assignment didn't.

Solution

  • Added FuncLitAssignment struct with Conditional flag
  • Use inspector.WithStack to detect if assignment is inside control structure
  • Check ALL assignments from last unconditional onwards
  • ALL must capture context for the check to pass

Test plan

  • All existing tests pass
  • Added 3 new test cases for conditional reassignment patterns in errgroup
    • badConditionalReassignFirstUsesCtx: First uses ctx, conditional doesn't → should warn
    • badConditionalReassignFirstNoCtx: First doesn't use ctx, conditional does → should warn
    • goodConditionalReassignAllUseCtx: All assignments use ctx → should NOT warn

🤖 Generated with Claude Code

Same issue as goroutine checker: FuncLitOfIdent only returned the last
assignment, missing conditional reassignment patterns. Now errgroup,
waitgroup, conc, and spawner checkers properly track all assignments
and verify ALL reachable paths capture context.

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

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

codecov bot commented Dec 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.18%. Comparing base (1069525) to head (0e6eafc).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #23      +/-   ##
==========================================
+ Coverage   83.96%   84.18%   +0.22%     
==========================================
  Files          24       24              
  Lines        1808     1834      +26     
==========================================
+ Hits         1518     1544      +26     
  Misses        173      173              
  Partials      117      117              
Flag Coverage Δ
unittests 84.18% <100.00%> (+0.22%) ⬆️

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.

Resolved conflict in internal/probe/assignment.go:
- Kept FuncLitsAssignedTo from main
- Removed duplicate FuncLitAssignmentsOfIdent (already added in main via PR #24)
@mpyw mpyw merged commit e8659b1 into main Dec 26, 2025
14 checks passed
@mpyw mpyw deleted the fix/errgroup-conditional-reassignment branch December 26, 2025 03:37
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