Skip to content

timesleepnocontext: stop crossing non-go/defer FuncLit boundaries when attributing time.Sleep#42946

Merged
pelikhan merged 4 commits into
mainfrom
copilot/fix-timesleepnocontext-issue
Jul 2, 2026
Merged

timesleepnocontext: stop crossing non-go/defer FuncLit boundaries when attributing time.Sleep#42946
pelikhan merged 4 commits into
mainfrom
copilot/fix-timesleepnocontext-issue

Conversation

Copilot AI commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

timesleepnocontext was attributing time.Sleep to the first outer ctx-bearing function across arbitrary FuncLit boundaries, which can misfire in callback closures and suggest the wrong cancellation source. This change preserves intended goroutine/defer behavior while preventing outer-ctx attribution for ordinary callbacks.

  • Analyzer boundary semantics

    • Updated the enclosing-scope walk in pkg/linters/timesleepnocontext/timesleepnocontext.go.
    • When encountering a FuncLit without its own context.Context parameter:
      • continue outward only if the literal is the direct callee of go or defer
      • otherwise stop at that boundary (no outer ctx attribution).
    • Kept reporting for:
      • functions/closures with their own ctx param
      • go func(){...}() and defer func(){...}() closures that share outer lifetime.
  • AST parent-walk safety

    • Added guard logic in go/defer closure detection to avoid nil parent-node assumptions during parent/grandparent inspection.
  • Fixture coverage updates

    • Extended testdata/src/timesleepnocontext/timesleepnocontext.go with:
      • Good: http.HandleFunc callback nested in ctx function (time.Sleep should not be flagged).
      • Good: synchronous callback arg (register(func(){...})) nested in ctx function (no diagnostic).
      • Bad: deferred closure in ctx function (defer func(){ time.Sleep(...) }() still flagged).
for encl := range cur.Enclosing((*ast.FuncDecl)(nil), (*ast.FuncLit)(nil)) {
    funcType := enclosingFuncType(encl.Node())
    ctxParamName, hasCtx := contextParamName(pass, funcType)

    if !hasCtx {
        if _, isFuncLit := encl.Node().(*ast.FuncLit); isFuncLit && !isGoOrDeferClosure(encl) {
            break // stop at ordinary callback boundary
        }
        continue // keep walking for go/defer closures
    }

    // report with ctxParamName
}

Copilot AI and others added 2 commits July 2, 2026 12:15
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix false positives in timesleepnocontext analyzer timesleepnocontext: stop crossing non-go/defer FuncLit boundaries when attributing time.Sleep Jul 2, 2026
Copilot AI requested a review from pelikhan July 2, 2026 12:19
@pelikhan pelikhan marked this pull request as ready for review July 2, 2026 12:22
Copilot AI review requested due to automatic review settings July 2, 2026 12:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request updates the timesleepnocontext analyzer’s enclosing-scope walk so time.Sleep attribution does not cross ordinary callback FuncLit boundaries, avoiding incorrect “outer ctx” suggestions while preserving attribution for go/defer closures that share the outer lifetime.

Changes:

  • Stop the enclosing walk at non-go/defer *ast.FuncLit boundaries when that literal does not define its own context.Context parameter.
  • Add isGoOrDeferClosure to identify go func(){...}() / defer func(){...}() closures during the walk.
  • Extend analyzer fixtures with “good callback” cases and an additional “bad defer” case.
Show a summary per file
File Description
pkg/linters/timesleepnocontext/timesleepnocontext.go Updates enclosing-scope attribution semantics and introduces go/defer-closure detection helper.
pkg/linters/timesleepnocontext/testdata/src/timesleepnocontext/timesleepnocontext.go Adds fixtures covering ordinary callbacks inside ctx-bearing functions and deferred closures that should still be flagged.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment thread pkg/linters/timesleepnocontext/timesleepnocontext.go
@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot please run the pr-finisher skill. The branch looks ready for a final pass: review the remaining Copilot review feedback, resolve any still-open review threads, and rerun checks if you make follow-up changes.

Generated by 👨‍🍳 PR Sous Chef · 21.1 AIC · ⌖ 16 AIC · ⊞ 3.1K ·

…er fixtures

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot AI commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please run the pr-finisher skill. The branch looks ready for a final pass: review the remaining Copilot review feedback, resolve any still-open review threads, and rerun checks if you make ...

Addressed the open review threads in commit fix: unwrap ParenExpr in isGoOrDeferClosure; add parenthesized go/defer fixtures:

  • isGoOrDeferClosure fix: walks up through any *ast.ParenExpr nodes above the FuncLit before checking for a CallExpr, then unwraps parens from call.Fun to verify the callee resolves to the FuncLit. This handles defer (func(){...})() and go (func(){...})() correctly.
  • Fixtures: added BadDeferParenWithCtx and BadGoParenWithCtx test cases with parenthesized closure forms; both produce the expected diagnostic.

Local make fmt, make agent-report-progress (lint + tests) all pass.

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

🤖 PR Triage

Field Value
Category bug
Risk 🟢 Low
Score 53 / 100
Action ✅ fast_track

Score breakdown: Impact 22 · Urgency 15 · Quality 16

CI running (1 check in progress). Targeted fix to timesleepnocontext linter boundary semantics — prevents incorrect ctx attribution across non-go/defer FuncLit boundaries. Includes test data coverage. Fast-track once CI completes.

Run §28591321991

Generated by 🔧 PR Triage Agent · 76 AIC · ⌖ 9.74 AIC · ⊞ 1.6K ·

@pelikhan pelikhan merged commit b72facd into main Jul 2, 2026
29 checks passed
@pelikhan pelikhan deleted the copilot/fix-timesleepnocontext-issue branch July 2, 2026 15:07
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.82.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

4 participants