Skip to content

Clear the helper-crate lint census - #805

Draft
leynos wants to merge 2 commits into
harden-lint-self-named-modules-20261022from
harden-lint-census-completion-20261022
Draft

leynos wants to merge 2 commits into
harden-lint-self-named-modules-20261022from
harden-lint-census-completion-20261022

Conversation

@leynos

@leynos leynos commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner

Summary

Completes the source-remediation half of the lint train. This layer sits on top
of #798 and clears the two remaining bodies of findings: the core crate's
missing_const_for_fn / idiom census, and the helper crates'
wireframe_testing and wireframe-verification censuses.

It does not adopt the final lint tables. It makes the source clean enough that
adopting them cannot produce a source avalanche — which is the point of the
ordering.

Position in the stack

main (b194531)
  └─ #795 codec/shared      6036697
      └─ #796 app/support   13d33c7
          └─ #797 examples  0cf5a8c
              └─ #798 self-named modules  d493667
                  └─ this PR                4515f8f

Review only the two commits below; everything above is #795-#798. This PR's
base is harden-lint-self-named-modules-20261022, the #798 branch, so the diff
it shows is exactly those two commits (61 files, +258/−230). It merges after
#798 into main; it is not independently mergeable.

What changed

39a548f — core crate idiom census. 57 signatures in src/ gain
const fn, taking that directory from 228 to 285, including the five helpers
the cascade reaches (FairnessTracker::reset, clear, set_config,
ConnectionState::set_fairness). The rest are small behaviour-preserving fixes:
ok_or with an eager allocation becomes ok_or_else; if let/else selection
becomes map_or/map_or_else in the codec EOF builder, scheduler waiter lookup,
panic formatter and connection registry; shadowed bindings are renamed to say
what they hold; &str::to_string() becomes to_owned(); Direction patterns
use Self::; Option<Arc<ServerError>> uses Arc<Self>.

4515f8f — helper-crate census. The wireframe_testing and
wireframe-verification findings, plus the self_named_module_files moves that
turn measurement.rs into measurement/mod.rs.

Across the two commits, 79 const fn signatures are added and none removed.

No #[allow] is added and no suppression is introduced: git diff over this
range adds zero #[allow] and zero #[expect].

The stack as a whole also reduces suppression. Counting every #[expect] in
the tree, origin/main has 70 and 4515f8f has 60; the ten removed are
multi-line #[expect( attributes replaced by source that no longer needs them.
The remaining suppressions are pre-existing policy exceptions (deprecated
compatibility helpers, poisoned-lock unwrap_used) and are untouched here.

What is deliberately still outstanding

Two files retain unsafe code, and they are the only unsafe code in the tree:

  • wireframe_testing/src/benchmark_alloc_support.rs
  • benches/codec_performance_alloc.rs

Both implement unsafe impl GlobalAlloc for a counting allocator. That is
unrepairable by rewriting: GlobalAlloc and the System::alloc calls inside it
are unsafe by definition, and unsafe_code = "forbid" admits no exemption
(the #[expect] is rejected with E0453). The other nine prose occurrences of
the word "unsafe" in the tree are documentation.

This is escalated rather than suppressed: see #804, which recommends isolating
the impl in a small audited boundary crate, forbidding unsafe in every other
Cargo target root, and covering the boundary with Miri (which does honour a
custom global allocator). #804 also records an interim shape that reaches
forbid on 89 of 91 target roots immediately.

Reviewer entrypoints

  • 39a548f for the core crate; start with src/fairness.rs and
    src/client/pool/ for the const cascade, which is the only part with
    transitive effects.
  • 4515f8f for the helper crates.
  • The two self_named_module_files moves are pure file moves plus one import
    path each.

The renames that matter for review correctness are prepared_shutdown,
prepared_app, configured_pacing, io_error, codec_error,
protocol_error, total_body_len, expected_sequence and paced_writer.
Each replaced a shadowed binding that reassigned a value of a different type or
meaning to an existing name.

Onboarding disposition

None of the five train onboardings is implemented or claimed here. The dev-fast
build standard, CV-005, the Markdown baseline, the shared Whitaker installer and
the tidied typos-config-builder all belong to the configuration and canon
layers. This layer only touches Rust source, and the helper crates change no
Make target, workflow or manifest.

Validation evidence

Restacked onto origin/main = b194531, which moved three commits since the
stack was first published, including #794's toolchain bump to
nightly-2026-10-22. All five layers were replayed with the user-wide weave
merge driver disabled; each rebase was verified against a driver-free
merge-tree replay rather than trusted.

Two conflicts in this layer, both caused by #794 adopting a new API where this
layer had rewritten the same expression:

  • src/client/pool/lease.rs — #794 renamed fetch_update to try_update.
    Resolution keeps this layer's intent (the update always yields Some, so the
    discarded result carries an explaining comment) on the new name.
  • src/server/runtime/startup.rs — the new toolchain no longer fires
    integer_division_remainder_used inside tokio::select!, so #794 deleted the
    #[expect] this layer had been preserving. Resolution keeps the rename and
    drops the now-dead #[expect].

