From 6f658816aab503a3e06ea4325c5e732e2a572109 Mon Sep 17 00:00:00 2001 From: apostasie Date: Fri, 28 Mar 2025 09:54:25 -0700 Subject: [PATCH 01/12] golangci yml configuration cleanup - re-activate gocritic, disabling rules we do not pass - disable prealloc (considered harmful) - activate golines formatter - comment and re-orgnize linters section Signed-off-by: apostasie --- .golangci.yml | 113 ++++++++++++++++++++++++++------------------------ 1 file changed, 59 insertions(+), 54 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index bf186a332d2..616b1ce2a9c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -10,16 +10,36 @@ issues: linters: default: none enable: - - depguard + # 1. This is the default enabled set of golanci + + # We should consider enabling errcheck + # - errcheck - govet - ineffassign - - misspell - - nakedret - - prealloc - - revive - staticcheck - - unconvert - unused + + # 2. These are not part of the default set + + # Important to prevent import of certain packages + - depguard + # Removes unnecessary conversions + - unconvert + # Flag common typos + - misspell + # A meta-linter seen as a good replacement for golint + - revive + # Gocritic + - gocritic + + # 3. We used to use these, but have now removed them + + # Use of prealloc is generally premature optimization and performance profiling should be done instead + # https://golangci-lint.run/usage/linters/#prealloc + # - prealloc + # Provided by revive in a better way + # - nakedret + settings: staticcheck: checks: @@ -119,7 +139,6 @@ linters: - name: line-length-limit # Better dealt with by formatter golines disabled: true - depguard: rules: no-patent: @@ -141,67 +160,53 @@ linters: - pkg: github.com/containerd/nerdctl/v2/cmd desc: pkg must not depend on any cmd files gocritic: - enabled-checks: + disabled-checks: + # Below are normally enabled by default, but we do not pass - appendAssign - - argOrder - - badCond - - caseOrder - - codegenComment - - commentedOutCode - - deprecatedComment - - dupArg - - dupBranchBody - - dupCase - - dupSubExpr - - exitAfterDefer - - flagDeref - - flagName + - ifElseChain + - unslice + - badCall + - assignOp + - commentFormatting + - captLocal + - singleCaseSwitch + - wrapperFunc + - elseif + - regexpMust + enabled-checks: + # Below used to be enabled, but we do not pass anymore + # - paramTypeCombine + # - octalLiteral + # - unnamedResult + # - equalFold + # - sloppyReassign + # - emptyStringTest + # - hugeParam + # - appendCombine + # - stringXbytes + # - ptrToRefParam + # - commentedOutCode + # - rangeValCopy + # - methodExprCall + # - yodaStyleExpr + # - typeUnparen + + # We enabled these and we pass - nilValReturn - - offBy1 - - sloppyReassign - weakCond - - octalLiteral - - appendCombine - - equalFold - - hugeParam - indexAlloc - rangeExprCopy - - rangeValCopy - - assignOp - boolExprSimplify - - captLocal - - commentFormatting - commentedOutImport - - defaultCaseOrder - docStub - - elseif - emptyFallthrough - - emptyStringTest - hexLiteral - - ifElseChain - - methodExprCall - - regexpMust - - singleCaseSwitch - - sloppyLen - - stringXbytes - - switchTrue - typeAssertChain - - typeSwitchVar - - underef - unlabelStmt - - unlambda - - unslice - - valSwap - - wrapperFunc - - yodaStyleExpr - builtinShadow - importShadow - initClause - nestingReduce - - paramTypeCombine - - ptrToRefParam - - typeUnparen - - unnamedResult - unnecessaryBlock exclusions: generated: disable @@ -220,7 +225,7 @@ formatters: gofumpt: extra-rules: true golines: - max-len: 100 + max-len: 500 tab-len: 4 shorten-comments: true enable: @@ -228,6 +233,6 @@ formatters: - gofmt # We might consider enabling the following: # - gofumpt - # - golines + - golines exclusions: generated: disable From 1b3b183af8db3b3f4689ce00002895ad2e19a3a8 Mon Sep 17 00:00:00 2001 From: apostasie Date: Fri, 28 Mar 2025 09:56:45 -0700 Subject: [PATCH 02/12] Lint: revive redundant-import-alias Signed-off-by: apostasie --- .golangci.yml | 2 -- pkg/api/types/image_types.go | 2 +- pkg/imgutil/commit/commit.go | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 616b1ce2a9c..395afd32adf 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -97,8 +97,6 @@ linters: # FIXME: we should enable these below - name: struct-tag disabled: true - - name: redundant-import-alias - disabled: true - name: empty-lines disabled: true - name: unhandled-error diff --git a/pkg/api/types/image_types.go b/pkg/api/types/image_types.go index 0a723ca2cd2..d48e6318026 100644 --- a/pkg/api/types/image_types.go +++ b/pkg/api/types/image_types.go @@ -19,7 +19,7 @@ package types import ( "io" - v1 "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/opencontainers/image-spec/specs-go/v1" ) // ImageListOptions specifies options for `nerdctl image list`. diff --git a/pkg/imgutil/commit/commit.go b/pkg/imgutil/commit/commit.go index 782a5cd6edb..fd5886bfb7f 100644 --- a/pkg/imgutil/commit/commit.go +++ b/pkg/imgutil/commit/commit.go @@ -48,7 +48,7 @@ import ( "github.com/containerd/nerdctl/v2/pkg/clientutil" "github.com/containerd/nerdctl/v2/pkg/cmd/image" "github.com/containerd/nerdctl/v2/pkg/containerutil" - imgutil "github.com/containerd/nerdctl/v2/pkg/imgutil" + "github.com/containerd/nerdctl/v2/pkg/imgutil" "github.com/containerd/nerdctl/v2/pkg/labels" ) From 39d5ef2860a6ca078ce3172a5dd8166312778904 Mon Sep 17 00:00:00 2001 From: apostasie Date: Fri, 28 Mar 2025 09:58:49 -0700 Subject: [PATCH 03/12] Lint: revive struct-tag Signed-off-by: apostasie --- .golangci.yml | 2 -- pkg/cmd/network/list.go | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 395afd32adf..0581083297a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -95,8 +95,6 @@ linters: - name: use-any disabled: true # FIXME: we should enable these below - - name: struct-tag - disabled: true - name: empty-lines disabled: true - name: unhandled-error diff --git a/pkg/cmd/network/list.go b/pkg/cmd/network/list.go index b2075356d8c..731c51b1b99 100644 --- a/pkg/cmd/network/list.go +++ b/pkg/cmd/network/list.go @@ -35,7 +35,7 @@ type networkPrintable struct { Name string Labels string // TODO: "CreatedAt", "Driver", "IPv6", "Internal", "Scope" - file string `json:"-"` + file string } func List(ctx context.Context, options types.NetworkListOptions) error { From 4e3f637a1b016c55cccfc47cf41a2e48fea64089 Mon Sep 17 00:00:00 2001 From: apostasie Date: Fri, 28 Mar 2025 10:06:39 -0700 Subject: [PATCH 04/12] Lint: revive time-equal Signed-off-by: apostasie --- .golangci.yml | 2 -- pkg/netutil/netutil_test.go | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 0581083297a..a824b76a0d9 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -111,8 +111,6 @@ linters: disabled: true - name: argument-limit disabled: true - - name: time-equal - disabled: true - name: defer disabled: true - name: early-return diff --git a/pkg/netutil/netutil_test.go b/pkg/netutil/netutil_test.go index a17f4cb6197..399a33ee436 100644 --- a/pkg/netutil/netutil_test.go +++ b/pkg/netutil/netutil_test.go @@ -198,7 +198,7 @@ func testDefaultNetworkCreation(t *testing.T) { assert.NilError(t, err) assert.Assert(t, len(files) == 2) // files[0] is the entry for '.' assert.Assert(t, filepath.Join(cniConfTestDir, files[1].Name()) == defaultNetConf.File) - assert.Assert(t, firstConfigModTime == files[1].ModTime()) + assert.Assert(t, firstConfigModTime.Equal(files[1].ModTime())) } // Tests whether nerdctl properly creates the default network @@ -297,7 +297,7 @@ func testDefaultNetworkCreationWithBridgeIP(t *testing.T) { assert.NilError(t, err) assert.Assert(t, len(files) == 2) // files[0] is the entry for '.' assert.Assert(t, filepath.Join(cniConfTestDir, files[1].Name()) == defaultNetConf.File) - assert.Assert(t, firstConfigModTime == files[1].ModTime()) + assert.Assert(t, firstConfigModTime.Equal(files[1].ModTime())) } // Tests whether nerdctl skips the creation of the default network if a From ebc89dc071ec4adbc4d7a020ca8754c3787ca8e0 Mon Sep 17 00:00:00 2001 From: apostasie Date: Fri, 28 Mar 2025 10:21:14 -0700 Subject: [PATCH 05/12] Lint: revive unexported-naming Signed-off-by: apostasie --- .golangci.yml | 2 -- pkg/cmd/container/run_windows.go | 8 ++++---- pkg/inspecttypes/dockercompat/dockercompat.go | 8 ++++---- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index a824b76a0d9..86cee86fcf7 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -119,8 +119,6 @@ linters: disabled: true - name: function-result-limit disabled: true - - name: unexported-naming - disabled: true - name: unnecessary-stmt disabled: true - name: if-return diff --git a/pkg/cmd/container/run_windows.go b/pkg/cmd/container/run_windows.go index 6b38e7e9f3f..6bc5b42e699 100644 --- a/pkg/cmd/container/run_windows.go +++ b/pkg/cmd/container/run_windows.go @@ -69,17 +69,17 @@ func setPlatformOptions( if err != nil { return nil, fmt.Errorf("failed to parse memory bytes %q: %w", options.Memory, err) } - UVMMemmory := map[string]string{ + uvmMemmory := map[string]string{ uvmMemorySizeInMB: fmt.Sprintf("%v", mem64), } - opts = append(opts, oci.WithAnnotations(UVMMemmory)) + opts = append(opts, oci.WithAnnotations(uvmMemmory)) } if options.CPUs > 0.0 { - UVMCPU := map[string]string{ + uvmCPU := map[string]string{ uvmCPUCount: fmt.Sprintf("%v", options.CPUs), } - opts = append(opts, oci.WithAnnotations(UVMCPU)) + opts = append(opts, oci.WithAnnotations(uvmCPU)) } opts = append(opts, oci.WithWindowsHyperV) case "host": diff --git a/pkg/inspecttypes/dockercompat/dockercompat.go b/pkg/inspecttypes/dockercompat/dockercompat.go index 4605de3d1e7..e5b797e786d 100644 --- a/pkg/inspecttypes/dockercompat/dockercompat.go +++ b/pkg/inspecttypes/dockercompat/dockercompat.go @@ -820,10 +820,10 @@ func getMemorySettingsFromNative(sp *specs.Spec) (*MemorySetting, error) { return res, nil } -func getDNSFromNative(Labels map[string]string) (*DNSSettings, error) { +func getDNSFromNative(lbls map[string]string) (*DNSSettings, error) { res := &DNSSettings{} - if dnsSettingJSON, ok := Labels[labels.DNSSetting]; ok { + if dnsSettingJSON, ok := lbls[labels.DNSSetting]; ok { if err := json.Unmarshal([]byte(dnsSettingJSON), &res); err != nil { return nil, fmt.Errorf("failed to parse DNS settings: %v", err) } @@ -832,10 +832,10 @@ func getDNSFromNative(Labels map[string]string) (*DNSSettings, error) { return res, nil } -func getHostConfigLabelFromNative(Labels map[string]string) (*HostConfigLabel, error) { +func getHostConfigLabelFromNative(lbls map[string]string) (*HostConfigLabel, error) { res := &HostConfigLabel{} - if hostConfigLabelJSON, ok := Labels[labels.HostConfigLabel]; ok { + if hostConfigLabelJSON, ok := lbls[labels.HostConfigLabel]; ok { if err := json.Unmarshal([]byte(hostConfigLabelJSON), &res); err != nil { return nil, fmt.Errorf("failed to parse DNS servers: %v", err) } From bc989da4548e151012f1c4d5399cf475c077d225 Mon Sep 17 00:00:00 2001 From: apostasie Date: Fri, 28 Mar 2025 10:29:25 -0700 Subject: [PATCH 06/12] Lint: revive if-return Signed-off-by: apostasie --- .golangci.yml | 2 -- pkg/containerutil/container_network_manager.go | 11 ++--------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 86cee86fcf7..bada400d3f3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -121,8 +121,6 @@ linters: disabled: true - name: unnecessary-stmt disabled: true - - name: if-return - disabled: true - name: unchecked-type-assertion disabled: true - name: bare-return diff --git a/pkg/containerutil/container_network_manager.go b/pkg/containerutil/container_network_manager.go index c16ee9dfb9a..9f082f9ce12 100644 --- a/pkg/containerutil/container_network_manager.go +++ b/pkg/containerutil/container_network_manager.go @@ -240,11 +240,7 @@ func (m *noneNetworkManager) SetupNetworking(ctx context.Context, containerID st } // Save the meta information - if err = hs.Acquire(hsMeta); err != nil { - return err - } - - return nil + return hs.Acquire(hsMeta) } // CleanupNetworking Performs any required cleanup actions for the given container. @@ -269,10 +265,7 @@ func (m *noneNetworkManager) CleanupNetworking(ctx context.Context, container co } // Release - if err = hs.Release(container.ID()); err != nil { - return err - } - return nil + return hs.Release(container.ID()) } // InternalNetworkingOptionLabels Returns the set of NetworkingOptions which should be set as labels on the container. From 702a97740ddb866c414abdff393cfa39e1bcc577 Mon Sep 17 00:00:00 2001 From: apostasie Date: Fri, 28 Mar 2025 10:38:26 -0700 Subject: [PATCH 07/12] Lint: revive filename-format Signed-off-by: apostasie --- .golangci.yml | 2 -- .../infoutilmock/{info.util.mock.go => infoutil_mock.go} | 0 2 files changed, 2 deletions(-) rename pkg/infoutil/infoutilmock/{info.util.mock.go => infoutil_mock.go} (100%) diff --git a/.golangci.yml b/.golangci.yml index bada400d3f3..781920f4ac7 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -90,8 +90,6 @@ linters: disabled: true - name: import-alias-naming disabled: true - - name: filename-format - disabled: true - name: use-any disabled: true # FIXME: we should enable these below diff --git a/pkg/infoutil/infoutilmock/info.util.mock.go b/pkg/infoutil/infoutilmock/infoutil_mock.go similarity index 100% rename from pkg/infoutil/infoutilmock/info.util.mock.go rename to pkg/infoutil/infoutilmock/infoutil_mock.go From 77b0edc861019996fa6f23bfafc9cc64732dfd95 Mon Sep 17 00:00:00 2001 From: apostasie Date: Fri, 28 Mar 2025 10:42:08 -0700 Subject: [PATCH 08/12] Lint: revive import-alias-naming Signed-off-by: apostasie --- .golangci.yml | 2 -- cmd/nerdctl/container/container_update.go | 4 ++-- cmd/nerdctl/inspect/inspect.go | 8 ++++---- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 781920f4ac7..e7d02f19cc2 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -88,8 +88,6 @@ linters: disabled: true - name: nested-structs disabled: true - - name: import-alias-naming - disabled: true - name: use-any disabled: true # FIXME: we should enable these below diff --git a/cmd/nerdctl/container/container_update.go b/cmd/nerdctl/container/container_update.go index fb728655d29..28ac0f6d078 100644 --- a/cmd/nerdctl/container/container_update.go +++ b/cmd/nerdctl/container/container_update.go @@ -38,7 +38,7 @@ import ( "github.com/containerd/nerdctl/v2/cmd/nerdctl/helpers" "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/clientutil" - nerdctlContainer "github.com/containerd/nerdctl/v2/pkg/cmd/container" + nerdctlcontainer "github.com/containerd/nerdctl/v2/pkg/cmd/container" "github.com/containerd/nerdctl/v2/pkg/formatter" "github.com/containerd/nerdctl/v2/pkg/idutil/containerwalker" "github.com/containerd/nerdctl/v2/pkg/infoutil" @@ -358,7 +358,7 @@ func updateContainer(ctx context.Context, client *containerd.Client, id string, return err } if cmd.Flags().Changed("restart") && restart != "" { - if err := nerdctlContainer.UpdateContainerRestartPolicyLabel(ctx, client, container, restart); err != nil { + if err := nerdctlcontainer.UpdateContainerRestartPolicyLabel(ctx, client, container, restart); err != nil { return err } } diff --git a/cmd/nerdctl/inspect/inspect.go b/cmd/nerdctl/inspect/inspect.go index 53d02858d5a..8c62f54e4d1 100644 --- a/cmd/nerdctl/inspect/inspect.go +++ b/cmd/nerdctl/inspect/inspect.go @@ -23,9 +23,9 @@ import ( "github.com/spf13/cobra" "github.com/containerd/nerdctl/v2/cmd/nerdctl/completion" - containerCmd "github.com/containerd/nerdctl/v2/cmd/nerdctl/container" + containercmd "github.com/containerd/nerdctl/v2/cmd/nerdctl/container" "github.com/containerd/nerdctl/v2/cmd/nerdctl/helpers" - imageCmd "github.com/containerd/nerdctl/v2/cmd/nerdctl/image" + imagecmd "github.com/containerd/nerdctl/v2/cmd/nerdctl/image" "github.com/containerd/nerdctl/v2/pkg/api/types" "github.com/containerd/nerdctl/v2/pkg/clientutil" "github.com/containerd/nerdctl/v2/pkg/cmd/container" @@ -117,13 +117,13 @@ func inspectAction(cmd *cobra.Command, args []string) error { var containerInspectOptions types.ContainerInspectOptions if inspectImage { platform := "" - imageInspectOptions, err = imageCmd.InspectOptions(cmd, &platform) + imageInspectOptions, err = imagecmd.InspectOptions(cmd, &platform) if err != nil { return err } } if inspectContainer { - containerInspectOptions, err = containerCmd.InspectOptions(cmd) + containerInspectOptions, err = containercmd.InspectOptions(cmd) if err != nil { return err } From 8893fe4121164aa1c412006cf17b703e2af9b6ad Mon Sep 17 00:00:00 2001 From: apostasie Date: Fri, 28 Mar 2025 11:26:49 -0700 Subject: [PATCH 09/12] Lint: revive: review and organization off all rules Signed-off-by: apostasie --- .golangci.yml | 109 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 72 insertions(+), 37 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index e7d02f19cc2..5cfdd2df7ac 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -63,68 +63,103 @@ linters: enable-all-rules: true rules: # See https://revive.run/r - # Below are unsorted, and we might want to review them to decide which one we want to enable - - name: exported + + ##### P0: we should do it ASAP. + - name: max-control-nesting + # 10 occurences (at default 5). Deep nesting hurts readibility. + arguments: [7] + - name: deep-exit + # 11 occurrences. Do not exit in random places. disabled: true - - name: add-constant + - name: unchecked-type-assertion + # 14 occurrences. This is generally risky and encourages bad coding for newcomers. disabled: true - - name: cognitive-complexity + - name: bare-return + # 31 occurrences. Bare returns are just evil, very unfriendly, and make reading and editing much harder. disabled: true - - name: package-comments + - name: import-shadowing + # 44 occurrences. Shadowing makes things prone to errors / confusing to read. disabled: true - - name: cyclomatic + - name: use-errors-new + # 84 occurrences. Improves error testing. disabled: true - - name: deep-exit + + ##### P1: consider making a dent on these, but not critical. + - name: argument-limit + # 4 occurrences (at default 8). Long windy arguments list for functions are hard to read. Use structs instead. + arguments: [12] + - name: unnecessary-stmt + # 5 occurrences. Increase readability. disabled: true - - name: function-length + - name: defer + # 7 occurrences. Confusing to read for newbies. disabled: true - - name: flag-parameter + - name: confusing-naming + # 10 occurrences. Hurts readability. disabled: true - - name: max-public-structs + - name: early-return + # 10 occurrences. Would improve readability. disabled: true - - name: max-control-nesting + - name: function-result-limit + # 12 occurrences (at default 3). A function returning many results is probably too big. + arguments: [7] + - name: function-length + # 155 occurrences (at default 0, 75). Really long functions should really be broken up in most cases. + arguments: [0, 400] + - name: cyclomatic + # 204 occurrences (at default 10) + arguments: [100] + - name: unhandled-error + # 222 occurrences. Could indicate failure to handle broken conditions. disabled: true + - name: cognitive-complexity + arguments: [197] + # 441 occurrences (at default 7). We should try to lower it (involves significant refactoring). + + ##### P2: nice to have. + - name: max-public-structs + # 7 occurrences (at default 5). Might indicate overcrowding of public API. + arguments: [21] - name: confusing-results + # 13 occurrences. Have named returns when the type stutters. + # Makes it a bit easier to figure out function behavior just looking at signature. disabled: true - - name: nested-structs + - name: comment-spacings + # 50 occurrences. Makes code look less wonky / ease readability. disabled: true - name: use-any + # 30 occurrences. `any` instead of `interface{}`. Cosmetic. disabled: true - # FIXME: we should enable these below - name: empty-lines + # 85 occurrences. Makes code look less wonky / ease readability. disabled: true - - name: unhandled-error - disabled: true - - name: confusing-naming - disabled: true - - name: unused-parameter - disabled: true - - name: unused-receiver - disabled: true - - name: import-shadowing - disabled: true - - name: use-errors-new - disabled: true - - name: argument-limit - disabled: true - - name: defer + - name: package-comments + # 100 occurrences. Better for documentation... disabled: true - - name: early-return + - name: exported + # 577 occurrences. Forces documentation of any exported symbol. disabled: true - - name: comment-spacings + + ###### Permanently disabled. Below have been reviewed and vetted to be unnecessary. + - name: line-length-limit + # Formatter `golines` takes care of this. disabled: true - - name: function-result-limit + - name: nested-structs + # 5 occurrences. Trivial. This is not that hard to read. disabled: true - - name: unnecessary-stmt + - name: flag-parameter + # 52 occurrences. Not sure if this is valuable. disabled: true - - name: unchecked-type-assertion + - name: unused-parameter + # 505 occurrences. A lot of work for a marginal improvement. disabled: true - - name: bare-return + - name: unused-receiver + # 31 occurrences. Ibid. disabled: true - # Below have been reviewed and disabled - - name: line-length-limit - # Better dealt with by formatter golines + - name: add-constant + # 2605 occurrences. Kind of useful in itself, but unacceptable amount of effort to fix disabled: true + depguard: rules: no-patent: From a46718f702fab7dc890064c517b33914632b2571 Mon Sep 17 00:00:00 2001 From: apostasie Date: Fri, 28 Mar 2025 11:28:40 -0700 Subject: [PATCH 10/12] Lint: staticcheck fix QF1004 Note that QF1009 has been fixed by previous commits working on revive. Signed-off-by: apostasie --- .golangci.yml | 2 -- pkg/dnsutil/hostsstore/hosts.go | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5cfdd2df7ac..cf30bd287ce 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -54,11 +54,9 @@ linters: # FIXME: this below this point is disabled for now, but we should investigate - "-QF1008" # Omit embedded fields from selector expression https://staticcheck.dev/docs/checks#QF1008 - "-QF1003" # Convert if/else-if chain to tagged switch https://staticcheck.dev/docs/checks#QF1003 - - "-QF1009" # Use time.Time.Equal instead of == operator https://staticcheck.dev/docs/checks#QF1009 - "-QF1001" # Apply De Morgan’s law https://staticcheck.dev/docs/checks#QF1001 - "-QF1012" # Use fmt.Fprintf(x, ...) instead of x.Write(fmt.Sprintf(...)) https://staticcheck.dev/docs/checks#QF1012 - "-ST1005" # Expand call to math.Pow https://staticcheck.dev/docs/checks#QF1005 - - "-QF1004" # Use strings.ReplaceAll instead of strings.Replace with n == -1 https://staticcheck.dev/docs/checks#QF1004 revive: enable-all-rules: true rules: diff --git a/pkg/dnsutil/hostsstore/hosts.go b/pkg/dnsutil/hostsstore/hosts.go index 1b25b02d765..1378ed39686 100644 --- a/pkg/dnsutil/hostsstore/hosts.go +++ b/pkg/dnsutil/hostsstore/hosts.go @@ -52,7 +52,7 @@ func parseHostsButSkipMarkedRegion(w io.Writer, r io.Reader) error { LINE: for scanner.Scan() { line := scanner.Text() - line = strings.Replace(strings.Trim(line, " \t"), "\t", " ", -1) + line = strings.ReplaceAll(strings.Trim(line, " \t"), "\t", " ") sawMarkerEnd := false if strings.HasPrefix(line, "#") { com := strings.TrimSpace(line[1:]) From 1298c20fb6d58e4b8481ce9ddd621ec23aa029fd Mon Sep 17 00:00:00 2001 From: apostasie Date: Fri, 28 Mar 2025 11:32:00 -0700 Subject: [PATCH 11/12] Lint: staticcheck fix ST1005 Signed-off-by: apostasie --- .golangci.yml | 1 - pkg/imgutil/filtering.go | 6 ++---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index cf30bd287ce..f1c64f6b5e9 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -56,7 +56,6 @@ linters: - "-QF1003" # Convert if/else-if chain to tagged switch https://staticcheck.dev/docs/checks#QF1003 - "-QF1001" # Apply De Morgan’s law https://staticcheck.dev/docs/checks#QF1001 - "-QF1012" # Use fmt.Fprintf(x, ...) instead of x.Write(fmt.Sprintf(...)) https://staticcheck.dev/docs/checks#QF1012 - - "-ST1005" # Expand call to math.Pow https://staticcheck.dev/docs/checks#QF1005 revive: enable-all-rules: true rules: diff --git a/pkg/imgutil/filtering.go b/pkg/imgutil/filtering.go index 26525d86cb2..30764f163c6 100644 --- a/pkg/imgutil/filtering.go +++ b/pkg/imgutil/filtering.go @@ -158,8 +158,7 @@ func FilterByCreatedAt(ctx context.Context, client *containerd.Client, before [] return []images.Image{}, err } if len(beforeImages) == 0 { - //nolint:stylecheck - return []images.Image{}, fmt.Errorf("No such image: %s", fetchImageNames(before)) + return []images.Image{}, fmt.Errorf("no such image: %s", fetchImageNames(before)) } maxTime = beforeImages[0].CreatedAt for _, image := range beforeImages { @@ -175,8 +174,7 @@ func FilterByCreatedAt(ctx context.Context, client *containerd.Client, before [] return []images.Image{}, err } if len(sinceImages) == 0 { - //nolint:stylecheck - return []images.Image{}, fmt.Errorf("No such image: %s", fetchImageNames(since)) + return []images.Image{}, fmt.Errorf("no such image: %s", fetchImageNames(since)) } minTime = sinceImages[0].CreatedAt for _, image := range sinceImages { From 46c6af6e11d31884f95c3b54827af5a7cf1494db Mon Sep 17 00:00:00 2001 From: apostasie Date: Fri, 28 Mar 2025 11:40:28 -0700 Subject: [PATCH 12/12] Lint: staticcheck review remaining checks Signed-off-by: apostasie --- .golangci.yml | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index f1c64f6b5e9..65b0fb1fa51 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -51,11 +51,23 @@ linters: - "-ST1020" - "-ST1021" - "-ST1022" - # FIXME: this below this point is disabled for now, but we should investigate - - "-QF1008" # Omit embedded fields from selector expression https://staticcheck.dev/docs/checks#QF1008 - - "-QF1003" # Convert if/else-if chain to tagged switch https://staticcheck.dev/docs/checks#QF1003 - - "-QF1001" # Apply De Morgan’s law https://staticcheck.dev/docs/checks#QF1001 - - "-QF1012" # Use fmt.Fprintf(x, ...) instead of x.Write(fmt.Sprintf(...)) https://staticcheck.dev/docs/checks#QF1012 + + ##### TODO: fix and enable these + # 4 occurrences. + # Use fmt.Fprintf(x, ...) instead of x.Write(fmt.Sprintf(...)) https://staticcheck.dev/docs/checks#QF1012 + - "-QF1012" + # 6 occurrences. + # Apply De Morgan’s law https://staticcheck.dev/docs/checks#QF1001 + - "-QF1001" + # 10 occurrences. + # Convert if/else-if chain to tagged switch https://staticcheck.dev/docs/checks#QF1003 + - "-QF1003" + + ##### These have been vetted to be disabled. + # 55 occurrences. Omit embedded fields from selector expression https://staticcheck.dev/docs/checks#QF1008 + # Usefulness is questionable. + - "-QF1008" + revive: enable-all-rules: true rules: