Update linter config and address lint warnings/failures

.golangci.yaml

@@ -1,27 +1,215 @@
 run:
   timeout: 10m
-
-  skip-dirs:
+  allow-parallel-runners: true
+  exclude-dirs:
     - client
-    - injection/client
-    - third_party
+    - injection/clients
+
+output:
+  uniq-by-line: true
+  sort-results: true
+  sort-order:
+  - linter
+  - file
+  show-stats: true
+
+
+issues:
+  max-issues-per-linter: 0
+  max-same-issues: 0
+  exclude-rules:
+  - path: test # Excludes /test, *_test.go etc.
+    linters:
+    - gosec
+    - unparam
+    - noctx
+    - protogetter
+  - linters: ["gocritic"]
+    # Fixes are non-trivial do in a follow up
+    text: "ifElseChain"
+
+linters-settings:
+  # goimports:
+  #   local-prefixes: knative.dev/pkg
+  gomodguard:
+    blocked:
+      modules:
+      - github.com/ghodss/yaml:
+          recommendations:
+            - sigs.k8s.io/yaml
+      - go.uber.org/atomic:
+          recommendations:
+          - sync/atomic
+      - io/ioutil:
+          recommendations:
+          - os
+          - io
+      - github.com/hashicorp/go-multierror:
+          reason: "use errors.Join"
+          recommendations:
+          - errors
+      - go.uber.org/multierr:
+          reason: "use errors.Join"
+          recommendations:
+          - errors
+  revive:
+    rules:
+    # use unparam linter instead - defaults are better
+    - name: unused-parameter
+      disabled: true
 
 linters:
+  disable:
+    - errcheck
   enable:
+    # Check for pass []any as any in variadic func(...any).
+    - asasalint
+
+    # Only use ASCII chars in indentifiers
     - asciicheck
+
+    # Dangerous unicode characters
+    - bidichk
+
+    # Checks whether HTTP response body is closed successfully.
+    - bodyclose
+
+    # Canonicalheader checks whether net/http.Header uses canonical header.
+    - canonicalheader
+
+    # TODO - do a follow up PR
+    # # Containedctx is a linter that detects struct contained context.Context
+    # # field.
+    # - containedctx
+
+    # TODO - do a follow up PR
+    # # Check whether the function uses a non-inherited context.
+    # - contextcheck
+
+    # Copyloopvar is a linter detects places where loop variables are copied.
+    - copyloopvar
+
+    # Check declaration order of types, consts, vars and funcs.
+    - decorder
+
+    # Check for two durations multiplied together.
+    - durationcheck
+
+    # Checks that sentinel errors are prefixed with the Err- and error types
+    # are suffixed with the -Error.
+    - errname
+
+    # Errorlint is a linter for that can be used to find code that will cause
+    # problems with the error wrapping scheme introduced in Go 1.13.
     - errorlint
+
+    # Checks for pointers to enclosing loop variables.
+    - exportloopref
+
+    # Detects nested contexts in loops.
+    - fatcontext
+
+    # Checks that go compiler directive comments (//go:) are valid.
+    - gocheckcompilerdirectives
+
+    # Provides diagnostics that check for bugs, performance and style issues.
+    # Extensible without recompilation through dynamic rules.
+    # Dynamic rules are written declaratively with AST patterns, filters,
+    # report message and optional suggestion.
+    - gocritic
+
+    # Gofmt checks whether code was gofmt-ed. By default this tool runs
+    # with -s option to check for code simplification.
+    - gofmt
+
+    # Gofumpt checks whether code was gofumpt-ed.
+    - gofumpt
+
+     # Check import statements are formatted according to the 'goimport'
+     # command. Reformat imports in autofix mode.
+    - goimports
+
+    # See config below
+    - gomodguard
+
+    # Inspects source code for security problems.
     - gosec
