test: make nil-guard branches terminate to satisfy staticcheck SA5011 - #534
Merged
Merged
Conversation
The release-path golangci-lint (running on the go1.26.5 toolchain) flags staticcheck SA5011 (possible nil pointer dereference) at test sites that guard a pointer with 'if x == nil { t.Fatal(...) }' and then dereference it: staticcheck does not always treat t.Fatal as terminating, so it sees the nil branch as able to fall through. Add an explicit return as the last statement of each such guard so the nil path provably terminates. The return is unreachable at runtime because t.Fatal already exits the goroutine, so no test behavior changes; the edit is purely additive and only removes the lint ambiguity. Fixes the three flagged sites and every sibling with the same idiom so a later cut does not surface a new one.
Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The nightly release's Orchestrate validation failed on the release-path golangci-lint (v2.10.1 on the go1.26.5 toolchain): staticcheck SA5011 (possible nil pointer dereference) at test sites using the idiom
if x == nil { t.Fatal(...) }followed by a deref ofx. staticcheck does not always treatt.Fatalas terminating, so it sees the nil branch as able to fall through. This blocked the release-candidate from being cut, so the fleet never ran. (It surfaced only on the release path, a golangci-lint-action toolchain/cache sensitivity; the PR-gate lint and a locally built linter do not reproduce it.)Fix
Add an explicit
returnas the last statement of each such nil guard so the nil path provably terminates. Thereturnis unreachable at runtime becauset.Fatalalready exits the goroutine, so no test behavior changes. Applied to the three flagged sites plus every sibling with the same idiom so a later cut does not surface a new one.Verification
Purely additive: 59
returninsertions, 0 deletions, across 22 test files.go build ./...andgo test ./...(2661 pass) clean; a golangci-lint v2.10.1 built with go1.26.5 reports 0 issues (no SA5011). No behavior, assertion, or format change.