Go-tools: stylecheck: decide fate of CheckBlankImports

[dominikh]

CheckBlankImports flags blank imports that aren't in a main or a test package and aren't documented. The check is fully implemented but not made available. Decide its fate.

[cespare]

I think this is high-signal but low-incidence. So I'd vote to make it available, but I don't feel strongly about it.

[dominikh]

When I wrote the check, it seemed high-incidence and low-signal to me. How often do you see people blank-import something they shouldn't? Whenever I see (undocumented) blank imports, they're things like database/sql drivers or image file formats.

[cespare]

How often do you see people blank-import something they shouldn't?

Almost never. I do remember one case where an underscore import (for one of the pprof packages) was unintentionally left in our code for a while. The thing about those is that tools like goimports can't ever remove them, so they have a way of sticking around forever.

Whenever I see (undocumented) blank imports, they're things like database/sql drivers or image file formats.

Yeah, I suppose different kinds of code from what I work on might exhibit more false positives. FWIW, I would consider both database/sql drivers or image file formats to be cases where explanatory comments are warranted, but I don't read a lot of code that needs to use these kinds of underscore imports.

[bcmills]

Things like database/sql drivers and image formats seem like true-positive issues: why should a library make an assumption about which SQL driver(s) the overall program is using? That needlessly bloats tests of dependent libraries whose behavior does not depend on the specific driver, or (especially) tests of dependent libraries that intentionally use a different driver (such as substituting a lightweight in-memory database for a slow external one).