From b4f448a7ea47049ad6392afc3684e17372cb4469 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 12 Aug 2021 19:14:54 +0200 Subject: [PATCH 1/3] non_fmt_panic: machine app. suggestion for assert with string msg. --- compiler/rustc_lint/src/non_fmt_panic.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_lint/src/non_fmt_panic.rs b/compiler/rustc_lint/src/non_fmt_panic.rs index ec53625f10525..23c4bc7d608f3 100644 --- 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(), cx.tcx.get_diagnostic_item(sym::string_type)), + (Some(ty_def), Some(string_type)) if ty_def.did == string_type + ); 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", From 5ad41069e65a10e8e5b6738ec53d874e2467d482 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 12 Aug 2021 19:15:47 +0200 Subject: [PATCH 2/3] Update non-fmt-panic tests. --- src/test/ui/non-fmt-panic.fixed | 54 ++++++++++++++++++++++++++++++++ src/test/ui/non-fmt-panic.rs | 2 ++ src/test/ui/non-fmt-panic.stderr | 46 +++++++++++---------------- 3 files changed, 75 insertions(+), 27 deletions(-) create mode 100644 src/test/ui/non-fmt-panic.fixed diff --git a/src/test/ui/non-fmt-panic.fixed b/src/test/ui/non-fmt-panic.fixed new file mode 100644 index 0000000000000..c85e1887d9603 --- /dev/null +++ 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 index 0de424ce279f3..020bcf00a016d 100644 --- 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 index 4b18f5546b9b8..513ffd37dc3c8 100644 --- 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,7 +165,7 @@ 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"); | ^^ @@ -181,7 +173,7 @@ 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}; | ^^^ From 8fedb31649d5bb9ec9b961e666a5b0b7d97d6041 Mon Sep 17 00:00:00 2001 From: Mara Bos Date: Thu, 12 Aug 2021 19:21:46 +0200 Subject: [PATCH 3/3] Use is_diagnostic_item instead of get_diagnostic_item. --- compiler/rustc_lint/src/non_fmt_panic.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_lint/src/non_fmt_panic.rs b/compiler/rustc_lint/src/non_fmt_panic.rs index 23c4bc7d608f3..3349063e5dcd9 100644 --- a/compiler/rustc_lint/src/non_fmt_panic.rs +++ b/compiler/rustc_lint/src/non_fmt_panic.rs @@ -124,10 +124,10 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc // 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 + ty::Ref(_, r, _) if *r.kind() == ty::Str, ) || matches!( - (ty.ty_adt_def(), cx.tcx.get_diagnostic_item(sym::string_type)), - (Some(ty_def), Some(string_type)) if ty_def.did == string_type + 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(),