Auto merge of #11499 - willcrichton:example-analyzer, r=weihanglo

Don't scrape examples from library targets by default

### What does this PR try to resolve?

Based on some [early feedback](https://www.reddit.com/r/rust/comments/zosle6/feedback_requested_rustdocs_scraped_examples/) about the scrape-examples feature, both documentation authors and consumers did not consider examples useful if they are scraped from a library's internals, at least in the common case. Therefore this PR changes the default behavior of `-Zrustdoc-scrape-examples` to *only* scrape from example targets, although library targets can still be configured for scraping.

### How should we test and review this PR?

I have updated the `docscrape` tests to reflect this new policy, as well as the Unstable Options page in the Cargo book.

r? `@weihanglo`

Author: bors

src/cargo/core/compiler/mod.rs

@@ -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()
     )
 }

src/cargo/ops/cargo_compile/mod.rs

@@ -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};
@@ -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()
     };

src/cargo/ops/cargo_compile/unit_generator.rs

@@ -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()
@@ -200,7 +200,7 @@ impl<'a> UnitGenerator<'a, '_> {
                     })
                     .collect()
             }
-            CompileMode::Doctest | CompileMode::RunCustomBuild => {
+            CompileMode::Doctest | CompileMode::RunCustomBuild | CompileMode::Docscrape => {
                 panic!("Invalid mode {:?}", self.mode)
             }
         }
@@ -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
@@ -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
@@ -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)
+    }
 }

src/doc/src/reference/unstable.md

@@ -1124,34 +1124,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

tests/testsuite/docscrape.rs

@@ -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());
@@ -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(){}",
@@ -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")]
@@ -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")]
@@ -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 [..]",
@@ -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 [..]",
@@ -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();
@@ -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")]