Initial pass of linter updates

.golangci.yaml

@@ -1,110 +1,70 @@
-# Reference: https://github.com/golangci/golangci-lint/blob/master/.golangci.reference.yml
+# Reference: https://golangci-lint.run/usage/configuration/
 run:
-  # timeout for analysis, e.g. 30s, 5m, default is 1m
-  deadline: 20m
-  timeout: 20m
+  # timeout for analysis, e.g. 30s, 5m. Default: 1m
+  timeout: 5m
 
 linters:
+  # Since a specific version of golangci-lint is used in CI, this approach of
+  # enabling all, but disabling specific linters is reasonable stable.
+  # But note that when the version is updated, new linters might be added which
+  # need to be disabled, or specially configured.
   enable-all: true
   disable:
-    - bodyclose
     - copyloopvar # prefer old style for now, more reliable across various scanners
-    - cyclop
-    - deadcode #deprecated
-    - depguard
-    - dupl
-    - funlen
-    - exhaustruct
-    - exhaustivestruct
-    - forcetypeassert
-    - gci # disable until it's more configurable
-    - gochecknoglobals
-    - gochecknoinits
-    - gocognit
-    - goconst
-    - gocyclo
-    - godot
-    - err113
-    - golint # replaced by revive
-    - gomnd
-    - gomoddirectives
-    - gosec
-    - ifshort
-    - interfacer
-    - intrange # prefer old style
-    - ireturn # New linter to consider
-    - maligned
-    - maintidx # New linter to consider
-    - nakedret
-    - nestif
-    - nilnil # New linter to consider
-    - nonamedreturns # New linter to consider
-    - paralleltest
-    - prealloc
-    - revive
-    - scopelint
-    - structcheck #deprecated
-    - stylecheck
-    - testpackage
-    - varcheck #deprecated
-    - varnamelen # New linter to consider
-    - wrapcheck
-  fast: false
+  #   - cyclop             -------- complexity
+    - depguard # no deps we need to guard against
+    - exhaustruct # too exhausting in many cases
+  #   - funlen             -------- complexity
+  #   - gci             -------- might need configuration
+    - gochecknoinits # inits can be good
+    - gochecksumtype # requires using another tool to define sumtypes
+  #   - gocognit             -------- complexity
+  #   - gocyclo             -------- complexity
+  #   - goheader             -------- might need configuration
+    - gomoddirectives # some directives are important
+    - gomodguard # no mods we need to guard against
+    - interfacebloat # a job for a reviwer, not a linter
+    - intrange # Too new of a language feature
+  #   - maintidx             -------- complexity
+  #   - nakedret             -------- config to be zero?
+  #   - nestif             -------- complexity
+  #   - nlreturn             -------- overlap with wsl?
+    - nonamedreturns # named returns can be good
+  #   - revive             -------- might need config
+  #   - stylecheck             -------- might need config
+    - testifylint # no current use of that package
+    - testpackage # the _test package is only sometimes helpful
+    - wrapcheck # always wrapping is overkill
+  #   - wsl             -------- might need config
+    - deadcode # Deprecated
+    - exhaustivestruct # Deprecated
+    - execinquery # Deprecated
+    - golint # Deprecated
+    - ifshort # Deprecated
+    - interfacer # Deprecated
+    - maligned # Deprecated
+    - gomnd # Deprecated
+    - nosnakecase # Deprecated
+    - scopelint # Deprecated
+    - structcheck # Deprecated
+    - varcheck # Deprecated
 
 linters-settings:
   errcheck:
-    # report about not checking of errors in type assetions: `a := b.(MyStruct)`;
-    # default is false: such cases aren't reported by default.
-    check-type-assertions: false
-
-    # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
-    # default is false: such cases aren't reported by default.
-    check-blank: false
-  govet:
-    # report about shadowed variables
-    check-shadowing: false
+    # Report about not checking of errors in type assertions: `a := b.(MyStruct)`.
+    # Such cases aren't reported by default.
+    check-type-assertions: true
+  funlen:
+    # Checks the number of lines in a function.
+    # If lower than 0, disable the check.
+    # Default: 60
+    lines: 120
   gci:
      sections:
-      - prefix(open-cluster-management.io/governance-policy-nucleus)
-  golint:
-    # minimal confidence for issues, default is 0.8
-    min-confidence: 0.0
-  gofmt:
-    # simplify code: gofmt with `-s` option, true by default
-    simplify: true
-  maligned:
-    # print struct with more effective memory layout or not, false by default
-    suggest-new: true
-  misspell:
-    # Correct spellings using locale preferences for US or UK.
-    # Default is to use a neutral variety of English.
-    # Setting locale to US will correct the British spelling of 'colour' to 'color'.
-    locale: US
-    ignore-words:
-    - cancelled
-  lll:
-    # max line length, lines longer will be reported. Default is 120.
-    # '\t' is counted as 1 character by default, and can be changed with the tab-width option
-    line-length: 120
-    # tab width in spaces. Default to 1.
-    tab-width: 4
-  unused:
-    # treat code as a program (not a library) and report unused exported identifiers; default is false.
-    # XXX: if you enable this setting, unused will report a lot of false-positives in text editors:
-    # if it's called for subdir of a project it can't find funcs usages. All text editor integrations
-    # with golangci-lint call it on a directory with the changed file.
-    check-exported: false
-  unparam:
-    # call graph construction algorithm (cha, rta). In general, use cha for libraries,
-    # and rta for programs with main packages. Default is cha.
-    algo: cha
-
-    # Inspect exported functions, default is false. Set to true if no external program/library imports your code.
-    # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors:
-    # if it's called for subdir of a project it can't find external interfaces. All text editor integrations
-    # with golangci-lint call it on a directory with the changed file.
-    check-exported: false
-  gocritic:
+      - standard
+      - default
+      - prefix(open-cluster-management.io)
+  gocritic: # TODO: look through these checks
     enabled-checks:
       - appendCombine
       - boolExprSimplify
