enable errorlint and testifylint linters (#2385)

Signed-off-by: Matthieu MOREL <[email protected]>
jaegertracing · Dec 13, 2023 · 7d16675 · 7d16675
1 parent 4f201e6
commit 7d16675
Show file tree

Hide file tree

Showing 85 changed files with 499 additions and 469 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -1,6 +1,10 @@
-run:
-  go: '1.21'
-  timeout: 10m
+issues:
+  # Excluding configuration per-path, per-linter, per-text and per-source
+  exclude-rules:
+    # Exclude some linters from running on tests files.
+    - path: _test\.go
+      linters:
+        - gosec
 
 linters-settings:
   goimports:
@@ -10,20 +14,17 @@ linters-settings:
 
 linters:
   enable:
-    - gofmt
+    - bidichk
+    - errorlint
     - gofumpt
     - goimports
     - gosec
     - govet
     - misspell
-    - bidichk
+    - testifylint
   disable:
     - errcheck
 
-issues:
-  # Excluding configuration per-path, per-linter, per-text and per-source
-  exclude-rules:
-    # Exclude some linters from running on tests files.
-    - path: _test\.go
-      linters:
-        - gosec
+run:
+  go: '1.20'
+  timeout: 10m
diff --git a/apis/v1/deployment_strategy_test.go b/apis/v1/deployment_strategy_test.go
@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestUnmarshalJSON(t *testing.T) {
@@ -28,7 +29,7 @@ func TestUnmarshalJSON(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			ds := DeploymentStrategy("")
 			err := json.Unmarshal([]byte(tc.json), &ds)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.Equal(t, tc.expected, ds)
 		})
 	}
@@ -48,7 +49,7 @@ func TestMarshalJSON(t *testing.T) {
 	for name, tc := range tcs {
 		t.Run(name, func(t *testing.T) {
 			data, err := json.Marshal(tc.strategy)
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.Equal(t, tc.expected, string(data))
 		})
 	}

diff --git a/apis/v1/freeform_test.go b/apis/v1/freeform_test.go
@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestFreeForm(t *testing.T) {
@@ -16,7 +17,7 @@ func TestFreeForm(t *testing.T) {
 		},
 	})
 	json, err := o.MarshalJSON()
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.NotNil(t, json)
 	assert.Equal(t, uiconfig, string(*o.json))
 }
@@ -26,7 +27,7 @@ func TestFreeFormUnmarhalMarshal(t *testing.T) {
 	o := NewFreeForm(nil)
 	o.UnmarshalJSON([]byte(uiconfig))
 	json, err := o.MarshalJSON()
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.NotNil(t, json)
 	assert.Equal(t, uiconfig, string(*o.json))
 }
@@ -66,9 +67,9 @@ func TestToMap(t *testing.T) {
 		f := NewFreeForm(test.m)
 		got, err := f.GetMap()
 		if test.err != "" {
-			assert.EqualError(t, err, test.err)
+			require.EqualError(t, err, test.err)
 		} else {
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.Equal(t, test.expected, got)
 		}
 	}

diff --git a/apis/v1/jaeger_webhook.go b/apis/v1/jaeger_webhook.go
@@ -104,7 +104,7 @@ func (j *Jaeger) ValidateUpdate(_ runtime.Object) (admission.Warnings, error) {
 			Name:      j.Spec.Storage.Elasticsearch.Name,
 		}, es)
 		if errors.IsNotFound(err) {
-			return nil, fmt.Errorf("elasticsearch instance not found: %v", err)
+			return nil, fmt.Errorf("elasticsearch instance not found: %w", err)
 		}
 	}
 

diff --git a/apis/v1/jaeger_webhook_test.go b/apis/v1/jaeger_webhook_test.go
@@ -13,6 +13,7 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestDefault(t *testing.T) {
@@ -179,7 +180,7 @@ func TestDefault(t *testing.T) {
 func TestValidateDelete(t *testing.T) {
 	warnings, err := new(Jaeger).ValidateDelete()
 	assert.Nil(t, warnings)
-	assert.Nil(t, err)
+	require.NoError(t, err)
 }
 
 func TestValidate(t *testing.T) {
@@ -279,10 +280,10 @@ func TestValidate(t *testing.T) {
 
 			warnings, err := test.current.ValidateCreate()
 			if test.err != "" {
-				assert.NotNil(t, err)
+				require.Error(t, err)
 				assert.Equal(t, test.err, err.Error())
 			} else {
-				assert.Nil(t, err)
+				require.NoError(t, err)
 			}
 			assert.Nil(t, warnings)
 		})

diff --git a/apis/v1/options_test.go b/apis/v1/options_test.go
@@ -18,7 +18,7 @@ func TestSimpleOption(t *testing.T) {
 
 func TestNoOptions(t *testing.T) {
 	o := Options{}
-	assert.Len(t, o.ToArgs(), 0)
+	assert.Empty(t, o.ToArgs())
 }
 
 func TestNestedOption(t *testing.T) {
@@ -40,7 +40,7 @@ func TestMarshalling(t *testing.T) {
 	})
 
 	b, err := json.Marshal(o)
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	s := string(b)
 	assert.Contains(t, s, `"es.password":"changeme"`)
 	assert.Contains(t, s, `"es.server-urls":"http://elasticsearch.default.svc:9200"`)
@@ -85,9 +85,9 @@ func TestUnmarshalToArgs(t *testing.T) {
 		opts := Options{}
 		err := opts.UnmarshalJSON([]byte(test.in))
 		if test.err != "" {
-			assert.EqualError(t, err, test.err)
+			require.EqualError(t, err, test.err)
 		} else {
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			args := opts.ToArgs()
 			sort.SliceStable(args, func(i, j int) bool {
 				return args[i] < args[j]
@@ -129,15 +129,15 @@ func TestMarshallRaw(t *testing.T) {
 	o := NewOptions(nil)
 	o.json = &json
 	bytes, err := o.MarshalJSON()
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, bytes, json)
 }
 
 func TestMarshallEmpty(t *testing.T) {
 	o := NewOptions(nil)
 	json := []byte(`{}`)
 	bytes, err := o.MarshalJSON()
-	assert.NoError(t, err)
+	require.NoError(t, err)
 	assert.Equal(t, bytes, json)
 }
 
@@ -151,7 +151,7 @@ func TestUpdate(t *testing.T) {
 	o.Map()["key"] = "new"
 
 	// verify
-	assert.Equal(t, o.opts["key"], "new")
+	assert.Equal(t, "new", o.opts["key"])
 }
 
 func TestStringMap(t *testing.T) {
@@ -170,7 +170,7 @@ func TestDeepCopy(t *testing.T) {
 	require.NoError(t, err)
 	copy := o1.opts.DeepCopy()
 
-	assert.Equal(t, copy, &(o1.opts))
+	assert.Equal(t, &(o1.opts), copy)
 }
 
 func TestRepetitiveArguments(t *testing.T) {

diff --git a/controllers/appsv1/namespace_controller_test.go b/controllers/appsv1/namespace_controller_test.go
@@ -3,7 +3,6 @@ package appsv1_test
 import (
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	k8sconfig "sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
@@ -27,5 +26,5 @@ func TestNamespaceControllerRegisterWithManager(t *testing.T) {
 	err = reconciler.SetupWithManager(mgr)
 
 	// verify
-	assert.NoError(t, err)
+	require.NoError(t, err)
 }
diff --git a/controllers/jaegertracing/jaeger_controller_test.go b/controllers/jaegertracing/jaeger_controller_test.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"k8s.io/apimachinery/pkg/types"
 	k8sconfig "sigs.k8s.io/controller-runtime/pkg/client/config"
@@ -52,5 +51,5 @@ func TestRegisterWithManager(t *testing.T) {
 	err = reconciler.SetupWithManager(mgr)
 
 	// verify
-	assert.NoError(t, err)
+	require.NoError(t, err)
 }
diff --git a/hack/install/install-golangci-lint.sh b/hack/install/install-golangci-lint.sh
@@ -1,5 +1,5 @@
 #!/bin/bash
-VERSION="1.53.2"
+VERSION="1.55.2"
 
 echo "Installing golangci-lint"
 

diff --git a/pkg/autoclean/main_test.go b/pkg/autoclean/main_test.go
@@ -92,7 +92,7 @@ func TestCleanDeployments(t *testing.T) {
 			dep = inject.Sidecar(jaeger, dep)
 
 			// sanity check
-			require.Equal(t, 2, len(dep.Spec.Template.Spec.Containers))
+			require.Len(t, dep.Spec.Template.Spec.Containers, 2)
 
 			// prepare the list of existing objects
 			objs := []runtime.Object{dep}
@@ -121,10 +121,10 @@ func TestCleanDeployments(t *testing.T) {
 
 			// should the sidecar have been deleted?
 			if tt.deleted {
-				assert.Equal(t, 1, len(persisted.Spec.Template.Spec.Containers))
+				assert.Len(t, persisted.Spec.Template.Spec.Containers, 1)
 				assert.NotContains(t, persisted.Labels, inject.Label)
 			} else {
-				assert.Equal(t, 2, len(persisted.Spec.Template.Spec.Containers))
+				assert.Len(t, persisted.Spec.Template.Spec.Containers, 2)
 				assert.Contains(t, persisted.Labels, inject.Label)
 			}
 		})

diff --git a/pkg/autodetect/main.go b/pkg/autodetect/main.go
@@ -186,7 +186,7 @@ func AvailableAPIs(discovery discovery.DiscoveryInterface, groups map[string]boo
 			if err == nil {
 				apiLists = append(apiLists, groupAPIList)
 			} else {
-				errors = fmt.Errorf("%v; Error getting resources for server group %s: %v", errors, sg.Name, err)
+				errors = fmt.Errorf("%w; Error getting resources for server group %s: %w", errors, sg.Name, err)
 			}
 		}
 	}

diff --git a/pkg/clusterrolebinding/main_test.go b/pkg/clusterrolebinding/main_test.go
@@ -36,7 +36,7 @@ func TestGetClusterRoleBinding(t *testing.T) {
 	assert.Len(t, crbs[0].Subjects, 1)
 	assert.Equal(t, account.OAuthProxyAccountNameFor(jaeger), crbs[0].Subjects[0].Name)
 	assert.Equal(t, "ServiceAccount", crbs[0].Subjects[0].Kind)
-	assert.Len(t, crbs[0].Subjects[0].Namespace, 0) // cluster roles aren't namespaced
+	assert.Empty(t, crbs[0].Subjects[0].Namespace) // cluster roles aren't namespaced
 }
 
 func TestIngressDisabled(t *testing.T) {
@@ -53,7 +53,7 @@ func TestIngressDisabled(t *testing.T) {
 	crbs := Get(jaeger)
 
 	// verify
-	assert.Len(t, crbs, 0)
+	assert.Empty(t, crbs)
 }
 
 func TestNotOAuthProxy(t *testing.T) {
@@ -70,7 +70,7 @@ func TestNotOAuthProxy(t *testing.T) {
 	crbs := Get(jaeger)
 
 	// verify
-	assert.Len(t, crbs, 0)
+	assert.Empty(t, crbs)
 }
 
 func TestAuthDelegatorNotAvailable(t *testing.T) {
@@ -90,5 +90,5 @@ func TestAuthDelegatorNotAvailable(t *testing.T) {
 	crbs := Get(jaeger)
 
 	// verify
-	assert.Len(t, crbs, 0)
+	assert.Empty(t, crbs)
 }
diff --git a/pkg/cmd/generate/main.go b/pkg/cmd/generate/main.go
@@ -2,6 +2,7 @@ package generate
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -55,7 +56,7 @@ func createSpecFromYAML(filename string) (*v1.Jaeger, error) {
 
 	var spec v1.Jaeger
 	decoder := yaml.NewYAMLOrJSONDecoder(f, 8192)
-	if err := decoder.Decode(&spec); err != nil && err != io.EOF {
+	if err := decoder.Decode(&spec); err != nil && !errors.Is(err, io.EOF) {
 		return nil, err
 	}
 

diff --git a/pkg/config/ca/ca_test.go b/pkg/config/ca/ca_test.go
@@ -108,8 +108,8 @@ func TestUpdateWithoutCAs(t *testing.T) {
 	AddServiceCA(jaeger, &commonSpec)
 
 	// verify
-	assert.Len(t, commonSpec.Volumes, 0)
-	assert.Len(t, commonSpec.VolumeMounts, 0)
+	assert.Empty(t, commonSpec.Volumes)
+	assert.Empty(t, commonSpec.VolumeMounts)
 }
 
 func TestUpdateWithTrustedCA(t *testing.T) {
@@ -152,6 +152,6 @@ func TestUpdateWithExistingTrustedCA(t *testing.T) {
 	AddServiceCA(jaeger, &commonSpec)
 
 	// verify
-	assert.Len(t, commonSpec.Volumes, 0)
-	assert.Len(t, commonSpec.VolumeMounts, 0)
+	assert.Empty(t, commonSpec.Volumes)
+	assert.Empty(t, commonSpec.VolumeMounts)
 }
diff --git a/pkg/config/sampling/sampling_test.go b/pkg/config/sampling/sampling_test.go
@@ -91,9 +91,9 @@ func TestUpdateWithSamplingConfigFileOption(t *testing.T) {
 	commonSpec := v1.JaegerCommonSpec{}
 
 	Update(jaeger, &commonSpec, &options)
-	assert.Len(t, commonSpec.Volumes, 0)
-	assert.Len(t, commonSpec.VolumeMounts, 0)
-	assert.Len(t, options, 0)
+	assert.Empty(t, commonSpec.Volumes)
+	assert.Empty(t, commonSpec.VolumeMounts)
+	assert.Empty(t, options)
 }
 
 func TestGetWithSamplingConfigFileOption(t *testing.T) {

diff --git a/pkg/config/tls/tls_test.go b/pkg/config/tls/tls_test.go
@@ -35,8 +35,8 @@ func TestIgnoreDefaultTLSSecretWhenGrpcHostPortIsSet(t *testing.T) {
 	options = append(options, "--reporter.grpc.host-port=my.host-port.com")
 
 	Update(jaeger, &commonSpec, &options)
-	assert.Len(t, commonSpec.Volumes, 0)
-	assert.Len(t, commonSpec.VolumeMounts, 0)
+	assert.Empty(t, commonSpec.Volumes)
+	assert.Empty(t, commonSpec.VolumeMounts)
 	assert.Len(t, options, 1)
 	assert.Equal(t, "--reporter.grpc.host-port=my.host-port.com", options[0])
 }
diff --git a/pkg/config/ui/ui_test.go b/pkg/config/ui/ui_test.go
@@ -49,9 +49,9 @@ func TestUpdateNoUIConfig(t *testing.T) {
 	options := []string{}
 
 	Update(jaeger, &commonSpec, &options)
-	assert.Len(t, commonSpec.Volumes, 0)
-	assert.Len(t, commonSpec.VolumeMounts, 0)
-	assert.Len(t, options, 0)
+	assert.Empty(t, commonSpec.Volumes)
+	assert.Empty(t, commonSpec.VolumeMounts)
+	assert.Empty(t, options)
 }
 
 func TestUpdateWithUIConfig(t *testing.T) {

diff --git a/pkg/consolelink/main_test.go b/pkg/consolelink/main_test.go
@@ -51,7 +51,7 @@ func TestUpdateHref(t *testing.T) {
 	}
 
 	link := Get(jaeger, &route)
-	assert.Equal(t, link.Spec.Href, "")
+	assert.Equal(t, "", link.Spec.Href)
 	route.Spec.Host = "namespace.somehostname"
 	newLinks := UpdateHref([]corev1.Route{route}, []consolev1.ConsoleLink{*link})
 	assert.Equal(t, fmt.Sprintf("https://%s", route.Spec.Host), newLinks[0].Spec.Href)