rules_go icon indicating copy to clipboard operation
rules_go copied to clipboard

Update go vet with latest analyzers

Open sluongng opened this issue 1 year ago • 5 comments

Upgrade golang.org/x/tools to v0.18.0. Note that the latest v0.19.0 has a bug that we should avoid using.

Update the analyzers (passes) in TOOLS_NOGO and the ones automatically included when vet = True is configured. The vet analyzers are taken from go tool vet help documentation.

sluongng avatar Mar 26 '24 13:03 sluongng

The nogo failure looks like it's easier to fix than ignore.

fmeum avatar Mar 26 '24 13:03 fmeum

@fmeum is it though?

The fieldalignment analyzer is quite noisy and is triggered on proto-generated code? 🤔

Im in favor of disabling it with a TODO.

sluongng avatar Mar 26 '24 13:03 sluongng

@fmeum is it though?

The fieldalignment analyzer is quite noisy and is triggered on proto-generated code? 🤔

Im in favor of disabling it with a TODO.

True, I missed that this was the generated code, not the test. In that case I agree that disabling it by default is the right move.

fmeum avatar Mar 26 '24 13:03 fmeum

Yeah. The other option is to add a relevant nogo_config.json file to exclude proto-generated files for that analyzer.

I don't think it's worth the noise it gonna generate for downstream users though.

sluongng avatar Mar 26 '24 13:03 sluongng

Hmm, these dependencies upgrades turn out to be a huge PITA.

Especially, for the popular_repo test, I think we should consider dropping x/tools testing entirely. The repo is where gopls is being actively developed and the recent code quality is not that high. v0.19.0 contains a nil pointer bug and a bunch of build/test targets in that repo are not gazelle/rules_go compatible.

I also had to patch Gazelle temporarily to avoid analyzer duplication. It should be a temp workaround until (a) we remove that in Gazelle, then (b) release a new Gazelle version, and then (c) upgrade the Gazelle version in rules_go.

sluongng avatar Mar 27 '24 10:03 sluongng