diff options
Diffstat (limited to 'clippy_lints/src')
29 files changed, 504 insertions, 199 deletions
diff --git a/clippy_lints/src/arbitrary_source_item_ordering.rs b/clippy_lints/src/arbitrary_source_item_ordering.rs index 8b6bfaebbe5..c410a5da775 100644 --- a/clippy_lints/src/arbitrary_source_item_ordering.rs +++ b/clippy_lints/src/arbitrary_source_item_ordering.rs @@ -266,7 +266,7 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering { .tcx .hir_attrs(item.hir_id()) .iter() - .any(|attr| matches!(attr, Attribute::Parsed(AttributeKind::Repr{ .. }))) + .any(|attr| matches!(attr, Attribute::Parsed(AttributeKind::Repr { .. }))) { // Do not lint items with a `#[repr]` attribute as their layout may be imposed by an external API. return; diff --git a/clippy_lints/src/casts/borrow_as_ptr.rs b/clippy_lints/src/casts/borrow_as_ptr.rs index ad0a4f8cdf3..e3b125a8d5b 100644 --- a/clippy_lints/src/casts/borrow_as_ptr.rs +++ b/clippy_lints/src/casts/borrow_as_ptr.rs @@ -18,7 +18,8 @@ pub(super) fn check<'tcx>( cast_to: &'tcx Ty<'_>, msrv: Msrv, ) -> bool { - if matches!(cast_to.kind, TyKind::Ptr(_)) + if let TyKind::Ptr(target) = cast_to.kind + && !matches!(target.ty.kind, TyKind::TraitObject(..)) && let ExprKind::AddrOf(BorrowKind::Ref, mutability, e) = cast_expr.kind && !is_lint_allowed(cx, BORROW_AS_PTR, expr.hir_id) { diff --git a/clippy_lints/src/casts/cast_possible_truncation.rs b/clippy_lints/src/casts/cast_possible_truncation.rs index a2ecb5fb44a..2eebe849232 100644 --- a/clippy_lints/src/casts/cast_possible_truncation.rs +++ b/clippy_lints/src/casts/cast_possible_truncation.rs @@ -3,7 +3,7 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; use clippy_utils::source::snippet; use clippy_utils::sugg::Sugg; use clippy_utils::ty::{get_discriminant_value, is_isize_or_usize}; -use clippy_utils::{expr_or_init, sym}; +use clippy_utils::{expr_or_init, is_in_const_context, sym}; use rustc_abi::IntegerType; use rustc_errors::{Applicability, Diag}; use rustc_hir::def::{DefKind, Res}; @@ -168,7 +168,9 @@ pub(super) fn check( span_lint_and_then(cx, CAST_POSSIBLE_TRUNCATION, expr.span, msg, |diag| { diag.help("if this is intentional allow the lint with `#[allow(clippy::cast_possible_truncation)]` ..."); - if !cast_from.is_floating_point() { + // TODO: Remove the condition for const contexts when `try_from` and other commonly used methods + // become const fn. + if !is_in_const_context(cx) && !cast_from.is_floating_point() { offer_suggestion(cx, expr, cast_expr, cast_to_span, diag); } }); diff --git a/clippy_lints/src/casts/mod.rs b/clippy_lints/src/casts/mod.rs index daae9a8bb08..37accff5eaa 100644 --- a/clippy_lints/src/casts/mod.rs +++ b/clippy_lints/src/casts/mod.rs @@ -878,7 +878,7 @@ impl<'tcx> LateLintPass<'tcx> for Casts { confusing_method_to_numeric_cast::check(cx, expr, cast_from_expr, cast_from, cast_to); fn_to_numeric_cast::check(cx, expr, cast_from_expr, cast_from, cast_to); fn_to_numeric_cast_with_truncation::check(cx, expr, cast_from_expr, cast_from, cast_to); - zero_ptr::check(cx, expr, cast_from_expr, cast_to_hir); + zero_ptr::check(cx, expr, cast_from_expr, cast_to_hir, self.msrv); if self.msrv.meets(cx, msrvs::MANUAL_DANGLING_PTR) { manual_dangling_ptr::check(cx, expr, cast_from_expr, cast_to_hir); diff --git a/clippy_lints/src/casts/zero_ptr.rs b/clippy_lints/src/casts/zero_ptr.rs index a34af6bc226..f4738e7b0d5 100644 --- a/clippy_lints/src/casts/zero_ptr.rs +++ b/clippy_lints/src/casts/zero_ptr.rs @@ -1,4 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::source::SpanRangeExt; use clippy_utils::{is_in_const_context, is_integer_literal, std_or_core}; use rustc_errors::Applicability; @@ -7,10 +8,10 @@ use rustc_lint::LateContext; use super::ZERO_PTR; -pub fn check(cx: &LateContext<'_>, expr: &Expr<'_>, from: &Expr<'_>, to: &Ty<'_>) { +pub fn check(cx: &LateContext<'_>, expr: &Expr<'_>, from: &Expr<'_>, to: &Ty<'_>, msrv: Msrv) { if let TyKind::Ptr(ref mut_ty) = to.kind && is_integer_literal(from, 0) - && !is_in_const_context(cx) + && (!is_in_const_context(cx) || msrv.meets(cx, msrvs::PTR_NULL)) && let Some(std_or_core) = std_or_core(cx) { let (msg, sugg_fn) = match mut_ty.mutbl { diff --git a/clippy_lints/src/coerce_container_to_any.rs b/clippy_lints/src/coerce_container_to_any.rs index 2b659253763..6217fc4c897 100644 --- a/clippy_lints/src/coerce_container_to_any.rs +++ b/clippy_lints/src/coerce_container_to_any.rs @@ -1,9 +1,10 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::snippet; +use clippy_utils::sugg::{self, Sugg}; use clippy_utils::sym; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::adjustment::{Adjust, PointerCoercion}; use rustc_middle::ty::{self, ExistentialPredicate, Ty, TyCtxt}; use rustc_session::declare_lint_pass; @@ -49,23 +50,18 @@ declare_lint_pass!(CoerceContainerToAny => [COERCE_CONTAINER_TO_ANY]); impl<'tcx> LateLintPass<'tcx> for CoerceContainerToAny { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { - // If this expression has an effective type of `&dyn Any` ... - { - let coerced_ty = cx.typeck_results().expr_ty_adjusted(e); - - let ty::Ref(_, coerced_ref_ty, _) = *coerced_ty.kind() else { - return; - }; - if !is_dyn_any(cx.tcx, coerced_ref_ty) { - return; - } + // If this expression was coerced to `&dyn Any` ... + if !cx.typeck_results().expr_adjustments(e).last().is_some_and(|adj| { + matches!(adj.kind, Adjust::Pointer(PointerCoercion::Unsize)) && is_ref_dyn_any(cx.tcx, adj.target) + }) { + return; } let expr_ty = cx.typeck_results().expr_ty(e); let ty::Ref(_, expr_ref_ty, _) = *expr_ty.kind() else { return; }; - // ... but only due to coercion ... + // ... but it's not actually `&dyn Any` ... if is_dyn_any(cx.tcx, expr_ref_ty) { return; } @@ -78,23 +74,37 @@ impl<'tcx> LateLintPass<'tcx> for CoerceContainerToAny { } // ... that's probably not intended. - let (span, deref_count) = match e.kind { + let (target_expr, deref_count) = match e.kind { // If `e` was already an `&` expression, skip `*&` in the suggestion - ExprKind::AddrOf(_, _, referent) => (referent.span, depth), - _ => (e.span, depth + 1), + ExprKind::AddrOf(_, _, referent) => (referent, depth), + _ => (e, depth + 1), }; + let ty::Ref(_, _, mutability) = *cx.typeck_results().expr_ty_adjusted(e).kind() else { + return; + }; + let sugg = sugg::make_unop( + &format!("{}{}", mutability.ref_prefix_str(), str::repeat("*", deref_count)), + Sugg::hir(cx, target_expr, ".."), + ); span_lint_and_sugg( cx, COERCE_CONTAINER_TO_ANY, e.span, - format!("coercing `{expr_ty}` to `&dyn Any`"), + format!("coercing `{expr_ty}` to `{}dyn Any`", mutability.ref_prefix_str()), "consider dereferencing", - format!("&{}{}", str::repeat("*", deref_count), snippet(cx, span, "x")), + sugg.to_string(), Applicability::MaybeIncorrect, ); } } +fn is_ref_dyn_any(tcx: TyCtxt<'_>, ty: Ty<'_>) -> bool { + let ty::Ref(_, ref_ty, _) = *ty.kind() else { + return false; + }; + is_dyn_any(tcx, ref_ty) +} + fn is_dyn_any(tcx: TyCtxt<'_>, ty: Ty<'_>) -> bool { let ty::Dynamic(traits, ..) = ty.kind() else { return false; diff --git a/clippy_lints/src/cognitive_complexity.rs b/clippy_lints/src/cognitive_complexity.rs index 5c64216dd92..d5d937d9133 100644 --- a/clippy_lints/src/cognitive_complexity.rs +++ b/clippy_lints/src/cognitive_complexity.rs @@ -14,18 +14,25 @@ use rustc_span::def_id::LocalDefId; declare_clippy_lint! { /// ### What it does - /// Checks for methods with high cognitive complexity. + /// We used to think it measured how hard a method is to understand. /// /// ### Why is this bad? - /// Methods of high cognitive complexity tend to be hard to - /// both read and maintain. Also LLVM will tend to optimize small methods better. + /// Ideally, we would like to be able to measure how hard a function is + /// to understand given its context (what we call its Cognitive Complexity). + /// But that's not what this lint does. See "Known problems" /// /// ### Known problems - /// Sometimes it's hard to find a way to reduce the - /// complexity. + /// The true Cognitive Complexity of a method is not something we can + /// calculate using modern technology. This lint has been left in the + /// `nursery` so as to not mislead users into using this lint as a + /// measurement tool. /// - /// ### Example - /// You'll see it when you get the warning. + /// For more detailed information, see [rust-clippy#3793](https://github.com/rust-lang/rust-clippy/issues/3793) + /// + /// ### Lints to consider instead of this + /// + /// * [`excessive_nesting`](https://rust-lang.github.io/rust-clippy/master/index.html#excessive_nesting) + /// * [`too_many_lines`](https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines) #[clippy::version = "1.35.0"] pub COGNITIVE_COMPLEXITY, nursery, diff --git a/clippy_lints/src/disallowed_script_idents.rs b/clippy_lints/src/disallowed_script_idents.rs index d1a8590c59b..cf964d4b580 100644 --- a/clippy_lints/src/disallowed_script_idents.rs +++ b/clippy_lints/src/disallowed_script_idents.rs @@ -89,6 +89,10 @@ impl EarlyLintPass for DisallowedScriptIdents { // Fast path for ascii-only idents. if !symbol_str.is_ascii() && let Some(script) = symbol_str.chars().find_map(|c| { + if c.is_ascii() { + return None; + } + c.script_extension() .iter() .find(|script| !self.whitelist.contains(script)) diff --git a/clippy_lints/src/doc/missing_headers.rs b/clippy_lints/src/doc/missing_headers.rs index 9ee32fced8c..3033ac0d0b0 100644 --- a/clippy_lints/src/doc/missing_headers.rs +++ b/clippy_lints/src/doc/missing_headers.rs @@ -3,7 +3,7 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_note}; use clippy_utils::macros::{is_panic, root_macro_call_first_node}; use clippy_utils::ty::{get_type_diagnostic_name, implements_trait_with_env, is_type_diagnostic_item}; use clippy_utils::visitors::for_each_expr; -use clippy_utils::{fulfill_or_allowed, is_doc_hidden, method_chain_args, return_ty}; +use clippy_utils::{fulfill_or_allowed, is_doc_hidden, is_inside_always_const_context, method_chain_args, return_ty}; use rustc_hir::{BodyId, FnSig, OwnerId, Safety}; use rustc_lint::LateContext; use rustc_middle::ty; @@ -99,13 +99,16 @@ fn find_panic(cx: &LateContext<'_>, body_id: BodyId) -> Option<Span> { let mut panic_span = None; let typeck = cx.tcx.typeck_body(body_id); for_each_expr(cx, cx.tcx.hir_body(body_id), |expr| { + if is_inside_always_const_context(cx.tcx, expr.hir_id) { + return ControlFlow::<!>::Continue(()); + } + if let Some(macro_call) = root_macro_call_first_node(cx, expr) && (is_panic(cx, macro_call.def_id) || matches!( cx.tcx.get_diagnostic_name(macro_call.def_id), Some(sym::assert_macro | sym::assert_eq_macro | sym::assert_ne_macro) )) - && !cx.tcx.hir_is_inside_const_context(expr.hir_id) && !fulfill_or_allowed(cx, MISSING_PANICS_DOC, [expr.hir_id]) && panic_span.is_none() { diff --git a/clippy_lints/src/doc/mod.rs b/clippy_lints/src/doc/mod.rs index 5ea55e102df..2bf52216b83 100644 --- a/clippy_lints/src/doc/mod.rs +++ b/clippy_lints/src/doc/mod.rs @@ -1249,7 +1249,9 @@ fn looks_like_refdef(doc: &str, range: Range<usize>) -> Option<Range<usize>> { b'[' => { start = Some(i + offset); }, - b']' if let Some(start) = start => { + b']' if let Some(start) = start + && doc.as_bytes().get(i + offset + 1) == Some(&b':') => + { return Some(start..i + offset + 1); }, _ => {}, diff --git a/clippy_lints/src/exhaustive_items.rs b/clippy_lints/src/exhaustive_items.rs index 7dda3e0fdb9..8ad09279071 100644 --- a/clippy_lints/src/exhaustive_items.rs +++ b/clippy_lints/src/exhaustive_items.rs @@ -1,12 +1,10 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::indent_of; +use rustc_attr_data_structures::{AttributeKind, find_attr}; use rustc_errors::Applicability; use rustc_hir::{Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; -use rustc_attr_data_structures::AttributeKind; -use rustc_attr_data_structures::find_attr; - declare_clippy_lint! { /// ### What it does diff --git a/clippy_lints/src/exit.rs b/clippy_lints/src/exit.rs index cc8e4d7d9e2..487db69027a 100644 --- a/clippy_lints/src/exit.rs +++ b/clippy_lints/src/exit.rs @@ -1,5 +1,4 @@ use clippy_utils::diagnostics::span_lint; -use clippy_utils::is_entrypoint_fn; use rustc_hir::{Expr, ExprKind, Item, ItemKind, OwnerNode}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; @@ -7,7 +6,8 @@ use rustc_span::sym; declare_clippy_lint! { /// ### What it does - /// Detects calls to the `exit()` function which terminates the program. + /// Detects calls to the `exit()` function that are not in the `main` function. Calls to `exit()` + /// immediately terminate the program. /// /// ### Why restrict this? /// `exit()` immediately terminates the program with no information other than an exit code. @@ -15,11 +15,24 @@ declare_clippy_lint! { /// /// Codebases may use this lint to require that all exits are performed either by panicking /// (which produces a message, a code location, and optionally a backtrace) - /// or by returning from `main()` (which is a single place to look). + /// or by calling `exit()` from `main()` (which is a single place to look). /// - /// ### Example + /// ### Good example /// ```no_run - /// std::process::exit(0) + /// fn main() { + /// std::process::exit(0); + /// } + /// ``` + /// + /// ### Bad example + /// ```no_run + /// fn main() { + /// other_function(); + /// } + /// + /// fn other_function() { + /// std::process::exit(0); + /// } /// ``` /// /// Use instead: @@ -36,7 +49,7 @@ declare_clippy_lint! { #[clippy::version = "1.41.0"] pub EXIT, restriction, - "detects `std::process::exit` calls" + "detects `std::process::exit` calls outside of `main`" } declare_lint_pass!(Exit => [EXIT]); @@ -48,10 +61,14 @@ impl<'tcx> LateLintPass<'tcx> for Exit { && let Some(def_id) = cx.qpath_res(path, path_expr.hir_id).opt_def_id() && cx.tcx.is_diagnostic_item(sym::process_exit, def_id) && let parent = cx.tcx.hir_get_parent_item(e.hir_id) - && let OwnerNode::Item(Item{kind: ItemKind::Fn{ .. }, ..}) = cx.tcx.hir_owner_node(parent) - // If the next item up is a function we check if it is an entry point + && let OwnerNode::Item(Item{kind: ItemKind::Fn{ ident, .. }, ..}) = cx.tcx.hir_owner_node(parent) + // If the next item up is a function we check if it isn't named "main" // and only then emit a linter warning - && !is_entrypoint_fn(cx, parent.to_def_id()) + + // if you instead check for the parent of the `exit()` call being the entrypoint function, as this worked before, + // in compilation contexts like --all-targets (which include --tests), you get false positives + // because in a test context, main is not the entrypoint function + && ident.name != sym::main { span_lint(cx, EXIT, e.span, "usage of `process::exit`"); } diff --git a/clippy_lints/src/floating_point_arithmetic.rs b/clippy_lints/src/floating_point_arithmetic.rs index b3c9e860758..d5abaa547e8 100644 --- a/clippy_lints/src/floating_point_arithmetic.rs +++ b/clippy_lints/src/floating_point_arithmetic.rs @@ -2,8 +2,8 @@ use clippy_utils::consts::Constant::{F32, F64, Int}; use clippy_utils::consts::{ConstEvalCtxt, Constant}; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::{ - eq_expr_value, get_parent_expr, higher, is_in_const_context, is_inherent_method_call, is_no_std_crate, - numeric_literal, peel_blocks, sugg, sym, + eq_expr_value, get_parent_expr, has_ambiguous_literal_in_expr, higher, is_in_const_context, + is_inherent_method_call, is_no_std_crate, numeric_literal, peel_blocks, sugg, sym, }; use rustc_ast::ast; use rustc_errors::Applicability; @@ -455,7 +455,6 @@ fn is_float_mul_expr<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<(&' None } -// TODO: Fix rust-lang/rust-clippy#4735 fn check_mul_add(cx: &LateContext<'_>, expr: &Expr<'_>) { if let ExprKind::Binary( Spanned { @@ -491,6 +490,14 @@ fn check_mul_add(cx: &LateContext<'_>, expr: &Expr<'_>) { return; }; + // Check if any variable in the expression has an ambiguous type (could be f32 or f64) + // see: https://github.com/rust-lang/rust-clippy/issues/14897 + if (matches!(recv.kind, ExprKind::Path(_)) || matches!(recv.kind, ExprKind::Call(_, _))) + && has_ambiguous_literal_in_expr(cx, recv) + { + return; + } + span_lint_and_sugg( cx, SUBOPTIMAL_FLOPS, diff --git a/clippy_lints/src/loops/empty_loop.rs b/clippy_lints/src/loops/empty_loop.rs index 823cf0f4322..e809987d75a 100644 --- a/clippy_lints/src/loops/empty_loop.rs +++ b/clippy_lints/src/loops/empty_loop.rs @@ -1,11 +1,22 @@ use super::EMPTY_LOOP; use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::{is_in_panic_handler, is_no_std_crate}; +use clippy_utils::{is_in_panic_handler, is_no_std_crate, sym}; -use rustc_hir::{Block, Expr}; +use rustc_hir::{Block, Expr, ItemKind, Node}; use rustc_lint::LateContext; pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, loop_block: &Block<'_>) { + let parent_hir_id = cx.tcx.parent_hir_id(expr.hir_id); + if let Node::Item(parent_node) = cx.tcx.hir_node(parent_hir_id) + && matches!(parent_node.kind, ItemKind::Fn { .. }) + && let attrs = cx.tcx.hir_attrs(parent_hir_id) + && attrs.iter().any(|attr| attr.has_name(sym::rustc_intrinsic)) + { + // Intrinsic functions are expanded into an empty loop when lowering the AST + // to simplify the job of later passes which might expect any function to have a body. + return; + } + if loop_block.stmts.is_empty() && loop_block.expr.is_none() && !is_in_panic_handler(cx, expr) { let msg = "empty `loop {}` wastes CPU cycles"; let help = if is_no_std_crate(cx) { diff --git a/clippy_lints/src/manual_let_else.rs b/clippy_lints/src/manual_let_else.rs index 9ff82cdcb66..1f9a943f13d 100644 --- a/clippy_lints/src/manual_let_else.rs +++ b/clippy_lints/src/manual_let_else.rs @@ -245,17 +245,36 @@ fn replace_in_pattern( } match pat.kind { - PatKind::Binding(_ann, _id, binding_name, opt_subpt) => { - let Some((pat_to_put, binding_mode)) = ident_map.get(&binding_name.name) else { - break 'a; - }; - let sn_pfx = binding_mode.prefix_str(); - let (sn_ptp, _) = snippet_with_context(cx, pat_to_put.span, span.ctxt(), "", app); - if let Some(subpt) = opt_subpt { - let subpt = replace_in_pattern(cx, span, ident_map, subpt, app, false); - return format!("{sn_pfx}{sn_ptp} @ {subpt}"); + PatKind::Binding(ann, _id, binding_name, opt_subpt) => { + match (ident_map.get(&binding_name.name), opt_subpt) { + (Some((pat_to_put, binding_mode)), opt_subpt) => { + let sn_pfx = binding_mode.prefix_str(); + let (sn_ptp, _) = snippet_with_context(cx, pat_to_put.span, span.ctxt(), "", app); + if let Some(subpt) = opt_subpt { + let subpt = replace_in_pattern(cx, span, ident_map, subpt, app, false); + return format!("{sn_pfx}{sn_ptp} @ {subpt}"); + } + return format!("{sn_pfx}{sn_ptp}"); + }, + (None, Some(subpt)) => { + let subpt = replace_in_pattern(cx, span, ident_map, subpt, app, false); + // scanning for a value that matches is not sensitive to order + #[expect(rustc::potential_query_instability)] + if ident_map.values().any(|(other_pat, _)| { + if let PatKind::Binding(_, _, other_name, _) = other_pat.kind { + other_name == binding_name + } else { + false + } + }) { + // this name is shadowed, and, therefore, not usable + return subpt; + } + let binding_pfx = ann.prefix_str(); + return format!("{binding_pfx}{binding_name} @ {subpt}"); + }, + (None, None) => break 'a, } - return format!("{sn_pfx}{sn_ptp}"); }, PatKind::Or(pats) => { let patterns = pats diff --git a/clippy_lints/src/manual_non_exhaustive.rs b/clippy_lints/src/manual_non_exhaustive.rs index 51696b38880..2d52a93f34e 100644 --- a/clippy_lints/src/manual_non_exhaustive.rs +++ b/clippy_lints/src/manual_non_exhaustive.rs @@ -4,16 +4,15 @@ use clippy_utils::is_doc_hidden; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::source::snippet_indent; use itertools::Itertools; +use rustc_attr_data_structures::{AttributeKind, find_attr}; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; use rustc_hir::{Expr, ExprKind, Item, ItemKind, QPath, TyKind, VariantData}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::impl_lint_pass; -use rustc_span::def_id::LocalDefId; use rustc_span::Span; -use rustc_attr_data_structures::find_attr; -use rustc_attr_data_structures::AttributeKind; +use rustc_span::def_id::LocalDefId; declare_clippy_lint! { /// ### What it does diff --git a/clippy_lints/src/matches/match_same_arms.rs b/clippy_lints/src/matches/match_same_arms.rs index ede68f30941..ae277da089f 100644 --- a/clippy_lints/src/matches/match_same_arms.rs +++ b/clippy_lints/src/matches/match_same_arms.rs @@ -332,9 +332,7 @@ impl<'a> NormalizedPat<'a> { // TODO: Handle negative integers. They're currently treated as a wild match. PatExprKind::Lit { lit, negated: false } => match lit.node { LitKind::Str(sym, _) => Self::LitStr(sym), - LitKind::ByteStr(byte_sym, _) | LitKind::CStr(byte_sym, _) => { - Self::LitBytes(byte_sym) - } + LitKind::ByteStr(byte_sym, _) | LitKind::CStr(byte_sym, _) => Self::LitBytes(byte_sym), LitKind::Byte(val) => Self::LitInt(val.into()), LitKind::Char(val) => Self::LitInt(val.into()), LitKind::Int(val, _) => Self::LitInt(val.get()), diff --git a/clippy_lints/src/methods/manual_is_variant_and.rs b/clippy_lints/src/methods/manual_is_variant_and.rs index 4a61c223d2c..93325ca488e 100644 --- a/clippy_lints/src/methods/manual_is_variant_and.rs +++ b/clippy_lints/src/methods/manual_is_variant_and.rs @@ -1,14 +1,16 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::get_parent_expr; use clippy_utils::msrvs::{self, Msrv}; -use clippy_utils::source::{snippet, snippet_opt}; +use clippy_utils::source::{snippet, snippet_with_applicability}; +use clippy_utils::sugg::Sugg; use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{get_parent_expr, sym}; +use rustc_ast::LitKind; use rustc_errors::Applicability; use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; -use rustc_hir::{BinOpKind, Expr, ExprKind, QPath}; +use rustc_hir::{BinOpKind, Closure, Expr, ExprKind, QPath}; use rustc_lint::LateContext; use rustc_middle::ty; -use rustc_span::{BytePos, Span, sym}; +use rustc_span::{Span, Symbol}; use super::MANUAL_IS_VARIANT_AND; @@ -62,54 +64,154 @@ pub(super) fn check( ); } -fn emit_lint(cx: &LateContext<'_>, op: BinOpKind, parent: &Expr<'_>, method_span: Span, is_option: bool) { - if let Some(before_map_snippet) = snippet_opt(cx, parent.span.with_hi(method_span.lo())) - && let Some(after_map_snippet) = snippet_opt(cx, method_span.with_lo(method_span.lo() + BytePos(3))) - { - span_lint_and_sugg( - cx, - MANUAL_IS_VARIANT_AND, - parent.span, - format!( - "called `.map() {}= {}()`", - if op == BinOpKind::Eq { '=' } else { '!' }, - if is_option { "Some" } else { "Ok" }, - ), - "use", - if is_option && op == BinOpKind::Ne { - format!("{before_map_snippet}is_none_or{after_map_snippet}",) - } else { +#[derive(Clone, Copy, PartialEq)] +enum Flavor { + Option, + Result, +} + +impl Flavor { + const fn symbol(self) -> Symbol { + match self { + Self::Option => sym::Option, + Self::Result => sym::Result, + } + } + + const fn positive(self) -> Symbol { + match self { + Self::Option => sym::Some, + Self::Result => sym::Ok, + } + } +} + +#[derive(Clone, Copy, PartialEq)] +enum Op { + Eq, + Ne, +} + +impl std::fmt::Display for Op { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Eq => write!(f, "=="), + Self::Ne => write!(f, "!="), + } + } +} + +impl TryFrom<BinOpKind> for Op { + type Error = (); + fn try_from(op: BinOpKind) -> Result<Self, Self::Error> { + match op { + BinOpKind::Eq => Ok(Self::Eq), + BinOpKind::Ne => Ok(Self::Ne), + _ => Err(()), + } + } +} + +/// Represents the argument of the `.map()` function, as a closure or as a path +/// in case η-reduction is used. +enum MapFunc<'hir> { + Closure(&'hir Closure<'hir>), + Path(&'hir Expr<'hir>), +} + +impl<'hir> TryFrom<&'hir Expr<'hir>> for MapFunc<'hir> { + type Error = (); + + fn try_from(expr: &'hir Expr<'hir>) -> Result<Self, Self::Error> { + match expr.kind { + ExprKind::Closure(closure) => Ok(Self::Closure(closure)), + ExprKind::Path(_) => Ok(Self::Path(expr)), + _ => Err(()), + } + } +} + +impl<'hir> MapFunc<'hir> { + /// Build a suggestion suitable for use in a `.map()`-like function. η-expansion will be applied + /// as needed. + fn sugg(self, cx: &LateContext<'hir>, invert: bool, app: &mut Applicability) -> String { + match self { + Self::Closure(closure) => { + let body = Sugg::hir_with_applicability(cx, cx.tcx.hir_body(closure.body).value, "..", app); format!( - "{}{before_map_snippet}{}{after_map_snippet}", - if op == BinOpKind::Eq { "" } else { "!" }, - if is_option { "is_some_and" } else { "is_ok_and" }, + "{} {}", + snippet_with_applicability(cx, closure.fn_decl_span, "|..|", app), + if invert { !body } else { body } ) }, - Applicability::MachineApplicable, - ); + Self::Path(expr) => { + let path = snippet_with_applicability(cx, expr.span, "_", app); + if invert { + format!("|x| !{path}(x)") + } else { + path.to_string() + } + }, + } } } +fn emit_lint<'tcx>( + cx: &LateContext<'tcx>, + span: Span, + op: Op, + flavor: Flavor, + in_some_or_ok: bool, + map_func: MapFunc<'tcx>, + recv: &Expr<'_>, +) { + let mut app = Applicability::MachineApplicable; + let recv = snippet_with_applicability(cx, recv.span, "_", &mut app); + + let (invert_expr, method, invert_body) = match (flavor, op) { + (Flavor::Option, Op::Eq) => (false, "is_some_and", !in_some_or_ok), + (Flavor::Option, Op::Ne) => (false, "is_none_or", in_some_or_ok), + (Flavor::Result, Op::Eq) => (false, "is_ok_and", !in_some_or_ok), + (Flavor::Result, Op::Ne) => (true, "is_ok_and", !in_some_or_ok), + }; + span_lint_and_sugg( + cx, + MANUAL_IS_VARIANT_AND, + span, + format!("called `.map() {op} {pos}()`", pos = flavor.positive(),), + "use", + format!( + "{inversion}{recv}.{method}({body})", + inversion = if invert_expr { "!" } else { "" }, + body = map_func.sugg(cx, invert_body, &mut app), + ), + app, + ); +} + pub(super) fn check_map(cx: &LateContext<'_>, expr: &Expr<'_>) { if let Some(parent_expr) = get_parent_expr(cx, expr) && let ExprKind::Binary(op, left, right) = parent_expr.kind - && matches!(op.node, BinOpKind::Eq | BinOpKind::Ne) && op.span.eq_ctxt(expr.span) + && let Ok(op) = Op::try_from(op.node) { // Check `left` and `right` expression in any order, and for `Option` and `Result` for (expr1, expr2) in [(left, right), (right, left)] { - for item in [sym::Option, sym::Result] { - if let ExprKind::Call(call, ..) = expr1.kind + for flavor in [Flavor::Option, Flavor::Result] { + if let ExprKind::Call(call, [arg]) = expr1.kind + && let ExprKind::Lit(lit) = arg.kind + && let LitKind::Bool(bool_cst) = lit.node && let ExprKind::Path(QPath::Resolved(_, path)) = call.kind && let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), _) = path.res && let ty = cx.typeck_results().expr_ty(expr1) && let ty::Adt(adt, args) = ty.kind() - && cx.tcx.is_diagnostic_item(item, adt.did()) + && cx.tcx.is_diagnostic_item(flavor.symbol(), adt.did()) && args.type_at(0).is_bool() - && let ExprKind::MethodCall(_, recv, _, span) = expr2.kind - && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), item) + && let ExprKind::MethodCall(_, recv, [map_expr], _) = expr2.kind + && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), flavor.symbol()) + && let Ok(map_func) = MapFunc::try_from(map_expr) { - return emit_lint(cx, op.node, parent_expr, span, item == sym::Option); + return emit_lint(cx, parent_expr.span, op, flavor, bool_cst, map_func, recv); } } } diff --git a/clippy_lints/src/methods/or_fun_call.rs b/clippy_lints/src/methods/or_fun_call.rs index 2139466ce74..6ce7dd3d4d0 100644 --- a/clippy_lints/src/methods/or_fun_call.rs +++ b/clippy_lints/src/methods/or_fun_call.rs @@ -136,7 +136,7 @@ pub(super) fn check<'tcx>( fun_span: Option<Span>, ) -> bool { // (path, fn_has_argument, methods, suffix) - const KNOW_TYPES: [(Symbol, bool, &[Symbol], &str); 5] = [ + const KNOW_TYPES: [(Symbol, bool, &[Symbol], &str); 7] = [ (sym::BTreeEntry, false, &[sym::or_insert], "with"), (sym::HashMapEntry, false, &[sym::or_insert], "with"), ( @@ -146,7 +146,9 @@ pub(super) fn check<'tcx>( "else", ), (sym::Option, false, &[sym::get_or_insert], "with"), + (sym::Option, true, &[sym::and], "then"), (sym::Result, true, &[sym::map_or, sym::or, sym::unwrap_or], "else"), + (sym::Result, true, &[sym::and], "then"), ]; if KNOW_TYPES.iter().any(|k| k.2.contains(&name)) diff --git a/clippy_lints/src/methods/return_and_then.rs b/clippy_lints/src/methods/return_and_then.rs index df8544f9220..54f38a322b8 100644 --- a/clippy_lints/src/methods/return_and_then.rs +++ b/clippy_lints/src/methods/return_and_then.rs @@ -1,5 +1,5 @@ use rustc_errors::Applicability; -use rustc_hir as hir; +use rustc_hir::{self as hir, Node}; use rustc_lint::LateContext; use rustc_middle::ty::{self, GenericArg, Ty}; use rustc_span::sym; @@ -9,7 +9,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::{indent_of, reindent_multiline, snippet_with_applicability}; use clippy_utils::ty::get_type_diagnostic_name; use clippy_utils::visitors::for_each_unconsumed_temporary; -use clippy_utils::{get_parent_expr, peel_blocks}; +use clippy_utils::{peel_blocks, potential_return_of_enclosing_body}; use super::RETURN_AND_THEN; @@ -21,7 +21,7 @@ pub(super) fn check<'tcx>( recv: &'tcx hir::Expr<'tcx>, arg: &'tcx hir::Expr<'_>, ) { - if cx.tcx.hir_get_fn_id_for_return_block(expr.hir_id).is_none() { + if !potential_return_of_enclosing_body(cx, expr) { return; } @@ -55,15 +55,28 @@ pub(super) fn check<'tcx>( None => &body_snip, }; - // If suggestion is going to get inserted as part of a `return` expression, it must be blockified. - let sugg = if let Some(parent_expr) = get_parent_expr(cx, expr) { - let base_indent = indent_of(cx, parent_expr.span); + // If suggestion is going to get inserted as part of a `return` expression or as a match expression + // arm, it must be blockified. + let (parent_span_for_indent, opening_paren, closing_paren) = match cx.tcx.parent_hir_node(expr.hir_id) { + Node::Expr(parent_expr) if matches!(parent_expr.kind, hir::ExprKind::Break(..)) => { + (Some(parent_expr.span), "(", ")") + }, + Node::Expr(parent_expr) => (Some(parent_expr.span), "", ""), + Node::Arm(match_arm) => (Some(match_arm.span), "", ""), + _ => (None, "", ""), + }; + let sugg = if let Some(span) = parent_span_for_indent { + let base_indent = indent_of(cx, span); let inner_indent = base_indent.map(|i| i + 4); format!( "{}\n{}\n{}", - reindent_multiline(&format!("{{\nlet {arg_snip} = {recv_snip}?;"), true, inner_indent), + reindent_multiline( + &format!("{opening_paren}{{\nlet {arg_snip} = {recv_snip}?;"), + true, + inner_indent + ), reindent_multiline(inner, false, inner_indent), - reindent_multiline("}", false, base_indent), + reindent_multiline(&format!("}}{closing_paren}"), false, base_indent), ) } else { format!( diff --git a/clippy_lints/src/methods/swap_with_temporary.rs b/clippy_lints/src/methods/swap_with_temporary.rs index de729fb343a..e378cbd6ae0 100644 --- a/clippy_lints/src/methods/swap_with_temporary.rs +++ b/clippy_lints/src/methods/swap_with_temporary.rs @@ -4,6 +4,7 @@ use rustc_ast::BorrowKind; use rustc_errors::{Applicability, Diag}; use rustc_hir::{Expr, ExprKind, Node, QPath}; use rustc_lint::LateContext; +use rustc_middle::ty::adjustment::Adjust; use rustc_span::sym; use super::SWAP_WITH_TEMPORARY; @@ -11,12 +12,12 @@ use super::SWAP_WITH_TEMPORARY; const MSG_TEMPORARY: &str = "this expression returns a temporary value"; const MSG_TEMPORARY_REFMUT: &str = "this is a mutable reference to a temporary value"; -pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, func: &Expr<'_>, args: &[Expr<'_>]) { +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, func: &Expr<'_>, args: &'tcx [Expr<'_>]) { if let ExprKind::Path(QPath::Resolved(_, func_path)) = func.kind && let Some(func_def_id) = func_path.res.opt_def_id() && cx.tcx.is_diagnostic_item(sym::mem_swap, func_def_id) { - match (ArgKind::new(&args[0]), ArgKind::new(&args[1])) { + match (ArgKind::new(cx, &args[0]), ArgKind::new(cx, &args[1])) { (ArgKind::RefMutToTemp(left_temp), ArgKind::RefMutToTemp(right_temp)) => { emit_lint_useless(cx, expr, &args[0], &args[1], left_temp, right_temp); }, @@ -28,10 +29,10 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, func: &Expr<'_>, args } enum ArgKind<'tcx> { - // Mutable reference to a place, coming from a macro - RefMutToPlaceAsMacro(&'tcx Expr<'tcx>), - // Place behind a mutable reference - RefMutToPlace(&'tcx Expr<'tcx>), + // Mutable reference to a place, coming from a macro, and number of dereferences to use + RefMutToPlaceAsMacro(&'tcx Expr<'tcx>, usize), + // Place behind a mutable reference, and number of dereferences to use + RefMutToPlace(&'tcx Expr<'tcx>, usize), // Temporary value behind a mutable reference RefMutToTemp(&'tcx Expr<'tcx>), // Any other case @@ -39,13 +40,29 @@ enum ArgKind<'tcx> { } impl<'tcx> ArgKind<'tcx> { - fn new(arg: &'tcx Expr<'tcx>) -> Self { - if let ExprKind::AddrOf(BorrowKind::Ref, _, target) = arg.kind { - if target.is_syntactic_place_expr() { + /// Build a new `ArgKind` from `arg`. There must be no false positive when returning a + /// `ArgKind::RefMutToTemp` variant, as this may cause a spurious lint to be emitted. + fn new(cx: &LateContext<'tcx>, arg: &'tcx Expr<'tcx>) -> Self { + if let ExprKind::AddrOf(BorrowKind::Ref, _, target) = arg.kind + && let adjustments = cx.typeck_results().expr_adjustments(arg) + && adjustments + .first() + .is_some_and(|adj| matches!(adj.kind, Adjust::Deref(None))) + && adjustments + .last() + .is_some_and(|adj| matches!(adj.kind, Adjust::Borrow(_))) + { + let extra_derefs = adjustments[1..adjustments.len() - 1] + .iter() + .filter(|adj| matches!(adj.kind, Adjust::Deref(_))) + .count(); + // If a deref is used, `arg` might be a place expression. For example, a mutex guard + // would dereference into the mutex content which is probably not temporary. + if target.is_syntactic_place_expr() || extra_derefs > 0 { if arg.span.from_expansion() { - ArgKind::RefMutToPlaceAsMacro(arg) + ArgKind::RefMutToPlaceAsMacro(arg, extra_derefs) } else { - ArgKind::RefMutToPlace(target) + ArgKind::RefMutToPlace(target, extra_derefs) } } else { ArgKind::RefMutToTemp(target) @@ -106,10 +123,15 @@ fn emit_lint_assign(cx: &LateContext<'_>, expr: &Expr<'_>, target: &ArgKind<'_>, let mut applicability = Applicability::MachineApplicable; let ctxt = expr.span.ctxt(); let assign_target = match target { - ArgKind::Expr(target) | ArgKind::RefMutToPlaceAsMacro(target) => { - Sugg::hir_with_context(cx, target, ctxt, "_", &mut applicability).deref() - }, - ArgKind::RefMutToPlace(target) => Sugg::hir_with_context(cx, target, ctxt, "_", &mut applicability), + ArgKind::Expr(target) => Sugg::hir_with_context(cx, target, ctxt, "_", &mut applicability).deref(), + ArgKind::RefMutToPlaceAsMacro(arg, derefs) => (0..*derefs).fold( + Sugg::hir_with_context(cx, arg, ctxt, "_", &mut applicability).deref(), + |sugg, _| sugg.deref(), + ), + ArgKind::RefMutToPlace(target, derefs) => (0..*derefs).fold( + Sugg::hir_with_context(cx, target, ctxt, "_", &mut applicability), + |sugg, _| sugg.deref(), + ), ArgKind::RefMutToTemp(_) => unreachable!(), }; let assign_source = Sugg::hir_with_context(cx, temp, ctxt, "_", &mut applicability); diff --git a/clippy_lints/src/methods/unnecessary_map_or.rs b/clippy_lints/src/methods/unnecessary_map_or.rs index b90748dd158..4a9007c607c 100644 --- a/clippy_lints/src/methods/unnecessary_map_or.rs +++ b/clippy_lints/src/methods/unnecessary_map_or.rs @@ -62,7 +62,8 @@ pub(super) fn check<'a>( let ext_def_span = def.span.until(map.span); - let (sugg, method, applicability) = if let ExprKind::Closure(map_closure) = map.kind + let (sugg, method, applicability) = if cx.typeck_results().expr_adjustments(recv).is_empty() + && let ExprKind::Closure(map_closure) = map.kind && let closure_body = cx.tcx.hir_body(map_closure.body) && let closure_body_value = closure_body.value.peel_blocks() && let ExprKind::Binary(op, l, r) = closure_body_value.kind diff --git a/clippy_lints/src/needless_bool.rs b/clippy_lints/src/needless_bool.rs index 3ed4b1c2ea9..b3aa1a7286a 100644 --- a/clippy_lints/src/needless_bool.rs +++ b/clippy_lints/src/needless_bool.rs @@ -199,11 +199,16 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBool { let mut applicability = Applicability::MachineApplicable; let cond = Sugg::hir_with_applicability(cx, cond, "..", &mut applicability); let lhs = snippet_with_applicability(cx, lhs_a.span, "..", &mut applicability); - let sugg = if a == b { + let mut sugg = if a == b { format!("{cond}; {lhs} = {a:?};") } else { format!("{lhs} = {};", if a { cond } else { !cond }) }; + + if is_else_clause(cx.tcx, e) { + sugg = format!("{{ {sugg} }}"); + } + span_lint_and_sugg( cx, NEEDLESS_BOOL_ASSIGN, diff --git a/clippy_lints/src/neg_multiply.rs b/clippy_lints/src/neg_multiply.rs index 442280f9998..946114e1041 100644 --- a/clippy_lints/src/neg_multiply.rs +++ b/clippy_lints/src/neg_multiply.rs @@ -1,13 +1,13 @@ use clippy_utils::consts::{self, Constant}; use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::snippet_with_context; +use clippy_utils::get_parent_expr; +use clippy_utils::source::{snippet, snippet_with_context}; use clippy_utils::sugg::has_enclosing_paren; use rustc_ast::util::parser::ExprPrecedence; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; -use rustc_span::Span; declare_clippy_lint! { /// ### What it does @@ -33,6 +33,19 @@ declare_clippy_lint! { declare_lint_pass!(NegMultiply => [NEG_MULTIPLY]); +fn is_in_parens_with_postfix(cx: &LateContext<'_>, mul_expr: &Expr<'_>) -> bool { + if let Some(parent) = get_parent_expr(cx, mul_expr) { + let mult_snippet = snippet(cx, mul_expr.span, ""); + if has_enclosing_paren(&mult_snippet) + && let ExprKind::MethodCall(_, _, _, _) = parent.kind + { + return true; + } + } + + false +} + impl<'tcx> LateLintPass<'tcx> for NegMultiply { fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { if let ExprKind::Binary(ref op, left, right) = e.kind @@ -40,15 +53,15 @@ impl<'tcx> LateLintPass<'tcx> for NegMultiply { { match (&left.kind, &right.kind) { (&ExprKind::Unary(..), &ExprKind::Unary(..)) => {}, - (&ExprKind::Unary(UnOp::Neg, lit), _) => check_mul(cx, e.span, lit, right), - (_, &ExprKind::Unary(UnOp::Neg, lit)) => check_mul(cx, e.span, lit, left), + (&ExprKind::Unary(UnOp::Neg, lit), _) => check_mul(cx, e, lit, right), + (_, &ExprKind::Unary(UnOp::Neg, lit)) => check_mul(cx, e, lit, left), _ => {}, } } } } -fn check_mul(cx: &LateContext<'_>, span: Span, lit: &Expr<'_>, exp: &Expr<'_>) { +fn check_mul(cx: &LateContext<'_>, mul_expr: &Expr<'_>, lit: &Expr<'_>, exp: &Expr<'_>) { const F16_ONE: u16 = 1.0_f16.to_bits(); const F128_ONE: u128 = 1.0_f128.to_bits(); if let ExprKind::Lit(l) = lit.kind @@ -63,8 +76,19 @@ fn check_mul(cx: &LateContext<'_>, span: Span, lit: &Expr<'_>, exp: &Expr<'_>) { && cx.typeck_results().expr_ty(exp).is_numeric() { let mut applicability = Applicability::MachineApplicable; - let (snip, from_macro) = snippet_with_context(cx, exp.span, span.ctxt(), "..", &mut applicability); - let suggestion = if !from_macro && cx.precedence(exp) < ExprPrecedence::Prefix && !has_enclosing_paren(&snip) { + let (snip, from_macro) = snippet_with_context(cx, exp.span, mul_expr.span.ctxt(), "..", &mut applicability); + + let needs_parens_for_postfix = is_in_parens_with_postfix(cx, mul_expr); + + let suggestion = if needs_parens_for_postfix { + // Special case: when the multiplication is in parentheses followed by a method call + // we need to preserve the grouping but negate the inner expression. + // Consider this expression: `((a.delta - 0.5).abs() * -1.0).total_cmp(&1.0)` + // We need to end up with: `(-(a.delta - 0.5).abs()).total_cmp(&1.0)` + // Otherwise, without the parentheses we would try to negate an Ordering: + // `-(a.delta - 0.5).abs().total_cmp(&1.0)` + format!("(-{snip})") + } else if !from_macro && cx.precedence(exp) < ExprPrecedence::Prefix && !has_enclosing_paren(&snip) { format!("-({snip})") } else { format!("-{snip}") @@ -72,7 +96,7 @@ fn check_mul(cx: &LateContext<'_>, span: Span, lit: &Expr<'_>, exp: &Expr<'_>) { span_lint_and_sugg( cx, NEG_MULTIPLY, - span, + mul_expr.span, "this multiplication by -1 can be written more succinctly", "consider using", suggestion, diff --git a/clippy_lints/src/no_effect.rs b/clippy_lints/src/no_effect.rs index 02c48166131..72e6503e7e4 100644 --- a/clippy_lints/src/no_effect.rs +++ b/clippy_lints/src/no_effect.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then}; use clippy_utils::source::SpanRangeExt; -use clippy_utils::ty::has_drop; +use clippy_utils::ty::{expr_type_is_certain, has_drop}; use clippy_utils::{ in_automatically_derived, is_inside_always_const_context, is_lint_allowed, path_to_local, peel_blocks, }; @@ -340,11 +340,13 @@ fn reduce_expression<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<Vec }, ExprKind::Array(v) | ExprKind::Tup(v) => Some(v.iter().collect()), ExprKind::Repeat(inner, _) - | ExprKind::Cast(inner, _) | ExprKind::Type(inner, _) | ExprKind::Unary(_, inner) | ExprKind::Field(inner, _) | ExprKind::AddrOf(_, _, inner) => reduce_expression(cx, inner).or_else(|| Some(vec![inner])), + ExprKind::Cast(inner, _) if expr_type_is_certain(cx, inner) => { + reduce_expression(cx, inner).or_else(|| Some(vec![inner])) + }, ExprKind::Struct(_, fields, ref base) => { if has_drop(cx, cx.typeck_results().expr_ty(expr)) { None diff --git a/clippy_lints/src/operators/op_ref.rs b/clippy_lints/src/operators/op_ref.rs index 0faa7b9e646..21e1ab0f4f2 100644 --- a/clippy_lints/src/operators/op_ref.rs +++ b/clippy_lints/src/operators/op_ref.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::get_enclosing_block; -use clippy_utils::source::snippet; +use clippy_utils::source::snippet_with_context; use clippy_utils::ty::{implements_trait, is_copy}; use rustc_errors::Applicability; use rustc_hir::def::Res; @@ -61,12 +61,13 @@ pub(crate) fn check<'tcx>( e.span, "needlessly taken reference of both operands", |diag| { - let lsnip = snippet(cx, l.span, "...").to_string(); - let rsnip = snippet(cx, r.span, "...").to_string(); + let mut applicability = Applicability::MachineApplicable; + let (lsnip, _) = snippet_with_context(cx, l.span, e.span.ctxt(), "...", &mut applicability); + let (rsnip, _) = snippet_with_context(cx, r.span, e.span.ctxt(), "...", &mut applicability); diag.multipart_suggestion( "use the values directly", - vec![(left.span, lsnip), (right.span, rsnip)], - Applicability::MachineApplicable, + vec![(left.span, lsnip.to_string()), (right.span, rsnip.to_string())], + applicability, ); }, ); @@ -80,13 +81,9 @@ pub(crate) fn check<'tcx>( e.span, "needlessly taken reference of left operand", |diag| { - let lsnip = snippet(cx, l.span, "...").to_string(); - diag.span_suggestion( - left.span, - "use the left value directly", - lsnip, - Applicability::MachineApplicable, - ); + let mut applicability = Applicability::MachineApplicable; + let (lsnip, _) = snippet_with_context(cx, l.span, e.span.ctxt(), "...", &mut applicability); + diag.span_suggestion(left.span, "use the left value directly", lsnip, applicability); }, ); } else if !lcpy @@ -99,7 +96,8 @@ pub(crate) fn check<'tcx>( e.span, "needlessly taken reference of right operand", |diag| { - let rsnip = snippet(cx, r.span, "...").to_string(); + let mut applicability = Applicability::MachineApplicable; + let (rsnip, _) = snippet_with_context(cx, r.span, e.span.ctxt(), "...", &mut applicability); diag.span_suggestion( right.span, "use the right value directly", @@ -131,7 +129,8 @@ pub(crate) fn check<'tcx>( e.span, "needlessly taken reference of left operand", |diag| { - let lsnip = snippet(cx, l.span, "...").to_string(); + let mut applicability = Applicability::MachineApplicable; + let (lsnip, _) = snippet_with_context(cx, l.span, e.span.ctxt(), "...", &mut applicability); diag.span_suggestion( left.span, "use the left value directly", @@ -158,7 +157,8 @@ pub(crate) fn check<'tcx>( && implements_trait(cx, cx.typeck_results().expr_ty(left), trait_id, &[rty.into()]) { span_lint_and_then(cx, OP_REF, e.span, "taken reference of right operand", |diag| { - let rsnip = snippet(cx, r.span, "...").to_string(); + let mut applicability = Applicability::MachineApplicable; + let (rsnip, _) = snippet_with_context(cx, r.span, e.span.ctxt(), "...", &mut applicability); diag.span_suggestion( right.span, "use the right value directly", diff --git a/clippy_lints/src/redundant_closure_call.rs b/clippy_lints/src/redundant_closure_call.rs index 84597269a58..1c23fe998ad 100644 --- a/clippy_lints/src/redundant_closure_call.rs +++ b/clippy_lints/src/redundant_closure_call.rs @@ -5,9 +5,7 @@ use hir::Param; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::intravisit::{Visitor as HirVisitor, Visitor}; -use rustc_hir::{ - ClosureKind, CoroutineDesugaring, CoroutineKind, CoroutineSource, ExprKind, Node, intravisit as hir_visit, -}; +use rustc_hir::{ClosureKind, CoroutineDesugaring, CoroutineKind, CoroutineSource, ExprKind, intravisit as hir_visit}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::hir::nested_filter; use rustc_middle::ty; @@ -198,15 +196,15 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClosureCall { hint = hint.asyncify(); } - let is_in_fn_call_arg = if let Node::Expr(expr) = cx.tcx.parent_hir_node(expr.hir_id) { - matches!(expr.kind, ExprKind::Call(_, _)) - } else { - false - }; - - // avoid clippy::double_parens - if !is_in_fn_call_arg { - hint = hint.maybe_paren(); + // If the closure body is a block with a single expression, suggest just the inner expression, + // not the block. Example: `(|| { Some(true) })()` should suggest + // `Some(true)` + if let ExprKind::Block(block, _) = body.kind + && block.stmts.is_empty() + && let Some(expr) = block.expr + { + hint = Sugg::hir_with_context(cx, expr, full_expr.span.ctxt(), "..", &mut applicability) + .maybe_paren(); } diag.span_suggestion(full_expr.span, "try doing something like", hint, applicability); diff --git a/clippy_lints/src/std_instead_of_core.rs b/clippy_lints/src/std_instead_of_core.rs index 442b3250d86..cf70e883bd0 100644 --- a/clippy_lints/src/std_instead_of_core.rs +++ b/clippy_lints/src/std_instead_of_core.rs @@ -1,10 +1,10 @@ use clippy_config::Conf; -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg}; use clippy_utils::is_from_proc_macro; use clippy_utils::msrvs::Msrv; use rustc_attr_data_structures::{StabilityLevel, StableSince}; use rustc_errors::Applicability; -use rustc_hir::def::Res; +use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::DefId; use rustc_hir::{Block, Body, HirId, Path, PathSegment}; use rustc_lint::{LateContext, LateLintPass, Lint, LintContext}; @@ -88,49 +88,52 @@ declare_clippy_lint! { } pub struct StdReexports { - lint_point: (Span, Option<LintPoint>), + lint_points: Option<(Span, Vec<LintPoint>)>, msrv: Msrv, } impl StdReexports { pub fn new(conf: &'static Conf) -> Self { Self { - lint_point: Default::default(), + lint_points: Option::default(), msrv: conf.msrv, } } - fn lint_if_finish(&mut self, cx: &LateContext<'_>, (span, item): (Span, Option<LintPoint>)) { - if span.source_equal(self.lint_point.0) { - return; + fn lint_if_finish(&mut self, cx: &LateContext<'_>, krate: Span, lint_point: LintPoint) { + match &mut self.lint_points { + Some((prev_krate, prev_lints)) if prev_krate.overlaps(krate) => { + prev_lints.push(lint_point); + }, + _ => emit_lints(cx, self.lint_points.replace((krate, vec![lint_point]))), } - - if !self.lint_point.0.is_dummy() { - emit_lints(cx, &self.lint_point); - } - - self.lint_point = (span, item); } } impl_lint_pass!(StdReexports => [STD_INSTEAD_OF_CORE, STD_INSTEAD_OF_ALLOC, ALLOC_INSTEAD_OF_CORE]); -type LintPoint = (&'static Lint, &'static str, &'static str); +#[derive(Debug)] +enum LintPoint { + Available(Span, &'static Lint, &'static str, &'static str), + Conflict, +} impl<'tcx> LateLintPass<'tcx> for StdReexports { fn check_path(&mut self, cx: &LateContext<'tcx>, path: &Path<'tcx>, _: HirId) { - if let Res::Def(_, def_id) = path.res + if let Res::Def(def_kind, def_id) = path.res && let Some(first_segment) = get_first_segment(path) && is_stable(cx, def_id, self.msrv) && !path.span.in_external_macro(cx.sess().source_map()) && !is_from_proc_macro(cx, &first_segment.ident) + && !matches!(def_kind, DefKind::Macro(_)) + && let Some(last_segment) = path.segments.last() { let (lint, used_mod, replace_with) = match first_segment.ident.name { sym::std => match cx.tcx.crate_name(def_id.krate) { sym::core => (STD_INSTEAD_OF_CORE, "std", "core"), sym::alloc => (STD_INSTEAD_OF_ALLOC, "std", "alloc"), _ => { - self.lint_if_finish(cx, (first_segment.ident.span, None)); + self.lint_if_finish(cx, first_segment.ident.span, LintPoint::Conflict); return; }, }, @@ -138,44 +141,84 @@ impl<'tcx> LateLintPass<'tcx> for StdReexports { if cx.tcx.crate_name(def_id.krate) == sym::core { (ALLOC_INSTEAD_OF_CORE, "alloc", "core") } else { - self.lint_if_finish(cx, (first_segment.ident.span, None)); + self.lint_if_finish(cx, first_segment.ident.span, LintPoint::Conflict); return; } }, - _ => return, + _ => { + self.lint_if_finish(cx, first_segment.ident.span, LintPoint::Conflict); + return; + }, }; - self.lint_if_finish(cx, (first_segment.ident.span, Some((lint, used_mod, replace_with)))); + self.lint_if_finish( + cx, + first_segment.ident.span, + LintPoint::Available(last_segment.ident.span, lint, used_mod, replace_with), + ); } } fn check_block_post(&mut self, cx: &LateContext<'tcx>, _: &Block<'tcx>) { - self.lint_if_finish(cx, Default::default()); + emit_lints(cx, self.lint_points.take()); } fn check_body_post(&mut self, cx: &LateContext<'tcx>, _: &Body<'tcx>) { - self.lint_if_finish(cx, Default::default()); + emit_lints(cx, self.lint_points.take()); } fn check_crate_post(&mut self, cx: &LateContext<'tcx>) { - self.lint_if_finish(cx, Default::default()); + emit_lints(cx, self.lint_points.take()); } } -fn emit_lints(cx: &LateContext<'_>, (span, item): &(Span, Option<LintPoint>)) { - let Some((lint, used_mod, replace_with)) = item else { +fn emit_lints(cx: &LateContext<'_>, lint_points: Option<(Span, Vec<LintPoint>)>) { + let Some((krate_span, lint_points)) = lint_points else { return; }; - span_lint_and_sugg( - cx, - lint, - *span, - format!("used import from `{used_mod}` instead of `{replace_with}`"), - format!("consider importing the item from `{replace_with}`"), - (*replace_with).to_string(), - Applicability::MachineApplicable, - ); + let mut lint: Option<(&'static Lint, &'static str, &'static str)> = None; + let mut has_conflict = false; + for lint_point in &lint_points { + match lint_point { + LintPoint::Available(_, l, used_mod, replace_with) + if lint.is_none_or(|(prev_l, ..)| l.name == prev_l.name) => + { + lint = Some((l, used_mod, replace_with)); + }, + _ => { + has_conflict = true; + break; + }, + } + } + + if !has_conflict && let Some((lint, used_mod, replace_with)) = lint { + span_lint_and_sugg( + cx, + lint, + krate_span, + format!("used import from `{used_mod}` instead of `{replace_with}`"), + format!("consider importing the item from `{replace_with}`"), + (*replace_with).to_string(), + Applicability::MachineApplicable, + ); + return; + } + + for lint_point in lint_points { + let LintPoint::Available(span, lint, used_mod, replace_with) = lint_point else { + continue; + }; + span_lint_and_help( + cx, + lint, + span, + format!("used import from `{used_mod}` instead of `{replace_with}`"), + None, + format!("consider importing the item from `{replace_with}`"), + ); + } } /// Returns the first named segment of a [`Path`]. diff --git a/clippy_lints/src/undocumented_unsafe_blocks.rs b/clippy_lints/src/undocumented_unsafe_blocks.rs index 6cc4b589a72..45c0d459d90 100644 --- a/clippy_lints/src/undocumented_unsafe_blocks.rs +++ b/clippy_lints/src/undocumented_unsafe_blocks.rs @@ -143,7 +143,8 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks { if let Some(tail) = block.expr && !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, tail.hir_id) && !tail.span.in_external_macro(cx.tcx.sess.source_map()) - && let HasSafetyComment::Yes(pos) = stmt_has_safety_comment(cx, tail.span, tail.hir_id) + && let HasSafetyComment::Yes(pos) = + stmt_has_safety_comment(cx, tail.span, tail.hir_id, self.accept_comment_above_attributes) && let Some(help_span) = expr_has_unnecessary_safety_comment(cx, tail, pos) { span_lint_and_then( @@ -167,7 +168,8 @@ impl<'tcx> LateLintPass<'tcx> for UndocumentedUnsafeBlocks { }; if !is_lint_allowed(cx, UNNECESSARY_SAFETY_COMMENT, stmt.hir_id) && !stmt.span.in_external_macro(cx.tcx.sess.source_map()) - && let HasSafetyComment::Yes(pos) = stmt_has_safety_comment(cx, stmt.span, stmt.hir_id) + && let HasSafetyComment::Yes(pos) = + stmt_has_safety_comment(cx, stmt.span, stmt.hir_id, self.accept_comment_above_attributes) && let Some(help_span) = expr_has_unnecessary_safety_comment(cx, expr, pos) { span_lint_and_then( @@ -540,7 +542,12 @@ fn item_has_safety_comment(cx: &LateContext<'_>, item: &hir::Item<'_>) -> HasSaf /// Checks if the lines immediately preceding the item contain a safety comment. #[allow(clippy::collapsible_match)] -fn stmt_has_safety_comment(cx: &LateContext<'_>, span: Span, hir_id: HirId) -> HasSafetyComment { +fn stmt_has_safety_comment( + cx: &LateContext<'_>, + span: Span, + hir_id: HirId, + accept_comment_above_attributes: bool, +) -> HasSafetyComment { match span_from_macro_expansion_has_safety_comment(cx, span) { HasSafetyComment::Maybe => (), has_safety_comment => return has_safety_comment, @@ -555,6 +562,13 @@ fn stmt_has_safety_comment(cx: &LateContext<'_>, span: Span, hir_id: HirId) -> H _ => return HasSafetyComment::Maybe, }; + // if span_with_attrs_has_safety_comment(cx, span, hir_id, accept_comment_above_attrib + // } + let mut span = span; + if accept_comment_above_attributes { + span = include_attrs_in_span(cx, hir_id, span); + } + let source_map = cx.sess().source_map(); if let Some(comment_start) = comment_start && let Ok(unsafe_line) = source_map.lookup_line(span.lo()) |