chore: enable early-return, indent-error-flow and unnecessary-stmt from revive (#674)

mmorel-35 · web-flow · commit 70bee6a3a5d4 · 2025-02-07T12:43:30.000-05:00
Signed-off-by: Matthieu MOREL &lt;matthieu.morel35@gmail.com&gt;
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -61,7 +61,6 @@ linters-settings:
       - name: duplicated-imports
         disabled: true
       - name: early-return
-        disabled: true
         arguments:
           - 'preserveScope'
       - name: empty-block
@@ -77,7 +76,6 @@ linters-settings:
       - name: increment-decrement
         disabled: true
       - name: indent-error-flow
-        disabled: true
         arguments:
           - 'preserveScope'
       - name: modifies-parameter
@@ -97,7 +95,6 @@ linters-settings:
       - name: unexported-return
         disabled: true
       - name: unnecessary-stmt
-        disabled: true
       - name: unreachable-code
       - name: unused-parameter
       - name: use-any
diff --git a/pkg/cache/cluster.go b/pkg/cache/cluster.go
@@ -646,10 +646,9 @@ func (c *clusterCache) loadInitialState(ctx context.Context, api kube.APIResourc
 			c.replaceResourceCache(api.GroupKind, items, ns)
 			return nil
 		})
-	} else {
-		c.replaceResourceCache(api.GroupKind, items, ns)
-		return resourceVersion, nil
 	}
+	c.replaceResourceCache(api.GroupKind, items, ns)
+	return resourceVersion, nil
 }
 
 func (c *clusterCache) watchEvents(ctx context.Context, api kube.APIResourceInfo, resClient dynamic.ResourceInterface, ns string, resourceVersion string) {
@@ -845,10 +844,10 @@ func (c *clusterCache) checkPermission(ctx context.Context, reviewInterface auth
 			}
 			if resp != nil && resp.Status.Allowed {
 				return true, nil
-			} else {
-				// unsupported, remove from watch list
-				return false, nil
 			}
+			// unsupported, remove from watch list
+			//nolint:staticcheck //FIXME
+			return false, nil
 		}
 	}
 	// checkPermission follows the same logic of determining namespace/cluster resource as the processApi function
@@ -1157,13 +1156,12 @@ func buildGraph(nsNodes map[kube.ResourceKey]*Resource) map[kube.ResourceKey]map
 					continue
 				}
 				graphKeyNode, ok := nsNodes[kube.ResourceKey{Group: group.Group, Kind: ownerRef.Kind, Namespace: childNode.Ref.Namespace, Name: ownerRef.Name}]
-				if ok {
-					ownerRef.UID = graphKeyNode.Ref.UID
-					childNode.OwnerRefs[i] = ownerRef
-				} else {
+				if !ok {
 					// No resource found with the given graph key, so move on.
 					continue
 				}
+				ownerRef.UID = graphKeyNode.Ref.UID
+				childNode.OwnerRefs[i] = ownerRef
 			}
 
 			// Now that we have the UID of the parent, update the graph.
diff --git a/pkg/diff/diff.go b/pkg/diff/diff.go
@@ -474,9 +474,8 @@ func handleResourceCreateOrDeleteDiff(config, live *unstructured.Unstructured) (
 			return nil, err
 		}
 		return &DiffResult{Modified: true, NormalizedLive: []byte("null"), PredictedLive: predictedLiveData}, nil
-	} else {
-		return nil, errors.New("both live and config are null objects")
 	}
+	return nil, errors.New("both live and config are null objects")
 }
 
 // generateSchemeDefaultPatch runs the scheme default functions on the given parameter, and
@@ -740,22 +739,21 @@ func threeWayMergePatch(orig, config, live *unstructured.Unstructured) ([]byte,
 			return scheme.Scheme.New(orig.GroupVersionKind())
 		}
 		return patch, newVersionedObject, nil
