Fix all linter warnings

.golangci.yaml

@@ -1,74 +1,219 @@
 run:
   timeout: 10m
-
+  allow-parallel-runners: true
+  exclude-dirs:
+  - pkg/client
   build-tags:
-    - e2e
-    - upgrade
+  - e2e
+  - hpa
+  - upgrade
+
+output:
+  sort-results: true
+  sort-order:
+  - linter
+  - file
+  show-stats: true
 
-  skip-dirs:
-    - pkg/client
 
-  skip-files:
-    - ".pb.go$"
+issues:
+  uniq-by-line: true
+  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:
+  gocritic:
+    disabled-checks:
+    # we typically have this pattern during setup
+    - exitAfterDefer
+    # a lot cases we have are legit
+    - appendAssign
+  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
-    - depguard
+
+    # 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
+
+    # 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
-    - importas
+
+      # 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
-    - tparallel
+
+    # 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
 
-linters-settings:
-  depguard:
-    rules:
-      main:
-        deny:
-          - pkg: sync/atomic
-            desc: "please use type-safe atomics from go.uber.org/atomic"
-  importas:
-    no-unaliased: true
-    alias:
-      - pkg: knative.dev/serving/pkg/apis/autoscaling/v1alpha1
-        alias: autoscalingv1alpha1
+    # A linter that detect the possibility to use variables/constants from the
+    # Go standard library.
+    - usestdlibvars
 
-issues:
-  include:
-    # Disable excluding issues about comments from golint.
-    - EXC0002
+    # 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
 
-  exclude-rules:
-    - path: test # Excludes /test, *_test.go etc.
-      linters:
-        - gosec
-        - unparam
-
-    # Ignore "context.Context should be the first parameter of a function" errors in tests.
-    # See: https://github.com/golang/lint/issues/422
-    - path: test
-      text: "context.Context should be the first"
-      linters:
-        - revive
-
-    # Allow source and sink receivers in conversion code for clarity.
-    - path: _conversion\.go
-      text: "ST1016:"
-      linters:
-        - stylecheck
-    - path: _conversion\.go
-      text: "receiver name"
-      linters:
-        - revive
-
-    # This check has quite a few false positives where there isn't much value in the package comment.
-    - text: "ST1000: at least one file in a package should have a package comment"
-      linters:
-        - stylecheck

cmd/activator/main.go

