Merge pull request kubernetes-sigs#1506 from saschagrunert/gocritic

Enable and fix all gocritic linters

.golangci.yml

@@ -1,7 +1,7 @@
 ---
 run:
   concurrency: 6
-  deadline: 5m
+  timeout: 5m
 linters:
   disable-all: true
   enable:
@@ -121,75 +121,4 @@ linters-settings:
   staticcheck:
     checks: ["SA1019"]
   gocritic:
-    enabled-checks:
-      - appendCombine
-      - badLock
-      - badRegexp
-      - badSorting
-      - badSyncOnceFunc
-      - boolExprSimplify
-      - builtinShadow
-      - builtinShadowDecl
-      - commentedOutCode
-      - commentedOutImport
-      - deferInLoop
-      - deferUnlambda
-      - docStub
-      - dupImport
-      - dynamicFmtString
-      - emptyDecl
-      - emptyFallthrough
-      - emptyStringTest
-      - equalFold
-      - evalOrder
-      - exposedSyncMutex
-      - externalErrorReassign
-      - filepathJoin
-      - hexLiteral
-      - httpNoBody
-      - importShadow
-      - indexAlloc
-      - initClause
-      - methodExprCall
-      - nestingReduce
-      - nilValReturn
-      - octalLiteral
-      - preferDecodeRune
-      - preferFilepathJoin
-      - preferFprint
-      - preferStringWriter
-      - preferWriteByte
-      - ptrToRefParam
-      - rangeExprCopy
-      - rangeValCopy
-      - redundantSprint
-      - regexpPattern
-      - regexpSimplify
-      - returnAfterHttpError
-      - ruleguard
-      - sliceClear
-      - sloppyReassign
-      - sortSlice
-      - sprintfQuotedString
-      - sqlQuery
-      - stringConcatSimplify
-      - stringXbytes
-      - stringsCompare
-      - syncMapLoadAndDelete
-      - timeExprSimplify
-      - todoCommentWithoutDetail
-      - tooManyResultsChecker
-      - truncateCmp
-      - typeAssertChain
-      - typeDefFirst
-      - typeUnparen
-      - uncheckedInlineErr
-      - unlabelStmt
-      - unnecessaryBlock
-      - unnecessaryDefer
-      - weakCond
-      - whyNoLint
-      - yodaStyleExpr
-      # - hugeParam
-      # - paramTypeCombine
-      # - unnamedResult
+    enable-all: true

cmd/crictl/container.go

@@ -330,7 +330,7 @@ var updateContainerCommand = &cli.Command{
 			return err
 		}
 
