fix(lint): clear the 14 findings ziglint reported on its first real run - #18
Merged
Merged
Conversation
`zig build lint` had never executed anywhere — it shells out to a binary nothing installed — so the findings had been accumulating unseen. With the tool now in the flake, it reports 14. 11 are Z030: deinit frees the struct's memory but leaves the struct readable, so a later use returns plausible bytes instead of failing. `self.* = undefined` poisons it, which in ReleaseSafe turns use-after-deinit into a loud crash. The GraphqlClient Response case is the one that matters: every parsed field is a slice into `parsed`'s arena, so any read after deinit was already use-after-free. bulk.Targets was resetting `items`/`storage` by hand at the end of deinit. It is only ever reached through `defer` at scope exit, so nothing could observe those resets — poisoning replaces them rather than adding to them. The rest: one anonymous struct binding `@This()` inline (now `const Self = @this()`), and `return Error.RequestTimedOut` -> `return error.RequestTimedOut`. ziglint flagged two of the three occurrences; the third is changed too, because leaving one behind would be the only reason a reader would think the two forms differ. They do not — Zig error sets are global, so both name the same error value. `pub const Error` stays as the module's declared error contract. CI gains a `lint` job running through the flake dev shell. A finding the harness should have caught earns a floor, not just a patch — without it this drifts straight back, which is how it got to 14. Verified: `zig build test` passes, `zig build lint` reports 0 findings, `zig fmt --check` clean.
alleneubank
force-pushed
the
fix/ziglint-findings
branch
from
August 21, 2026 17:02
e6e0bd8 to
dd375aa
Compare
Merged
alleneubank
added a commit
that referenced
this pull request
Aug 21, 2026
Bumps the four manifests. 0.3.0 shipped to GitHub but never reached npm (the release token had expired), so this is the first tag that exercises trusted publishing end to end and the first that gets npm off 0.2.11. Content is 0.3.0 plus the ziglint fixes from #18.
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
1 participant
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.
zig build linthad never executed anywhere — it shells out to a binary nothing installed — so findings had been accumulating unseen. With ziglint now in the flake (#17), it reports 14. This clears them and adds the floor that keeps them cleared.The findings
11x Z030 —
deinitfrees the struct's memory but leaves the struct readable, so a later use returns plausible bytes instead of failing.self.* = undefinedpoisons it, which in ReleaseSafe turns use-after-deinit into a loud crash.The
GraphqlClient.Responsecase is the one that matters: every parsed field is a slice intoparsed's arena, so any read afterdeinitwas already use-after-free. It now fails loudly instead.bulk.Targetswas resettingitems/storageby hand at the end ofdeinit. It is only ever reached throughdeferat scope exit, so nothing could observe those resets — poisoning replaces them rather than adding to them.1x Z020 — an anonymous struct bound
@This()inline; nowconst Self = @This().2x Z010 —
return Error.RequestTimedOutwhere the inferred error set already covers it; nowreturn error.RequestTimedOut. Note ziglint flagged 2 of the 3 occurrences; the third is fixed too for consistency.The floor
CI gains a
lintjob runningnix develop -c zig build lint. A finding the harness should have caught earns a floor, not just a patch — without one this drifts straight back, which is how it reached 14 in the first place.Verification
zig build test— passeszig build lint— 0 findings (was 14)zig fmt --check— clean