From 820537c70643557a6376863d52911318a9770111 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Wed, 17 Feb 2021 20:41:38 -0800 Subject: [PATCH] Change Fixing to Fixed, and add a verbose "Fixing". What was previously "Fixing" was a message for after the fixes had been applied. I think it would be clearer if it said "Fixed", to indicate that the fixes had actually finished. The new "Fixing" is posted just before it starts. This is verbose-only since it is a little noisy. --- crates/cargo-test-support/src/lib.rs | 1 + src/cargo/ops/fix.rs | 9 +++- src/cargo/util/diagnostic_server.rs | 11 +++- tests/testsuite/fix.rs | 76 ++++++++++++++-------------- tests/testsuite/glob_targets.rs | 4 ++ 5 files changed, 60 insertions(+), 41 deletions(-) diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs index fbaa9dd0d..b23561ad7 100644 --- a/crates/cargo-test-support/src/lib.rs +++ b/crates/cargo-test-support/src/lib.rs @@ -1524,6 +1524,7 @@ fn substitute_macros(input: &str) -> String { ("[REPLACING]", " Replacing"), ("[UNPACKING]", " Unpacking"), ("[SUMMARY]", " Summary"), + ("[FIXED]", " Fixed"), ("[FIXING]", " Fixing"), ("[EXE]", env::consts::EXE_SUFFIX), ("[IGNORED]", " Ignored"), diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index 0ba8b41c6..78e29ab81 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -238,7 +238,7 @@ pub fn fix_maybe_exec_rustc() -> CargoResult { if output.status.success() { for (path, file) in fixes.files.iter() { - Message::Fixing { + Message::Fixed { file: path.clone(), fixes: file.fixes_applied, } @@ -695,7 +695,12 @@ impl FixArgs { fn verify_not_preparing_for_enabled_edition(&self) -> CargoResult<()> { let edition = match self.prepare_for_edition { Some(s) => s, - None => return Ok(()), + None => { + return Message::Fixing { + file: self.file.display().to_string(), + } + .post(); + } }; let enabled = match self.enabled_edition { Some(s) => s, diff --git a/src/cargo/util/diagnostic_server.rs b/src/cargo/util/diagnostic_server.rs index dd75e1c18..dba68c08a 100644 --- a/src/cargo/util/diagnostic_server.rs +++ b/src/cargo/util/diagnostic_server.rs @@ -33,6 +33,9 @@ const PLEASE_REPORT_THIS_BUG: &str = pub enum Message { Fixing { file: String, + }, + Fixed { + file: String, fixes: u32, }, FixFailed { @@ -89,10 +92,14 @@ impl<'a> DiagnosticPrinter<'a> { pub fn print(&mut self, msg: &Message) -> CargoResult<()> { match msg { - Message::Fixing { file, fixes } => { + Message::Fixing { file } => self + .config + .shell() + .verbose(|shell| shell.status("Fixing", file)), + Message::Fixed { file, fixes } => { let msg = if *fixes == 1 { "fix" } else { "fixes" }; let msg = format!("{} ({} {})", file, fixes, msg); - self.config.shell().status("Fixing", msg) + self.config.shell().status("Fixed", msg) } Message::ReplaceFailed { file, message } => { let msg = format!("error applying suggestions to `{}`\n", file); diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs index cbb70ad22..c87f957e1 100644 --- a/tests/testsuite/fix.rs +++ b/tests/testsuite/fix.rs @@ -1,8 +1,9 @@ //! Tests for the `cargo fix` command. +use cargo::core::Edition; use cargo_test_support::git; use cargo_test_support::paths; -use cargo_test_support::{basic_manifest, project}; +use cargo_test_support::{basic_manifest, is_nightly, project}; #[cargo_test] fn do_not_fix_broken_builds() { @@ -161,7 +162,7 @@ fn broken_fixes_backed_out() { ) .with_stderr_contains("Original diagnostics will follow.") .with_stderr_contains("[WARNING] variable does not need to be mutable") - .with_stderr_does_not_contain("[..][FIXING][..]") + .with_stderr_does_not_contain("[..][FIXED][..]") .run(); // Make sure the fix which should have been applied was backed out @@ -213,9 +214,9 @@ fn fix_path_deps() { .with_stderr_unordered( "\ [CHECKING] bar v0.1.0 ([..]) -[FIXING] bar/src/lib.rs (1 fix) +[FIXED] bar/src/lib.rs (1 fix) [CHECKING] foo v0.1.0 ([..]) -[FIXING] src/lib.rs (1 fix) +[FIXED] src/lib.rs (1 fix) [FINISHED] [..] ", ) @@ -285,7 +286,7 @@ fn prepare_for_2018() { let stderr = "\ [CHECKING] foo v0.0.1 ([..]) -[FIXING] src/lib.rs (2 fixes) +[FIXED] src/lib.rs (2 fixes) [FINISHED] [..] "; p.cargo("fix --edition --allow-no-vcs") @@ -319,14 +320,14 @@ fn local_paths() { ) .build(); - let stderr = "\ -[CHECKING] foo v0.0.1 ([..]) -[FIXING] src/lib.rs (1 fix) -[FINISHED] [..] -"; - p.cargo("fix --edition --allow-no-vcs") - .with_stderr(stderr) + .with_stderr( + "\ +[CHECKING] foo v0.0.1 ([..]) +[FIXED] src/lib.rs (1 fix) +[FINISHED] [..] +", + ) .with_stdout("") .run(); @@ -372,7 +373,7 @@ fn upgrade_extern_crate() { let stderr = "\ [CHECKING] bar v0.1.0 ([..]) [CHECKING] foo v0.1.0 ([..]) -[FIXING] src/lib.rs (1 fix) +[FIXED] src/lib.rs (1 fix) [FINISHED] [..] "; p.cargo("fix --allow-no-vcs") @@ -403,14 +404,15 @@ fn specify_rustflags() { ) .build(); - let stderr = "\ -[CHECKING] foo v0.0.1 ([..]) -[FIXING] src/lib.rs (1 fix) -[FINISHED] [..] -"; p.cargo("fix --edition --allow-no-vcs") .env("RUSTFLAGS", "-C linker=cc") - .with_stderr(stderr) + .with_stderr( + "\ +[CHECKING] foo v0.0.1 ([..]) +[FIXED] src/lib.rs (1 fix) +[FINISHED] [..] +", + ) .with_stdout("") .run(); } @@ -445,7 +447,7 @@ fn fixes_extra_mut() { let stderr = "\ [CHECKING] foo v0.0.1 ([..]) -[FIXING] src/lib.rs (1 fix) +[FIXED] src/lib.rs (1 fix) [FINISHED] [..] "; p.cargo("fix --allow-no-vcs") @@ -472,7 +474,7 @@ fn fixes_two_missing_ampersands() { let stderr = "\ [CHECKING] foo v0.0.1 ([..]) -[FIXING] src/lib.rs (2 fixes) +[FIXED] src/lib.rs (2 fixes) [FINISHED] [..] "; p.cargo("fix --allow-no-vcs") @@ -498,7 +500,7 @@ fn tricky() { let stderr = "\ [CHECKING] foo v0.0.1 ([..]) -[FIXING] src/lib.rs (2 fixes) +[FIXED] src/lib.rs (2 fixes) [FINISHED] [..] "; p.cargo("fix --allow-no-vcs") @@ -594,8 +596,8 @@ fn fix_two_files() { p.cargo("fix --allow-no-vcs") .env("__CARGO_FIX_YOLO", "1") - .with_stderr_contains("[FIXING] src/bar.rs (1 fix)") - .with_stderr_contains("[FIXING] src/lib.rs (1 fix)") + .with_stderr_contains("[FIXED] src/bar.rs (1 fix)") + .with_stderr_contains("[FIXED] src/lib.rs (1 fix)") .run(); assert!(!p.read_file("src/lib.rs").contains("let mut x = 3;")); assert!(!p.read_file("src/bar.rs").contains("let mut x = 3;")); @@ -629,16 +631,16 @@ fn fixes_missing_ampersand() { .env("__CARGO_FIX_YOLO", "1") .with_stdout("") .with_stderr_contains("[COMPILING] foo v0.0.1 ([..])") - .with_stderr_contains("[FIXING] build.rs (1 fix)") + .with_stderr_contains("[FIXED] build.rs (1 fix)") // Don't assert number of fixes for this one, as we don't know if we're // fixing it once or twice! We run this all concurrently, and if we // compile (and fix) in `--test` mode first, we get two fixes. Otherwise // we'll fix one non-test thing, and then fix another one later in // test mode. - .with_stderr_contains("[FIXING] src/lib.rs[..]") - .with_stderr_contains("[FIXING] src/main.rs (1 fix)") - .with_stderr_contains("[FIXING] examples/foo.rs (1 fix)") - .with_stderr_contains("[FIXING] tests/a.rs (1 fix)") + .with_stderr_contains("[FIXED] src/lib.rs[..]") + .with_stderr_contains("[FIXED] src/main.rs (1 fix)") + .with_stderr_contains("[FIXED] examples/foo.rs (1 fix)") + .with_stderr_contains("[FIXED] tests/a.rs (1 fix)") .with_stderr_contains("[FINISHED] [..]") .run(); p.cargo("build").run(); @@ -836,14 +838,14 @@ fn fix_overlapping() { ) .build(); - let stderr = "\ -[CHECKING] foo [..] -[FIXING] src/lib.rs (2 fixes) -[FINISHED] dev [..] -"; - p.cargo("fix --allow-no-vcs --prepare-for 2018 --lib") - .with_stderr(stderr) + .with_stderr( + "\ +[CHECKING] foo [..] +[FIXED] src/lib.rs (2 fixes) +[FINISHED] dev [..] +", + ) .run(); let contents = p.read_file("src/lib.rs"); @@ -876,7 +878,7 @@ fn fix_idioms() { let stderr = "\ [CHECKING] foo [..] -[FIXING] src/lib.rs (1 fix) +[FIXED] src/lib.rs (1 fix) [FINISHED] [..] "; p.cargo("fix --edition-idioms --allow-no-vcs") diff --git a/tests/testsuite/glob_targets.rs b/tests/testsuite/glob_targets.rs index a2dc4fdff..643572e42 100644 --- a/tests/testsuite/glob_targets.rs +++ b/tests/testsuite/glob_targets.rs @@ -150,6 +150,7 @@ fn fix_example() { "\ [CHECKING] foo v0.0.1 ([CWD]) [RUNNING] `[..] rustc --crate-name example1 [..]` +[FIXING] examples/example1.rs [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] ", ) @@ -164,6 +165,7 @@ fn fix_bin() { "\ [CHECKING] foo v0.0.1 ([CWD]) [RUNNING] `[..] rustc --crate-name bin1 [..]` +[FIXING] src/bin/bin1.rs [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] ", ) @@ -178,6 +180,7 @@ fn fix_bench() { "\ [CHECKING] foo v0.0.1 ([CWD]) [RUNNING] `[..] rustc --crate-name bench1 [..]` +[FIXING] benches/bench1.rs [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] ", ) @@ -192,6 +195,7 @@ fn fix_test() { "\ [CHECKING] foo v0.0.1 ([CWD]) [RUNNING] `[..] rustc --crate-name test1 [..]` +[FIXING] tests/test1.rs [FINISHED] dev [unoptimized + debuginfo] target(s) in [..] ", )