Skip to content

Commit 283c3f4

Browse files
committed
add a note that some warnings (and/or errors) can be auto-fixed
1 parent 8494149 commit 283c3f4

File tree

5 files changed

+191
-7
lines changed

5 files changed

+191
-7
lines changed

src/cargo/core/compiler/job_queue.rs

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,14 @@ struct DrainState<'cfg> {
135135
/// the number that were suppressed because they were duplicates of a
136136
/// previous warning.
137137
warning_count: HashMap<JobId, (usize, usize)>,
138+
/// This keeps track of the total number of fixable errors and warnings.
139+
///i.e. [Applicability::MachineApplicable].
140+
///
141+
/// The first value is the number of fixable errors, the second value is
142+
/// the number of fixable warnings.
143+
///
144+
/// [Applicability::MachineApplicable]: rustfix::diagnostics::Applicability::MachineApplicable
145+
fixable_count: (usize, usize),
138146
active: HashMap<JobId, Unit>,
139147
compiled: HashSet<PackageId>,
140148
documented: HashSet<PackageId>,
@@ -311,10 +319,12 @@ enum Message {
311319
id: JobId,
312320
level: String,
313321
diag: String,
322+
fixable: bool,
314323
},
315324
WarningCount {
316325
id: JobId,
317326
emitted: bool,
327+
fixable: bool,
318328
},
319329
FixDiagnostic(diagnostic_server::Message),
320330
Token(io::Result<Acquired>),
@@ -363,20 +373,22 @@ impl<'a, 'cfg> JobState<'a, 'cfg> {
363373
Ok(())
364374
}
365375

