bump mordant and turn on the generic body lint - #41082
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. WalkthroughThe workspace now pins a newer Mordant revision and enables the ChangesLint configuration and diagnostics
Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change updates the lint dependency, enables the generic body lint, and refreshes its baseline; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Full details: Description checkExplanation The description clearly explains the dependency update, lint activation, and baseline regeneration. It does not include the required verification section or test results, but it is otherwise relevant and sufficiently complete. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — tooling-only bump with a regenerated baseline.
What was reviewed:
Cargo.toml: mordant rev bump stays a full git SHA pin; grepped the repo for the old rev — no other references to update.dylint.toml: newgeneric-body-not-generic-enabledkey sits alongside existing mordant config keys;disabledlist untouched.mordant-baseline.toml: net reduction ingeneric_body_not_genericcounts; the handful of new/increased entries (h2_frame_parser, create_command, Blob, Sink) are baseline grandfathering of existing code — no source files changed in this PR, so nothing newly introduced is being suppressed.
Extended reasoning...
Overview
This PR bumps the mordant dylint lint pack pinned in [workspace.metadata.dylint] from rev 7c5f269… to 8f0803f…, adds generic-body-not-generic-enabled = true to dylint.toml to opt into the now-opt-in lint, and regenerates mordant-baseline.toml per the documented bun run rust:mordant:baseline workflow. No .rs, .cpp, or .ts source is touched — this is purely lint-tooling configuration.
Security risks
None. The change affects only a dev-time linter dependency (fetched by cargo dylint, not linked into the shipped binary) and a suppression baseline consumed by CI. Nothing here reaches runtime code paths, user input handling, auth, crypto, or network code.
Level of scrutiny
Low. This matches the "config tweaks / version bumps" category the approval guidelines call out as approvable without human review. The pattern is established in the repo: dylint.toml already carries mordant-specific keys and a comment documenting exactly this regeneration flow. The rev remains a full 40-char SHA (not a branch or tag), satisfying the REVIEW.md dependencies convention of pinning to an immutable commit. I grepped for the old rev across the tree — no duplicates left behind.
Other factors
The baseline diff is a net reduction (~30 fewer entries), consistent with the PR description that the upstream change tightened the lint. The few new/increased entries baseline pre-existing code only — since no source files changed in this diff, they cannot be masking freshly introduced violations; the ratchet still fails any PR that adds one on top. No CODEOWNERS entries cover the three touched files, and the PR timeline shows no prior reviewer objections. No tests are expected for a lint-config change.
…41130) ### Problem - The `mordant` job fails on every PR since 2026-09-01: one `generic_body_not_generic` finding over the baseline, named as `src/runtime/server/RequestContext.rs:4439` (`get_remote_socket_info`). The baseline holds 8, the file has 9. Mordant names the last finding in the file. - The new one is the block #41080 added to `finalize_without_deinit`, a copy of the block in `handle_reject_stream`. It uses no type parameter and is compiled eight times. #41082 wrote the baseline ten minutes before #41080 merged. ### Fix - Move the block into `release_body_stream`, a free `#[inline(never)]` function called from both places. One copy instead of sixteen. - No behavior change: the statements and their order are the same. - The file drops from nine findings to seven, so `mordant-baseline.toml` goes from 8 to 7. Checked with the pinned mordant: 7 reports nothing over, 6 reports one over. - Verified: seven stream, abort and leak serve tests (85 pass, listed in Notes), `serve.test.ts`, and the `mordant` job on this PR. ### Background - `RequestContext<ThisServer, SSL, DEBUG, MUX>` is the per-request state of `Bun.serve`. It has eight monomorphizations. Every method body is compiled once per instantiation. - `generic_body_not_generic` is a mordant lint. It flags a region of a generic function that uses no type parameter (30 MIR statements here) and asks for a non-generic function that takes what the region reads. - `mordant-baseline.toml` is a ratchet: per lint and file, the number of findings that predate the job. A file over its count fails the job. A fixed finding lowers the entry. - #40423 carries f0b5a3b, which hoists `get_remote_socket_info` itself. It touches other lines, so both can land. <details><summary>Notes</summary> No `test/` change. The diff moves two identical blocks into one function and changes no statement, so no test can fail before it and pass after it. The regression check for this PR is the `mordant` job itself: red on main and on every open PR, green here. The behavior of the moved block is pinned by the existing tests listed below, in particular `serve-pending-promise-abort-leak.test.ts` (#41080, the `finalize_without_deinit` caller) and `serve-stream-reject-flush-leak.test.ts` (the `handle_reject_stream` caller). Tests run with the debug build: `test/js/bun/http/serve-pending-promise-abort-leak.test.ts`, `serve-stream-reject-flush-leak.test.ts`, `serve-async-stream-client-abort.test.ts`, `serve-response-stream-sink-leak.test.ts`, `serve-stream-body-error.test.ts`, `serve-error-handler-stream.test.ts`, `serve-body-leak.test.ts`: 85 pass. `serve.test.ts` on this container: 295 pass, 2 fail (`root range port` and `/bun:info to loopback clients`). Both fail on main here too; #41080 noted the same two. How the cause was found: with the pinned mordant (`cargo dylint --all -p bun_runtime`), reverting only the `RequestContext.rs` hunk of e5a18d5 leaves nothing over the baseline. With the baseline line commented out and `--cap-lints warn`, the nine findings in the file are `cancel_unread_body`, `render_missing_invalid_response`, `finalize_without_deinit` (30 of 373 statements, the #41080 block), `do_sendfile`, `handle_reject_stream` (30 of 246 statements, the same block), `render_metadata`, `do_write_status`, `on_buffered_body_chunk` and `get_remote_socket_info`. Mordant reports the findings past the baseline count in file order, which is why the job names the last one. With this PR and f0b5a3b both on main, the file has six findings under a baseline of seven. </details>
1 participant
Updates the mordant pin to the version where generic_body_not_generic is opt-in, enables it in dylint.toml, and regenerates the baseline (134 findings).