From 7b2a0fa2b8b26a2d70a1bfbf6a1dfe05617bfa66 Mon Sep 17 00:00:00 2001 From: tabokie Date: Mon, 25 Jul 2022 13:25:23 +0800 Subject: [PATCH] address comment about macro invokation Signed-off-by: tabokie --- .../src/assertions_on_result_states.rs | 11 ++++++----- tests/ui/assertions_on_result_states.fixed | 9 +++++++++ tests/ui/assertions_on_result_states.rs | 9 +++++++++ tests/ui/assertions_on_result_states.stderr | 18 ++++++++++++------ 4 files changed, 36 insertions(+), 11 deletions(-) diff --git a/clippy_lints/src/assertions_on_result_states.rs b/clippy_lints/src/assertions_on_result_states.rs index 856784a2ea51..b6affdee5236 100644 --- a/clippy_lints/src/assertions_on_result_states.rs +++ b/clippy_lints/src/assertions_on_result_states.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::macros::{find_assert_args, root_macro_call_first_node, PanicExpn}; use clippy_utils::path_res; -use clippy_utils::source::snippet_opt; +use clippy_utils::source::snippet_with_context; use clippy_utils::ty::{implements_trait, is_copy, is_type_diagnostic_item}; use clippy_utils::usage::local_used_after_expr; use rustc_errors::Applicability; @@ -54,6 +54,7 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnResultStates { return; } } + let mut app = Applicability::MachineApplicable; match method_segment.ident.as_str() { "is_ok" if has_debug_impl(cx, substs.type_at(1)) => { span_lint_and_sugg( @@ -64,9 +65,9 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnResultStates { "replace with", format!( "{}.unwrap()", - snippet_opt(cx, recv.span).unwrap() + snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0 ), - Applicability::MachineApplicable, + app, ); } "is_err" if has_debug_impl(cx, substs.type_at(0)) => { @@ -78,9 +79,9 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnResultStates { "replace with", format!( "{}.unwrap_err()", - snippet_opt(cx, recv.span).unwrap() + snippet_with_context(cx, recv.span, condition.span.ctxt(), "..", &mut app).0 ), - Applicability::MachineApplicable, + app, ); } _ => (), diff --git a/tests/ui/assertions_on_result_states.fixed b/tests/ui/assertions_on_result_states.fixed index dcea38957d4d..7bde72e4b6b5 100644 --- a/tests/ui/assertions_on_result_states.fixed +++ b/tests/ui/assertions_on_result_states.fixed @@ -11,6 +11,12 @@ struct DebugFoo; #[derive(Copy, Clone, Debug)] struct CopyFoo; +macro_rules! get_ok_macro { + () => { + Ok::<_, DebugFoo>(Foo) + }; +} + fn main() { // test ok let r: Result = Ok(Foo); @@ -27,6 +33,9 @@ fn main() { } get_ok().unwrap(); + // test macro ok + get_ok_macro!().unwrap(); + // test ok that shouldn't be moved let r: Result = Ok(CopyFoo); fn test_ref_unmoveable_ok(r: &Result) { diff --git a/tests/ui/assertions_on_result_states.rs b/tests/ui/assertions_on_result_states.rs index c6bb6160da20..4c5af81efc23 100644 --- a/tests/ui/assertions_on_result_states.rs +++ b/tests/ui/assertions_on_result_states.rs @@ -11,6 +11,12 @@ struct DebugFoo; #[derive(Copy, Clone, Debug)] struct CopyFoo; +macro_rules! get_ok_macro { + () => { + Ok::<_, DebugFoo>(Foo) + }; +} + fn main() { // test ok let r: Result = Ok(Foo); @@ -27,6 +33,9 @@ fn main() { } assert!(get_ok().is_ok()); + // test macro ok + assert!(get_ok_macro!().is_ok()); + // test ok that shouldn't be moved let r: Result = Ok(CopyFoo); fn test_ref_unmoveable_ok(r: &Result) { diff --git a/tests/ui/assertions_on_result_states.stderr b/tests/ui/assertions_on_result_states.stderr index 8df16e4b68be..13c2dd877a97 100644 --- a/tests/ui/assertions_on_result_states.stderr +++ b/tests/ui/assertions_on_result_states.stderr @@ -1,5 +1,5 @@ error: called `assert!` with `Result::is_ok` - --> $DIR/assertions_on_result_states.rs:18:5 + --> $DIR/assertions_on_result_states.rs:24:5 | LL | assert!(r.is_ok()); | ^^^^^^^^^^^^^^^^^^ help: replace with: `r.unwrap()` @@ -7,28 +7,34 @@ LL | assert!(r.is_ok()); = note: `-D clippy::assertions-on-result-states` implied by `-D warnings` error: called `assert!` with `Result::is_ok` - --> $DIR/assertions_on_result_states.rs:28:5 + --> $DIR/assertions_on_result_states.rs:34:5 | LL | assert!(get_ok().is_ok()); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `get_ok().unwrap()` error: called `assert!` with `Result::is_ok` - --> $DIR/assertions_on_result_states.rs:41:5 + --> $DIR/assertions_on_result_states.rs:37:5 + | +LL | assert!(get_ok_macro!().is_ok()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `get_ok_macro!().unwrap()` + +error: called `assert!` with `Result::is_ok` + --> $DIR/assertions_on_result_states.rs:50:5 | LL | assert!(r.is_ok()); | ^^^^^^^^^^^^^^^^^^ help: replace with: `r.unwrap()` error: called `assert!` with `Result::is_ok` - --> $DIR/assertions_on_result_states.rs:47:9 + --> $DIR/assertions_on_result_states.rs:56:9 | LL | assert!(r.is_ok()); | ^^^^^^^^^^^^^^^^^^ help: replace with: `r.unwrap()` error: called `assert!` with `Result::is_err` - --> $DIR/assertions_on_result_states.rs:55:5 + --> $DIR/assertions_on_result_states.rs:64:5 | LL | assert!(r.is_err()); | ^^^^^^^^^^^^^^^^^^^ help: replace with: `r.unwrap_err()` -error: aborting due to 5 previous errors +error: aborting due to 6 previous errors