Skip to content

Commit 692f53f

Browse files
borsbors
committed
Auto merge of rust-lang#12136 - y21:issue12135, r=Jarcho
[`useless_asref`]: check that the clone receiver is the parameter Fixes rust-lang#12135 There was no check for the receiver of the `clone` call in the map closure. This makes sure that it's a path to the parameter. changelog: [`useless_asref`]: check that the clone receiver is the closure parameter
2 parents d6ff2d2 + be5707c commit 692f53f

File tree

4 files changed

+110
-6
lines changed

4 files changed

+110
-6
lines changed

Diff for: ‎clippy_lints/src/methods/useless_asref.rs

+13-5
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::source::snippet_with_applicability;
33
use clippy_utils::ty::walk_ptrs_ty_depth;
4-
use clippy_utils::{get_parent_expr, is_diag_trait_item, match_def_path, paths, peel_blocks};
4+
use clippy_utils::{
5+
get_parent_expr, is_diag_trait_item, match_def_path, path_to_local_id, paths, peel_blocks, strip_pat_refs,
6+
};
57
use rustc_errors::Applicability;
68
use rustc_hir as hir;
79
use rustc_lint::LateContext;
@@ -108,9 +110,12 @@ fn check_qpath(cx: &LateContext<'_>, qpath: hir::QPath<'_>, hir_id: hir::HirId)
108110

109111
fn is_calling_clone(cx: &LateContext<'_>, arg: &hir::Expr<'_>) -> bool {
110112
match arg.kind {
111-
hir::ExprKind::Closure(&hir::Closure { body, .. }) => {
113+
hir::ExprKind::Closure(&hir::Closure { body, .. })
112114
// If it's a closure, we need to check what is called.
113-
let closure_body = cx.tcx.hir().body(body);
115+
if let closure_body = cx.tcx.hir().body(body)
116+
&& let [param] = closure_body.params
117+
&& let hir::PatKind::Binding(_, local_id, ..) = strip_pat_refs(param.pat).kind =>
118+
{
114119
let closure_expr = peel_blocks(closure_body.value);
115120
match closure_expr.kind {
116121
hir::ExprKind::MethodCall(method, obj, [], _) => {
@@ -122,14 +127,17 @@ fn is_calling_clone(cx: &LateContext<'_>, arg: &hir::Expr<'_>) -> bool {
122127
// no autoderefs
123128
&& !cx.typeck_results().expr_adjustments(obj).iter()
124129
.any(|a| matches!(a.kind, Adjust::Deref(Some(..))))
130+
&& path_to_local_id(obj, local_id)
125131
{
126132
true
127133
} else {
128134
false
129135
}
130136
},
131-
hir::ExprKind::Call(call, [_]) => {
132-
if let hir::ExprKind::Path(qpath) = call.kind {
137+
hir::ExprKind::Call(call, [recv]) => {
138+
if let hir::ExprKind::Path(qpath) = call.kind
139+
&& path_to_local_id(recv, local_id)
140+
{
133141
check_qpath(cx, qpath, call.hir_id)
134142
} else {
135143
false

Diff for: ‎tests/ui/useless_asref.fixed

+36
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,42 @@ fn foo() {
144144
//~^ ERROR: this call to `as_ref.map(...)` does nothing
145145
}
146146

147+
mod issue12135 {
148+
pub struct Struct {
149+
field: Option<InnerStruct>,
150+
}
151+
152+
#[derive(Clone)]
153+
pub struct Foo;
154+
155+
#[derive(Clone)]
156+
struct InnerStruct {
157+
x: Foo,
158+
}
159+
160+
impl InnerStruct {
161+
fn method(&self) -> &Foo {
162+
&self.x
163+
}
164+
}
165+
166+
pub fn f(x: &Struct) -> Option<Foo> {
167+
x.field.clone();
168+
//~^ ERROR: this call to `as_ref.map(...)` does nothing
169+
x.field.clone();
170+
//~^ ERROR: this call to `as_ref.map(...)` does nothing
171+
x.field.clone();
172+
//~^ ERROR: this call to `as_ref.map(...)` does nothing
173+
174+
// https://github.com/rust-lang/rust-clippy/pull/12136#discussion_r1451565223
175+
#[allow(clippy::clone_on_copy)]
176+
Some(1).clone();
177+
//~^ ERROR: this call to `as_ref.map(...)` does nothing
178+
179+
x.field.as_ref().map(|v| v.method().clone())
180+
}
181+
}
182+
147183
fn main() {
148184
not_ok();
149185
ok();

Diff for: ‎tests/ui/useless_asref.rs

+36
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,42 @@ fn foo() {
144144
//~^ ERROR: this call to `as_ref.map(...)` does nothing
145145
}
146146

147+
mod issue12135 {
148+
pub struct Struct {
149+
field: Option<InnerStruct>,
150+
}
151+
152+
#[derive(Clone)]
153+
pub struct Foo;
154+
155+
#[derive(Clone)]
156+
struct InnerStruct {
157+
x: Foo,
158+
}
159+
160+
impl InnerStruct {
161+
fn method(&self) -> &Foo {
162+
&self.x
163+
}
164+
}
165+
166+
pub fn f(x: &Struct) -> Option<Foo> {
167+
x.field.as_ref().map(|v| v.clone());
168+
//~^ ERROR: this call to `as_ref.map(...)` does nothing
169+
x.field.as_ref().map(Clone::clone);
170+
//~^ ERROR: this call to `as_ref.map(...)` does nothing
171+
x.field.as_ref().map(|v| Clone::clone(v));
172+
//~^ ERROR: this call to `as_ref.map(...)` does nothing
173+
174+
// https://github.com/rust-lang/rust-clippy/pull/12136#discussion_r1451565223
175+
#[allow(clippy::clone_on_copy)]
176+
Some(1).as_ref().map(|&x| x.clone());
177+
//~^ ERROR: this call to `as_ref.map(...)` does nothing
178+
179+
x.field.as_ref().map(|v| v.method().clone())
180+
}
181+
}
182+
147183
fn main() {
148184
not_ok();
149185
ok();

Diff for: ‎tests/ui/useless_asref.stderr

+25-1
Original file line numberDiff line numberDiff line change
@@ -88,5 +88,29 @@ error: this call to `as_ref.map(...)` does nothing
8888
LL | let z = x.as_ref().map(|z| String::clone(z));
8989
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.clone()`
9090

91-
error: aborting due to 14 previous errors
91+
error: this call to `as_ref.map(...)` does nothing
92+
--> $DIR/useless_asref.rs:167:9
93+
|
94+
LL | x.field.as_ref().map(|v| v.clone());
95+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.field.clone()`
96+
97+
error: this call to `as_ref.map(...)` does nothing
98+
--> $DIR/useless_asref.rs:169:9
99+
|
100+
LL | x.field.as_ref().map(Clone::clone);
101+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.field.clone()`
102+
103+
error: this call to `as_ref.map(...)` does nothing
104+
--> $DIR/useless_asref.rs:171:9
105+
|
106+
LL | x.field.as_ref().map(|v| Clone::clone(v));
107+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `x.field.clone()`
108+
109+
error: this call to `as_ref.map(...)` does nothing
110+
--> $DIR/useless_asref.rs:176:9
111+
|
112+
LL | Some(1).as_ref().map(|&x| x.clone());
113+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(1).clone()`
114+
115+
error: aborting due to 18 previous errors
92116

0 commit comments

Comments
 (0)