Skip to content

cargo doc is not in the gate, so a deleted item can leave broken intra-doc links behind #388

Description

@wormeyman

Deleting a documented Rust item leaves broken rustdoc links behind, and nothing in the gate can see them. This was found the expensive way in #387: it shipped two of them through a fully green pnpm run verify and a fully green CI.

Why the gate is blind

rustdoc::broken_intra_doc_links is a rustdoc lint. It is not a rustc lint and not a clippy lint, so cargo clippy -D warnings never fires on it. scripts/verify-rust.sh runs cargo fmt --check, clippy, cargo test, the zero-dependency assertion, the engine.wasm byte comparison and cargo deny - and no cargo doc at all. So the only tool that reports the class is one the repo never runs.

In #387 that meant deleting cliff_slider_to_linear left the module doc above it saying the function "now lives here too" and linking to it twice. Both the prose and the links were wrong. verify:rust was green locally at RUST_RC=0, and the rust job was green on the runner.

CodeRabbit is what caught it, and only indirectly - it asked for a deprecated forwarding wrapper, which is wrong for an unpublished internal crate with a refuted implementation. The finding was declined; the adjacent real defect was fixed.

What is there now

cargo doc --no-deps -p fmw-noise 2>&1 | grep -c 'unresolved link'

main reports 9, in two groups. I first wrote that all nine were the same shape and that was wrong, so here is each one.

Seven are links to test functions, which rustdoc cannot resolve in a normal build because the items are #[cfg(test)]. Five spell the module path and two do not:

  • cliffs/catalog.rs:42 - tests::the_rotbb_derivation_reproduces_every_shipped_box
  • cliffs/connections.rs:39 - tests::a_cell_can_lose_both_ends_in_one_pass
  • cliffs/connections.rs:86 - tests::the_end_table_is_what_the_orientation_names_say
  • cliffs/vulcanus_fields.rs:105 - tests::the_richness_term_is_exactly_zero_at_vulcanuss_pinned_richness
  • expressions/vulcanus_resources.rs:623 - tests::the_ore_region_projection_agrees_with_the_full_eval_bit_for_bit
  • resources/fulgora_catalog.rs:23 - places_a_strict_nonempty_subset_of_the_footprint (bare)
  • resources/fulgora_catalog.rs:57 - the_collision_box_cannot_reject_anything (bare)

Two are not test links at all, and both point at items that really exist and are simply addressed wrongly:

  • voronoi_noise.rs:20 links [delta_to] unqualified from a module doc. It is a private associated function at voronoi_noise.rs:611, called as Self::delta_to.
  • trees/field.rs:158 links [Self::eval_at] from a doc on cheap_from. eval_at is pub fn at trees/field.rs:277, so Self in that impl block is not the type that carries it.

That second pair is worth separating out, because it is a straight fix rather than a judgement call - and the trees/field.rs one sits on a comment about the four addends staying bit-identical, so a reader following it lands nowhere on a load-bearing claim.

The seven test links are the reason a cargo doc check cannot simply be switched on: they point at real, useful tests and the prose around them is worth keeping, so making them resolve is a rewrite rather than a deletion.

The decision this needs

Whether to make the class gateable at all, and at what cost:

  1. Rewrite the 9 so they resolve, then add cargo doc to verify:rust. Turns a whole defect class into an impossibility. Costs a pass over five files, plus whatever cargo doc adds to a job that is already 54% of the gate at ~112s.
  2. Add the check but scope it to new breakage, e.g. assert the count does not exceed 9 - the same ratchet shape fixtureProvenance.spec.ts uses for maxUnknown. Cheaper, and it would have caught Narrow slider_to_linear's bounds to f32 before using them (#324) #387 (11 > 9).
  3. Leave it. Accept that a deleted documented item can take its docs' correctness with it silently.

Option 2 looks like the best value, and it matches a pattern this repo already trusts, but the numbers above are the whole input - no work has been done on any of them.

Not urgent

No user-visible behaviour depends on this. cargo doc is not published anywhere; the cost is that a reader of the source is told something false, which is exactly what #387's module doc did until it was rewritten.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions