Skip to content

fix(lint): work around nolintlint false positives from cache bug - #10136

Merged
mnencia merged 1 commit into
mainfrom
dev/fix-nolintlint-false-positives
Mar 5, 2026
Merged

mnencia merged 1 commit into
mainfrom
dev/fix-nolintlint-false-positives

Conversation

@mnencia

@mnencia mnencia commented Mar 4, 2026

Copy link
Copy Markdown
Member

Set allow-unused: true for nolintlint to prevent flaky CI failures caused by a known golangci-lint bug (golangci/golangci-lint#3228) where the internal cache loses staticcheck SA1019 deprecation facts across runs, causing nolintlint to falsely report //nolint:staticcheck directives as unused.

@mnencia
mnencia requested a review from a team March 4, 2026 18:07
@dosubot dosubot Bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Mar 4, 2026
@cnpg-bot cnpg-bot added backport-requested ◀️ This pull request should be backported to all supported releases release-1.25 release-1.28 labels Mar 4, 2026
@github-actions

github-actions Bot commented Mar 4, 2026

Copy link
Copy Markdown
Contributor

❗ By default, the pull request is configured to backport to all release branches.

  • To stop backporting this pr, remove the label: backport-requested ◀️ or add the label 'do not backport'
  • To stop backporting this pr to a certain release branch, remove the specific branch label: release-x.y

@dosubot dosubot Bot added the chore Intangible work to reduce technical debt label Mar 4, 2026
@mnencia mnencia added no-issue and removed chore Intangible work to reduce technical debt labels Mar 4, 2026
Set allow-unused: true for nolintlint to prevent flaky CI failures
caused by golangci-lint's internal cache losing staticcheck SA1019
deprecation facts across runs.

Signed-off-by: Marco Nenciarini <[email protected]>
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Mar 5, 2026
@mnencia

mnencia commented Mar 5, 2026

Copy link
Copy Markdown
Member Author

/ok-to-merge

@cnpg-bot cnpg-bot added the ok to merge 👌 This PR can be merged label Mar 5, 2026
@mnencia
mnencia merged commit e7d6db3 into main Mar 5, 2026
55 of 58 checks passed
@mnencia
mnencia deleted the dev/fix-nolintlint-false-positives branch March 5, 2026 09:50
cnpg-bot pushed a commit that referenced this pull request Mar 5, 2026
)

Set allow-unused: true for nolintlint to prevent flaky CI failures
caused by a known golangci-lint bug
(golangci/golangci-lint#3228) where the
internal cache loses staticcheck SA1019 deprecation facts across runs,
causing nolintlint to falsely report `//nolint:staticcheck` directives
as unused.

Signed-off-by: Marco Nenciarini <[email protected]>
(cherry picked from commit e7d6db3)
cnpg-bot pushed a commit that referenced this pull request Mar 5, 2026
)

Set allow-unused: true for nolintlint to prevent flaky CI failures
caused by a known golangci-lint bug
(golangci/golangci-lint#3228) where the
internal cache loses staticcheck SA1019 deprecation facts across runs,
causing nolintlint to falsely report `//nolint:staticcheck` directives
as unused.

Signed-off-by: Marco Nenciarini <[email protected]>
(cherry picked from commit e7d6db3)
cnpg-bot pushed a commit that referenced this pull request Mar 5, 2026
)

Set allow-unused: true for nolintlint to prevent flaky CI failures
caused by a known golangci-lint bug
(golangci/golangci-lint#3228) where the
internal cache loses staticcheck SA1019 deprecation facts across runs,
causing nolintlint to falsely report `//nolint:staticcheck` directives
as unused.

Signed-off-by: Marco Nenciarini <[email protected]>
(cherry picked from commit e7d6db3)
clement0010 added a commit to Twingate/gateway that referenced this pull request May 21, 2026
…cache bug (#298)

Issue: #300

## Changes

- Set `allow-unused: true` for `nolintlint` in `.golangci.yml` to
prevent flaky CI failures

## Notes

### Problem

The `lint-go` CI job intermittently fails with errors like:

```
directive `//nolint:ireturn` is unused for linter "ireturn" (nolintlint)
```

**Evidence**: [Failed CI
run](https://github.com/Twingate/gateway/actions/runs/26206509968/job/77107441411?pr=296)

### Root Cause

This is a [known golangci-lint
bug](golangci/golangci-lint#3228) where the
internal linter cache loses analysis facts (in our case, `ireturn`
violations) across runs. When this happens:

1. `ireturn` violations become invisible due to stale/partial cache
2. `nolintlint` checks whether each `//nolint` directive suppresses a
real violation
3. With the cached facts missing, `nolintlint` concludes the directive
is "unused" and flags it
4. With `--fix` enabled (as in `make lint`), these "unused" directives
get silently deleted
5. The next clean run sees the real violations again and fails

The bug is flaky — it depends on which cache entries are evicted, so it
affects a different subset of directives each time. It is more likely in
CI where the GitHub Actions cache is restored from previous runs against
different code states.

### Fix

Following the same [workaround adopted by
cloudnative-pg](cloudnative-pg/cloudnative-pg#10136),
we set `allow-unused: true` for `nolintlint` to tolerate potentially
"unused" nolint directives rather than falsely flagging them.

Co-authored-by: Claude Opus 4.6 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-requested ◀️ This pull request should be backported to all supported releases lgtm This PR has been approved by a maintainer no-issue ok to merge 👌 This PR can be merged release-1.28 size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants