From c604a98b49a8e7f93aa9286d3df440c3c8bba277 Mon Sep 17 00:00:00 2001 From: bartoszmajsak Date: Wed, 26 Jun 2024 16:37:18 +0200 Subject: [PATCH] fix(lint): explicitly configures ireturn exceptions Instead of using `//nolint:ireturn` the linter allows a use of `generic` return types (on top of the listed defaults which now have been explicitly set). This approach also eliminates a re-occuring issue where `nolintlint` reports aforementioned directive as not used by defined linter. Related issue: https://github.com/golangci/golangci-lint/issues/3228 On top of that we have `--fix` flag enabled for golangci-lint runner. This results in removal of these comments as `nolintlint` tries to autofix. As a consequence the subsequent run of `make lint` yields errors for previously disabled linters and the cycle continues :) > [!IMPORTANT] > This behaviour has also been observed for other linters. As part of this the declarations of kustomize plugin constructor funcs has been reworkted to return struct pointers instead. --- .golangci.yml | 9 +++++++++ controllers/status/reporter.go | 4 ++-- pkg/feature/feature_data.go | 1 - pkg/feature/provider/types.go | 3 +-- pkg/plugins/addLabelsplugin.go | 4 +--- pkg/plugins/namespacePlugin.go | 4 +--- 6 files changed, 14 insertions(+), 11 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index f6305b15132..33d7ac791a0 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -43,6 +43,15 @@ linters-settings: alias: $1$2 - pkg: github.com/openshift/api/(\w+)/(v[\w\d]+) alias: $1$2 + ireturn: + allow: + # defaults https://golangci-lint.run/usage/linters/#ireturn + - anon + - error + - empty + - stdlib + # also allow generics + - generic revive: rules: - name: dot-imports diff --git a/controllers/status/reporter.go b/controllers/status/reporter.go index d2d7ddf27b7..de9dbe7e6fc 100644 --- a/controllers/status/reporter.go +++ b/controllers/status/reporter.go @@ -31,7 +31,7 @@ func NewStatusReporter[T client.Object](cli client.Client, object T, determine D } // ReportCondition updates the status of the object using the determineCondition function. -func (r *Reporter[T]) ReportCondition(ctx context.Context, optionalErr error) (T, error) { //nolint:ireturn //reason: returns T satisfying client.Object interface +func (r *Reporter[T]) ReportCondition(ctx context.Context, optionalErr error) (T, error) { return UpdateWithRetry[T](ctx, r.client, r.object, r.determineCondition(optionalErr)) } @@ -39,7 +39,7 @@ func (r *Reporter[T]) ReportCondition(ctx context.Context, optionalErr error) (T type SaveStatusFunc[T client.Object] func(saved T) // UpdateWithRetry updates the status of object using passed function and retries on conflict. -func UpdateWithRetry[T client.Object](ctx context.Context, cli client.Client, original T, update SaveStatusFunc[T]) (T, error) { //nolint:ireturn,lll //reason: returns T satisfying client.Object interface +func UpdateWithRetry[T client.Object](ctx context.Context, cli client.Client, original T, update SaveStatusFunc[T]) (T, error) { saved, ok := original.DeepCopyObject().(T) if !ok { return *new(T), errors.New("failed to deep copy object") diff --git a/pkg/feature/feature_data.go b/pkg/feature/feature_data.go index b883187195f..bd6829c4d95 100644 --- a/pkg/feature/feature_data.go +++ b/pkg/feature/feature_data.go @@ -1,4 +1,3 @@ -//nolint:ireturn //reason: returning generic T data type package feature import ( diff --git a/pkg/feature/provider/types.go b/pkg/feature/provider/types.go index 7699343dd77..1f0e4a33780 100644 --- a/pkg/feature/provider/types.go +++ b/pkg/feature/provider/types.go @@ -1,4 +1,3 @@ -//nolint:ireturn //reason: returning generic T any (interface{}) package provider import ( @@ -47,7 +46,7 @@ func (d DataProviderWithDefault[T]) Get(_ context.Context, _ client.Client) (T, } // Value returns actual value stored by the provider. -func (d DataProviderWithDefault[T]) Value() T { //nolint:ireturn //reason: returns generic T +func (d DataProviderWithDefault[T]) Value() T { return d.value } diff --git a/pkg/plugins/addLabelsplugin.go b/pkg/plugins/addLabelsplugin.go index 5faeac0d454..5991c157d4c 100644 --- a/pkg/plugins/addLabelsplugin.go +++ b/pkg/plugins/addLabelsplugin.go @@ -1,9 +1,7 @@ -//nolint:ireturn //reason: false positive, builtins.LabelTransformerPlugin is a struct package plugins import ( "sigs.k8s.io/kustomize/api/builtins" //nolint:staticcheck //Remove after package update - "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/resid" @@ -17,7 +15,7 @@ import ( // - It adds labels to the "metadata/labels" path for all resource kinds. // - It adds labels to the "spec/template/metadata/labels" and "spec/selector/matchLabels" paths // for resources of kind "Deployment". -func CreateAddLabelsPlugin(componentName string) resmap.Transformer { //nolint:ireturn //reason returning struct conflicts due to pointer receiver +func CreateAddLabelsPlugin(componentName string) *builtins.LabelTransformerPlugin { return &builtins.LabelTransformerPlugin{ Labels: map[string]string{ labels.ODH.Component(componentName): "true", diff --git a/pkg/plugins/namespacePlugin.go b/pkg/plugins/namespacePlugin.go index 0eb80fcdcb3..5204fdaa892 100644 --- a/pkg/plugins/namespacePlugin.go +++ b/pkg/plugins/namespacePlugin.go @@ -1,16 +1,14 @@ -//nolint:ireturn //reason: false positive, builtins.NamespaceTransformerPlugin is a struct package plugins import ( "sigs.k8s.io/kustomize/api/builtins" //nolint:staticcheck // Remove after package update "sigs.k8s.io/kustomize/api/filters/namespace" - "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/resid" ) // CreateNamespaceApplierPlugin creates a plugin to ensure resources have the specified target namespace. -func CreateNamespaceApplierPlugin(targetNamespace string) resmap.Transformer { //nolint:ireturn //reason returning struct conflicts due to pointer receiver +func CreateNamespaceApplierPlugin(targetNamespace string) *builtins.NamespaceTransformerPlugin { return &builtins.NamespaceTransformerPlugin{ ObjectMeta: types.ObjectMeta{ Name: "odh-namespace-plugin",