diff --git a/src/tools/clippy/clippy_lints/src/format_args.rs b/src/tools/clippy/clippy_lints/src/format_args.rs index 0c39aae9ca913..fd1e6700698ce 100644 --- a/src/tools/clippy/clippy_lints/src/format_args.rs +++ b/src/tools/clippy/clippy_lints/src/format_args.rs @@ -163,7 +163,7 @@ declare_clippy_lint! { /// nothing will be suggested, e.g. `println!("{0}={1}", var, 1+2)`. #[clippy::version = "1.66.0"] pub UNINLINED_FORMAT_ARGS, - style, + pedantic, "using non-inlined variables in `format!` calls" } diff --git a/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs b/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs index 4a61c223d2c1e..40aad03960c43 100644 --- a/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs +++ b/src/tools/clippy/clippy_lints/src/methods/manual_is_variant_and.rs @@ -1,22 +1,18 @@ 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; use clippy_utils::ty::is_type_diagnostic_item; use rustc_errors::Applicability; -use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; -use rustc_hir::{BinOpKind, Expr, ExprKind, QPath}; use rustc_lint::LateContext; -use rustc_middle::ty; -use rustc_span::{BytePos, Span, sym}; +use rustc_span::{Span, sym}; use super::MANUAL_IS_VARIANT_AND; -pub(super) fn check( +pub(super) fn check<'tcx>( cx: &LateContext<'_>, - expr: &Expr<'_>, - map_recv: &Expr<'_>, - map_arg: &Expr<'_>, + expr: &'tcx rustc_hir::Expr<'_>, + map_recv: &'tcx rustc_hir::Expr<'_>, + map_arg: &'tcx rustc_hir::Expr<'_>, map_span: Span, msrv: Msrv, ) { @@ -61,57 +57,3 @@ pub(super) fn check( Applicability::MachineApplicable, ); } - -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 { - format!( - "{}{before_map_snippet}{}{after_map_snippet}", - if op == BinOpKind::Eq { "" } else { "!" }, - if is_option { "is_some_and" } else { "is_ok_and" }, - ) - }, - Applicability::MachineApplicable, - ); - } -} - -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) - { - // 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 - && 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()) - && 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) - { - return emit_lint(cx, op.node, parent_expr, span, item == sym::Option); - } - } - } - } -} diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs index 347960e0003d7..6d3f59520a264 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mod.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs @@ -5242,7 +5242,6 @@ impl Methods { unused_enumerate_index::check(cx, expr, recv, m_arg); map_clone::check(cx, expr, recv, m_arg, self.msrv); map_with_unused_argument_over_ranges::check(cx, expr, recv, m_arg, self.msrv, span); - manual_is_variant_and::check_map(cx, expr); match method_call(recv) { Some((map_name @ (sym::iter | sym::into_iter), recv2, _, _, _)) => { iter_kv_map::check(cx, map_name, expr, recv2, m_arg, self.msrv); diff --git a/src/tools/clippy/clippy_lints/src/methods/return_and_then.rs b/src/tools/clippy/clippy_lints/src/methods/return_and_then.rs index df8544f92203e..91643b0dfefde 100644 --- a/src/tools/clippy/clippy_lints/src/methods/return_and_then.rs +++ b/src/tools/clippy/clippy_lints/src/methods/return_and_then.rs @@ -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::{is_expr_final_block_expr, peel_blocks}; 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 !is_expr_final_block_expr(cx.tcx, expr) { return; } @@ -55,24 +55,12 @@ 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); - 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(inner, false, inner_indent), - reindent_multiline("}", false, base_indent), - ) - } else { - format!( - "let {} = {}?;\n{}", - arg_snip, - recv_snip, - reindent_multiline(inner, false, indent_of(cx, expr.span)) - ) - }; + let sugg = format!( + "let {} = {}?;\n{}", + arg_snip, + recv_snip, + reindent_multiline(inner, false, indent_of(cx, expr.span)) + ); span_lint_and_sugg( cx, diff --git a/src/tools/clippy/clippy_lints/src/methods/swap_with_temporary.rs b/src/tools/clippy/clippy_lints/src/methods/swap_with_temporary.rs index de729fb343a34..e378cbd6ae0ad 100644 --- a/src/tools/clippy/clippy_lints/src/methods/swap_with_temporary.rs +++ b/src/tools/clippy/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/src/tools/clippy/tests/ui/manual_is_variant_and.fixed b/src/tools/clippy/tests/ui/manual_is_variant_and.fixed index 18a72188ab593..c9c184561dd69 100644 --- a/src/tools/clippy/tests/ui/manual_is_variant_and.fixed +++ b/src/tools/clippy/tests/ui/manual_is_variant_and.fixed @@ -4,44 +4,6 @@ #[macro_use] extern crate option_helpers; -struct Foo(T); - -impl Foo { - fn map bool>(self, mut f: F) -> Option { - Some(f(self.0)) - } -} - -fn foo() -> Option { - Some(true) -} - -macro_rules! some_true { - () => { - Some(true) - }; -} -macro_rules! some_false { - () => { - Some(false) - }; -} - -macro_rules! mac { - (some $e:expr) => { - Some($e) - }; - (some_map $e:expr) => { - Some($e).map(|x| x % 2 == 0) - }; - (map $e:expr) => { - $e.map(|x| x % 2 == 0) - }; - (eq $a:expr, $b:expr) => { - $a == $b - }; -} - #[rustfmt::skip] fn option_methods() { let opt = Some(1); @@ -59,15 +21,6 @@ fn option_methods() { let _ = opt .is_some_and(|x| x > 1); - let _ = Some(2).is_some_and(|x| x % 2 == 0); - //~^ manual_is_variant_and - let _ = Some(2).is_none_or(|x| x % 2 == 0); - //~^ manual_is_variant_and - let _ = Some(2).is_some_and(|x| x % 2 == 0); - //~^ manual_is_variant_and - let _ = Some(2).is_none_or(|x| x % 2 == 0); - //~^ manual_is_variant_and - // won't fix because the return type of the closure is not `bool` let _ = opt.map(|x| x + 1).unwrap_or_default(); @@ -75,14 +28,6 @@ fn option_methods() { let _ = opt2.is_some_and(char::is_alphanumeric); // should lint //~^ manual_is_variant_and let _ = opt_map!(opt2, |x| x == 'a').unwrap_or_default(); // should not lint - - // Should not lint. - let _ = Foo::(0).map(|x| x % 2 == 0) == Some(true); - let _ = Some(2).map(|x| x % 2 == 0) != foo(); - let _ = mac!(eq Some(2).map(|x| x % 2 == 0), Some(true)); - let _ = mac!(some 2).map(|x| x % 2 == 0) == Some(true); - let _ = mac!(some_map 2) == Some(true); - let _ = mac!(map Some(2)) == Some(true); } #[rustfmt::skip] @@ -96,13 +41,6 @@ fn result_methods() { }); let _ = res.is_ok_and(|x| x > 1); - let _ = Ok::(2).is_ok_and(|x| x % 2 == 0); - //~^ manual_is_variant_and - let _ = !Ok::(2).is_ok_and(|x| x % 2 == 0); - //~^ manual_is_variant_and - let _ = !Ok::(2).is_ok_and(|x| x % 2 == 0); - //~^ manual_is_variant_and - // won't fix because the return type of the closure is not `bool` let _ = res.map(|x| x + 1).unwrap_or_default(); diff --git a/src/tools/clippy/tests/ui/manual_is_variant_and.rs b/src/tools/clippy/tests/ui/manual_is_variant_and.rs index a92f7c0436959..52c7b56804ce2 100644 --- a/src/tools/clippy/tests/ui/manual_is_variant_and.rs +++ b/src/tools/clippy/tests/ui/manual_is_variant_and.rs @@ -4,44 +4,6 @@ #[macro_use] extern crate option_helpers; -struct Foo(T); - -impl Foo { - fn map bool>(self, mut f: F) -> Option { - Some(f(self.0)) - } -} - -fn foo() -> Option { - Some(true) -} - -macro_rules! some_true { - () => { - Some(true) - }; -} -macro_rules! some_false { - () => { - Some(false) - }; -} - -macro_rules! mac { - (some $e:expr) => { - Some($e) - }; - (some_map $e:expr) => { - Some($e).map(|x| x % 2 == 0) - }; - (map $e:expr) => { - $e.map(|x| x % 2 == 0) - }; - (eq $a:expr, $b:expr) => { - $a == $b - }; -} - #[rustfmt::skip] fn option_methods() { let opt = Some(1); @@ -65,15 +27,6 @@ fn option_methods() { //~^ manual_is_variant_and .unwrap_or_default(); - let _ = Some(2).map(|x| x % 2 == 0) == Some(true); - //~^ manual_is_variant_and - let _ = Some(2).map(|x| x % 2 == 0) != Some(true); - //~^ manual_is_variant_and - let _ = Some(2).map(|x| x % 2 == 0) == some_true!(); - //~^ manual_is_variant_and - let _ = Some(2).map(|x| x % 2 == 0) != some_false!(); - //~^ manual_is_variant_and - // won't fix because the return type of the closure is not `bool` let _ = opt.map(|x| x + 1).unwrap_or_default(); @@ -81,14 +34,6 @@ fn option_methods() { let _ = opt2.map(char::is_alphanumeric).unwrap_or_default(); // should lint //~^ manual_is_variant_and let _ = opt_map!(opt2, |x| x == 'a').unwrap_or_default(); // should not lint - - // Should not lint. - let _ = Foo::(0).map(|x| x % 2 == 0) == Some(true); - let _ = Some(2).map(|x| x % 2 == 0) != foo(); - let _ = mac!(eq Some(2).map(|x| x % 2 == 0), Some(true)); - let _ = mac!(some 2).map(|x| x % 2 == 0) == Some(true); - let _ = mac!(some_map 2) == Some(true); - let _ = mac!(map Some(2)) == Some(true); } #[rustfmt::skip] @@ -105,13 +50,6 @@ fn result_methods() { //~^ manual_is_variant_and .unwrap_or_default(); - let _ = Ok::(2).map(|x| x % 2 == 0) == Ok(true); - //~^ manual_is_variant_and - let _ = Ok::(2).map(|x| x % 2 == 0) != Ok(true); - //~^ manual_is_variant_and - let _ = Ok::(2).map(|x| x % 2 == 0) != Ok(true); - //~^ manual_is_variant_and - // won't fix because the return type of the closure is not `bool` let _ = res.map(|x| x + 1).unwrap_or_default(); diff --git a/src/tools/clippy/tests/ui/manual_is_variant_and.stderr b/src/tools/clippy/tests/ui/manual_is_variant_and.stderr index 1fb437a8bc744..a4fa500580d0a 100644 --- a/src/tools/clippy/tests/ui/manual_is_variant_and.stderr +++ b/src/tools/clippy/tests/ui/manual_is_variant_and.stderr @@ -1,5 +1,5 @@ error: called `map().unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:51:17 + --> tests/ui/manual_is_variant_and.rs:13:17 | LL | let _ = opt.map(|x| x > 1) | _________________^ @@ -11,7 +11,7 @@ LL | | .unwrap_or_default(); = help: to override `-D warnings` add `#[allow(clippy::manual_is_variant_and)]` error: called `map().unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:56:17 + --> tests/ui/manual_is_variant_and.rs:18:17 | LL | let _ = opt.map(|x| { | _________________^ @@ -30,13 +30,13 @@ LL ~ }); | error: called `map().unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:61:17 + --> tests/ui/manual_is_variant_and.rs:23:17 | LL | let _ = opt.map(|x| x > 1).unwrap_or_default(); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_some_and(|x| x > 1)` error: called `map().unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:64:10 + --> tests/ui/manual_is_variant_and.rs:26:10 | LL | .map(|x| x > 1) | __________^ @@ -44,38 +44,14 @@ LL | | LL | | .unwrap_or_default(); | |____________________________^ help: use: `is_some_and(|x| x > 1)` -error: called `.map() == Some()` - --> tests/ui/manual_is_variant_and.rs:68:13 - | -LL | let _ = Some(2).map(|x| x % 2 == 0) == Some(true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_some_and(|x| x % 2 == 0)` - -error: called `.map() != Some()` - --> tests/ui/manual_is_variant_and.rs:70:13 - | -LL | let _ = Some(2).map(|x| x % 2 == 0) != Some(true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_none_or(|x| x % 2 == 0)` - -error: called `.map() == Some()` - --> tests/ui/manual_is_variant_and.rs:72:13 - | -LL | let _ = Some(2).map(|x| x % 2 == 0) == some_true!(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_some_and(|x| x % 2 == 0)` - -error: called `.map() != Some()` - --> tests/ui/manual_is_variant_and.rs:74:13 - | -LL | let _ = Some(2).map(|x| x % 2 == 0) != some_false!(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Some(2).is_none_or(|x| x % 2 == 0)` - error: called `map().unwrap_or_default()` on an `Option` value - --> tests/ui/manual_is_variant_and.rs:81:18 + --> tests/ui/manual_is_variant_and.rs:34:18 | LL | let _ = opt2.map(char::is_alphanumeric).unwrap_or_default(); // should lint | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_some_and(char::is_alphanumeric)` error: called `map().unwrap_or_default()` on a `Result` value - --> tests/ui/manual_is_variant_and.rs:99:17 + --> tests/ui/manual_is_variant_and.rs:44:17 | LL | let _ = res.map(|x| { | _________________^ @@ -94,7 +70,7 @@ LL ~ }); | error: called `map().unwrap_or_default()` on a `Result` value - --> tests/ui/manual_is_variant_and.rs:104:17 + --> tests/ui/manual_is_variant_and.rs:49:17 | LL | let _ = res.map(|x| x > 1) | _________________^ @@ -102,29 +78,11 @@ LL | | LL | | .unwrap_or_default(); | |____________________________^ help: use: `is_ok_and(|x| x > 1)` -error: called `.map() == Ok()` - --> tests/ui/manual_is_variant_and.rs:108:13 - | -LL | let _ = Ok::(2).map(|x| x % 2 == 0) == Ok(true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `Ok::(2).is_ok_and(|x| x % 2 == 0)` - -error: called `.map() != Ok()` - --> tests/ui/manual_is_variant_and.rs:110:13 - | -LL | let _ = Ok::(2).map(|x| x % 2 == 0) != Ok(true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `!Ok::(2).is_ok_and(|x| x % 2 == 0)` - -error: called `.map() != Ok()` - --> tests/ui/manual_is_variant_and.rs:112:13 - | -LL | let _ = Ok::(2).map(|x| x % 2 == 0) != Ok(true); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `!Ok::(2).is_ok_and(|x| x % 2 == 0)` - error: called `map().unwrap_or_default()` on a `Result` value - --> tests/ui/manual_is_variant_and.rs:119:18 + --> tests/ui/manual_is_variant_and.rs:57:18 | LL | let _ = res2.map(char::is_alphanumeric).unwrap_or_default(); // should lint | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `is_ok_and(char::is_alphanumeric)` -error: aborting due to 15 previous errors +error: aborting due to 8 previous errors diff --git a/src/tools/clippy/tests/ui/return_and_then.fixed b/src/tools/clippy/tests/ui/return_and_then.fixed index 8d9481d159512..74efa14eeec81 100644 --- a/src/tools/clippy/tests/ui/return_and_then.fixed +++ b/src/tools/clippy/tests/ui/return_and_then.fixed @@ -67,60 +67,8 @@ fn main() { .first() // creates temporary reference .and_then(|x| test_opt_block(Some(*x))) } - - fn in_closure() -> bool { - let _ = || { - let x = Some("")?; - if x.len() > 2 { Some(3) } else { None } - //~^ return_and_then - }; - true - } - - fn with_return(shortcut: bool) -> Option { - if shortcut { - return { - let x = Some("")?; - if x.len() > 2 { Some(3) } else { None } - }; - //~^ return_and_then - }; - None - } - - fn with_return_multiline(shortcut: bool) -> Option { - if shortcut { - return { - let mut x = Some("")?; - let x = format!("{x}."); - if x.len() > 2 { Some(3) } else { None } - }; - //~^^^^ return_and_then - }; - None - } } fn gen_option(n: i32) -> Option { Some(n) } - -mod issue14781 { - fn foo(_: &str, _: (u32, u32)) -> Result<(u32, u32), ()> { - Ok((1, 1)) - } - - fn bug(_: Option<&str>) -> Result<(), ()> { - let year: Option<&str> = None; - let month: Option<&str> = None; - let day: Option<&str> = None; - - let _day = if let (Some(year), Some(month)) = (year, month) { - day.and_then(|day| foo(day, (1, 31)).ok()) - } else { - None - }; - - Ok(()) - } -} diff --git a/src/tools/clippy/tests/ui/return_and_then.rs b/src/tools/clippy/tests/ui/return_and_then.rs index beada921a9187..188dc57e588cf 100644 --- a/src/tools/clippy/tests/ui/return_and_then.rs +++ b/src/tools/clippy/tests/ui/return_and_then.rs @@ -63,55 +63,8 @@ fn main() { .first() // creates temporary reference .and_then(|x| test_opt_block(Some(*x))) } - - fn in_closure() -> bool { - let _ = || { - Some("").and_then(|x| if x.len() > 2 { Some(3) } else { None }) - //~^ return_and_then - }; - true - } - - fn with_return(shortcut: bool) -> Option { - if shortcut { - return Some("").and_then(|x| if x.len() > 2 { Some(3) } else { None }); - //~^ return_and_then - }; - None - } - - fn with_return_multiline(shortcut: bool) -> Option { - if shortcut { - return Some("").and_then(|mut x| { - let x = format!("{x}."); - if x.len() > 2 { Some(3) } else { None } - }); - //~^^^^ return_and_then - }; - None - } } fn gen_option(n: i32) -> Option { Some(n) } - -mod issue14781 { - fn foo(_: &str, _: (u32, u32)) -> Result<(u32, u32), ()> { - Ok((1, 1)) - } - - fn bug(_: Option<&str>) -> Result<(), ()> { - let year: Option<&str> = None; - let month: Option<&str> = None; - let day: Option<&str> = None; - - let _day = if let (Some(year), Some(month)) = (year, month) { - day.and_then(|day| foo(day, (1, 31)).ok()) - } else { - None - }; - - Ok(()) - } -} diff --git a/src/tools/clippy/tests/ui/return_and_then.stderr b/src/tools/clippy/tests/ui/return_and_then.stderr index 5feca88286057..a7acbe7b3401c 100644 --- a/src/tools/clippy/tests/ui/return_and_then.stderr +++ b/src/tools/clippy/tests/ui/return_and_then.stderr @@ -101,50 +101,5 @@ LL + })?; LL + if x.len() > 2 { Some(3) } else { None } | -error: use the `?` operator instead of an `and_then` call - --> tests/ui/return_and_then.rs:69:13 - | -LL | Some("").and_then(|x| if x.len() > 2 { Some(3) } else { None }) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try - | -LL ~ let x = Some("")?; -LL + if x.len() > 2 { Some(3) } else { None } - | - -error: use the `?` operator instead of an `and_then` call - --> tests/ui/return_and_then.rs:77:20 - | -LL | return Some("").and_then(|x| if x.len() > 2 { Some(3) } else { None }); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | -help: try - | -LL ~ return { -LL + let x = Some("")?; -LL + if x.len() > 2 { Some(3) } else { None } -LL ~ }; - | - -error: use the `?` operator instead of an `and_then` call - --> tests/ui/return_and_then.rs:85:20 - | -LL | return Some("").and_then(|mut x| { - | ____________________^ -LL | | let x = format!("{x}."); -LL | | if x.len() > 2 { Some(3) } else { None } -LL | | }); - | |______________^ - | -help: try - | -LL ~ return { -LL + let mut x = Some("")?; -LL + let x = format!("{x}."); -LL + if x.len() > 2 { Some(3) } else { None } -LL ~ }; - | - -error: aborting due to 10 previous errors +error: aborting due to 7 previous errors diff --git a/src/tools/clippy/tests/ui/swap_with_temporary.fixed b/src/tools/clippy/tests/ui/swap_with_temporary.fixed index 4007d998ba068..24c667dd47958 100644 --- a/src/tools/clippy/tests/ui/swap_with_temporary.fixed +++ b/src/tools/clippy/tests/ui/swap_with_temporary.fixed @@ -72,3 +72,49 @@ fn dont_lint_those(s: &mut S, v: &mut [String], w: Option<&mut String>) { swap(&mut s.t, v.get_mut(0).unwrap()); swap(w.unwrap(), &mut s.t); } + +fn issue15166() { + use std::sync::Mutex; + + struct A { + thing: Mutex>, + } + + impl A { + fn a(&self) { + let mut new_vec = vec![42]; + // Do not lint here, as neither `new_vec` nor the result of `.lock().unwrap()` are temporaries + swap(&mut new_vec, &mut self.thing.lock().unwrap()); + for v in new_vec { + // Do something with v + } + // Here `vec![42]` is temporary though, and a proper dereference will have to be used in the fix + *self.thing.lock().unwrap() = vec![42]; + //~^ ERROR: swapping with a temporary value is inefficient + } + } +} + +fn multiple_deref() { + let mut v1 = &mut &mut &mut vec![42]; + ***v1 = vec![]; + //~^ ERROR: swapping with a temporary value is inefficient + + struct Wrapper(T); + impl std::ops::Deref for Wrapper { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } + } + impl std::ops::DerefMut for Wrapper { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } + } + + use std::sync::Mutex; + let mut v1 = Mutex::new(Wrapper(Wrapper(vec![42]))); + *(*(*v1.lock().unwrap())) = vec![]; + //~^ ERROR: swapping with a temporary value is inefficient +} diff --git a/src/tools/clippy/tests/ui/swap_with_temporary.rs b/src/tools/clippy/tests/ui/swap_with_temporary.rs index d403c086c0f4f..8e35e6144d99a 100644 --- a/src/tools/clippy/tests/ui/swap_with_temporary.rs +++ b/src/tools/clippy/tests/ui/swap_with_temporary.rs @@ -72,3 +72,49 @@ fn dont_lint_those(s: &mut S, v: &mut [String], w: Option<&mut String>) { swap(&mut s.t, v.get_mut(0).unwrap()); swap(w.unwrap(), &mut s.t); } + +fn issue15166() { + use std::sync::Mutex; + + struct A { + thing: Mutex>, + } + + impl A { + fn a(&self) { + let mut new_vec = vec![42]; + // Do not lint here, as neither `new_vec` nor the result of `.lock().unwrap()` are temporaries + swap(&mut new_vec, &mut self.thing.lock().unwrap()); + for v in new_vec { + // Do something with v + } + // Here `vec![42]` is temporary though, and a proper dereference will have to be used in the fix + swap(&mut vec![42], &mut self.thing.lock().unwrap()); + //~^ ERROR: swapping with a temporary value is inefficient + } + } +} + +fn multiple_deref() { + let mut v1 = &mut &mut &mut vec![42]; + swap(&mut ***v1, &mut vec![]); + //~^ ERROR: swapping with a temporary value is inefficient + + struct Wrapper(T); + impl std::ops::Deref for Wrapper { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } + } + impl std::ops::DerefMut for Wrapper { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } + } + + use std::sync::Mutex; + let mut v1 = Mutex::new(Wrapper(Wrapper(vec![42]))); + swap(&mut vec![], &mut v1.lock().unwrap()); + //~^ ERROR: swapping with a temporary value is inefficient +} diff --git a/src/tools/clippy/tests/ui/swap_with_temporary.stderr b/src/tools/clippy/tests/ui/swap_with_temporary.stderr index 59355771a9648..c0ea592fb05a7 100644 --- a/src/tools/clippy/tests/ui/swap_with_temporary.stderr +++ b/src/tools/clippy/tests/ui/swap_with_temporary.stderr @@ -96,5 +96,41 @@ note: this expression returns a temporary value LL | swap(mac!(refmut y), &mut func()); | ^^^^^^ -error: aborting due to 8 previous errors +error: swapping with a temporary value is inefficient + --> tests/ui/swap_with_temporary.rs:92:13 + | +LL | swap(&mut vec![42], &mut self.thing.lock().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use assignment instead: `*self.thing.lock().unwrap() = vec![42]` + | +note: this expression returns a temporary value + --> tests/ui/swap_with_temporary.rs:92:23 + | +LL | swap(&mut vec![42], &mut self.thing.lock().unwrap()); + | ^^^^^^^^ + +error: swapping with a temporary value is inefficient + --> tests/ui/swap_with_temporary.rs:100:5 + | +LL | swap(&mut ***v1, &mut vec![]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use assignment instead: `***v1 = vec![]` + | +note: this expression returns a temporary value + --> tests/ui/swap_with_temporary.rs:100:27 + | +LL | swap(&mut ***v1, &mut vec![]); + | ^^^^^^ + +error: swapping with a temporary value is inefficient + --> tests/ui/swap_with_temporary.rs:118:5 + | +LL | swap(&mut vec![], &mut v1.lock().unwrap()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use assignment instead: `*(*(*v1.lock().unwrap())) = vec![]` + | +note: this expression returns a temporary value + --> tests/ui/swap_with_temporary.rs:118:15 + | +LL | swap(&mut vec![], &mut v1.lock().unwrap()); + | ^^^^^^ + +error: aborting due to 11 previous errors