search

googleapis / google-cloud-go

Google Cloud Client Libraries for Go.
https://cloud.google.com/go/docs/reference
Apache License 2.0
3.72k stars 1.27k forks source link

add staticcheck as a presubmit check #10779

Open julieqiu opened 2 weeks ago

julieqiu commented 2 weeks ago

staticcheck is a common linter for Go projects. We should consider adding it as a presubmit check for PRs. See https://github.com/googleapis/gapic-generator-go/issues/1404 and https://github.com/googleapis/gapic-showcase/issues/1529 for related issues.

This first requires fixing these issues in this repository:

$ staticcheck ./...                                                                         
debugger/apiv2/controller2_client.go:23:2: "io/ioutil" has been deprecated since Go 1.19: As of Go 1.16, the same functionality is now provided by package [io] or package [os], and those implementations should be preferred in new code. See the specific function documentation for details.  (SA1019)
debugger/apiv2/controller2_client.go:53:3: internaloption.WithDefaultEndpoint is deprecated: WithDefaultEndpoint does not support setting the universe domain. Use WithDefaultEndpointTemplate and WithDefaultUniverseDomain to compose the default endpoint instead.  (SA1019)
debugger/apiv2/controller2_client.go:386:3: internaloption.WithDefaultEndpoint is deprecated: WithDefaultEndpoint does not support setting the universe domain. Use WithDefaultEndpointTemplate and WithDefaultUniverseDomain to compose the default endpoint instead.  (SA1019)
debugger/apiv2/debugger2_client.go:23:2: "io/ioutil" has been deprecated since Go 1.19: As of Go 1.16, the same functionality is now provided by package [io] or package [os], and those implementations should be preferred in new code. See the specific function documentation for details.  (SA1019)
debugger/apiv2/debugger2_client.go:55:3: internaloption.WithDefaultEndpoint is deprecated: WithDefaultEndpoint does not support setting the universe domain. Use WithDefaultEndpointTemplate and WithDefaultUniverseDomain to compose the default endpoint instead.  (SA1019)
debugger/apiv2/debugger2_client.go:394:3: internaloption.WithDefaultEndpoint is deprecated: WithDefaultEndpoint does not support setting the universe domain. Use WithDefaultEndpointTemplate and WithDefaultUniverseDomain to compose the default endpoint instead.  (SA1019)
debugger/apiv2/debugger2_client.go:715:5: req.GetStripResults is deprecated: Do not use.  (SA1019)
debugger/apiv2/debugger2_client.go:716:48: req.GetStripResults is deprecated: Do not use.  (SA1019)
httpreplay/internal/proxy/debug.go:24:6: type debugTransport is unused (U1000)
httpreplay/internal/proxy/debug.go:29:25: func debugTransport.RoundTrip is unused (U1000)
httpreplay/internal/proxy/debug.go:46:6: func logHeaders is unused (U1000)
internal/btree/btree.go:579:33: field byte is unused (U1000)
internal/btree/btree_test.go:32:2: rand.Seed has been deprecated since Go 1.20 and an alternative has been available since Go 1.0: As of Go 1.20 there is no reason to call Seed with a random value. Programs that call Seed with a known value to get a specific sequence of results should use New(NewSource(seed)) to obtain a local random generator.  (SA1019)
internal/testutil/server_test.go:32:15: grpc.Dial is deprecated: use NewClient instead.  Will be supported throughout 1.x.  (SA1019)
internal/testutil/server_test.go:32:35: grpc.WithInsecure is deprecated: use WithTransportCredentials and insecure.NewCredentials() instead. Will be supported throughout 1.x.  (SA1019)
rpcreplay/example_test.go:34:15: grpc.Dial is deprecated: use NewClient instead.  Will be supported throughout 1.x.  (SA1019)
rpcreplay/example_test.go:47:15: grpc.Dial is deprecated: use NewClient instead.  Will be supported throughout 1.x.  (SA1019)
rpcreplay/rpcreplay.go:372:3: grpc.WithBlock is deprecated: this DialOption is not supported by NewClient. Will be supported throughout 1.x.  (SA1019)
rpcreplay/rpcreplay.go:393:15: grpc.Dial is deprecated: use NewClient instead.  Will be supported throughout 1.x.  (SA1019)
rpcreplay/rpcreplay.go:394:28: grpc.WithInsecure is deprecated: use WithTransportCredentials and insecure.NewCredentials() instead. Will be supported throughout 1.x.  (SA1019)
rpcreplay/rpcreplay_test.go:259:15: grpc.Dial is deprecated: use NewClient instead.  Will be supported throughout 1.x.  (SA1019)
rpcreplay/rpcreplay_test.go:260:28: grpc.WithInsecure is deprecated: use WithTransportCredentials and insecure.NewCredentials() instead. Will be supported throughout 1.x.  (SA1019)
rpcreplay/rpcreplay_test.go:461:17: grpc.DialContext is deprecated: use NewClient instead.  Will be supported throughout 1.x.  (SA1019)
rpcreplay/rpcreplay_test.go:461:74: grpc.WithInsecure is deprecated: use WithTransportCredentials and insecure.NewCredentials() instead. Will be supported throughout 1.x.  (SA1019)
rpcreplay/rpcreplay_test.go:551:17: grpc.DialContext is deprecated: use NewClient instead.  Will be supported throughout 1.x.  (SA1019)
rpcreplay/rpcreplay_test.go:551:74: grpc.WithInsecure is deprecated: use WithTransportCredentials and insecure.NewCredentials() instead. Will be supported throughout 1.x.  (SA1019)
rpcreplay/rpcreplay_test.go:572:16: grpc.DialContext is deprecated: use NewClient instead.  Will be supported throughout 1.x.  (SA1019)
rpcreplay/rpcreplay_test.go:572:73: grpc.WithInsecure is deprecated: use WithTransportCredentials and insecure.NewCredentials() instead. Will be supported throughout 1.x.  (SA1019)
third_party/pkgsite/synopsis.go:64:5: the surrounding loop is unconditionally terminated (SA4004)
codyoss commented 2 weeks ago

