Upgrade to Go 1.21 and make general improvements (#52)

Author: danielkee

.github/workflows/code-quality.yml renamed to .github/workflows/code-checks.yml

@@ -1,4 +1,4 @@
-name: Code Quality
+name: Code Checks
 
 on:
   pull_request:
@@ -14,9 +14,22 @@ jobs:
       - name: Set up Go environment
         uses: actions/setup-go@v4
         with:
-          go-version: 1.19
+          go-version: 1.21
       - name: Run linters
         uses: golangci/golangci-lint-action@v3
         with:
           args: "--timeout=3m --verbose"
           version: v1.55.2
+  unit-test:
+    name: Unit Test
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out repository
+        uses: actions/checkout@v3
+      - name: Set up Go environment
+        uses: actions/setup-go@v4
+        with:
+          go-version: 1.21
+      - name: Run unit tests
+        run: |
+          make test

.github/workflows/testing.yml

@@ -4,34 +4,42 @@ run:
 linters:
   enable-all: true
   disable:
-    - deadcode            # deprecated
-    - exhaustive          # not practical
-    - exhaustivestruct    # deprecated
-    - exhaustruct         # not practical
-    - golint              # deprecated
-    - ifshort             # deprecated
-    - interfacer          # deprecated
-    - ireturn             # good principle but occasional exceptions exist
-    - maligned            # deprecated
-    - nolintlint          # (*) disabled until 'leading space' clash with gofmt/gofumpt formatting is resolved
-    - nonamedreturns      # named returns are required to handle errors from deferred functions
-    - nosnakecase         # deprecated
-    - scopelint           # deprecated
-    - structcheck         # deprecated
-    - varcheck            # deprecated
-    - varnamelen          # disallows short variable names even if they are appropriate
-    - wsl                 # too aggressive
-    - depguard
+    - cyclop            # better judged on a case-by-case basis
+    - depguard          # we do not restrict package imports
+    - exhaustruct       # mandatory initialization of all struct fields is impractical
+    - funlen            # better judged on a case-by-case basis
+    - gochecknoglobals  # better judged on a case-by-case basis
+    - gocognit          # better judged on a case-by-case basis
+    - goconst           # better judged on a case-by-case basis
+    - gomnd             # better judged on a case-by-case basis
+    - nestif            # better judged on a case-by-case basis
+    - nonamedreturns    # named returns are required to handle errors from deferred functions
+    - varnamelen        # better judged on a case-by-case basis
+    - wsl               # too aggressive
+
+    # linters disabled because they are deprecated:
+    - deadcode
+    - exhaustivestruct
+    - golint
+    - ifshort
+    - interfacer
+    - maligned
+    - nosnakecase
+    - scopelint
+    - structcheck
+    - varcheck
 
 linters-settings:
-  funlen:
-    lines: -1
   lll:
     tab-width: 4
+  tagliatelle:
+    case:
+      rules:
+        json: snake
 
 issues:
   exclude-rules:
-    - path: ^.*main\.go$
+    - path: cmd/
       linters:
         - cyclop
         - forbidigo

.golangci.yml

@@ -4,20 +4,23 @@ TEST_SERVICE=test-generated-service
 
 all: lint test build test-generated-service
 
-# install development tools
+# install the necessary development tools
 PHONY+= install-tools
 install-tools:
-	@go install github.com/githubnemo/CompileDaemon@latest
+	@go install github.com/4meepo/tagalign/cmd/tagalign@latest
 	@go install github.com/daixiang0/gci@latest
+	@go install github.com/golangci/golangci-lint/cmd/[email protected]
+	@go install github.com/ypresto/CompileDaemon@bump-sys-fork # using fork until bug resolved: https://github.com/githubnemo/CompileDaemon/pull/76
 	@go install golang.org/x/tools/cmd/goimports@latest
-	@go install mvdan.cc/gofumpt@v0.3.0 # v0.3.1 fails to install on Go 1.16
+	@go install mvdan.cc/gofumpt@latest
 
 # run formatting utilities
 PHONY+= format
 format:
 	@goimports -l ./
 	@gci write ./
 	@gofumpt -l -w ./
+	@tagalign -fix -sort ./...
 
 # run linters
 PHONY+= lint

Makefile

@@ -1,6 +1,6 @@
 module github.com/puppetlabs/go-libs
 
-go 1.19
+go 1.21
 
 require (
 	github.com/cnjack/throttle v0.0.0-20160727064406-525175b56e18

go.mod

@@ -12,7 +12,8 @@ const DefaultLevel = logrus.DebugLevel
 
 // init runs when this library is first loaded.
 // This function sets the defaults that we want to use.
-// nolint: gochecknoinits // pragmatic usage of init function, not doing much harm here
+//
+//nolint:gochecknoinits
 func init() {
 	logrus.SetFormatter(&logrus.TextFormatter{
 		ForceColors: true,

internal/log/logger.go

@@ -40,7 +40,9 @@ var errTimedOutWaitingToSubmitTask = errors.New("timed out waiting to submit tas
 // NewDispatcher create a new Dispatcher.  The ID is used to identify the dispatcher in log messages.
 // workers is the number of go routines that this dispatcher will create to process work.
 // queueSize is the size of the channel used to store tasks.
-func NewDispatcher(id string, workers, queueSize int) Dispatcher {
+//
+//nolint:ireturn,nolintlint // the nolintlint linter generates a false positive here
+func NewDispatcher(id string, workers int, queueSize int) Dispatcher {
 	return &dispatcher{
 		ID:      id,
 		workers: workers,

pkg/concurrency/dispatcher.go

@@ -22,11 +22,12 @@ var (
 	errViperConfigNonPointerArgument = errors.New("reading Viper config requires a pointer argument")
 )
 
-// nolint:cyclop // naturally complex, we may be able to simplify this function
 func setUpViperConfig(cfg interface{}, v *viper.Viper) error {
 	// Get the type from the pointer or the struct itself - N.B. It will be a struct when called recursively.
 	var t reflect.Type
 	cfgType := reflect.TypeOf(cfg).Kind()
+
+	//nolint:exhaustive
 	switch cfgType {
 	case reflect.Ptr:
 		t = reflect.TypeOf(cfg).Elem()

pkg/config/config.go

@@ -38,14 +38,14 @@ type StructMissingDefaultTags struct {
 
 // StructEmptyDefaultTag holds one field with no default tag
 type StructEmptyDefaultTag struct {
-	TestVal string `env:"TEST_VAL" default:""`
+	TestVal string `default:"" env:"TEST_VAL"`
 }
 
 // StructAllTagged holds multiple fields with tags.
 type StructAllTagged struct {
-	TestVal  string `env:"TEST_VAL" default:"xyz"`
-	TestVal2 string `env:"TEST_VAL2" default:"abc"`
-	TestInt  int    `env:"TEST_INT" default:"2"`
+	TestVal  string `default:"xyz" env:"TEST_VAL"`
+	TestVal2 string `default:"abc" env:"TEST_VAL2"`
+	TestInt  int    `default:"2"   env:"TEST_INT"`
 }
 
 // InnerStructError is the nested struct
@@ -55,41 +55,41 @@ type InnerStructError struct {
 
 // OuterStructInnerError is the nested struct
 type OuterStructInnerError struct {
-	TestNestOuter string `env:"TEST_NEST_OUTER" default:"outer"`
+	TestNestOuter string `default:"outer" env:"TEST_NEST_OUTER"`
 	InnerStructError
 }
 
 // InnerStruct is the nested struct
 type InnerStruct struct {
-	TestNestInner string `env:"TEST_NEST_INNER" default:"inner"`
+	TestNestInner string `default:"inner" env:"TEST_NEST_INNER"`
 	InnerMostStruct
 }
 
 // InnerMostStruct is the nested struct
 type InnerMostStruct struct {
-	TestNestInnerMost string `env:"TEST_NEST_INNER_MOST" default:"innermost"`
+	TestNestInnerMost string `default:"innermost" env:"TEST_NEST_INNER_MOST"`
 }
 
 // OuterStruct is the struct containing the nested struct
 type OuterStruct struct {
-	TestNestOuter string `env:"TEST_NEST_OUTER" default:"outer"`
+	TestNestOuter string `default:"outer" env:"TEST_NEST_OUTER"`
 	InnerStruct
 }
 
 // Database contains the postgres config.
 type Database struct {
-	DBName   string `env:"UT_DB_NAME" default:"nottest"`
-	Host     string `env:"UT_DB_HOST" default:"xyz"`
-	Password string `env:"UT_DB_PASSWORD" default:"12345"`
-	Port     uint   `env:"UT_DB_PORT" default:"5"`
-	SSLMode  string `env:"UT_DB_SSL_MODE" default:"disabled"`
-	User     string `env:"UT_DB_USER" default:"abc"`
+	DBName   string `default:"nottest"  env:"UT_DB_NAME"`
+	Host     string `default:"xyz"      env:"UT_DB_HOST"`
+	Password string `default:"12345"    env:"UT_DB_PASSWORD"`
+	Port     uint   `default:"5"        env:"UT_DB_PORT"`
+	SSLMode  string `default:"disabled" env:"UT_DB_SSL_MODE"`
+	User     string `default:"abc"      env:"UT_DB_USER"`
 }
 
 // WebServer contains webserver configuration.
 type WebServer struct {
-	APIReadTimeout  time.Duration `env:"UT_WS_API_READ_TIMEOUT" default:"60s"`
-	APIWriteTimeout time.Duration `env:"UT_WS_API_WRITE_TIMEOUT" default:"60s"`
+	APIReadTimeout  time.Duration `default:"60s" env:"UT_WS_API_READ_TIMEOUT"`
+	APIWriteTimeout time.Duration `default:"60s" env:"UT_WS_API_WRITE_TIMEOUT"`
 }
 
 // AppConfig The configuration.

pkg/config/config_test.go

@@ -93,7 +93,6 @@ var (
 	errRecoveredFromPanic             = errors.New("recovered from panic")
 )
 
-// nolint: gochecknoglobals // pragmatic use of global variable, not doing much harm here
 var routerMap = make(map[string]*gin.RouterGroup)
 
 // Defining the readiness handler for potential use by k8s.