Skip to content

Commit c0e3e5a

Browse files
committed
Support specifying different lint levels
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 :).
1 parent 7bccfc3 commit c0e3e5a

File tree

5 files changed

+282
-94
lines changed

5 files changed

+282
-94
lines changed

src/cargo/core/compiler/job_queue.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ use super::job::{
7070
Job,
7171
};
7272
use super::timings::Timings;
73-
use super::unused_dependencies::UnusedDepState;
73+
use super::unused_dependencies::{UnusedDepState, UnusedExterns};
7474
use super::{BuildContext, BuildPlan, CompileMode, Context, Unit};
7575
use crate::core::compiler::future_incompat::{
7676
FutureBreakageItem, OnDiskReport, FUTURE_INCOMPAT_FILE,
@@ -244,7 +244,7 @@ enum Message {
244244
Token(io::Result<Acquired>),
245245
Finish(JobId, Artifact, CargoResult<()>),
246246
FutureIncompatReport(JobId, Vec<FutureBreakageItem>),
247-
UnusedExterns(JobId, Vec<String>),
247+
UnusedExterns(JobId, UnusedExterns),
248248

249249
// This client should get release_raw called on it with one of our tokens
250250
NeedsToken(JobId),
@@ -308,7 +308,7 @@ impl<'a> JobState<'a> {
308308
///
309309
/// This is useful for checking unused dependencies.
310310
/// Should only be called once, as the compiler only emits it once per compilation.
311-
pub fn unused_externs(&self, unused_externs: Vec<String>) {
311+
pub fn unused_externs(&self, unused_externs: UnusedExterns) {
312312
self.messages
313313
.push(Message::UnusedExterns(self.id, unused_externs));
314314
}

src/cargo/core/compiler/mod.rs

+7-9
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ pub(crate) use self::layout::Layout;
5050
pub use self::lto::Lto;
5151
use self::output_depinfo::output_depinfo;
5252
use self::unit_graph::UnitDep;
53+
use self::unused_dependencies::UnusedExterns;
5354
use crate::core::compiler::future_incompat::FutureIncompatReport;
5455
pub use crate::core::compiler::unit::{Unit, UnitInterner};
5556
use crate::core::manifest::TargetSourcePath;
@@ -216,6 +217,10 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
216217

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

220+
if cx.bcx.config.cli_unstable().warn_unused_deps && unit.show_warnings(cx.bcx.config) {
221+
rustc.arg("-W").arg("unused_crate_dependencies");
222+
}
223+
219224
let outputs = cx.outputs(unit)?;
220225
let root = cx.files().out_dir(unit);
221226

@@ -1338,16 +1343,9 @@ fn on_stderr_line_inner(
13381343
}
13391344
}
13401345

1341-
#[derive(serde::Deserialize)]
1342-
struct UnusedExterns {
1343-
unused_extern_names: Vec<String>,
1344-
}
13451346
if let Ok(uext) = serde_json::from_str::<UnusedExterns>(compiler_message.get()) {
1346-
log::trace!(
1347-
"obtained unused externs list from rustc: `{:?}`",
1348-
uext.unused_extern_names
1349-
);
1350-
state.unused_externs(uext.unused_extern_names);
1347+
log::trace!("obtained unused externs message from rustc: `{:?}`", uext);
1348+
state.unused_externs(uext);
13511349
return Ok(true);
13521350
}
13531351

src/cargo/core/compiler/unused_dependencies.rs

+115-71
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,28 @@ use std::collections::{HashMap, HashSet};
1414

1515
pub type AllowedKinds = HashSet<DepKind>;
1616

17+
#[derive(serde::Deserialize, Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
18+
#[serde(rename_all = "lowercase")]
19+
/// Lint levels
20+
///
21+
/// Note that order is important here
22+
pub enum LintLevel {
23+
// Allow isn't mentioned as the unused dependencies message
24+
// isn't emitted if the lint is set to allow.
25+
Warn,
26+
Deny,
27+
Forbid,
28+
}
29+
30+
#[derive(serde::Deserialize, Debug)]
31+
pub struct UnusedExterns {
32+
lint_level: LintLevel,
33+
unused_extern_names: Vec<String>,
34+
}
35+
1736
#[derive(Default, Clone)]
1837
struct State {
19-
/// All externs of a root unit.
38+
/// All externs passed to units
2039
externs: HashMap<InternedString, Option<Dependency>>,
2140
/// The used externs so far.
2241
/// The DepKind is included so that we can tell when
@@ -28,6 +47,8 @@ struct State {
2847
#[derive(Clone)]
2948
pub struct UnusedDepState {
3049
states: HashMap<(PackageId, Option<DepKind>), State>,
50+
/// The worst encountered lint level so far
51+
worst_lint_level: LintLevel,
3152
/// Tracking for which units we have received reports from.
3253
///
3354
/// When we didn't receive reports, e.g. because of an error,
@@ -152,6 +173,7 @@ impl UnusedDepState {
152173

153174
Self {
154175
states,
176+
worst_lint_level: LintLevel::Warn,
155177
reports_obtained: HashSet::new(),
156178
}
157179
}
@@ -161,15 +183,18 @@ impl UnusedDepState {
161183
&mut self,
162184
unit_deps: &[UnitDep],
163185
unit: &Unit,
164-
unused_externs: Vec<String>,
186+
unused_externs: UnusedExterns,
165187
) {
166188
self.reports_obtained.insert(unit.clone());
189+
self.worst_lint_level = self.worst_lint_level.max(unused_externs.lint_level);
190+
167191
let usable_deps_iter = unit_deps
168192
.iter()
169193
// compare with similar check in extern_args
170194
.filter(|dep| dep.unit.target.is_linkable() && !dep.unit.mode.is_doc());
171195

172196
let unused_externs_set = unused_externs
197+
.unused_extern_names
173198
.iter()
174199
.map(|ex| InternedString::new(ex))
175200
.collect::<HashSet<InternedString>>();
@@ -220,89 +245,108 @@ impl UnusedDepState {
220245
allowed_kinds_or_late
221246
);
222247

223-
// Sort the states to have a consistent output
224-
let mut states_sorted = self.states.iter().collect::<Vec<_>>();
225-
states_sorted.sort_by_key(|(k, _v)| k.clone());
226-
for ((pkg_id, dep_kind), state) in states_sorted.iter() {
227-
let outstanding_reports = state
228-
.reports_needed_by
229-
.iter()
230-
.filter(|report| !self.reports_obtained.contains(report))
231-
.collect::<Vec<_>>();
232-
if !outstanding_reports.is_empty() {
233-
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),
248+
let mut error_count = 0;
249+
{
250+
let mut emit_lint: Box<dyn FnMut(String) -> CargoResult<()>> =
251+
if self.worst_lint_level == LintLevel::Warn {
252+
Box::new(|msg| config.shell().warn(msg))
253+
} else {
254+
Box::new(|msg| {
255+
error_count += 1;
256+
config.shell().error(msg)
257+
})
258+
};
259+
260+
// Sort the states to have a consistent output
261+
let mut states_sorted = self.states.iter().collect::<Vec<_>>();
262+
states_sorted.sort_by_key(|(k, _v)| k.clone());
263+
for ((pkg_id, dep_kind), state) in states_sorted.iter() {
264+
let outstanding_reports = state
265+
.reports_needed_by
266+
.iter()
267+
.filter(|report| !self.reports_obtained.contains(report))
268+
.collect::<Vec<_>>();
269+
if !outstanding_reports.is_empty() {
270+
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),
234271
outstanding_reports.iter().map(|unit|
235272
unit_desc(unit)).collect::<Vec<_>>());
236273

237-
// Some compilations errored without printing the unused externs.
238-
// Don't print the warning in order to reduce false positive
239-
// spam during errors.
240-
continue;
241-
}
242-
// Sort the externs to have a consistent output
243-
let mut externs_sorted = state.externs.iter().collect::<Vec<_>>();
244-
externs_sorted.sort_by_key(|(k, _v)| k.clone());
245-
for (ext, dependency) in externs_sorted.iter() {
246-
let dep_kind = if let Some(dep_kind) = dep_kind {
247-
dep_kind
248-
} else {
249-
// Internal dep_kind isn't interesting to us
250-
continue;
251-
};
252-
if state.used_externs.contains(&(**ext, *dep_kind)) {
253-
// The dependency is used
274+
// Some compilations errored without printing the unused externs.
275+
// Don't print the warning in order to reduce false positive
276+
// spam during errors.
254277
continue;
255278
}
256-
// Implicitly added dependencies (in the same crate) aren't interesting
257-
let dependency = if let Some(dependency) = dependency {
258-
dependency
259-
} else {
260-
continue;
261-
};
262-
if let Some(allowed_kinds) = allowed_kinds_or_late {
263-
if !allowed_kinds.contains(dep_kind) {
264-
// We can't warn for dependencies of this target kind
265-
// as we aren't compiling all the units
266-
// that use the dependency kind
267-
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)));
279+
// Sort the externs to have a consistent output
280+
let mut externs_sorted = state.externs.iter().collect::<Vec<_>>();
281+
externs_sorted.sort_by_key(|(k, _v)| k.clone());
282+
for (ext, dependency) in externs_sorted.iter() {
283+
let dep_kind = if let Some(dep_kind) = dep_kind {
284+
dep_kind
285+
} else {
286+
// Internal dep_kind isn't interesting to us
287+
continue;
288+
};
289+
if state.used_externs.contains(&(**ext, *dep_kind)) {
290+
// The dependency is used
268291
continue;
269292
}
270-
} else {
271-
}
272-
if dependency.name_in_toml().starts_with("_") {
273-
// Dependencies starting with an underscore
274-
// are marked as ignored
275-
trace!(
276-
"Supressing unused deps warning of {} in pkg {} v{} due to name",
277-
dependency.name_in_toml(),
278-
pkg_id.name(),
279-
pkg_id.version()
280-
);
281-
continue;
282-
}
283-
if dep_kind == &DepKind::Normal
284-
&& state.used_externs.contains(&(**ext, DepKind::Development))
285-
{
286-
// The dependency is used but only by dev targets,
287-
// which means it should be a dev-dependency instead
288-
config.shell().warn(format!(
289-
"dependency {} in package {} v{} is only used by dev targets",
293+
// Implicitly added dependencies (in the same crate) aren't interesting
294+
let dependency = if let Some(dependency) = dependency {
295+
dependency
296+
} else {
297+
continue;
298+
};
299+
if let Some(allowed_kinds) = allowed_kinds_or_late {
300+
if !allowed_kinds.contains(dep_kind) {
301+
// We can't warn for dependencies of this target kind
302+
// as we aren't compiling all the units
303+
// that use the dependency kind
304+
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)));
305+
continue;
306+
}
307+
} else {
308+
}
309+
if dependency.name_in_toml().starts_with("_") {
310+
// Dependencies starting with an underscore
311+
// are marked as ignored
312+
trace!(
313+
"Supressing unused deps warning of {} in pkg {} v{} due to name",
314+
dependency.name_in_toml(),
315+
pkg_id.name(),
316+
pkg_id.version()
317+
);
318+
continue;
319+
}
320+
if dep_kind == &DepKind::Normal
321+
&& state.used_externs.contains(&(**ext, DepKind::Development))
322+
{
323+
// The dependency is used but only by dev targets,
324+
// which means it should be a dev-dependency instead
325+
emit_lint(format!(
326+
"dependency {} in package {} v{} is only used by dev targets",
327+
dependency.name_in_toml(),
328+
pkg_id.name(),
329+
pkg_id.version()
330+
))?;
331+
continue;
332+
}
333+
334+
emit_lint(format!(
335+
"unused {}dependency {} in package {} v{}",
336+
dep_kind_desc(Some(*dep_kind)),
290337
dependency.name_in_toml(),
291338
pkg_id.name(),
292339
pkg_id.version()
293340
))?;
294-
continue;
295341
}
296-
297-
config.shell().warn(format!(
298-
"unused {}dependency {} in package {} v{}",
299-
dep_kind_desc(Some(*dep_kind)),
300-
dependency.name_in_toml(),
301-
pkg_id.name(),
302-
pkg_id.version()
303-
))?;
304342
}
305343
}
344+
if error_count > 0 {
345+
anyhow::bail!(
346+
"exiting because of {} unused dependencies error(s)",
347+
error_count
348+
);
349+
}
306350
Ok(())
307351
}
308352
}

src/cargo/ops/cargo_test.rs

+2-9
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::core::compiler::unused_dependencies::UnusedExterns;
12
use crate::core::compiler::{Compilation, CompileKind, Doctest, UnitOutput};
23
use crate::core::shell::Verbosity;
34
use crate::core::{TargetKind, Workspace};
@@ -260,16 +261,8 @@ fn run_doc_tests(
260261
Ok(())
261262
},
262263
&mut |line| {
263-
#[derive(serde::Deserialize)]
264-
struct UnusedExterns {
265-
unused_extern_names: Vec<String>,
266-
}
267264
if let Ok(uext) = serde_json::from_str::<UnusedExterns>(line) {
268-
unused_dep_state.record_unused_externs_for_unit(
269-
&unit_deps,
270-
unit,
271-
uext.unused_extern_names,
272-
);
265+
unused_dep_state.record_unused_externs_for_unit(&unit_deps, unit, uext);
273266
// Supress output of the json formatted unused extern message
274267
return Ok(());
275268
}

0 commit comments

Comments
 (0)