dominikh / go-tools

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

staticcheck: correctness checks for build constraints #188

Open dominikh opened 7 years ago

dominikh commented 7 years ago

(Copied from https://github.com/dominikh/go-tools/issues/137#issuecomment-330000748)

We should flag unsatisfiable build constraints, such as +build windows,linux. While rare in production code, it is a common mistake while writing code, especially for people who are not used to the way build constraints compose. For example, people often incorrectly assume that multiple +build instructions are ORed together, while really they are ANDed.

This will start out as a paid feature, primarily for organisational reasons. It will depend on Z3, we have existing Z3 work in the commercial branch, and it will complicate building the tools due to requiring cgo. The feature will, however, likely be backported to the open version eventually, either guarded behind an optional build tag, or by vendoring Z3, in a similar way to how go-sqlite3 vendors the SQLite sources, or by using a pure Go SAT solver in the open version.

ainar-g commented 3 years ago

I'm not sure if you're still planning on doing that (especially as a paid feature), but you should probably also include the analysis of the new go:build directives here.

dominikh commented 3 years ago

but you should probably also include the analysis of the new go:build directives here.

The switch to go:build might actually make this feature unnecessary. Users will be far less likely to make mistakes with go:build. Especially the confusion between AND and OR will go away.

especially as a paid feature

The "paid feature" stuff is a leftover from when I planned a "Staticcheck Pro". That no longer exists.

dominikh commented 1 year ago

It's also worth noting that we would only be able to flag unsatisfiable subexpressions of the constraint. Something like //go:build linux && windows will never make it to Staticcheck in the first place, because the Go build system won't include the file in the package. We'd only be able to flag it in something like //go:build linux || (windows && plan9).

We also really don't need Z3 for this. There are usually very few unique tags, and we can brute force all the combinations, especially if we manually encode the exclusivity of different GOOS and GOARCH.