Skip to content

Commit d83da1d

Browse files
borsbors
committed
Auto merge of rust-lang#88083 - m-ou-se:non-fmt-panics-suggest-debug, r=estebank
Improve non_fmt_panics suggestion based on trait impls. This improves the non_fmt_panics lint suggestions by checking first which trait (Display or Debug) are actually implemented on the type. Fixes rust-lang#87313 Fixes rust-lang#87999 Before: ``` help: add a "{}" format string to Display the message | 2 | panic!("{}", Some(1)); | +++++ help: or use std::panic::panic_any instead | 2 | std::panic::panic_any(Some(1)); | ~~~~~~~~~~~~~~~~~~~~~ ``` After: ``` help: add a "{:?}" format string to use the Debug implementation of `Option<i32>` | 2 | panic!("{:?}", Some(1)); | +++++++ help: or use std::panic::panic_any instead | 2 | std::panic::panic_any(Some(1)); | ~~~~~~~~~~~~~~~~~~~~~ ``` r? `@estebank`
2 parents 806b399 + ab8cbc3 commit d83da1d

File tree

6 files changed

+288
-33
lines changed

6 files changed

+288
-33
lines changed

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3933,6 +3933,7 @@ dependencies = [
39333933
"rustc_feature",
39343934
"rustc_hir",
39353935
"rustc_index",
3936+
"rustc_infer",
39363937
"rustc_middle",
39373938
"rustc_parse_format",
39383939
"rustc_serialize",

‎compiler/rustc_lint/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,4 @@ rustc_session = { path = "../rustc_session" }
2222
rustc_serialize = { path = "../rustc_serialize" }
2323
rustc_trait_selection = { path = "../rustc_trait_selection" }
2424
rustc_parse_format = { path = "../rustc_parse_format" }
25+
rustc_infer = { path = "../rustc_infer" }

‎compiler/rustc_lint/src/non_fmt_panic.rs‎

Lines changed: 53 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,15 @@ use crate::{LateContext, LateLintPass, LintContext};
22
use rustc_ast as ast;
33
use rustc_errors::{pluralize, Applicability};
44
use rustc_hir as hir;
5+
use rustc_infer::infer::TyCtxtInferExt;
56
use rustc_middle::lint::in_external_macro;
67
use rustc_middle::ty;
8+
use rustc_middle::ty::subst::InternalSubsts;
79
use rustc_parse_format::{ParseMode, Parser, Piece};
810
use rustc_session::lint::FutureIncompatibilityReason;
911
use rustc_span::edition::Edition;
1012
use rustc_span::{hygiene, sym, symbol::kw, symbol::SymbolStr, InnerSpan, Span, Symbol};
13+
use rustc_trait_selection::infer::InferCtxtExt;
1114

1215
declare_lint! {
1316
/// The `non_fmt_panics` lint detects `panic!(..)` invocations where the first
@@ -99,7 +102,7 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
99102

100103
cx.struct_span_lint(NON_FMT_PANICS, arg_span, |lint| {
101104
let mut l = lint.build("panic message is not a string literal");
102-
l.note("this usage of panic!() is deprecated; it will be a hard error in Rust 2021");
105+
l.note(&format!("this usage of {}!() is deprecated; it will be a hard error in Rust 2021", symbol_str));
103106
l.note("for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2021/panic-macro-consistency.html>");
104107
if !is_arg_inside_call(arg_span, span) {
105108
// No clue where this argument is coming from.
@@ -129,20 +132,57 @@ fn check_panic<'tcx>(cx: &LateContext<'tcx>, f: &'tcx hir::Expr<'tcx>, arg: &'tc
129132
ty.ty_adt_def(),
130133
Some(ty_def) if cx.tcx.is_diagnostic_item(sym::string_type, ty_def.did),
131134
);
132-
l.span_suggestion_verbose(
133-
arg_span.shrink_to_lo(),
134-
"add a \"{}\" format string to Display the message",
135-
"\"{}\", ".into(),
136-
if is_str {
137-
Applicability::MachineApplicable
138-
} else {
139-
Applicability::MaybeIncorrect
140-
},
141-
);
142-
if !is_str && panic == sym::std_panic_macro {
135+
136+
let (suggest_display, suggest_debug) = cx.tcx.infer_ctxt().enter(|infcx| {
137+
let display = is_str || cx.tcx.get_diagnostic_item(sym::display_trait).map(|t| {
138+
infcx.type_implements_trait(t, ty, InternalSubsts::empty(), cx.param_env).may_apply()
139+
}) == Some(true);
140+
let debug = !display && cx.tcx.get_diagnostic_item(sym::debug_trait).map(|t| {
141+
infcx.type_implements_trait(t, ty, InternalSubsts::empty(), cx.param_env).may_apply()
142+
}) == Some(true);
143+
(display, debug)
144+
});
145+
146+
let suggest_panic_any = !is_str && panic == sym::std_panic_macro;
147+
148+
let fmt_applicability = if suggest_panic_any {
149+
// If we can use panic_any, use that as the MachineApplicable suggestion.
150+
Applicability::MaybeIncorrect
151+
} else {
152+
// If we don't suggest panic_any, using a format string is our best bet.
153+
Applicability::MachineApplicable
154+
};
155+
156+
if suggest_display {
157+
l.span_suggestion_verbose(
158+
arg_span.shrink_to_lo(),
159+
"add a \"{}\" format string to Display the message",
160+
"\"{}\", ".into(),
161+
fmt_applicability,
162+
);
163+
} else if suggest_debug {
164+
l.span_suggestion_verbose(
165+
arg_span.shrink_to_lo(),
166+
&format!(
167+
"add a \"{{:?}}\" format string to use the Debug implementation of `{}`",
168+
ty,
169+
),
170+
"\"{:?}\", ".into(),
171+
fmt_applicability,
172+
);
173+
}
174+
175+
if suggest_panic_any {
143176
if let Some((open, close, del)) = find_delimiters(cx, span) {
144177
l.multipart_suggestion(
145-
"or use std::panic::panic_any instead",
178+
&format!(
179+
"{}use std::panic::panic_any instead",
180+
if suggest_display || suggest_debug {
181+
"or "
182+
} else {
183+
""
184+
},
185+
),
146186
if del == '(' {
147187
vec![(span.until(open), "std::panic::panic_any".into())]
148188
} else {

‎src/test/ui/non-fmt-panic.fixed‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,16 @@ fn main() {
1717
//~^ WARN panic message contains unused formatting placeholders
1818
assert!(false, "{}", S);
1919
//~^ WARN panic message is not a string literal
20+
assert!(false, "{}", 123);
21+
//~^ WARN panic message is not a string literal
22+
assert!(false, "{:?}", Some(123));
23+
//~^ WARN panic message is not a string literal
2024
debug_assert!(false, "{}", "{{}} bla"); //~ WARN panic message contains braces
2125
panic!("{}", C); //~ WARN panic message is not a string literal
2226
panic!("{}", S); //~ WARN panic message is not a string literal
2327
std::panic::panic_any(123); //~ WARN panic message is not a string literal
2428
core::panic!("{}", &*"abc"); //~ WARN panic message is not a string literal
29+
std::panic::panic_any(Some(123)); //~ WARN panic message is not a string literal
2530
panic!("{}", concat!("{", "}")); //~ WARN panic message contains an unused formatting placeholder
2631
panic!("{}", concat!("{", "{")); //~ WARN panic message contains braces
2732

@@ -51,4 +56,29 @@ fn main() {
5156
}
5257
panic!("{}"); // OK
5358
panic!(S); // OK
59+
60+
a(1);
61+
b(1);
62+
c(1);
63+
d(1);
64+
}
65+
66+
fn a<T: Send + 'static>(v: T) {
67+
std::panic::panic_any(v); //~ WARN panic message is not a string literal
68+
assert!(false, v); //~ WARN panic message is not a string literal
69+
}
70+
71+
fn b<T: std::fmt::Debug + Send + 'static>(v: T) {
72+
std::panic::panic_any(v); //~ WARN panic message is not a string literal
73+
assert!(false, "{:?}", v); //~ WARN panic message is not a string literal
74+
}
75+
76+
fn c<T: std::fmt::Display + Send + 'static>(v: T) {
77+
std::panic::panic_any(v); //~ WARN panic message is not a string literal
78+
assert!(false, "{}", v); //~ WARN panic message is not a string literal
79+
}
80+
81+
fn d<T: std::fmt::Display + std::fmt::Debug + Send + 'static>(v: T) {
82+
std::panic::panic_any(v); //~ WARN panic message is not a string literal
83+
assert!(false, "{}", v); //~ WARN panic message is not a string literal
5484
}

‎src/test/ui/non-fmt-panic.rs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,16 @@ fn main() {
1717
//~^ WARN panic message contains unused formatting placeholders
1818
assert!(false, S);
1919
//~^ WARN panic message is not a string literal
20+
assert!(false, 123);
21+
//~^ WARN panic message is not a string literal
22+
assert!(false, Some(123));
23+
//~^ WARN panic message is not a string literal
2024
debug_assert!(false, "{{}} bla"); //~ WARN panic message contains braces
2125
panic!(C); //~ WARN panic message is not a string literal
2226
panic!(S); //~ WARN panic message is not a string literal
2327
std::panic!(123); //~ WARN panic message is not a string literal
2428
core::panic!(&*"abc"); //~ WARN panic message is not a string literal
29+
panic!(Some(123)); //~ WARN panic message is not a string literal
2530
panic!(concat!("{", "}")); //~ WARN panic message contains an unused formatting placeholder
2631
panic!(concat!("{", "{")); //~ WARN panic message contains braces
2732

@@ -51,4 +56,29 @@ fn main() {
5156
}
5257
panic!("{}"); // OK
5358
panic!(S); // OK
59+
60+
a(1);
61+
b(1);
62+
c(1);
63+
d(1);
64+
}
65+
66+
fn a<T: Send + 'static>(v: T) {
67+
panic!(v); //~ WARN panic message is not a string literal
68+
assert!(false, v); //~ WARN panic message is not a string literal
69+
}
70+
71+
fn b<T: std::fmt::Debug + Send + 'static>(v: T) {
72+
panic!(v); //~ WARN panic message is not a string literal
73+
assert!(false, v); //~ WARN panic message is not a string literal
74+
}
75+
76+
fn c<T: std::fmt::Display + Send + 'static>(v: T) {
77+
panic!(v); //~ WARN panic message is not a string literal
78+
assert!(false, v); //~ WARN panic message is not a string literal
79+
}
80+
81+
fn d<T: std::fmt::Display + std::fmt::Debug + Send + 'static>(v: T) {
82+
panic!(v); //~ WARN panic message is not a string literal
83+
assert!(false, v); //~ WARN panic message is not a string literal
5484
}

0 commit comments

Comments
 (0)