Skip to content

Commit 22f0579

Browse files
committed
Match any expr for panic message
1 parent d66acc2 commit 22f0579

File tree

3 files changed

+25
-16
lines changed

3 files changed

+25
-16
lines changed

clippy_lints/src/assertions_on_constants.rs

+9-11
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
use crate::consts::{constant, Constant};
22
use crate::utils::paths;
3-
use crate::utils::{is_direct_expn_of, is_expn_of, match_def_path, resolve_node, span_help_and_lint};
3+
use crate::utils::{is_direct_expn_of, is_expn_of, match_def_path, span_help_and_lint, snippet};
44
use if_chain::if_chain;
55
use rustc::hir::*;
66
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
77
use rustc::{declare_lint_pass, declare_tool_lint};
88
use syntax::ast::LitKind;
9-
use syntax::source_map::symbol::LocalInternedString;
9+
use std::borrow::Cow;
1010

1111
declare_clippy_lint! {
1212
/// **What it does:** Checks for `assert!(true)` and `assert!(false)` calls.
@@ -75,7 +75,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
7575
"`assert!(true)` will be optimized out by the compiler",
7676
"remove it",
7777
);
78-
} else if panic_message.starts_with("assertion failed: ") {
78+
} else if panic_message.is_empty() || panic_message.starts_with("\"assertion failed: ") {
7979
span_help_and_lint(
8080
cx,
8181
ASSERTIONS_ON_CONSTANTS,
@@ -88,9 +88,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
8888
cx,
8989
ASSERTIONS_ON_CONSTANTS,
9090
e.span,
91-
&format!("`assert!(false, \"{}\")` should probably be replaced", panic_message,),
91+
&format!("`assert!(false, {})` should probably be replaced", panic_message,),
9292
&format!(
93-
"use `panic!(\"{}\")` or `unreachable!(\"{}\")`",
93+
"use `panic!({})` or `unreachable!({})`",
9494
panic_message, panic_message,
9595
),
9696
);
@@ -119,7 +119,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssertionsOnConstants {
119119
///
120120
/// Returns the `message` argument of `begin_panic` and the value of `c` which is the
121121
/// first argument of `assert!`.
122-
fn assert_with_message<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> Option<(LocalInternedString, bool)> {
122+
fn assert_with_message<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> Option<(Cow<'a, str>, bool)> {
123123
if_chain! {
124124
if let ExprKind::Match(ref expr, ref arms, _) = expr.kind;
125125
// matches { let _t = expr; _t }
@@ -140,14 +140,12 @@ fn assert_with_message<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -
140140
// function call
141141
if let Some(args) = match_function_call(cx, begin_panic_call, &paths::BEGIN_PANIC);
142142
if args.len() == 2;
143-
if let ExprKind::Lit(ref lit) = args[0].kind;
144-
if let LitKind::Str(ref s, _) = lit.node;
145143
// bind the second argument of the `assert!` macro
146-
let panic_message = s.as_str();
144+
let panic_message_arg = snippet(cx, args[0].span, "..");
147145
// second argument of begin_panic is irrelevant
148146
// as is the second match arm
149147
then {
150-
return Some((panic_message, is_true));
148+
return Some((panic_message_arg, is_true));
151149
}
152150
}
153151
None
@@ -164,7 +162,7 @@ fn match_function_call<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, p
164162
if_chain! {
165163
if let ExprKind::Call(ref fun, ref args) = expr.kind;
166164
if let ExprKind::Path(ref qpath) = fun.kind;
167-
if let Some(fun_def_id) = resolve_node(cx, qpath, fun.hir_id).opt_def_id();
165+
if let Some(fun_def_id) = cx.tables.qpath_res(qpath, fun.hir_id).opt_def_id();
168166
if match_def_path(cx, fun_def_id, path);
169167
then {
170168
return Some(&args)

tests/ui/assertions_on_constants.rs

+3
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ fn main() {
1111
assert!(true, "true message");
1212
assert!(false, "false message");
1313

14+
let msg = "panic message";
15+
assert!(false, msg.to_uppercase());
16+
1417
const B: bool = true;
1518
assert!(B);
1619

tests/ui/assertions_on_constants.stderr

+13-5
Original file line numberDiff line numberDiff line change
@@ -31,38 +31,46 @@ LL | assert!(false, "false message");
3131
|
3232
= help: use `panic!("false message")` or `unreachable!("false message")`
3333

34-
error: `assert!(true)` will be optimized out by the compiler
34+
error: `assert!(false, msg.to_uppercase())` should probably be replaced
3535
--> $DIR/assertions_on_constants.rs:15:5
3636
|
37+
LL | assert!(false, msg.to_uppercase());
38+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
39+
|
40+
= help: use `panic!(msg.to_uppercase())` or `unreachable!(msg.to_uppercase())`
41+
42+
error: `assert!(true)` will be optimized out by the compiler
43+
--> $DIR/assertions_on_constants.rs:18:5
44+
|
3745
LL | assert!(B);
3846
| ^^^^^^^^^^^
3947
|
4048
= help: remove it
4149

4250
error: `assert!(false)` should probably be replaced
43-
--> $DIR/assertions_on_constants.rs:18:5
51+
--> $DIR/assertions_on_constants.rs:21:5
4452
|
4553
LL | assert!(C);
4654
| ^^^^^^^^^^^
4755
|
4856
= help: use `panic!()` or `unreachable!()`
4957

5058
error: `assert!(false, "C message")` should probably be replaced
51-
--> $DIR/assertions_on_constants.rs:19:5
59+
--> $DIR/assertions_on_constants.rs:22:5
5260
|
5361
LL | assert!(C, "C message");
5462
| ^^^^^^^^^^^^^^^^^^^^^^^^
5563
|
5664
= help: use `panic!("C message")` or `unreachable!("C message")`
5765

5866
error: `assert!(true)` will be optimized out by the compiler
59-
--> $DIR/assertions_on_constants.rs:21:5
67+
--> $DIR/assertions_on_constants.rs:24:5
6068
|
6169
LL | debug_assert!(true);
6270
| ^^^^^^^^^^^^^^^^^^^^
6371
|
6472
= help: remove it
6573
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
6674

67-
error: aborting due to 8 previous errors
75+
error: aborting due to 9 previous errors
6876

0 commit comments

Comments
 (0)