From 786f874c349b995ebed5e3d8f20db1cf65f20782 Mon Sep 17 00:00:00 2001 From: bors Date: Tue, 4 Jan 2022 22:32:02 +0000 Subject: [PATCH] New macro utils changelog: none Sorry, this is a big one. A lot of interrelated changes and I wanted to put the new utils to use to make sure they are somewhat battle-tested. We may want to divide some of the lint-specific refactoring commits into batches for smaller reviewing tasks. I could also split into more PRs. Introduces a bunch of new utils at `clippy_utils::macros::...`. Please read through the docs and give any feedback! I'm happy to introduce `MacroCall` and various functions to retrieve an instance. It feels like the missing puzzle piece. I'm also introducing `ExpnId` from rustc as "useful for Clippy too". `@rust-lang/clippy` Fixes #7843 by not parsing every node of macro implementations, at least the major offenders. I probably want to get rid of `is_expn_of` at some point. --- clippy_lints/src/assertions_on_constants.rs | 120 +--- clippy_lints/src/attrs.rs | 18 +- clippy_lints/src/bool_assert_comparison.rs | 73 ++- clippy_lints/src/doc.rs | 33 +- clippy_lints/src/eq_op.rs | 45 +- clippy_lints/src/explicit_write.rs | 6 +- clippy_lints/src/fallible_impl_from.rs | 15 +- clippy_lints/src/format.rs | 33 +- clippy_lints/src/format_args.rs | 53 +- clippy_lints/src/manual_assert.rs | 68 +-- clippy_lints/src/matches.rs | 24 +- clippy_lints/src/methods/expect_fun_call.rs | 21 +- clippy_lints/src/mutable_debug_assertion.rs | 42 +- clippy_lints/src/panic_in_result_fn.rs | 32 +- clippy_lints/src/panic_unimplemented.rs | 56 +- clippy_lints/src/unit_types/unit_cmp.rs | 40 +- clippy_lints/src/utils/internal_lints.rs | 11 +- clippy_utils/Cargo.toml | 1 + clippy_utils/src/ast_utils.rs | 32 -- clippy_utils/src/higher.rs | 295 +--------- clippy_utils/src/lib.rs | 40 +- clippy_utils/src/macros.rs | 539 ++++++++++++++++++ clippy_utils/src/paths.rs | 5 - tests/ui/assertions_on_constants.rs | 1 - tests/ui/assertions_on_constants.stderr | 42 +- tests/ui/eq_op_macros.stderr | 44 +- tests/ui/missing_panics_doc.stderr | 6 - tests/ui/panic_in_result_fn.stderr | 6 - tests/ui/panic_in_result_fn_assertions.stderr | 3 - tests/ui/panicking_macros.stderr | 21 - tests/ui/unit_cmp.stderr | 8 - 31 files changed, 868 insertions(+), 865 deletions(-) create mode 100644 clippy_utils/src/macros.rs diff --git a/clippy_lints/src/assertions_on_constants.rs b/clippy_lints/src/assertions_on_constants.rs index b7f414742f15..c82837746bd5 100644 --- a/clippy_lints/src/assertions_on_constants.rs +++ b/clippy_lints/src/assertions_on_constants.rs @@ -1,12 +1,10 @@ use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::higher; -use clippy_utils::source::snippet_opt; -use clippy_utils::{is_direct_expn_of, is_expn_of, match_panic_call, peel_blocks}; -use if_chain::if_chain; -use rustc_hir::{Expr, ExprKind, UnOp}; +use clippy_utils::macros::{find_assert_args, root_macro_call_first_node, PanicExpn}; +use rustc_hir::Expr; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; declare_clippy_lint! { /// ### What it does @@ -36,107 +34,39 @@ declare_lint_pass!(AssertionsOnConstants => [ASSERTIONS_ON_CONSTANTS]); impl<'tcx> LateLintPass<'tcx> for AssertionsOnConstants { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { - let lint_true = |is_debug: bool| { - span_lint_and_help( - cx, - ASSERTIONS_ON_CONSTANTS, - e.span, - if is_debug { - "`debug_assert!(true)` will be optimized out by the compiler" - } else { - "`assert!(true)` will be optimized out by the compiler" - }, - None, - "remove it", - ); + let Some(macro_call) = root_macro_call_first_node(cx, e) else { return }; + let is_debug = match cx.tcx.get_diagnostic_name(macro_call.def_id) { + Some(sym::debug_assert_macro) => true, + Some(sym::assert_macro) => false, + _ => return, }; - let lint_false_without_message = || { + let Some((condition, panic_expn)) = find_assert_args(cx, e, macro_call.expn) else { return }; + let Some((Constant::Bool(val), _)) = constant(cx, cx.typeck_results(), condition) else { return }; + if val { span_lint_and_help( cx, ASSERTIONS_ON_CONSTANTS, - e.span, - "`assert!(false)` should probably be replaced", + macro_call.span, + &format!( + "`{}!(true)` will be optimized out by the compiler", + cx.tcx.item_name(macro_call.def_id) + ), None, - "use `panic!()` or `unreachable!()`", + "remove it", ); - }; - let lint_false_with_message = |panic_message: String| { + } else if !is_debug { + let (assert_arg, panic_arg) = match panic_expn { + PanicExpn::Empty => ("", ""), + _ => (", ..", ".."), + }; span_lint_and_help( cx, ASSERTIONS_ON_CONSTANTS, - e.span, - &format!("`assert!(false, {})` should probably be replaced", panic_message), + macro_call.span, + &format!("`assert!(false{})` should probably be replaced", assert_arg), None, - &format!("use `panic!({})` or `unreachable!({})`", panic_message, panic_message), + &format!("use `panic!({})` or `unreachable!({0})`", panic_arg), ); - }; - - if let Some(debug_assert_span) = is_expn_of(e.span, "debug_assert") { - if debug_assert_span.from_expansion() { - return; - } - if_chain! { - if let ExprKind::Unary(_, lit) = e.kind; - if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.typeck_results(), lit); - if is_true; - then { - lint_true(true); - } - }; - } else if let Some(assert_span) = is_direct_expn_of(e.span, "assert") { - if assert_span.from_expansion() { - return; - } - if let Some(assert_match) = match_assert_with_message(cx, e) { - match assert_match { - // matched assert but not message - AssertKind::WithoutMessage(false) => lint_false_without_message(), - AssertKind::WithoutMessage(true) | AssertKind::WithMessage(_, true) => lint_true(false), - AssertKind::WithMessage(panic_message, false) => lint_false_with_message(panic_message), - }; - } - } - } -} - -/// Result of calling `match_assert_with_message`. -enum AssertKind { - WithMessage(String, bool), - WithoutMessage(bool), -} - -/// Check if the expression matches -/// -/// ```rust,ignore -/// if !c { -/// { -/// ::std::rt::begin_panic(message, _) -/// } -/// } -/// ``` -/// -/// where `message` is any expression and `c` is a constant bool. -fn match_assert_with_message<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option { - if_chain! { - if let Some(higher::If { cond, then, .. }) = higher::If::hir(expr); - if let ExprKind::Unary(UnOp::Not, expr) = cond.kind; - // bind the first argument of the `assert!` macro - if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.typeck_results(), expr); - let begin_panic_call = peel_blocks(then); - // function call - if let Some(arg) = match_panic_call(cx, begin_panic_call); - // bind the second argument of the `assert!` macro if it exists - if let panic_message = snippet_opt(cx, arg.span); - // second argument of begin_panic is irrelevant - // as is the second match arm - then { - // an empty message occurs when it was generated by the macro - // (and not passed by the user) - return panic_message - .filter(|msg| !msg.is_empty()) - .map(|msg| AssertKind::WithMessage(msg, is_true)) - .or(Some(AssertKind::WithoutMessage(is_true))); } } - None } diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 0629674307ba..a58d12ddd6b4 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -1,9 +1,10 @@ //! checks for attributes use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then}; +use clippy_utils::macros::{is_panic, macro_backtrace}; use clippy_utils::msrvs; use clippy_utils::source::{first_line_of_span, is_present_in_source, snippet_opt, without_block_comments}; -use clippy_utils::{extract_msrv_attr, match_panic_def_id, meets_msrv}; +use clippy_utils::{extract_msrv_attr, meets_msrv}; use if_chain::if_chain; use rustc_ast::{AttrKind, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem}; use rustc_errors::Applicability; @@ -443,20 +444,15 @@ fn is_relevant_block(cx: &LateContext<'_>, typeck_results: &ty::TypeckResults<'_ } fn is_relevant_expr(cx: &LateContext<'_>, typeck_results: &ty::TypeckResults<'_>, expr: &Expr<'_>) -> bool { + if macro_backtrace(expr.span).last().map_or(false, |macro_call| { + is_panic(cx, macro_call.def_id) || cx.tcx.item_name(macro_call.def_id) == sym::unreachable + }) { + return false; + } match &expr.kind { ExprKind::Block(block, _) => is_relevant_block(cx, typeck_results, block), ExprKind::Ret(Some(e)) => is_relevant_expr(cx, typeck_results, e), ExprKind::Ret(None) | ExprKind::Break(_, None) => false, - ExprKind::Call(path_expr, _) => { - if let ExprKind::Path(qpath) = &path_expr.kind { - typeck_results - .qpath_res(qpath, path_expr.hir_id) - .opt_def_id() - .map_or(true, |fun_id| !match_panic_def_id(cx, fun_id)) - } else { - true - } - }, _ => true, } } diff --git a/clippy_lints/src/bool_assert_comparison.rs b/clippy_lints/src/bool_assert_comparison.rs index d0b8c52a36a9..ea2cfb0bc082 100644 --- a/clippy_lints/src/bool_assert_comparison.rs +++ b/clippy_lints/src/bool_assert_comparison.rs @@ -1,4 +1,5 @@ -use clippy_utils::{diagnostics::span_lint_and_sugg, higher, is_direct_expn_of, ty::implements_trait}; +use clippy_utils::macros::{find_assert_eq_args, root_macro_call_first_node}; +use clippy_utils::{diagnostics::span_lint_and_sugg, ty::implements_trait}; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, Lit}; @@ -66,44 +67,40 @@ fn is_impl_not_trait_with_bool_out(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - let macros = ["assert_eq", "debug_assert_eq"]; - let inverted_macros = ["assert_ne", "debug_assert_ne"]; - - for mac in macros.iter().chain(inverted_macros.iter()) { - if let Some(span) = is_direct_expn_of(expr.span, mac) { - if let Some(args) = higher::extract_assert_macro_args(expr) { - if let [a, b, ..] = args[..] { - let nb_bool_args = usize::from(is_bool_lit(a)) + usize::from(is_bool_lit(b)); - - if nb_bool_args != 1 { - // If there are two boolean arguments, we definitely don't understand - // what's going on, so better leave things as is... - // - // Or there is simply no boolean and then we can leave things as is! - return; - } - - if !is_impl_not_trait_with_bool_out(cx, a) || !is_impl_not_trait_with_bool_out(cx, b) { - // At this point the expression which is not a boolean - // literal does not implement Not trait with a bool output, - // so we cannot suggest to rewrite our code - return; - } + let Some(macro_call) = root_macro_call_first_node(cx, expr) else { return }; + let macro_name = cx.tcx.item_name(macro_call.def_id); + if !matches!( + macro_name.as_str(), + "assert_eq" | "debug_assert_eq" | "assert_ne" | "debug_assert_ne" + ) { + return; + } + let Some ((a, b, _)) = find_assert_eq_args(cx, expr, macro_call.expn) else { return }; + if !(is_bool_lit(a) ^ is_bool_lit(b)) { + // If there are two boolean arguments, we definitely don't understand + // what's going on, so better leave things as is... + // + // Or there is simply no boolean and then we can leave things as is! + return; + } - let non_eq_mac = &mac[..mac.len() - 3]; - span_lint_and_sugg( - cx, - BOOL_ASSERT_COMPARISON, - span, - &format!("used `{}!` with a literal bool", mac), - "replace it with", - format!("{}!(..)", non_eq_mac), - Applicability::MaybeIncorrect, - ); - return; - } - } - } + if !is_impl_not_trait_with_bool_out(cx, a) || !is_impl_not_trait_with_bool_out(cx, b) { + // At this point the expression which is not a boolean + // literal does not implement Not trait with a bool output, + // so we cannot suggest to rewrite our code + return; } + + let macro_name = macro_name.as_str(); + let non_eq_mac = ¯o_name[..macro_name.len() - 3]; + span_lint_and_sugg( + cx, + BOOL_ASSERT_COMPARISON, + macro_call.span, + &format!("used `{}!` with a literal bool", macro_name), + "replace it with", + format!("{}!(..)", non_eq_mac), + Applicability::MaybeIncorrect, + ); } } diff --git a/clippy_lints/src/doc.rs b/clippy_lints/src/doc.rs index 3650e4f91a00..9d4195e400db 100644 --- a/clippy_lints/src/doc.rs +++ b/clippy_lints/src/doc.rs @@ -1,8 +1,9 @@ use clippy_utils::attrs::is_doc_hidden; use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_note, span_lint_and_then}; +use clippy_utils::macros::{is_panic, root_macro_call_first_node}; use clippy_utils::source::{first_line_of_span, snippet_with_applicability}; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; -use clippy_utils::{is_entrypoint_fn, is_expn_of, match_panic_def_id, method_chain_args, return_ty}; +use clippy_utils::{is_entrypoint_fn, method_chain_args, return_ty}; use if_chain::if_chain; use itertools::Itertools; use rustc_ast::ast::{Async, AttrKind, Attribute, Fn, FnRetTy, ItemKind}; @@ -13,7 +14,7 @@ use rustc_errors::emitter::EmitterWriter; use rustc_errors::{Applicability, Handler, SuggestionStyle}; use rustc_hir as hir; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; -use rustc_hir::{AnonConst, Expr, ExprKind, QPath}; +use rustc_hir::{AnonConst, Expr}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::map::Map; use rustc_middle::lint::in_external_macro; @@ -805,24 +806,17 @@ impl<'a, 'tcx> Visitor<'tcx> for FindPanicUnwrap<'a, 'tcx> { return; } - // check for `begin_panic` - if_chain! { - if let ExprKind::Call(func_expr, _) = expr.kind; - if let ExprKind::Path(QPath::Resolved(_, path)) = func_expr.kind; - if let Some(path_def_id) = path.res.opt_def_id(); - if match_panic_def_id(self.cx, path_def_id); - if is_expn_of(expr.span, "unreachable").is_none(); - if !is_expn_of_debug_assertions(expr.span); - then { - self.panic_span = Some(expr.span); + if let Some(macro_call) = root_macro_call_first_node(self.cx, expr) { + if is_panic(self.cx, macro_call.def_id) + || matches!( + self.cx.tcx.item_name(macro_call.def_id).as_str(), + "assert" | "assert_eq" | "assert_ne" | "todo" + ) + { + self.panic_span = Some(macro_call.span); } } - // check for `assert_eq` or `assert_ne` - if is_expn_of(expr.span, "assert_eq").is_some() || is_expn_of(expr.span, "assert_ne").is_some() { - self.panic_span = Some(expr.span); - } - // check for `unwrap` if let Some(arglists) = method_chain_args(expr, &["unwrap"]) { let receiver_ty = self.typeck_results.expr_ty(&arglists[0][0]).peel_refs(); @@ -844,8 +838,3 @@ impl<'a, 'tcx> Visitor<'tcx> for FindPanicUnwrap<'a, 'tcx> { NestedVisitorMap::OnlyBodies(self.cx.tcx.hir()) } } - -fn is_expn_of_debug_assertions(span: Span) -> bool { - const MACRO_NAMES: &[&str] = &["debug_assert", "debug_assert_eq", "debug_assert_ne"]; - MACRO_NAMES.iter().any(|name| is_expn_of(span, name).is_some()) -} diff --git a/clippy_lints/src/eq_op.rs b/clippy_lints/src/eq_op.rs index 101234605273..df75b815436b 100644 --- a/clippy_lints/src/eq_op.rs +++ b/clippy_lints/src/eq_op.rs @@ -1,10 +1,11 @@ use clippy_utils::diagnostics::{multispan_sugg, span_lint, span_lint_and_then}; +use clippy_utils::macros::{find_assert_eq_args, first_node_macro_backtrace}; use clippy_utils::source::snippet; use clippy_utils::ty::{implements_trait, is_copy}; -use clippy_utils::{ast_utils::is_useless_with_eq_exprs, eq_expr_value, higher, is_expn_of, is_in_test_function}; +use clippy_utils::{ast_utils::is_useless_with_eq_exprs, eq_expr_value, is_in_test_function}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, StmtKind}; +use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -68,32 +69,26 @@ declare_clippy_lint! { declare_lint_pass!(EqOp => [EQ_OP, OP_REF]); -const ASSERT_MACRO_NAMES: [&str; 4] = ["assert_eq", "assert_ne", "debug_assert_eq", "debug_assert_ne"]; - impl<'tcx> LateLintPass<'tcx> for EqOp { #[allow(clippy::similar_names, clippy::too_many_lines)] fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { - if let ExprKind::Block(block, _) = e.kind { - for stmt in block.stmts { - for amn in &ASSERT_MACRO_NAMES { - if_chain! { - if is_expn_of(stmt.span, amn).is_some(); - if let StmtKind::Semi(matchexpr) = stmt.kind; - if let Some(macro_args) = higher::extract_assert_macro_args(matchexpr); - if macro_args.len() == 2; - let (lhs, rhs) = (macro_args[0], macro_args[1]); - if eq_expr_value(cx, lhs, rhs); - if !is_in_test_function(cx.tcx, e.hir_id); - then { - span_lint( - cx, - EQ_OP, - lhs.span.to(rhs.span), - &format!("identical args used in this `{}!` macro call", amn), - ); - } - } - } + if_chain! { + if let Some((macro_call, macro_name)) = first_node_macro_backtrace(cx, e).find_map(|macro_call| { + let name = cx.tcx.item_name(macro_call.def_id); + matches!(name.as_str(), "assert_eq" | "assert_ne" | "debug_assert_eq" | "debug_assert_ne") + .then(|| (macro_call, name)) + }); + if let Some((lhs, rhs, _)) = find_assert_eq_args(cx, e, macro_call.expn); + if eq_expr_value(cx, lhs, rhs); + if macro_call.is_local(); + if !is_in_test_function(cx.tcx, e.hir_id); + then { + span_lint( + cx, + EQ_OP, + lhs.span.to(rhs.span), + &format!("identical args used in this `{}!` macro call", macro_name), + ); } } if let ExprKind::Binary(op, left, right) = e.kind { diff --git a/clippy_lints/src/explicit_write.rs b/clippy_lints/src/explicit_write.rs index 6b327b9ce172..98e5234e0aa9 100644 --- a/clippy_lints/src/explicit_write.rs +++ b/clippy_lints/src/explicit_write.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; -use clippy_utils::higher::FormatArgsExpn; +use clippy_utils::macros::FormatArgsExpn; use clippy_utils::{is_expn_of, match_function_call, paths}; use if_chain::if_chain; use rustc_errors::Applicability; @@ -48,7 +48,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitWrite { } else { None }; - if let Some(format_args) = FormatArgsExpn::parse(write_arg); + if let Some(format_args) = FormatArgsExpn::parse(cx, write_arg); then { let calling_macro = // ordering is important here, since `writeln!` uses `write!` internally @@ -80,7 +80,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitWrite { ) }; let msg = format!("use of `{}.unwrap()`", used); - if let [write_output] = *format_args.format_string_symbols { + if let [write_output] = *format_args.format_string_parts { let mut write_output = write_output.to_string(); if write_output.ends_with('\n') { write_output.pop(); diff --git a/clippy_lints/src/fallible_impl_from.rs b/clippy_lints/src/fallible_impl_from.rs index 05d300058cf4..02f1baf27fae 100644 --- a/clippy_lints/src/fallible_impl_from.rs +++ b/clippy_lints/src/fallible_impl_from.rs @@ -1,6 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::macros::{is_panic, root_macro_call_first_node}; +use clippy_utils::method_chain_args; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{is_expn_of, match_panic_def_id, method_chain_args}; use if_chain::if_chain; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass}; @@ -68,7 +69,7 @@ impl<'tcx> LateLintPass<'tcx> for FallibleImplFrom { fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_items: &[hir::ImplItemRef]) { use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; - use rustc_hir::{Expr, ExprKind, ImplItemKind, QPath}; + use rustc_hir::{Expr, ImplItemKind}; struct FindPanicUnwrap<'a, 'tcx> { lcx: &'a LateContext<'tcx>, @@ -80,14 +81,8 @@ fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_items: &[h type Map = Map<'tcx>; fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { - // check for `begin_panic` - if_chain! { - if let ExprKind::Call(func_expr, _) = expr.kind; - if let ExprKind::Path(QPath::Resolved(_, path)) = func_expr.kind; - if let Some(path_def_id) = path.res.opt_def_id(); - if match_panic_def_id(self.lcx, path_def_id); - if is_expn_of(expr.span, "unreachable").is_none(); - then { + if let Some(macro_call) = root_macro_call_first_node(self.lcx, expr) { + if is_panic(self.lcx, macro_call.def_id) { self.result.push(expr.span); } } diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index 3f043e5f2f1c..688d8f8630f3 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::higher::FormatExpn; +use clippy_utils::macros::{root_macro_call_first_node, FormatArgsExpn}; use clippy_utils::source::{snippet_opt, snippet_with_applicability}; use clippy_utils::sugg::Sugg; use if_chain::if_chain; @@ -43,38 +43,41 @@ declare_lint_pass!(UselessFormat => [USELESS_FORMAT]); impl<'tcx> LateLintPass<'tcx> for UselessFormat { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - let FormatExpn { call_site, format_args } = match FormatExpn::parse(expr) { - Some(e) if !e.call_site.from_expansion() => e, - _ => return, + let (format_args, call_site) = if_chain! { + if let Some(macro_call) = root_macro_call_first_node(cx, expr); + if cx.tcx.is_diagnostic_item(sym::format_macro, macro_call.def_id); + if let Some(format_args) = FormatArgsExpn::find_nested(cx, expr, macro_call.expn); + then { + (format_args, macro_call.span) + } else { + return + } }; let mut applicability = Applicability::MachineApplicable; if format_args.value_args.is_empty() { - if format_args.format_string_parts.is_empty() { - span_useless_format_empty(cx, call_site, "String::new()".to_owned(), applicability); - } else { - if_chain! { - if let [e] = &*format_args.format_string_parts; - if let ExprKind::Lit(lit) = &e.kind; - if let Some(s_src) = snippet_opt(cx, lit.span); - then { + match *format_args.format_string_parts { + [] => span_useless_format_empty(cx, call_site, "String::new()".to_owned(), applicability), + [_] => { + if let Some(s_src) = snippet_opt(cx, format_args.format_string_span) { // Simulate macro expansion, converting {{ and }} to { and }. let s_expand = s_src.replace("{{", "{").replace("}}", "}"); let sugg = format!("{}.to_string()", s_expand); span_useless_format(cx, call_site, sugg, applicability); } - } + }, + [..] => {}, } } else if let [value] = *format_args.value_args { if_chain! { - if format_args.format_string_symbols == [kw::Empty]; + if format_args.format_string_parts == [kw::Empty]; if match cx.typeck_results().expr_ty(value).peel_refs().kind() { ty::Adt(adt, _) => cx.tcx.is_diagnostic_item(sym::String, adt.did), ty::Str => true, _ => false, }; if let Some(args) = format_args.args(); - if args.iter().all(|arg| arg.is_display() && !arg.has_string_formatting()); + if args.iter().all(|arg| arg.format_trait == sym::Display && !arg.has_string_formatting()); then { let is_new_string = match value.kind { ExprKind::Binary(..) => true, diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index f0e1a67dcddb..ae423d799d71 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; -use clippy_utils::higher::{FormatArgsArg, FormatArgsExpn, FormatExpn}; +use clippy_utils::macros::{FormatArgsArg, FormatArgsExpn}; use clippy_utils::source::snippet_opt; use clippy_utils::ty::implements_trait; use clippy_utils::{is_diag_trait_item, match_def_path, paths}; @@ -83,7 +83,7 @@ const FORMAT_MACRO_DIAG_ITEMS: &[Symbol] = &[sym::format_macro, sym::std_panic_m impl<'tcx> LateLintPass<'tcx> for FormatArgs { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if_chain! { - if let Some(format_args) = FormatArgsExpn::parse(expr); + if let Some(format_args) = FormatArgsExpn::parse(cx, expr); let expr_expn_data = expr.span.ctxt().outer_expn_data(); let outermost_expn_data = outermost_expn_data(expr_expn_data); if let Some(macro_def_id) = outermost_expn_data.macro_def_id; @@ -97,7 +97,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs { if let Some(args) = format_args.args(); then { for (i, arg) in args.iter().enumerate() { - if !arg.is_display() { + if arg.format_trait != sym::Display { continue; } if arg.has_string_formatting() { @@ -106,8 +106,8 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs { if is_aliased(&args, i) { continue; } - check_format_in_format_args(cx, outermost_expn_data.call_site, name, arg); - check_to_string_in_format_args(cx, name, arg); + check_format_in_format_args(cx, outermost_expn_data.call_site, name, arg.value); + check_to_string_in_format_args(cx, name, arg.value); } } } @@ -122,30 +122,31 @@ fn outermost_expn_data(expn_data: ExpnData) -> ExpnData { } } -fn check_format_in_format_args(cx: &LateContext<'_>, call_site: Span, name: Symbol, arg: &FormatArgsArg<'_>) { - if_chain! { - if FormatExpn::parse(arg.value).is_some(); - if !arg.value.span.ctxt().outer_expn_data().call_site.from_expansion(); - then { - span_lint_and_then( - cx, - FORMAT_IN_FORMAT_ARGS, - call_site, - &format!("`format!` in `{}!` args", name), - |diag| { - diag.help(&format!( - "combine the `format!(..)` arguments with the outer `{}!(..)` call", - name - )); - diag.help("or consider changing `format!` to `format_args!`"); - }, - ); - } +fn check_format_in_format_args(cx: &LateContext<'_>, call_site: Span, name: Symbol, arg: &Expr<'_>) { + let expn_data = arg.span.ctxt().outer_expn_data(); + if expn_data.call_site.from_expansion() { + return; + } + let Some(mac_id) = expn_data.macro_def_id else { return }; + if !cx.tcx.is_diagnostic_item(sym::format_macro, mac_id) { + return; } + span_lint_and_then( + cx, + FORMAT_IN_FORMAT_ARGS, + call_site, + &format!("`format!` in `{}!` args", name), + |diag| { + diag.help(&format!( + "combine the `format!(..)` arguments with the outer `{}!(..)` call", + name + )); + diag.help("or consider changing `format!` to `format_args!`"); + }, + ); } -fn check_to_string_in_format_args<'tcx>(cx: &LateContext<'tcx>, name: Symbol, arg: &FormatArgsArg<'tcx>) { - let value = arg.value; +fn check_to_string_in_format_args(cx: &LateContext<'_>, name: Symbol, value: &Expr<'_>) { if_chain! { if !value.span.from_expansion(); if let ExprKind::MethodCall(_, _, [receiver], _) = value.kind; diff --git a/clippy_lints/src/manual_assert.rs b/clippy_lints/src/manual_assert.rs index 5a2a965716cc..e3a34b22e325 100644 --- a/clippy_lints/src/manual_assert.rs +++ b/clippy_lints/src/manual_assert.rs @@ -1,11 +1,12 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::higher::PanicExpn; +use clippy_utils::macros::{root_macro_call, FormatArgsExpn}; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::{is_expn_of, sugg}; +use clippy_utils::{peel_blocks_with_stmt, sugg}; use rustc_errors::Applicability; -use rustc_hir::{Block, Expr, ExprKind, StmtKind, UnOp}; +use rustc_hir::{Expr, ExprKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; declare_clippy_lint! { /// ### What it does @@ -35,64 +36,33 @@ declare_clippy_lint! { declare_lint_pass!(ManualAssert => [MANUAL_ASSERT]); impl LateLintPass<'_> for ManualAssert { - fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) { if_chain! { - if let Expr { - kind: ExprKind:: If(cond, Expr { - kind: ExprKind::Block( - Block { - stmts: [stmt], - .. - }, - _), - .. - }, None), - .. - } = &expr; - if is_expn_of(stmt.span, "panic").is_some(); + if let ExprKind::If(cond, then, None) = expr.kind; if !matches!(cond.kind, ExprKind::Let(_)); - if let StmtKind::Semi(semi) = stmt.kind; + if !expr.span.from_expansion(); + let then = peel_blocks_with_stmt(then); + if let Some(macro_call) = root_macro_call(then.span); + if cx.tcx.item_name(macro_call.def_id) == sym::panic; if !cx.tcx.sess.source_map().is_multiline(cond.span); - + if let Some(format_args) = FormatArgsExpn::find_nested(cx, then, macro_call.expn); then { - let call = if_chain! { - if let ExprKind::Block(block, _) = semi.kind; - if let Some(init) = block.expr; - then { - init - } else { - semi - } - }; - let span = if let Some(panic_expn) = PanicExpn::parse(call) { - match *panic_expn.format_args.value_args { - [] => panic_expn.format_args.format_string_span, - [.., last] => panic_expn.format_args.format_string_span.to(last.span), - } - } else if let ExprKind::Call(_, [format_args]) = call.kind { - format_args.span - } else { - return - }; let mut applicability = Applicability::MachineApplicable; - let sugg = snippet_with_applicability(cx, span, "..", &mut applicability); - let cond_sugg = if let ExprKind::DropTemps(e, ..) = cond.kind { - if let Expr{kind: ExprKind::Unary(UnOp::Not, not_expr), ..} = e { - sugg::Sugg::hir_with_applicability(cx, not_expr, "..", &mut applicability).maybe_par().to_string() - } else { - format!("!{}", sugg::Sugg::hir_with_applicability(cx, e, "..", &mut applicability).maybe_par()) - } - } else { - format!("!{}", sugg::Sugg::hir_with_applicability(cx, cond, "..", &mut applicability).maybe_par()) + let format_args_snip = snippet_with_applicability(cx, format_args.inputs_span(), "..", &mut applicability); + let cond = cond.peel_drop_temps(); + let (cond, not) = match cond.kind { + ExprKind::Unary(UnOp::Not, e) => (e, ""), + _ => (cond, "!"), }; - + let cond_sugg = sugg::Sugg::hir_with_applicability(cx, cond, "..", &mut applicability).maybe_par(); + let sugg = format!("assert!({not}{cond_sugg}, {format_args_snip});"); span_lint_and_sugg( cx, MANUAL_ASSERT, expr.span, "only a `panic!` in `if`-then statement", "try", - format!("assert!({}, {});", cond_sugg, sugg), + sugg, Applicability::MachineApplicable, ); } diff --git a/clippy_lints/src/matches.rs b/clippy_lints/src/matches.rs index 22970507f964..0d693499bc67 100644 --- a/clippy_lints/src/matches.rs +++ b/clippy_lints/src/matches.rs @@ -2,16 +2,17 @@ use clippy_utils::consts::{constant, constant_full_int, miri_to_const, FullInt}; use clippy_utils::diagnostics::{ multispan_sugg, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then, }; -use clippy_utils::higher; +use clippy_utils::macros::{is_panic, root_macro_call}; use clippy_utils::source::{expr_block, indent_of, snippet, snippet_block, snippet_opt, snippet_with_applicability}; use clippy_utils::sugg::Sugg; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, match_type, peel_mid_ty_refs}; use clippy_utils::visitors::is_local_used; use clippy_utils::{ - get_parent_expr, is_expn_of, is_lang_ctor, is_lint_allowed, is_refutable, is_unit_expr, is_wild, meets_msrv, msrvs, + get_parent_expr, is_lang_ctor, is_lint_allowed, is_refutable, is_unit_expr, is_wild, meets_msrv, msrvs, path_to_local, path_to_local_id, peel_blocks, peel_hir_pat_refs, peel_n_hir_expr_refs, recurse_or_patterns, strip_pat_refs, }; +use clippy_utils::{higher, peel_blocks_with_stmt}; use clippy_utils::{paths, search_same, SpanlessEq, SpanlessHash}; use core::iter::{once, ExactSizeIterator}; use if_chain::if_chain; @@ -974,7 +975,8 @@ fn check_wild_err_arm<'tcx>(cx: &LateContext<'tcx>, ex: &Expr<'tcx>, arms: &[Arm } if_chain! { if matching_wild; - if is_panic_call(arm.body); + if let Some(macro_call) = root_macro_call(peel_blocks_with_stmt(arm.body).span); + if is_panic(cx, macro_call.def_id); then { // `Err(_)` or `Err(_e)` arm with `panic!` found span_lint_and_note(cx, @@ -1179,22 +1181,6 @@ fn check_wild_enum_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>]) }; } -// If the block contains only a `panic!` macro (as expression or statement) -fn is_panic_call(expr: &Expr<'_>) -> bool { - // Unwrap any wrapping blocks - let span = if let ExprKind::Block(block, _) = expr.kind { - match (&block.expr, block.stmts.len(), block.stmts.first()) { - (&Some(exp), 0, _) => exp.span, - (&None, 1, Some(stmt)) => stmt.span, - _ => return false, - } - } else { - expr.span - }; - - is_expn_of(span, "panic").is_some() && is_expn_of(span, "unreachable").is_none() -} - fn check_match_ref_pats<'a, 'b, I>(cx: &LateContext<'_>, ex: &Expr<'_>, pats: I, expr: &Expr<'_>) where 'b: 'a, diff --git a/clippy_lints/src/methods/expect_fun_call.rs b/clippy_lints/src/methods/expect_fun_call.rs index 0ec9387f9c46..866eaff5b1d7 100644 --- a/clippy_lints/src/methods/expect_fun_call.rs +++ b/clippy_lints/src/methods/expect_fun_call.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::higher::FormatExpn; +use clippy_utils::macros::{root_macro_call_first_node, FormatArgsExpn}; use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::is_type_diagnostic_item; use rustc_errors::Applicability; @@ -14,7 +14,13 @@ use super::EXPECT_FUN_CALL; /// Checks for the `EXPECT_FUN_CALL` lint. #[allow(clippy::too_many_lines)] -pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_span: Span, name: &str, args: &[hir::Expr<'_>]) { +pub(super) fn check( + cx: &LateContext<'tcx>, + expr: &hir::Expr<'_>, + method_span: Span, + name: &str, + args: &'tcx [hir::Expr<'tcx>], +) { // Strip `&`, `as_ref()` and `as_str()` off `arg` until we're left with either a `String` or // `&str` fn get_arg_root<'a>(cx: &LateContext<'_>, arg: &'a hir::Expr<'a>) -> &'a hir::Expr<'a> { @@ -128,11 +134,12 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, method_span: Spa let mut applicability = Applicability::MachineApplicable; //Special handling for `format!` as arg_root - if let Some(format_expn) = FormatExpn::parse(arg_root) { - let span = match *format_expn.format_args.value_args { - [] => format_expn.format_args.format_string_span, - [.., last] => format_expn.format_args.format_string_span.to(last.span), - }; + if let Some(macro_call) = root_macro_call_first_node(cx, arg_root) { + if !cx.tcx.is_diagnostic_item(sym::format_macro, macro_call.def_id) { + return; + } + let Some(format_args) = FormatArgsExpn::find_nested(cx, arg_root, macro_call.expn) else { return }; + let span = format_args.inputs_span(); let sugg = snippet_with_applicability(cx, span, "..", &mut applicability); span_lint_and_sugg( cx, diff --git a/clippy_lints/src/mutable_debug_assertion.rs b/clippy_lints/src/mutable_debug_assertion.rs index fa7274990db3..49d4b3a1850f 100644 --- a/clippy_lints/src/mutable_debug_assertion.rs +++ b/clippy_lints/src/mutable_debug_assertion.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint; -use clippy_utils::{higher, is_direct_expn_of}; +use clippy_utils::macros::{find_assert_eq_args, root_macro_call_first_node}; use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; use rustc_hir::{BorrowKind, Expr, ExprKind, MatchSource, Mutability}; use rustc_lint::{LateContext, LateLintPass}; @@ -34,26 +34,30 @@ declare_clippy_lint! { declare_lint_pass!(DebugAssertWithMutCall => [DEBUG_ASSERT_WITH_MUT_CALL]); -const DEBUG_MACRO_NAMES: [&str; 3] = ["debug_assert", "debug_assert_eq", "debug_assert_ne"]; - impl<'tcx> LateLintPass<'tcx> for DebugAssertWithMutCall { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { - for dmn in &DEBUG_MACRO_NAMES { - if is_direct_expn_of(e.span, dmn).is_some() { - if let Some(macro_args) = higher::extract_assert_macro_args(e) { - for arg in macro_args { - let mut visitor = MutArgVisitor::new(cx); - visitor.visit_expr(arg); - if let Some(span) = visitor.expr_span() { - span_lint( - cx, - DEBUG_ASSERT_WITH_MUT_CALL, - span, - &format!("do not call a function with mutable arguments inside of `{}!`", dmn), - ); - } - } - } + let Some(macro_call) = root_macro_call_first_node(cx, e) else { return }; + let macro_name = cx.tcx.item_name(macro_call.def_id); + if !matches!( + macro_name.as_str(), + "debug_assert" | "debug_assert_eq" | "debug_assert_ne" + ) { + return; + } + let Some((lhs, rhs, _)) = find_assert_eq_args(cx, e, macro_call.expn) else { return }; + for arg in [lhs, rhs] { + let mut visitor = MutArgVisitor::new(cx); + visitor.visit_expr(arg); + if let Some(span) = visitor.expr_span() { + span_lint( + cx, + DEBUG_ASSERT_WITH_MUT_CALL, + span, + &format!( + "do not call a function with mutable arguments inside of `{}!`", + macro_name + ), + ); } } } diff --git a/clippy_lints/src/panic_in_result_fn.rs b/clippy_lints/src/panic_in_result_fn.rs index 8769c0452146..b7a56970b335 100644 --- a/clippy_lints/src/panic_in_result_fn.rs +++ b/clippy_lints/src/panic_in_result_fn.rs @@ -1,8 +1,10 @@ use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::macros::root_macro_call_first_node; +use clippy_utils::return_ty; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{find_macro_calls, is_expn_of, return_ty}; +use clippy_utils::visitors::expr_visitor_no_bodies; use rustc_hir as hir; -use rustc_hir::intravisit::FnKind; +use rustc_hir::intravisit::{FnKind, Visitor}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{sym, Span}; @@ -55,19 +57,19 @@ impl<'tcx> LateLintPass<'tcx> for PanicInResultFn { } fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir::Body<'tcx>) { - let mut panics = find_macro_calls( - &[ - "unimplemented", - "unreachable", - "panic", - "todo", - "assert", - "assert_eq", - "assert_ne", - ], - body, - ); - panics.retain(|span| is_expn_of(*span, "debug_assert").is_none()); + let mut panics = Vec::new(); + expr_visitor_no_bodies(|expr| { + let Some(macro_call) = root_macro_call_first_node(cx, expr) else { return true }; + if matches!( + &*cx.tcx.item_name(macro_call.def_id).as_str(), + "unimplemented" | "unreachable" | "panic" | "todo" | "assert" | "assert_eq" | "assert_ne" + ) { + panics.push(macro_call.span); + return false; + } + true + }) + .visit_expr(&body.value); if !panics.is_empty() { span_lint_and_then( cx, diff --git a/clippy_lints/src/panic_unimplemented.rs b/clippy_lints/src/panic_unimplemented.rs index edfac824ded9..6ef6b9a20aa4 100644 --- a/clippy_lints/src/panic_unimplemented.rs +++ b/clippy_lints/src/panic_unimplemented.rs @@ -1,10 +1,8 @@ use clippy_utils::diagnostics::span_lint; -use clippy_utils::{is_expn_of, match_panic_call}; -use if_chain::if_chain; +use clippy_utils::macros::{is_panic, root_macro_call_first_node}; use rustc_hir::Expr; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::Span; declare_clippy_lint! { /// ### What it does @@ -78,37 +76,37 @@ declare_lint_pass!(PanicUnimplemented => [UNIMPLEMENTED, UNREACHABLE, TODO, PANI impl<'tcx> LateLintPass<'tcx> for PanicUnimplemented { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if match_panic_call(cx, expr).is_some() - && (is_expn_of(expr.span, "debug_assert").is_none() && is_expn_of(expr.span, "assert").is_none()) - { - let span = get_outer_span(expr); - if is_expn_of(expr.span, "unimplemented").is_some() { + let Some(macro_call) = root_macro_call_first_node(cx, expr) else { return }; + if is_panic(cx, macro_call.def_id) { + span_lint( + cx, + PANIC, + macro_call.span, + "`panic` should not be present in production code", + ); + return; + } + match cx.tcx.item_name(macro_call.def_id).as_str() { + "todo" => { + span_lint( + cx, + TODO, + macro_call.span, + "`todo` should not be present in production code", + ); + }, + "unimplemented" => { span_lint( cx, UNIMPLEMENTED, - span, + macro_call.span, "`unimplemented` should not be present in production code", ); - } else if is_expn_of(expr.span, "todo").is_some() { - span_lint(cx, TODO, span, "`todo` should not be present in production code"); - } else if is_expn_of(expr.span, "unreachable").is_some() { - span_lint(cx, UNREACHABLE, span, "usage of the `unreachable!` macro"); - } else if is_expn_of(expr.span, "panic").is_some() { - span_lint(cx, PANIC, span, "`panic` should not be present in production code"); - } - } - } -} - -fn get_outer_span(expr: &Expr<'_>) -> Span { - if_chain! { - if expr.span.from_expansion(); - let first = expr.span.ctxt().outer_expn_data().call_site; - if first.from_expansion(); - then { - first.ctxt().outer_expn_data().call_site - } else { - expr.span + }, + "unreachable" => { + span_lint(cx, UNREACHABLE, macro_call.span, "usage of the `unreachable!` macro"); + }, + _ => {}, } } } diff --git a/clippy_lints/src/unit_types/unit_cmp.rs b/clippy_lints/src/unit_types/unit_cmp.rs index 6d9aff474214..1dd8895ebd07 100644 --- a/clippy_lints/src/unit_types/unit_cmp.rs +++ b/clippy_lints/src/unit_types/unit_cmp.rs @@ -1,35 +1,29 @@ use clippy_utils::diagnostics::span_lint; +use clippy_utils::macros::{find_assert_eq_args, root_macro_call_first_node}; use rustc_hir::{BinOpKind, Expr, ExprKind}; use rustc_lint::LateContext; -use rustc_span::hygiene::{ExpnKind, MacroKind}; use super::UNIT_CMP; pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>) { if expr.span.from_expansion() { - if let Some(callee) = expr.span.source_callee() { - if let ExpnKind::Macro(MacroKind::Bang, symbol) = callee.kind { - if let ExprKind::Binary(ref cmp, left, _) = expr.kind { - let op = cmp.node; - if op.is_comparison() && cx.typeck_results().expr_ty(left).is_unit() { - let result = match symbol.as_str() { - "assert_eq" | "debug_assert_eq" => "succeed", - "assert_ne" | "debug_assert_ne" => "fail", - _ => return, - }; - span_lint( - cx, - UNIT_CMP, - expr.span, - &format!( - "`{}` of unit values detected. This will always {}", - symbol.as_str(), - result - ), - ); - } - } + if let Some(macro_call) = root_macro_call_first_node(cx, expr) { + let macro_name = cx.tcx.item_name(macro_call.def_id); + let result = match macro_name.as_str() { + "assert_eq" | "debug_assert_eq" => "succeed", + "assert_ne" | "debug_assert_ne" => "fail", + _ => return, + }; + let Some ((left, _, _)) = find_assert_eq_args(cx, expr, macro_call.expn) else { return }; + if !cx.typeck_results().expr_ty(left).is_unit() { + return; } + span_lint( + cx, + UNIT_CMP, + macro_call.span, + &format!("`{}` of unit values detected. This will always {}", macro_name, result), + ); } return; } diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index e98dcd3cf983..165004e5b413 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -1,5 +1,6 @@ use clippy_utils::consts::{constant_simple, Constant}; use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then}; +use clippy_utils::macros::root_macro_call_first_node; use clippy_utils::source::snippet; use clippy_utils::ty::match_type; use clippy_utils::{ @@ -410,9 +411,13 @@ impl<'tcx> LateLintPass<'tcx> for LintWithoutLintPass { } self.declared_lints.insert(item.ident.name, item.span); } - } else if is_expn_of(item.span, "impl_lint_pass").is_some() - || is_expn_of(item.span, "declare_lint_pass").is_some() - { + } else if let Some(macro_call) = root_macro_call_first_node(cx, item) { + if !matches!( + &*cx.tcx.item_name(macro_call.def_id).as_str(), + "impl_lint_pass" | "declare_lint_pass" + ) { + return; + } if let hir::ItemKind::Impl(hir::Impl { of_trait: None, items: impl_item_refs, diff --git a/clippy_utils/Cargo.toml b/clippy_utils/Cargo.toml index 0ba0b59ed13d..af1e2eb4dd11 100644 --- a/clippy_utils/Cargo.toml +++ b/clippy_utils/Cargo.toml @@ -5,6 +5,7 @@ edition = "2021" publish = false [dependencies] +arrayvec = { version = "0.7", default-features = false } if_chain = "1.0" rustc-semver = "1.1" diff --git a/clippy_utils/src/ast_utils.rs b/clippy_utils/src/ast_utils.rs index 8400bfbbd99d..3d3180521ab7 100644 --- a/clippy_utils/src/ast_utils.rs +++ b/clippy_utils/src/ast_utils.rs @@ -5,7 +5,6 @@ #![allow(clippy::similar_names, clippy::wildcard_imports, clippy::enum_glob_use)] use crate::{both, over}; -use if_chain::if_chain; use rustc_ast::ptr::P; use rustc_ast::{self as ast, *}; use rustc_span::symbol::Ident; @@ -679,34 +678,3 @@ pub fn eq_mac_args(l: &MacArgs, r: &MacArgs) -> bool { _ => false, } } - -/// Extract args from an assert-like macro. -/// -/// Currently working with: -/// - `assert_eq!` and `assert_ne!` -/// - `debug_assert_eq!` and `debug_assert_ne!` -/// -/// For example: -/// -/// `debug_assert_eq!(a, b)` will return Some([a, b]) -pub fn extract_assert_macro_args(mut expr: &Expr) -> Option<[&Expr; 2]> { - if_chain! { - if let ExprKind::If(_, ref block, _) = expr.kind; - if let StmtKind::Semi(ref e) = block.stmts.get(0)?.kind; - then { - expr = e; - } - } - if_chain! { - if let ExprKind::Block(ref block, _) = expr.kind; - if let StmtKind::Expr(ref expr) = block.stmts.get(0)?.kind; - if let ExprKind::Match(ref match_expr, _) = expr.kind; - if let ExprKind::Tup(ref tup) = match_expr.kind; - if let [a, b, ..] = tup.as_slice(); - if let (&ExprKind::AddrOf(_, _, ref a), &ExprKind::AddrOf(_, _, ref b)) = (&a.kind, &b.kind); - then { - return Some([&*a, &*b]); - } - } - None -} diff --git a/clippy_utils/src/higher.rs b/clippy_utils/src/higher.rs index c764c35d444f..160a51740cd7 100644 --- a/clippy_utils/src/higher.rs +++ b/clippy_utils/src/higher.rs @@ -3,15 +3,13 @@ #![deny(clippy::missing_docs_in_private_items)] use crate::ty::is_type_diagnostic_item; -use crate::{is_expn_of, last_path_segment, match_def_path, paths}; +use crate::{is_expn_of, match_def_path, paths}; use if_chain::if_chain; use rustc_ast::ast::{self, LitKind}; use rustc_hir as hir; -use rustc_hir::{ - Arm, Block, BorrowKind, Expr, ExprKind, HirId, LoopSource, MatchSource, Node, Pat, QPath, StmtKind, UnOp, -}; +use rustc_hir::{Arm, Block, Expr, ExprKind, HirId, LoopSource, MatchSource, Node, Pat, QPath}; use rustc_lint::LateContext; -use rustc_span::{sym, symbol, ExpnKind, Span, Symbol}; +use rustc_span::{sym, symbol, Span}; /// The essential nodes of a desugared for loop as well as the entire span: /// `for pat in arg { body }` becomes `(pat, arg, body)`. Return `(pat, arg, body, span)`. @@ -428,293 +426,6 @@ pub fn binop(op: hir::BinOpKind) -> ast::BinOpKind { } } -/// Extract args from an assert-like macro. -/// Currently working with: -/// - `assert!`, `assert_eq!` and `assert_ne!` -/// - `debug_assert!`, `debug_assert_eq!` and `debug_assert_ne!` -/// For example: -/// `assert!(expr)` will return `Some([expr])` -/// `debug_assert_eq!(a, b)` will return `Some([a, b])` -pub fn extract_assert_macro_args<'tcx>(e: &'tcx Expr<'tcx>) -> Option>> { - /// Try to match the AST for a pattern that contains a match, for example when two args are - /// compared - fn ast_matchblock(matchblock_expr: &'tcx Expr<'tcx>) -> Option>> { - if_chain! { - if let ExprKind::Match(headerexpr, _, _) = &matchblock_expr.kind; - if let ExprKind::Tup([lhs, rhs]) = &headerexpr.kind; - if let ExprKind::AddrOf(BorrowKind::Ref, _, lhs) = lhs.kind; - if let ExprKind::AddrOf(BorrowKind::Ref, _, rhs) = rhs.kind; - then { - return Some(vec![lhs, rhs]); - } - } - None - } - - if let ExprKind::Block(block, _) = e.kind { - if block.stmts.len() == 1 { - if let StmtKind::Semi(matchexpr) = block.stmts.get(0)?.kind { - // macros with unique arg: `{debug_}assert!` (e.g., `debug_assert!(some_condition)`) - if_chain! { - if let Some(If { cond, .. }) = If::hir(matchexpr); - if let ExprKind::Unary(UnOp::Not, condition) = cond.kind; - then { - return Some(vec![condition]); - } - } - - // debug macros with two args: `debug_assert_{ne, eq}` (e.g., `assert_ne!(a, b)`) - if_chain! { - if let ExprKind::Block(matchblock,_) = matchexpr.kind; - if let Some(matchblock_expr) = matchblock.expr; - then { - return ast_matchblock(matchblock_expr); - } - } - } - } else if let Some(matchblock_expr) = block.expr { - // macros with two args: `assert_{ne, eq}` (e.g., `assert_ne!(a, b)`) - return ast_matchblock(matchblock_expr); - } - } - None -} - -/// A parsed `format!` expansion -pub struct FormatExpn<'tcx> { - /// Span of `format!(..)` - pub call_site: Span, - /// Inner `format_args!` expansion - pub format_args: FormatArgsExpn<'tcx>, -} - -impl FormatExpn<'tcx> { - /// Parses an expanded `format!` invocation - pub fn parse(expr: &'tcx Expr<'tcx>) -> Option { - if_chain! { - if let ExprKind::Block(block, _) = expr.kind; - if let [stmt] = block.stmts; - if let StmtKind::Local(local) = stmt.kind; - if let Some(init) = local.init; - if let ExprKind::Call(_, [format_args]) = init.kind; - let expn_data = expr.span.ctxt().outer_expn_data(); - if let ExpnKind::Macro(_, sym::format) = expn_data.kind; - if let Some(format_args) = FormatArgsExpn::parse(format_args); - then { - Some(FormatExpn { - call_site: expn_data.call_site, - format_args, - }) - } else { - None - } - } - } -} - -/// A parsed `format_args!` expansion -pub struct FormatArgsExpn<'tcx> { - /// Span of the first argument, the format string - pub format_string_span: Span, - /// Values passed after the format string - pub value_args: Vec<&'tcx Expr<'tcx>>, - - /// String literal expressions which represent the format string split by "{}" - pub format_string_parts: &'tcx [Expr<'tcx>], - /// Symbols corresponding to [`Self::format_string_parts`] - pub format_string_symbols: Vec, - /// Expressions like `ArgumentV1::new(arg0, Debug::fmt)` - pub args: &'tcx [Expr<'tcx>], - /// The final argument passed to `Arguments::new_v1_formatted`, if applicable - pub fmt_expr: Option<&'tcx Expr<'tcx>>, -} - -impl FormatArgsExpn<'tcx> { - /// Parses an expanded `format_args!` or `format_args_nl!` invocation - pub fn parse(expr: &'tcx Expr<'tcx>) -> Option { - if_chain! { - if let ExpnKind::Macro(_, name) = expr.span.ctxt().outer_expn_data().kind; - let name = name.as_str(); - if name.ends_with("format_args") || name.ends_with("format_args_nl"); - if let ExprKind::Call(_, args) = expr.kind; - if let Some((strs_ref, args, fmt_expr)) = match args { - // Arguments::new_v1 - [strs_ref, args] => Some((strs_ref, args, None)), - // Arguments::new_v1_formatted - [strs_ref, args, fmt_expr, _unsafe_arg] => Some((strs_ref, args, Some(fmt_expr))), - _ => None, - }; - if let ExprKind::AddrOf(BorrowKind::Ref, _, strs_arr) = strs_ref.kind; - if let ExprKind::Array(format_string_parts) = strs_arr.kind; - if let Some(format_string_symbols) = format_string_parts - .iter() - .map(|e| { - if let ExprKind::Lit(lit) = &e.kind { - if let LitKind::Str(symbol, _style) = lit.node { - return Some(symbol); - } - } - None - }) - .collect(); - if let ExprKind::AddrOf(BorrowKind::Ref, _, args) = args.kind; - if let ExprKind::Match(args, [arm], _) = args.kind; - if let ExprKind::Tup(value_args) = args.kind; - if let Some(value_args) = value_args - .iter() - .map(|e| match e.kind { - ExprKind::AddrOf(_, _, e) => Some(e), - _ => None, - }) - .collect(); - if let ExprKind::Array(args) = arm.body.kind; - then { - Some(FormatArgsExpn { - format_string_span: strs_ref.span, - value_args, - format_string_parts, - format_string_symbols, - args, - fmt_expr, - }) - } else { - None - } - } - } - - /// Returns a vector of `FormatArgsArg`. - pub fn args(&self) -> Option>> { - if let Some(expr) = self.fmt_expr { - if_chain! { - if let ExprKind::AddrOf(BorrowKind::Ref, _, expr) = expr.kind; - if let ExprKind::Array(exprs) = expr.kind; - then { - exprs.iter().map(|fmt| { - if_chain! { - // struct `core::fmt::rt::v1::Argument` - if let ExprKind::Struct(_, fields, _) = fmt.kind; - if let Some(position_field) = fields.iter().find(|f| f.ident.name == sym::position); - if let ExprKind::Lit(lit) = &position_field.expr.kind; - if let LitKind::Int(position, _) = lit.node; - if let Ok(i) = usize::try_from(position); - let arg = &self.args[i]; - if let ExprKind::Call(_, [arg_name, _]) = arg.kind; - if let ExprKind::Field(_, j) = arg_name.kind; - if let Ok(j) = j.name.as_str().parse::(); - then { - Some(FormatArgsArg { value: self.value_args[j], arg, fmt: Some(fmt) }) - } else { - None - } - } - }).collect() - } else { - None - } - } - } else { - Some( - self.value_args - .iter() - .zip(self.args.iter()) - .map(|(value, arg)| FormatArgsArg { value, arg, fmt: None }) - .collect(), - ) - } - } -} - -/// Type representing a `FormatArgsExpn`'s format arguments -pub struct FormatArgsArg<'tcx> { - /// An element of `value_args` according to `position` - pub value: &'tcx Expr<'tcx>, - /// An element of `args` according to `position` - pub arg: &'tcx Expr<'tcx>, - /// An element of `fmt_expn` - pub fmt: Option<&'tcx Expr<'tcx>>, -} - -impl<'tcx> FormatArgsArg<'tcx> { - /// Returns true if any formatting parameters are used that would have an effect on strings, - /// like `{:+2}` instead of just `{}`. - pub fn has_string_formatting(&self) -> bool { - self.fmt.map_or(false, |fmt| { - // `!` because these conditions check that `self` is unformatted. - !if_chain! { - // struct `core::fmt::rt::v1::Argument` - if let ExprKind::Struct(_, fields, _) = fmt.kind; - if let Some(format_field) = fields.iter().find(|f| f.ident.name == sym::format); - // struct `core::fmt::rt::v1::FormatSpec` - if let ExprKind::Struct(_, subfields, _) = format_field.expr.kind; - let mut precision_found = false; - let mut width_found = false; - if subfields.iter().all(|field| { - match field.ident.name { - sym::precision => { - precision_found = true; - if let ExprKind::Path(ref precision_path) = field.expr.kind { - last_path_segment(precision_path).ident.name == sym::Implied - } else { - false - } - } - sym::width => { - width_found = true; - if let ExprKind::Path(ref width_qpath) = field.expr.kind { - last_path_segment(width_qpath).ident.name == sym::Implied - } else { - false - } - } - _ => true, - } - }); - if precision_found && width_found; - then { true } else { false } - } - }) - } - - /// Returns true if the argument is formatted using `Display::fmt`. - pub fn is_display(&self) -> bool { - if_chain! { - if let ExprKind::Call(_, [_, format_field]) = self.arg.kind; - if let ExprKind::Path(QPath::Resolved(_, path)) = format_field.kind; - if let [.., t, _] = path.segments; - if t.ident.name == sym::Display; - then { true } else { false } - } - } -} - -/// A parsed `panic!` expansion -pub struct PanicExpn<'tcx> { - /// Span of `panic!(..)` - pub call_site: Span, - /// Inner `format_args!` expansion - pub format_args: FormatArgsExpn<'tcx>, -} - -impl PanicExpn<'tcx> { - /// Parses an expanded `panic!` invocation - pub fn parse(expr: &'tcx Expr<'tcx>) -> Option { - if_chain! { - if let ExprKind::Call(_, [format_args]) = expr.kind; - let expn_data = expr.span.ctxt().outer_expn_data(); - if let Some(format_args) = FormatArgsExpn::parse(format_args); - then { - Some(PanicExpn { - call_site: expn_data.call_site, - format_args, - }) - } else { - None - } - } - } -} - /// A parsed `Vec` initialization expression #[derive(Clone, Copy)] pub enum VecInitKind { diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 54d470ca7382..bbb70e3adb5f 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -44,6 +44,7 @@ pub mod diagnostics; pub mod eager_or_lazy; pub mod higher; mod hir_utils; +pub mod macros; pub mod msrvs; pub mod numeric_literal; pub mod paths; @@ -1138,19 +1139,6 @@ pub fn contains_return(expr: &hir::Expr<'_>) -> bool { found } -/// Finds calls of the specified macros in a function body. -pub fn find_macro_calls(names: &[&str], body: &Body<'_>) -> Vec { - let mut result = Vec::new(); - expr_visitor_no_bodies(|expr| { - if names.iter().any(|fun| is_expn_of(expr.span, fun).is_some()) { - result.push(expr.span); - } - true - }) - .visit_expr(&body.value); - result -} - /// Extends the span to the beginning of the spans line, incl. whitespaces. /// /// ```rust @@ -1679,32 +1667,6 @@ pub fn match_libc_symbol(cx: &LateContext<'_>, did: DefId, name: &str) -> bool { path.first().map_or(false, |s| s.as_str() == "libc") && path.last().map_or(false, |s| s.as_str() == name) } -pub fn match_panic_call(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> { - if let ExprKind::Call(func, [arg]) = expr.kind { - expr_path_res(cx, func) - .opt_def_id() - .map_or(false, |id| match_panic_def_id(cx, id)) - .then(|| arg) - } else { - None - } -} - -pub fn match_panic_def_id(cx: &LateContext<'_>, did: DefId) -> bool { - match_any_def_paths( - cx, - did, - &[ - &paths::BEGIN_PANIC, - &paths::PANIC_ANY, - &paths::PANICKING_PANIC, - &paths::PANICKING_PANIC_FMT, - &paths::PANICKING_PANIC_STR, - ], - ) - .is_some() -} - /// Returns the list of condition expressions and the list of blocks in a /// sequence of `if/else`. /// E.g., this returns `([a, b], [c, d, e])` for the expression diff --git a/clippy_utils/src/macros.rs b/clippy_utils/src/macros.rs new file mode 100644 index 000000000000..2bf43aeb995a --- /dev/null +++ b/clippy_utils/src/macros.rs @@ -0,0 +1,539 @@ +#![allow(clippy::similar_names)] // `expr` and `expn` + +use crate::visitors::expr_visitor_no_bodies; + +use arrayvec::ArrayVec; +use if_chain::if_chain; +use rustc_ast::ast::LitKind; +use rustc_hir::intravisit::Visitor; +use rustc_hir::{self as hir, Expr, ExprKind, HirId, Node, QPath}; +use rustc_lint::LateContext; +use rustc_span::def_id::DefId; +use rustc_span::hygiene::{MacroKind, SyntaxContext}; +use rustc_span::{sym, ExpnData, ExpnId, ExpnKind, Span, Symbol}; +use std::ops::ControlFlow; + +/// A macro call, like `vec![1, 2, 3]`. +/// +/// Use `tcx.item_name(macro_call.def_id)` to get the macro name. +/// Even better is to check if it is a diagnostic item. +/// +/// This structure is similar to `ExpnData` but it precludes desugaring expansions. +#[derive(Debug)] +pub struct MacroCall { + /// Macro `DefId` + pub def_id: DefId, + /// Kind of macro + pub kind: MacroKind, + /// The expansion produced by the macro call + pub expn: ExpnId, + /// Span of the macro call site + pub span: Span, +} + +impl MacroCall { + pub fn is_local(&self) -> bool { + span_is_local(self.span) + } +} + +/// Returns an iterator of expansions that created the given span +pub fn expn_backtrace(mut span: Span) -> impl Iterator { + std::iter::from_fn(move || { + let ctxt = span.ctxt(); + if ctxt == SyntaxContext::root() { + return None; + } + let expn = ctxt.outer_expn(); + let data = expn.expn_data(); + span = data.call_site; + Some((expn, data)) + }) +} + +/// Checks whether the span is from the root expansion or a locally defined macro +pub fn span_is_local(span: Span) -> bool { + !span.from_expansion() || expn_is_local(span.ctxt().outer_expn()) +} + +/// Checks whether the expansion is the root expansion or a locally defined macro +pub fn expn_is_local(expn: ExpnId) -> bool { + if expn == ExpnId::root() { + return true; + } + let data = expn.expn_data(); + let backtrace = expn_backtrace(data.call_site); + std::iter::once((expn, data)) + .chain(backtrace) + .find_map(|(_, data)| data.macro_def_id) + .map_or(true, DefId::is_local) +} + +/// Returns an iterator of macro expansions that created the given span. +/// Note that desugaring expansions are skipped. +pub fn macro_backtrace(span: Span) -> impl Iterator { + expn_backtrace(span).filter_map(|(expn, data)| match data { + ExpnData { + kind: ExpnKind::Macro(kind, _), + macro_def_id: Some(def_id), + call_site: span, + .. + } => Some(MacroCall { + def_id, + kind, + expn, + span, + }), + _ => None, + }) +} + +/// If the macro backtrace of `span` has a macro call at the root expansion +/// (i.e. not a nested macro call), returns `Some` with the `MacroCall` +pub fn root_macro_call(span: Span) -> Option { + macro_backtrace(span).last() +} + +/// Like [`root_macro_call`], but only returns `Some` if `node` is the "first node" +/// produced by the macro call, as in [`first_node_in_macro`]. +pub fn root_macro_call_first_node(cx: &LateContext<'_>, node: &impl HirNode) -> Option { + if first_node_in_macro(cx, node) != Some(ExpnId::root()) { + return None; + } + root_macro_call(node.span()) +} + +/// Like [`macro_backtrace`], but only returns macro calls where `node` is the "first node" of the +/// macro call, as in [`first_node_in_macro`]. +pub fn first_node_macro_backtrace(cx: &LateContext<'_>, node: &impl HirNode) -> impl Iterator { + let span = node.span(); + first_node_in_macro(cx, node) + .into_iter() + .flat_map(move |expn| macro_backtrace(span).take_while(move |macro_call| macro_call.expn != expn)) +} + +/// If `node` is the "first node" in a macro expansion, returns `Some` with the `ExpnId` of the +/// macro call site (i.e. the parent of the macro expansion). This generally means that `node` +/// is the outermost node of an entire macro expansion, but there are some caveats noted below. +/// This is useful for finding macro calls while visiting the HIR without processing the macro call +/// at every node within its expansion. +/// +/// If you already have immediate access to the parent node, it is simpler to +/// just check the context of that span directly (e.g. `parent.span.from_expansion()`). +/// +/// If a macro call is in statement position, it expands to one or more statements. +/// In that case, each statement *and* their immediate descendants will all yield `Some` +/// with the `ExpnId` of the containing block. +/// +/// A node may be the "first node" of multiple macro calls in a macro backtrace. +/// The expansion of the outermost macro call site is returned in such cases. +pub fn first_node_in_macro(cx: &LateContext<'_>, node: &impl HirNode) -> Option { + // get the macro expansion or return `None` if not found + // `macro_backtrace` importantly ignores desugaring expansions + let expn = macro_backtrace(node.span()).next()?.expn; + + // get the parent node, possibly skipping over a statement + // if the parent is not found, it is sensible to return `Some(root)` + let hir = cx.tcx.hir(); + let mut parent_iter = hir.parent_iter(node.hir_id()); + let (parent_id, _) = match parent_iter.next() { + None => return Some(ExpnId::root()), + Some((_, Node::Stmt(_))) => match parent_iter.next() { + None => return Some(ExpnId::root()), + Some(next) => next, + }, + Some(next) => next, + }; + + // get the macro expansion of the parent node + let parent_span = hir.span(parent_id); + let Some(parent_macro_call) = macro_backtrace(parent_span).next() else { + // the parent node is not in a macro + return Some(ExpnId::root()); + }; + + if parent_macro_call.expn.is_descendant_of(expn) { + // `node` is input to a macro call + return None; + } + + Some(parent_macro_call.expn) +} + +/* Specific Macro Utils */ + +/// Is `def_id` of `std::panic`, `core::panic` or any inner implementation macros +pub fn is_panic(cx: &LateContext<'_>, def_id: DefId) -> bool { + let Some(name) = cx.tcx.get_diagnostic_name(def_id) else { return false }; + matches!( + name.as_str(), + "core_panic_macro" + | "std_panic_macro" + | "core_panic_2015_macro" + | "std_panic_2015_macro" + | "core_panic_2021_macro" + ) +} + +pub enum PanicExpn<'a> { + /// No arguments - `panic!()` + Empty, + /// A string literal or any `&str` - `panic!("message")` or `panic!(message)` + Str(&'a Expr<'a>), + /// A single argument that implements `Display` - `panic!("{}", object)` + Display(&'a Expr<'a>), + /// Anything else - `panic!("error {}: {}", a, b)` + Format(FormatArgsExpn<'a>), +} + +impl<'a> PanicExpn<'a> { + pub fn parse(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option { + if !macro_backtrace(expr.span).any(|macro_call| is_panic(cx, macro_call.def_id)) { + return None; + } + let ExprKind::Call(callee, [arg]) = expr.kind else { return None }; + let ExprKind::Path(QPath::Resolved(_, path)) = callee.kind else { return None }; + let result = match path.segments.last().unwrap().ident.as_str() { + "panic" if arg.span.ctxt() == expr.span.ctxt() => Self::Empty, + "panic" | "panic_str" => Self::Str(arg), + "panic_display" => { + let ExprKind::AddrOf(_, _, e) = arg.kind else { return None }; + Self::Display(e) + }, + "panic_fmt" => Self::Format(FormatArgsExpn::parse(cx, arg)?), + _ => return None, + }; + Some(result) + } +} + +/// Finds the arguments of an `assert!` or `debug_assert!` macro call within the macro expansion +pub fn find_assert_args<'a>( + cx: &LateContext<'_>, + expr: &'a Expr<'a>, + expn: ExpnId, +) -> Option<(&'a Expr<'a>, PanicExpn<'a>)> { + find_assert_args_inner(cx, expr, expn).map(|([e], p)| (e, p)) +} + +/// Finds the arguments of an `assert_eq!` or `debug_assert_eq!` macro call within the macro +/// expansion +pub fn find_assert_eq_args<'a>( + cx: &LateContext<'_>, + expr: &'a Expr<'a>, + expn: ExpnId, +) -> Option<(&'a Expr<'a>, &'a Expr<'a>, PanicExpn<'a>)> { + find_assert_args_inner(cx, expr, expn).map(|([a, b], p)| (a, b, p)) +} + +fn find_assert_args_inner<'a, const N: usize>( + cx: &LateContext<'_>, + expr: &'a Expr<'a>, + expn: ExpnId, +) -> Option<([&'a Expr<'a>; N], PanicExpn<'a>)> { + let macro_id = expn.expn_data().macro_def_id?; + let (expr, expn) = match cx.tcx.item_name(macro_id).as_str().strip_prefix("debug_") { + None => (expr, expn), + Some(inner_name) => find_assert_within_debug_assert(cx, expr, expn, Symbol::intern(inner_name))?, + }; + let mut args = ArrayVec::new(); + let mut panic_expn = None; + expr_visitor_no_bodies(|e| { + if args.is_full() { + if panic_expn.is_none() && e.span.ctxt() != expr.span.ctxt() { + panic_expn = PanicExpn::parse(cx, e); + } + panic_expn.is_none() + } else if is_assert_arg(cx, e, expn) { + args.push(e); + false + } else { + true + } + }) + .visit_expr(expr); + let args = args.into_inner().ok()?; + // if no `panic!(..)` is found, use `PanicExpn::Empty` + // to indicate that the default assertion message is used + let panic_expn = panic_expn.unwrap_or(PanicExpn::Empty); + Some((args, panic_expn)) +} + +fn find_assert_within_debug_assert<'a>( + cx: &LateContext<'_>, + expr: &'a Expr<'a>, + expn: ExpnId, + assert_name: Symbol, +) -> Option<(&'a Expr<'a>, ExpnId)> { + let mut found = None; + expr_visitor_no_bodies(|e| { + if found.is_some() || !e.span.from_expansion() { + return false; + } + let e_expn = e.span.ctxt().outer_expn(); + if e_expn == expn { + return true; + } + if e_expn.expn_data().macro_def_id.map(|id| cx.tcx.item_name(id)) == Some(assert_name) { + found = Some((e, e_expn)); + } + false + }) + .visit_expr(expr); + found +} + +fn is_assert_arg(cx: &LateContext<'_>, expr: &'a Expr<'a>, assert_expn: ExpnId) -> bool { + if !expr.span.from_expansion() { + return true; + } + let result = macro_backtrace(expr.span).try_for_each(|macro_call| { + if macro_call.expn == assert_expn { + ControlFlow::Break(false) + } else { + match cx.tcx.item_name(macro_call.def_id) { + // `cfg!(debug_assertions)` in `debug_assert!` + sym::cfg => ControlFlow::CONTINUE, + // assert!(other_macro!(..)) + _ => ControlFlow::Break(true), + } + } + }); + match result { + ControlFlow::Break(is_assert_arg) => is_assert_arg, + ControlFlow::Continue(()) => true, + } +} + +/// A parsed `format_args!` expansion +pub struct FormatArgsExpn<'tcx> { + /// Span of the first argument, the format string + pub format_string_span: Span, + /// The format string split by formatted args like `{..}` + pub format_string_parts: Vec, + /// Values passed after the format string + pub value_args: Vec<&'tcx Expr<'tcx>>, + /// Each element is a `value_args` index and a formatting trait (e.g. `sym::Debug`) + pub formatters: Vec<(usize, Symbol)>, + /// List of `fmt::v1::Argument { .. }` expressions. If this is empty, + /// then `formatters` represents the format args (`{..}`). + /// If this is non-empty, it represents the format args, and the `position` + /// parameters within the struct expressions are indexes of `formatters`. + pub specs: Vec<&'tcx Expr<'tcx>>, +} + +impl FormatArgsExpn<'tcx> { + /// Parses an expanded `format_args!` or `format_args_nl!` invocation + pub fn parse(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Option { + macro_backtrace(expr.span).find(|macro_call| { + matches!( + cx.tcx.item_name(macro_call.def_id), + sym::const_format_args | sym::format_args | sym::format_args_nl + ) + })?; + let mut format_string_span: Option = None; + let mut format_string_parts: Vec = Vec::new(); + let mut value_args: Vec<&Expr<'_>> = Vec::new(); + let mut formatters: Vec<(usize, Symbol)> = Vec::new(); + let mut specs: Vec<&Expr<'_>> = Vec::new(); + expr_visitor_no_bodies(|e| { + // if we're still inside of the macro definition... + if e.span.ctxt() == expr.span.ctxt() { + // ArgumnetV1::new(, ::fmt) + if_chain! { + if let ExprKind::Call(callee, [val, fmt_path]) = e.kind; + if let ExprKind::Path(QPath::TypeRelative(ty, seg)) = callee.kind; + if seg.ident.name == sym::new; + if let hir::TyKind::Path(QPath::Resolved(_, path)) = ty.kind; + if path.segments.last().unwrap().ident.name == sym::ArgumentV1; + if let ExprKind::Path(QPath::Resolved(_, path)) = fmt_path.kind; + if let [.., fmt_trait, _fmt] = path.segments; + then { + let val_idx = if_chain! { + if val.span.ctxt() == expr.span.ctxt(); + if let ExprKind::Field(_, field) = val.kind; + if let Ok(idx) = field.name.as_str().parse(); + then { + // tuple index + idx + } else { + // assume the value expression is passed directly + formatters.len() + } + }; + formatters.push((val_idx, fmt_trait.ident.name)); + } + } + if let ExprKind::Struct(QPath::Resolved(_, path), ..) = e.kind { + if path.segments.last().unwrap().ident.name == sym::Argument { + specs.push(e); + } + } + // walk through the macro expansion + return true; + } + // assume that the first expr with a differing context represents + // (and has the span of) the format string + if format_string_span.is_none() { + format_string_span = Some(e.span); + let span = e.span; + // walk the expr and collect string literals which are format string parts + expr_visitor_no_bodies(|e| { + if e.span.ctxt() != span.ctxt() { + // defensive check, probably doesn't happen + return false; + } + if let ExprKind::Lit(lit) = &e.kind { + if let LitKind::Str(symbol, _s) = lit.node { + format_string_parts.push(symbol); + } + } + true + }) + .visit_expr(e); + } else { + // assume that any further exprs with a differing context are value args + value_args.push(e); + } + // don't walk anything not from the macro expansion (e.a. inputs) + false + }) + .visit_expr(expr); + Some(FormatArgsExpn { + format_string_span: format_string_span?, + format_string_parts, + value_args, + formatters, + specs, + }) + } + + /// Finds a nested call to `format_args!` within a `format!`-like macro call + pub fn find_nested(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, expn_id: ExpnId) -> Option { + let mut format_args = None; + expr_visitor_no_bodies(|e| { + if format_args.is_some() { + return false; + } + let e_ctxt = e.span.ctxt(); + if e_ctxt == expr.span.ctxt() { + return true; + } + if e_ctxt.outer_expn().is_descendant_of(expn_id) { + format_args = FormatArgsExpn::parse(cx, e); + } + false + }) + .visit_expr(expr); + format_args + } + + /// Returns a vector of `FormatArgsArg`. + pub fn args(&self) -> Option>> { + if self.specs.is_empty() { + let args = std::iter::zip(&self.value_args, &self.formatters) + .map(|(value, &(_, format_trait))| FormatArgsArg { + value, + format_trait, + spec: None, + }) + .collect(); + return Some(args); + } + self.specs + .iter() + .map(|spec| { + if_chain! { + // struct `core::fmt::rt::v1::Argument` + if let ExprKind::Struct(_, fields, _) = spec.kind; + if let Some(position_field) = fields.iter().find(|f| f.ident.name == sym::position); + if let ExprKind::Lit(lit) = &position_field.expr.kind; + if let LitKind::Int(position, _) = lit.node; + if let Ok(i) = usize::try_from(position); + if let Some(&(j, format_trait)) = self.formatters.get(i); + then { + Some(FormatArgsArg { value: self.value_args[j], format_trait, spec: Some(spec) }) + } else { + None + } + } + }) + .collect() + } + + /// Span of all inputs + pub fn inputs_span(&self) -> Span { + match *self.value_args { + [] => self.format_string_span, + [.., last] => self.format_string_span.to(last.span), + } + } +} + +/// Type representing a `FormatArgsExpn`'s format arguments +pub struct FormatArgsArg<'tcx> { + /// An element of `value_args` according to `position` + pub value: &'tcx Expr<'tcx>, + /// An element of `args` according to `position` + pub format_trait: Symbol, + /// An element of `specs` + pub spec: Option<&'tcx Expr<'tcx>>, +} + +impl<'tcx> FormatArgsArg<'tcx> { + /// Returns true if any formatting parameters are used that would have an effect on strings, + /// like `{:+2}` instead of just `{}`. + pub fn has_string_formatting(&self) -> bool { + self.spec.map_or(false, |spec| { + // `!` because these conditions check that `self` is unformatted. + !if_chain! { + // struct `core::fmt::rt::v1::Argument` + if let ExprKind::Struct(_, fields, _) = spec.kind; + if let Some(format_field) = fields.iter().find(|f| f.ident.name == sym::format); + // struct `core::fmt::rt::v1::FormatSpec` + if let ExprKind::Struct(_, subfields, _) = format_field.expr.kind; + if subfields.iter().all(|field| match field.ident.name { + sym::precision | sym::width => match field.expr.kind { + ExprKind::Path(QPath::Resolved(_, path)) => { + path.segments.last().unwrap().ident.name == sym::Implied + } + _ => false, + } + _ => true, + }); + then { true } else { false } + } + }) + } +} + +/// A node with a `HirId` and a `Span` +pub trait HirNode { + fn hir_id(&self) -> HirId; + fn span(&self) -> Span; +} + +macro_rules! impl_hir_node { + ($($t:ident),*) => { + $(impl HirNode for hir::$t<'_> { + fn hir_id(&self) -> HirId { + self.hir_id + } + fn span(&self) -> Span { + self.span + } + })* + }; +} + +impl_hir_node!(Expr, Pat); + +impl HirNode for hir::Item<'_> { + fn hir_id(&self) -> HirId { + self.hir_id() + } + + fn span(&self) -> Span { + self.span + } +} diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index aa3b3af23567..27db53a6e6d5 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -25,7 +25,6 @@ pub const ASSERT_MACRO: [&str; 4] = ["core", "macros", "builtin", "assert"]; pub const ASSERT_NE_MACRO: [&str; 3] = ["core", "macros", "assert_ne"]; pub const ASMUT_TRAIT: [&str; 3] = ["core", "convert", "AsMut"]; pub const ASREF_TRAIT: [&str; 3] = ["core", "convert", "AsRef"]; -pub(super) const BEGIN_PANIC: [&str; 3] = ["std", "panicking", "begin_panic"]; /// Preferably use the diagnostic item `sym::Borrow` where possible pub const BORROW_TRAIT: [&str; 3] = ["core", "borrow", "Borrow"]; pub const BORROW_MUT_TRAIT: [&str; 3] = ["core", "borrow", "BorrowMut"]; @@ -110,10 +109,6 @@ pub const OPTION_SOME: [&str; 4] = ["core", "option", "Option", "Some"]; pub const ORD: [&str; 3] = ["core", "cmp", "Ord"]; pub const OS_STRING_AS_OS_STR: [&str; 5] = ["std", "ffi", "os_str", "OsString", "as_os_str"]; pub const OS_STR_TO_OS_STRING: [&str; 5] = ["std", "ffi", "os_str", "OsStr", "to_os_string"]; -pub(super) const PANICKING_PANIC: [&str; 3] = ["core", "panicking", "panic"]; -pub(super) const PANICKING_PANIC_FMT: [&str; 3] = ["core", "panicking", "panic_fmt"]; -pub(super) const PANICKING_PANIC_STR: [&str; 3] = ["core", "panicking", "panic_str"]; -pub(super) const PANIC_ANY: [&str; 3] = ["std", "panic", "panic_any"]; pub const PARKING_LOT_RAWMUTEX: [&str; 3] = ["parking_lot", "raw_mutex", "RawMutex"]; pub const PARKING_LOT_RAWRWLOCK: [&str; 3] = ["parking_lot", "raw_rwlock", "RawRwLock"]; pub const PARKING_LOT_MUTEX_GUARD: [&str; 2] = ["parking_lot", "MutexGuard"]; diff --git a/tests/ui/assertions_on_constants.rs b/tests/ui/assertions_on_constants.rs index cb516d0f9778..7477c01ca782 100644 --- a/tests/ui/assertions_on_constants.rs +++ b/tests/ui/assertions_on_constants.rs @@ -1,4 +1,3 @@ -//FIXME: suggestions are wrongly expanded, this should be fixed along with #7843 #![allow(non_fmt_panics)] macro_rules! assert_const { diff --git a/tests/ui/assertions_on_constants.stderr b/tests/ui/assertions_on_constants.stderr index ec80ec702fb5..e1f818814d50 100644 --- a/tests/ui/assertions_on_constants.stderr +++ b/tests/ui/assertions_on_constants.stderr @@ -1,75 +1,75 @@ error: `assert!(true)` will be optimized out by the compiler - --> $DIR/assertions_on_constants.rs:11:5 + --> $DIR/assertions_on_constants.rs:10:5 | LL | assert!(true); | ^^^^^^^^^^^^^ | = note: `-D clippy::assertions-on-constants` implied by `-D warnings` = help: remove it - = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info) error: `assert!(false)` should probably be replaced - --> $DIR/assertions_on_constants.rs:12:5 + --> $DIR/assertions_on_constants.rs:11:5 | LL | assert!(false); | ^^^^^^^^^^^^^^ | = help: use `panic!()` or `unreachable!()` - = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info) error: `assert!(true)` will be optimized out by the compiler - --> $DIR/assertions_on_constants.rs:13:5 + --> $DIR/assertions_on_constants.rs:12:5 | LL | assert!(true, "true message"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: remove it - = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info) -error: `assert!(false, $crate::const_format_args!($($t)+))` should probably be replaced - --> $DIR/assertions_on_constants.rs:14:5 +error: `assert!(false, ..)` should probably be replaced + --> $DIR/assertions_on_constants.rs:13:5 | LL | assert!(false, "false message"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: use `panic!($crate::const_format_args!($($t)+))` or `unreachable!($crate::const_format_args!($($t)+))` - = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info) + = help: use `panic!(..)` or `unreachable!(..)` + +error: `assert!(false, ..)` should probably be replaced + --> $DIR/assertions_on_constants.rs:16:5 + | +LL | assert!(false, "{}", msg.to_uppercase()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: use `panic!(..)` or `unreachable!(..)` error: `assert!(true)` will be optimized out by the compiler - --> $DIR/assertions_on_constants.rs:20:5 + --> $DIR/assertions_on_constants.rs:19:5 | LL | assert!(B); | ^^^^^^^^^^ | = help: remove it - = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info) error: `assert!(false)` should probably be replaced - --> $DIR/assertions_on_constants.rs:23:5 + --> $DIR/assertions_on_constants.rs:22:5 | LL | assert!(C); | ^^^^^^^^^^ | = help: use `panic!()` or `unreachable!()` - = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info) -error: `assert!(false, $crate::const_format_args!($($t)+))` should probably be replaced - --> $DIR/assertions_on_constants.rs:24:5 +error: `assert!(false, ..)` should probably be replaced + --> $DIR/assertions_on_constants.rs:23:5 | LL | assert!(C, "C message"); | ^^^^^^^^^^^^^^^^^^^^^^^ | - = help: use `panic!($crate::const_format_args!($($t)+))` or `unreachable!($crate::const_format_args!($($t)+))` - = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info) + = help: use `panic!(..)` or `unreachable!(..)` error: `debug_assert!(true)` will be optimized out by the compiler - --> $DIR/assertions_on_constants.rs:26:5 + --> $DIR/assertions_on_constants.rs:25:5 | LL | debug_assert!(true); | ^^^^^^^^^^^^^^^^^^^ | = help: remove it - = note: this error originates in the macro `$crate::assert` (in Nightly builds, run with -Z macro-backtrace for more info) -error: aborting due to 8 previous errors +error: aborting due to 9 previous errors diff --git a/tests/ui/eq_op_macros.stderr b/tests/ui/eq_op_macros.stderr index 885415b42c78..cd9f1826e59b 100644 --- a/tests/ui/eq_op_macros.stderr +++ b/tests/ui/eq_op_macros.stderr @@ -21,6 +21,28 @@ LL | assert_in_macro_def!(); | = note: this error originates in the macro `assert_in_macro_def` (in Nightly builds, run with -Z macro-backtrace for more info) +error: identical args used in this `debug_assert_eq!` macro call + --> $DIR/eq_op_macros.rs:9:26 + | +LL | debug_assert_eq!(a, a); + | ^^^^ +... +LL | assert_in_macro_def!(); + | ---------------------- in this macro invocation + | + = note: this error originates in the macro `assert_in_macro_def` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: identical args used in this `debug_assert_ne!` macro call + --> $DIR/eq_op_macros.rs:10:26 + | +LL | debug_assert_ne!(a, a); + | ^^^^ +... +LL | assert_in_macro_def!(); + | ---------------------- in this macro invocation + | + = note: this error originates in the macro `assert_in_macro_def` (in Nightly builds, run with -Z macro-backtrace for more info) + error: identical args used in this `assert_eq!` macro call --> $DIR/eq_op_macros.rs:22:16 | @@ -45,28 +67,6 @@ error: identical args used in this `assert_ne!` macro call LL | assert_ne!(a + 1, a + 1); | ^^^^^^^^^^^^ -error: identical args used in this `debug_assert_eq!` macro call - --> $DIR/eq_op_macros.rs:9:26 - | -LL | debug_assert_eq!(a, a); - | ^^^^ -... -LL | assert_in_macro_def!(); - | ---------------------- in this macro invocation - | - = note: this error originates in the macro `assert_in_macro_def` (in Nightly builds, run with -Z macro-backtrace for more info) - -error: identical args used in this `debug_assert_ne!` macro call - --> $DIR/eq_op_macros.rs:10:26 - | -LL | debug_assert_ne!(a, a); - | ^^^^ -... -LL | assert_in_macro_def!(); - | ---------------------- in this macro invocation - | - = note: this error originates in the macro `assert_in_macro_def` (in Nightly builds, run with -Z macro-backtrace for more info) - error: identical args used in this `debug_assert_eq!` macro call --> $DIR/eq_op_macros.rs:38:22 | diff --git a/tests/ui/missing_panics_doc.stderr b/tests/ui/missing_panics_doc.stderr index 8bccbaefe23c..91ebd695238b 100644 --- a/tests/ui/missing_panics_doc.stderr +++ b/tests/ui/missing_panics_doc.stderr @@ -27,7 +27,6 @@ note: first possible panic found here | LL | panic!("This function panics") | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in the macro `$crate::panic::panic_2021` (in Nightly builds, run with -Z macro-backtrace for more info) error: docs for function which may panic missing `# Panics` section --> $DIR/missing_panics_doc.rs:17:1 @@ -42,7 +41,6 @@ note: first possible panic found here | LL | todo!() | ^^^^^^^ - = note: this error originates in the macro `todo` (in Nightly builds, run with -Z macro-backtrace for more info) error: docs for function which may panic missing `# Panics` section --> $DIR/missing_panics_doc.rs:22:1 @@ -61,7 +59,6 @@ note: first possible panic found here | LL | panic!() | ^^^^^^^^ - = note: this error originates in the macro `$crate::panic::panic_2021` (in Nightly builds, run with -Z macro-backtrace for more info) error: docs for function which may panic missing `# Panics` section --> $DIR/missing_panics_doc.rs:31:1 @@ -76,7 +73,6 @@ note: first possible panic found here | LL | if true { unreachable!() } else { panic!() } | ^^^^^^^^ - = note: this error originates in the macro `$crate::panic::panic_2021` (in Nightly builds, run with -Z macro-backtrace for more info) error: docs for function which may panic missing `# Panics` section --> $DIR/missing_panics_doc.rs:36:1 @@ -92,7 +88,6 @@ note: first possible panic found here | LL | assert_eq!(x, 0); | ^^^^^^^^^^^^^^^^ - = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info) error: docs for function which may panic missing `# Panics` section --> $DIR/missing_panics_doc.rs:42:1 @@ -108,7 +103,6 @@ note: first possible panic found here | LL | assert_ne!(x, 0); | ^^^^^^^^^^^^^^^^ - = note: this error originates in the macro `assert_ne` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to 7 previous errors diff --git a/tests/ui/panic_in_result_fn.stderr b/tests/ui/panic_in_result_fn.stderr index 78d09b8b2108..561503ae54fa 100644 --- a/tests/ui/panic_in_result_fn.stderr +++ b/tests/ui/panic_in_result_fn.stderr @@ -14,7 +14,6 @@ note: return Err() instead of panicking | LL | panic!("error"); | ^^^^^^^^^^^^^^^ - = note: this error originates in the macro `$crate::panic::panic_2021` (in Nightly builds, run with -Z macro-backtrace for more info) error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:11:5 @@ -31,7 +30,6 @@ note: return Err() instead of panicking | LL | unimplemented!(); | ^^^^^^^^^^^^^^^^ - = note: this error originates in the macro `unimplemented` (in Nightly builds, run with -Z macro-backtrace for more info) error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:16:5 @@ -48,7 +46,6 @@ note: return Err() instead of panicking | LL | unreachable!(); | ^^^^^^^^^^^^^^ - = note: this error originates in the macro `unreachable` (in Nightly builds, run with -Z macro-backtrace for more info) error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:21:5 @@ -65,7 +62,6 @@ note: return Err() instead of panicking | LL | todo!("Finish this"); | ^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in the macro `todo` (in Nightly builds, run with -Z macro-backtrace for more info) error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:52:1 @@ -82,7 +78,6 @@ note: return Err() instead of panicking | LL | panic!("error"); | ^^^^^^^^^^^^^^^ - = note: this error originates in the macro `$crate::panic::panic_2021` (in Nightly builds, run with -Z macro-backtrace for more info) error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn.rs:67:1 @@ -99,7 +94,6 @@ note: return Err() instead of panicking | LL | todo!("finish main method"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in the macro `todo` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to 6 previous errors diff --git a/tests/ui/panic_in_result_fn_assertions.stderr b/tests/ui/panic_in_result_fn_assertions.stderr index 7501d6d85edd..b6aa005e7b52 100644 --- a/tests/ui/panic_in_result_fn_assertions.stderr +++ b/tests/ui/panic_in_result_fn_assertions.stderr @@ -15,7 +15,6 @@ note: return Err() instead of panicking | LL | assert!(x == 5, "wrong argument"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in the macro `assert` (in Nightly builds, run with -Z macro-backtrace for more info) error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn_assertions.rs:13:5 @@ -33,7 +32,6 @@ note: return Err() instead of panicking | LL | assert_eq!(x, 5); | ^^^^^^^^^^^^^^^^ - = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info) error: used `unimplemented!()`, `unreachable!()`, `todo!()`, `panic!()` or assertion in a function that returns `Result` --> $DIR/panic_in_result_fn_assertions.rs:19:5 @@ -51,7 +49,6 @@ note: return Err() instead of panicking | LL | assert_ne!(x, 1); | ^^^^^^^^^^^^^^^^ - = note: this error originates in the macro `assert_ne` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to 3 previous errors diff --git a/tests/ui/panicking_macros.stderr b/tests/ui/panicking_macros.stderr index 2b607ff58889..bfd1c7a38014 100644 --- a/tests/ui/panicking_macros.stderr +++ b/tests/ui/panicking_macros.stderr @@ -25,23 +25,18 @@ LL | todo!(); | ^^^^^^^ | = note: `-D clippy::todo` implied by `-D warnings` - = note: this error originates in the macro `todo` (in Nightly builds, run with -Z macro-backtrace for more info) error: `todo` should not be present in production code --> $DIR/panicking_macros.rs:17:5 | LL | todo!("message"); | ^^^^^^^^^^^^^^^^ - | - = note: this error originates in the macro `todo` (in Nightly builds, run with -Z macro-backtrace for more info) error: `todo` should not be present in production code --> $DIR/panicking_macros.rs:18:5 | LL | todo!("{} {}", "panic with", "multiple arguments"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: this error originates in the macro `todo` (in Nightly builds, run with -Z macro-backtrace for more info) error: `unimplemented` should not be present in production code --> $DIR/panicking_macros.rs:24:5 @@ -50,23 +45,18 @@ LL | unimplemented!(); | ^^^^^^^^^^^^^^^^ | = note: `-D clippy::unimplemented` implied by `-D warnings` - = note: this error originates in the macro `unimplemented` (in Nightly builds, run with -Z macro-backtrace for more info) error: `unimplemented` should not be present in production code --> $DIR/panicking_macros.rs:25:5 | LL | unimplemented!("message"); | ^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: this error originates in the macro `unimplemented` (in Nightly builds, run with -Z macro-backtrace for more info) error: `unimplemented` should not be present in production code --> $DIR/panicking_macros.rs:26:5 | LL | unimplemented!("{} {}", "panic with", "multiple arguments"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: this error originates in the macro `unimplemented` (in Nightly builds, run with -Z macro-backtrace for more info) error: usage of the `unreachable!` macro --> $DIR/panicking_macros.rs:32:5 @@ -75,23 +65,18 @@ LL | unreachable!(); | ^^^^^^^^^^^^^^ | = note: `-D clippy::unreachable` implied by `-D warnings` - = note: this error originates in the macro `unreachable` (in Nightly builds, run with -Z macro-backtrace for more info) error: usage of the `unreachable!` macro --> $DIR/panicking_macros.rs:33:5 | LL | unreachable!("message"); | ^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: this error originates in the macro `$crate::unreachable` (in Nightly builds, run with -Z macro-backtrace for more info) error: usage of the `unreachable!` macro --> $DIR/panicking_macros.rs:34:5 | LL | unreachable!("{} {}", "panic with", "multiple arguments"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: this error originates in the macro `unreachable` (in Nightly builds, run with -Z macro-backtrace for more info) error: `panic` should not be present in production code --> $DIR/panicking_macros.rs:40:5 @@ -104,24 +89,18 @@ error: `todo` should not be present in production code | LL | todo!(); | ^^^^^^^ - | - = note: this error originates in the macro `todo` (in Nightly builds, run with -Z macro-backtrace for more info) error: `unimplemented` should not be present in production code --> $DIR/panicking_macros.rs:42:5 | LL | unimplemented!(); | ^^^^^^^^^^^^^^^^ - | - = note: this error originates in the macro `unimplemented` (in Nightly builds, run with -Z macro-backtrace for more info) error: usage of the `unreachable!` macro --> $DIR/panicking_macros.rs:43:5 | LL | unreachable!(); | ^^^^^^^^^^^^^^ - | - = note: this error originates in the macro `unreachable` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to 16 previous errors diff --git a/tests/ui/unit_cmp.stderr b/tests/ui/unit_cmp.stderr index 2b5a7b348b98..824506a4257e 100644 --- a/tests/ui/unit_cmp.stderr +++ b/tests/ui/unit_cmp.stderr @@ -33,8 +33,6 @@ LL | | }, LL | | } LL | | ); | |_____^ - | - = note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info) error: `debug_assert_eq` of unit values detected. This will always succeed --> $DIR/unit_cmp.rs:32:5 @@ -47,8 +45,6 @@ LL | | }, LL | | } LL | | ); | |_____^ - | - = note: this error originates in the macro `$crate::assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info) error: `assert_ne` of unit values detected. This will always fail --> $DIR/unit_cmp.rs:41:5 @@ -61,8 +57,6 @@ LL | | }, LL | | } LL | | ); | |_____^ - | - = note: this error originates in the macro `assert_ne` (in Nightly builds, run with -Z macro-backtrace for more info) error: `debug_assert_ne` of unit values detected. This will always fail --> $DIR/unit_cmp.rs:49:5 @@ -75,8 +69,6 @@ LL | | }, LL | | } LL | | ); | |_____^ - | - = note: this error originates in the macro `$crate::assert_ne` (in Nightly builds, run with -Z macro-backtrace for more info) error: aborting due to 6 previous errors