Skip to content

Commit 746b8d8

Browse files
committed
add a note that some warnings can be auto-fixed
1 parent 1c1e9a6 commit 746b8d8

File tree

4 files changed

+447
-20
lines changed

4 files changed

+447
-20
lines changed

src/cargo/core/compiler/job_queue.rs

Lines changed: 122 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,8 @@ struct DrainState<'cfg> {
129129
messages: Arc<Queue<Message>>,
130130
/// Diagnostic deduplication support.
131131
diag_dedupe: DiagDedupe<'cfg>,
132-
/// Count of warnings, used to print a summary after the job succeeds.
133-
///
134-
/// First value is the total number of warnings, and the second value is
135-
/// the number that were suppressed because they were duplicates of a
136-
/// previous warning.
137-
warning_count: HashMap<JobId, (usize, usize)>,
132+
/// Count of warnings, used to print a summary after the job succeeds
133+
warning_count: HashMap<JobId, WarningCount>,
138134
active: HashMap<JobId, Unit>,
139135
compiled: HashSet<PackageId>,
140136
documented: HashSet<PackageId>,
@@ -170,6 +166,49 @@ struct DrainState<'cfg> {
170166
per_package_future_incompat_reports: Vec<FutureIncompatReportPackage>,
171167
}
172168

169+
/// Count of warnings, used to print a summary after the job succeeds
170+
#[derive(Default)]
171+
pub struct WarningCount {
172+
/// total number of warnings
173+
pub total: usize,
174+
/// number of warnings that were suppressed because they
175+
/// were duplicates of a previous warning
176+
pub duplicates: usize,
177+
/// number of fixable warnings
178+
/// set to -1 if there are any errors
179+
pub fixable: FixableWarnings,
180+
}
181+
182+
impl WarningCount {
183+
/// If an error is seem this should be called
184+
/// to set the `fixable` count to -1 or
185+
/// no fixable warnings allowed
186+
fn disallow_fixable(&mut self) {
187+
self.fixable = FixableWarnings::NowAllowed;
188+
}
189+
190+
/// Checks fixable if warnings are allowed
191+
/// fixable warnings are allowed if there are no
192+
/// errors. If an error was seen `fixable`
193+
/// will be -1.
194+
fn fixable_allowed(&self) -> bool {
195+
match &self.fixable {
196+
FixableWarnings::NowAllowed => false,
197+
FixableWarnings::Positive(_) | FixableWarnings::Zero => true,
198+
}
199+
}
200+
}
201+
202+
/// Used to keep track of how many fixable warnings there are
203+
/// and if fixable warnings are allowed
204+
#[derive(Default)]
205+
pub enum FixableWarnings {
206+
NowAllowed,
207+
#[default]
208+
Zero,
209+
Positive(usize),
210+
}
211+
173212
pub struct ErrorsDuringDrain {
174213
pub count: usize,
175214
}
@@ -311,10 +350,12 @@ enum Message {
311350
id: JobId,
312351
level: String,
313352
diag: String,
353+
fixable: bool,
314354
},
315355
WarningCount {
316356
id: JobId,
317357
emitted: bool,
358+
fixable: bool,
318359
},
319360
FixDiagnostic(diagnostic_server::Message),
320361
Token(io::Result<Acquired>),
@@ -363,20 +404,22 @@ impl<'a, 'cfg> JobState<'a, 'cfg> {
363404
Ok(())
364405
}
365406

366-
pub fn emit_diag(&self, level: String, diag: String) -> CargoResult<()> {
407+
pub fn emit_diag(&self, level: String, diag: String, fixable: bool) -> CargoResult<()> {
367408
if let Some(dedupe) = self.output {
368409
let emitted = dedupe.emit_diag(&diag)?;
369410
if level == "warning" {
370411
self.messages.push(Message::WarningCount {
371412
id: self.id,
372413
emitted,
414+
fixable,
373415
});
374416
}
375417
} else {
376418
self.messages.push_bounded(Message::Diagnostic {
377419
id: self.id,
378420
level,
379421
diag,
422+
fixable,
380423
});
381424
}
382425
Ok(())
@@ -679,14 +722,28 @@ impl<'cfg> DrainState<'cfg> {
679722
shell.print_ansi_stderr(err.as_bytes())?;
680723
shell.err().write_all(b"\n")?;
681724
}
682-
Message::Diagnostic { id, level, diag } => {
725+
Message::Diagnostic {
726+
id,
727+
level,
728+
diag,
729+
fixable,
730+
} => {
683731
let emitted = self.diag_dedupe.emit_diag(&diag)?;
684732
if level == "warning" {
685-
self.bump_warning_count(id, emitted);
733+
self.bump_warning_count(id, emitted, fixable);
734+
}
735+
if level == "error" {
736+
let cnts = self.warning_count.entry(id).or_default();
737+
// If there is an error, the `cargo fix`message should not show
738+
cnts.disallow_fixable();
686739
}
687740
}
688-
Message::WarningCount { id, emitted } => {
689-
self.bump_warning_count(id, emitted);
741+
Message::WarningCount {
742+
id,
743+
emitted,
744+
fixable,
745+
} => {
746+
self.bump_warning_count(id, emitted, fixable);
690747
}
691748
Message::FixDiagnostic(msg) => {
692749
self.print.print(&msg)?;
@@ -1127,19 +1184,34 @@ impl<'cfg> DrainState<'cfg> {
11271184
Ok(())
11281185
}
11291186

1130-
fn bump_warning_count(&mut self, id: JobId, emitted: bool) {
1187+
fn bump_warning_count(&mut self, id: JobId, emitted: bool, fixable: bool) {
11311188
let cnts = self.warning_count.entry(id).or_default();
1132-
cnts.0 += 1;
1189+
cnts.total += 1;
11331190
if !emitted {
1134-
cnts.1 += 1;
1191+
cnts.duplicates += 1;
1192+
// Don't add to fixable if it's already been emitted
1193+
} else if fixable {
1194+
// Do not add anything to the fixable warning count if
1195+
// is `-1` since that indicates there was an error while
1196+
// building this `Unit`
1197+
if cnts.fixable_allowed() {
1198+
cnts.fixable = match cnts.fixable {
1199+
FixableWarnings::NowAllowed => FixableWarnings::NowAllowed,
1200+
FixableWarnings::Zero => FixableWarnings::Positive(1),
1201+
FixableWarnings::Positive(fixable) => FixableWarnings::Positive(fixable + 1),
1202+
};
1203+
}
11351204
}
11361205
}
11371206

11381207
/// Displays a final report of the warnings emitted by a particular job.
11391208
fn report_warning_count(&mut self, config: &Config, id: JobId) {
11401209
let count = match self.warning_count.remove(&id) {
1141-
Some(count) => count,
1142-
None => return,
1210+
// An error could add an entry for a `Unit`
1211+
// with 0 warnings but having fixable
1212+
// warnings be disallowed
1213+
Some(count) if count.total > 0 => count,
1214+
None | Some(_) => return,
11431215
};
11441216
let unit = &self.active[&id];
11451217
let mut message = format!("`{}` ({}", unit.pkg.name(), unit.target.description_named());
@@ -1151,15 +1223,47 @@ impl<'cfg> DrainState<'cfg> {
11511223
message.push_str(" doc");
11521224
}
11531225
message.push_str(") generated ");
1154-
match count.0 {
1226+
match count.total {
11551227
1 => message.push_str("1 warning"),
11561228
n => drop(write!(message, "{} warnings", n)),
11571229
};
1158-
match count.1 {
1230+
match count.duplicates {
11591231
0 => {}
11601232
1 => message.push_str(" (1 duplicate)"),
11611233
n => drop(write!(message, " ({} duplicates)", n)),
11621234
}
1235+
// Only show the `cargo fix` message if its a local `Unit`
1236+
if unit.is_local() && config.nightly_features_allowed {
1237+
// Do not show this if there are any errors or no fixable warnings
1238+
if let FixableWarnings::Positive(fixable) = count.fixable {
1239+
// `cargo fix` doesnt have an option for custom builds
1240+
if !unit.target.is_custom_build() {
1241+
let mut command = {
1242+
let named = unit.target.description_named();
1243+
// if its a lib we need to add the package to fix
1244+
if named == "lib" {
1245+
format!("{} -p {}", named, unit.pkg.name())
1246+
} else {
1247+
named
1248+
}
1249+
};
1250+
if unit.mode.is_rustc_test()
1251+
&& !(unit.target.is_test() || unit.target.is_bench())
1252+
{
1253+
command.push_str(" --tests");
1254+
}
1255+
let mut suggestions = format!("{} suggestion", fixable);
1256+
if fixable > 1 {
1257+
suggestions.push_str("s")
1258+
}
1259+
drop(write!(
1260+
message,
1261+
" (run `cargo fix --{}` to apply {})",
1262+
command, suggestions
1263+
))
1264+
}
1265+
}
1266+
}
11631267
// Errors are ignored here because it is tricky to handle them
11641268
// correctly, and they aren't important.
11651269
drop(config.shell().warn(message));

src/cargo/core/compiler/mod.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ use crate::util::interning::InternedString;
6161
use crate::util::machine_message::{self, Message};
6262
use crate::util::{add_path_args, internal, iter_join_onto, profile};
6363
use cargo_util::{paths, ProcessBuilder, ProcessError};
64+
use rustfix::diagnostics::{Applicability, Diagnostic};
6465

6566
const RUSTDOC_CRATE_VERSION_FLAG: &str = "--crate-version";
6667

@@ -1420,7 +1421,9 @@ fn on_stderr_line_inner(
14201421
rendered: String,
14211422
message: String,
14221423
level: String,
1424+
children: Vec<Diagnostic>,
14231425
}
1426+
14241427
if let Ok(mut msg) = serde_json::from_str::<CompilerMessage>(compiler_message.get()) {
14251428
if msg.message.starts_with("aborting due to")
14261429
|| msg.message.ends_with("warning emitted")
@@ -1443,8 +1446,19 @@ fn on_stderr_line_inner(
14431446
.expect("strip should never fail")
14441447
};
14451448
if options.show_diagnostics {
1449+
let machine_applicable: bool = msg
1450+
.children
1451+
.iter()
1452+
.map(|child| {
1453+
child
1454+
.spans
1455+
.iter()
1456+
.filter_map(|span| span.suggestion_applicability)
1457+
.any(|app| app == Applicability::MachineApplicable)
1458+
})
1459+
.any(|b| b);
14461460
count_diagnostic(&msg.level, options);
1447-
state.emit_diag(msg.level, rendered)?;
1461+
state.emit_diag(msg.level, rendered, machine_applicable)?;
14481462
}
14491463
return Ok(true);
14501464
}

0 commit comments

Comments
 (0)