Improve golang ci lint (#18)

Signed-off-by: Aniruddha Basak <aniruddha.basak@syself.com>
SovereignCloudStack · Oct 10, 2023 · 0c4a44a · 0c4a44a
1 parent 812c71b
commit 0c4a44a
Show file tree

Hide file tree

Showing 9 changed files with 137 additions and 194 deletions.
diff --git a/.golangci.yaml b/.golangci.yaml
@@ -6,17 +6,20 @@ linters:
     - bidichk
     - bodyclose
     - containedctx
-    # - depguard
-    - dogsled
+    - contextcheck
     - durationcheck
-    - errcheck
     - errchkjson
+    - errname
+    - errorlint
+    - exhaustive
     - exportloopref
+    - forcetypeassert
     - gci
     - goconst
     - gocritic
     - godot
     - gofmt
+    - gofumpt
     - goimports
     - goprintffuncname
     - gosec
@@ -25,37 +28,29 @@ linters:
     - importas
     - ineffassign
     - loggercheck
+    - makezero
     - misspell
     - nakedret
     - nilerr
     - noctx
     - nolintlint
+    - nosprintfhostport
     - prealloc
     - predeclared
+    - reassign
     - revive
     - rowserrcheck
     - staticcheck
     - stylecheck
+    - tagliatelle
     - thelper
+    - tparallel
     - typecheck
     - unconvert
-    - unparam
-    - unused
-    - whitespace
-    - wrapcheck
-    - contextcheck
-    - errname
-    - errorlint
-    - exhaustive
-    - forcetypeassert
-    - gofmt
-    - gofumpt
-    - makezero
-    - nosprintfhostport
-    - tagliatelle
-    - tparallel
     - usestdlibvars
+    - unused
     - wastedassign
+    - wrapcheck
 
 linters-settings:
   godot:
@@ -85,58 +80,27 @@ linters-settings:
       # Controller Runtime
       - pkg: sigs.k8s.io/controller-runtime
         alias: ctrl
+  gofumpt:
+    extra-rules: true
   nolintlint:
     allow-unused: false
     allow-leading-space: false
     require-specific: true
   staticcheck:
-    go: "1.20"
+    go: "1.21"
   stylecheck:
-    go: "1.20"
+    go: "1.21"
     checks: ["all", "-ST1006"]
     dot-import-whitelist:
       - "github.com/onsi/gomega"
       - "github.com/onsi/ginkgo/v2"
-  gosec:
-    excludes:
-      - G307 # Deferring unsafe method "Close" on type "\*os.File"
-      - G108 # Profiling endpoint is automatically exposed on /debug/pprof
-      - G106 # Allowing for insecure ssh
   gocritic:
     enabled-tags:
       - diagnostic
-      - experimental
+      - style
       - performance
-    disabled-checks:
-      - appendAssign
-      - dupImport # https://github.com/go-critic/go-critic/issues/845
-      - evalOrder
-      - ifElseChain
-      - octalLiteral
-      - regexpSimplify
-      - sloppyReassign
-      - truncateCmp
-      - typeDefFirst
-      - unnamedResult
-      - unnecessaryDefer
-      - whyNoLint
-      - wrapperFunc
-      - rangeValCopy
-      - hugeParam
-  unused:
-    go: "1.20"
-  wrapcheck:
-    ignoreSigs:
-      - status.Error(
-      - .Errorf(
-      - errors.New(
-      - errors.Unwrap(
-      - .Wrap(
-      - .Wrapf(
-      - .WithMessage(
-      - .WithMessagef(
-      - .WithStack(
-      - .Complete(
+      - experimental
+      - opinionated
   revive:
     enable-all-rules: true
     rules:
@@ -145,11 +109,12 @@ linters-settings:
       # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#add-constant
       - name: add-constant
         severity: warning
-        disabled: false
+        disabled: true
         arguments:
           - maxLitCount: "3"
             allowStrs: '""'
             allowInts: "0,1,2,3,42,100"
+
       # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#argument-limit
       - name: argument-limit
         severity: warning
@@ -242,113 +207,85 @@ linters-settings:
       # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#comment-spacings
       - name: comment-spacings
         disabled: true
-      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#add-constant
-      - name: add-constant
-        disabled: true
       # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#use-any
       - name: use-any
         disabled: true
       # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#deep-exit
       - name: deep-exit
         disabled: true
-      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#struct-tag
-      - name: struct-tag
-        disabled: true
-      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#receiver-naming
-      - name: receiver-naming
-        disabled: true
       # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#receiver-naming
       - name: nested-structs
         disabled: true
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#struct-tag
+      - name: struct-tag
+        disabled: true
+  unused:
+    go: "1.21"
+  usestdlibvars:
+    # Suggest the use of http.MethodXX.
+    # Default: true
+    http-method: true
+    # Suggest the use of http.StatusXX.
+    # Default: true
+    http-status-code: true
+    # Suggest the use of time.Weekday.String().
+    # Default: true
+    time-weekday: true
+    # Suggest the use of time.Month.String().
+    # Default: false
+    time-month: true
+    # Suggest the use of time.Layout.
+    # Default: false
+    time-layout: true
+    # Suggest the use of crypto.Hash.String().
+    # Default: false
+    crypto-hash: true
+    # Suggest the use of rpc.DefaultXXPath.
+    # Default: false
+    default-rpc-path: true
+    # Suggest the use of os.DevNull.
+    # Default: false
+    os-dev-null: true
+    # Suggest the use of sql.LevelXX.String().
+    # Default: false
+    sql-isolation-level: true
+    # Suggest the use of tls.SignatureScheme.String().
+    # Default: false
+    tls-signature-scheme: true
+    # Suggest the use of constant.Kind.String().
+    # Default: false
+    constant-kind: true
+    # Suggest the use of syslog.Priority.
+    # Default: false
+    syslog-priority: true
+  wrapcheck:
+    ignoreSigs:
+      - status.Error(
+      - .Errorf(
+      - errors.New(
+      - errors.Unwrap(
+      - .Wrap(
+      - .Wrapf(
+      - .WithMessage(
+      - .WithMessagef(
+      - .WithStack(
+      - .Complete(
 issues:
   max-same-issues: 0
   max-issues-per-linter: 0
-  # We are disabling default golangci exclusions because we want to help reviewers to focus on reviewing the most relevant
+  # We are disabling default golangci exclusions
+  # because we want to help reviewers to focus on reviewing the most relevant
   # changes in PRs and avoid nitpicking.
   exclude-use-default: false
   exclude-rules:
     - linters:
         - wrapcheck
       path: _test\.go
-    - linters:
-        - revive
-      text: "exported: exported method .*\\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported"
-    - linters:
-        - errcheck
-      text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
-    # Exclude some packages or code to require comments, for example test code, or fake clients.
-    - linters:
-        - revive
-      text: exported (method|function|type|const) (.+) should have comment or be unexported
-      source: (func|type).*Fake.*
-    - linters:
-        - revive
-      text: exported (method|function|type|const) (.+) should have comment or be unexported
-      path: fake_\.go
-    - linters:
-        - revive
-      text: exported (method|function|type|const) (.+) should have comment or be unexported
-      path: "(framework|e2e)/.*.go"
-    # Disable unparam "always receives" which might not be really
-    # useful when building libraries.
-    - linters:
-        - unparam
-      text: always receives
-    # Dot imports for gomega or ginkgo are allowed
-    # within test files.
-    - path: _test\.go
-      text: should not use dot imports
-    - path: (framework|e2e)/.*.go
-      text: should not use dot imports
-    - path: _test\.go
-      text: cyclomatic complexity
-    # Append should be able to assign to a different var/slice.
-    - linters:
-        - gocritic
-      text: "appendAssign: append result not assigned to the same slice"
-    # Disable linters for conversion
-    - linters:
-        - staticcheck
-      text: "SA1019: in.(.+) is deprecated"
-      path: .*(api|types)\/.*\/conversion.*\.go$
-    - linters:
-        - revive
-      text: exported (method|function|type|const) (.+) should have comment or be unexported
-      path: .*(api|types|test)\/.*\/conversion.*\.go$
-    - linters:
-        - revive
-      text: "var-naming: don't use underscores in Go names;"
-      path: .*(api|types|test)\/.*\/conversion.*\.go$
-    - linters:
-        - revive
-      text: "receiver-naming: receiver name"
-      path: .*(api|types)\/.*\/conversion.*\.go$
-    - linters:
-        - stylecheck
-      text: "ST1003: should not use underscores in Go names;"
-      path: .*(api|types|test)\/.*\/conversion.*\.go$
-    - linters:
-        - stylecheck
-      text: "ST1016: methods on the same type should have the same receiver name"
-      path: .*(api|types)\/.*\/conversion.*\.go$
-    # hack/tools
-    - linters:
-        - typecheck
-      text: import (".+") is a program, not an importable package
-      path: ^tools\.go$
-    # We don't care about defer in for loops in test files.
-    - linters:
-        - gocritic
-      text: "deferInLoop: Possible resource leak, 'defer' is called in the 'for' loop"
-      path: _test\.go
 run:
   timeout: 10m
-  skip-files:
-    - "zz_generated.*\\.go$"
-    - ".*conversion.*\\.go$"
-    - "test/e2e/*\\.go$"
-  skip-dirs:
-    - "third_party"
-    - "tilt_modules"
+  go: "1.21"
   allow-parallel-runners: true
-  modules-download-mode: readonly
+  modules-download-mode: vendor
+  skip-dirs:
+    - vendor$
+    - test/vendor$
diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml
@@ -6,18 +6,14 @@ commonLabels:
   cluster.x-k8s.io/provider: "infrastructure-cluster-stack-operator"
 
 resources:
-- ../crd
-- ../rbac
-- ../manager
-#- ../webhook
-- ../certmanager
+  - ../crd
+  - ../rbac
+  - ../manager
+  - ../certmanager
 
 patchesStrategicMerge:
-- manager_config_patch.yaml
-#- manager_webhook_patch.yaml
-#- webhookcainjection_patch.yaml
-- manager_pull_policy.yaml
-
+  - manager_config_patch.yaml
+  - manager_pull_policy.yaml
 # vars:
 # - name: CERTIFICATE_NAMESPACE  # namespace of the certificate CR
 #   objref:
@@ -44,4 +40,4 @@ patchesStrategicMerge:
 #   objref:
 #     kind: Service
 #     version: v1
-#     name: webhook-service
+#     name: webhook-service
diff --git a/internal/clusterstackrelease/util.go b/internal/clusterstackrelease/util.go
@@ -48,11 +48,12 @@ func Summary(csr *csov1alpha1.ClusterStackRelease) (csov1alpha1.ClusterStackRele
 
 	// if provider-specific work is done, we are left with applying objects
 	// We don't expect the condition to be not set at all, hence no else case here
-	if conditions.IsTrue(csr, csov1alpha1.ProviderClusterStackReleaseReadyCondition) {
+	switch {
+	case conditions.IsTrue(csr, csov1alpha1.ProviderClusterStackReleaseReadyCondition):
 		summary.Phase = csov1alpha1.ClusterStackReleasePhaseApplyingObjects
-	} else if conditions.IsTrue(csr, csov1alpha1.ClusterStackReleaseAssetsReadyCondition) {
+	case conditions.IsTrue(csr, csov1alpha1.ClusterStackReleaseAssetsReadyCondition):
 		summary.Phase = csov1alpha1.ClusterStackReleasePhaseProviderSpecificWork
-	} else if conditions.IsFalse(csr, csov1alpha1.ClusterStackReleaseAssetsReadyCondition) {
+	case conditions.IsFalse(csr, csov1alpha1.ClusterStackReleaseAssetsReadyCondition):
 		summary.Phase = csov1alpha1.ClusterStackReleasePhaseDownloadingAssets
 	}