upgrade golangci-lint tool and enable more tools (#214)

Also fixed another 21 issues identified

Signed-off-by: Leslie Qi Wang <qiwa@pensando.io>

Author: leslie-qiwa

.github/workflows/lint.yml

@@ -34,7 +34,7 @@ jobs:
       run: go mod download
 
     - name: Run golangci-lint
-      uses: golangci/golangci-lint-action@v4
+      uses: golangci/golangci-lint-action@v8
       with:
         version: latest
         args: --timeout=5m

.golangci.yml

@@ -1,115 +1,131 @@
+version: "2"
 run:
-  timeout: 5m
-  go: '1.21'
-
-linters-settings:
-  govet:
-    enable:
-      - shadow
-  revive:
-    min-confidence: 0.8
-  gocyclo:
-    min-complexity: 15
-  dupl:
-    threshold: 100
-  goconst:
-    min-len: 2
-    min-occurrences: 2
-  misspell:
-    locale: US
-  lll:
-    line-length: 140
-  goimports:
-    local-prefixes: github.com/ChristopherHX/github-act-runner
-  gocritic:
-    enabled-tags:
-      - diagnostic
-      - experimental
-      - opinionated
-      - performance
-      - style
-    disabled-checks:
-      - dupImport # https://github.com/go-critic/go-critic/issues/845
-      - ifElseChain
-      - octalLiteral
-      - whyNoLint
-      - wrapperFunc
-
+  go: "1.21"
 linters:
   enable:
+    - asciicheck
+    - bidichk
     - bodyclose
-    # - depguard  # Disabled for now as it's too restrictive
+    - copyloopvar
     - dogsled
     - dupl
-    - errcheck
-    - copyloopvar  # replaces exportloopref for Go 1.22+
+    - durationcheck
+    - errname
+    - errorlint
     - exhaustive
+    - gocheckcompilerdirectives
     - goconst
     - gocritic
-    - gofmt
-    - goimports
-    - revive # replaces golint
-    - mnd    # replaces gomnd
     - goprintffuncname
     - gosec
-    - gosimple
-    - govet
-    - ineffassign
+    - importas
     - lll
+    - makezero
     - misspell
+    - mnd
     - nakedret
+    - nilerr
     - noctx
     - nolintlint
+    - prealloc
+    - revive
     - rowserrcheck
     - staticcheck
-    - stylecheck
-    - typecheck
+    - tparallel
     - unconvert
     - unparam
-    - unused
+    - usestdlibvars
     - whitespace
-
+  settings:
+    dupl:
+      threshold: 100
+    goconst:
+      min-len: 2
+      min-occurrences: 2
+    gocritic:
+      disabled-checks:
+        - dupImport
+        - ifElseChain
+        - octalLiteral
+        - whyNoLint
+        - wrapperFunc
+      enabled-tags:
+        - diagnostic
+        - experimental
+        - opinionated
+        - performance
+        - style
+    gocyclo:
+      min-complexity: 15
+    govet:
+      enable:
+        - shadow
+    lll:
+      line-length: 140
+    misspell:
+      locale: US
+    revive:
+      confidence: 0.8
+  exclusions:
+    generated: lax
+    rules:
+      - linters:
+          - gocritic
+          - gosec
+          - mnd
+        path: _test\.go
+      - linters:
+          - mnd
+        path: main\.go
+      - linters:
+          - revive
+        path: protocol/
+      - linters:
+          - gosec
+        text: weak cryptographic primitive
+      - linters:
+          - gosec
+        text: Use of weak random number generator
+      - linters:
+          - staticcheck
+        text: at least one file in a package should have a package comment
+      - linters:
+          - staticcheck
+        text: should have a package comment
+      - linters:
+          - revive
+        text: 'package-comments: should have a package comment'
+      - linters:
+          - revive
+        text: exported function .* should have comment or be unexported
+      - linters:
+          - revive
+        text: exported method .* should have comment or be unexported
+      - linters:
+          - revive
+        text: exported type .* should have comment or be unexported
+      - linters:
+          - revive
+        text: exported var .* should have comment or be unexported
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
 issues:
-  exclude-rules:
-    - path: _test\.go
-      linters:
-        - mnd
-        - gocritic
-        - gosec
-    - path: main\.go
-      linters:
-        - mnd
-    - path: protocol/
-      linters:
-        - revive # Protocol has many generated-like structs
-    - text: "weak cryptographic primitive"
-      linters:
-        - gosec
-    - text: "Use of weak random number generator"
-      linters:
-        - gosec
-    - text: "at least one file in a package should have a package comment"
-      linters:
-        - stylecheck
-    - text: "should have a package comment"
-      linters:
-        - stylecheck
-    - text: "package-comments: should have a package comment"
-      linters:
-        - revive
-    - text: "exported function .* should have comment or be unexported"
-      linters:
-        - revive
-    - text: "exported method .* should have comment or be unexported"
-      linters:
-        - revive
-    - text: "exported type .* should have comment or be unexported"
-      linters:
-        - revive
-    - text: "exported var .* should have comment or be unexported"
-      linters:
-        - revive
-
-  exclude-use-default: false
   max-issues-per-linter: 0
   max-same-issues: 0
+formatters:
+  enable:
+    - gofmt
+    - gofumpt
+    - goimports
+  settings:
+    goimports:
+      local-prefixes:
+        - github.com/ChristopherHX/github-act-runner
+  exclusions:
+    generated: lax
+    paths:
+      - third_party$
+      - builtin$
+      - examples$

actionsdotnetactcompat/act_runner.go

@@ -10,7 +10,8 @@ type ActRunner struct {
 }
 
 func (arunner *ActRunner) ExecWorker(run *actionsrunner.RunRunner, wc actionsrunner.WorkerContext,
-	jobreq *protocol.AgentJobRequestMessage, src []byte) error {
+	jobreq *protocol.AgentJobRequestMessage, src []byte,
+) error {
 	if len(arunner.WorkerArgs) == 0 {
 		ExecWorker(jobreq, wc)
 		return nil

actionsdotnetactcompat/act_worker.go

@@ -38,9 +38,15 @@ const (
 	maxRedirects = 10
 	jobTimeout   = 5 * time.Minute
 	// File permissions
-	directoryPermissions = 0777
+	directoryPermissions = 0o777
 	// Stage constants
 	mainStage = "Main"
+	// Log prefix constants
+	debugPrefix = "##[debug]"
+	warnPrefix  = "##[warn]"
+	errorPrefix = "##[error]"
+	panicPrefix = "##[panic]"
+	fatalPrefix = "##[fatal]"
 )
 
 type ghaFormatter struct {
@@ -54,11 +60,14 @@ type ghaFormatter struct {
 }
 
 func flushInternal(rec *protocol.TimelineRecord, res *model.StepResult) {
-	if res.Conclusion == model.StepStatusSuccess {
+	switch res.Conclusion {
+	case model.StepStatusSuccess:
 		rec.Complete("Succeeded")
-	} else if res.Conclusion == model.StepStatusSkipped {
+	case model.StepStatusSkipped:
 		rec.Complete("Skipped")
-	} else {
+	case model.StepStatusFailure:
+		rec.Complete("Failed")
+	default:
 		rec.Complete("Failed")
 	}
 }
@@ -176,12 +185,21 @@ func (f *ghaFormatter) Format(entry *logrus.Entry) ([]byte, error) {
 	}
 
 	prefix := entry.Time.UTC().Format(protocol.TimestampOutputFormat) + " "
-	if entry.Level == logrus.DebugLevel {
-		prefix += "##[debug]"
-	} else if entry.Level == logrus.WarnLevel {
-		prefix += "##[warning]"
-	} else if entry.Level == logrus.ErrorLevel {
-		prefix += "##[error]"
+	switch entry.Level {
+	case logrus.DebugLevel:
+		prefix += debugPrefix
+	case logrus.WarnLevel:
+		prefix += warnPrefix
+	case logrus.ErrorLevel:
+		prefix += errorPrefix
+	case logrus.PanicLevel:
+		prefix += panicPrefix
+	case logrus.FatalLevel:
+		prefix += fatalPrefix
+	case logrus.InfoLevel:
+		// No special prefix for info level
+	case logrus.TraceLevel:
+		prefix += debugPrefix
 	}
 	entry.Message = f.linefeedregex.ReplaceAllString(prefix+strings.Trim(entry.Message, "\r\n"), "\n"+prefix)
 
@@ -639,7 +657,7 @@ func downloadAndExtractAction(
 		if contextLogger != nil {
 			contextLogger.Infof("Downloading action %v/%v (sha:%v) from %v", owner, name, resolvedSha, tarURL)
 		}
-		req, err := http.NewRequestWithContext(ctx, "GET", tarURL, http.NoBody)
+		req, err := http.NewRequestWithContext(ctx, http.MethodGet, tarURL, http.NoBody)
 		if err != nil {
 			return err
 		}

actionsrunner/logger.go

@@ -8,8 +8,7 @@ type BasicLogger interface {
 	Printf(format string, a ...interface{})
 }
 
-type ConsoleLogger struct {
-}
+type ConsoleLogger struct{}
 
 func (*ConsoleLogger) Printf(format string, a ...interface{}) {
 	fmt.Printf(format, a...)

actionsrunner/runner.go

@@ -61,7 +61,6 @@ type RunnerEnvironment interface {
 	ExecWorker(run *RunRunner, wc WorkerContext, jobreq *protocol.AgentJobRequestMessage, src []byte) error
 }
 
-//nolint:revive // context-as-argument: API compatibility requirement - cannot change parameter order
 func (run *RunRunner) Run(runnerenv RunnerEnvironment, listenerctx, corectx context.Context) error {
 	settings := run.Settings
 	for i := 0; i < len(settings.Instances); i++ {
@@ -448,17 +447,16 @@ type RunnerJobRequestRef struct {
 	RunServiceURL   string `json:"run_service_url"`
 }
 
-type plainTextFormatter struct {
-}
+type plainTextFormatter struct{}
 
 func (f *plainTextFormatter) Format(entry *logrus.Entry) ([]byte, error) {
 	return []byte(entry.Time.UTC().Format(protocol.TimestampOutputFormat) + " " + entry.Message + "\n"), nil
 }
 
-//nolint:revive // context-as-argument: Legacy function signature for compatibility
 func runJob(runnerenv RunnerEnvironment, joblock *sync.Mutex, vssConnection *protocol.VssConnection, run *RunRunner,
 	cancel context.CancelFunc, cancelJob context.CancelFunc, finishJob context.CancelFunc, jobExecCtx context.Context, jobctx context.Context,
-	session *protocol.AgentMessageConnection, message protocol.TaskAgentMessage, instance *runnerconfiguration.RunnerInstance) {
+	session *protocol.AgentMessageConnection, message protocol.TaskAgentMessage, instance *runnerconfiguration.RunnerInstance,
+) {
 	go func() {
 		plogger := &PrefixConsoleLogger{
 			Parent: runnerenv,

actionsrunner/worker_runner_environment.go

@@ -16,7 +16,7 @@ const (
 	messageIDSize    = 4
 	cancelRequestCmd = 2
 	// File permissions
-	filePermissions = 0664
+	filePermissions = 0o664
 )
 
 type WorkerRunnerEnvironment struct {
@@ -57,7 +57,7 @@ func (arunner *WorkerRunnerEnvironment) ExecWorker(
 		return fmt.Errorf("missing WorkerArgs to execute an external worker")
 	}
 	//nolint:gosec // WorkerArgs are configured by the administrator, not user input
-	worker := exec.Command(arunner.WorkerArgs[0], arunner.WorkerArgs[1:]...)
+	worker := exec.CommandContext(jobExecCtx, arunner.WorkerArgs[0], arunner.WorkerArgs[1:]...)
 	in, err := worker.StdinPipe()
 	if err != nil {
 		return err

common/io.go

@@ -7,7 +7,7 @@ import (
 
 const (
 	// File permissions
-	filePermissions = 0664
+	filePermissions = 0o664
 )
 
 func WriteJSON(path string, value interface{}) error {