(I edited the submission to fix the formatting of the Go code.)

@mvdan @alandonovan

Thankfully not a regression from 1.11 :) A conservative fix here might be simple enough to add to 1.12.

Seems to me like the simplest conservative fix would be to turn off this warning in the general ctx2, _ := context.WithFoo(ctx, ...), where we know ctx not to be the background context. However, this analyzer is a bit complex, so I'll let @alandonovan decide what the best fix would be.

the simplest conservative fix would be to turn off this warning in the general ctx2, _ := context.WithFoo(ctx, ...), where we know ctx not to be the background context.

That would substantially diminish the value of the check: it's probably the common case that ctx is not Background.

This is one of those cases in which a higher-level invariant of the program that is beyond the scope of the tool ensures that the apparent problem is benign. I suggest adding a workaround:

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
if true {
  ctx, unused = context.WithTimeout(ctx, time.Second)
  _ = unused // pacify vet lostcancel check: ctx is always canceled through its parent
}

I'm not sure I agree. The program initially posted is correct in my mind, so I don't see why it should be annotated. Seems to me like it's vet that should get smarter about false positives.

Unless you're talking about the more general case, where we don't know if the parent cancel func is called properly:

func Foo(ctx context.Context) {
    // do all the Foo callers cancel ctx above properly?
    ctx2, _ := context.WithTimeout(ctx, time.Second)
    Bar(ctx2)
}

I'd probably agree that in the generic case where you can't know if the parent will be cancelled properly, the child's cancel func being discarded should be very obvious. But the program originally posted has no such ambiguity.

Yes, the program is correct and the diagnostic is a false positive, but vet checks are nearly all heuristics, so we can't realistically fix every false positive. There has to be a cost/benefit calculation. In this example, the relationship between the two contexts is evident within the body of a single function, so it's probably feasible to expand the scope of the algorithm. However: it's already not a trivial piece of logic; minor refactorings could easily obscure this relationship; and this is the first report of this problem we've had in 2.5 years since the check was added, and there's an easy workaround. So I'm inclined not to spend more effort on it yet.

Marking as "help wanted", to leave the issue to someone who cares enough to write a fix.

this is the first report of this problem we've had in 2.5 years since the check was added, and there's an easy workaround. So I'm inclined not to spend more effort on it yet.

That's a good point. I hadn't realised that the lostcancel check was like this for years.

I have a similar issue that shows up when using a BDD test tool like ginkgo. The standard practice there is do setup and teardown in separate contexts, meaning you might have:

BeforeEach(func() {
    ctx, cancel = context.WithCancel(context.Background)
}

JustBeforeEach(func() {
    output, err = FunctionUnderTest(ctx)
})

AfterEach(func() {
    cancel()
})

If the function under test is synchronous, then you can put both of these steps into JustBeforeEach and satisfy go/vet, but if FunctionUnderTest is intended to still be running during your assertions, that's not an option. Any thoughts on workarounds for this situation? Alternatively, is there any way run go/vet but to disable this specific check? It'd be nice to be able to do that when linting test files (but of course this is a useful checl that I'd want on my actual code).

I'm hitting this, too. I already use defer cancel() for my parent context, which closes all Done channels of the child context but the linter doesn't accept it.

I went with using // nolint:govet for that specific line.