Upgrade golangci-lint to v2

.github/workflows/ci.yaml

@@ -218,7 +218,7 @@ jobs:
     name: lint
     runs-on: ubuntu-latest
     env:
-      GOLANGCI_LINT_VERSION: v1.64.4
+      GOLANGCI_LINT_VERSION: v2.0.0
     permissions:
       contents: read
       # allow read access to pull request. Use with `only-new-issues` option.
@@ -234,7 +234,7 @@ jobs:
         uses: actions/checkout@v4
 
       - name: Lint
-        uses: golangci/golangci-lint-action@v4
+        uses: golangci/golangci-lint-action@v7
         with:
           # golangci-lint needs to be run separately for every Go module, and
           # its GitHub Action doesn't provide any way to do that. Have it fetch

.golangci.yaml

@@ -1,16 +1,7 @@
-issues:
-  exclude:
-    - 'Error return value of .(\w+\.Rollback(.*)). is not checked'
+version: "2"
 
 linters:
-  presets:
-    - bugs
-    - comment
-    - format
-    - performance
-    - style
-    - test
-    - unused
+  default: all
 
   disable:
     # disabled, but which we should enable with discussion
@@ -21,82 +12,109 @@ linters:
     - testpackage # requires tests in test packages like `river_test`
 
     # disabled because they're annoying/bad
+    - cyclop # screams into the void at "cyclomatic complexity"
+    - funlen # screams when functions are more than 60 lines long; what are we even doing here guys
     - interfacebloat # we do in fact want >10 methods on the Adapter interface or wherever we see fit.
+    - gocognit # yells that "cognitive complexity" is too high; why
+    - gocyclo # ANOTHER "cyclomatic complexity" checker (see also "cyclop" and "gocyclo")
     - godox # bans TODO statements; total non-starter at the moment
     - err113 # wants all errors to be defined as variables at the package level; quite obnoxious
+    - maintidx # ANOTHER ANOTHER "cyclomatic complexity" lint (see also "cyclop" and "gocyclo")
     - mnd # detects "magic numbers", which it defines as any number; annoying
+    - nestif # yells when if blocks are nested; what planet do these people come from?
     - ireturn # bans returning interfaces; questionable as is, but also buggy as hell; very, very annoying
     - lll # restricts maximum line length; annoying
     - nlreturn # requires a blank line before returns; annoying
     - wsl # a bunch of style/whitespace stuff; annoying
 
-linters-settings:
-  depguard:
-    rules:
-      all:
-        files: ["$all"]
-        allow:
-        deny:
-          - desc: "Use `github.com/google/uuid` package for UUIDs instead."
-            pkg: "github.com/xtgo/uuid"
-      not-test:
-        files: ["!$test"]
-        deny:
-          - desc: "Don't use `dbadaptertest` package outside of test environments."
-            pkg: "github.com/riverqueue/river/internal/dbadaptertest"
-          - desc: "Don't use `riverinternaltest` package outside of test environments."
-            pkg: "github.com/riverqueue/river/internal/riverinternaltest"
+  settings:
+    depguard:
+      rules:
+        all:
+          files: ["$all"]
+          deny:
+            - desc: Use `github.com/google/uuid` package for UUIDs instead.
+              pkg: github.com/xtgo/uuid
+        not-test:
+          files: ["!$test"]
+          deny:
+            - desc: Don't use `dbadaptertest` package outside of test environments.
+              pkg: github.com/riverqueue/river/internal/dbadaptertest
+            - desc: Don't use `riverinternaltest` package outside of test environments.
+              pkg: github.com/riverqueue/river/internal/riverinternaltest
 