@@ -126,37 +86,54 @@ linters-settings:
       - typeUnparen
       - unnecessaryBlock
       - weakCond
+  lll:
+    # Max line length, lines longer will be reported.
+    # '\t' is counted as 1 character by default, and can be changed with the tab-width option.
+    line-length: 120
+    # Tab width in spaces. Default: 1
+    tab-width: 4
+  misspell:
+    # Correct spellings using locale preferences for US or UK.
+    # Setting locale to US will correct the British spelling of 'colour' to 'color'.
+    # Default is to use a neutral variety of English.
+    locale: US
+    ignore-words:
+    - cancelled
+  # revive:
+  #   rules:
+  #     - name: dot-imports
+  #       exclude: ["github.com/onsi/ginkgo/v2", "github.com/onsi/gomega"]
+  varnamelen:
+    max-distance: 15
+    ignore-decls:
+      - r client.Reader
+      - ok bool
 
 issues:
   # List of regexps of issue texts to exclude, empty list by default.
   # But independently from this option we use default exclude patterns,
   # it can be disabled by `exclude-use-default: false`. To list all
   # excluded by default patterns execute `golangci-lint run --help`
   exclude:
-    - composite literal uses unkeyed fields
-    - return statements should not be cuddled if block has more than two lines
+    # - composite literal uses unkeyed fields
+    # - return statements should not be cuddled if block has more than two lines
     - declarations should never be cuddled
     - don't use leading k in Go names
+    - Function TestAPIs missing the call to method parallel
 
   exclude-rules:
     # Allow dot imports in the tests.
-    - path: _test\.go$|^test/
+    - path: _test\.go
       linters:
-        - gci
-        - golint
+        # - gci
+        # - golint
         - revive
-        - stylecheck
-      source: \. "github\.com/onsi/(gomega|ginkgo)"
-    # Allow interface in the tests.
-    - path: _test\.go$|^test/
-      linters:
-        - forbidgo
-      source: fmt\.Print
-    # Allow returning interface in the tests.
-    - path: _test\.go$|^test/
-      linters:
-        - ireturn
-      source: func
+        # - stylecheck
+      text: dot imports
+    # - linters:
+    #     - varnamelen
+    #   source: r client.Reader
+    #   text: parameter name 'r' is too short for the scope of its usage
     # Don't enforce max line length for kubebuilder markers
     - linters:
         - lll
@@ -165,20 +142,49 @@ issues:
     - linters:
         - lll
       source: \/\/ ?https?:\/\/
-    # Don't enforce whitespace rules on test files
+    # Don't enforce some rules on test files
     - path: _test\.go$
       linters:
+        - varnamelen
         - wsl
+        - ireturn
+        - gochecknoglobals
+        - dupl
+        - funlen
+    # main.go tends to have longer functions
+    - path: main.go
+      linters:
+        - funlen
+        - cyclop
 
-  # Independently from option `exclude` we use default exclude patterns,
-  # it can be disabled by this option. To list all
-  # excluded by default patterns execute `golangci-lint run --help`.
-  # Default value for this option is true.
-  exclude-use-default: true
+  # # Independently of option `exclude` we use default exclude patterns,
+  # # it can be disabled by this option.
+  # # To list all excluded by default patterns execute `golangci-lint run --help`.
+  # # Default: true
+  # exclude-use-default: false
 
-  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
-  max-per-linter: 0
+  # # The list of ids of default excludes to include or disable.
+  # # https://golangci-lint.run/usage/false-positives/#default-exclusions
+  # include:
+  #   - EXC0001 # ignore return values of certain common functions
+  #   # - EXC0002
+  #   # - EXC0003
+  #   # - EXC0004
+  #   # - EXC0005
+  #   # - EXC0006
+  #   # - EXC0007
+  #   # - EXC0008
+  #   - EXC0009 # ignore common file/dir permissions warning
+  #   - EXC0010 # False positive is triggered by 'src, err := ioutil.ReadFile(filename)'
+  #   - EXC0011 # uncommon comments
+  #   # - EXC0012
+  #   - EXC0013 # package comments
+  #   # - EXC0014
+  #   - EXC0015 # package comments
 
   # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
   max-same-issues: 0
+
+  # Fix found issues (if it's supported by the linter). Default: false
+  fix: true
 

Makefile

@@ -89,7 +89,8 @@ generate: $(CONTROLLER_GEN) ## Generate code containing DeepCopy, DeepCopyInto,
 fmt: $(GOFUMPT) $(GCI)
 	go mod tidy
 	find . -not \( -path "./.go" -prune \) -name "*.go" | xargs $(GOFUMPT) -l -w
-	find . -not \( -path "./.go" -prune \) -name "*.go" | xargs $(GCI) write --skip-generated -s standard -s default -s localmodule
+	find . -not \( -path "./.go" -prune \) -name "*.go" | xargs $(GCI) write --skip-generated \
+	  -s standard -s default -s Prefix\(open-cluster-management.io\)
 
 .PHONY: vet
 vet: $(GOSEC)