From 0366002c15abf2c2a2df8607baa42985feda1b41 Mon Sep 17 00:00:00 2001 From: Paulina Kalicka <71526180+paulinek13@users.noreply.github.com> Date: Mon, 6 Jan 2025 22:44:12 +0100 Subject: [PATCH] fix(lint): update `golangci-lint` configuration and resolve deprecation warnings (#4082) - Replace deprecated `run.skip-dirs` with `issues.exclude-dirs` - Update `output.format` to `output.formats` - Replace deprecated linters: - `megacheck` (split into `gosimple`, `staticcheck`, and `unused`) - `exportloopref` (no longer relevant since Go 1.22, replaced by `copyloopvar`) - Remove redundant variable copies flagged by `copyloopvar` (context: https://go.dev/blog/loopvar-preview) - Adjust `defer` statement comments in `localsdk.go` to use `staticcheck` instead of the deprecated `megacheck` Signed-off-by: Paulina Kalicka <71526180+paulinek13@users.noreply.github.com> --- .golangci.yml | 31 ++++++++++++------- pkg/allocation/converters/converter.go | 1 - pkg/allocation/converters/converter_test.go | 4 --- .../allocation_cache_test.go | 2 -- pkg/sdkserver/localsdk.go | 2 +- pkg/sdkserver/localsdk_test.go | 6 ---- test/e2e/fleet_test.go | 9 ------ test/e2e/gameserverallocation_test.go | 4 --- 8 files changed, 20 insertions(+), 39 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index fc917eb4e0..3539265b04 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -18,14 +18,6 @@ run: # list of build tags, all linters use it. Default is empty list. build-tags: - # which dirs to skip: they won't be analyzed; - # can use regexp here: generated.*, regexp is applied on full path; - # default value is empty list, but next dirs are always skipped independently - # from this option's value: - # vendor$, third_party$, testdata$, examples$, Godeps$, builtin$ - skip-dirs: - - test/sdk/restapi - # which files to skip: they will be analyzed, but issues from them # won't be reported. Default value is empty list, but there is # no need to include all autogenerated files, we confidently recognize @@ -34,8 +26,15 @@ run: modules-download-mode: vendor # output configuration options output: - # colored-line-number|line-number|json|tab|checkstyle, default is "colored-line-number" - format: colored-line-number + # the formats used to render issues + # formats: colored-line-number, line-number, json, colored-tab, + # tab, html, checkstyle, code-climate, junit-xml, + # junit-xml-extended, github-actions, teamcity, sarif + # output path can be either `stdout`, `stderr` + # or path to the file to write to + formats: + - format: colored-line-number + path: stdout # print lines of code with issue, default is true print-issued-lines: true @@ -45,20 +44,21 @@ output: linters: enable: - bodyclose + - copyloopvar - dupl - - exportloopref - goconst - gocritic - gocyclo - goimports + - gosimple - govet - - megacheck - misspell - nakedret - revive - staticcheck - unconvert - unparam + - unused linters-settings: govet: disable-all: false @@ -88,3 +88,10 @@ issues: - path: (.+)_test\.go|^test/|^cmd/.*|^pkg/apis/.* # fieldalignment is in the `govet` linter, so exclude based on text instead of all of govet text: '^fieldalignment: .*' + + # which dirs to exclude: issues from them won't be reported + # default dirs are skipped independently of this option's value + # (see exclude-dirs-use-default) + # default: [] + exclude-dirs: + - test/sdk/restapi diff --git a/pkg/allocation/converters/converter.go b/pkg/allocation/converters/converter.go index b4b80cfc68..3210a097f2 100644 --- a/pkg/allocation/converters/converter.go +++ b/pkg/allocation/converters/converter.go @@ -286,7 +286,6 @@ func convertInternalLabelSelectorToLabelSelector(in *metav1.LabelSelector) *pb.L func convertInternalLabelSelectorsToLabelSelectors(in []allocationv1.GameServerSelector) []*pb.GameServerSelector { var result []*pb.GameServerSelector for _, l := range in { - l := l c := convertInternalGameServerSelectorToGameServer(&l) result = append(result, c) } diff --git a/pkg/allocation/converters/converter_test.go b/pkg/allocation/converters/converter_test.go index 4a26e011fb..e3144a18c8 100644 --- a/pkg/allocation/converters/converter_test.go +++ b/pkg/allocation/converters/converter_test.go @@ -626,7 +626,6 @@ func TestConvertAllocationRequestToGameServerAllocation(t *testing.T) { }, } for _, tc := range tests { - tc := tc t.Run(tc.name, func(t *testing.T) { t.Parallel() @@ -834,7 +833,6 @@ func TestConvertGSAToAllocationRequest(t *testing.T) { }, } for _, tc := range tests { - tc := tc t.Run(tc.name, func(t *testing.T) { t.Parallel() @@ -1370,7 +1368,6 @@ func TestConvertGSAToAllocationResponse(t *testing.T) { }, } for _, tc := range tests { - tc := tc t.Run(tc.name, func(t *testing.T) { t.Parallel() runtime.FeatureTestMutex.Lock() @@ -1561,7 +1558,6 @@ func TestConvertAllocationResponseToGSA(t *testing.T) { }, } for _, tc := range tests { - tc := tc t.Run(tc.name, func(t *testing.T) { t.Parallel() runtime.FeatureTestMutex.Lock() diff --git a/pkg/gameserverallocations/allocation_cache_test.go b/pkg/gameserverallocations/allocation_cache_test.go index bde3bfc222..5904f12e2b 100644 --- a/pkg/gameserverallocations/allocation_cache_test.go +++ b/pkg/gameserverallocations/allocation_cache_test.go @@ -174,8 +174,6 @@ func TestAllocationCacheListSortedGameServers(t *testing.T) { } for k, v := range fixtures { - k := k - v := v t.Run(k, func(t *testing.T) { // deliberately not resetting the Feature state, to catch any possible unknown regressions with the // new feature flags diff --git a/pkg/sdkserver/localsdk.go b/pkg/sdkserver/localsdk.go index 2f5a2ce4ce..a4b8b3489f 100644 --- a/pkg/sdkserver/localsdk.go +++ b/pkg/sdkserver/localsdk.go @@ -882,7 +882,7 @@ func (l *LocalSDKServer) setGameServerFromFilePath(filePath string) error { l.logger.WithField("filePath", filePath).Info("Reading GameServer configuration") reader, err := os.Open(filePath) // nolint: gosec - defer reader.Close() // nolint: megacheck,errcheck + defer reader.Close() // nolint: staticcheck,errcheck if err != nil { return err diff --git a/pkg/sdkserver/localsdk_test.go b/pkg/sdkserver/localsdk_test.go index d103713902..593d2a1536 100644 --- a/pkg/sdkserver/localsdk_test.go +++ b/pkg/sdkserver/localsdk_test.go @@ -160,9 +160,6 @@ func TestLocalSDKServerSetLabel(t *testing.T) { } for k, v := range fixtures { - // pin variables here, see scopelint for details - k := k - v := v t.Run(k, func(t *testing.T) { ctx := context.Background() e := &sdk.Empty{} @@ -470,9 +467,6 @@ func TestLocalSDKServerPlayerConnectAndDisconnect(t *testing.T) { } for k, v := range fixtures { - // pin variables here, see https://github.com/kyoh86/scopelint for the details - k := k - v := v t.Run(k, func(t *testing.T) { var l *LocalSDKServer var err error diff --git a/test/e2e/fleet_test.go b/test/e2e/fleet_test.go index 365ead45a3..6306a608fe 100644 --- a/test/e2e/fleet_test.go +++ b/test/e2e/fleet_test.go @@ -303,7 +303,6 @@ func TestFleetScaleUpEditAndScaleDown(t *testing.T) { fixtures := []bool{true, false} for _, usePatch := range fixtures { - usePatch := usePatch t.Run("Use fleet Patch "+fmt.Sprint(usePatch), func(t *testing.T) { t.Parallel() ctx := context.Background() @@ -633,7 +632,6 @@ func TestScaleFleetUpAndDownWithGameServerAllocation(t *testing.T) { fixtures := []bool{false, true} for _, usePatch := range fixtures { - usePatch := usePatch t.Run("Use fleet Patch "+fmt.Sprint(usePatch), func(t *testing.T) { t.Parallel() @@ -714,8 +712,6 @@ func TestFleetUpdates(t *testing.T) { } for k, v := range fixtures { - k := k - v := v t.Run(k, func(t *testing.T) { t.Parallel() client := framework.AgonesClient.AgonesV1() @@ -1380,7 +1376,6 @@ func TestFleetRecreateGameServers(t *testing.T) { podClient := framework.KubeClient.CoreV1().Pods(framework.Namespace) for _, gs := range list.Items { - gs := gs pod, err := podClient.Get(ctx, gs.ObjectMeta.Name, metav1.GetOptions{}) assert.NoError(t, err) @@ -1392,7 +1387,6 @@ func TestFleetRecreateGameServers(t *testing.T) { }}, "gameserver shutdown": {f: func(t *testing.T, list *agonesv1.GameServerList) { for _, gs := range list.Items { - gs := gs var reply string reply, err := framework.SendGameServerUDP(t, &gs, "EXIT") if err != nil { @@ -1410,7 +1404,6 @@ func TestFleetRecreateGameServers(t *testing.T) { }}, "gameserver unhealthy": {f: func(t *testing.T, list *agonesv1.GameServerList) { for _, gs := range list.Items { - gs := gs var reply string reply, err := framework.SendGameServerUDP(t, &gs, "UNHEALTHY") if err != nil { @@ -1423,8 +1416,6 @@ func TestFleetRecreateGameServers(t *testing.T) { } for k, v := range tests { - k := k - v := v t.Run(k, func(t *testing.T) { t.Parallel() client := framework.AgonesClient.AgonesV1() diff --git a/test/e2e/gameserverallocation_test.go b/test/e2e/gameserverallocation_test.go index 9a09a3ce59..02d19e9c73 100644 --- a/test/e2e/gameserverallocation_test.go +++ b/test/e2e/gameserverallocation_test.go @@ -44,7 +44,6 @@ func TestCreateFleetAndGameServerAllocate(t *testing.T) { fixtures := []apis.SchedulingStrategy{apis.Packed, apis.Distributed} for _, strategy := range fixtures { - strategy := strategy t.Run(string(strategy), func(t *testing.T) { t.Parallel() ctx := context.Background() @@ -999,7 +998,6 @@ func TestMultiClusterAllocationOnLocalCluster(t *testing.T) { fixtures := []apis.SchedulingStrategy{apis.Packed, apis.Distributed} for _, strategy := range fixtures { - strategy := strategy t.Run(string(strategy), func(t *testing.T) { if strategy == apis.Distributed { framework.SkipOnCloudProduct(t, "gke-autopilot", "Autopilot does not support Distributed scheduling") @@ -1138,8 +1136,6 @@ func TestCreateFullFleetAndCantGameServerAllocate(t *testing.T) { fixtures := []apis.SchedulingStrategy{apis.Packed, apis.Distributed} for _, strategy := range fixtures { - strategy := strategy - t.Run(string(strategy), func(t *testing.T) { if strategy == apis.Distributed { framework.SkipOnCloudProduct(t, "gke-autopilot", "Autopilot does not support Distributed scheduling")