lint: enable revive if-return check and fix findings

Author: oscr

.golangci.yml

@@ -165,7 +165,7 @@ linters-settings:
       - name: error-strings
       - name: error-naming
       - name: exported
-      #- name: if-return  # TODO This is a recommended rule with many findings which may require it's own pr.
+      - name: if-return
       - name: increment-decrement
       - name: var-naming
       - name: var-declaration
@@ -268,17 +268,29 @@ issues:
     - staticcheck
     text: "SA1019: in.(.+) is deprecated"
     path: .*(api|types)\/.*\/conversion.*\.go$
+  - linters:
+      - revive
+    # Checking if an error is nil to just after return the error or nil is redundant
+    text: "if-return: redundant if ...; err != nil check, just return error instead"
+    # Ignoring stylistic checks for generated code
+    path: .*(api|types|test)\/.*\/conversion.*\.go$
   - linters:
     - revive
+    # Exported function and methods should have comments. This warns on undocumented exported functions and methods.
     text: exported (method|function|type|const) (.+) should have comment or be unexported
+    # Ignoring stylistic checks for generated code
     path: .*(api|types|test)\/.*\/conversion.*\.go$
   - linters:
     - revive
+    # This rule warns when initialism, variable or package naming conventions are not followed.
     text: "var-naming: don't use underscores in Go names;"
+    # Ignoring stylistic checks for generated code
     path: .*(api|types|test)\/.*\/conversion.*\.go$
   - linters:
     - revive
+    # By convention, receiver names in a method should reflect their identity.
     text: "receiver-naming: receiver name"
+    # Ignoring stylistic checks for generated code
     path: .*(api|types)\/.*\/conversion.*\.go$
   - linters:
     - stylecheck

cmd/clusterctl/client/cluster/cert_manager.go

@@ -289,11 +289,7 @@ func (cm *certManagerClient) migrateCRDs() error {
 		return err
 	}
 
-	if err := newCRDMigrator(c).Run(ctx, objs); err != nil {
-		return err
-	}
-
-	return nil
+	return newCRDMigrator(c).Run(ctx, objs)
 }
 
 func (cm *certManagerClient) deleteObjs(objs []unstructured.Unstructured) error {

cmd/clusterctl/client/cluster/proxy.go

@@ -190,13 +190,10 @@ func (k *proxy) CheckClusterAvailable() error {
 	}
 
 	connectBackoff := newShortConnectBackoff()
-	if err := retryWithExponentialBackoff(connectBackoff, func() error {
+	return retryWithExponentialBackoff(connectBackoff, func() error {
 		_, err := client.New(config, client.Options{Scheme: localScheme})
 		return err
-	}); err != nil {
-		return err
-	}
-	return nil
+	})
 }
 
 // ListResources lists namespaced and cluster-wide resources for a component matching the labels. Namespaced resources are only listed

hack/tools/mdbook/tabulate/tabulate.go

@@ -75,13 +75,9 @@ func (l Tabulate) Process(input *plugin.Input) error {
 		return err
 	}
 
-	if err := plugin.EachCommand(&input.Book, "/tabs", func(chapter *plugin.BookChapter, args string) (string, error) {
+	return plugin.EachCommand(&input.Book, "/tabs", func(chapter *plugin.BookChapter, args string) (string, error) {
 		return "</div></div>", nil
-	}); err != nil {
-		return err
-	}
-
-	return nil
+	})
 }
 
 func main() {

internal/controllers/clusterclass/clusterclass_controller_test.go

@@ -125,13 +125,9 @@ func assertInfrastructureClusterTemplate(ctx context.Context, actualClusterClass
 	}
 
 	// Assert the ClusterClass has the expected APIVersion and Kind of to the infrastructure cluster template
-	if err := referenceExistsWithCorrectKindAndAPIVersion(actualClusterClass.Spec.Infrastructure.Ref,
+	return referenceExistsWithCorrectKindAndAPIVersion(actualClusterClass.Spec.Infrastructure.Ref,
 		builder.GenericInfrastructureClusterTemplateKind,
-		builder.InfrastructureGroupVersion); err != nil {
-		return err
-	}
-
-	return nil
+		builder.InfrastructureGroupVersion)
 }
 
 func assertControlPlaneTemplate(ctx context.Context, actualClusterClass *clusterv1.ClusterClass, ns *corev1.Namespace) error {
@@ -224,13 +220,9 @@ func assertMachineDeploymentClass(ctx context.Context, actualClusterClass *clust
 	}
 
 	// Assert the MachineDeploymentClass has the expected APIVersion and Kind to the bootstrap template
-	if err := referenceExistsWithCorrectKindAndAPIVersion(mdClass.Template.Bootstrap.Ref,
+	return referenceExistsWithCorrectKindAndAPIVersion(mdClass.Template.Bootstrap.Ref,
 		builder.GenericBootstrapConfigTemplateKind,
-		builder.BootstrapGroupVersion); err != nil {
-		return err
-	}
-
-	return nil
+		builder.BootstrapGroupVersion)
 }
 
 func assertHasOwnerReference(obj client.Object, ownerRef metav1.OwnerReference) error {

internal/controllers/topology/cluster/cluster_controller_test.go

@@ -837,12 +837,9 @@ func assertClusterReconcile(cluster *clusterv1.Cluster) error {
 	}
 
 	// Check if ControlPlaneRef exists is of the expected Kind and APIVersion.
-	if err := referenceExistsWithCorrectKindAndAPIVersion(cluster.Spec.ControlPlaneRef,
+	return referenceExistsWithCorrectKindAndAPIVersion(cluster.Spec.ControlPlaneRef,
 		builder.TestControlPlaneKind,
-		builder.ControlPlaneGroupVersion); err != nil {
-		return err
-	}
-	return nil
+		builder.ControlPlaneGroupVersion)
 }
 
 // assertInfrastructureClusterReconcile checks if the infrastructureCluster object:
@@ -1023,10 +1020,7 @@ func assertLabelsAndAnnotations(got client.Object, clusterName string) error {
 	if err := assertClusterNameLabel(got, clusterName); err != nil {
 		return err
 	}
-	if err := assertTemplateClonedFromNameAnnotation(got); err != nil {
-		return err
-	}
-	return nil
+	return assertTemplateClonedFromNameAnnotation(got)
 }
 
 // assertClusterTopologyOwnedLabel asserts the label exists.

internal/controllers/topology/cluster/reconcile_state.go

@@ -190,11 +190,7 @@ func (r *Reconciler) callAfterHooks(ctx context.Context, s *scope.Scope) error {
 		return err
 	}
 
-	if err := r.callAfterClusterUpgrade(ctx, s); err != nil {
-		return err
-	}
-
-	return nil
+	return r.callAfterClusterUpgrade(ctx, s)
 }
 
 func (r *Reconciler) callAfterControlPlaneInitialized(ctx context.Context, s *scope.Scope) error {

internal/runtime/client/client.go

@@ -108,10 +108,7 @@ type client struct {
 }
 
 func (c *client) WarmUp(extensionConfigList *runtimev1.ExtensionConfigList) error {
-	if err := c.registry.WarmUp(extensionConfigList); err != nil {
-		return err
-	}
-	return nil
+	return c.registry.WarmUp(extensionConfigList)
 }
 
 func (c *client) IsReady() bool {