Merge pull request #5653 from sbueringer/pr-remove-nolintifshort

🐛 Improve nolint linter config, fix ifshort flake

Author: k8s-ci-robot

.golangci.yml

@@ -70,6 +70,10 @@ linters-settings:
       # Controller Runtime
       - pkg: sigs.k8s.io/controller-runtime
         alias: ctrl
+  nolintlint:
+    allow-unused: false
+    allow-leading-space: false
+    require-specific: true
   staticcheck:
     go: "1.17"
   stylecheck:
@@ -165,6 +169,12 @@ issues:
     - typecheck
     text: import (".+") is a program, not an importable package
     path: ^tools\.go$
+  # Ignore ifshort false positive
+  # TODO(sbueringer) false positive: https://github.com/esimonov/ifshort/issues/23
+  - linters:
+    - ifshort
+    text:  "variable 'isDeleteNodeAllowed' is only used in the if-statement.*"
+    path: ^controllers/machine_controller\.go$
 
 run:
   timeout: 10m

cmd/clusterctl/client/config/reader_memory.go

@@ -75,7 +75,7 @@ func (f *MemoryReader) Set(key, value string) {
 func (f *MemoryReader) UnmarshalKey(key string, rawval interface{}) error {
 	data, err := f.Get(key)
 	if err != nil {
-		return nil // nolint:nilerr // We expect to not error if the key is not present
+		return nil //nolint:nilerr // We expect to not error if the key is not present
 	}
 	return yaml.Unmarshal([]byte(data), rawval)
 }

cmd/clusterctl/client/repository/repository_versions.go

@@ -42,12 +42,12 @@ func latestContractRelease(repo Repository, contract string) (string, error) {
 	file, err := repo.GetFile(latest, metadataFile)
 	if err != nil {
 		// if we can't get the metadata file from the release, we return latest.
-		return latest, nil // nolint:nilerr
+		return latest, nil //nolint:nilerr
 	}
 	latestMetadata := &clusterctlv1.Metadata{}
 	codecFactory := serializer.NewCodecFactory(scheme.Scheme)
 	if err := runtime.DecodeInto(codecFactory.UniversalDecoder(), file, latestMetadata); err != nil {
-		return latest, nil // nolint:nilerr
+		return latest, nil //nolint:nilerr
 	}
 
 	releaseSeries := latestMetadata.GetReleaseSeriesForContract(contract)
@@ -57,7 +57,7 @@ func latestContractRelease(repo Repository, contract string) (string, error) {
 
 	sv, err := version.ParseSemantic(latest)
 	if err != nil {
-		return latest, nil // nolint:nilerr
+		return latest, nil //nolint:nilerr
 	}
 
 	// If the Major or Minor version of the latest release doesn't match the release series for the current contract,

cmd/clusterctl/cmd/version_checker.go

@@ -145,7 +145,7 @@ func (v *versionChecker) getLatestRelease() (*ReleaseInfo, error) {
 			log.V(1).Info("⚠️ Unable to get latest github release for clusterctl")
 			// failing silently here so we don't error out in air-gapped
 			// environments.
-			return nil, nil // nolint:nilerr
+			return nil, nil //nolint:nilerr
 		}
 
 		vs = &VersionState{

cmd/clusterctl/internal/test/fake_proxy.go

@@ -94,7 +94,7 @@ func (f *FakeProxy) CheckClusterAvailable() error {
 
 // ListResources returns all the resources known by the FakeProxy.
 func (f *FakeProxy) ListResources(labels map[string]string, namespaces ...string) ([]unstructured.Unstructured, error) {
-	var ret []unstructured.Unstructured //nolint
+	var ret []unstructured.Unstructured //nolint:prealloc
 	for _, o := range f.objs {
 		u := unstructured.Unstructured{}
 		err := FakeScheme.Convert(o, &u, nil)

cmd/clusterctl/internal/test/fake_reader.go

@@ -73,7 +73,7 @@ func (f *FakeReader) Set(key, value string) {
 func (f *FakeReader) UnmarshalKey(key string, rawval interface{}) error {
 	data, err := f.Get(key)
 	if err != nil {
-		return nil // nolint:nilerr // We expect to not error if the key is not present
+		return nil //nolint:nilerr // We expect to not error if the key is not present
 	}
 	return yaml.Unmarshal([]byte(data), rawval)
 }

controllers/cluster_controller.go

@@ -430,7 +430,7 @@ func (c clusterDescendants) filterOwnedDescendants(cluster *clusterv1.Cluster) (
 		obj := o.(client.Object)
 		acc, err := meta.Accessor(obj)
 		if err != nil {
-			return nil // nolint:nilerr // We don't want to exit the EachListItem loop, just continue
+			return nil //nolint:nilerr // We don't want to exit the EachListItem loop, just continue
 		}
 
 		if util.IsOwnedByObject(acc, cluster) {

controllers/machine_controller.go

@@ -277,7 +277,7 @@ func (r *MachineReconciler) reconcileDelete(ctx context.Context, cluster *cluste
 	log := ctrl.LoggerFrom(ctx, "cluster", cluster.Name)
 
 	err := r.isDeleteNodeAllowed(ctx, cluster, m)
-	isDeleteNodeAllowed := err == nil //nolint:ifshort
+	isDeleteNodeAllowed := err == nil
 	if err != nil {
 		switch err {
 		case errNoControlPlaneNodes, errLastControlPlaneNode, errNilNodeRef, errClusterIsBeingDeleted, errControlPlaneIsBeingDeleted:

controllers/machine_controller_phases.go

@@ -46,7 +46,7 @@ var (
 )
 
 func (r *MachineReconciler) reconcilePhase(_ context.Context, m *clusterv1.Machine) {
-	originalPhase := m.Status.Phase // nolint:ifshort
+	originalPhase := m.Status.Phase //nolint:ifshort // Cannot be inlined because m.Status.Phase might be changed before it is used in the if.
 
 	// Set the phase to "pending" if nil.
 	if m.Status.Phase == "" {

controllers/suite_test.go

@@ -152,7 +152,7 @@ func (matcher *refGroupKindMatcher) Match(actual interface{}) (success bool, err
 	for _, ref := range ownerRefs {
 		gv, err := schema.ParseGroupVersion(ref.APIVersion)
 		if err != nil {
-			return false, nil // nolint:nilerr // If we can't get the group version we can't match, but it's not a failure
+			return false, nil //nolint:nilerr // If we can't get the group version we can't match, but it's not a failure
 		}
 		if ref.Kind == matcher.kind && gv.Group == clusterv1.GroupVersion.Group {
 			return true, nil