@julieqiu We do actually run staticcheck, but we ignore some warnings from it today. See: https://github.com/googleapis/google-cloud-go/blob/1e21bcf262bca9781fd42e357cdddf0ed9ad9f1e/.github/workflows/vet.sh#L82-L90

quartzmo commented 2 weeks ago

@codyoss Do you think any of the current ignores could be addressed and removed at this time?

quartzmo commented 2 weeks ago

@julieqiu: #10780 removes some of these SA1019 failures. Did you run into trouble trying to remove the remaining SA1019 failures?

codyoss commented 2 weeks ago

I honestly don't know without looking. Some things could likely be cleaned up but I think some of these that we skip would take a breaking change to actually fix the issue -- at least I vaguely recall this being the case from a couple of years back when I had looked at this.

egonelbre commented 2 weeks ago

There's an existing related issue https://github.com/googleapis/google-cloud-go/issues/9784

Most linting tooling works in a module boundary. So if you run go vet ./... or staticcheck ./... it only verifies the current module, and in this repo most folders are separate modules.

Or in other words neither go vet or staticcheck are running for the submodules.

I created a proof of concept script in https://github.com/egonelbre/google-cloud-go/pull/1/files#diff-25b8fc8d894e1d2f71768a6e38f68cecaad5d7d861ba6b16f1ad40f9f94516fb -- but it probably would need to be adjusted to fit Google's needs better. More of a starting point.

And I've been fixing linting issues for spanner https://github.com/googleapis/google-cloud-go/pulls/egonelbre

quartzmo commented 2 weeks ago

@egonelbre Thank you (again!) for your help with these issues. Appreciate your comments and POC!