+
+      # Linter that specializes in simplifying code.
+    - gosimple
+    - govet
+
+    # Intrange is a linter to find places where for loops could make use of
+    # an integer range.
+    - intrange
+
+    # Checks key value pairs for common logger libraries (kitlog,klog,logr,zap).
+    - loggercheck
+
+    # Finds slice declarations with non-zero initial length.
+    - makezero
+
+    # Reports wrong mirror patterns of bytes/strings usage
+    - mirror
+
+    # Finds commonly misspelled English words.
+    - misspell
+
+    # Finds the code that returns nil even if it checks that the error is not nil.
+    - nilerr
+
+    # Finds sending HTTP request without context.Context.
+    - noctx
+
+    # Reports ill-formed or insufficient nolint directives.
+    - nolintlint
+
+    # Checks for misuse of Sprintf to construct a host with port in a URL.
+    - nosprintfhostport
+
+    # Checks that fmt.Sprintf can be replaced with a faster alternative.
+    - perfsprint
+
+    # Finds slice declarations that could potentially be pre-allocated.
     - prealloc
+
+    # Reports direct reads from proto message fields when getters should be used.
+    - protogetter
+
+    # Checks that package variables are not reassigned.
+    - reassign
+
+    # Fast, configurable, extensible, flexible, and beautiful linter for
+    # Go. Drop-in replacement of golint.
     - revive
+
+    # Checks for mistakes with OpenTelemetry/Census spans.
+    - spancheck
+
+    # Stylecheck is a replacement for golint.
     - stylecheck
+
+    # Tenv is analyzer that detects using os.Setenv instead of t.Setenv
+    # since Go1.17.
+    - tenv
+
+    # Linter checks if examples are testable (have an expected output).
+    - testableexamples
+
+    # Remove unnecessary type conversions.
     - unconvert
+
+    # Reports unused function parameters and results in your code.
     - unparam
-  disable:
-    - errcheck
 
-issues:
-  exclude-rules:
-    - path: test # Excludes /test, *_test.go etc.
-      linters:
-        - gosec
-        - unparam
+    # A linter that detect the possibility to use variables/constants from the
+    # Go standard library.
+    - usestdlibvars
+
+    # Finds wasted assignment statements.
+    - wastedassign
+
+    # Whitespace is a linter that checks for unnecessary newlines at the start
+    # and end of functions, if, for, etc.
+    - whitespace
+

apiextensions/storageversion/migrator_test.go

