From 145ebb1cd723e34dc5a940ec3e498de814e6293d Mon Sep 17 00:00:00 2001 From: Daniel Macovei Date: Tue, 2 Aug 2022 10:39:35 -0500 Subject: [PATCH 1/2] add paren before '?' when suggesting deref --- clippy_lints/src/methods/clone_on_copy.rs | 6 +++++- tests/ui/clone_on_copy.fixed | 6 +++++- tests/ui/clone_on_copy.rs | 6 +++++- tests/ui/clone_on_copy.stderr | 8 +++++++- 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/methods/clone_on_copy.rs b/clippy_lints/src/methods/clone_on_copy.rs index 0b38a07204e8..a813d39441b5 100644 --- a/clippy_lints/src/methods/clone_on_copy.rs +++ b/clippy_lints/src/methods/clone_on_copy.rs @@ -4,7 +4,7 @@ use clippy_utils::source::snippet_with_context; use clippy_utils::sugg; use clippy_utils::ty::is_copy; use rustc_errors::Applicability; -use rustc_hir::{BindingAnnotation, Expr, ExprKind, MatchSource, Node, PatKind}; +use rustc_hir::{BindingAnnotation, Expr, ExprKind, MatchSource, Node, PatKind, QPath}; use rustc_lint::LateContext; use rustc_middle::ty::{self, adjustment::Adjust}; use rustc_span::symbol::{sym, Symbol}; @@ -86,6 +86,10 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: Symbol, { return; }, + ExprKind::Call(hir_callee, _) => match hir_callee.kind { + ExprKind::Path(QPath::LangItem(rustc_hir::LangItem::TryTraitBranch, _, _)) => true, + _ => false, + }, ExprKind::MethodCall(_, [self_arg, ..], _) if expr.hir_id == self_arg.hir_id => true, ExprKind::Match(_, _, MatchSource::TryDesugar | MatchSource::AwaitDesugar) | ExprKind::Field(..) diff --git a/tests/ui/clone_on_copy.fixed b/tests/ui/clone_on_copy.fixed index dc062762604e..43849121b043 100644 --- a/tests/ui/clone_on_copy.fixed +++ b/tests/ui/clone_on_copy.fixed @@ -21,7 +21,7 @@ fn is_ascii(ch: char) -> bool { ch.is_ascii() } -fn clone_on_copy() { +fn clone_on_copy() -> Option<(i32)> { 42; vec![1].clone(); // ok, not a Copy type @@ -71,4 +71,8 @@ fn clone_on_copy() { // Issue #5436 let mut vec = Vec::new(); vec.push(42); + + let opt: &Option = &None; + let value = (*opt)?; + None } diff --git a/tests/ui/clone_on_copy.rs b/tests/ui/clone_on_copy.rs index 8c39d0d55dd8..1f10599da228 100644 --- a/tests/ui/clone_on_copy.rs +++ b/tests/ui/clone_on_copy.rs @@ -21,7 +21,7 @@ fn is_ascii(ch: char) -> bool { ch.is_ascii() } -fn clone_on_copy() { +fn clone_on_copy() -> Option<(i32)> { 42.clone(); vec![1].clone(); // ok, not a Copy type @@ -71,4 +71,8 @@ fn clone_on_copy() { // Issue #5436 let mut vec = Vec::new(); vec.push(42.clone()); + + let opt: &Option = &None; + let value = opt.clone()?; + None } diff --git a/tests/ui/clone_on_copy.stderr b/tests/ui/clone_on_copy.stderr index 861543d0aa90..483ea35af2a0 100644 --- a/tests/ui/clone_on_copy.stderr +++ b/tests/ui/clone_on_copy.stderr @@ -48,5 +48,11 @@ error: using `clone` on type `i32` which implements the `Copy` trait LL | vec.push(42.clone()); | ^^^^^^^^^^ help: try removing the `clone` call: `42` -error: aborting due to 8 previous errors +error: using `clone` on type `std::option::Option` which implements the `Copy` trait + --> $DIR/clone_on_copy.rs:76:17 + | +LL | let value = opt.clone()?; + | ^^^^^^^^^^^ help: try dereferencing it: `(*opt)` + +error: aborting due to 9 previous errors From 503c03c5584bb697071f3585953ed6e5b16d2e10 Mon Sep 17 00:00:00 2001 From: Daniel Macovei Date: Tue, 2 Aug 2022 12:06:22 -0500 Subject: [PATCH 2/2] clean up --- clippy_lints/src/methods/clone_on_copy.rs | 9 +++++---- tests/ui/clone_on_copy.fixed | 3 ++- tests/ui/clone_on_copy.rs | 3 ++- tests/ui/clone_on_copy.stderr | 4 ++-- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/clippy_lints/src/methods/clone_on_copy.rs b/clippy_lints/src/methods/clone_on_copy.rs index a813d39441b5..60e1355f9b92 100644 --- a/clippy_lints/src/methods/clone_on_copy.rs +++ b/clippy_lints/src/methods/clone_on_copy.rs @@ -86,10 +86,11 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: Symbol, { return; }, - ExprKind::Call(hir_callee, _) => match hir_callee.kind { - ExprKind::Path(QPath::LangItem(rustc_hir::LangItem::TryTraitBranch, _, _)) => true, - _ => false, - }, + // ? is a Call, makes sure not to rec *x?, but rather (*x)? + ExprKind::Call(hir_callee, _) => matches!( + hir_callee.kind, + ExprKind::Path(QPath::LangItem(rustc_hir::LangItem::TryTraitBranch, _, _)) + ), ExprKind::MethodCall(_, [self_arg, ..], _) if expr.hir_id == self_arg.hir_id => true, ExprKind::Match(_, _, MatchSource::TryDesugar | MatchSource::AwaitDesugar) | ExprKind::Field(..) diff --git a/tests/ui/clone_on_copy.fixed b/tests/ui/clone_on_copy.fixed index 43849121b043..72b122270981 100644 --- a/tests/ui/clone_on_copy.fixed +++ b/tests/ui/clone_on_copy.fixed @@ -72,7 +72,8 @@ fn clone_on_copy() -> Option<(i32)> { let mut vec = Vec::new(); vec.push(42); + // Issue #9277 let opt: &Option = &None; - let value = (*opt)?; + let value = (*opt)?; // operator precedence needed (*opt)? None } diff --git a/tests/ui/clone_on_copy.rs b/tests/ui/clone_on_copy.rs index 1f10599da228..03e210ebad98 100644 --- a/tests/ui/clone_on_copy.rs +++ b/tests/ui/clone_on_copy.rs @@ -72,7 +72,8 @@ fn clone_on_copy() -> Option<(i32)> { let mut vec = Vec::new(); vec.push(42.clone()); + // Issue #9277 let opt: &Option = &None; - let value = opt.clone()?; + let value = opt.clone()?; // operator precedence needed (*opt)? None } diff --git a/tests/ui/clone_on_copy.stderr b/tests/ui/clone_on_copy.stderr index 483ea35af2a0..42ae227777c7 100644 --- a/tests/ui/clone_on_copy.stderr +++ b/tests/ui/clone_on_copy.stderr @@ -49,9 +49,9 @@ LL | vec.push(42.clone()); | ^^^^^^^^^^ help: try removing the `clone` call: `42` error: using `clone` on type `std::option::Option` which implements the `Copy` trait - --> $DIR/clone_on_copy.rs:76:17 + --> $DIR/clone_on_copy.rs:77:17 | -LL | let value = opt.clone()?; +LL | let value = opt.clone()?; // operator precedence needed (*opt)? | ^^^^^^^^^^^ help: try dereferencing it: `(*opt)` error: aborting due to 9 previous errors