diff --git a/clippy_lints/src/manual_assert.rs b/clippy_lints/src/manual_assert.rs index 76cb22864779..8797a82f5fe2 100644 --- a/clippy_lints/src/manual_assert.rs +++ b/clippy_lints/src/manual_assert.rs @@ -1,10 +1,13 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::macros::{is_panic, root_macro_call}; -use clippy_utils::{higher, is_else_clause, is_parent_stmt, peel_blocks_with_stmt, span_extract_comment, sugg}; +use clippy_utils::source::snippet_opt; +use clippy_utils::sugg::Sugg; +use clippy_utils::{higher, is_else_clause, is_parent_stmt, peel_blocks_with_stmt, span_extract_comment}; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::declare_lint_pass; +use rustc_span::sym; declare_clippy_lint! { /// ### What it does @@ -37,12 +40,14 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { if let Some(higher::If { cond, then, r#else: None }) = higher::If::hir(expr) && !matches!(cond.kind, ExprKind::Let(_)) + // Don't lint on `if cfg!(..) { panic!(..) } + && root_macro_call(cond.span).is_none_or(|macro_call| cx.tcx.item_name(macro_call.def_id) != sym::cfg) && !expr.span.from_expansion() && let then = peel_blocks_with_stmt(then) && let Some(macro_call) = root_macro_call(then.span) && is_panic(cx, macro_call.def_id) - && !cx.tcx.sess.source_map().is_multiline(cond.span) - && let Ok(panic_snippet) = cx.sess().source_map().span_to_snippet(macro_call.span) + && !cx.sess().source_map().is_multiline(cond.span) + && let Some(panic_snippet) = snippet_opt(cx.sess(), macro_call.span) && let Some(panic_snippet) = panic_snippet.strip_suffix(')') && let Some((_, format_args_snip)) = panic_snippet.split_once('(') // Don't change `else if foo { panic!(..) }` to `else { assert!(foo, ..) }` as it just @@ -55,7 +60,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert { if !comments.is_empty() { comments += "\n"; } - let cond_sugg = !sugg::Sugg::hir_with_context(cx, cond, expr.span.ctxt(), "..", &mut applicability); + let cond_sugg = !Sugg::hir_with_context(cx, cond, expr.span.ctxt(), "..", &mut applicability); let semicolon = if is_parent_stmt(cx, expr.hir_id) { ";" } else { "" }; let sugg = format!("assert!({cond_sugg}, {format_args_snip}){semicolon}"); // we show to the user the suggestion without the comments, but when applying the fix, include the diff --git a/tests/ui/manual_assert.edition2018.stderr b/tests/ui/manual_assert.edition2018.stderr index 2e9c9045caae..cf43f08abc68 100644 --- a/tests/ui/manual_assert.edition2018.stderr +++ b/tests/ui/manual_assert.edition2018.stderr @@ -190,7 +190,25 @@ LL + const BAR: () = assert!(N != 0, ); | error: only a `panic!` in `if`-then statement - --> tests/ui/manual_assert.rs:116:5 + --> tests/ui/manual_assert.rs:119:5 + | +LL | / if foo!() { +LL | | +LL | | panic!("not a `cfg`, so lint") +LL | | } + | |_____^ + | +help: try instead + | +LL - if foo!() { +LL - +LL - panic!("not a `cfg`, so lint") +LL - } +LL + assert!(!foo!(), "not a `cfg`, so lint") + | + +error: only a `panic!` in `if`-then statement + --> tests/ui/manual_assert.rs:132:5 | LL | / if !is_x86_feature_detected!("ssse3") { LL | | @@ -207,5 +225,5 @@ LL - } LL + assert!(is_x86_feature_detected!("ssse3"), "SSSE3 is not supported"); | -error: aborting due to 11 previous errors +error: aborting due to 12 previous errors diff --git a/tests/ui/manual_assert.edition2021.stderr b/tests/ui/manual_assert.edition2021.stderr index 2e9c9045caae..cf43f08abc68 100644 --- a/tests/ui/manual_assert.edition2021.stderr +++ b/tests/ui/manual_assert.edition2021.stderr @@ -190,7 +190,25 @@ LL + const BAR: () = assert!(N != 0, ); | error: only a `panic!` in `if`-then statement - --> tests/ui/manual_assert.rs:116:5 + --> tests/ui/manual_assert.rs:119:5 + | +LL | / if foo!() { +LL | | +LL | | panic!("not a `cfg`, so lint") +LL | | } + | |_____^ + | +help: try instead + | +LL - if foo!() { +LL - +LL - panic!("not a `cfg`, so lint") +LL - } +LL + assert!(!foo!(), "not a `cfg`, so lint") + | + +error: only a `panic!` in `if`-then statement + --> tests/ui/manual_assert.rs:132:5 | LL | / if !is_x86_feature_detected!("ssse3") { LL | | @@ -207,5 +225,5 @@ LL - } LL + assert!(is_x86_feature_detected!("ssse3"), "SSSE3 is not supported"); | -error: aborting due to 11 previous errors +error: aborting due to 12 previous errors diff --git a/tests/ui/manual_assert.rs b/tests/ui/manual_assert.rs index ab02bd5f5e53..eee37b7fd3bf 100644 --- a/tests/ui/manual_assert.rs +++ b/tests/ui/manual_assert.rs @@ -106,6 +106,22 @@ fn issue12505() { } } +fn issue13883() { + if cfg!(debug_assertions) { + panic!("foobar") + } + + macro_rules! foo { + () => { + true + }; + } + if foo!() { + //~^ manual_assert + panic!("not a `cfg`, so lint") + } +} + fn issue15227(left: u64, right: u64) -> u64 { macro_rules! is_x86_feature_detected { ($feature:literal) => {