lint: use crossplane golangci-lint config, fix issues

Signed-off-by: Jared Watts <[email protected]>

Author: jbw976

.golangci.yml

@@ -1,55 +1,121 @@
+# copied from https://github.com/crossplane/crossplane/blob/main/.golangci.yml and
+# updated slightly for this repository.
 version: "2"
+
 output:
   formats:
     text:
-      path: stdout
+      path: stderr
+
 linters:
-  enable:
-    - asasalint
-    - asciicheck
-    - bidichk
-    - bodyclose
-    - contextcheck
-    - durationcheck
-    - errchkjson
-    - errorlint
-    - exhaustive
-    - gocheckcompilerdirectives
-    - gochecksumtype
-    - goconst
-    - gocritic
+  default: all
+  disable:
+    # These are linters we'd like to enable, but that will be labor intensive to
+    # make existing code compliant.
+    - wrapcheck
+    - varnamelen
+    - testpackage
+    - paralleltest
+    - nilnil
+    - funcorder
+
+    # Below are linters that lint for things we don't value. Each entry below
+    # this line must have a comment explaining the rationale.
+
+    # These linters add whitespace in an attempt to make code more readable.
+    # This isn't a widely accepted Go best practice, and would be laborious to
+    # apply to existing code.
+    - wsl
+    - wsl_v5
+    - nlreturn
+
+    # Warns about uses of fmt.Sprintf that are less performant than alternatives
+    # such as string concatenation. We value readability more than performance
+    # unless performance is measured to be an issue.
+    - perfsprint
+
+    # This linter:
+    #
+    # 1. Requires errors.Is/errors.As to test equality.
+    # 2. Requires all errors be wrapped with fmt.Errorf specifically.
+    # 3. Disallows errors.New inline - requires package level errors.
+    #
+    # 1 is covered by other linters. 2 is covered by wrapcheck, which can also
+    # handle our use of crossplane-runtime's errors package. 3 is more strict
+    # than we need. Not every error needs to be tested for equality.
+    - err113
+
+    # These linters duplicate gocognit, but calculate complexity differently.
     - gocyclo
-    - gosec
-    - gosmopolitan
-    - loggercheck
-    - makezero
-    - misspell
-    - musttag
-    - nakedret
-    - nilerr
-    - nilnesserr
-    - noctx
-    - nolintlint
-    - prealloc
-    - protogetter
-    - reassign
-    - recvcheck
-    - revive
-    - rowserrcheck
-    - spancheck
-    - sqlclosecheck
-    - testifylint
-    - unconvert
-    - unparam
-    - zerologlint
+    - cyclop
+    - nestif
+    - funlen
+    - maintidx
+
+    # Enforces max line length. It's not idiomatic to enforce a strict limit on
+    # line length in Go. We'd prefer to lint for things that often cause long
+    # lines, like functions with too many parameters or long parameter names
+    # that duplicate their types.
+    - lll
+
+    # Warns about struct instantiations that don't specify every field. Could be
+    # useful in theory to catch fields that are accidentally omitted. Seems like
+    # it would have many more false positives than useful catches, though.
+    - exhaustruct
+
+    # Warns about TODO comments. The rationale being they should be issues
+    # instead. We're okay with using TODO to track minor cleanups for next time
+    # we touch a particular file.
+    - godox
+
+    # Warns about duplicated code blocks within the same file. Could be useful
+    # to prompt folks to think about whether code should be broken out into a
+    # function, but generally we're less worried about DRY and fine with a
+    # little copying. We don't want to give folks the impression that we require
+    # every duplicated code block to be factored out into a function.
+    - dupl
+
+    # Warns about returning interfaces rather than concrete types. We do think
+    # it's best to avoid returning interfaces where possible. However, at the
+    # time of writing enabling this linter would only catch the (many) cases
+    # where we must return an interface.
+    - ireturn
+
+    # Warns about returning named variables. We do think it's best to avoid
+    # returning named variables where possible. However, at the time of writing
+    # enabling this linter would only catch the (many) cases where returning
+    # named variables is useful to document what the variables are. For example
+    # we believe it makes sense to return (ready bool) rather than just (bool)
+    # to communicate what the bool means.
+    - nonamedreturns
+
+    # Warns about using magic numbers. We do think it's best to avoid magic
+    # numbers, but we should not be strict about it.
+    - mnd
+
+    # Warns about if err := Foo(); err != nil style error checks. Seems to go
+    # against idiomatic Go programming, which encourages this approach - e.g.
+    # to scope errors.
+    - noinlineerr
   settings:
+    depguard:
+      rules:
+        no_third_party_test_libraries:
+          list-mode: lax
+          files:
+            - $test
+          deny:
+            - pkg: github.com/stretchr/testify
+              desc: See https://go.dev/wiki/TestComments#assert-libraries
+            - pkg: github.com/onsi/ginkgo
+              desc: See https://go.dev/wiki/TestComments#assert-libraries
+            - pkg: github.com/onsi/gomega
+              desc: See https://go.dev/wiki/TestComments#assert-libraries
     dupl:
       threshold: 100
     errcheck:
       check-type-assertions: false
       check-blank: false
-      exclude-functions:
-        - io/ioutil.ReadFile
     goconst:
       min-len: 3
       min-occurrences: 5