Both resolutions remove code rather than add it.

The local gate sequence is running on 4515f8f on nightly-2026-10-22. This
section will carry its results before the PR leaves draft. Because this head
contains all five layers, its result is the evidence for the whole stack, and
the four PRs below point here rather than repeating a per-layer run.

Hosted, at this head. build-test ran at 4515f8f and every one of its 17
substantive steps passed: Format, Install Whitaker, Lint, Lint Markdown,
Spelling, Mermaid, workflow contracts, feature-gate compile checks, and Test
and Measure Coverage.

https://github.com/leynos/wireframe/actions/runs/39974035999/job/120483866980

The instrumented run executed 1207 tests, 1207 passed, 0 skipped, and the
ratchet reported 87.85% current against the committed 87.8% baseline — an
improvement of 0.05 pp, read from the job log rather than inferred from the
job conclusion.

That run is still narrower than the Make targets: cargo llvm-cov nextest --workspace takes default features and default targets, so there is no
--all-targets, no --all-features, and no doctest pass. The local sequence
below closes that gap.

Known baseline failures

None known at this head. The census lives in the companion ledger.

Deferred work

References

🤖 Generated with Claude Code

Summary by Sourcery

Clean the core and helper crates’ remaining lint census findings so the stricter lint baselines can be adopted without source churn.

Enhancements:

  • Remediate the remaining core and helper-crate lint findings by making eligible APIs const, applying idiomatic Rust improvements, and clarifying shadowed bindings and ownership-related expressions.
  • Reorganize the push benchmark measurement module to satisfy module-layout linting without changing behavior.

Chores:

  • Leave the remaining allocator-related unsafe code for a separate boundary-extraction effort without adding lint suppressions.

leynos and others added 2 commits October 23, 2026 15:14
Measure the workspace under the proposed final lint configuration and fix
every finding the core crate reports, so the configuration layer can be
adopted without a source avalanche behind it.

The bulk is `missing_const_for_fn`: 50 signatures gain `const`, plus the
five helpers they transitively call. Making a mutation helper `const`
required its callees (`FairnessTracker::reset`, `clear`, `set_config` and
`ConnectionState::set_fairness`) to become `const` too; the cascade stops
at `after_low`, which only calls into `reset`.

The rest are small, behaviour-preserving idiom fixes:

- `ok_or` with an eager `format!`-style allocation becomes `ok_or_else`.
- `if let`/`else` selection becomes `Option::map_or`/`map_or_else` in the
  codec EOF builder, the scheduler waiter lookup, the panic formatter and
  the connection registry retain closure.
- Shadowed bindings are renamed to say what they hold: `io_error`,
  `codec_error`, `protocol_error`, `rejected`, `prepared_shutdown`,
  `prepared_app`, `configured_pacing`, `total_body_len`,
  `expected_sequence`, `paced_writer`.
- `&str::to_string()` becomes `to_owned()`, `Direction` patterns use
  `Self::`, and `Option<Arc<ServerError>>` uses `Arc<Self>`.
- The `fetch_update` discard becomes `.ok()`, and the debug assertion in
  the supervisor observer gains a message.

`src/testkit/slow_io.rs` and `src/testkit/support.rs` needed their use
sites updated alongside the renames: the validated configuration and the
paced writer handles are now driven through the names that actually hold
the values.

Co-Authored-By: Claude Code <noreply@anthropic.com>
The core crate was already clean under the proposed final Clippy table,
but the helper crates still carried 72 findings, so the workspace as a
whole could not yet adopt it.

`wireframe_testing` and `wireframe-verification` now report zero findings
against the same strict configuration. Most were mechanical: `const fn`
where the body already qualified, `to_string` on a `&str` becomes
`to_owned`, and `option_if_let_else` rewrites that read as `let`-`else`
rather than `unwrap_or_else` because the success arm is itself a
`Result`.

