Skip to content

chore(tools): Remove doc pass from cargo_check #933

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JeanMertz merged 1 commit into from
Aug 3, 2026
Merged

chore(tools): Remove doc pass from cargo_check #933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions .config/jp/tools/src/cargo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,7 @@ pub async fn run(ctx: Context, t: Tool) -> ToolResult {
let checksum_freshness = t.option_or("checksum_freshness", false);

match t.name.trim_start_matches("cargo_") {
"check" => {
// Documentation lints fire on doc comments rather than on code, so
// clippy never sees them. Checking them here keeps them from
// reaching CI, at the cost of a `cargo doc` pass; set
// `options.docs = false` to trade that back.
let docs = t.option_or("docs", true);
cargo_check(&ctx, t.opt("package")?, checksum_freshness, docs).await
}
"check" => cargo_check(&ctx, t.opt("package")?, checksum_freshness).await,
"expand" => cargo_expand(&ctx, t.req("item")?, t.opt("package")?, checksum_freshness).await,
"test" => {
cargo_test(
Expand Down
128 changes: 5 additions & 123 deletions .config/jp/tools/src/cargo/check.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,32 +9,15 @@ use crate::util::{
truncate,
};

/// Rustdoc lints that are denied, kept in lockstep with `just docs-ci`.
///
/// These fire on documentation content rather than on code, so `cargo clippy`
/// and `cargo check` never see them and they otherwise surface only on CI.
const RUSTDOC_LINTS: &[&str] = &[
"-D rustdoc::broken-intra-doc-links",
"-D rustdoc::private-intra-doc-links",
"-D rustdoc::invalid-codeblock-attributes",
"-D rustdoc::invalid-html-tags",
"-D rustdoc::invalid-rust-codeblocks",
"-D rustdoc::bare-urls",
"-D rustdoc::unescaped-backticks",
"-D rustdoc::redundant-explicit-links",
];

pub(crate) async fn cargo_check(
ctx: &Context,
package: Option<String>,
checksum_freshness: bool,
docs: bool,
) -> ToolResult {
cargo_check_impl(
ctx,
package.as_deref(),
checksum_freshness,
docs,
&DuctProcessRunner,
)
}
Expand All @@ -43,7 +26,6 @@ fn cargo_check_impl<R: ProcessRunner>(
ctx: &Context,
package: Option<&str>,
checksum_freshness: bool,
docs: bool,
runner: &R,
) -> ToolResult {
let clippy_scope = package.map_or("--workspace".to_owned(), |v| format!("--package={v}"));
Expand Down Expand Up @@ -85,18 +67,6 @@ fn cargo_check_impl<R: ProcessRunner>(
let clippy = strip_ansi_escapes::strip_str(stderr);
let clippy = truncate(clippy.trim(), MAX_DIAGNOSTIC_BYTES);

let doc_note = match doc_check(ctx, package, checksum_freshness, docs, runner)? {
DocCheck::Skipped | DocCheck::Clean => None,
// Deliberately silent on *why* it failed: exit 101 also covers cargo
// errors, `cfg(doc)` compile errors and rustdoc crashes, and nothing in
// the exit status distinguishes those from a denied lint. The
// diagnostics below say which it was.
DocCheck::Failed(diagnostics) => Some(format!(
"`cargo doc` failed. This pass runs the documentation lints CI denies (`just \
docs-ci`), which clippy does not report:\n\n```\n{diagnostics}\n```"
)),
};

let comfort_note = match comfort_check(ctx, package, runner)? {
ComfortCheck::Clean => None,
ComfortCheck::Drift(note) => Some(note),
Expand All @@ -108,113 +78,25 @@ fn cargo_check_impl<R: ProcessRunner>(
}
};

// Hardest-to-ignore first: a failed doc pass blocks CI, comfort drift is
// auto-fixable.
let extra: Vec<String> = doc_note.into_iter().chain(comfort_note).collect();

if extra.is_empty() {
let Some(note) = comfort_note else {
return Ok(if clippy.is_empty() {
"Check succeeded. No warnings or errors found."
.to_owned()
.into()
} else {
format!("```\n{clippy}\n```\n").into()
});
}
};

// Something below failed, so the header is scoped to what clippy alone
// found. A bare "Check succeeded" would contradict the sections that follow.
// The header is scoped to what clippy alone found. A bare "Check succeeded"
// would contradict the drift note that follows.
let header = if clippy.is_empty() {
"`cargo clippy` found no warnings or errors.".to_owned()
} else {
format!("```\n{clippy}\n```")
};

let mut sections = vec![header];
sections.extend(extra);
Ok(sections.join("\n\n").into())
}

enum DocCheck {
/// The caller opted out of the documentation pass.
Skipped,
/// `cargo doc` succeeded, so no denied lint fired.
Clean,
/// `cargo doc` exited non-zero; carries whatever it reported.
///
/// A denied lint is the expected cause, but the exit status alone cannot
/// rule out a cargo error, a `cfg(doc)` compile error or a rustdoc crash,
/// so this variant does not claim to know which.
Failed(String),
}

/// Run `cargo doc` with the rustdoc lints CI denies.
///
/// `--document-private-items` is required, not cosmetic: without it
/// `private-intra-doc-links` cannot fire at all, which is the lint that catches
/// a public doc comment linking to a private item.
///
/// Shares cargo's default profile and feature set with the clippy pass above,
/// rather than the `docs` profile CI runs under: that pass already holds the
/// profile's build lock (`target/<profile>/.cargo-lock`) and has already built
/// every dependency unit, so this leaves rustdoc over the workspace crates as
/// the only new work and adds no contention the same invocation wasn't causing
/// already.
/// Profile choice does not affect which documentation lints fire.
fn doc_check<R: ProcessRunner>(
ctx: &Context,
package: Option<&str>,
checksum_freshness: bool,
enabled: bool,
runner: &R,
) -> Result<DocCheck, std::io::Error> {
if !enabled {
return Ok(DocCheck::Skipped);
}

let scope = package.map_or("--workspace".to_owned(), |v| format!("--package={v}"));
let rustdocflags = RUSTDOC_LINTS.join(" ");

let mut env = vec![("RUSTDOCFLAGS", rustdocflags.as_str())];
if checksum_freshness {
env.push(("CARGO_UNSTABLE_CHECKSUM_FRESHNESS", "true"));
}

let ProcessOutput { stderr, status, .. } = runner.run_with_env(
"cargo",
&[
"doc",
"--color=never",
&scope,
"--quiet",
// `just docs-ci` documents every feature. Without this, code behind
// an optional feature is never compiled, so its doc comments go
// unlinted here and fail on CI instead.
"--all-features",
"--no-deps",
"--document-private-items",
// Report every crate's diagnostics in one pass rather than stopping
// at the first failing crate.
"--keep-going",
],
&ctx.root,
&env,
)?;

if status.is_success() {
return Ok(DocCheck::Clean);
}

let diagnostics = strip_ansi_escapes::strip_str(stderr);
let diagnostics = diagnostics.trim();

// An empty stderr leaves nothing to report but the status, which is still
// worth surfacing.
Ok(DocCheck::Failed(if diagnostics.is_empty() {
format!("`cargo doc` failed with exit status {status} and no diagnostics.")
} else {
truncate(diagnostics, MAX_DIAGNOSTIC_BYTES)
}))
Ok(format!("{header}\n\n{note}").into())
}

enum ComfortCheck {
Expand Down
Loading
Loading