Skip to content

Commit

Permalink
Support doctests
Browse files Browse the repository at this point in the history
  • Loading branch information
est31 committed Apr 6, 2021
1 parent 6a1697c commit 7bccfc3
Show file tree
Hide file tree
Showing 8 changed files with 169 additions and 53 deletions.
1 change: 1 addition & 0 deletions src/bin/cargo/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]'"
Expand Down
8 changes: 7 additions & 1 deletion src/cargo/core/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion src/cargo/core/compiler/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?);
Expand Down Expand Up @@ -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),
Expand Down
22 changes: 15 additions & 7 deletions src/cargo/core/compiler/job_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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(),
Expand Down Expand Up @@ -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")?;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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")),)
Expand Down
53 changes: 34 additions & 19 deletions src/cargo/core/compiler/unused_dependencies.rs
Original file line number Diff line number Diff line change
@@ -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>>,
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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(),
Expand All @@ -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(),
Expand Down
7 changes: 0 additions & 7 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -975,7 +975,6 @@ fn generate_targets(
}
}
allowed_kinds.insert(DepKind::Normal);
allowed_kinds.insert(DepKind::Development);
}
CompileFilter::Only {
all_targets,
Expand Down Expand Up @@ -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);
Expand Down
39 changes: 38 additions & 1 deletion src/cargo/ops/cargo_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ fn run_doc_tests(
args,
unstable_opts,
unit,
unit_deps,
linker,
script_meta,
} = doctest_info;
Expand Down Expand Up @@ -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);
}
Expand All @@ -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))
}
Loading

0 comments on commit 7bccfc3

Please sign in to comment.