Skip to content
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.

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

Don't scrape examples from library targets by default #11499

Merged
merged 2 commits into from
Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ fn make_failed_scrape_diagnostic(
"\
{top_line}
Try running with `--verbose` to see the error message.
If an example or library should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` or `[lib]` definition in {}",
If an example should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` definition in {}",
relative_manifest_path.display()
)
}
Expand Down
15 changes: 3 additions & 12 deletions src/cargo/ops/cargo_compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ use std::collections::{HashMap, HashSet};
use std::hash::{Hash, Hasher};
use std::sync::Arc;

use crate::core::compiler::rustdoc::RustdocScrapeExamples;
use crate::core::compiler::unit_dependencies::build_unit_dependencies;
use crate::core::compiler::unit_graph::{self, UnitDep, UnitGraph};
use crate::core::compiler::{standard_lib, CrateType, TargetInfo};
Expand Down Expand Up @@ -371,19 +370,11 @@ pub fn create_bcx<'a, 'cfg>(

let should_scrape = build_config.mode.is_doc() && config.cli_unstable().rustdoc_scrape_examples;
let mut scrape_units = if should_scrape {
let scrape_generator = UnitGenerator {
UnitGenerator {
mode: CompileMode::Docscrape,
..generator
};
let mut scrape_units = scrape_generator.generate_root_units()?;
scrape_units.retain(|unit| {
ws.unit_needs_doc_scrape(unit)
&& !matches!(
unit.target.doc_scrape_examples(),
RustdocScrapeExamples::Disabled
)
});
scrape_units
}
.generate_scrape_units(&units)?
} else {
Vec::new()
};
Expand Down
61 changes: 38 additions & 23 deletions src/cargo/ops/cargo_compile/unit_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ impl<'a> UnitGenerator<'a, '_> {
.iter()
.filter(|t| t.is_bin() || t.is_lib())
.collect(),
CompileMode::Doc { .. } | CompileMode::Docscrape => {
CompileMode::Doc { .. } => {
// `doc` does lib and bins (bin with same name as lib is skipped).
targets
.iter()
Expand All @@ -200,7 +200,7 @@ impl<'a> UnitGenerator<'a, '_> {
})
.collect()
}
CompileMode::Doctest | CompileMode::RunCustomBuild => {
CompileMode::Doctest | CompileMode::RunCustomBuild | CompileMode::Docscrape => {
panic!("Invalid mode {:?}", self.mode)
}
}
Expand Down Expand Up @@ -426,15 +426,11 @@ impl<'a> UnitGenerator<'a, '_> {
}
}

if self.mode.is_doc_scrape() {
self.add_docscrape_proposals(&mut proposals);
}

Ok(proposals)
}

/// Add additional targets from which to scrape examples for documentation
fn add_docscrape_proposals(&self, proposals: &mut Vec<Proposal<'a>>) {
/// Proposes targets from which to scrape examples for documentation
fn create_docscrape_proposals(&self, doc_units: &[Unit]) -> CargoResult<Vec<Proposal<'a>>> {
// In general, the goal is to scrape examples from (a) whatever targets
// the user is documenting, and (b) Example targets. However, if the user
// is documenting a library with dev-dependencies, those dev-deps are not
Expand All @@ -456,23 +452,30 @@ impl<'a> UnitGenerator<'a, '_> {
let reqs_dev_deps = matches!(self.has_dev_units, HasDevUnits::Yes);
let safe_to_scrape_example_targets = no_pkg_has_dev_deps || reqs_dev_deps;

let proposed_targets: HashSet<&Target> = proposals.iter().map(|p| p.target).collect();
let pkgs_to_scrape = doc_units
.iter()
.filter(|unit| self.ws.unit_needs_doc_scrape(unit))
.map(|u| &u.pkg)
.collect::<HashSet<_>>();

let can_scrape = |target: &Target| {
let not_redundant = !proposed_targets.contains(target);
not_redundant
&& match (target.doc_scrape_examples(), target.is_example()) {
// Targets configured by the user to not be scraped should never be scraped
(RustdocScrapeExamples::Disabled, _) => false,
// Targets configured by the user to be scraped should always be scraped
(RustdocScrapeExamples::Enabled, _) => true,
// Example targets with no configuration should be conditionally scraped if
// it's guaranteed not to break the build
(RustdocScrapeExamples::Unset, true) => safe_to_scrape_example_targets,
// All other targets are ignored for now. This may change in the future!
(RustdocScrapeExamples::Unset, false) => false,
}
match (target.doc_scrape_examples(), target.is_example()) {
// Targets configured by the user to not be scraped should never be scraped
(RustdocScrapeExamples::Disabled, _) => false,
// Targets configured by the user to be scraped should always be scraped
(RustdocScrapeExamples::Enabled, _) => true,
// Example targets with no configuration should be conditionally scraped if
// it's guaranteed not to break the build
(RustdocScrapeExamples::Unset, true) => safe_to_scrape_example_targets,
// All other targets are ignored for now. This may change in the future!
(RustdocScrapeExamples::Unset, false) => false,
}
};
proposals.extend(self.filter_targets(can_scrape, false, CompileMode::Docscrape));

let mut scrape_proposals = self.filter_targets(can_scrape, false, CompileMode::Docscrape);
scrape_proposals.retain(|proposal| pkgs_to_scrape.contains(proposal.pkg));

Ok(scrape_proposals)
}

/// Checks if the unit list is empty and the user has passed any combination of
Expand Down Expand Up @@ -674,4 +677,16 @@ impl<'a> UnitGenerator<'a, '_> {
let proposals = self.create_proposals()?;
self.proposals_to_units(proposals)
}

/// Generates units specfically for doc-scraping.
///
/// This requires a separate entrypoint from [`generate_root_units`] because it
/// takes the documented units as input.
///
/// [`generate_root_units`]: Self::generate_root_units
pub fn generate_scrape_units(&self, doc_units: &[Unit]) -> CargoResult<Vec<Unit>> {
let scrape_proposals = self.create_docscrape_proposals(&doc_units)?;
let scrape_units = self.proposals_to_units(scrape_proposals)?;
Ok(scrape_units)
}
}
25 changes: 13 additions & 12 deletions src/doc/src/reference/unstable.md
Original file line number Diff line number Diff line change
Expand Up @@ -1162,34 +1162,35 @@ like this:
cargo doc -Z unstable-options -Z rustdoc-scrape-examples
```

By default, Cargo will scrape from the packages that are being documented, as well as scrape from
examples for the packages (i.e. those corresponding to the `--examples` flag). You can individually
enable or disable targets from being scraped with the `doc-scrape-examples` flag, such as:
By default, Cargo will scrape examples from the example targets of packages being documented.
You can individually enable or disable targets from being scraped with the `doc-scrape-examples` flag, such as:

```toml
# Disable scraping examples from a library
# Enable scraping examples from a library
[lib]
doc-scrape-examples = false

# Enable scraping examples from a binary
[[bin]]
name = "my-bin"
doc-scrape-examples = true