-  forbidigo:
-    forbid:
-      - msg: "Use `require` variants instead."
-        p: '^assert\.'
-      - msg: "Use `Func` suffix for function variables instead."
-        p: 'Fn\b'
-      - msg: "Use built-in `max` function instead."
-        p: '\bmath\.Max\b'
-      - msg: "Use built-in `min` function instead."
-        p: '\bmath\.Min\b'
+    forbidigo:
+      forbid:
+        - msg: Use `require` variants instead.
+          pattern: ^assert\.
+        - msg: Use `Func` suffix for function variables instead.
+          pattern: Fn\b
+        - msg: Use built-in `max` function instead.
+          pattern: \bmath\.Max\b
+        - msg: Use built-in `min` function instead.
+          pattern: \bmath\.Min\b
 
-  gci:
-    sections:
-      - Standard
-      - Default
-      - Prefix(github.com/riverqueue)
+    gomoddirectives:
+      replace-local: true
 
-  gomoddirectives:
-    replace-local: true
+    gosec:
+      excludes:
+        - G404 # use of non-crypto random; overly broad for our use case
 
-  gosec:
-    excludes:
-      - G404 # use of non-crypto random; overly broad for our use case
+    revive:
+      rules:
+        - name: unused-parameter
+          disabled: true
 
-  revive:
-    rules:
-      - name: unused-parameter
-        disabled: true
+    tagliatelle:
+      case:
+        rules:
+          json: snake
 
-  tagliatelle:
-    case:
-      rules:
-        json: snake
+    testifylint:
+      enable-all: true
+      disable:
+        - go-require
+
+    varnamelen:
+      ignore-names:
+        - db
+        - eg
+        - f
+        - i
+        - id
+        - j
+        - mu
+        - r
+        - sb # common convention for string builder
+        - t
+        - tt # common convention for table tests
+        - tx
+        - w
+        - wg
+
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    rules:
+      - path: (.+)\.go$
+        text: Error return value of .(\w+\.Rollback(.*)). is not checked
 
-  testifylint:
-    enable-all: true
-    disable:
-      - go-require
+formatters:
+  enable:
+    - gci
+    - gofmt
+    - gofumpt
+    - goimports
 
-  varnamelen:
-    ignore-names:
-      - db
-      - eg
-      - f
-      - i
-      - id
-      - j
-      - mu
-      - sb # common convention for string builder
-      - t
-      - tt # common convention for table tests
-      - tx
-      - wg
+  settings:
+    gci:
+      sections:
+        - Standard
+        - Default
+        - Prefix(github.com/riverqueue)
+        - Prefix(riverqueue.com/riverpro)

error_test.go

@@ -31,7 +31,7 @@ func TestUnknownJobKindError_As(t *testing.T) {
 		t.Parallel()
 
 		var err *river.UnknownJobKindError
-		require.False(t, errors.As(errors.New("some other error"), &err))
+		require.NotErrorAs(t, errors.New("some other error"), &err)
 	})
 }
 

internal/dblist/db_list.go