-		options := updateOptions{
+		options := &updateOptions{
 			CPUCount:           c.Int64("cpu-count"),
 			CPUMaximum:         c.Int64("cpu-maximum"),
 			CPUPeriod:          c.Int64("cpu-period"),
@@ -622,7 +622,7 @@ var listContainersCommand = &cli.Command{
 			return err
 		}
 
-		opts := listOptions{
+		opts := &listOptions{
 			id:               c.String("id"),
 			podID:            c.String("pod"),
 			state:            c.String("state"),
@@ -882,7 +882,7 @@ type updateOptions struct {
 
 // UpdateContainerResources sends an UpdateContainerResourcesRequest to the server, and parses
 // the returned UpdateContainerResourcesResponse.
-func UpdateContainerResources(client internalapi.RuntimeService, id string, opts updateOptions) error {
+func UpdateContainerResources(client internalapi.RuntimeService, id string, opts *updateOptions) error {
 	if id == "" {
 		return errors.New("ID cannot be empty")
 	}
@@ -1007,7 +1007,7 @@ func marshalContainerStatus(cs *pb.ContainerStatus) (string, error) {
 // the returned ContainerStatusResponse.
 //
 //nolint:dupl // pods and containers are similar, but still different
-func containerStatus(client internalapi.RuntimeService, ids []string, output string, tmplStr string, quiet bool) error {
+func containerStatus(client internalapi.RuntimeService, ids []string, output, tmplStr string, quiet bool) error {
 	verbose := !(quiet)
 	if output == "" { // default to json output
 		output = "json"
@@ -1089,7 +1089,7 @@ func outputContainerStatusTable(r *pb.ContainerStatusResponse, verbose bool) {
 
 // ListContainers sends a ListContainerRequest to the server, and parses
 // the returned ListContainerResponse.
-func ListContainers(runtimeClient internalapi.RuntimeService, imageClient internalapi.ImageManagerService, opts listOptions) error {
+func ListContainers(runtimeClient internalapi.RuntimeService, imageClient internalapi.ImageManagerService, opts *listOptions) error {
 	filter := &pb.ContainerFilter{}
 	if opts.id != "" {
 		filter.Id = opts.id
@@ -1248,7 +1248,7 @@ func getPodNameFromLabels(label map[string]string) string {
 	return "unknown"
 }
 
-func getContainersList(containersList []*pb.Container, opts listOptions) []*pb.Container {
+func getContainersList(containersList []*pb.Container, opts *listOptions) []*pb.Container {
 	filtered := []*pb.Container{}
 	for _, c := range containersList {
 		// Filter by pod name/namespace regular expressions.

cmd/crictl/container_stats.go

@@ -106,7 +106,7 @@ var statsCommand = &cli.Command{
 			id = c.Args().First()
 		}
 
-		opts := statsOptions{
+		opts := &statsOptions{
 			all:    c.Bool("all"),
 			id:     id,
 			podID:  c.String("pod"),
@@ -135,14 +135,14 @@ func (c containerStatsByID) Less(i, j int) bool {
 }
 
 type containerStatsDisplayer struct {
-	opts    statsOptions
+	opts    *statsOptions
 	request *pb.ListContainerStatsRequest
 	*display
 }
 
 // ContainerStats sends a ListContainerStatsRequest to the server, and
 // parses the returned ListContainerStatsResponse.
-func ContainerStats(client internalapi.RuntimeService, opts statsOptions) error {
+func ContainerStats(client internalapi.RuntimeService, opts *statsOptions) error {
 	d := containerStatsDisplayer{
 		opts: opts,
 		request: &pb.ListContainerStatsRequest{

cmd/crictl/container_test.go

@@ -46,7 +46,7 @@ func fakeContainer(name string, createdAt int64) *pb.Container {
 }
 
 var _ = DescribeTable("getContainersList",
-	func(input []*pb.Container, options listOptions, indexes []int) {
+	func(input []*pb.Container, options *listOptions, indexes []int) {
 		actual := getContainersList(input, options)
 		var expected []*pb.Container
 		for _, i := range indexes {
@@ -60,7 +60,7 @@ var _ = DescribeTable("getContainersList",
 			fakeContainer("test1", time.Date(2023, 1, 1, 12, 0o0, 0o0, 0o0, time.UTC).UnixNano()),
 			fakeContainer("test2", time.Date(2023, 1, 3, 12, 0o0, 0o0, 0o0, time.UTC).UnixNano()),
 		},
-		listOptions{},
+		&listOptions{},
 		[]int{2, 0, 1},
 	),
 	Entry("regards a container with no creation date as the oldest container",
@@ -72,42 +72,42 @@ var _ = DescribeTable("getContainersList",
 			},
 			fakeContainer("v1", time.Date(2023, 1, 1, 12, 0o0, 0o0, 0o0, time.UTC).UnixNano()),
 		},
-		listOptions{},
+		&listOptions{},
 		[]int{1, 0},
 	),
 	Entry("returns containers filtered with the regexp",
 		fakeContainersWithCreatedAtDesc("Test0", "Dev1", "Test2", "Dev3"),
-		listOptions{nameRegexp: "Test.*"},
+		&listOptions{nameRegexp: "Test.*"},
 		[]int{0, 2},
 	),
 	Entry("returns no containers when there are no containers matched with the regexp",
 		fakeContainersWithCreatedAtDesc("Test0", "Dev1", "Test2", "Dev3"),
-		listOptions{nameRegexp: "Prod.*"},
+		&listOptions{nameRegexp: "Prod.*"},
 		[]int{},
 	),
 	Entry("returns the most recent container with the latest option",
 		fakeContainersWithCreatedAtDesc("v0", "v1", "v2"),
-		listOptions{latest: true},
+		&listOptions{latest: true},
 		[]int{0},
 	),
 	Entry("returns last n containers with the last option",
 		fakeContainersWithCreatedAtDesc("v0", "v1", "v2"),
-		listOptions{last: 2},
+		&listOptions{last: 2},
 		[]int{0, 1},
 	),
 	Entry("prioritizes last more than latest",
 		fakeContainersWithCreatedAtDesc("v0", "v1", "v2"),
-		listOptions{last: 2, latest: true},
+		&listOptions{last: 2, latest: true},
 		[]int{0, 1},
 	),
 	Entry("returns all containers when the last is larger than the input length in order by createdAt desc",
 		fakeContainersWithCreatedAtDesc("v0", "v1", "v2"),
-		listOptions{last: 5},
+		&listOptions{last: 5},
 		[]int{0, 1, 2},
 	),
 	Entry("returns nothing when last is set and there are no containers",
 		fakeContainersWithCreatedAtDesc(),
-		listOptions{last: 2},
+		&listOptions{last: 2},
 		[]int{},
 	),
 )

cmd/crictl/image.go

@@ -566,7 +566,7 @@ func ouputImageFsInfoTable(r *pb.ImageFsInfoResponse) {
 	tablePrintFileSystem("Image", r.ImageFilesystems)
 }
 
-func parseCreds(creds string) (string, string, error) {
+func parseCreds(creds string) (username, password string, err error) {
 	if creds == "" {
 		return "", "", errors.New("credentials can't be empty")
 	}
@@ -580,7 +580,7 @@ func parseCreds(creds string) (string, string, error) {
 	return up[0], up[1], nil
 }
 
-func getAuth(creds string, auth string, username string) (*pb.AuthConfig, error) {
+func getAuth(creds, auth, username string) (*pb.AuthConfig, error) {
 	if username != "" {
 		fmt.Print("Enter Password:")
 		bytePassword, err := term.ReadPassword(int(syscall.Stdin)) //nolint:unconvert // required for windows
@@ -638,7 +638,7 @@ func normalizeRepoTagPair(repoTags []string, imageName string) (repoTagPairs [][
 	return
 }
 
-func normalizeRepoDigest(repoDigests []string) (string, string) {
+func normalizeRepoDigest(repoDigests []string) (repo, digest string) {
 	if len(repoDigests) == 0 {
 		return "<none>", "<none>"
 	}

cmd/crictl/image_test.go

@@ -22,11 +22,11 @@ import (
 	pb "k8s.io/cri-api/pkg/apis/runtime/v1"
 )
 
-func fakeImage(id string, digest []string, tags []string) *pb.Image {
+func fakeImage(id string, digest, tags []string) *pb.Image {
 	return &pb.Image{Id: id, RepoDigests: digest, RepoTags: tags}
 }
 
-func assert(input []*pb.Image, options []string, images []string) {
+func assert(input []*pb.Image, options, images []string) {
 	actual, _ := filterImagesList(input, options)
 	expected := []string{}
 	for _, img := range actual {

cmd/crictl/sandbox.go

@@ -311,7 +311,7 @@ var listPodCommand = &cli.Command{
 			return err
 		}
 
-		opts := listOptions{
+		opts := &listOptions{
 			id:                 c.String("id"),
 			state:              c.String("state"),
 			verbose:            c.Bool("verbose"),
@@ -487,7 +487,7 @@ func outputPodSandboxStatusTable(r *pb.PodSandboxStatusResponse, verbose bool) {
 
 // ListPodSandboxes sends a ListPodSandboxRequest to the server, and parses
 // the returned ListPodSandboxResponse.
-func ListPodSandboxes(client internalapi.RuntimeService, opts listOptions) error {
+func ListPodSandboxes(client internalapi.RuntimeService, opts *listOptions) error {
 	filter := &pb.PodSandboxFilter{}
 	if opts.id != "" {
 		filter.Id = opts.id
@@ -630,7 +630,7 @@ func getSandboxesRuntimeHandler(sandbox *pb.PodSandbox) string {
 	return sandbox.RuntimeHandler
 }
 
-func getSandboxesList(sandboxesList []*pb.PodSandbox, opts listOptions) []*pb.PodSandbox {
+func getSandboxesList(sandboxesList []*pb.PodSandbox, opts *listOptions) []*pb.PodSandbox {
 	filtered := []*pb.PodSandbox{}
 	for _, p := range sandboxesList {
 		// Filter by pod name/namespace regular expressions.

cmd/crictl/templates.go

@@ -51,7 +51,7 @@ func jsonBuiltinTmplFunc(v interface{}) string {
 
 // tmplExecuteRawJSON executes the template with interface{} with decoded by
 // rawJSON string.
-func tmplExecuteRawJSON(tmplStr string, rawJSON string) (string, error) {
+func tmplExecuteRawJSON(tmplStr, rawJSON string) (string, error) {
 	dec := json.NewDecoder(
 		bytes.NewReader([]byte(rawJSON)),
 	)

cmd/crictl/util.go

@@ -289,7 +289,7 @@ type statusData struct {
 	info            map[string]string
 }
 
-func outputStatusData(statuses []statusData, format string, tmplStr string) (err error) {
+func outputStatusData(statuses []statusData, format, tmplStr string) (err error) {
 	if len(statuses) == 0 {
 		return nil
 	}
@@ -371,7 +371,7 @@ func outputStatusData(statuses []statusData, format string, tmplStr string) (err
 	return nil
 }
 
-func outputEvent(event protoiface.MessageV1, format string, tmplStr string) error {
+func outputEvent(event protoiface.MessageV1, format, tmplStr string) error {
 	switch format {
 	case "yaml":
 		err := outputProtobufObjAsYAML(event)
@@ -468,7 +468,7 @@ func matchesRegex(pattern, target string) bool {
 	return matched
 }
 
-func matchesImage(imageClient internalapi.ImageManagerService, image string, containerImage string) (bool, error) {
+func matchesImage(imageClient internalapi.ImageManagerService, image, containerImage string) (bool, error) {
 	if image == "" {
 		return true, nil
 	}

pkg/common/file.go

@@ -44,7 +44,7 @@ func ReadConfig(filepath string) (*Config, error) {
 	if err != nil {
 		return nil, err
 	}
-	yamlConfig := yaml.Node{}
+	yamlConfig := &yaml.Node{}
 	err = yaml.Unmarshal(data, &yamlConfig)
 	if err != nil {
 		return nil, err
@@ -78,9 +78,9 @@ func WriteConfig(c *Config, filepath string) error {
 }
 
 // Extracts config options from the yaml data which is loaded from file.
-func getConfigOptions(yamlData yaml.Node) (*Config, error) {
+func getConfigOptions(yamlData *yaml.Node) (*Config, error) {
 	config := Config{}
-	config.yamlData = &yamlData
+	config.yamlData = yamlData
 
 	if yamlData.Content == nil || len(yamlData.Content) == 0 ||
 		yamlData.Content[0].Content == nil {

pkg/framework/test_context.go

@@ -170,7 +170,7 @@ func RegisterFlags() {
 }
 
 // Loads any external file-based parameters into the TestContextType.
-func (tc TestContextType) LoadYamlConfigFiles() error {
+func (tc *TestContextType) LoadYamlConfigFiles() error {
 	// Attempt to load custom images file:
 	if testImagesFilePath != "" {
 		err := LoadYamlFile(testImagesFilePath, &TestContext.TestImageList)

pkg/framework/util.go

@@ -161,7 +161,7 @@ func nowStamp() string {
 	return time.Now().Format(time.StampMilli)
 }
 
-func logf(level string, format string, args ...interface{}) {
+func logf(level, format string, args ...interface{}) {
 	fmt.Fprintf(GinkgoWriter, nowStamp()+": "+level+": "+format+"\n", args...)
 }