-
Notifications
You must be signed in to change notification settings - Fork 18.7k
Description
Proposal Details
api changes
go.mod
A new vettol type would be added, acting like the existing tool definitions,
but they must be vet tools invocable by the current go vet -vettool prog.
That is, they must be built on top of golang.org/x/tools/go/analysis/unitchecker.
// go.mod
vettool (
example.com/my/analysis/one
example.com/my/analysis/two
github.com/other/analysis
)
go vet
Running go vet would run the standard vet checks in cmd/vet,
in addition to any vet tools defined in go.mod as described above.
The -vettool flag currently only accepts absolute file paths.
It would change to work like go tool, resolving:
allto mean all declared vettoolsvettocmd/vet,- single element names and import paths to definitions in vettool
- absolute paths work as it currently does
go test
Currently, by default, go test invokes a subset of vet checks.
This proposal would not change the default subset of checks run during a test.
go test -vet=all would change include these externally defined checks.
A new go test -vet=vet would represent the standard vet checks (equivalent to the current -vet=all)
why
The bar for the Go project to add a check in to cmd/vet is quite high,
we require correctness, frequency, and precision across pretty much all of written Go code.
Individual projects are much more tightly scoped,
they can afford to be stricter, and more opinionated in the style and checks enforced.
And they frequently are, as seen by the proliferation of custom analyzers
and tools to run them in both OSS and enterprise.
All this means there's a fragmented ecosystem
with no standard way of running all the checks a project will require of its contributors.
For these projects, the additional analyzers they want to run will often be required and enforced in CI.
I think it would be great to eliminate the mess of having to install and run
potentially multiple external analysis tools and have them all under the umbrella of go vet,
since for the end user, there isn't much functional difference in which checks must pass.
I previously filed #71478, but running the analysis as tests seems wrong,
as the scope for tests is a package,
while analysis should be on the entire module.
security
The defaults I chose above mean go vet would now execute in control of the module being vetted (through the analyzers it declares) where it previously didn't.
To me, this is an ok tradeoff in terms of security vs convenience, as vet is commonly followed by test which by nature executes code.
If we wish to maintain the original security boundary, then vettool would default to -vettool=vet and the new behavior would be opt in with -vettool=all.