Fix minor staticcheck errors (#2913)

KsaweryZietara · web-flow · commit 336a3d9acd12 · 2026-02-24T15:25:51.000+01:00
* Fix minor staticcheck errors

* Increase timeout

* Increase timeout

* wip
diff --git a/.golangci.yml b/.golangci.yml
@@ -8,12 +8,6 @@ issues:
   max-issues-per-linter: 0
   max-same-issues: 0
 
-output:
-  formats:
-    text:
-      print-issued-lines: true
-      print-linter-name: true
-
 linters:
   default: none
   enable:
@@ -25,12 +19,27 @@ linters:
     - unconvert   # Detect redundant type conversions
     - unused      # Detect unused constants, variables, functions and types
     - cyclop      # Check code complexity
+    - staticcheck # Check set of rules from staticcheck
   settings:
     unused:
-      # Mark all struct fields that have been written to as used.
-      field-writes-are-uses: false
+      field-writes-are-uses: false # Mark all struct fields that have been written to as used.
     cyclop:
       max-complexity: 30
+    staticcheck:
+      checks:
+        - "all"
+        - "-QF1008" # Omit embedded fields from selector expression.
+        - "-ST1000" # Incorrect or missing package comment.
+        - "-ST1005" # Incorrectly formatted error string.
+        - "-S1009"  # Omit redundant nil check on slices, maps, and channels.
+        - "-ST1019" # Importing the same package multiple times.
+        - "-SA4009" # # A function argument is overwritten before its first use.
+        - "-S1008"  # Simplify returning boolean expression.
+        - "-SA1029" # Inappropriate key in call to 'context.WithValue'.
+        - "-SA1019" # Using a deprecated function, variable, constant or field.
+        - "-ST1003" # Poorly chosen identifier.
+        - "-ST1020" # The documentation of an exported function should start with the function's name.
+        - "-ST1012" # Poorly chosen name for error variable.
 formatters:
   enable:
     - goimports
diff --git a/Makefile b/Makefile
@@ -1,6 +1,6 @@
 GOLINT_VER = v2.8.0
 ifeq (,$(GOLINT_TIMEOUT))
-GOLINT_TIMEOUT=2m
+GOLINT_TIMEOUT=4m
 endif
 
 ifndef ARTIFACTS
diff --git a/internal/archive/service.go b/internal/archive/service.go
@@ -31,13 +31,13 @@ func NewService(db storage.BrokerStorage, dryRun bool, performDeletion bool, bat
 	}
 }
 
