Skip to content

Commit

Permalink
Support specifying different lint levels
Browse files Browse the repository at this point in the history 
Specifying via the command line is not possible for now because cargo
currently has to pass -W via the command line, but once the lint
is set to warn by default, this won't be needed :).
  • Loading branch information
@est31
est31 committed Apr 6, 2021
1 parent 7bccfc3 commit c0e3e5a
Show file tree
Hide file tree
Showing 5 changed files with 282 additions and 94 deletions.
6 changes: 3 additions & 3 deletions src/cargo/core/compiler/job_queue.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ use super::job::{
Job,
};
use super::timings::Timings;
use super::unused_dependencies::UnusedDepState;
use super::unused_dependencies::{UnusedDepState, UnusedExterns};
use super::{BuildContext, BuildPlan, CompileMode, Context, Unit};
use crate::core::compiler::future_incompat::{
FutureBreakageItem, OnDiskReport, FUTURE_INCOMPAT_FILE,
Expand Down Expand Up @@ -244,7 +244,7 @@ enum Message {
Token(io::Result<Acquired>),
Finish(JobId, Artifact, CargoResult<()>),
FutureIncompatReport(JobId, Vec<FutureBreakageItem>),
UnusedExterns(JobId, Vec<String>),
UnusedExterns(JobId, UnusedExterns),

// This client should get release_raw called on it with one of our tokens
NeedsToken(JobId),
Expand Down Expand Up @@ -308,7 +308,7 @@ impl<'a> JobState<'a> {
///
/// This is useful for checking unused dependencies.
/// Should only be called once, as the compiler only emits it once per compilation.
pub fn unused_externs(&self, unused_externs: Vec<String>) {
pub fn unused_externs(&self, unused_externs: UnusedExterns) {
self.messages
.push(Message::UnusedExterns(self.id, unused_externs));
}
Expand Down
16 changes: 7 additions & 9 deletions src/cargo/core/compiler/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ pub(crate) use self::layout::Layout;
pub use self::lto::Lto;
use self::output_depinfo::output_depinfo;
use self::unit_graph::UnitDep;
use self::unused_dependencies::UnusedExterns;
use crate::core::compiler::future_incompat::FutureIncompatReport;
pub use crate::core::compiler::unit::{Unit, UnitInterner};
use crate::core::manifest::TargetSourcePath;
Expand Down Expand Up @@ -216,6 +217,10 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car

add_cap_lints(cx.bcx, unit, &mut rustc);

if cx.bcx.config.cli_unstable().warn_unused_deps && unit.show_warnings(cx.bcx.config) {
rustc.arg("-W").arg("unused_crate_dependencies");
}

let outputs = cx.outputs(unit)?;
let root = cx.files().out_dir(unit);

Expand Down Expand Up @@ -1338,16 +1343,9 @@ fn on_stderr_line_inner(
}
}

#[derive(serde::Deserialize)]
struct UnusedExterns {
unused_extern_names: Vec<String>,
}
if let Ok(uext) = serde_json::from_str::<UnusedExterns>(compiler_message.get()) {
log::trace!(
"obtained unused externs list from rustc: `{:?}`",
uext.unused_extern_names
);
state.unused_externs(uext.unused_extern_names);
log::trace!("obtained unused externs message from rustc: `{:?}`", uext);
state.unused_externs(uext);
return Ok(true);
}

Expand Down
186 changes: 115 additions & 71 deletions src/cargo/core/compiler/unused_dependencies.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,28 @@ use std::collections::{HashMap, HashSet};

pub type AllowedKinds = HashSet<DepKind>;

#[derive(serde::Deserialize, Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
#[serde(rename_all = "lowercase")]
/// Lint levels
///
/// Note that order is important here
pub enum LintLevel {
// Allow isn't mentioned as the unused dependencies message
// isn't emitted if the lint is set to allow.
Warn,
Deny,
Forbid,
}

#[derive(serde::Deserialize, Debug)]
pub struct UnusedExterns {
lint_level: LintLevel,
unused_extern_names: Vec<String>,
}

#[derive(Default, Clone)]
struct State {
/// All externs of a root unit.
/// All externs passed to units
externs: HashMap<InternedString, Option<Dependency>>,
/// The used externs so far.
/// The DepKind is included so that we can tell when
Expand All @@ -28,6 +47,8 @@ struct State {
#[derive(Clone)]
pub struct UnusedDepState {
states: HashMap<(PackageId, Option<DepKind>), State>,
/// The worst encountered lint level so far
worst_lint_level: LintLevel,
/// Tracking for which units we have received reports from.
///
/// When we didn't receive reports, e.g. because of an error,
Expand Down Expand Up @@ -152,6 +173,7 @@ impl UnusedDepState {

Self {
states,
worst_lint_level: LintLevel::Warn,
reports_obtained: HashSet::new(),
}
}
Expand All @@ -161,15 +183,18 @@ impl UnusedDepState {
&mut self,
unit_deps: &[UnitDep],
unit: &Unit,
unused_externs: Vec<String>,
unused_externs: UnusedExterns,
) {
self.reports_obtained.insert(unit.clone());
self.worst_lint_level = self.worst_lint_level.max(unused_externs.lint_level);

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

let unused_externs_set = unused_externs
.unused_extern_names
.iter()
.map(|ex| InternedString::new(ex))
.collect::<HashSet<InternedString>>();
Expand Down Expand Up @@ -220,89 +245,108 @@ impl UnusedDepState {
allowed_kinds_or_late
);

// Sort the states to have a consistent output
let mut states_sorted = self.states.iter().collect::<Vec<_>>();
states_sorted.sort_by_key(|(k, _v)| k.clone());
for ((pkg_id, dep_kind), state) in states_sorted.iter() {
let outstanding_reports = state
.reports_needed_by
.iter()
.filter(|report| !self.reports_obtained.contains(report))
.collect::<Vec<_>>();
if !outstanding_reports.is_empty() {
trace!("Supressing unused deps warning of pkg {} v{} mode '{}dep' due to outstanding reports {:?}", pkg_id.name(), pkg_id.version(), dep_kind_desc(*dep_kind),
let mut error_count = 0;
{
let mut emit_lint: Box<dyn FnMut(String) -> CargoResult<()>> =
if self.worst_lint_level == LintLevel::Warn {
Box::new(|msg| config.shell().warn(msg))
} else {
Box::new(|msg| {
error_count += 1;
config.shell().error(msg)
})
};

// Sort the states to have a consistent output
let mut states_sorted = self.states.iter().collect::<Vec<_>>();
states_sorted.sort_by_key(|(k, _v)| k.clone());
for ((pkg_id, dep_kind), state) in states_sorted.iter() {
let outstanding_reports = state
.reports_needed_by
.iter()
.filter(|report| !self.reports_obtained.contains(report))
.collect::<Vec<_>>();
if !outstanding_reports.is_empty() {
trace!("Supressing unused deps warning of pkg {} v{} mode '{}dep' due to outstanding reports {:?}", pkg_id.name(), pkg_id.version(), dep_kind_desc(*dep_kind),
outstanding_reports.iter().map(|unit|
unit_desc(unit)).collect::<Vec<_>>());

// Some compilations errored without printing the unused externs.
// Don't print the warning in order to reduce false positive
// spam during errors.
continue;
}
// Sort the externs to have a consistent output
let mut externs_sorted = state.externs.iter().collect::<Vec<_>>();
externs_sorted.sort_by_key(|(k, _v)| k.clone());
for (ext, dependency) in externs_sorted.iter() {
let dep_kind = if let Some(dep_kind) = dep_kind {
dep_kind
} else {
// Internal dep_kind isn't interesting to us
continue;
};
if state.used_externs.contains(&(**ext, *dep_kind)) {
// The dependency is used
// Some compilations errored without printing the unused externs.
// Don't print the warning in order to reduce false positive
// spam during errors.
continue;
}
// Implicitly added dependencies (in the same crate) aren't interesting
let dependency = if let Some(dependency) = dependency {
dependency
} else {
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)));
// Sort the externs to have a consistent output
let mut externs_sorted = state.externs.iter().collect::<Vec<_>>();
externs_sorted.sort_by_key(|(k, _v)| k.clone());
for (ext, dependency) in externs_sorted.iter() {
let dep_kind = if let Some(dep_kind) = dep_kind {
dep_kind
} else {
// Internal dep_kind isn't interesting to us
continue;
};
if state.used_externs.contains(&(**ext, *dep_kind)) {
// The dependency is used
continue;
}
} else {
}
if dependency.name_in_toml().starts_with("_") {
// Dependencies starting with an underscore
// are marked as ignored
trace!(
"Supressing unused deps warning of {} in pkg {} v{} due to name",
dependency.name_in_toml(),
pkg_id.name(),
pkg_id.version()
);
continue;
}
if dep_kind == &DepKind::Normal
&& state.used_externs.contains(&(**ext, DepKind::Development))
{
// The dependency is used but only by dev targets,
// which means it should be a dev-dependency instead
config.shell().warn(format!(
"dependency {} in package {} v{} is only used by dev targets",
// Implicitly added dependencies (in the same crate) aren't interesting
let dependency = if let Some(dependency) = dependency {
dependency
} else {
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
// are marked as ignored
trace!(
"Supressing unused deps warning of {} in pkg {} v{} due to name",
dependency.name_in_toml(),
pkg_id.name(),
pkg_id.version()
);
continue;
}
if dep_kind == &DepKind::Normal
&& state.used_externs.contains(&(**ext, DepKind::Development))
{
// The dependency is used but only by dev targets,
// which means it should be a dev-dependency instead
emit_lint(format!(
"dependency {} in package {} v{} is only used by dev targets",
dependency.name_in_toml(),
pkg_id.name(),
pkg_id.version()
))?;
continue;
}

emit_lint(format!(
"unused {}dependency {} in package {} v{}",
dep_kind_desc(Some(*dep_kind)),
dependency.name_in_toml(),
pkg_id.name(),
pkg_id.version()
))?;
continue;
}

config.shell().warn(format!(
"unused {}dependency {} in package {} v{}",
dep_kind_desc(Some(*dep_kind)),
dependency.name_in_toml(),
pkg_id.name(),
pkg_id.version()
))?;
}
}
if error_count > 0 {
anyhow::bail!(
"exiting because of {} unused dependencies error(s)",
error_count
);
}
Ok(())
}
}
11 changes: 2 additions & 9 deletions src/cargo/ops/cargo_test.rs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::core::compiler::unused_dependencies::UnusedExterns;
use crate::core::compiler::{Compilation, CompileKind, Doctest, UnitOutput};
use crate::core::shell::Verbosity;
use crate::core::{TargetKind, Workspace};
Expand Down Expand Up @@ -260,16 +261,8 @@ fn run_doc_tests(
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,
);
unused_dep_state.record_unused_externs_for_unit(&unit_deps, unit, uext);
// Supress output of the json formatted unused extern message
return Ok(());
}
Expand Down
Loading

0 comments on commit c0e3e5a

Please sign in to comment.