-	} else {
-		// Remove defaulted fields from the live object.
-		// This subtracts any extra fields in the live object which are not present in last-applied-configuration.
-		live = &unstructured.Unstructured{Object: jsonutil.RemoveMapFields(orig.Object, live.Object)}
+	}
+	// Remove defaulted fields from the live object.
+	// This subtracts any extra fields in the live object which are not present in last-applied-configuration.
+	live = &unstructured.Unstructured{Object: jsonutil.RemoveMapFields(orig.Object, live.Object)}
 
-		liveBytes, err := json.Marshal(live.Object)
-		if err != nil {
-			return nil, nil, err
-		}
+	liveBytes, err := json.Marshal(live.Object)
+	if err != nil {
+		return nil, nil, err
+	}
 
-		patch, err := jsonmergepatch.CreateThreeWayJSONMergePatch(origBytes, configBytes, liveBytes)
-		if err != nil {
-			return nil, nil, err
-		}
-		return patch, nil, nil
+	patch, err := jsonmergepatch.CreateThreeWayJSONMergePatch(origBytes, configBytes, liveBytes)
+	if err != nil {
+		return nil, nil, err
 	}
+	return patch, nil, nil
 }
 
 func GetLastAppliedConfigAnnotation(live *unstructured.Unstructured) (*unstructured.Unstructured, error) {
diff --git a/pkg/health/health.go b/pkg/health/health.go
@@ -112,23 +112,19 @@ func GetHealthCheckFunc(gvk schema.GroupVersionKind) func(obj *unstructured.Unst
 			return getDaemonSetHealth
 		}
 	case "extensions":
-		switch gvk.Kind {
-		case kube.IngressKind:
+		if gvk.Kind == kube.IngressKind {
 			return getIngressHealth
 		}
 	case "argoproj.io":
-		switch gvk.Kind {
-		case "Workflow":
+		if gvk.Kind == "Workflow" {
 			return getArgoWorkflowHealth
 		}
 	case "apiregistration.k8s.io":
-		switch gvk.Kind {
-		case kube.APIServiceKind:
+		if gvk.Kind == kube.APIServiceKind {
 			return getAPIServiceHealth
 		}
 	case "networking.k8s.io":
-		switch gvk.Kind {
-		case kube.IngressKind:
+		if gvk.Kind == kube.IngressKind {
 			return getIngressHealth
 		}
 	case "":
@@ -141,13 +137,11 @@ func GetHealthCheckFunc(gvk schema.GroupVersionKind) func(obj *unstructured.Unst
 			return getPodHealth
 		}
 	case "batch":
-		switch gvk.Kind {
-		case kube.JobKind:
+		if gvk.Kind == kube.JobKind {
 			return getJobHealth
 		}
 	case "autoscaling":
-		switch gvk.Kind {
-		case kube.HorizontalPodAutoscalerKind:
+		if gvk.Kind == kube.HorizontalPodAutoscalerKind {
 			return getHPAHealth
 		}
 	}
diff --git a/pkg/health/health_apiservice.go b/pkg/health/health_apiservice.go
@@ -35,19 +35,17 @@ func getAPIServiceHealth(obj *unstructured.Unstructured) (*HealthStatus, error)
 
 func getApiregistrationv1APIServiceHealth(apiservice *apiregistrationv1.APIService) (*HealthStatus, error) {
 	for _, c := range apiservice.Status.Conditions {
-		switch c.Type {
-		case apiregistrationv1.Available:
+		if c.Type == apiregistrationv1.Available {
 			if c.Status == apiregistrationv1.ConditionTrue {
 				return &HealthStatus{
 					Status:  HealthStatusHealthy,
 					Message: fmt.Sprintf("%s: %s", c.Reason, c.Message),
 				}, nil
-			} else {
-				return &HealthStatus{
-					Status:  HealthStatusProgressing,
-					Message: fmt.Sprintf("%s: %s", c.Reason, c.Message),
-				}, nil
 			}
+			return &HealthStatus{
+				Status:  HealthStatusProgressing,
+				Message: fmt.Sprintf("%s: %s", c.Reason, c.Message),
+			}, nil
 		}
 	}
 	return &HealthStatus{
@@ -58,19 +56,17 @@ func getApiregistrationv1APIServiceHealth(apiservice *apiregistrationv1.APIServi
 
 func getApiregistrationv1beta1APIServiceHealth(apiservice *apiregistrationv1beta1.APIService) (*HealthStatus, error) {
 	for _, c := range apiservice.Status.Conditions {
-		switch c.Type {
-		case apiregistrationv1beta1.Available:
+		if c.Type == apiregistrationv1beta1.Available {
 			if c.Status == apiregistrationv1beta1.ConditionTrue {
 				return &HealthStatus{
 					Status:  HealthStatusHealthy,
 					Message: fmt.Sprintf("%s: %s", c.Reason, c.Message),
 				}, nil
-			} else {
-				return &HealthStatus{
-					Status:  HealthStatusProgressing,
-					Message: fmt.Sprintf("%s: %s", c.Reason, c.Message),
-				}, nil
 			}
+			return &HealthStatus{
+				Status:  HealthStatusProgressing,
+				Message: fmt.Sprintf("%s: %s", c.Reason, c.Message),
+			}, nil
 		}
 	}
 	return &HealthStatus{
diff --git a/pkg/health/health_daemonset.go b/pkg/health/health_daemonset.go
@@ -27,31 +27,30 @@ func getDaemonSetHealth(obj *unstructured.Unstructured) (*HealthStatus, error) {
 
 func getAppsv1DaemonSetHealth(daemon *appsv1.DaemonSet) (*HealthStatus, error) {
 	// Borrowed at kubernetes/kubectl/rollout_status.go https://github.com/kubernetes/kubernetes/blob/5232ad4a00ec93942d0b2c6359ee6cd1201b46bc/pkg/kubectl/rollout_status.go#L110
-	if daemon.Generation <= daemon.Status.ObservedGeneration {
-		if daemon.Spec.UpdateStrategy.Type == appsv1.OnDeleteDaemonSetStrategyType {
-			return &HealthStatus{
-				Status:  HealthStatusHealthy,
-				Message: fmt.Sprintf("daemon set %d out of %d new pods have been updated", daemon.Status.UpdatedNumberScheduled, daemon.Status.DesiredNumberScheduled),
-			}, nil
-		}
-		if daemon.Status.UpdatedNumberScheduled < daemon.Status.DesiredNumberScheduled {
-			return &HealthStatus{
-				Status:  HealthStatusProgressing,
-				Message: fmt.Sprintf("Waiting for daemon set %q rollout to finish: %d out of %d new pods have been updated...", daemon.Name, daemon.Status.UpdatedNumberScheduled, daemon.Status.DesiredNumberScheduled),
-			}, nil
-		}
-		if daemon.Status.NumberAvailable < daemon.Status.DesiredNumberScheduled {
-			return &HealthStatus{
-				Status:  HealthStatusProgressing,
-				Message: fmt.Sprintf("Waiting for daemon set %q rollout to finish: %d of %d updated pods are available...", daemon.Name, daemon.Status.NumberAvailable, daemon.Status.DesiredNumberScheduled),
-			}, nil
-		}
-	} else {
+	if daemon.Generation > daemon.Status.ObservedGeneration {
 		return &HealthStatus{
 			Status:  HealthStatusProgressing,
 			Message: "Waiting for rollout to finish: observed daemon set generation less than desired generation",
 		}, nil
 	}
+	if daemon.Spec.UpdateStrategy.Type == appsv1.OnDeleteDaemonSetStrategyType {
+		return &HealthStatus{
+			Status:  HealthStatusHealthy,
+			Message: fmt.Sprintf("daemon set %d out of %d new pods have been updated", daemon.Status.UpdatedNumberScheduled, daemon.Status.DesiredNumberScheduled),
+		}, nil
+	}
+	if daemon.Status.UpdatedNumberScheduled < daemon.Status.DesiredNumberScheduled {
+		return &HealthStatus{
+			Status:  HealthStatusProgressing,
+			Message: fmt.Sprintf("Waiting for daemon set %q rollout to finish: %d out of %d new pods have been updated...", daemon.Name, daemon.Status.UpdatedNumberScheduled, daemon.Status.DesiredNumberScheduled),
+		}, nil
+	}
+	if daemon.Status.NumberAvailable < daemon.Status.DesiredNumberScheduled {
+		return &HealthStatus{
+			Status:  HealthStatusProgressing,
+			Message: fmt.Sprintf("Waiting for daemon set %q rollout to finish: %d of %d updated pods are available...", daemon.Name, daemon.Status.NumberAvailable, daemon.Status.DesiredNumberScheduled),
+		}, nil
+	}
 	return &HealthStatus{
 		Status: HealthStatusHealthy,
 	}, nil
diff --git a/pkg/health/health_job.go b/pkg/health/health_job.go
@@ -65,10 +65,9 @@ func getBatchv1JobHealth(job *batchv1.Job) (*HealthStatus, error) {
 			Status:  HealthStatusSuspended,
 			Message: failMsg,
 		}, nil
-	} else {
-		return &HealthStatus{
-			Status:  HealthStatusHealthy,
-			Message: message,
-		}, nil
 	}
+	return &HealthStatus{
+		Status:  HealthStatusHealthy,
+		Message: message,
+	}, nil
 }
diff --git a/pkg/sync/sync_context.go b/pkg/sync/sync_context.go
@@ -905,9 +905,8 @@ func isNamespaceKind(res *unstructured.Unstructured) bool {
 func obj(a, b *unstructured.Unstructured) *unstructured.Unstructured {
 	if a != nil {
 		return a
-	} else {
-		return b
 	}
+	return b
 }
 
 func (sc *syncContext) liveObj(obj *unstructured.Unstructured) *unstructured.Unstructured {
@@ -1021,21 +1020,19 @@ func (sc *syncContext) pruneObject(liveObj *unstructured.Unstructured, prune, dr
 		return common.ResultCodePruneSkipped, "ignored (requires pruning)"
 	} else if resourceutil.HasAnnotationOption(liveObj, common.AnnotationSyncOptions, common.SyncOptionDisablePrune) {
 		return common.ResultCodePruneSkipped, "ignored (no prune)"
-	} else {
-		if dryRun {
-			return common.ResultCodePruned, "pruned (dry run)"
-		} else {
-			// Skip deletion if object is already marked for deletion, so we don't cause a resource update hotloop
-			deletionTimestamp := liveObj.GetDeletionTimestamp()
-			if deletionTimestamp == nil || deletionTimestamp.IsZero() {
-				err := sc.kubectl.DeleteResource(context.TODO(), sc.config, liveObj.GroupVersionKind(), liveObj.GetName(), liveObj.GetNamespace(), sc.getDeleteOptions())
-				if err != nil {
-					return common.ResultCodeSyncFailed, err.Error()
-				}
-			}
-			return common.ResultCodePruned, "pruned"
+	}
+	if dryRun {
+		return common.ResultCodePruned, "pruned (dry run)"
+	}
+	// Skip deletion if object is already marked for deletion, so we don't cause a resource update hotloop
+	deletionTimestamp := liveObj.GetDeletionTimestamp()
+	if deletionTimestamp == nil || deletionTimestamp.IsZero() {
+		err := sc.kubectl.DeleteResource(context.TODO(), sc.config, liveObj.GroupVersionKind(), liveObj.GetName(), liveObj.GetNamespace(), sc.getDeleteOptions())
+		if err != nil {
+			return common.ResultCodeSyncFailed, err.Error()
 		}
 	}
+	return common.ResultCodePruned, "pruned"
 }
 
 func (sc *syncContext) getDeleteOptions() metav1.DeleteOptions {
diff --git a/pkg/sync/sync_phase.go b/pkg/sync/sync_phase.go
@@ -23,7 +23,6 @@ func syncPhases(obj *unstructured.Unstructured) []common.SyncPhase {
 			phases = append(phases, phase)
 		}
 		return phases
-	} else {
-		return []common.SyncPhase{common.SyncPhaseSync}
 	}
+	return []common.SyncPhase{common.SyncPhaseSync}
 }
diff --git a/pkg/sync/sync_task.go b/pkg/sync/sync_task.go
@@ -29,9 +29,8 @@ type syncTask struct {
 func ternary(val bool, a, b string) string {
 	if val {
 		return a
-	} else {
-		return b
 	}
+	return b
 }
 
 func (t *syncTask) String() string {
@@ -115,9 +114,8 @@ func (t *syncTask) pruned() bool {
 func (t *syncTask) hookType() common.HookType {
 	if t.isHook() {
 		return common.HookType(t.phase)
-	} else {
-		return ""
 	}
+	return ""
 }
 
 func (t *syncTask) hasHookDeletePolicy(policy common.HookDeletePolicy) bool {
diff --git a/pkg/utils/kube/kube.go b/pkg/utils/kube/kube.go
@@ -190,11 +190,10 @@ func ServerResourceForGroupVersionKind(disco discovery.DiscoveryInterface, gvk s
 		if r.Kind == gvk.Kind {
 			if isSupportedVerb(&r, verb) {
 				return &r, nil
-			} else {
-				// We have a match, but the API does not support the action
-				// that was requested. Memorize this.
-				retErr = apierr.NewMethodNotSupported(schema.GroupResource{Group: gvk.Group, Resource: gvk.Kind}, verb)
 			}
+			// We have a match, but the API does not support the action
+			// that was requested. Memorize this.
+			retErr = apierr.NewMethodNotSupported(schema.GroupResource{Group: gvk.Group, Resource: gvk.Kind}, verb)
 		}
 	}
 	return nil, retErr

Original file line number	Diff line number	Diff line change
`@@ -646,10 +646,9 @@ func (c *clusterCache) loadInitialState(ctx context.Context, api kube.APIResourc`
`646`	`646`	`c.replaceResourceCache(api.GroupKind, items, ns)`
`647`	`647`	`return nil`
`648`	`648`	`})`
`649`		`- } else {`
`650`		`- c.replaceResourceCache(api.GroupKind, items, ns)`
`651`		`- return resourceVersion, nil`
`652`	`649`	`}`
	`650`	`+ c.replaceResourceCache(api.GroupKind, items, ns)`
	`651`	`+ return resourceVersion, nil`
`653`	`652`	`}`
`654`	`653`
`655`	`654`	`func (c *clusterCache) watchEvents(ctx context.Context, api kube.APIResourceInfo, resClient dynamic.ResourceInterface, ns string, resourceVersion string) {`
`@@ -845,10 +844,10 @@ func (c *clusterCache) checkPermission(ctx context.Context, reviewInterface auth`
`845`	`844`	`}`
`846`	`845`	`if resp != nil && resp.Status.Allowed {`
`847`	`846`	`return true, nil`
`848`		`- } else {`
`849`		`- // unsupported, remove from watch list`
`850`		`- return false, nil`
`851`	`847`	`}`
	`848`	`+ // unsupported, remove from watch list`
	`849`	`+ //nolint:staticcheck //FIXME`
	`850`	`+ return false, nil`
`852`	`851`	`}`
`853`	`852`	`}`
`854`	`853`	`// checkPermission follows the same logic of determining namespace/cluster resource as the processApi function`
`@@ -1157,13 +1156,12 @@ func buildGraph(nsNodes map[kube.ResourceKey]*Resource) map[kube.ResourceKey]map`
`1157`	`1156`	`continue`
`1158`	`1157`	`}`
`1159`	`1158`	`graphKeyNode, ok := nsNodes[kube.ResourceKey{Group: group.Group, Kind: ownerRef.Kind, Namespace: childNode.Ref.Namespace, Name: ownerRef.Name}]`
`1160`		`- if ok {`
`1161`		`- ownerRef.UID = graphKeyNode.Ref.UID`
`1162`		`- childNode.OwnerRefs[i] = ownerRef`
`1163`		`- } else {`
	`1159`	`+ if !ok {`
`1164`	`1160`	`// No resource found with the given graph key, so move on.`
`1165`	`1161`	`continue`
`1166`	`1162`	`}`
	`1163`	`+ ownerRef.UID = graphKeyNode.Ref.UID`
	`1164`	`+ childNode.OwnerRefs[i] = ownerRef`
`1167`	`1165`	`}`
`1168`	`1166`
`1169`	`1167`	`// Now that we have the UID of the parent, update the graph.`
Original file line number	Diff line number	Diff line change
`@@ -112,23 +112,19 @@ func GetHealthCheckFunc(gvk schema.GroupVersionKind) func(obj *unstructured.Unst`
`112`	`112`	`return getDaemonSetHealth`
`113`	`113`	`}`
`114`	`114`	`case "extensions":`
`115`		`- switch gvk.Kind {`
`116`		`- case kube.IngressKind:`
	`115`	`+ if gvk.Kind == kube.IngressKind {`
`117`	`116`	`return getIngressHealth`
`118`	`117`	`}`
`119`	`118`	`case "argoproj.io":`
`120`		`- switch gvk.Kind {`
`121`		`- case "Workflow":`
	`119`	`+ if gvk.Kind == "Workflow" {`
`122`	`120`	`return getArgoWorkflowHealth`
`123`	`121`	`}`
`124`	`122`	`case "apiregistration.k8s.io":`
`125`		`- switch gvk.Kind {`
`126`		`- case kube.APIServiceKind:`
	`123`	`+ if gvk.Kind == kube.APIServiceKind {`
`127`	`124`	`return getAPIServiceHealth`
`128`	`125`	`}`
`129`	`126`	`case "networking.k8s.io":`
`130`		`- switch gvk.Kind {`
`131`		`- case kube.IngressKind:`
	`127`	`+ if gvk.Kind == kube.IngressKind {`
`132`	`128`	`return getIngressHealth`
`133`	`129`	`}`
`134`	`130`	`case "":`
`@@ -141,13 +137,11 @@ func GetHealthCheckFunc(gvk schema.GroupVersionKind) func(obj *unstructured.Unst`
`141`	`137`	`return getPodHealth`
`142`	`138`	`}`
`143`	`139`	`case "batch":`
`144`		`- switch gvk.Kind {`
`145`		`- case kube.JobKind:`
	`140`	`+ if gvk.Kind == kube.JobKind {`
`146`	`141`	`return getJobHealth`
`147`	`142`	`}`
`148`	`143`	`case "autoscaling":`
`149`		`- switch gvk.Kind {`
`150`		`- case kube.HorizontalPodAutoscalerKind:`
	`144`	`+ if gvk.Kind == kube.HorizontalPodAutoscalerKind {`
`151`	`145`	`return getHPAHealth`
`152`	`146`	`}`
`153`	`147`	`}`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,6 @@ func syncPhases(obj *unstructured.Unstructured) []common.SyncPhase {`
`23`	`23`	`phases = append(phases, phase)`
`24`	`24`	`}`
`25`	`25`	`return phases`
`26`		`- } else {`
`27`		`- return []common.SyncPhase{common.SyncPhaseSync}`
`28`	`26`	`}`
	`27`	`+ return []common.SyncPhase{common.SyncPhaseSync}`
`29`	`28`	`}`
Original file line number	Diff line number	Diff line change
`@@ -29,9 +29,8 @@ type syncTask struct {`
`29`	`29`	`func ternary(val bool, a, b string) string {`
`30`	`30`	`if val {`
`31`	`31`	`return a`
`32`		`- } else {`
`33`		`- return b`
`34`	`32`	`}`
	`33`	`+ return b`
`35`	`34`	`}`
`36`	`35`
`37`	`36`	`func (t *syncTask) String() string {`
`@@ -115,9 +114,8 @@ func (t *syncTask) pruned() bool {`
`115`	`114`	`func (t *syncTask) hookType() common.HookType {`
`116`	`115`	`if t.isHook() {`
`117`	`116`	`return common.HookType(t.phase)`
`118`		`- } else {`
`119`		`- return ""`
`120`	`117`	`}`
	`118`	`+ return ""`
`121`	`119`	`}`
`122`	`120`
`123`	`121`	`func (t *syncTask) hasHookDeletePolicy(policy common.HookDeletePolicy) bool {`