# Disable scraping examples from an example target
[[example]]
name = "my-example"
doc-scrape-examples = false
```

**Note on tests:** enabling `doc-scrape-examples` on test targets will not currently have any effect. Scraping
examples from tests is a work-in-progress.

**Note on dev-dependencies:** documenting a library does not normally require the crate's dev-dependencies. However,
example units do require dev-deps. For backwards compatibility, `-Z rustdoc-scrape-examples` will *not* introduce a
example targets require dev-deps. For backwards compatibility, `-Z rustdoc-scrape-examples` will *not* introduce a
dev-deps requirement for `cargo doc`. Therefore examples will *not* be scraped from example targets under the
following conditions:

1. No target being documented requires dev-deps, AND
2. At least one crate being documented requires dev-deps, AND
3. The `doc-scrape-examples` parameter is unset for `[[example]]` targets.
2. At least one crate with targets being documented has dev-deps, AND
3. The `doc-scrape-examples` parameter is unset or false for all `[[example]]` targets.

If you want examples to be scraped from example targets, then you must not satisfy one of the above conditions.
For example, you can set `doc-scrape-examples` to true for one example target, and that signals to Cargo that
you are ok with dev-deps being build for `cargo doc`.


### check-cfg
Expand Down
95 changes: 41 additions & 54 deletions tests/testsuite/docscrape.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn basic() {

let doc_html = p.read_file("target/doc/foo/fn.foo.html");
assert!(doc_html.contains("Examples found in repository"));
assert!(doc_html.contains("More examples"));
assert!(!doc_html.contains("More examples"));

// Ensure that the reverse-dependency has its sources generated
assert!(p.build_dir().join("doc/src/ex/ex.rs.html").exists());
Expand Down Expand Up @@ -178,18 +178,25 @@ fn configure_target() {
authors = []

[lib]
doc-scrape-examples = false
doc-scrape-examples = true

[[bin]]
name = "a_bin"
doc-scrape-examples = true

[[example]]
name = "a"
doc-scrape-examples = false
"#,
)
.file(
"src/lib.rs",
"pub fn foo() {} fn lib_must_not_appear() { foo(); }",
"pub fn foo() {} fn lib_must_appear() { foo(); }",
)
.file(
"examples/a.rs",
"fn example_must_not_appear() { foo::foo(); }",
)
.file("examples/a.rs", "fn example_must_appear() { foo::foo(); }")
.file(
"src/bin/a_bin.rs",
"fn bin_must_appear() { foo::foo(); } fn main(){}",
Expand All @@ -201,9 +208,9 @@ fn configure_target() {
.run();

let doc_html = p.read_file("target/doc/foo/fn.foo.html");
assert!(!doc_html.contains("lib_must_not_appear"));
assert!(doc_html.contains("example_must_appear"));
assert!(doc_html.contains("lib_must_appear"));
assert!(doc_html.contains("bin_must_appear"));
assert!(!doc_html.contains("example_must_not_appear"));
}

#[cargo_test(nightly, reason = "rustdoc scrape examples flags are unstable")]
Expand Down Expand Up @@ -231,7 +238,6 @@ fn configure_profile_issue_10500() {

let doc_html = p.read_file("target/doc/foo/fn.foo.html");
assert!(doc_html.contains("Examples found in repository"));
assert!(doc_html.contains("More examples"));
}

#[cargo_test(nightly, reason = "rustdoc scrape examples flags are unstable")]
Expand Down Expand Up @@ -342,21 +348,17 @@ fn no_fail_bad_lib() {
"\
[CHECKING] foo v0.0.1 ([CWD])
[SCRAPING] foo v0.0.1 ([CWD])
warning: failed to scan lib in package `foo` for example code usage
Try running with `--verbose` to see the error message.
If an example or library should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` or `[lib]` definition in Cargo.toml
warning: `foo` (lib) generated 1 warning
warning: failed to check lib in package `foo` as a prerequisite for scraping examples from: example \"ex\", example \"ex2\"
Try running with `--verbose` to see the error message.
If an example or library should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` or `[lib]` definition in Cargo.toml
If an example should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` definition in Cargo.toml
warning: `foo` (lib) generated 1 warning
warning: failed to scan example \"ex\" in package `foo` for example code usage
Try running with `--verbose` to see the error message.
If an example or library should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` or `[lib]` definition in Cargo.toml
If an example should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` definition in Cargo.toml
warning: `foo` (example \"ex\") generated 1 warning
warning: failed to scan example \"ex2\" in package `foo` for example code usage
Try running with `--verbose` to see the error message.
If an example or library should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` or `[lib]` definition in Cargo.toml
If an example should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` definition in Cargo.toml
warning: `foo` (example \"ex2\") generated 1 warning
[DOCUMENTING] foo v0.0.1 ([CWD])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]",
Expand Down Expand Up @@ -389,7 +391,7 @@ fn no_fail_bad_example() {
[SCRAPING] foo v0.0.1 ([CWD])
warning: failed to scan example \"ex1\" in package `foo` for example code usage
Try running with `--verbose` to see the error message.
If an example or library should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` or `[lib]` definition in Cargo.toml
If an example should not be scanned, then consider adding `doc-scrape-examples = false` to its `[[example]]` definition in Cargo.toml
warning: `foo` (example \"ex1\") generated 1 warning
[DOCUMENTING] foo v0.0.1 ([CWD])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]",
Expand All @@ -415,7 +417,6 @@ error: expected one of `!` or `::`, found `NOT`
| ^^^ expected one of `!` or `::`

[DOCUMENTING] foo v0.0.1 ([CWD])
[RUNNING] `rustdoc[..] --crate-name foo[..]
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]",
)
.run();
Expand Down Expand Up @@ -538,62 +539,48 @@ fn use_dev_deps_if_explicitly_enabled() {
#[cargo_test(nightly, reason = "rustdoc scrape examples flags are unstable")]
fn only_scrape_documented_targets() {
// package bar has doc = false and should not be eligible for documtation.
let run_with_config = |config: &str, should_scrape: bool| {
let p = project()
.file(
"Cargo.toml",
&format!(
r#"
let p = project()
.file(
"Cargo.toml",
&format!(
r#"
[package]
name = "bar"
version = "0.0.1"
authors = []

[lib]
{config}
doc = false

[workspace]
members = ["foo"]

[dependencies]
foo = {{ path = "foo" }}
"#
),
)
.file("src/lib.rs", "pub fn bar() { foo::foo(); }")
.file(
"foo/Cargo.toml",
r#"
),
)
.file("src/lib.rs", "")
.file("examples/ex.rs", "pub fn main() { foo::foo(); }")
.file(
"foo/Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
authors = []
"#,
)
.file("foo/src/lib.rs", "pub fn foo() {}")
.build();

p.cargo("doc --workspace -Zunstable-options -Zrustdoc-scrape-examples")
.masquerade_as_nightly_cargo(&["rustdoc-scrape-examples"])
.run();

let doc_html = p.read_file("target/doc/foo/fn.foo.html");
let example_found = doc_html.contains("Examples found in repository");
if should_scrape {
assert!(example_found);
} else {
assert!(!example_found);
}
};

// By default, bar should be scraped.
run_with_config("", true);
// If bar isn't supposed to be documented, then it is not eligible
// for scraping.
run_with_config("doc = false", false);
// But if the user explicitly says bar should be scraped, then it should
// be scraped.
run_with_config("doc = false\ndoc-scrape-examples = true", true);
)
.file("foo/src/lib.rs", "pub fn foo() {}")
.build();

p.cargo("doc --workspace -Zunstable-options -Zrustdoc-scrape-examples")
.masquerade_as_nightly_cargo(&["rustdoc-scrape-examples"])
.run();

let doc_html = p.read_file("target/doc/foo/fn.foo.html");
let example_found = doc_html.contains("Examples found in repository");
assert!(!example_found);
}

#[cargo_test(nightly, reason = "rustdoc scrape examples flags are unstable")]
Expand Down