Skip to content

Commit abe7238

Browse files
authored
Upgrade golangci-lint to v2 (#2169)
Summary: Upgrade golangci-lint to v2 This upgrades golangci-lint to v2 and remediates the golangci-lint crashes seen in other PRs (#2167). The vast majority of the new violations have been resolved with the exception of some more stylistic staticcheck cases. Rather than ignoring these cases, they should be easy to handle if/when the offending code is modified. The specific violations can be seen in the Test plan below. The pr-genfile-checker issues related to go generate will be fixed in #2167. I opted to split these changes into two PRs despite them both needing to be available for a successful build. Relevant Issues: N/A Type of change: /kind dependency Test Plan: Existing build and the following checks - [x] `CGO_ENABLED=0 golangci-lint run` succeeds for the majority of checks <details><summary>command output</summary> ``` root@px-dev-docker-dev-vm:/px/src/px.dev/pixie# CGO_ENABLED=0 golangci-lint run src/cloud/api/controllers/session_middleware.go:173:2: QF1007: could merge conditional assignment into variable declaration (staticcheck) forceBearer := false ^ src/cloud/artifact_tracker/controllers/server.go:176:5: QF1001: could apply De Morgan's law (staticcheck) if !(at == vpb.AT_DARWIN_AMD64 || at == vpb.AT_LINUX_AMD64 || at == vpb.AT_CONTAINER_SET_YAMLS || at == vpb.AT_CONTAINER_SET_TEMPLATE_YAMLS) { ^ src/cloud/autocomplete/suggester.go:176:5: QF1003: could use tagged switch on a.Type (staticcheck) if a.Type == vispb.PX_POD { ^ src/cloud/autocomplete/suggester.go:276:23: QF1001: could apply De Morgan's law (staticcheck) if res.NS != "" && !(md.EsMDType(res.Kind) == md.EsMDTypeNamespace || md.EsMDType(res.Kind) == md.EsMDTypeNode) { ^ src/cloud/plugin/controllers/server.go:563:5: S1009: should omit nil check; len() for nil maps is defined as zero (staticcheck) if req.Configurations != nil && len(req.Configurations) > 0 { ^ src/cloud/profile/controllers/server.go:170:2: QF1003: could use tagged switch on err (staticcheck) if err == datastore.ErrOrgNotFound { ^ src/cloud/project_manager/datastore/datastore_test.go:27:2: ST1019: package "github.com/golang-migrate/migrate/source/go_bindata" is being imported more than once (staticcheck) _ "github.com/golang-migrate/migrate/source/go_bindata" ^ src/cloud/project_manager/datastore/datastore_test.go:28:2: ST1019(related information): other import of "github.com/golang-migrate/migrate/source/go_bindata" (staticcheck) bindata "github.com/golang-migrate/migrate/source/go_bindata" ^ src/cloud/shared/idprovider/client.go:535:6: QF1001: could apply De Morgan's law (staticcheck) if !(k == "Set-Cookie" || k == "Location") { ^ src/cloud/shared/idprovider/client_test.go:96:44: ST1013: should use constant http.StatusFound instead of numeric literal 302 (staticcheck) http.Redirect(w, r, consentURL.String(), 302) ^ src/common/testing/test_utils/cert_generator/cert_generator.go:120:2: QF1003: could use tagged switch on keyType (staticcheck) if keyType == "pkcs1" { ^ src/e2e_test/perf_tool/pkg/metrics/data_loss_handler.go:87:6: QF1009: probably want to use time.Time.Equal instead (staticcheck) if (ts == time.Time{}) { ^ src/e2e_test/perf_tool/pkg/metrics/prometheus_recorder.go:129:7: QF1001: could apply De Morgan's law (staticcheck) if !(mf.GetType() == io_prometheus_client.MetricType_COUNTER || mf.GetType() == io_prometheus_client.MetricType_GAUGE) { ^ src/e2e_test/perf_tool/pkg/suites/experiments.go:27:2: ST1019: package "px.dev/pixie/src/e2e_test/perf_tool/experimentpb" is being imported more than once (staticcheck) "px.dev/pixie/src/e2e_test/perf_tool/experimentpb" ^ src/e2e_test/perf_tool/pkg/suites/experiments.go:28:2: ST1019(related information): other import of "px.dev/pixie/src/e2e_test/perf_tool/experimentpb" (staticcheck) pb "px.dev/pixie/src/e2e_test/perf_tool/experimentpb" ^ src/e2e_test/vizier/exectime/cmd/benchmark.go:136:29: ST1016: methods on the same type should have the same receiver name (seen 1x "t", 4x "d") (staticcheck) func (d *ErrorDistribution) Append(v interface{}) { ^ src/e2e_test/vizier/exectime/cmd/benchmark.go:175:29: ST1016: methods on the same type should have the same receiver name (seen 1x "t", 5x "d") (staticcheck) func (d *BytesDistribution) Append(v interface{}) { ^ src/operator/controllers/monitor.go:49:2: ST1019: package "px.dev/pixie/src/operator/apis/px.dev/v1alpha1" is being imported more than once (staticcheck) "px.dev/pixie/src/operator/apis/px.dev/v1alpha1" ^ src/operator/controllers/monitor.go:50:2: ST1019(related information): other import of "px.dev/pixie/src/operator/apis/px.dev/v1alpha1" (staticcheck) pixiev1alpha1 "px.dev/pixie/src/operator/apis/px.dev/v1alpha1" ^ src/operator/controllers/monitor.go:194:24: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) m.podStates.write(pod.ObjectMeta.Labels["name"], pod.ObjectMeta.Name, &podWrapper{pod: pod}) ^ src/operator/controllers/monitor.go:204:26: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) m.podStates.delete(pod.ObjectMeta.Labels["name"], pod.ObjectMeta.Name) ^ src/operator/controllers/monitor.go:207:24: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) m.podStates.write(pod.ObjectMeta.Labels["name"], pod.ObjectMeta.Name, &podWrapper{pod: pod}) ^ src/operator/controllers/monitor.go:215:25: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) m.podStates.delete(pod.ObjectMeta.Labels["name"], pod.ObjectMeta.Name) ^ src/operator/controllers/monitor.go:418:7: QF1001: could apply De Morgan's law (staticcheck) if !(ownerRef.Kind == "StatefulSet") { ^ src/operator/controllers/monitor.go:455:13: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) if p.pod.ObjectMeta.Labels["plane"] != "control" { ^ src/operator/controllers/monitor.go:675:2: QF1003: could use tagged switch on state.Reason (staticcheck) if state.Reason == status.NATSPodFailed { ^ src/pixie_cli/pkg/components/table_renderer.go:167:3: QF1012: Use fmt.Fprintf(...) instead of WriteString(fmt.Sprintf(...)) (staticcheck) buf.WriteString(fmt.Sprintf("%q:", fmt.Sprintf("%v", mi.Key))) ^ src/pixie_cli/pkg/components/table_renderer.go:331:14: QF1004: could use strings.ReplaceAll instead (staticcheck) dataStr = strings.Replace(dataStr, "\"", "\"\"", -1) ^ src/shared/artifacts/manifest/manifest.go:51:28: ST1016: methods on the same type should have the same receiver name (seen 2x "m", 5x "a") (staticcheck) func (a *ArtifactManifest) ArtifactSets() []*versionspb.ArtifactSet { ^ src/shared/k8s/proto_utils_test.go:1017:35: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) assert.Equal(t, "object_md", obj.ObjectMeta.Name) ^ src/shared/k8s/proto_utils_test.go:1319:33: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) assert.Equal(t, "object_md", e.ObjectMeta.Name) ^ src/shared/k8s/proto_utils_test.go:1478:35: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) assert.Equal(t, "object_md", obj.ObjectMeta.Name) ^ src/shared/services/election/election.go:58:56: ST1011: var expectedMaxSkewMS is of type time.Duration; don't use unit-specific suffix "MS" (staticcheck) func NewK8sLeaderElectionMgr(electionNamespace string, expectedMaxSkewMS, renewDeadlineMS time.Duration, electionName string) (*K8sLeaderElectionMgr, error) { ^ src/stirling/testing/demo_apps/go_http/go_http_client/main.go:73:3: QF1003: could use tagged switch on *reqType (staticcheck) if *reqType == "get" || *reqType == "mix" { ^ src/utils/artifacts/versions_gen/main.go:50:2: QF1002: could use tagged switch on artifactName (staticcheck) switch { ^ src/utils/dev_dns_updater/dev_dns_updater.go:128:18: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) svcName := svc.ObjectMeta.Name ^ src/utils/dev_dns_updater/dev_dns_updater.go:142:19: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) SvcName: svc.ObjectMeta.Name, ^ src/utils/dev_dns_updater/dev_dns_updater.go:153:20: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) SvcName: svc.ObjectMeta.Name, ^ src/utils/shared/k8s/delete.go:380:45: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) err = svcs.Delete(context.Background(), s.ObjectMeta.Name, metav1.DeleteOptions{}) ^ src/utils/shared/k8s/delete.go:398:45: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) err = pods.Delete(context.Background(), s.ObjectMeta.Name, metav1.DeleteOptions{}) ^ src/vizier/services/cloud_connector/bridge/vzinfo.go:419:30: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) return unhealthyPEMPods[i].ObjectMeta.Name < unhealthyPEMPods[j].ObjectMeta.Name ^ src/vizier/services/cloud_connector/bridge/vzinfo.go:594:14: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) if len(j.ObjectMeta.OwnerReferences) > 0 && j.ObjectMeta.OwnerReferences[0].Name == cronJob && j.Status.Succeeded == 1 { ^ src/vizier/services/cloud_connector/bridge/vzinfo.go:595:76: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) err = v.clientset.BatchV1().Jobs(v.ns).Delete(context.Background(), j.ObjectMeta.Name, metav1.DeleteOptions{ ^ src/vizier/services/cloud_connector/vzmetrics/scrape.go:115:8: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) if p.ObjectMeta.Annotations[scrapeAnnotationName] != "true" { ^ src/vizier/services/cloud_connector/vzmetrics/scrape.go:118:16: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) podName := p.ObjectMeta.Name ^ src/vizier/services/cloud_connector/vzmetrics/scrape.go:119:17: QF1008: could remove embedded field "ObjectMeta" from selector (staticcheck) port, ok := p.ObjectMeta.Annotations[portAnnotationName] ^ src/vizier/services/metadata/controllers/k8smeta/k8s_metadata_handler_test.go:504:3: QF1003: could use tagged switch on prevUpdateVersion (staticcheck) if prevUpdateVersion == 3 { ^ src/vizier/services/metadata/controllers/k8smeta/k8s_metadata_handler_test.go:523:3: QF1003: could use tagged switch on prevUpdateVersion (staticcheck) if prevUpdateVersion == 3 { ^ src/vizier/services/metadata/controllers/k8smeta/k8s_metadata_store.go:141:3: QF1006: could lift into loop condition (staticcheck) if tIdx == len(tKeys) && uIdx == len(uKeys) { ^ src/vizier/services/metadata/controllers/server.go:260:8: QF1001: could apply De Morgan's law (staticcheck) for !(finishedUpdates && finishedSchema) { ^ 50 issues: * staticcheck: 50 ``` </details> --------- Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
1 parent d08b5aa commit abe7238

File tree

162 files changed

+606
-592
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

162 files changed

+606
-592
lines changed

.arclint

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,20 +127,14 @@
127127
"script-and-regex.script": "./tools/linters/gazelle.sh",
128128
"script-and-regex.regex": "/^(?P<severity>[[:alpha:]]+)\n(?P<file>[^\n]+)\n(?P<message>[^\n]+)\n((?P<line>\\d),(?P<char>\\d)\n<<<<<\n(?P<original>.*)=====\n(?P<replacement>.*)>>>>>\n)$/s"
129129
},
130-
"goimports": {
131-
"type": "goimports",
132-
"include": [
133-
"(\\.go$)"
134-
]
135-
},
136130
"golangci-lint": {
137131
"type": "golangci-lint",
138132
"include": [
139133
"(\\.go$)"
140134
],
141135
"flags": [
142136
"--timeout=5m0s",
143-
"--out-format=checkstyle"
137+
"--output.checkstyle.path=stdout"
144138
]
145139
},
146140
"jshint-ui": {

.golangci.yaml

Lines changed: 58 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,11 @@
11
---
2+
version: "2"
23
run:
3-
# Tell golangci-lint to not acquire a lock because
4-
# arcanist will run multiple instances in parallel.
54
allow-parallel-runners: true
6-
# arcanist runs many of these in parallel causing
7-
# CPU contention and longer runtimes.
8-
timeout: 3m
9-
10-
output:
11-
sort-results: true
12-
13-
issues:
14-
max-issues-per-linter: 0
15-
max-same-issues: 0
16-
# TODO(ddelnano): Remove once typecheck is upgraded in next golangci-lint upgrade
17-
# This error originates from the stdlib due to generics usage
18-
exclude-rules:
19-
- path: .*slices\/sort.go
20-
linters:
21-
- typecheck
22-
text: "^(undefined: (min|max))"
23-
245
linters:
256
enable:
267
- asciicheck
278
- errcheck
28-
# Although goimports includes gofmt, it doesn't support the simplify option.
29-
# So we include gofmt here.
30-
- gofmt
31-
- gosimple
329
- govet
3310
- ineffassign
3411
- makezero
@@ -39,24 +16,62 @@ linters:
3916
- predeclared
4017
- revive
4118
- staticcheck
42-
# https://github.com/golangci/golangci-lint/issues/2649
43-
# - structcheck
44-
- typecheck
4519
- unused
46-
# https://github.com/golangci/golangci-lint/issues/2649
47-
# - wastedassign
4820
- whitespace
49-
disable:
50-
# The following linters are run separately by arcanist at the moment.
51-
# This is because we have autofix hooks for these linters.
52-
- goimports
53-
disable-all: false
54-
55-
linters-settings:
56-
errcheck:
57-
# yamllint disable-line rule:line-length
58-
ignore: io:Close,github.com/fatih/color,github.com/spf13/pflag:MarkHidden,github.com/spf13/viper:(BindEnv|BindPFlag),github.com/spf13/cobra:(Help|MarkFlagRequired|Usage),github.com/segmentio/analytics-go/v3:Enqueue,database/sql:Rollback,github.com/nats-io/nats.go:Unsubscribe
59-
goimports:
60-
local-prefixes: px.dev
61-
nakedret:
62-
max-func-lines: 0
21+
settings:
22+
errcheck:
23+
exclude-functions:
24+
- io.Close
25+
- (*github.com/spf13/pflag.FlagSet).MarkHidden
26+
- github.com/spf13/viper.BindEnv
27+
- github.com/spf13/viper.BindPFlag
28+
- github.com/spf13/viper.BindPFlags
29+
- (*github.com/spf13/cobra.Command).Help
30+
- (*github.com/spf13/cobra.Command).MarkFlagRequired
31+
- (*github.com/spf13/cobra.Command).Usage
32+
- (github.com/segmentio/analytics-go/v3.Client).Enqueue
33+
- (*database/sql.Tx).Rollback
34+
- (*github.com/nats-io/nats.go.Subscription).Unsubscribe
35+
revive:
36+
rules:
37+
- name: unused-parameter
38+
disabled: true
39+
staticcheck:
40+
checks:
41+
- all
42+
- "-ST1005" # ignore the "ST1005: error strings should not be capitalized" check
43+
- "-QF1008" # ignore omit embedded fields from selector expression
44+
exclusions:
45+
generated: lax
46+
presets:
47+
- comments
48+
- common-false-positives
49+
- legacy
50+
- std-error-handling
51+
paths:
52+
- third_party$
53+
- builtin$
54+
- examples$
55+
issues:
56+
max-issues-per-linter: 0
57+
max-same-issues: 0
58+
formatters:
59+
enable:
60+
- gci
61+
- gofumpt
62+
settings:
63+
gci:
64+
sections:
65+
- standard
66+
- default
67+
- prefix(px.dev)
68+
custom-order: true
69+
goimports:
70+
local-prefixes:
71+
- px.dev
72+
exclusions:
73+
generated: lax
74+
paths:
75+
- third_party$
76+
- builtin$
77+
- examples$

docker.properties

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
DOCKER_IMAGE_TAG=202502242148
2-
LINTER_IMAGE_DIGEST=eb7613e8aab9eb4620d6fae2618c966092abca8734abf90f0238db7de4535f15
3-
DEV_IMAGE_DIGEST=8316e718b16de940e2a5ac64fb6e57dc9ed28ca2ed437302e339964e9b42c9f2
4-
DEV_IMAGE_WITH_EXTRAS_DIGEST=b6d4c2404b30d06c2322d531620dcbe810a2c534f1cd0aa2389c75ed91d9bc24
1+
DOCKER_IMAGE_TAG=202504121153
2+
LINTER_IMAGE_DIGEST=ff369d95c4c84c95b668498219fda60ff8126828839171262f2eee58bd95ce19
3+
DEV_IMAGE_DIGEST=91e7fb85e0497340df5efaf035b65d98eab458908f852a782aeeb5ea0b69b5c9
4+
DEV_IMAGE_WITH_EXTRAS_DIGEST=f90e8b9b69d5870a7115ad434388da7bcef05f4a6c47e937a5a6348a22613ab4

src/api/go/pxapi/examples/basic_example/example.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,13 @@ import (
2828
"px.dev/pixie/src/api/go/pxapi/types"
2929
)
3030

31-
var (
32-
pxl = `
31+
var pxl = `
3332
import px
3433
df = px.DataFrame('http_events')
3534
df = df[['upid', 'req_path', 'remote_addr', 'req_method']]
3635
df = df.head(10)
3736
px.display(df, 'http')
3837
`
39-
)
4038

4139
type tablePrinter struct{}
4240

@@ -56,8 +54,7 @@ func (t *tablePrinter) HandleDone(ctx context.Context) error {
5654
return nil
5755
}
5856

59-
type tableMux struct {
60-
}
57+
type tableMux struct{}
6158

6259
func (s *tableMux) AcceptTable(ctx context.Context, metadata types.TableMetadata) (pxapi.TableRecordHandler, error) {
6360
return &tablePrinter{}, nil

src/api/go/pxapi/examples/encryption_example/example.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,13 @@ import (
2828
"px.dev/pixie/src/api/go/pxapi/types"
2929
)
3030

31-
var (
32-
pxl = `
31+
var pxl = `
3332
import px
3433
df = px.DataFrame('http_events')
3534
df = df[['upid', 'req_path', 'remote_addr', 'req_method']]
3635
df = df.head(10)
3736
px.display(df, 'http')
3837
`
39-
)
4038

4139
type tablePrinter struct{}
4240

@@ -56,8 +54,7 @@ func (t *tablePrinter) HandleDone(ctx context.Context) error {
5654
return nil
5755
}
5856

59-
type tableMux struct {
60-
}
57+
type tableMux struct{}
6158

6259
func (s *tableMux) AcceptTable(ctx context.Context, metadata types.TableMetadata) (pxapi.TableRecordHandler, error) {
6360
return &tablePrinter{}, nil

src/api/go/pxapi/examples/example_mux/example.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,14 @@ import (
3030
"px.dev/pixie/src/api/go/pxapi/types"
3131
)
3232

33-
var (
34-
pxl = `
33+
var pxl = `
3534
import px
3635
df = px.DataFrame('http_events')
3736
df = df[['upid', 'req_path', 'remote_addr', 'req_method']]
3837
df = df.head(10)
3938
px.display(df, 'http_as_json')
4039
px.display(df, 'http_as_table')
4140
`
42-
)
4341

4442
func main() {
4543
apiKey, ok := os.LookupEnv("PX_API_KEY")

src/api/go/pxapi/examples/standalone_pem_example/example.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ type tablePrinter struct{}
9090
func (t *tablePrinter) HandleInit(ctx context.Context, metadata types.TableMetadata) error {
9191
return nil
9292
}
93+
9394
func (t *tablePrinter) HandleRecord(ctx context.Context, r *types.Record) error {
9495
for _, d := range r.Data {
9596
fmt.Printf("%s ", d.String())
@@ -103,8 +104,7 @@ func (t *tablePrinter) HandleDone(ctx context.Context) error {
103104
}
104105

105106
// Satisfies the TableMuxer interface.
106-
type tableMux struct {
107-
}
107+
type tableMux struct{}
108108

109109
func (s *tableMux) AcceptTable(ctx context.Context, metadata types.TableMetadata) (pxapi.TableRecordHandler, error) {
110110
return &tablePrinter{}, nil

src/api/go/pxapi/examples/streaming_example/example.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,11 @@ import (
2828
"px.dev/pixie/src/api/go/pxapi/types"
2929
)
3030

31-
var (
32-
pxl = `
31+
var pxl = `
3332
import px
3433
df = px.DataFrame('http_events', start_time='-5m')[['resp_status','req_path']]
3534
px.display(df.stream(), 'http_table')
3635
`
37-
)
3836

3937
type tablePrinter struct{}
4038

@@ -54,8 +52,7 @@ func (t *tablePrinter) HandleDone(ctx context.Context) error {
5452
return nil
5553
}
5654

57-
type tableMux struct {
58-
}
55+
type tableMux struct{}
5956

6057
func (s *tableMux) AcceptTable(ctx context.Context, metadata types.TableMetadata) (pxapi.TableRecordHandler, error) {
6158
return &tablePrinter{}, nil

src/api/go/pxapi/results.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,6 @@ func (s *ScriptResults) run() error {
181181
ctx := s.c.Context()
182182
for {
183183
resp, err := s.c.Recv()
184-
185184
if err != nil {
186185
if err == io.EOF {
187186
// Stream has terminated.

src/api/go/pxapi/vizier.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"context"
2323

2424
"px.dev/pixie/src/api/go/pxapi/errdefs"
25-
2625
"px.dev/pixie/src/api/proto/vizierpb"
2726
)
2827

0 commit comments

Comments
 (0)