Thanks for the report and the code to reproduce the issue. It indeed looks like vet's analysis isn't sophisticated enough. It already builds a control flow graph, so hopefully the fix won't be too complex.

Here's a smaller repro:

package p

import "context"

func f(ctx context.Context) {
        var cancel func()
        defer func() { cancel() }()
        ctx, cancel = context.WithCancel(ctx)
}

If the defer is moved below the WithCancel line, vet works as expected. The issue here is that the lostcancel check never notices the cancel() call as a use right before the return (since that's how defers work).

It seems to me like this is simply that vet doesn't handle defers properly. The internal/cfg package doesn't treat them especially, and the check itself does nothing special either.

@alandonovan do you think this would be a cfg or a vet fix? For example, when building the cfg, we could move the deferred blocks to be right before the appropriate return statements. But I'm not sure if that would be outside the scope of what a control flow graph should be.

It just dawned on me that the cfg builder can't possibly do what I described above. For example, if we have:

if x {
    defer call()
}
return y

The CFG will end up with a single return statement, and statically it's impossible to know if there will be a defer or not. So it seems to me like the fix must be after the CFG has been built.

The error message is slightly confusing, but I think the real problem here is that you're deferring a function that uses an uninitialized variable. Admittedly WithCancel is not supposed to panic, but if it did, the deferred function would call a nil function. The right place for a cleanup operation is after the variable has been successfully initialized.

I'm not convinced it is worth complicating the checker logic to handle this case.

The original snippet does not have any uninitialized variables, and still produces the error

@Wessie yes, but along the same lines, the cancel function that gets called isn’t guaranteed to be the intended one.

discovered another variant that causes the same issues

type Thing struct {
  ctx context.Context
  cancel context.CancelFunc
}

func main() {
  ctx, cancel := context.WithCancel(parentCTX)
 
  // There is some unrelated code here.
  ...

  thing := Thing{
    ctx: ctx,
    cancel: cancel,
  }
}

Go vet doesn't complain from the above if the ctx, cancel is right next to the thing := Thing{} line though.

Yeah, this is the case. As long as you use cancel in any form or factor, vet won't complain. I just simply assign cancel to _

ctx, cancel := context.WithCancel(parentCTX)
// vet doesn't complain if I do this
_ = cancel

// Some code here

go func() {
    // cancel on some condition
    cancel()
}()

I also encountered the same issue. And _= cancel does not seem to help when cancel() is called inside a defer function.

The reproduction is exactly same as included in #58850 under "sophisticated example"

@jiangsunan Here is the sophisticated example in #58850 (comment) the playground https://go.dev/play/p/LpWXSoZ5Ck8. Here is the relevant bit:

			cancel()
			ctx, cancel = context.WithCancel(parent)
			// _ = cancel // Uncomment and vet will pass.
			// use ctx

If you uncomment the line _ = cancel in the playground, this now passes vet in 1.20 whereas it fails when this is uncommented. Does this help with "And _= cancel does not seem to help"?

I just ran into this as well. This is pretty clearly a bug in the linter / vetter.
The only risk here is for the same cancel() being called twice, which would only happen if context.WithCancel() panics.

Certainly it is a bug in the linter, and the tool could definitely be smarter about the ramifications of defer, but it's not a straightforward change. It needs a more sophisticated control-dependent analysis of defer and the state of local variables.

I continue to believe that the cases where the tool falls down are often the same places where humans also struggle to understand the code.