@@ -99,10 +99,13 @@ func JobList(ctx context.Context, exec riverdriver.Executor, params *JobListPara
 
 	for i, orderBy := range params.OrderBy {
 		orderByBuilder.WriteString(orderBy.Expr)
-		if orderBy.Order == SortOrderAsc {
+		switch orderBy.Order {
+		case SortOrderAsc:
 			orderByBuilder.WriteString(" ASC")
-		} else if orderBy.Order == SortOrderDesc {
+		case SortOrderDesc:
 			orderByBuilder.WriteString(" DESC")
+		case SortOrderUnspecified:
+			return nil, errors.New("should not have gotten SortOrderUnspecified by this point before executing list (bug?)")
 		}
 		if i < len(params.OrderBy)-1 {
 			orderByBuilder.WriteString(", ")

internal/jobexecutor/job_executor_test.go

@@ -233,7 +233,7 @@ func TestJobExecutor_Execute(t *testing.T) {
 
 		executor, bundle := setup(t)
 
-		now := executor.Archetype.Time.StubNowUTC(time.Now().UTC())
+		now := executor.Time.StubNowUTC(time.Now().UTC())
 
 		workerErr := errors.New("job error")
 		executor.WorkUnit = newWorkUnitFactoryWithCustomRetry(func() error { return workerErr }, nil).MakeUnit(bundle.jobRow)
@@ -249,7 +249,7 @@ func TestJobExecutor_Execute(t *testing.T) {
 		require.Equal(t, now.Truncate(1*time.Microsecond), job.Errors[0].At.Truncate(1*time.Microsecond))
 		require.Equal(t, 1, job.Errors[0].Attempt)
 		require.Equal(t, "job error", job.Errors[0].Error)
-		require.Equal(t, "", job.Errors[0].Trace)
+		require.Empty(t, job.Errors[0].Trace)
 	})
 
 	t.Run("ErrorAgainAfterRetry", func(t *testing.T) {
@@ -361,7 +361,7 @@ func TestJobExecutor_Execute(t *testing.T) {
 		require.WithinDuration(t, time.Now(), job.Errors[0].At, 2*time.Second)
 		require.Equal(t, 1, job.Errors[0].Attempt)
 		require.Equal(t, "JobCancelError: throw away this job", job.Errors[0].Error)
-		require.Equal(t, "", job.Errors[0].Trace)
+		require.Empty(t, job.Errors[0].Trace)
 	})
 
 	t.Run("JobSnoozeErrorReschedulesJobAndDecrementsAttempt", func(t *testing.T) {
@@ -721,7 +721,7 @@ func TestJobExecutor_Execute(t *testing.T) {
 		require.Equal(t, 1, job.Errors[0].Attempt)
 		require.Equal(t, "JobCancelError: job cancelled remotely", job.Errors[0].Error)
 		require.Equal(t, rivertype.ErrJobCancelledRemotely.Error(), job.Errors[0].Error)
-		require.Equal(t, "", job.Errors[0].Trace)
+		require.Empty(t, job.Errors[0].Trace)
 	})
 
 	t.Run("RemoteCancellationJobNotCancelledIfNoErrorReturned", func(t *testing.T) {

job_list_params_test.go

@@ -25,8 +25,8 @@ func Test_JobListCursor_JobListCursorFromJob(t *testing.T) {
 	cursor := JobListCursorFromJob(jobRow)
 	require.Zero(t, cursor.id)
 	require.Equal(t, jobRow, cursor.job)
-	require.Zero(t, cursor.kind)
-	require.Zero(t, cursor.queue)
+	require.Empty(t, cursor.kind)
+	require.Empty(t, cursor.queue)
 	require.Zero(t, cursor.sortField)
 	require.Zero(t, cursor.time)
 }
@@ -163,7 +163,7 @@ func Test_JobListCursor_MarshalJSON(t *testing.T) {
 
 		text, err := json.Marshal(cursor)
 		require.NoError(t, err)
-		require.NotEqual(t, "", text)
+		require.NotEmpty(t, text)
 
 		unmarshaledParams := &JobListCursor{}
 		require.NoError(t, json.Unmarshal(text, unmarshaledParams))

producer.go

@@ -525,10 +525,7 @@ func (p *producer) innerFetchLoop(workCtx context.Context, fetchResultCh chan pr
 func (p *producer) executorShutdownLoop() {
 	// No more jobs will be fetched or executed. However, we must wait for all
 	// in-progress jobs to complete.
-	for {
-		if len(p.activeJobs) == 0 {
-			break
-		}
+	for len(p.activeJobs) != 0 {
 		result := <-p.jobResultCh
 		p.removeActiveJob(result.ID)
 	}

rivershared/testfactory/test_factory.go

@@ -46,7 +46,7 @@ func Job(ctx context.Context, tb testing.TB, exec riverdriver.Executor, opts *Jo
 	return job
 }
 
-func Job_Build(tb testing.TB, opts *JobOpts) *riverdriver.JobInsertFullParams { //nolint:stylecheck
+func Job_Build(tb testing.TB, opts *JobOpts) *riverdriver.JobInsertFullParams {
 	tb.Helper()
 
 	encodedArgs := opts.EncodedArgs