linter fixes (#1034)

* fix whitespace

* use stdlib constants

* run golangci-lint run --fix

* address remaining linter changes

* run ./hack/update-deps.sh

.golangci.yaml

@@ -1,29 +1,211 @@
 run:
-  timeout: 5m
-
+  timeout: 10m
+  allow-parallel-runners: true
+  exclude-dirs:
+  - pkg/client
   build-tags:
-    - e2e
+  - e2e
+
+output:
+  sort-results: true
+  sort-order:
+  - linter
+  - file
+  show-stats: true
+
+
+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"
 
-  skip-dirs:
-    - pkg/client
+linters-settings:
+  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
+
+    # 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
-    - 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
 
-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
+

go.mod

@@ -7,7 +7,6 @@ require (
 	github.com/google/go-cmp v0.6.0
 	github.com/gorilla/websocket v1.5.1
 	github.com/rs/dnscache v0.0.0-20230804202142-fc85eb664529
-	go.uber.org/atomic v1.9.0
 	go.uber.org/zap v1.27.0
 	golang.org/x/sync v0.10.0
 	golang.org/x/time v0.6.0

go.sum

@@ -330,8 +330,6 @@ go.opentelemetry.io/otel/sdk/metric v1.31.0/go.mod h1:CRInTMVvNhUKgSAMbKyTMxqOBC
 go.opentelemetry.io/otel/trace v1.31.0 h1:ffjsj1aRouKewfr85U2aGagJ46+MvodynlQ1HYdmJys=
 go.opentelemetry.io/otel/trace v1.31.0/go.mod h1:TXZkRk7SM2ZQLtR6eoAWQFIHPvzQ06FJAsO1tJg480A=
 go.uber.org/atomic v1.4.0/go.mod h1:gD2HeocX3+yG+ygLZcrzQJaqmWj9AIm7n08wl/qW/PE=
-go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE=
-go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
 go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=
 go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE=
 go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/0=

pkg/apis/config/store_test.go

@@ -47,7 +47,6 @@ func TestStoreLoadWithContext(t *testing.T) {
 			t.Error("Unexpected defaults config (-want, +got):", diff)
 		}
 	})
-
 }
 
 func TestStoreLoadWithContextOrDefaults(t *testing.T) {

pkg/apis/networking/metadata_validation.go

@@ -24,19 +24,17 @@ import (
 	"knative.dev/pkg/apis"
 )
 
-var (
-	allowedAnnotations = sets.New[string](
-		IngressClassAnnotationKey,
-		CertificateClassAnnotationKey,
-		DisableAutoTLSAnnotationKey,
-		DisableExternalDomainTLSAnnotationKey,
-		HTTPOptionAnnotationKey,
-
-		IngressClassAnnotationAltKey,
-		CertificateClassAnnotationAltKey,
-		DisableAutoTLSAnnotationAltKey,
-		HTTPProtocolAnnotationKey,
-	)
+var allowedAnnotations = sets.New[string](
+	IngressClassAnnotationKey,
+	CertificateClassAnnotationKey,
+	DisableAutoTLSAnnotationKey,
+	DisableExternalDomainTLSAnnotationKey,
+	HTTPOptionAnnotationKey,
+
+	IngressClassAnnotationAltKey,
+	CertificateClassAnnotationAltKey,
+	DisableAutoTLSAnnotationAltKey,
+	HTTPProtocolAnnotationKey,
 )
 
 // ValidateAnnotations validates that `annotations` in `metadata` stanza of the

pkg/apis/networking/ports.go

@@ -40,10 +40,8 @@ const (
 	ServicePortNameHTTPS = "https"
 )
 
-var (
-	// AppProtocolH2C is the name of the external port of the service for HTTP/2, from https://github.com/kubernetes/enhancements/tree/master/keps/sig-network/3726-standard-application-protocols#new-standard-protocols
-	AppProtocolH2C = "kubernetes.io/h2c"
-)
+// AppProtocolH2C is the name of the external port of the service for HTTP/2, from https://github.com/kubernetes/enhancements/tree/master/keps/sig-network/3726-standard-application-protocols#new-standard-protocols
+var AppProtocolH2C = "kubernetes.io/h2c"
 
 // ServicePortName returns the port for the app level protocol.
 func ServicePortName(proto ProtocolType) string {

pkg/apis/networking/v1alpha1/certificate_validation_test.go

@@ -95,6 +95,7 @@ func TestCertificateSpecValidation(t *testing.T) {
 		})
 	}
 }
+
 func TestCertificateValidation(t *testing.T) {
 	tests := []struct {
 		name string

pkg/apis/networking/v1alpha1/domainclaim_types.go

@@ -39,10 +39,8 @@ type ClusterDomainClaim struct {
 	Spec ClusterDomainClaimSpec `json:"spec,omitempty"`
 }
 
-var (
-	// Check that we can create OwnerReferences to a ClusterDomainClaim.
-	_ kmeta.OwnerRefable = (*ClusterDomainClaim)(nil)
-)
+// Check that we can create OwnerReferences to a ClusterDomainClaim.
+var _ kmeta.OwnerRefable = (*ClusterDomainClaim)(nil)
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 

pkg/apis/networking/v1alpha1/ingress_defaults_test.go

@@ -147,5 +147,4 @@ func TestIngressDefaulting(t *testing.T) {
 			}
 		})
 	}
-
 }

pkg/apis/networking/v1alpha1/ingress_helpers.go

@@ -27,13 +27,13 @@ import (
 func (i *Ingress) GetIngressTLSForVisibility(visibility IngressVisibility) []IngressTLS {
 	ingressTLS := make([]IngressTLS, 0, len(i.Spec.TLS))
 
-	if i.Spec.TLS == nil || len(i.Spec.TLS) == 0 {
+	if len(i.Spec.TLS) == 0 {
 		return ingressTLS
 	}
 
 	for _, rule := range i.Spec.Rules {
 		if rule.Visibility == visibility {
-			if rule.Hosts == nil || len(rule.Hosts) == 0 {
+			if len(rule.Hosts) == 0 {
 				return ingressTLS
 			}
 

pkg/apis/networking/v1alpha1/ingress_helpers_test.go

@@ -22,9 +22,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 )
 
-var (
-	hosts = []string{"foo", "bar", "foo.bar"}
-)
+var hosts = []string{"foo", "bar", "foo.bar"}
 
 func TestGetIngressTLSForVisibility(t *testing.T) {
 	tests := []struct {

pkg/certificates/constants.go

@@ -21,10 +21,10 @@ import "strings"
 const (
 	Organization = "knative.dev"
 
-	// nolint:all
+	//nolint:all
 	LegacyFakeDnsName = "data-plane." + Organization
 
-	// nolint:all
+	//nolint:all
 	// Deprecated: FakeDnsName is deprecated.
 	// Please use the DataPlaneRoutingSAN for calls to the Activator
 	// and the DataPlaneUserSAN function for calls to a Knative-Service via Queue-Proxy.