Auto merge of #87982 - m-ou-se:non-fmt-panic-assert-str, r=cjgillot

Add automatic migration for assert!(.., string). Fixes part of #87313.
rust-lang · Aug 15, 2021 · 2d2bc94 · 2d2bc94
2 parents c0490a2 + 8fedb31
commit 2d2bc94
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 29 deletions.
diff --git a/compiler/rustc_lint/src/non_fmt_panic.rs b/compiler/rustc_lint/src/non_fmt_panic.rs
@@ -120,13 +120,26 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
                 );
             }
         } else {
+            let ty = cx.typeck_results().expr_ty(arg);
+            // If this is a &str or String, we can confidently give the `"{}", ` suggestion.
+            let is_str = matches!(
+                ty.kind(),
+                ty::Ref(_, r, _) if *r.kind() == ty::Str,
+            ) || matches!(
+                ty.ty_adt_def(),
+                Some(ty_def) if cx.tcx.is_diagnostic_item(sym::string_type, ty_def.did),
+            );
             l.span_suggestion_verbose(
                 arg_span.shrink_to_lo(),
                 "add a \"{}\" format string to Display the message",
                 "\"{}\", ".into(),
-                Applicability::MaybeIncorrect,
+                if is_str {
+                    Applicability::MachineApplicable
+                } else {
+                    Applicability::MaybeIncorrect
+                },
             );
-            if panic == sym::std_panic_macro {
+            if !is_str && panic == sym::std_panic_macro {
                 if let Some((open, close, del)) = find_delimiters(cx, span) {
                     l.multipart_suggestion(
                         "or use std::panic::panic_any instead",

diff --git a/src/test/ui/non-fmt-panic.fixed b/src/test/ui/non-fmt-panic.fixed
@@ -0,0 +1,54 @@
+// run-rustfix
+// rustfix-only-machine-applicable
+// build-pass (FIXME(62277): should be check-pass)
+// aux-build:fancy-panic.rs
+
+extern crate fancy_panic;
+
+const C: &str = "abc {}";
+static S: &str = "{bla}";
+
+#[allow(unreachable_code)]
+fn main() {
+    panic!("{}", "here's a brace: {"); //~ WARN panic message contains a brace
+    std::panic!("{}", "another one: }"); //~ WARN panic message contains a brace
+    core::panic!("{}", "Hello {}"); //~ WARN panic message contains an unused formatting placeholder
+    assert!(false, "{}", "{:03x} {test} bla");
+    //~^ WARN panic message contains unused formatting placeholders
+    assert!(false, "{}", S);
+    //~^ WARN panic message is not a string literal
+    debug_assert!(false, "{}", "{{}} bla"); //~ WARN panic message contains braces
+    panic!("{}", C); //~ WARN panic message is not a string literal
+    panic!("{}", S); //~ WARN panic message is not a string literal
+    std::panic::panic_any(123); //~ WARN panic message is not a string literal
+    core::panic!("{}", &*"abc"); //~ WARN panic message is not a string literal
+    panic!("{}", concat!("{", "}")); //~ WARN panic message contains an unused formatting placeholder
+    panic!("{}", concat!("{", "{")); //~ WARN panic message contains braces
+
+    fancy_panic::fancy_panic!("test {} 123");
+    //~^ WARN panic message contains an unused formatting placeholder
+
+    fancy_panic::fancy_panic!(); // OK
+    fancy_panic::fancy_panic!(S); // OK
+
+    macro_rules! a {
+        () => { 123 };
+    }
+
+    std::panic::panic_any(a!()); //~ WARN panic message is not a string literal
+
+    panic!("{}", 1); //~ WARN panic message is not a string literal
+    assert!(false, "{}", 1); //~ WARN panic message is not a string literal
+    debug_assert!(false, "{}", 1); //~ WARN panic message is not a string literal
+
+    std::panic::panic_any(123); //~ WARN panic message is not a string literal
+    std::panic::panic_any(123); //~ WARN panic message is not a string literal
+
+    // Check that the lint only triggers for std::panic and core::panic,
+    // not any panic macro:
+    macro_rules! panic {
+        ($e:expr) => ();
+    }
+    panic!("{}"); // OK
+    panic!(S); // OK
+}
diff --git a/src/test/ui/non-fmt-panic.rs b/src/test/ui/non-fmt-panic.rs
@@ -1,3 +1,5 @@
+// run-rustfix
+// rustfix-only-machine-applicable
 // build-pass (FIXME(62277): should be check-pass)
 // aux-build:fancy-panic.rs
 

diff --git a/src/test/ui/non-fmt-panic.stderr b/src/test/ui/non-fmt-panic.stderr
@@ -1,5 +1,5 @@
 warning: panic message contains a brace
-  --> $DIR/non-fmt-panic.rs:11:29
+  --> $DIR/non-fmt-panic.rs:13:29
    |
 LL |     panic!("here's a brace: {");
    |                             ^
@@ -12,7 +12,7 @@ LL |     panic!("{}", "here's a brace: {");
    |            +++++
 
 warning: panic message contains a brace
-  --> $DIR/non-fmt-panic.rs:12:31
+  --> $DIR/non-fmt-panic.rs:14:31
    |
 LL |     std::panic!("another one: }");
    |                               ^
@@ -24,7 +24,7 @@ LL |     std::panic!("{}", "another one: }");
    |                 +++++
 
 warning: panic message contains an unused formatting placeholder
-  --> $DIR/non-fmt-panic.rs:13:25
+  --> $DIR/non-fmt-panic.rs:15:25
    |
 LL |     core::panic!("Hello {}");
    |                         ^^
@@ -40,7 +40,7 @@ LL |     core::panic!("{}", "Hello {}");
    |                  +++++
 
 warning: panic message contains unused formatting placeholders
-  --> $DIR/non-fmt-panic.rs:14:21
+  --> $DIR/non-fmt-panic.rs:16:21
    |
 LL |     assert!(false, "{:03x} {test} bla");
    |                     ^^^^^^ ^^^^^^
@@ -56,7 +56,7 @@ LL |     assert!(false, "{}", "{:03x} {test} bla");
    |                    +++++
 
 warning: panic message is not a string literal
-  --> $DIR/non-fmt-panic.rs:16:20
+  --> $DIR/non-fmt-panic.rs:18:20
    |
 LL |     assert!(false, S);
    |                    ^
@@ -69,7 +69,7 @@ LL |     assert!(false, "{}", S);
    |                    +++++
 
 warning: panic message contains braces
-  --> $DIR/non-fmt-panic.rs:18:27
+  --> $DIR/non-fmt-panic.rs:20:27
    |
 LL |     debug_assert!(false, "{{}} bla");
    |                           ^^^^
@@ -81,7 +81,7 @@ LL |     debug_assert!(false, "{}", "{{}} bla");
    |                          +++++
 
 warning: panic message is not a string literal
-  --> $DIR/non-fmt-panic.rs:19:12
+  --> $DIR/non-fmt-panic.rs:21:12
    |
 LL |     panic!(C);
    |            ^
@@ -92,13 +92,9 @@ help: add a "{}" format string to Display the message
    |
 LL |     panic!("{}", C);
    |            +++++
-help: or use std::panic::panic_any instead
-   |
-LL |     std::panic::panic_any(C);
-   |     ~~~~~~~~~~~~~~~~~~~~~
 
 warning: panic message is not a string literal
-  --> $DIR/non-fmt-panic.rs:20:12
+  --> $DIR/non-fmt-panic.rs:22:12
    |
 LL |     panic!(S);
    |            ^
@@ -109,13 +105,9 @@ help: add a "{}" format string to Display the message
    |
 LL |     panic!("{}", S);
    |            +++++
-help: or use std::panic::panic_any instead
-   |
-LL |     std::panic::panic_any(S);
-   |     ~~~~~~~~~~~~~~~~~~~~~
 
 warning: panic message is not a string literal
-  --> $DIR/non-fmt-panic.rs:21:17
+  --> $DIR/non-fmt-panic.rs:23:17
    |
 LL |     std::panic!(123);
    |                 ^^^
@@ -132,7 +124,7 @@ LL |     std::panic::panic_any(123);
    |     ~~~~~~~~~~~~~~~~~~~~~
 
 warning: panic message is not a string literal
-  --> $DIR/non-fmt-panic.rs:22:18
+  --> $DIR/non-fmt-panic.rs:24:18
    |
 LL |     core::panic!(&*"abc");
    |                  ^^^^^^^
@@ -145,7 +137,7 @@ LL |     core::panic!("{}", &*"abc");
    |                  +++++
 
 warning: panic message contains an unused formatting placeholder
-  --> $DIR/non-fmt-panic.rs:23:12
+  --> $DIR/non-fmt-panic.rs:25:12
    |
 LL |     panic!(concat!("{", "}"));
    |            ^^^^^^^^^^^^^^^^^
@@ -161,7 +153,7 @@ LL |     panic!("{}", concat!("{", "}"));
    |            +++++
 
 warning: panic message contains braces
-  --> $DIR/non-fmt-panic.rs:24:5
+  --> $DIR/non-fmt-panic.rs:26:5
    |
 LL |     panic!(concat!("{", "{"));
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -173,15 +165,15 @@ LL |     panic!("{}", concat!("{", "{"));
    |            +++++
 
 warning: panic message contains an unused formatting placeholder
-  --> $DIR/non-fmt-panic.rs:26:37
+  --> $DIR/non-fmt-panic.rs:28:37
    |
 LL |     fancy_panic::fancy_panic!("test {} 123");
    |                                     ^^
    |
    = note: this message is not used as a format string when given without arguments, but will be in Rust 2021
 
 warning: panic message is not a string literal
-  --> $DIR/non-fmt-panic.rs:36:12
+  --> $DIR/non-fmt-panic.rs:38:12
    |
 LL |     panic!(a!());
    |            ^^^^
@@ -198,7 +190,7 @@ LL |     std::panic::panic_any(a!());
    |     ~~~~~~~~~~~~~~~~~~~~~
 
 warning: panic message is not a string literal
-  --> $DIR/non-fmt-panic.rs:38:12
+  --> $DIR/non-fmt-panic.rs:40:12
    |
 LL |     panic!(format!("{}", 1));
    |            ^^^^^^^^^^^^^^^^
@@ -213,7 +205,7 @@ LL +     panic!("{}", 1);
    | 
 
 warning: panic message is not a string literal
-  --> $DIR/non-fmt-panic.rs:39:20
+  --> $DIR/non-fmt-panic.rs:41:20
    |
 LL |     assert!(false, format!("{}", 1));
    |                    ^^^^^^^^^^^^^^^^
@@ -228,7 +220,7 @@ LL +     assert!(false, "{}", 1);
    | 
 
 warning: panic message is not a string literal
-  --> $DIR/non-fmt-panic.rs:40:26
+  --> $DIR/non-fmt-panic.rs:42:26
    |
 LL |     debug_assert!(false, format!("{}", 1));
    |                          ^^^^^^^^^^^^^^^^
@@ -243,7 +235,7 @@ LL +     debug_assert!(false, "{}", 1);
    | 
 
 warning: panic message is not a string literal
-  --> $DIR/non-fmt-panic.rs:42:12
+  --> $DIR/non-fmt-panic.rs:44:12
    |
 LL |     panic![123];
    |            ^^^
@@ -260,7 +252,7 @@ LL |     std::panic::panic_any(123);
    |     ~~~~~~~~~~~~~~~~~~~~~~   ~
 
 warning: panic message is not a string literal
-  --> $DIR/non-fmt-panic.rs:43:12
+  --> $DIR/non-fmt-panic.rs:45:12
    |
 LL |     panic!{123};
    |            ^^^