chore(golangci-lint): add gocritic linter

viniciussaturnino · viniciussaturnino · commit d9fc4b92c965 · 2022-02-09T20:42:01.000-03:00
diff --git a/.golangci.yml b/.golangci.yml
@@ -18,6 +18,7 @@ linters:
     - exportloopref
     - gci
     - goconst
+    - gocritic
     - gocyclo
     - godot
     - gofmt
diff --git a/api/v1alpha3/azuremachine_types.go b/api/v1alpha3/azuremachine_types.go
@@ -42,7 +42,7 @@ type AzureMachineSpec struct {
 	// as defined in Cluster API. This relates to an Azure Availability Zone
 	FailureDomain *string `json:"failureDomain,omitempty"`
 
-	// DEPRECATED: use FailureDomain instead
+	// Deprecated: use FailureDomain instead
 	AvailabilityZone AvailabilityZone `json:"availabilityZone,omitempty"`
 
 	// Image is used to provide details of an image to use during VM creation.
@@ -79,7 +79,7 @@ type AzureMachineSpec struct {
 	// DataDisk specifies the parameters that are used to add one or more data disks to the machine
 	DataDisks []DataDisk `json:"dataDisks,omitempty"`
 
-	// DEPRECATED: to support old clients, will be removed in v1alpha4/v1beta1
+	// Deprecated: to support old clients, will be removed in v1alpha4/v1beta1
 	Location string `json:"location"`
 
 	SSHPublicKey string `json:"sshPublicKey"`
diff --git a/api/v1alpha3/types.go b/api/v1alpha3/types.go
@@ -72,7 +72,7 @@ type VnetSpec struct {
 	Name string `json:"name"`
 
 	// CidrBlock is the CIDR block to be used when the provider creates a managed virtual network.
-	// DEPRECATED: Use CIDRBlocks instead
+	// Deprecated: Use CIDRBlocks instead
 	// +optional
 	CidrBlock string `json:"cidrBlock,omitempty"`
 
@@ -311,7 +311,7 @@ type AzureSharedGalleryImage struct {
 
 // AvailabilityZone specifies an Azure Availability Zone.
 //
-// DEPRECATED: Use FailureDomain instead.
+// Deprecated: Use FailureDomain instead.
 type AvailabilityZone struct {
 	ID      *string `json:"id,omitempty"`
 	Enabled *bool   `json:"enabled,omitempty"`
@@ -432,7 +432,7 @@ type SubnetSpec struct {
 	Name string `json:"name"`
 
 	// CidrBlock is the CIDR block to be used when the provider creates a managed Vnet.
-	// DEPRECATED: Use CIDRBlocks instead
+	// Deprecated: Use CIDRBlocks instead
 	// +optional
 	CidrBlock string `json:"cidrBlock,omitempty"`
 
diff --git a/api/v1alpha4/types.go b/api/v1alpha4/types.go
@@ -242,7 +242,7 @@ type PublicIPSpec struct {
 }
 
 // VMState describes the state of an Azure virtual machine.
-// DEPRECATED: use ProvisioningState.
+// Deprecated: use ProvisioningState.
 type VMState string
 
 // ProvisioningState describes the provisioning state of an Azure resource.
diff --git a/api/v1beta1/azurecluster_webhook_test.go b/api/v1beta1/azurecluster_webhook_test.go
@@ -33,9 +33,7 @@ func TestAzureCluster_ValidateCreate(t *testing.T) {
 	}{
 		{
 			name: "azurecluster with pre-existing vnet - valid spec",
-			cluster: func() *AzureCluster {
-				return createValidCluster()
-			}(),
+			cluster: createValidCluster(),
 			wantErr: false,
 		},
 		{
@@ -140,9 +138,7 @@ func TestAzureCluster_ValidateUpdate(t *testing.T) {
 		},
 		{
 			name: "azurecluster with no control plane endpoint - valid spec",
-			oldCluster: func() *AzureCluster {
-				return createValidCluster()
-			}(),
+			oldCluster: createValidCluster(),
 			cluster: func() *AzureCluster {
 				cluster := createValidCluster()
 				cluster.Spec.ControlPlaneEndpoint = clusterv1.APIEndpoint{
@@ -155,12 +151,8 @@ func TestAzureCluster_ValidateUpdate(t *testing.T) {
 		},
 		{
 			name: "azurecluster with pre-existing vnet - valid spec",
-			oldCluster: func() *AzureCluster {
-				return createValidCluster()
-			}(),
-			cluster: func() *AzureCluster {
-				return createValidCluster()
-			}(),
+			oldCluster: createValidCluster(),
+			cluster: createValidCluster(),
 			wantErr: false,
 		},
 		{
@@ -179,9 +171,7 @@ func TestAzureCluster_ValidateUpdate(t *testing.T) {
 		},
 		{
 			name: "azurecluster with pre-existing vnet - lack control plane subnet",
-			oldCluster: func() *AzureCluster {
-				return createValidCluster()
-			}(),
+			oldCluster: createValidCluster(),
 			cluster: func() *AzureCluster {
 				cluster := createValidCluster()
 				cluster.Spec.NetworkSpec.Subnets = cluster.Spec.NetworkSpec.Subnets[1:]
@@ -191,9 +181,7 @@ func TestAzureCluster_ValidateUpdate(t *testing.T) {
 		},
 		{
 			name: "azurecluster with pre-existing vnet - lack node subnet",
-			oldCluster: func() *AzureCluster {
-				return createValidCluster()
-			}(),
+			oldCluster: createValidCluster(),
 			cluster: func() *AzureCluster {
 				cluster := createValidCluster()
 				cluster.Spec.NetworkSpec.Subnets = cluster.Spec.NetworkSpec.Subnets[:1]
@@ -203,9 +191,7 @@ func TestAzureCluster_ValidateUpdate(t *testing.T) {
 		},
 		{
 			name: "azurecluster with pre-existing vnet - invalid resourcegroup name",
-			oldCluster: func() *AzureCluster {
-				return createValidCluster()
-			}(),
+			oldCluster: createValidCluster(),
 			cluster: func() *AzureCluster {
 				cluster := createValidCluster()
 				cluster.Spec.NetworkSpec.Vnet.ResourceGroup = "invalid-name###"
@@ -215,9 +201,7 @@ func TestAzureCluster_ValidateUpdate(t *testing.T) {
 		},
 		{
 			name: "azurecluster with pre-existing vnet - invalid subnet name",
-			oldCluster: func() *AzureCluster {
-				return createValidCluster()
-			}(),
+			oldCluster: createValidCluster(),
 			cluster: func() *AzureCluster {
 				cluster := createValidCluster()
 				cluster.Spec.NetworkSpec.Subnets = append(cluster.Spec.NetworkSpec.Subnets,
diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go
@@ -269,7 +269,7 @@ type PublicIPSpec struct {
 }
 
 // VMState describes the state of an Azure virtual machine.
-// DEPRECATED: use ProvisioningState.
+// Deprecated: use ProvisioningState.
 type VMState string
 
 // ProvisioningState describes the provisioning state of an Azure resource.
diff --git a/azure/converters/loadbalancers.go b/azure/converters/loadbalancers.go
@@ -23,8 +23,7 @@ import (
 
 // SKUtoSDK converts infrav1.SKU into a network.LoadBalancerSkuName.
 func SKUtoSDK(src infrav1.SKU) network.LoadBalancerSkuName {
-	switch src {
-	case infrav1.SKUStandard:
+	if src == infrav1.SKUStandard {
 		return network.LoadBalancerSkuNameStandard
 	}
 	return ""
diff --git a/azure/defaults.go b/azure/defaults.go
@@ -317,7 +317,7 @@ func GetDefaultWindowsImage(k8sVersion, runtime string) (*infrav1.Image, error)
 
 	// Starting with 1.22 we default to containerd for Windows unless the runtime flag is set.
 	if v.GTE(v122) && runtime != "dockershim" {
-		skuID = skuID + "-containerd"
+		skuID += "-containerd"
 	}
 
 	defaultImage := &infrav1.Image{
diff --git a/azure/scope/cluster.go b/azure/scope/cluster.go
@@ -656,7 +656,7 @@ func (s *ClusterScope) GenerateFQDN(ipName string) string {
 }
 
 // GenerateLegacyFQDN generates an IP name and a fully qualified domain name, based on a hash, cluster name and cluster location.
-// DEPRECATED: use GenerateFQDN instead.
+// Deprecated: use GenerateFQDN instead.
 func (s *ClusterScope) GenerateLegacyFQDN() (string, string) {
 	h := fnv.New32a()
 	if _, err := h.Write([]byte(fmt.Sprintf("%s/%s/%s", s.SubscriptionID(), s.ResourceGroup(), s.ClusterName()))); err != nil {
@@ -831,20 +831,20 @@ func (s *ClusterScope) SetDNSName() {
 func (s *ClusterScope) getOutboundLBPublicIPSpecs(outboundLB *infrav1.LoadBalancerSpec, generateOutboundIPName func(string) string) []azure.PublicIPSpec {
 	var outboundIPSpecs []azure.PublicIPSpec
 	loadBalancerNodeOutboundIPs := outboundLB.FrontendIPsCount
-	if loadBalancerNodeOutboundIPs == nil || *loadBalancerNodeOutboundIPs == 0 {
+	switch {
+	case loadBalancerNodeOutboundIPs == nil || *loadBalancerNodeOutboundIPs == 0:
 		// do nothing
-	} else if *loadBalancerNodeOutboundIPs == 1 {
+	case *loadBalancerNodeOutboundIPs == 1:
 		outboundIPSpecs = append(outboundIPSpecs, azure.PublicIPSpec{
 			Name: generateOutboundIPName(s.ClusterName()),
 		})
-	} else {
+	default:
 		for i := 0; i < int(*loadBalancerNodeOutboundIPs); i++ {
 			outboundIPSpecs = append(outboundIPSpecs, azure.PublicIPSpec{
 				Name: azure.WithIndex(generateOutboundIPName(s.ClusterName()), i+1),
 			})
 		}
 	}
-
 	return outboundIPSpecs
 }
 
diff --git a/azure/scope/identity.go b/azure/scope/identity.go
@@ -270,7 +270,7 @@ func createAzureIdentityWithBindings(ctx context.Context, azureIdentity *infrav1
 		},
 		Spec: aadpodv1.AzureIdentityBindingSpec{
 			AzureIdentity: copiedIdentity.Name,
-			Selector:      infrav1.AzureIdentityBindingSelector, //should be same as selector added on controller
+			Selector:      infrav1.AzureIdentityBindingSelector, // should be same as selector added on controller
 		},
 	}
 	err = kubeClient.Create(ctx, azureIdentityBinding)
diff --git a/azure/scope/machine.go b/azure/scope/machine.go
@@ -333,7 +333,7 @@ func (m *MachineScope) AvailabilityZone() string {
 	if m.Machine.Spec.FailureDomain != nil {
 		return *m.Machine.Spec.FailureDomain
 	}
-	// DEPRECATED: to support old clients
+	// Deprecated: to support old clients
 	if m.AzureMachine.Spec.FailureDomain != nil {
 		return *m.AzureMachine.Spec.FailureDomain
 	}
diff --git a/azure/scope/machine_test.go b/azure/scope/machine_test.go
@@ -1410,7 +1410,7 @@ func TestMachineScope_NICSpecs(t *testing.T) {
 					ObjectMeta: metav1.ObjectMeta{
 						Name:   "machine",
 						Labels: map[string]string{
-							//clusterv1.MachineControlPlaneLabelName: "true",
+							// clusterv1.MachineControlPlaneLabelName: "true",
 						},
 					},
 				},
@@ -1505,7 +1505,7 @@ func TestMachineScope_NICSpecs(t *testing.T) {
 					ObjectMeta: metav1.ObjectMeta{
 						Name:   "machine",
 						Labels: map[string]string{
-							//clusterv1.MachineControlPlaneLabelName: "true",
+							// clusterv1.MachineControlPlaneLabelName: "true",
 						},
 					},
 				},
@@ -1610,7 +1610,7 @@ func TestMachineScope_NICSpecs(t *testing.T) {
 					ObjectMeta: metav1.ObjectMeta{
 						Name:   "machine",
 						Labels: map[string]string{
-							//clusterv1.MachineControlPlaneLabelName: "true",
+							// clusterv1.MachineControlPlaneLabelName: "true",
 						},
 					},
 				},
@@ -1706,7 +1706,7 @@ func TestMachineScope_NICSpecs(t *testing.T) {
 					ObjectMeta: metav1.ObjectMeta{
 						Name:   "machine",
 						Labels: map[string]string{
-							//clusterv1.MachineControlPlaneLabelName: "true",
+							// clusterv1.MachineControlPlaneLabelName: "true",
 						},
 					},
 				},
diff --git a/azure/scope/strategies/machinepool_deployments/machinepool_deployment_strategy.go b/azure/scope/strategies/machinepool_deployments/machinepool_deployment_strategy.go
@@ -195,7 +195,7 @@ func (rollingUpdateStrategy rollingUpdateStrategy) SelectMachinesToDelete(ctx co
 		return toDelete, nil
 	}
 
-	if len(machinesWithoutLatestModel) <= 0 {
+	if len(machinesWithoutLatestModel) == 0 {
 		log.Info("nothing more to do since all the AzureMachinePoolMachine(s) are the latest model and not over-provisioned")
 		return []infrav1exp.AzureMachinePoolMachine{}, nil
 	}
diff --git a/azure/services/bastionhosts/client.go b/azure/services/bastionhosts/client.go
@@ -149,7 +149,7 @@ func (ac *azureClient) Result(ctx context.Context, future azureautorest.FutureAP
 		if err := json.Unmarshal(jsonData, &createFuture); err != nil {
 			return nil, errors.Wrap(err, "failed to unmarshal future data")
 		}
-		return (*createFuture).Result(ac.bastionhosts)
+		return createFuture.Result(ac.bastionhosts)
 
 	case infrav1.DeleteFuture:
 		// Delete does not return a result bastion host
diff --git a/azure/services/inboundnatrules/client.go b/azure/services/inboundnatrules/client.go
@@ -185,7 +185,7 @@ func (ac *azureClient) Result(ctx context.Context, future azureautorest.FutureAP
 		if err := json.Unmarshal(jsonData, &createFuture); err != nil {
 			return nil, errors.Wrap(err, "failed to unmarshal future data")
 		}
-		return (*createFuture).Result(ac.inboundnatrules)
+		return createFuture.Result(ac.inboundnatrules)
 
 	case infrav1.DeleteFuture:
 		// Delete does not return a result inbound NAT rule
diff --git a/azure/services/loadbalancers/client.go b/azure/services/loadbalancers/client.go
@@ -165,7 +165,7 @@ func (ac *azureClient) Result(ctx context.Context, future azureautorest.FutureAP
 		if err := json.Unmarshal(jsonData, &createFuture); err != nil {
 			return nil, errors.Wrap(err, "failed to unmarshal future data")
 		}
-		return (*createFuture).Result(ac.loadbalancers)
+		return createFuture.Result(ac.loadbalancers)
 
 	case infrav1.DeleteFuture:
 		// Delete does not return a result load balancer
diff --git a/azure/services/natgateways/client.go b/azure/services/natgateways/client.go
@@ -149,7 +149,7 @@ func (ac *azureClient) Result(ctx context.Context, future azureautorest.FutureAP
 		if err := json.Unmarshal(jsonData, &createFuture); err != nil {
 			return nil, errors.Wrap(err, "failed to unmarshal future data")
 		}
-		return (*createFuture).Result(ac.natgateways)
+		return createFuture.Result(ac.natgateways)
 
 	case infrav1.DeleteFuture:
 		// Delete does not return a result NAT gateway
diff --git a/azure/services/networkinterfaces/client.go b/azure/services/networkinterfaces/client.go
@@ -149,7 +149,7 @@ func (ac *AzureClient) Result(ctx context.Context, future azureautorest.FutureAP
 		if err := json.Unmarshal(jsonData, &createFuture); err != nil {
 			return nil, errors.Wrap(err, "failed to unmarshal future data")
 		}
-		return (*createFuture).Result(ac.interfaces)
+		return createFuture.Result(ac.interfaces)
 
 	case infrav1.DeleteFuture:
 		// Delete does not return a result network interface
diff --git a/azure/services/resourceskus/sku.go b/azure/services/resourceskus/sku.go
@@ -54,7 +54,7 @@ const (
 	EphemeralOSDisk = "EphemeralOSDiskSupported"
 	// AcceleratedNetworking identifies the capability for accelerated networking support.
 	AcceleratedNetworking = "AcceleratedNetworkingEnabled"
-	//VCPUs identifies the capability for the number of vCPUS.
+	// VCPUs identifies the capability for the number of vCPUS.
 	VCPUs = "vCPUs"
 	// MemoryGB identifies the capability for memory Size.
 	MemoryGB = "MemoryGB"
diff --git a/azure/services/routetables/client.go b/azure/services/routetables/client.go
@@ -148,7 +148,7 @@ func (ac *azureClient) Result(ctx context.Context, future azureautorest.FutureAP
 		if err := json.Unmarshal(jsonData, &createFuture); err != nil {
 			return nil, errors.Wrap(err, "failed to unmarshal future data")
 		}
-		return (*createFuture).Result(ac.routetables)
+		return createFuture.Result(ac.routetables)
 
 	case infrav1.DeleteFuture:
 		// Delete does not return a result route table.
diff --git a/azure/services/scalesets/scalesets_test.go b/azure/services/scalesets/scalesets_test.go
@@ -744,15 +744,15 @@ func getFakeSkus() []compute.ResourceSku {
 					Location: to.StringPtr("test-location"),
 					Zones:    &[]string{"1", "3"},
 					//ZoneDetails: &[]compute.ResourceSkuZoneDetails{
-					//	{
-					//		Capabilities: &[]compute.ResourceSkuCapabilities{
-					//			{
-					//				Name:  pointer.String("UltraSSDAvailable"),
-					//				Value: pointer.String("True"),
-					//			},
-					//		},
-					//	},
-					//},
+					//    {
+					//        	Capabilities: &[]compute.ResourceSkuCapabilities{
+					//        		{
+					//        			Name:  pointer.String("UltraSSDAvailable"),
+					//        			Value: pointer.String("True"),
+					//        		},
+					//        	},
+					//        },
+					//    },
 				},
 			},
 			Capabilities: &[]compute.ResourceSkuCapabilities{
@@ -837,16 +837,16 @@ func getFakeSkus() []compute.ResourceSku {
 				{
 					Location: to.StringPtr("test-location"),
 					Zones:    &[]string{"1", "3"},
-					//ZoneDetails: &[]compute.ResourceSkuZoneDetails{
-					//	{
-					//		Capabilities: &[]compute.ResourceSkuCapabilities{
-					//			{
-					//				Name:  pointer.String("UltraSSDAvailable"),
-					//				Value: pointer.String("True"),
-					//			},
-					//		},
-					//	},
-					//},
+					//  ZoneDetails: &[]compute.ResourceSkuZoneDetails{
+					//	    {
+					//    		Capabilities: &[]compute.ResourceSkuCapabilities{
+					//    			{
+					//    				Name:  pointer.String("UltraSSDAvailable"),
+					//    				Value: pointer.String("True"),
+					//    			},
+					//    		},
+					//	    },
+					//  },
 				},
 			},
 			Capabilities: &[]compute.ResourceSkuCapabilities{
diff --git a/azure/services/subnets/client.go b/azure/services/subnets/client.go
@@ -149,7 +149,7 @@ func (ac *AzureClient) Result(ctx context.Context, future azureautorest.FutureAP
 		if err := json.Unmarshal(jsonData, &createFuture); err != nil {
 			return nil, errors.Wrap(err, "failed to unmarshal future data")
 		}
-		return (*createFuture).Result(ac.subnets)
+		return createFuture.Result(ac.subnets)
 
 	case infrav1.DeleteFuture:
 		// Delete does not return a result subnet
diff --git a/azure/services/virtualmachines/client.go b/azure/services/virtualmachines/client.go
diff --git a/azure/services/virtualnetworks/client.go b/azure/services/virtualnetworks/client.go
diff --git a/azure/services/vnetpeerings/client.go b/azure/services/vnetpeerings/client.go
diff --git a/exp/controllers/helpers.go b/exp/controllers/helpers.go
diff --git a/util/tele/composite_logger.go b/util/tele/composite_logger.go

Original file line number	Diff line number	Diff line change
`@@ -242,7 +242,7 @@ type PublicIPSpec struct {`
`242`	`242`	`}`
`243`	`243`
`244`	`244`	`// VMState describes the state of an Azure virtual machine.`
`245`		`-// DEPRECATED: use ProvisioningState.`
	`245`	`+// Deprecated: use ProvisioningState.`
`246`	`246`	`type VMState string`
`247`	`247`
`248`	`248`	`// ProvisioningState describes the provisioning state of an Azure resource.`
Original file line number	Diff line number	Diff line change
`@@ -269,7 +269,7 @@ type PublicIPSpec struct {`
`269`	`269`	`}`
`270`	`270`
`271`	`271`	`// VMState describes the state of an Azure virtual machine.`
`272`		`-// DEPRECATED: use ProvisioningState.`
	`272`	`+// Deprecated: use ProvisioningState.`
`273`	`273`	`type VMState string`
`274`	`274`
`275`	`275`	`// ProvisioningState describes the provisioning state of an Azure resource.`
Original file line number	Diff line number	Diff line change
`@@ -23,8 +23,7 @@ import (`
`23`	`23`
`24`	`24`	`// SKUtoSDK converts infrav1.SKU into a network.LoadBalancerSkuName.`
`25`	`25`	`func SKUtoSDK(src infrav1.SKU) network.LoadBalancerSkuName {`
`26`		`- switch src {`
`27`		`- case infrav1.SKUStandard:`
	`26`	`+ if src == infrav1.SKUStandard {`
`28`	`27`	`return network.LoadBalancerSkuNameStandard`
`29`	`28`	`}`
`30`	`29`	`return ""`
Original file line number	Diff line number	Diff line change
`@@ -317,7 +317,7 @@ func GetDefaultWindowsImage(k8sVersion, runtime string) (*infrav1.Image, error)`
`317`	`317`
`318`	`318`	`// Starting with 1.22 we default to containerd for Windows unless the runtime flag is set.`
`319`	`319`	`if v.GTE(v122) && runtime != "dockershim" {`
`320`		`- skuID = skuID + "-containerd"`
	`320`	`+ skuID += "-containerd"`
`321`	`321`	`}`
`322`	`322`
`323`	`323`	`defaultImage := &infrav1.Image{`
Original file line number	Diff line number	Diff line change
`@@ -270,7 +270,7 @@ func createAzureIdentityWithBindings(ctx context.Context, azureIdentity *infrav1`
`270`	`270`	`},`
`271`	`271`	`Spec: aadpodv1.AzureIdentityBindingSpec{`
`272`	`272`	`AzureIdentity: copiedIdentity.Name,`
`273`		`- Selector: infrav1.AzureIdentityBindingSelector, //should be same as selector added on controller`
	`273`	`+ Selector: infrav1.AzureIdentityBindingSelector, // should be same as selector added on controller`
`274`	`274`	`},`
`275`	`275`	`}`
`276`	`276`	`err = kubeClient.Create(ctx, azureIdentityBinding)`