@@ -19,7 +19,6 @@ package storageversion
 import (
 	"context"
 	"errors"
-	"fmt"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -109,10 +108,10 @@ func TestMigrate_SingleStoredVersion(t *testing.T) {
 	dclient := dynamicFake.NewSimpleDynamicClient(runtime.NewScheme())
 	cclient := apixFake.NewSimpleClientset(singleVersionFakeCRD)
 
-	cclient.Fake.PrependReactor("patch", "customresourcedefinitions", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
+	cclient.PrependReactor("patch", "customresourcedefinitions", func(action k8stesting.Action) (handled bool, ret runtime.Object, err error) {
 		if pa, ok := action.(k8stesting.PatchAction); ok {
 			if pa.GetName() == singleVersionFakeCRD.Name {
-				return false, nil, fmt.Errorf("resource shouldn't have been patched")
+				return false, nil, errors.New("resource shouldn't have been patched")
 			}
 		}
 
@@ -177,7 +176,7 @@ func TestMigrate_Errors(t *testing.T) {
 						return true, nil, apierrs.NewNotFound(fakeGR, "resource-removed")
 					})
 			},
-			// Resouce not found error should not block the storage migration.
+			// Resource not found error should not block the storage migration.
 			pass: true,
 		},
 		// todo paging fails

apis/condition_set.go

@@ -17,6 +17,7 @@ limitations under the License.
 package apis
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 	"sort"
@@ -233,7 +234,7 @@ func (r conditionsImpl) ClearCondition(t ConditionType) error {
 	}
 	// Terminal conditions are not handled as they can't be nil
 	if r.isTerminal(t) {
-		return fmt.Errorf("clearing terminal conditions not implemented")
+		return errors.New("clearing terminal conditions not implemented")
 	}
 	cond := r.GetCondition(t)
 	if cond == nil {

apis/contexts_test.go

@@ -231,7 +231,7 @@ func TestGetHTTPRequest(t *testing.T) {
 		t.Errorf("GetHTTPRequest() = %v, wanted %v", got, nil)
 	}
 
-	req, err := http.NewRequest("GET", "https://google.com", nil)
+	req, err := http.NewRequest(http.MethodGet, "https://google.com", nil)
 	if err != nil {
 		t.Fatalf("NewRequest() = %v", err)
 	}

apis/deprecated.go

@@ -94,7 +94,7 @@ func getPrefixedNamedFieldValues(prefix string, obj interface{}) (map[string]ref
 		return fields, inlined
 	}
 
-	for i := 0; i < objValue.NumField(); i++ {
+	for i := range objValue.NumField() {
 		tf := objValue.Type().Field(i)
 		if v := objValue.Field(i); v.IsValid() {
 			jTag := tf.Tag.Get("json")

apis/duck/cached_test.go

@@ -72,7 +72,7 @@ func TestSameGVR(t *testing.T) {
 	}
 
 	const iter = 10
-	for i := 0; i < iter; i++ {
+	for range iter {
 		errGrp.Go(func() error {
 			_, _, err := cif.Get(ctx, gvr)
 			retGetCount.Add(1)
@@ -124,7 +124,7 @@ func TestDifferentGVRs(t *testing.T) {
 	errGrp, ctx := errgroup.WithContext(context.Background())
 
 	const iter = 10
-	for i := 0; i < iter; i++ {
+	for i := range iter {
 		// Use a different GVR each iteration to check that calls
 		// to bif.Get can proceed even if a call is in progress
 		// for another GVR.

apis/duck/v1/destination.go

@@ -103,11 +103,11 @@ func (d *Destination) SetDefaults(ctx context.Context) {
 	}
 }
 
-func validateCACerts(CACert *string) *apis.FieldError {
+func validateCACerts(caCert *string) *apis.FieldError {
 	// Check the object.
 	var errs *apis.FieldError
 
-	block, err := pem.Decode([]byte(*CACert))
+	block, err := pem.Decode([]byte(*caCert))
 	if err != nil && block == nil {
 		errs = errs.Also(apis.ErrInvalidValue("CA Cert provided is invalid", "caCert"))
 		return errs

apis/duck/v1/knative_reference.go

@@ -102,7 +102,6 @@ func (kr *KReference) Validate(ctx context.Context) *apis.FieldError {
 					Details: fmt.Sprintf("parent namespace: %q does not match ref: %q", parentNS, kr.Namespace),
 				})
 			}
-
 		}
 	}
 	return errs

apis/duck/v1/status_types.go

@@ -97,7 +97,6 @@ func (s *Status) ConvertTo(ctx context.Context, sink *Status, predicates ...func
 
 	conditions := make(apis.Conditions, 0, len(s.Conditions))
 	for _, c := range s.Conditions {
-
 		// Copy over the "happy" condition, which is the only condition that
 		// we can reliably transfer.
 		if c.Type == apis.ConditionReady || c.Type == apis.ConditionSucceeded {

apis/duck/v1beta1/destination.go

@@ -168,11 +168,11 @@ func validateDestinationRef(ref corev1.ObjectReference) *apis.FieldError {
 	return errs
 }
 
-func validateCACerts(CACert *string) *apis.FieldError {
+func validateCACerts(caCert *string) *apis.FieldError {
 	// Check the object.
 	var errs *apis.FieldError
 
-	block, err := pem.Decode([]byte(*CACert))
+	block, err := pem.Decode([]byte(*caCert))
 	if err != nil && block == nil {
 		errs = errs.Also(apis.ErrInvalidValue("CA Cert provided is invalid", "caCert"))
 		return errs

apis/duck/verify_test.go

@@ -302,6 +302,7 @@ type (
 		Status SliceStatus `json:"status"`
 	}
 )
+
 type SliceStatus struct {
 	Sliceable *Sliceable `json:"sliceable,omitempty"`
 }
@@ -338,6 +339,7 @@ type (
 		Status StringStatus `json:"status"`
 	}
 )
+
 type StringStatus struct {
 	Stringable Stringable `json:"stringable,omitempty"`
 }

apis/field_error_test.go

@@ -227,7 +227,7 @@ Second: X, Y, Z`,
 			}
 
 			for _, p := range []string{"3", "2", "1"} {
-				for i := 0; i < 3; i++ {
+				for range 3 {
 					fe = fe.Also(fe)
 				}
 				fe = fe.ViaField(p)
@@ -247,7 +247,7 @@ Second: X, Y, Z`,
 			for _, p := range []string{"3", "2", "1"} {
 				e := fe.ViaField(p)
 				e.Details = "here at " + p
-				for i := 0; i < 3; i++ {
+				for range 3 {
 					fe = fe.Also(e)
 				}
 			}