-func (s *Service) Run() (error, int, int) {
+func (s *Service) Run() (int, int, error) {
 	l := slog.New(slog.NewJSONHandler(os.Stdout, nil))
 
 	instanceIDs, err := s.instances.ListDeletedInstanceIDs(s.batchSize)
 	if err != nil {
 		slog.Error(fmt.Sprintf("Unable to get instance IDs: %s", err.Error()))
-		return err, 0, 0
+		return 0, 0, err
 	}
 	l.Info(fmt.Sprintf("Got %d instance IDs to process", len(instanceIDs)))
 
@@ -51,10 +51,10 @@ func (s *Service) Run() (error, int, int) {
 		if errInstance == nil {
 			logger.Error(fmt.Sprintf("the instance (createdAt: %s, planName: %s) still exists, aborting the process",
 				instance.InstanceID, instance.CreatedAt))
-			return fmt.Errorf("instance exists"), numberOfInstancesProcessed, numberOfOperationsDeleted
+			return numberOfInstancesProcessed, numberOfOperationsDeleted, fmt.Errorf("instance exists")
 		}
 		if !dberr.IsNotFound(errInstance) {
-			return errInstance, numberOfInstancesProcessed, numberOfOperationsDeleted
+			return numberOfInstancesProcessed, numberOfOperationsDeleted, errInstance
 		}
 
 		operations, err := s.operations.ListOperationsByInstanceID(instanceId)
@@ -110,5 +110,5 @@ func (s *Service) Run() (error, int, int) {
 		numberOfInstancesProcessed++
 	}
 
-	return nil, numberOfInstancesProcessed, numberOfOperationsDeleted
+	return numberOfInstancesProcessed, numberOfOperationsDeleted, nil
 }
diff --git a/internal/archive/service_test.go b/internal/archive/service_test.go
@@ -28,7 +28,7 @@ func TestService_Run(t *testing.T) {
 	service := NewService(db, false, true, 10)
 
 	// when
-	err, numberOfInstancesProcessed, numberOfOperationsDeleted := service.Run()
+	numberOfInstancesProcessed, numberOfOperationsDeleted, err := service.Run()
 
 	// then
 	require.NoError(t, err)
diff --git a/internal/broker/bind_create.go b/internal/broker/bind_create.go
@@ -62,7 +62,7 @@ func (b *BindingContext) CreatedBy() string {
 }
 
 type BindingParams struct {
-	ExpirationSeconds int `json:"expiration_seconds,omit"`
+	ExpirationSeconds int `json:"expiration_seconds,omitempty"`
 }
 
 type Credentials struct {
diff --git a/internal/cis/fake_server_test.go b/internal/cis/fake_server_test.go
@@ -11,8 +11,8 @@ import (
 
 func TestCisFakeServer(t *testing.T) {
 	srv, err := NewFakeServer()
-	defer srv.Close()
 	require.NoError(t, err)
+	defer srv.Close()
 
 	client := srv.Client()
 
diff --git a/internal/docker_helper.go b/internal/docker_helper.go
@@ -27,7 +27,7 @@ func NewDockerHandler() (*DockerHelper, error) {
 	if err != nil {
 		return nil, err
 	}
-	fmt.Println(fmt.Sprintf("host is -> %s", dockerClient.DaemonHost()))
+	fmt.Println("host is ->", dockerClient.DaemonHost())
 
 	return &DockerHelper{
 		client: dockerClient,
diff --git a/internal/error/errors.go b/internal/error/errors.go
@@ -20,7 +20,7 @@ type ErrorReporter interface {
 	GetComponent() Component
 }
 
-// error reporter
+// LastError holds information about the most recent error.
 type LastError struct {
 	Message   string    `json:"message,omitempty"`
 	Reason    Reason    `json:"reason,omitempty"`
diff --git a/internal/error/temporary_error.go b/internal/error/temporary_error.go
@@ -34,8 +34,8 @@ func IsTemporaryError(err error) bool {
 	return ok && nfe.Temporary()
 }
 
-// can be used for temporary error
-// but still storing the original error in case returned to Execute
+// WrapTemporaryError can be used for temporary errors,
+// but still stores the original error in case it's returned to Execute.
 type WrapTemporaryError struct {
 	err error
 }
@@ -48,8 +48,8 @@ func WrapNewTemporaryError(err error) *WrapTemporaryError {
 	return &WrapTemporaryError{err: err}
 }
 
-func (te WrapTemporaryError) Error() string { return te.err.Error() }
-func (WrapTemporaryError) Temporary() bool  { return true }
+func (wte WrapTemporaryError) Error() string { return wte.err.Error() }
+func (WrapTemporaryError) Temporary() bool   { return true }
 
 func (wte WrapTemporaryError) GetReason() Reason {
 	return ReasonForError(wte.err, NotSet).GetReason()
diff --git a/internal/expiration/handler.go b/internal/expiration/handler.go
@@ -99,8 +99,6 @@ func (h *handler) expireInstance(w http.ResponseWriter, req *http.Request) {
 
 	res := expirationResponse{suspensionOpID}
 	httputil.WriteResponse(w, http.StatusAccepted, res)
-
-	return
 }
 
 func (h *handler) setInstanceExpirationTime(instance *internal.Instance, log *slog.Logger) (*internal.Instance, error) {
diff --git a/internal/health/server.go b/internal/health/server.go
@@ -34,6 +34,5 @@ func (srv *Server) ServeAsync() {
 func livenessHandler() func(w http.ResponseWriter, _ *http.Request) {
 	return func(w http.ResponseWriter, _ *http.Request) {
 		w.WriteHeader(http.StatusOK)
-		return
 	}
 }
diff --git a/internal/httputil/client.go b/internal/httputil/client.go
diff --git a/internal/process/provisioning/apply_kyma_step_test.go b/internal/process/provisioning/apply_kyma_step_test.go
@@ -347,11 +347,6 @@ func TestUpdatinInternalKymaResourceIfExists(t *testing.T) {
 }
 
 func assertLabelsExists(t *testing.T, expectedLabels map[string]string, obj unstructured.Unstructured) {
-	keys := make([]string, 0, len(obj.GetLabels()))
-	for k := range obj.GetLabels() {
-		keys = append(keys, k)
-	}
-
 	if !assert.Subset(t, obj.GetLabels(), expectedLabels) {
 		t.Logf("Expected labels: %v", expectedLabels)
 		t.Logf("Actual labels:   %v", obj.GetLabels())
diff --git a/internal/process/provisioning/resolve_credentials_binding.go b/internal/process/provisioning/resolve_credentials_binding.go
@@ -182,12 +182,12 @@ func (s *ResolveCredentialsBindingStep) claimCredentialsBinding(credentialsBindi
 	return s.gardenerClient.UpdateCredentialsBinding(credentialsBinding)
 }
 
-func (step *ResolveCredentialsBindingStep) updateInstance(id, subscriptionSecretName string) error {
-	instance, err := step.instanceStorage.GetByID(id)
+func (s *ResolveCredentialsBindingStep) updateInstance(id, subscriptionSecretName string) error {
+	instance, err := s.instanceStorage.GetByID(id)
 	if err != nil {
 		return err
 	}
 	instance.SubscriptionSecretName = subscriptionSecretName
-	_, err = step.instanceStorage.Update(*instance)
+	_, err = s.instanceStorage.Update(*instance)
 	return err
 }
diff --git a/internal/process/update/update_runtime_step.go b/internal/process/update/update_runtime_step.go
@@ -173,7 +173,7 @@ func (s *UpdateRuntimeStep) getCurrentAdditionalWorkers(runtime imv1.Runtime) ma
 
 func (s *UpdateRuntimeStep) getRequiredClaims(dto *pkg.OIDCConfigDTO) map[string]string {
 	requiredClaims := make(map[string]string)
-	if !(len(dto.RequiredClaims) == 1 && dto.RequiredClaims[0] == "-") {
+	if len(dto.RequiredClaims) != 1 || dto.RequiredClaims[0] != "-" {
 		for _, claim := range dto.RequiredClaims {
 			parts := strings.SplitN(claim, "=", 2)
 			if len(parts) == 2 {
diff --git a/internal/storage/driver/postsql/events/events.go b/internal/storage/driver/postsql/events/events.go
@@ -43,13 +43,10 @@ func (e *events) RunGarbageCollection(pollingPeriod, retention time.Duration) {
 		return
 	}
 	ticker := time.NewTicker(pollingPeriod)
-	for {
-		select {
-		case <-ticker.C:
-			sess := e.Factory.NewWriteSession()
-			if err := sess.DeleteEvents(time.Now().Add(-retention)); err != nil {
-				slog.Error(fmt.Sprintf("failed to delete old events: %v", err))
-			}
+	for range ticker.C {
+		sess := e.Factory.NewWriteSession()
+		if err := sess.DeleteEvents(time.Now().Add(-retention)); err != nil {
+			slog.Error(fmt.Sprintf("failed to delete old events: %v", err))
 		}
 	}
 }
diff --git a/internal/storage/encrypt.go b/internal/storage/encrypt.go
@@ -80,7 +80,7 @@ func (e *Encrypter) encryptGCM(data []byte) ([]byte, error) {
 	return []byte(base64.StdEncoding.EncodeToString(encoded)), nil
 }
 
-// Decryption
+// DecryptFunc decrypts a byte slice.
 type DecryptFunc func(data []byte) ([]byte, error)
 
 func (e *Encrypter) decryptGCM(ciphertext []byte) ([]byte, error) {
diff --git a/internal/subaccountsync/state_reconciler_test.go b/internal/subaccountsync/state_reconciler_test.go
@@ -103,8 +103,8 @@ func TestStateReconcilerWithFakeCisServer(t *testing.T) {
 	defer teardownSuite(t)
 
 	srv, err := cis.NewFakeServer()
-	defer srv.Close()
 	require.NoError(t, err)
+	defer srv.Close()
 
 	cisClient := srv.Client()
 	cisConfig := CisEndpointConfig{

Original file line number	Diff line number	Diff line change
`@@ -31,13 +31,13 @@ func NewService(db storage.BrokerStorage, dryRun bool, performDeletion bool, bat`
`31`	`31`	`}`
`32`	`32`	`}`
`33`	`33`
`34`		`-func (s *Service) Run() (error, int, int) {`
	`34`	`+func (s *Service) Run() (int, int, error) {`
`35`	`35`	`l := slog.New(slog.NewJSONHandler(os.Stdout, nil))`
`36`	`36`
`37`	`37`	`instanceIDs, err := s.instances.ListDeletedInstanceIDs(s.batchSize)`
`38`	`38`	`if err != nil {`
`39`	`39`	`slog.Error(fmt.Sprintf("Unable to get instance IDs: %s", err.Error()))`
`40`		`- return err, 0, 0`
	`40`	`+ return 0, 0, err`
`41`	`41`	`}`
`42`	`42`	`l.Info(fmt.Sprintf("Got %d instance IDs to process", len(instanceIDs)))`
`43`	`43`
`@@ -51,10 +51,10 @@ func (s *Service) Run() (error, int, int) {`
`51`	`51`	`if errInstance == nil {`
`52`	`52`	`logger.Error(fmt.Sprintf("the instance (createdAt: %s, planName: %s) still exists, aborting the process",`
`53`	`53`	`instance.InstanceID, instance.CreatedAt))`
`54`		`- return fmt.Errorf("instance exists"), numberOfInstancesProcessed, numberOfOperationsDeleted`
	`54`	`+ return numberOfInstancesProcessed, numberOfOperationsDeleted, fmt.Errorf("instance exists")`
`55`	`55`	`}`
`56`	`56`	`if !dberr.IsNotFound(errInstance) {`
`57`		`- return errInstance, numberOfInstancesProcessed, numberOfOperationsDeleted`
	`57`	`+ return numberOfInstancesProcessed, numberOfOperationsDeleted, errInstance`
`58`	`58`	`}`
`59`	`59`
`60`	`60`	`operations, err := s.operations.ListOperationsByInstanceID(instanceId)`
`@@ -110,5 +110,5 @@ func (s *Service) Run() (error, int, int) {`
`110`	`110`	`numberOfInstancesProcessed++`
`111`	`111`	`}`
`112`	`112`
`113`		`- return nil, numberOfInstancesProcessed, numberOfOperationsDeleted`
	`113`	`+ return numberOfInstancesProcessed, numberOfOperationsDeleted, nil`
`114`	`114`	`}`
Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,7 @@ func (b *BindingContext) CreatedBy() string {`
`62`	`62`	`}`
`63`	`63`
`64`	`64`	`type BindingParams struct {`
`65`		- ExpirationSeconds int `json:"expiration_seconds,omit"`
	`65`	+ ExpirationSeconds int `json:"expiration_seconds,omitempty"`
`66`	`66`	`}`
`67`	`67`
`68`	`68`	`type Credentials struct {`
Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ func NewDockerHandler() (*DockerHelper, error) {`
`27`	`27`	`if err != nil {`
`28`	`28`	`return nil, err`
`29`	`29`	`}`
`30`		`- fmt.Println(fmt.Sprintf("host is -> %s", dockerClient.DaemonHost()))`
	`30`	`+ fmt.Println("host is ->", dockerClient.DaemonHost())`
`31`	`31`
`32`	`32`	`return &DockerHelper{`
`33`	`33`	`client: dockerClient,`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ type ErrorReporter interface {`
`20`	`20`	`GetComponent() Component`
`21`	`21`	`}`
`22`	`22`
`23`		`-// error reporter`
	`23`	`+// LastError holds information about the most recent error.`
`24`	`24`	`type LastError struct {`
`25`	`25`	Message string `json:"message,omitempty"`
`26`	`26`	Reason Reason `json:"reason,omitempty"`
Original file line number	Diff line number	Diff line change
`@@ -34,8 +34,8 @@ func IsTemporaryError(err error) bool {`
`34`	`34`	`return ok && nfe.Temporary()`
`35`	`35`	`}`
`36`	`36`
`37`		`-// can be used for temporary error`
`38`		`-// but still storing the original error in case returned to Execute`
	`37`	`+// WrapTemporaryError can be used for temporary errors,`
	`38`	`+// but still stores the original error in case it's returned to Execute.`
`39`	`39`	`type WrapTemporaryError struct {`
`40`	`40`	`err error`
`41`	`41`	`}`
`@@ -48,8 +48,8 @@ func WrapNewTemporaryError(err error) *WrapTemporaryError {`
`48`	`48`	`return &WrapTemporaryError{err: err}`
`49`	`49`	`}`
`50`	`50`
`51`		`-func (te WrapTemporaryError) Error() string { return te.err.Error() }`
`52`		`-func (WrapTemporaryError) Temporary() bool { return true }`
	`51`	`+func (wte WrapTemporaryError) Error() string { return wte.err.Error() }`
	`52`	`+func (WrapTemporaryError) Temporary() bool { return true }`
`53`	`53`
`54`	`54`	`func (wte WrapTemporaryError) GetReason() Reason {`
`55`	`55`	`return ReasonForError(wte.err, NotSet).GetReason()`
Original file line number	Diff line number	Diff line change
`@@ -99,8 +99,6 @@ func (h handler) expireInstance(w http.ResponseWriter, req http.Request) {`
`99`	`99`
`100`	`100`	`res := expirationResponse{suspensionOpID}`
`101`	`101`	`httputil.WriteResponse(w, http.StatusAccepted, res)`
`102`		`-`
`103`		`- return`
`104`	`102`	`}`
`105`	`103`
`106`	`104`	`func (h handler) setInstanceExpirationTime(instance internal.Instance, log slog.Logger) (internal.Instance, error) {`