Skip to content

Commit 95c86a3

Browse files
committed
add [assert_ok] lint
Signed-off-by: tabokie <[email protected]>
1 parent 05a51e5 commit 95c86a3

File tree

9 files changed

+96
-0
lines changed

9 files changed

+96
-0
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3439,6 +3439,7 @@ Released 2018-09-13
34393439
[`approx_constant`]: https://rust-lang.github.io/rust-clippy/master/index.html#approx_constant
34403440
[`as_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions
34413441
[`as_underscore`]: https://rust-lang.github.io/rust-clippy/master/index.html#as_underscore
3442+
[`assert_ok`]: https://rust-lang.github.io/rust-clippy/master/index.html#assert_ok
34423443
[`assertions_on_constants`]: https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_constants
34433444
[`assign_op_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern
34443445
[`assign_ops`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_ops

clippy_lints/src/assert_ok.rs

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::macros::{find_assert_args, root_macro_call_first_node, PanicExpn};
3+
use clippy_utils::source::snippet_opt;
4+
use if_chain::if_chain;
5+
use rustc_errors::Applicability;
6+
use rustc_hir::{Expr, ExprKind};
7+
use rustc_lint::{LateContext, LateLintPass};
8+
use rustc_session::{declare_lint_pass, declare_tool_lint};
9+
use rustc_span::sym;
10+
11+
declare_clippy_lint! {
12+
/// ### What it does
13+
/// Checks for `assert!(r.is_ok())` calls.
14+
///
15+
/// ### Why is this bad?
16+
/// An assertion failure cannot output a useful message of the error.
17+
///
18+
/// ### Known problems
19+
/// The error type needs to implement `Debug`.
20+
///
21+
/// ### Example
22+
/// ```rust,ignore
23+
/// # let r = Ok::<_, ()>(());
24+
/// assert!(r.is_ok());
25+
/// ```
26+
#[clippy::version = "1.64.0"]
27+
pub ASSERT_OK,
28+
style,
29+
"`assert!(r.is_ok())` gives worse error message than directly calling `r.unwrap()`"
30+
}
31+
32+
declare_lint_pass!(AssertOk => [ASSERT_OK]);
33+
34+
impl<'tcx> LateLintPass<'tcx> for AssertOk {
35+
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
36+
if_chain! {
37+
if let Some(macro_call) = root_macro_call_first_node(cx, e);
38+
if matches!(cx.tcx.get_diagnostic_name(macro_call.def_id), Some(sym::assert_macro));
39+
if let Some((condition, panic_expn)) = find_assert_args(cx, e, macro_call.expn);
40+
if matches!(panic_expn, PanicExpn::Empty);
41+
if let ExprKind::MethodCall(method_segment, args, _) = condition.kind;
42+
if method_segment.ident.name == sym!(is_ok);
43+
let method_receiver = &args[0];
44+
if let Some(method_receiver_snippet) = snippet_opt(cx, method_receiver.span);
45+
then {
46+
span_lint_and_sugg(
47+
cx,
48+
ASSERT_OK,
49+
macro_call.span,
50+
&format!(
51+
"`assert!({}.is_ok())` gives bad error message",
52+
method_receiver_snippet
53+
),
54+
"replace with",
55+
format!(
56+
"{}.unwrap()",
57+
method_receiver_snippet
58+
),
59+
Applicability::Unspecified,
60+
);
61+
}
62+
}
63+
}
64+
}

clippy_lints/src/lib.register_all.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
store.register_group(true, "clippy::all", Some("clippy_all"), vec![
66
LintId::of(almost_complete_letter_range::ALMOST_COMPLETE_LETTER_RANGE),
77
LintId::of(approx_const::APPROX_CONSTANT),
8+
LintId::of(assert_ok::ASSERT_OK),
89
LintId::of(assertions_on_constants::ASSERTIONS_ON_CONSTANTS),
910
LintId::of(async_yields_async::ASYNC_YIELDS_ASYNC),
1011
LintId::of(attrs::BLANKET_CLIPPY_RESTRICTION_LINTS),

clippy_lints/src/lib.register_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ store.register_lints(&[
4141
as_underscore::AS_UNDERSCORE,
4242
asm_syntax::INLINE_ASM_X86_ATT_SYNTAX,
4343
asm_syntax::INLINE_ASM_X86_INTEL_SYNTAX,
44+
assert_ok::ASSERT_OK,
4445
assertions_on_constants::ASSERTIONS_ON_CONSTANTS,
4546
async_yields_async::ASYNC_YIELDS_ASYNC,
4647
attrs::ALLOW_ATTRIBUTES_WITHOUT_REASON,

clippy_lints/src/lib.register_style.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// Manual edits will be overwritten.
44

55
store.register_group(true, "clippy::style", Some("clippy_style"), vec![
6+
LintId::of(assert_ok::ASSERT_OK),
67
LintId::of(assertions_on_constants::ASSERTIONS_ON_CONSTANTS),
78
LintId::of(blacklisted_name::BLACKLISTED_NAME),
89
LintId::of(blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS),

clippy_lints/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ mod approx_const;
173173
mod as_conversions;
174174
mod as_underscore;
175175
mod asm_syntax;
176+
mod assert_ok;
176177
mod assertions_on_constants;
177178
mod async_yields_async;
178179
mod attrs;
@@ -726,6 +727,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
726727
store.register_late_pass(|| Box::new(slow_vector_initialization::SlowVectorInit));
727728
store.register_late_pass(|| Box::new(unnecessary_sort_by::UnnecessarySortBy));
728729
store.register_late_pass(move || Box::new(unnecessary_wraps::UnnecessaryWraps::new(avoid_breaking_exported_api)));
730+
store.register_late_pass(|| Box::new(assert_ok::AssertOk));
729731
store.register_late_pass(|| Box::new(assertions_on_constants::AssertionsOnConstants));
730732
store.register_late_pass(|| Box::new(transmuting_null::TransmutingNull));
731733
store.register_late_pass(|| Box::new(path_buf_push_overwrite::PathBufPushOverwrite));

tests/ui/assert_ok.fixed

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// run-rustfix
2+
#![warn(clippy::assert_ok)]
3+
4+
fn main() {
5+
// basic case
6+
let r: std::result::Result<(), ()> = Ok(());
7+
r.unwrap();
8+
}

tests/ui/assert_ok.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// run-rustfix
2+
#![warn(clippy::assert_ok)]
3+
4+
fn main() {
5+
// basic case
6+
let r: std::result::Result<(), ()> = Ok(());
7+
assert!(r.is_ok());
8+
}

tests/ui/assert_ok.stderr

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
error: `assert!(r.is_ok())` gives bad error message
2+
--> $DIR/assert_ok.rs:7:5
3+
|
4+
LL | assert!(r.is_ok());
5+
| ^^^^^^^^^^^^^^^^^^ help: replace with: `r.unwrap()`
6+
|
7+
= note: `-D clippy::assert-ok` implied by `-D warnings`
8+
9+
error: aborting due to previous error
10+

0 commit comments

Comments
 (0)