search

dominikh / go-tools

Staticcheck - The advanced Go linter
https://staticcheck.dev
MIT License
6.09k stars 368 forks source link

stylecheck: decide fate of CheckBlankImports #484

Open dominikh opened 5 years ago

dominikh commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 4 years ago

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).