Support doctests

Author: est31

src/bin/cargo/cli.rs

@@ -46,6 +46,7 @@ Available unstable (nightly-only) flags:
     -Z terminal-width      -- Provide a terminal width to rustc for error truncation
     -Z namespaced-features -- Allow features with `dep:` prefix
     -Z weak-dep-features   -- Allow `dep_name?/feature` feature syntax
+    -Z warn-unused-deps -- Emit warnings about unused dependencies
     -Z patch-in-config     -- Allow `[patch]` sections in .cargo/config.toml files
 
 Run with 'cargo -Z [FLAG] [SUBCOMMAND]'"

src/cargo/core/compiler/compilation.rs

@@ -7,7 +7,8 @@ use cargo_platform::CfgExpr;
 use cargo_util::{paths, ProcessBuilder};
 use semver::Version;
 
-use super::BuildContext;
+use super::unused_dependencies::UnusedDepState;
+use super::{BuildContext, UnitDep};
 use crate::core::compiler::{CompileKind, Metadata, Unit};
 use crate::core::Package;
 use crate::util::{config, CargoResult, Config};
@@ -16,6 +17,8 @@ use crate::util::{config, CargoResult, Config};
 pub struct Doctest {
     /// What's being doctested
     pub unit: Unit,
+    /// Dependencies of the unit
+    pub unit_deps: Vec<UnitDep>,
     /// Arguments needed to pass to rustdoc to run this test.
     pub args: Vec<OsString>,
     /// Whether or not -Zunstable-options is needed.
@@ -86,6 +89,8 @@ pub struct Compilation<'cfg> {
     /// The target host triple.
     pub host: String,
 
+    pub(crate) unused_dep_state: Option<UnusedDepState>,
+
     config: &'cfg Config,
 
     /// Rustc process to be used by default
@@ -141,6 +146,7 @@ impl<'cfg> Compilation<'cfg> {
             to_doc_test: Vec::new(),
             config: bcx.config,
             host: bcx.host_triple().to_string(),
+            unused_dep_state: None,
             rustc_process: rustc,
             rustc_workspace_wrapper_process,
             primary_rustc_process,

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

@@ -168,7 +168,9 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
         }
 
         // Now that we've figured out everything that we're going to do, do it!
-        queue.execute(&mut self, &mut plan)?;
+        let unused_dep_state = queue.execute(&mut self, &mut plan)?;
+
+        self.compilation.unused_dep_state = Some(unused_dep_state);
 
         if build_plan {
             plan.set_inputs(self.build_plan_inputs()?);
@@ -255,6 +257,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> {
 
                 self.compilation.to_doc_test.push(compilation::Doctest {
                     unit: unit.clone(),
+                    unit_deps: self.unit_deps(&unit).to_vec(),
                     args,
                     unstable_opts,
                     linker: self.bcx.linker(unit.kind),

src/cargo/core/compiler/job_queue.rs

@@ -415,7 +415,11 @@ impl<'cfg> JobQueue<'cfg> {
     /// This function will spawn off `config.jobs()` workers to build all of the
     /// necessary dependencies, in order. Freshness is propagated as far as
     /// possible along each dependency chain.
-    pub fn execute(mut self, cx: &mut Context<'_, '_>, plan: &mut BuildPlan) -> CargoResult<()> {
+    pub fn execute(
+        mut self,
+        cx: &mut Context<'_, '_>,
+        plan: &mut BuildPlan,
+    ) -> CargoResult<UnusedDepState> {
         let _p = profile::start("executing the job graph");
         self.queue.queue_finished();
 
@@ -435,7 +439,7 @@ impl<'cfg> JobQueue<'cfg> {
             progress,
             next_id: 0,
             timings: self.timings,
-            unused_dep_state: UnusedDepState::new_with_graph(cx), // TODO
+            unused_dep_state: UnusedDepState::new_with_graph(cx),
             tokens: Vec::new(),
             rustc_tokens: HashMap::new(),
             to_send_clients: BTreeMap::new(),
@@ -629,8 +633,12 @@ impl<'cfg> DrainState<'cfg> {
             }
             Message::UnusedExterns(id, unused_externs) => {
                 let unit = &self.active[&id];
-                self.unused_dep_state
-                    .record_unused_externs_for_unit(cx, unit, unused_externs);
+                let unit_deps = cx.unit_deps(&unit);
+                self.unused_dep_state.record_unused_externs_for_unit(
+                    unit_deps,
+                    unit,
+                    unused_externs,
+                );
             }
             Message::Token(acquired_token) => {
                 let token = acquired_token.chain_err(|| "failed to acquire jobserver token")?;
@@ -716,7 +724,7 @@ impl<'cfg> DrainState<'cfg> {
         plan: &mut BuildPlan,
         scope: &Scope<'_>,
         jobserver_helper: &HelperThread,
-    ) -> (Result<(), anyhow::Error>,) {
+    ) -> (Result<UnusedDepState, anyhow::Error>,) {
         trace!("queue: {:#?}", self.queue);
 
         // Iteratively execute the entire dependency graph. Each turn of the
@@ -805,7 +813,7 @@ impl<'cfg> DrainState<'cfg> {
         }
 
         if !cx.bcx.build_config.build_plan && cx.bcx.config.cli_unstable().warn_unused_deps {
-            drop(self.unused_dep_state.emit_unused_warnings(cx));
+            drop(self.unused_dep_state.emit_unused_early_warnings(cx));
         }
 
         if let Some(e) = error {
@@ -821,7 +829,7 @@ impl<'cfg> DrainState<'cfg> {
                 self.emit_future_incompat(cx);
             }
 
-            (Ok(()),)
+            (Ok(self.unused_dep_state),)
         } else {
             debug!("queue: {:#?}", self.queue);
             (Err(internal("finished with jobs still left in the queue")),)

src/cargo/core/compiler/unused_dependencies.rs

@@ -1,19 +1,20 @@
 use super::unit::Unit;
-use super::Context;
+use super::{Context, UnitDep};
 use crate::core::compiler::build_config::CompileMode;
 use crate::core::dependency::DepKind;
 use crate::core::manifest::TargetKind;
 use crate::core::Dependency;
 use crate::core::PackageId;
 use crate::util::errors::CargoResult;
 use crate::util::interning::InternedString;
+use crate::Config;
 use log::trace;
 
 use std::collections::{HashMap, HashSet};
 
 pub type AllowedKinds = HashSet<DepKind>;
 
-#[derive(Default)]
+#[derive(Default, Clone)]
 struct State {
     /// All externs of a root unit.
     externs: HashMap<InternedString, Option<Dependency>>,
@@ -24,6 +25,7 @@ struct State {
     reports_needed_by: HashSet<Unit>,
 }
 
+#[derive(Clone)]
 pub struct UnusedDepState {
     states: HashMap<(PackageId, Option<DepKind>), State>,
     /// Tracking for which units we have received reports from.
@@ -51,6 +53,8 @@ fn dep_kind_of(unit: &Unit) -> DepKind {
         TargetKind::Lib(_) => match unit.mode {
             // To support lib.rs with #[cfg(test)] use foo_crate as _;
             CompileMode::Test => DepKind::Development,
+            // To correctly register dev-dependencies
+            CompileMode::Doctest => DepKind::Development,
             _ => DepKind::Normal,
         },
         TargetKind::Bin => DepKind::Normal,
@@ -120,10 +124,9 @@ impl UnusedDepState {
                 root.pkg.name(),
                 unit_desc(root),
             );
-            // We aren't getting output from doctests, so skip them (for now)
             if root.mode == CompileMode::Doctest {
-                trace!("    -> skipping doctest");
-                continue;
+                //trace!("    -> skipping doctest");
+                //continue;
             }
             for dep in cx.unit_deps(root).iter() {
                 trace!(
@@ -156,13 +159,12 @@ impl UnusedDepState {
     /// and then updating the global list of used externs
     pub fn record_unused_externs_for_unit(
         &mut self,
-        cx: &mut Context<'_, '_>,
+        unit_deps: &[UnitDep],
         unit: &Unit,
         unused_externs: Vec<String>,
     ) {
         self.reports_obtained.insert(unit.clone());
-        let usable_deps_iter = cx
-            .unit_deps(unit)
+        let usable_deps_iter = unit_deps
             .iter()
             // compare with similar check in extern_args
             .filter(|dep| dep.unit.target.is_linkable() && !dep.unit.mode.is_doc());
@@ -194,18 +196,28 @@ impl UnusedDepState {
                 let record_kind = dep_kind_of(unit);
                 trace!(
                     "   => updating state of {}dep",
-                    dep_kind_desc(Some(record_kind))
+                    dep_kind_desc(Some(record_kind)),
                 );
                 state
                     .used_externs
                     .insert((used_dep.extern_crate_name, record_kind));
             }
         }
     }
-    pub fn emit_unused_warnings(&self, cx: &mut Context<'_, '_>) -> CargoResult<()> {
+    pub fn emit_unused_early_warnings(&self, cx: &mut Context<'_, '_>) -> CargoResult<()> {
+        self.emit_unused_warnings_inner(cx.bcx.config, Some(&cx.bcx.allowed_kinds))
+    }
+    pub fn emit_unused_late_warnings(&self, config: &Config) -> CargoResult<()> {
+        self.emit_unused_warnings_inner(config, None)
+    }
+    fn emit_unused_warnings_inner(
+        &self,
+        config: &Config,
+        allowed_kinds_or_late: Option<&AllowedKinds>,
+    ) -> CargoResult<()> {
         trace!(
             "Allowed dependency kinds for the unused deps check: {:?}",
-            cx.bcx.allowed_kinds
+            allowed_kinds_or_late
         );
 
         // Sort the states to have a consistent output
@@ -247,12 +259,15 @@ impl UnusedDepState {
                 } else {
                     continue;
                 };
-                if !cx.bcx.allowed_kinds.contains(dep_kind) {
-                    // We can't warn for dependencies of this target kind
-                    // as we aren't compiling all the units
-                    // that use the dependency kind
-                    trace!("Supressing unused deps warning of {} in pkg {} v{} as mode '{}dep' not allowed", dependency.name_in_toml(), pkg_id.name(), pkg_id.version(), dep_kind_desc(Some(*dep_kind)));
-                    continue;
+                if let Some(allowed_kinds) = allowed_kinds_or_late {
+                    if !allowed_kinds.contains(dep_kind) {
+                        // We can't warn for dependencies of this target kind
+                        // as we aren't compiling all the units
+                        // that use the dependency kind
+                        trace!("Supressing unused deps warning of {} in pkg {} v{} as mode '{}dep' not allowed", dependency.name_in_toml(), pkg_id.name(), pkg_id.version(), dep_kind_desc(Some(*dep_kind)));
+                        continue;
+                    }
+                } else {
                 }
                 if dependency.name_in_toml().starts_with("_") {
                     // Dependencies starting with an underscore
@@ -270,7 +285,7 @@ impl UnusedDepState {
                 {
                     // The dependency is used but only by dev targets,
                     // which means it should be a dev-dependency instead
-                    cx.bcx.config.shell().warn(format!(
+                    config.shell().warn(format!(
                         "dependency {} in package {} v{} is only used by dev targets",
                         dependency.name_in_toml(),
                         pkg_id.name(),
@@ -279,7 +294,7 @@ impl UnusedDepState {
                     continue;
                 }
 
-                cx.bcx.config.shell().warn(format!(
+                config.shell().warn(format!(
                     "unused {}dependency {} in package {} v{}",
                     dep_kind_desc(Some(*dep_kind)),
                     dependency.name_in_toml(),

src/cargo/ops/cargo_compile.rs

@@ -975,7 +975,6 @@ fn generate_targets(
                 }
             }
             allowed_kinds.insert(DepKind::Normal);
-            allowed_kinds.insert(DepKind::Development);
         }
         CompileFilter::Only {
             all_targets,
@@ -1036,12 +1035,6 @@ fn generate_targets(
             };
 
             if *lib != LibRule::False {
-                match (bins, examples, tests, benches) {
-                    (FilterRule::All, FilterRule::All, FilterRule::All, FilterRule::All) => {
-                        allowed_kinds.insert(DepKind::Development);
-                    }
-                    _ => (),
-                }
                 match (bins, examples, tests, benches) {
                     (FilterRule::All, ..) => {
                         allowed_kinds.insert(DepKind::Normal);

src/cargo/ops/cargo_test.rs

@@ -168,6 +168,7 @@ fn run_doc_tests(
             args,
             unstable_opts,
             unit,
+            unit_deps,
             linker,
             script_meta,
         } = doctest_info;
@@ -228,6 +229,12 @@ fn run_doc_tests(
             p.arg("-L").arg(arg);
         }
 
+        if config.cli_unstable().warn_unused_deps {
+            p.arg("-Z").arg("unstable-options");
+            p.arg("--error-format=json");
+            p.arg("--json=unused-externs");
+        }
+
         for native_dep in compilation.native_dirs.iter() {
             p.arg("-L").arg(native_dep);
         }
@@ -245,13 +252,43 @@ fn run_doc_tests(
         config
             .shell()
             .verbose(|shell| shell.status("Running", p.to_string()))?;
-        if let Err(e) = p.exec() {
+
+        let mut unused_dep_state = compilation.unused_dep_state.clone().unwrap();
+        if let Err(e) = p.exec_with_streaming(
+            &mut |line| {
+                writeln!(config.shell().out(), "{}", line)?;
+                Ok(())
+            },
+            &mut |line| {
+                #[derive(serde::Deserialize)]
+                struct UnusedExterns {
+                    unused_extern_names: Vec<String>,
+                }
+                if let Ok(uext) = serde_json::from_str::<UnusedExterns>(line) {
+                    unused_dep_state.record_unused_externs_for_unit(
+                        &unit_deps,
+                        unit,
+                        uext.unused_extern_names,
+                    );
+                    // Supress output of the json formatted unused extern message
+                    return Ok(());
+                }
+                writeln!(config.shell().err(), "{}", line)?;
+                Ok(())
+            },
+            false,
+        ) {
             let e = e.downcast::<ProcessError>()?;
             errors.push(e);
             if !options.no_fail_fast {
                 return Ok((Test::Doc, errors));
             }
         }
+        if config.cli_unstable().warn_unused_deps {
+            // Emit unused dependencies report which has been held back
+            // until now, as doctests could use things
+            unused_dep_state.emit_unused_late_warnings(config)?;
+        }
     }
     Ok((Test::Doc, errors))
 }