Add doc comments to explain scrape-examples feature

Author: willcrichton

src/cargo/core/compiler/context/mod.rs

@@ -78,6 +78,9 @@ pub struct Context<'a, 'cfg> {
     /// See Context::find_metadata_units for more details.
     pub metadata_for_doc_units: HashMap<Unit, Metadata>,
 
+    /// Set of metadata of Docscrape units that fail before completion, e.g.
+    /// because the target has a type error. This is in an Arc<Mutex<..>>
+    /// because it is continuously updated as the job progresses.
     pub failed_scrape_units: Arc<Mutex<HashSet<Metadata>>>,
 }
 

src/cargo/core/compiler/job_queue.rs

@@ -348,23 +348,29 @@ enum Message {
     BuildPlanMsg(String, ProcessBuilder, Arc<Vec<OutputFile>>),
     Stdout(String),
     Stderr(String),
+
+    // This is for general stderr output from subprocesses
     Diagnostic {
         id: JobId,
         level: String,
         diag: String,
         fixable: bool,
     },
-    // This is distinct from Diagnostic because it gets emitted through
-    // a different Shell pathway
-    Warning {
-        id: JobId,
-        warning: String,
-    },
+    // This handles duplicate output that is suppressed, for showing
+    // only a count of duplicate messages instead
     WarningCount {
         id: JobId,
         emitted: bool,
         fixable: bool,
     },
+    // This is for warnings generated by Cargo's interpretation of the
+    // subprocess output, e.g. scrape-examples prints a warning if a
+    // unit fails to be scraped
+    Warning {
+        id: JobId,
+        warning: String,
+    },
+
     FixDiagnostic(diagnostic_server::Message),
     Token(io::Result<Acquired>),
     Finish(JobId, Artifact, CargoResult<()>),
@@ -412,6 +418,7 @@ impl<'a, 'cfg> JobState<'a, 'cfg> {
         Ok(())
     }
 
+    /// See [`Message::Diagnostic`] and [`Message::WarningCount`].
     pub fn emit_diag(&self, level: String, diag: String, fixable: bool) -> CargoResult<()> {
         if let Some(dedupe) = self.output {
             let emitted = dedupe.emit_diag(&diag)?;
@@ -433,6 +440,7 @@ impl<'a, 'cfg> JobState<'a, 'cfg> {
         Ok(())
     }
 
+    /// See [`Message::Warning`].
     pub fn warning(&self, warning: String) -> CargoResult<()> {
         self.messages.push_bounded(Message::Warning {
             id: self.id,

src/cargo/core/compiler/mod.rs

@@ -654,8 +654,7 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
     rustdoc.arg("-C").arg(format!("metadata={}", metadata));
 
     let scrape_output_path = |unit: &Unit| -> CargoResult<PathBuf> {
-        cx.outputs(unit)
-            .map(|outputs| outputs[0].path.to_path_buf())
+        cx.outputs(unit).map(|outputs| outputs[0].path.clone())
     };
 
     if unit.mode.is_doc_scrape() {

src/cargo/core/compiler/rustdoc.rs

@@ -191,6 +191,8 @@ pub fn add_root_urls(
     Ok(())
 }
 
+/// Indicates whether a target should have examples scraped from it
+/// by rustdoc. Configured within Cargo.toml.
 #[derive(Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Debug, Copy)]
 pub enum RustdocScrapeExamples {
     Enabled,

src/cargo/core/manifest.rs

@@ -12,6 +12,7 @@ use serde::Serialize;
 use toml_edit::easy as toml;
 use url::Url;
 
+use crate::core::compiler::rustdoc::RustdocScrapeExamples;
 use crate::core::compiler::{CompileKind, CrateType};
 use crate::core::resolver::ResolveBehavior;
 use crate::core::{Dependency, PackageId, PackageIdSpec, SourceId, Summary};
@@ -21,8 +22,6 @@ use crate::util::interning::InternedString;
 use crate::util::toml::{TomlManifest, TomlProfiles};
 use crate::util::{short_hash, Config, Filesystem};
 
-use super::compiler::rustdoc::RustdocScrapeExamples;
-
 pub enum EitherManifest {
     Real(Manifest),
     Virtual(VirtualManifest),

src/cargo/ops/cargo_compile/compile_filter.rs

@@ -1,7 +1,9 @@
 //! Filters and their rules to select which Cargo targets will be built.
 
 use crate::core::compiler::CompileMode;
-use crate::core::{Target, TargetKind};
+use crate::core::dependency::DepKind;
+use crate::core::resolver::HasDevUnits;
+use crate::core::{Package, Target, TargetKind};
 use crate::util::restricted_names::is_glob_pattern;
 
 #[derive(Debug, PartialEq, Eq, Clone)]
@@ -299,4 +301,67 @@ impl CompileFilter {
             }
         }
     }
+
+    /// Generate a CompileFilter that represents the maximal set of targets
+    /// that should be considered for scraped examples.
+    pub(super) fn refine_for_docscrape(
+        &self,
+        to_builds: &[&Package],
+        has_dev_units: HasDevUnits,
+    ) -> CompileFilter {
+        // 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
+        // needed for the library, while dev-deps are needed for the examples.
+        //
+        // If scrape-examples caused `cargo doc` to start requiring dev-deps, this
+        // would be a breaking change to crates whose dev-deps don't compile.
+        // Therefore we ONLY want to scrape Example targets if either:
+        //    (1) No package has dev-dependencies, so this is a moot issue, OR
+        //    (2) The provided CompileFilter requires dev-dependencies anyway.
+        //
+        // The next two variables represent these two conditions.
+
+        let no_pkg_has_dev_deps = to_builds.iter().all(|pkg| {
+            pkg.summary()
+                .dependencies()
+                .iter()
+                .all(|dep| !matches!(dep.kind(), DepKind::Development))
+        });
+
+        let reqs_dev_deps = matches!(has_dev_units, HasDevUnits::Yes);
+
+        let example_filter = if no_pkg_has_dev_deps || reqs_dev_deps {
+            FilterRule::All
+        } else {
+            FilterRule::none()
+        };
+
+        match self {
+            CompileFilter::Only {
+                all_targets,
+                lib,
+                bins,
+                tests,
+                benches,
+                ..
+            } => CompileFilter::Only {
+                all_targets: *all_targets,
+                lib: lib.clone(),
+                bins: bins.clone(),
+                examples: example_filter,
+                tests: tests.clone(),
+                benches: benches.clone(),
+            },
+
+            CompileFilter::Default { .. } => CompileFilter::Only {
+                all_targets: false,
+                lib: LibRule::Default,
+                bins: FilterRule::none(),
+                examples: example_filter,
+                tests: FilterRule::none(),
+                benches: FilterRule::none(),
+            },
+        }
+    }
 }

src/cargo/ops/cargo_compile/mod.rs

@@ -41,7 +41,6 @@ use crate::core::compiler::{standard_lib, CrateType, TargetInfo};
 use crate::core::compiler::{BuildConfig, BuildContext, Compilation, Context};
 use crate::core::compiler::{CompileKind, CompileMode, CompileTarget, RustcTargetData, Unit};
 use crate::core::compiler::{DefaultExecutor, Executor, UnitInterner};
-use crate::core::dependency::DepKind;
 use crate::core::profiles::{Profiles, UnitFor};
 use crate::core::resolver::features::{self, CliFeatures, FeaturesFor};
 use crate::core::resolver::{HasDevUnits, Resolve};
@@ -370,7 +369,7 @@ 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_filter = filter_for_scrape_units(&to_builds, has_dev_units, filter);
+        let scrape_filter = filter.refine_for_docscrape(&to_builds, has_dev_units);
         let all_units = generate_targets(
             ws,
             &to_builds,
@@ -567,56 +566,6 @@ pub fn create_bcx<'a, 'cfg>(
     Ok(bcx)
 }
 
-/// Generate a CompileFilter that represents the maximal set of targets that should be
-/// considered for scraping. Should be a subset of the CLI-provided CompileFilter.
-fn filter_for_scrape_units(
-    to_builds: &[&Package],
-    has_dev_units: HasDevUnits,
-    filter: &CompileFilter,
-) -> CompileFilter {
-    let no_pkg_has_dev_deps = to_builds.iter().all(|pkg| {
-        pkg.summary()
-            .dependencies()
-            .iter()
-            .all(|dep| !matches!(dep.kind(), DepKind::Development))
-    });
-
-    // We are allowed to include examples ONLY if they cannot possibly require dev dependencies,
-    // or if dev dependencies are already required anyway due to the user's configuration.
-    let example_filter = if matches!(has_dev_units, HasDevUnits::Yes) || no_pkg_has_dev_deps {
-        FilterRule::All
-    } else {
-        FilterRule::none()
-    };
-
-    match filter {
-        CompileFilter::Only {
-            all_targets,
-            lib,
-            bins,
-            tests,
-            benches,
-            ..
-        } => CompileFilter::Only {
-            all_targets: *all_targets,
-            lib: lib.clone(),
-            bins: bins.clone(),
-            examples: example_filter,
-            tests: tests.clone(),
-            benches: benches.clone(),
-        },
-
-        CompileFilter::Default { .. } => CompileFilter::Only {
-            all_targets: false,
-            lib: LibRule::Default,
-            bins: FilterRule::none(),
-            examples: example_filter,
-            tests: FilterRule::none(),
-            benches: FilterRule::none(),
-        },
-    }
-}
-
 /// A proposed target.
 ///
 /// Proposed targets are later filtered into actual `Unit`s based on whether or

tests/testsuite/docscrape.rs

@@ -1,6 +1,5 @@
-//! Tests for the `cargo doc` command.
+//! Tests for the `cargo doc` command with `-Zrustdoc-scrape-examples`.
 
-use cargo_test_support::is_nightly;
 use cargo_test_support::project;
 
 #[cargo_test(nightly, reason = "rustdoc scrape examples flags are unstable")]
@@ -41,11 +40,6 @@ fn basic() {
 
 #[cargo_test(nightly, reason = "rustdoc scrape examples flags are unstable")]
 fn avoid_build_script_cycle() {
-    if !is_nightly() {
-        // -Z rustdoc-scrape-examples is unstable
-        return;
-    }
-
     let p = project()
         // package with build dependency
         .file(
@@ -81,7 +75,7 @@ fn avoid_build_script_cycle() {
         .file("bar/build.rs", "fn main(){}")
         .build();
 
-    p.cargo("doc --all -Zunstable-options -Zrustdoc-scrape-examples")
+    p.cargo("doc --workspace -Zunstable-options -Zrustdoc-scrape-examples")
         .masquerade_as_nightly_cargo(&["rustdoc-scrape-examples"])
         .run();
 }
@@ -138,7 +132,7 @@ fn complex_reverse_dependencies() {
         .file("b/src/lib.rs", "")
         .build();
 
-    p.cargo("doc --all --examples -Zunstable-options -Zrustdoc-scrape-examples")
+    p.cargo("doc --workspace --examples -Zunstable-options -Zrustdoc-scrape-examples")
         .masquerade_as_nightly_cargo(&["rustdoc-scrape-examples"])
         .run();
 }
@@ -208,7 +202,7 @@ fn configure_target() {
 }
 
 #[cargo_test(nightly, reason = "rustdoc scrape examples flags are unstable")]
-fn configure_profile() {
+fn configure_profile_issue_10500() {
     let p = project()
         .file(
             "Cargo.toml",
@@ -277,7 +271,7 @@ fn issue_10545() {
         .file("b/src/lib.rs", "")
         .build();
 
-    p.cargo("doc --all -Zunstable-options -Zrustdoc-scrape-examples")
+    p.cargo("doc --workspace -Zunstable-options -Zrustdoc-scrape-examples")
         .masquerade_as_nightly_cargo(&["rustdoc-scrape-examples"])
         .run();
 }
@@ -412,6 +406,10 @@ error: expected one of `!` or `::`, found `NOT`
 
 #[cargo_test(nightly, reason = "rustdoc scrape examples flags are unstable")]
 fn no_scrape_with_dev_deps() {
+    // Tests that a crate with dev-dependencies does not have its examples
+    // scraped unless explicitly prompted to check them. See
+    // `CompileFilter::refine_for_docscrape` for details on why.
+
     let p = project()
         .file(
             "Cargo.toml",