366-
pub fn emit_diag(&self, level: String, diag: String) -> CargoResult<()> {
376+
pub fn emit_diag(&self, level: String, diag: String, fixable: bool) -> CargoResult<()> {
367377
if let Some(dedupe) = self.output {
368378
let emitted = dedupe.emit_diag(&diag)?;
369379
if level == "warning" {
370380
self.messages.push(Message::WarningCount {
371381
id: self.id,
372382
emitted,
383+
fixable,
373384
});
374385
}
375386
} else {
376387
self.messages.push_bounded(Message::Diagnostic {
377388
id: self.id,
378389
level,
379390
diag,
391+
fixable,
380392
});
381393
}
382394
Ok(())
@@ -516,6 +528,7 @@ impl<'cfg> JobQueue<'cfg> {
516528
messages: Arc::new(Queue::new(100)),
517529
diag_dedupe: DiagDedupe::new(cx.bcx.config),
518530
warning_count: HashMap::new(),
531+
fixable_count: (0, 0),
519532
active: HashMap::new(),
520533
compiled: HashSet::new(),
521534
documented: HashSet::new(),
@@ -668,14 +681,26 @@ impl<'cfg> DrainState<'cfg> {
668681
shell.print_ansi_stderr(err.as_bytes())?;
669682
shell.err().write_all(b"\n")?;
670683
}
671-
Message::Diagnostic { id, level, diag } => {
684+
Message::Diagnostic {
685+
id,
686+
level,
687+
diag,
688+
fixable,
689+
} => {
672690
let emitted = self.diag_dedupe.emit_diag(&diag)?;
673691
if level == "warning" {
674-
self.bump_warning_count(id, emitted);
692+
self.bump_warning_count(id, emitted, fixable);
693+
}
694+
if level == "error" && fixable {
695+
self.fixable_count.0 += 1;
675696
}
676697
}
677-
Message::WarningCount { id, emitted } => {
678-
self.bump_warning_count(id, emitted);
698+
Message::WarningCount {
699+
id,
700+
emitted,
701+
fixable,
702+
} => {
703+
self.bump_warning_count(id, emitted, fixable);
679704
}
680705
Message::FixDiagnostic(msg) => {
681706
self.print.print(&msg)?;
@@ -860,6 +885,34 @@ impl<'cfg> DrainState<'cfg> {
860885
}
861886
self.progress.clear();
862887

888+
let fixable_errors = match self.fixable_count.0 {
889+
0 => String::new(),
890+
1 => "1 error".to_string(),
891+
n => format!("{n} errors"),
892+
};
893+
894+
let fixable_warnings = match self.fixable_count.1 {
895+
0 => String::new(),
896+
1 => "1 warning".to_string(),
897+
n => format!("{n} warnings"),
898+
};
899+
900+
let fixable = match (fixable_errors.is_empty(), fixable_warnings.is_empty()) {
901+
(true, true) => String::new(),
902+
(true, false) => fixable_warnings,
903+
(false, true) => fixable_errors,
904+
(false, false) => format!("{fixable_errors} and {fixable_warnings}"),
905+
};
906+
907+
if !fixable.is_empty() {
908+
drop(
909+
cx.bcx
910+
.config
911+
.shell()
912+
.note(format!("to fix {fixable} automatically, run `cargo fix`")),
913+
);
914+
}
915+
863916
let profile_name = cx.bcx.build_config.requested_profile;
864917
// NOTE: this may be a bit inaccurate, since this may not display the
865918
// profile for what was actually built. Profile overrides can change
@@ -1116,12 +1169,15 @@ impl<'cfg> DrainState<'cfg> {
11161169
Ok(())
11171170
}
11181171

1119-
fn bump_warning_count(&mut self, id: JobId, emitted: bool) {
1172+
fn bump_warning_count(&mut self, id: JobId, emitted: bool, fixable: bool) {
11201173
let cnts = self.warning_count.entry(id).or_default();
11211174
cnts.0 += 1;
11221175
if !emitted {
11231176
cnts.1 += 1;
11241177
}
1178+
if fixable {
1179+
self.fixable_count.1 += 1;
1180+
}
11251181
}
11261182

11271183
/// Displays a final report of the warnings emitted by a particular job.

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

@@ -1407,7 +1408,9 @@ fn on_stderr_line_inner(
14071408
rendered: String,
14081409
message: String,
14091410
level: String,
1411+
children: Vec<Diagnostic>,
14101412
}
1413+
14111414
if let Ok(mut msg) = serde_json::from_str::<CompilerMessage>(compiler_message.get()) {
14121415
if msg.message.starts_with("aborting due to")
14131416
|| msg.message.ends_with("warning emitted")
@@ -1430,8 +1433,19 @@ fn on_stderr_line_inner(
14301433
.expect("strip should never fail")
14311434
};
14321435
if options.show_diagnostics {
1436+
let machine_applicable: bool = msg
1437+
.children
1438+
.iter()
1439+
.map(|child| {
1440+
child
1441+
.spans
1442+
.iter()
1443+
.filter_map(|span| span.suggestion_applicability)
1444+
.any(|app| app == Applicability::MachineApplicable)
1445+
})
1446+
.any(|b| b);
14331447
count_diagnostic(&msg.level, options);
1434-
state.emit_diag(msg.level, rendered)?;
1448+
state.emit_diag(msg.level, rendered, machine_applicable)?;
14351449
}
14361450
return Ok(true);
14371451
}

tests/testsuite/check.rs

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,80 @@ fn check_fail() {
6969
.run();
7070
}
7171

72+
#[cargo_test]
73+
fn check_fixable_warning() {
74+
let foo = project()
75+
.file(
76+
"Cargo.toml",
77+
r#"
78+
[package]
79+
name = "foo"
80+
version = "0.0.1"
81+
authors = []
82+
"#,
83+
)
84+
.file("src/lib.rs", "use std::io;")
85+
.build();
86+
87+
foo.cargo("check")
88+
.with_status(0)
89+
.with_stderr_contains("[..]note: to fix 1 warning automatically, run `cargo fix`[..]")
90+
.run();
91+
}
92+
93+
#[cargo_test]
94+
fn check_fixable_error() {
95+
let foo = project()
96+
.file(
97+
"Cargo.toml",
98+
r#"
99+
[package]
100+
name = "foo"
101+
version = "0.0.1"
102+
authors = []
103+
"#,
104+
)
105+
.file("src/lib.rs", "#[derive(Debug(x))]\nstruct Foo;")
106+
.build();
107+
108+
foo.cargo("check")
109+
.with_status(101)
110+
.with_stderr_contains("[..]note: to fix 1 error automatically, run `cargo fix`[..]")
111+
.with_stderr_does_not_contain(
112+
"[..]note: to fix 1 warning automatically, run `cargo fix`[..]",
113+
)
114+
.run();
115+
}
116+
117+
#[cargo_test]
118+
fn check_fixable_both() {
119+
let foo = project()
120+
.file(
121+
"Cargo.toml",
122+
r#"
123+
[package]
124+
name = "foo"
125+
version = "0.0.1"
126+
authors = []
127+
"#,
128+
)
129+
.file(
130+
"src/lib.rs",
131+
"use std::io;\n#[derive(Debug(x))]\nstruct Foo;",
132+
)
133+
.build();
134+
135+
foo.cargo("check")
136+
.with_status(101)
137+
.with_stderr_contains(
138+
"[..]note: to fix 1 error and 1 warning automatically, run `cargo fix`[..]",
139+
)
140+
.with_stderr_does_not_contain(
141+
"[..]note: to fix 1 warning automatically, run `cargo fix`[..]",
142+
)
143+
.run();
144+
}
145+
72146
#[cargo_test]
73147
fn custom_derive() {
74148
let foo = project()

tests/testsuite/install.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,3 +2032,41 @@ fn install_semver_metadata() {
20322032
)
20332033
.run();
20342034
}
2035+
2036+
#[cargo_test]
2037+
fn no_auto_fix_note() {
2038+
Package::new("auto_fix", "0.0.1")
2039+
.file("src/lib.rs", "use std::io;")
2040+
.file(
2041+
"src/main.rs",
2042+
&format!("extern crate {}; use std::io; fn main() {{}}", "auto_fix"),
2043+
)
2044+
.publish();
2045+
2046+
// This should not contain something along the lines of:
2047+
// "note: to fix 1 warning automatically, run `cargo fix"
2048+
//
2049+
// This is checked by matching the full output as `with_stderr_does_not_contain`
2050+
// can be brittle
2051+
cargo_process("install auto_fix")
2052+
.with_stderr(
2053+
"\
2054+
[UPDATING] `[..]` index
2055+
[DOWNLOADING] crates ...
2056+
[DOWNLOADED] auto_fix v0.0.1 (registry [..])
2057+
[INSTALLING] auto_fix v0.0.1
2058+
[COMPILING] auto_fix v0.0.1
2059+
[FINISHED] release [optimized] target(s) in [..]
2060+
[INSTALLING] [CWD]/home/.cargo/bin/auto_fix[EXE]
2061+
[INSTALLED] package `auto_fix v0.0.1` (executable `auto_fix[EXE]`)
2062+
[WARNING] be sure to add `[..]` to your PATH to be able to run the installed binaries
2063+
",
2064+
)
2065+
.run();
2066+
assert_has_installed_exe(cargo_home(), "auto_fix");
2067+
2068+
cargo_process("uninstall auto_fix")
2069+
.with_stderr("[REMOVING] [CWD]/home/.cargo/bin/auto_fix[EXE]")
2070+
.run();
2071+
assert_has_not_installed_exe(cargo_home(), "auto_fix");
2072+
}

tests/testsuite/messages.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ fn deduplicate_messages_basic() {
6262
"{}\
6363
warning: `foo` (lib) generated 1 warning
6464
warning: `foo` (lib test) generated 1 warning (1 duplicate)
65+
note: to fix 2 warnings automatically, run `cargo fix`
6566
[FINISHED] [..]
6667
[EXECUTABLE] unittests src/lib.rs (target/debug/deps/foo-[..][EXE])
6768
",
@@ -106,6 +107,7 @@ fn deduplicate_messages_mismatched_warnings() {
106107
warning: `foo` (lib) generated 1 warning
107108
{}\
108109
warning: `foo` (lib test) generated 2 warnings (1 duplicate)
110+
note: to fix 2 warnings automatically, run `cargo fix`
109111
[FINISHED] [..]
110112
[EXECUTABLE] unittests src/lib.rs (target/debug/deps/foo-[..][EXE])
111113
",

0 commit comments

Comments
 (0)