@@ -61,11 +127,11 @@ linters:
           paramsOnly: true
         rangeValCopy:
           sizeThreshold: 32
-    gocyclo:
-      min-complexity: 10
     govet:
       disable:
         - shadow
+    interfacebloat:
+      max: 5
     lll:
       tab-width: 1
     nakedret:
@@ -77,60 +143,96 @@ linters:
       simple: true
       range-loops: true
       for-loops: false
+    tagliatelle:
+      case:
+        rules:
+          json: goCamel
     unparam:
       check-exported: false
+    unused:
+      exported-fields-are-used: true
   exclusions:
     generated: lax
     rules:
       - linters:
-          - all
-        path: zz_generated\..+\.go$
-      - linters:
-          - dupl
+          - containedctx
           - errcheck
-          - gocyclo
+          - forcetypeassert
+          - gochecknoglobals
+          - gochecknoinits
+          - gocognit
           - gosec
           - scopelint
           - unparam
+          - embeddedstructfieldcheck
         path: _test(ing)?\.go
+
       - linters:
           - gocritic
         path: _test\.go
         text: (unnamedResult|exitAfterDefer)
+
+      # These are performance optimisations rather than style issues per se.
+      # They warn when function arguments or range values copy a lot of memory
+      # rather than using a pointer.
       - linters:
           - gocritic
         text: '(hugeParam|rangeValCopy):'
+
+      # This "TestMain should call os.Exit to set exit code" warning is not clever
+      # enough to notice that we call a helper method that calls os.Exit.
       - linters:
           - staticcheck
         text: 'SA3000:'
+
+      # This is a "potential hardcoded credentials" warning. It's triggered by
+      # any variable with 'secret' in the same, and thus hits a lot of false
+      # positives in Kubernetes land where a Secret is an object type.
       - linters:
           - gosec
         text: 'G101:'
+
+      # This is an 'errors unhandled' warning that duplicates errcheck.
       - linters:
           - gosec
         text: 'G104:'
+
+      # This is about implicit memory aliasing in a range loop.
+      # This is a false positive with Go v1.22 and above.
+      - linters:
+          - gosec
+        text: 'G601:'
+
+      # Some k8s dependencies do not have JSON tags on all fields in structs.
       - linters:
           - musttag
         path: k8s.io/
+
     paths:
+      - zz_generated\..+\.go$
+      - .+\.pb.go$
       - third_party$
       - builtin$
-      - examples$
+      - example$
+
 issues:
   max-issues-per-linter: 0
   max-same-issues: 0
   new: false
+
 formatters:
   enable:
     - gci
     - gofmt
+    - gofumpt
+    - goimports
   settings:
     gci:
       sections:
         - standard
         - default
-        - prefix(github.com/crossplane)
-        - prefix(github.com/crossplane-contrib)
+        - prefix(github.com/crossplane/function-sdk-go)
+        - prefix(github.com/crossplane/function-template-go)
         - blank
         - dot
       custom-order: true
@@ -139,6 +241,8 @@ formatters:
   exclusions:
     generated: lax
     paths:
+      - zz_generated\..+\.go$
+      - .+\.pb.go$
       - third_party$
       - builtin$
-      - examples$
+      - example$

fn.go

@@ -8,6 +8,7 @@ import (
 	fnv1 "github.com/crossplane/function-sdk-go/proto/v1"
 	"github.com/crossplane/function-sdk-go/request"
 	"github.com/crossplane/function-sdk-go/response"
+
 	"github.com/crossplane/function-template-go/input/v1beta1"
 )
 

fn_test.go

@@ -16,7 +16,6 @@ import (
 )
 
 func TestRunFunction(t *testing.T) {
-
 	type args struct {
 		ctx context.Context
 		req *fnv1.RunFunctionRequest

main.go

@@ -9,13 +9,13 @@ import (
 
 // CLI of this Function.
 type CLI struct {
-	Debug bool `short:"d" help:"Emit debug logs in addition to info logs."`
+	Debug bool `help:"Emit debug logs in addition to info logs." short:"d"`
 
-	Network            string `help:"Network on which to listen for gRPC connections." default:"tcp"`
-	Address            string `help:"Address at which to listen for gRPC connections." default:":9443"`
-	TLSCertsDir        string `help:"Directory containing server certs (tls.key, tls.crt) and the CA used to verify client certificates (ca.crt)" env:"TLS_SERVER_CERTS_DIR"`
+	Network            string `default:"tcp"                                                                                        help:"Network on which to listen for gRPC connections."`
+	Address            string `default:":9443"                                                                                      help:"Address at which to listen for gRPC connections."`
+	TLSCertsDir        string `env:"TLS_SERVER_CERTS_DIR"                                                                           help:"Directory containing server certs (tls.key, tls.crt) and the CA used to verify client certificates (ca.crt)"`
 	Insecure           bool   `help:"Run without mTLS credentials. If you supply this flag --tls-server-certs-dir will be ignored."`
-	MaxRecvMessageSize int    `help:"Maximum size of received messages in MB." default:"4"`
+	MaxRecvMessageSize int    `default:"4"                                                                                          help:"Maximum size of received messages in MB."`
 }
 
 // Run this Function.