Skip to content

Commit

Permalink
add a note that some warnings can be auto-fixed
Browse files Browse the repository at this point in the history
  • Loading branch information
Muscraft committed Oct 18, 2022
1 parent 3ff0443 commit dd69ce1
Show file tree
Hide file tree
Showing 5 changed files with 405 additions and 13 deletions.
93 changes: 83 additions & 10 deletions src/cargo/core/compiler/job_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,11 @@ struct DrainState<'cfg> {
diag_dedupe: DiagDedupe<'cfg>,
/// Count of warnings, used to print a summary after the job succeeds.
///
/// First value is the total number of warnings, and the second value is
/// First value is the total number of warnings, the second value is
/// the number that were suppressed because they were duplicates of a
/// previous warning.
warning_count: HashMap<JobId, (usize, usize)>,
/// previous warning, and the third value is for the number of fixable
/// warnings (set to -1 if there are any errors).
warning_count: HashMap<JobId, (usize, usize, isize)>,
active: HashMap<JobId, Unit>,
compiled: HashSet<PackageId>,
documented: HashSet<PackageId>,
Expand Down Expand Up @@ -311,10 +312,12 @@ enum Message {
id: JobId,
level: String,
diag: String,
fixable: bool,
},
WarningCount {
id: JobId,
emitted: bool,
fixable: bool,
},
FixDiagnostic(diagnostic_server::Message),
Token(io::Result<Acquired>),
Expand Down Expand Up @@ -363,20 +366,22 @@ impl<'a, 'cfg> JobState<'a, 'cfg> {
Ok(())
}

pub fn emit_diag(&self, level: String, diag: String) -> CargoResult<()> {
pub fn emit_diag(&self, level: String, diag: String, fixable: bool) -> CargoResult<()> {
if let Some(dedupe) = self.output {
let emitted = dedupe.emit_diag(&diag)?;
if level == "warning" {
self.messages.push(Message::WarningCount {
id: self.id,
emitted,
fixable,
});
}
} else {
self.messages.push_bounded(Message::Diagnostic {
id: self.id,
level,
diag,
fixable,
});
}
Ok(())
Expand Down Expand Up @@ -679,14 +684,31 @@ impl<'cfg> DrainState<'cfg> {
shell.print_ansi_stderr(err.as_bytes())?;
shell.err().write_all(b"\n")?;
}
Message::Diagnostic { id, level, diag } => {
Message::Diagnostic {
id,
level,
diag,
fixable,
} => {
let emitted = self.diag_dedupe.emit_diag(&diag)?;
if level == "warning" {
self.bump_warning_count(id, emitted);
self.bump_warning_count(id, emitted, fixable);
}
if level == "error" {
// If there is an error set fixable count to -1 so the `cargo fix`
// message does not show
let cnts = self.warning_count.entry(id).or_default();
if fixable {
cnts.2 = -1;
}
}
}
Message::WarningCount { id, emitted } => {
self.bump_warning_count(id, emitted);
Message::WarningCount {
id,
emitted,
fixable,
} => {
self.bump_warning_count(id, emitted, fixable);
}
Message::FixDiagnostic(msg) => {
self.print.print(&msg)?;
Expand Down Expand Up @@ -1127,18 +1149,34 @@ impl<'cfg> DrainState<'cfg> {
Ok(())
}

fn bump_warning_count(&mut self, id: JobId, emitted: bool) {
fn bump_warning_count(&mut self, id: JobId, emitted: bool, fixable: bool) {
let cnts = self.warning_count.entry(id).or_default();
cnts.0 += 1;
if !emitted {
cnts.1 += 1;
// Don't add to fixable if it's already been emitted
} else if fixable {
// Do not add anything to the fixable warning count if
// is `-1` since that indicates there was an error while
// building this `Unit`
if cnts.2 >= 0 {
cnts.2 += 1;
}
}
}

/// Displays a final report of the warnings emitted by a particular job.
fn report_warning_count(&mut self, config: &Config, id: JobId) {
let count = match self.warning_count.remove(&id) {
Some(count) => count,
Some(count) => {
// An error could add an entry for a `Unit`
// with 0 warnings but with count.2 = -1
if count.0 > 0 {
count
} else {
return;
}
}
None => return,
};
let unit = &self.active[&id];
Expand All @@ -1160,6 +1198,41 @@ impl<'cfg> DrainState<'cfg> {
1 => message.push_str(" (1 duplicate)"),
n => drop(write!(message, " ({} duplicates)", n)),
}
// Only show the `cargo fix` message if its a local `Unit`
if unit.is_local() && config.nightly_features_allowed {
match count.2 {
// Do not show this if there are any errors (-1) or no fixable warnings
-1 | 0 => {}
n => {
// `cargo fix` doesnt have an option for custom builds
if !unit.target.is_custom_build() {
let mut command = {
let named = unit.target.description_named();
// if its a lib we need to add the package to fix
if named == "lib" {
format!("{} -p {}", named, unit.pkg.name())
} else {
named
}
};
if unit.mode.is_rustc_test()
&& !(unit.target.is_test() || unit.target.is_bench())
{
command.push_str(" --tests");
}
let mut suggestions = format!("{} suggestion", n);
if n > 1 {
suggestions.push_str("s")
}
drop(write!(
message,
" (run `cargo fix --{}` to apply {})",
command, suggestions
))
}
}
}
}
// Errors are ignored here because it is tricky to handle them
// correctly, and they aren't important.
drop(config.shell().warn(message));
Expand Down
16 changes: 15 additions & 1 deletion src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ use crate::util::interning::InternedString;
use crate::util::machine_message::{self, Message};
use crate::util::{add_path_args, internal, iter_join_onto, profile};
use cargo_util::{paths, ProcessBuilder, ProcessError};
use rustfix::diagnostics::{Applicability, Diagnostic};

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

Expand Down Expand Up @@ -1420,7 +1421,9 @@ fn on_stderr_line_inner(
rendered: String,
message: String,
level: String,
children: Vec<Diagnostic>,
}

if let Ok(mut msg) = serde_json::from_str::<CompilerMessage>(compiler_message.get()) {
if msg.message.starts_with("aborting due to")
|| msg.message.ends_with("warning emitted")
Expand All @@ -1443,8 +1446,19 @@ fn on_stderr_line_inner(
.expect("strip should never fail")
};
if options.show_diagnostics {
let machine_applicable: bool = msg
.children
.iter()
.map(|child| {
child
.spans
.iter()
.filter_map(|span| span.suggestion_applicability)
.any(|app| app == Applicability::MachineApplicable)
})
.any(|b| b);
count_diagnostic(&msg.level, options);
state.emit_diag(msg.level, rendered)?;
state.emit_diag(msg.level, rendered, machine_applicable)?;
}
return Ok(true);
}
Expand Down
Loading

0 comments on commit dd69ce1

Please sign in to comment.