The remainder were shadowed bindings. Each is renamed to say what the new
value actually is (`resolved_capacity`, `declared_len`,
`contender_barrier`, `startup_handle`) so the reader can tell the
converted value from the parameter it came from. `record.args()` returns
`&str`, so the log assertions no longer allocate a `String` per record to
call `contains`.

`push_benchmark_support/measurement.rs` moves to `measurement/mod.rs` to
satisfy `self_named_module_files`; the sibling `measurement/` directory
already held its submodules.

The two `unsafe impl GlobalAlloc` blocks remain. `GlobalAlloc` members are
intrinsically unsafe and `unsafe_code = "forbid"` cannot be escaped by
`#[expect]` (E0453), so that conflict belongs to the lint-configuration
layer, not here.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@sourcery-ai

sourcery-ai Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

This PR completes source remediation for the core and helper-crate lint censuses: it adds the eligible const APIs, rewrites idiom and conversion cases to eliminate suppressions, restructures helper modules and tests, and introduces injectable environment lookup while intentionally leaving only the documented GlobalAlloc unsafe boundary for follow-up.

Sequence diagram for injectable SERVER_ADDR lookup

sequenceDiagram
    participant Run as run
    participant Env as DefaultEnv
    participant Parser as parse_server_addr
    participant Server as WireframeServer

    Run->>Env: raw
    Env-->>Run: Result<String, VarError>
    Run->>Parser: parse_server_addr(addr)
    alt lookup fails
        Parser-->>Run: DEFAULT_ADDR
    else configured address
        Parser-->>Run: SocketAddr
    end
    Run->>Server: bind(SocketAddr)
Loading

Flow diagram for checked network-prefix encoding

flowchart TD
    Input["u64_to_bytes"] --> Convert["convert_len_to_value"]
    Convert --> Endian{"Endianness"}
    Endian --> Big["write_big_endian_prefix"]
    Endian --> Little["write_little_endian_prefix"]
    Big --> Fits["ensure_fits_prefix"]
    Fits -->|fits| Encode["write_network_u64"]
    Fits -->|too wide| Error["ERR_FRAME_TOO_LARGE"]
    Encode --> Output["prefix bytes"]
    Little --> Output
Loading

File-Level Changes

Change Details Files
Remediate the core crate’s idiom and const-function lint census without changing intended behavior.
  • Promote 57 eligible functions and methods to const fn, including fairness and client-pool cascade entrypoints.
  • Replace lint-triggering eager error construction, conditional selection, shadowed bindings, allocation spelling, enum patterns, and error-source typing with idiomatic equivalents.
  • Remove several lint suppressions by using checked conversions and bytes::Buf/BufMut for endian encoding and decoding, with expanded boundary and property tests.
  • Preserve the updated module layout and API rename resolutions from the rebased stack, including try_update and the removed dead expectation.
src/fairness.rs
src/client/pool/
src/codec/
src/byte_order.rs
src/frame/conversion.rs
src/connection/
src/app/
src/message_assembler/
src/middleware.rs
src/request/
src/response.rs
src/session.rs
src/server/
src/test_helpers/
src/testkit/
src/error.rs
src/extractor/
src/metrics/
src/push/queues/
src/rewind_stream.rs
src/panic.rs
examples/
Cargo.toml
Cargo.lock
Clear the helper-crate lint census while keeping test and benchmark behavior explicit and preserving the unavoidable allocator boundary.
  • Promote eligible helper and verification APIs to const fn and clean remaining naming, allocation, match, and ignored-result findings.
  • Refactor packet-enum address parsing to accept an injected environment lookup and add tests for missing, non-Unicode, valid, and malformed values without mutating process state.
  • Rename measurement.rs files to measurement/mod.rs and update module paths for self-named-module compliance.
  • Improve teardown and benchmark diagnostics by explicitly discarding expected results, naming task handles and resolved values, and retaining failure reporting.
wireframe_testing/src/
crates/wireframe-verification/src/
examples/packet_enum.rs
wireframe_testing/src/runtime_ownership_benchmarks/push_benchmark_support/measurement/mod.rs
Add the helper dependency required for deterministic environment access in the packet-enum example.
  • Add mockable as a workspace development dependency and use its default environment provider in production code.
Cargo.toml
Cargo.lock

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant