Skip to content

Commit 1af48ef

Browse files
ci(lint): resolve golangci-lint v2 findings on existing main
Lint v2 has stricter defaults than the v1.51 the repo used previously, surfacing pre-existing issues that v1 silently ignored. None of these are bugs introduced by the security bump; they're cleanups so the upgraded linter runs clean. - .golangci.yml: depguard v2 no longer auto-allows the project's own third-party deps — list them explicitly so legitimate imports of go-retryablehttp, oauth2, thrift, jwt/v5, etc. don't trip the rule. Drop the v2-invalid `gosec.exclude-generated` key (the linter now config-verifies and would refuse to start with it present). - Makefile: bump local install target from golangci-lint@v1.51.0 to v2.12.2 (matches the version the v9.2.1 GitHub Action installs). - parameters.go: reflect.Ptr -> reflect.Pointer (Ptr is the legacy alias kept for backwards compat; govet's `inline` check flags it). - internal/rows/rows.go: complete the //nolint:gosec annotation block the original PR author added on the very next line — the int32 conversion on line 183 is in the same telemetry counter scope. - telemetry/aggregator_test.go: //nolint:gosec on test-only counter. - connection_test.go: //nolint:gosec on os.ReadFile(localFile) — the filename comes from a deterministic httptest fixture path. - internal/client/client.go: //nolint:staticcheck on the deprecated thrift.StdLogger sentinel — it's a no-op since thrift v0.13 but remains the documented argument type for NewTDebugProtocolFactoryWithLogger. Local lint run after these changes: 0 issues. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
1 parent cf77582 commit 1af48ef

7 files changed

Lines changed: 22 additions & 8 deletions

File tree

.golangci.yml

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,20 @@ linters:
2424
allow:
2525
- $gostd
2626
- github.com/databricks/databricks-sql-go
27+
- github.com/apache/arrow/go/v12
28+
- github.com/apache/thrift/lib/go/thrift
29+
- github.com/coreos/go-oidc/v3/oidc
30+
- github.com/golang-jwt/jwt/v5
31+
- github.com/hashicorp/go-retryablehttp
32+
- github.com/joho/godotenv
33+
- github.com/mattn/go-isatty
34+
- github.com/pierrec/lz4/v4
35+
- github.com/pkg/browser
36+
- github.com/pkg/errors
37+
- github.com/rs/zerolog
38+
- github.com/stretchr/testify
39+
- golang.org/x/oauth2
2740
gosec:
28-
exclude-generated: true
2941
severity: low
3042
confidence: low
3143
nolintlint:

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ help: ## Show this help.
3535
all: gen fmt lint test coverage ## format and test everything
3636

3737
bin/golangci-lint: go.mod go.sum
38-
GOBIN=$(pwd)/bin go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.0
38+
GOBIN=$(pwd)/bin go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2
3939

4040
bin/gotestsum: go.mod go.sum
4141
@mkdir -p bin/

connection_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2116,7 +2116,7 @@ func TestConn_handleStagingRetry(t *testing.T) {
21162116
assert.Nil(t, err)
21172117
assert.Equal(t, int32(2), atomic.LoadInt32(&attempts))
21182118

2119-
got, readErr := os.ReadFile(localFile)
2119+
got, readErr := os.ReadFile(localFile) //nolint:gosec
21202120
assert.Nil(t, readErr)
21212121
assert.Equal(t, body, got, "GET should write the final-attempt body to local file")
21222122
})

internal/client/client.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,9 @@ func InitThriftClient(cfg *config.Config, httpclient *http.Client) (*ThriftServi
300300
return nil, dbsqlerrint.NewRequestError(context.TODO(), fmt.Sprintf("invalid protocol specified %s", cfg.ThriftProtocol), nil)
301301
}
302302
if cfg.ThriftDebugClientProtocol {
303-
protocolFactory = thrift.NewTDebugProtocolFactoryWithLogger(protocolFactory, "client:", thrift.StdLogger(nil))
303+
// thrift.StdLogger is deprecated as of v0.13 (no-op since), but is still the documented
304+
// argument type for NewTDebugProtocolFactoryWithLogger; passing nil is the upstream pattern.
305+
protocolFactory = thrift.NewTDebugProtocolFactoryWithLogger(protocolFactory, "client:", thrift.StdLogger(nil)) //nolint:staticcheck
304306
}
305307

306308
var tTrans thrift.TTransport

internal/rows/rows.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ func NewRows(
180180
// For CloudFetch direct results, use the number of result links.
181181
var totalPresent int32
182182
if directResults.CloseOperation != nil {
183-
totalPresent = int32(r.chunkCount)
183+
totalPresent = int32(r.chunkCount) //nolint:gosec
184184
} else if directResults.ResultSet != nil && directResults.ResultSet.Results != nil &&
185185
directResults.ResultSet.Results.ResultLinks != nil {
186186
totalPresent = int32(len(directResults.ResultSet.Results.ResultLinks)) //nolint:gosec

parameters.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func inferTypes(params []Parameter) {
102102
}
103103

104104
func inferType(param *Parameter) {
105-
if param.Value != nil && reflect.ValueOf(param.Value).Kind() == reflect.Ptr {
105+
if param.Value != nil && reflect.ValueOf(param.Value).Kind() == reflect.Pointer {
106106
param.Value = reflect.ValueOf(param.Value).Elem().Interface()
107107
inferType(param)
108108
return

telemetry/aggregator_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func TestAggregatorClose_WaitsForInFlightWorkerExports(t *testing.T) {
2727
body, _ := io.ReadAll(r.Body)
2828
var req TelemetryRequest
2929
if err := json.Unmarshal(body, &req); err == nil {
30-
atomic.AddInt32(&receivedCount, int32(len(req.ProtoLogs)))
30+
atomic.AddInt32(&receivedCount, int32(len(req.ProtoLogs))) //nolint:gosec
3131
}
3232
// Simulate slow server — forces the worker to be mid-HTTP-export when close() runs
3333
time.Sleep(exportDelay)
@@ -138,7 +138,7 @@ func TestAggregatorFlushUnlocked_InFlightAddBeforeSend(t *testing.T) {
138138
body, _ := io.ReadAll(r.Body)
139139
var req TelemetryRequest
140140
if err := json.Unmarshal(body, &req); err == nil {
141-
atomic.AddInt32(&receivedCount, int32(len(req.ProtoLogs)))
141+
atomic.AddInt32(&receivedCount, int32(len(req.ProtoLogs))) //nolint:gosec
142142
}
143143
time.Sleep(20 * time.Millisecond)
144144
w.WriteHeader(http.StatusOK)

0 commit comments

Comments
 (0)