Skip to content

Commit

Permalink
fix(lint): explicitly configures ireturn exceptions
Browse files Browse the repository at this point in the history
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: golangci/golangci-lint#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.

\#stack-pr
  • Loading branch information
bartoszmajsak committed Jun 26, 2024
1 parent 155dad0 commit ee05226
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 11 deletions.
9 changes: 9 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions controllers/status/reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ 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))
}

// SaveStatusFunc is a function that allow to define custom logic of updating status of a concrete resource object.
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")
Expand Down
1 change: 0 additions & 1 deletion pkg/feature/feature_data.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
//nolint:ireturn //reason: returning generic T data type
package feature

import (
Expand Down
3 changes: 1 addition & 2 deletions pkg/feature/provider/types.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
//nolint:ireturn //reason: returning generic T any (interface{})
package provider

import (
Expand Down Expand Up @@ -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
}

Expand Down
4 changes: 1 addition & 3 deletions pkg/plugins/addLabelsplugin.go
Original file line number Diff line number Diff line change
@@ -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"

Expand All @@ -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",
Expand Down
4 changes: 1 addition & 3 deletions pkg/plugins/namespacePlugin.go
Original file line number Diff line number Diff line change
@@ -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",
Expand Down

0 comments on commit ee05226

Please sign in to comment.