Skip to content

Commit

Permalink
Upgrade to golangci-lint 1.61.0 (riverqueue#592)
Browse files Browse the repository at this point in the history
I accidentally upgraded my golangci-lint locally, so figured I may as
well take it up a version in CI as well.

The main new drama queen for this version is fatcontext [1]. It looks
like it's a well-intentioned lint, which tries to avoid having callers
set context in loops and inadvertently create a super deep context that
causes performance issues:

    func notOk() {
            ctx := context.Background()

            for i := 0; i < 10; i++ {
                    ctx = context.WithValue(ctx, "key", i) // "nested context in loop"
                    _ = ctx
            }
    }

Unfortunately, as with many well-intentioned golangci-lint additions,
its heuristic is overly broad, and it produces a lot of false positives
(in fact, 100% of lines it found were false positives). I've marked
lines `nolint:fatcontext` for now, but this may be one that we disable
if it gets too annoying because it's almost certainly more cost than
benefit.

The good news is that we get upgraded version of `gosec`, and some of
the lines that were marked as `nolint` and which were false positives
on unsafe integer conversion (they were always safe) can be removed.

[1] https://github.com/Crocmagnon/fatcontext
  • Loading branch information
brandur authored and tigrato committed Dec 18, 2024
1 parent 9844587 commit 2b12a3a
Show file tree
Hide file tree
Showing 8 changed files with 9 additions and 9 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ jobs:
name: lint
runs-on: ubuntu-latest
env:
GOLANGCI_LINT_VERSION: v1.60.1
GOLANGCI_LINT_VERSION: v1.61.0
permissions:
contents: read
# allow read access to pull request. Use with `only-new-issues` option.
Expand Down
2 changes: 1 addition & 1 deletion client.go
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ func (c *Client[TTx]) Start(ctx context.Context) error {
}
}

c.queues.fetchCtx = fetchCtx
c.queues.fetchCtx = fetchCtx //nolint:fatcontext
c.queues.workCtx = workCtx
c.workCancel = workCancel

Expand Down
2 changes: 1 addition & 1 deletion internal/notifier/notifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ func (n *Notifier) listenerUnlisten(ctx context.Context, topic NotificationTopic
func (n *Notifier) waitOnce(ctx context.Context) error {
n.withLock(func() {
n.isWaiting = true
ctx, n.waitCancel = context.WithCancel(ctx)
ctx, n.waitCancel = context.WithCancel(ctx) //nolint:fatcontext
})
defer n.withLock(func() {
n.isWaiting = false
Expand Down
2 changes: 1 addition & 1 deletion job_list_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ func (p *JobListParams) First(count int) *JobListParams {
panic("count must be <= 10000")
}
paramsCopy := p.copy()
paramsCopy.paginationCount = int32(count) //nolint:gosec
paramsCopy.paginationCount = int32(count)
return paramsCopy
}

Expand Down
2 changes: 1 addition & 1 deletion queue_list_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@ func (p *QueueListParams) First(count int) *QueueListParams {
panic("count must be <= 10000")
}
result := p.copy()
result.paginationCount = int32(count) //nolint:gosec
result.paginationCount = int32(count)
return result
}
4 changes: 2 additions & 2 deletions rivershared/levenshtein/levenshtein.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func ComputeDistance(str1, str2 string) int {

// we start from 1 because index 0 is already 0.
for i := 1; i < len(distances); i++ {
distances[i] = uint16(i)
distances[i] = uint16(i) //nolint:gosec
}

// Make a dummy bounds check to prevent the 2 bounds check down below. The
Expand All @@ -71,7 +71,7 @@ func ComputeDistance(str1, str2 string) int {

// fill in the rest
for i := 1; i <= lenRuneSlice2; i++ {
prev := uint16(i)
prev := uint16(i) //nolint:gosec
for j := 1; j <= lenRuneSlice1; j++ {
current := distances[j-1] // match
if runeSlice2[i-1] != runeSlice1[j-1] {
Expand Down
2 changes: 1 addition & 1 deletion rivershared/levenshtein/levenshtein_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func TestUnicode(t *testing.T) {
}

// Benchmarks
// ----------------------------------------------
// ----------------------------------------------.
var sink int //nolint:gochecknoglobals

func BenchmarkSimple(b *testing.B) {
Expand Down
2 changes: 1 addition & 1 deletion rivershared/startstop/start_stop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ func TestErrStop(t *testing.T) {
return nil
}

workCtx = ctx
workCtx = ctx //nolint:fatcontext

go func() {
started()
Expand Down

0 comments on commit 2b12a3a

Please sign in to comment.