staticcheck: detect unclosed resources

[dominikh]

Many APIs have a Close or Stop method that has to be called in order to avoid leaks. Tickers, file handles, database transactions, and more.

We should detect cases where calls to these are missing. The general case isn't a trivial problem, due to infeasible paths, but we may be able to restrict it to a simpler subset, and tolerate a higher amount of false negatives.

This issue merges https://github.com/dominikh/go-staticcheck/issues/68 and #139.

[c9845]

@dominikh Responding to your note in #1341... Yes, I agree this is a difficult thing to check for. My initial though was something like looking for a defer "close-keyword" within x-many lines after an "open-keyword" but it (1) requires maintaining a list of keywords for other packages which is impossible, and (2) would lead to a lot of false-positives and -negatives due the the arbitrary line count. Then again, a warning requiring a //lint line is better than no warning at all.

Maybe this is something more for the go compiler to catch.

[dominikh]

a warning requiring a //lint line is better than no warning at all

For Staticcheck, I strongly disagree. Our goal is to have approximately zero false positives, at the cost of false negatives.

This is even less of a fit for the Go compiler; it isn't the compiler's job to reject code because it might be wrong.

Your proposed implementation fails to take into consideration that (among other things) opening and closing a resource can happen in entirely different places. An open resource can be passed to another function, or stored in a struct field, etc. It is impossible to track this accurately without either false positives or false negatives.

Even within a single function, opening and closing of resources may happen on branches. To avoid false negatives, you would have to ensure that an opened resource gets closed on all possible paths through the function. To avoid false positives, however, you would need to restrict the set of paths to those that are feasible, which itself isn't doable without false positives.

False positives are not an option for Staticcheck, the Go compiler, or go vet, and an implementation that favours false negatives will only match trivial cases.

In a language like Go, it is much more feasible to detect resource leaks at runtime.