@@ -214,11 +214,11 @@ func main() {
 	ah := activatorhandler.New(ctx, throttler, transport, networkConfig.EnableMeshPodAddressability, logger, tlsEnabled)
 	ah = handler.NewTimeoutHandler(ah, "activator request timeout", func(r *http.Request) (time.Duration, time.Duration, time.Duration) {
 		if rev := activatorhandler.RevisionFrom(r.Context()); rev != nil {
-			var responseStartTimeout = 0 * time.Second
+			responseStartTimeout := 0 * time.Second
 			if rev.Spec.ResponseStartTimeoutSeconds != nil {
 				responseStartTimeout = time.Duration(*rev.Spec.ResponseStartTimeoutSeconds) * time.Second
 			}
-			var idleTimeout = 0 * time.Second
+			idleTimeout := 0 * time.Second
 			if rev.Spec.IdleTimeoutSeconds != nil {
 				idleTimeout = time.Duration(*rev.Spec.IdleTimeoutSeconds) * time.Second
 			}

cmd/autoscaler/leaderelection.go

@@ -66,7 +66,6 @@ func setupSharedElector(ctx context.Context, controllers []*controller.Impl) (le
 	noopEnqueue := func(reconciler.Bucket, types.NamespacedName) {}
 
 	el, err := leaderelection.BuildElector(ctx, coalesce(reconcilers), queueName, noopEnqueue)
-
 	if err != nil {
 		return nil, err
 	}

cmd/autoscaler/main.go

@@ -231,7 +231,8 @@ func main() {
 }
 
 func uniScalerFactoryFunc(podLister corev1listers.PodLister,
-	metricClient asmetrics.MetricClient) scaling.UniScalerFactory {
+	metricClient asmetrics.MetricClient,
+) scaling.UniScalerFactory {
 	return func(decider *scaling.Decider) (scaling.UniScaler, error) {
 		configName := decider.Labels[serving.ConfigurationLabelKey]
 		if configName == "" {

cmd/default-domain/main.go

@@ -218,7 +218,7 @@ func main() {
 		}
 		ipAddr, err := net.ResolveIPAddr("ip", address.Hostname)
 		if err != nil {
-			logger.Fatalw("Error resolving the IP address of %q", address.Hostname, zap.Error(err))
+			logger.Fatalw(fmt.Sprintf("Error resolving the IP address of %q", address.Hostname), zap.Error(err))
 		}
 		ip = ipAddr.String()
 	}

cmd/schema-tweak/main.go

@@ -44,7 +44,6 @@ func main() {
 
 		processFile(file)
 	}
-
 }
 
 func processFile(file fs.DirEntry) {
@@ -81,7 +80,6 @@ func processFile(file fs.DirEntry) {
 	if err = os.WriteFile(filename, buf.Bytes(), file.Type().Perm()); err != nil {
 		log.Fatalln("failed to write CRD", err)
 	}
-
 }
 
 func applyOverrides(root *yaml.Node) {
@@ -149,7 +147,7 @@ func updateFeatureFlags(node *yaml.Node, features []flagField) {
 }
 
 func updateFeatureFlagProperty(root *yaml.Node, f flagField) {
-	desc := fmt.Sprintf("This is accessible behind a feature flag - %s", f.flag)
+	desc := "This is accessible behind a feature flag - " + f.flag
 
 	setString(root, "description", desc)
 
@@ -184,7 +182,6 @@ func updateFeatureFlagProperty(root *yaml.Node, f flagField) {
 			Value: "true",
 		},
 	)
-
 }
 
 func filterAllowedFields(node *yaml.Node, allowed sets.Set[string], features []flagField) {
@@ -216,8 +213,7 @@ func filterAllowedFields(node *yaml.Node, allowed sets.Set[string], features []f
 func dropRequiredFields(node *yaml.Node, fields sets.Set[string]) {
 	dataType := dataType(node)
 
-	switch dataType {
-	case "array":
+	if dataType == "array" {
 		node = items(node)
 	}
 

cmd/schema-tweak/overrides.go

@@ -153,9 +153,8 @@ func revSpecOverrides(prefixPath string) []entry {
 			"drop",
 		),
 	}, {
-		path: "containers.securityContext.capabilities.add",
-		description: fmt.Sprintf("This is accessible behind a feature flag - %s",
-			config.FeatureContainerSpecAddCapabilities),
+		path:        "containers.securityContext.capabilities.add",
+		description: "This is accessible behind a feature flag - " + config.FeatureContainerSpecAddCapabilities,
 	}, {
 		path: "containers.resources",
 		allowedFields: sets.New(
@@ -345,7 +344,7 @@ func revSpecOverrides(prefixPath string) []entry {
 
 	for _, probe := range probes {
 		entries = append(entries, entry{
-			path: fmt.Sprintf("containers.%s", probe),
+			path: "containers." + probe,
 			allowedFields: sets.New(
 				"initialDelaySeconds",
 				"timeoutSeconds",
@@ -406,11 +405,11 @@ func revSpecOverrides(prefixPath string) []entry {
 
 func revisionSpecFields() []string {
 	var (
-		fields  []string
 		revType = reflect.TypeOf(v1.RevisionSpec{})
+		fields  = make([]string, 0, revType.NumField())
 	)
 
-	for i := 0; i < revType.NumField(); i++ {
+	for i := range revType.NumField() {
 		if revType.Field(i).Name == "PodSpec" {
 			continue
 		}

cmd/schema-tweak/yaml.go

@@ -150,5 +150,4 @@ func deleteKeysExcluding(node *yaml.Node, keys ...string) {
 			i -= 2 // reset index
 		}
 	}
-
 }