Skip to content

Commit 7437998

Browse files
cyrganicyrgani
committed
noop_method_call: remove nonlocal derive suggestion, add suggestion to derive on generic argument
1 parent 7cb0849 commit 7437998

File tree

4 files changed

+140
-9
lines changed

4 files changed

+140
-9
lines changed

‎compiler/rustc_lint/src/noop_method_call.rs

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use rustc_hir::def::DefKind;
22
use rustc_hir::{Expr, ExprKind};
33
use rustc_middle::ty;
4+
use rustc_middle::ty::ClauseKind;
45
use rustc_middle::ty::adjustment::Adjust;
56
use rustc_session::{declare_lint, declare_lint_pass};
67
use rustc_span::sym;
@@ -125,7 +126,32 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
125126

126127
if receiver_ty == expr_ty {
127128
let suggest_derive = match orig_ty.kind() {
128-
ty::Adt(def, _) => Some(cx.tcx.def_span(def.did()).shrink_to_lo()),
129+
ty::Adt(def, args) => {
130+
if def.did().is_local() {
131+
Some(cx.tcx.def_span(def.did()).shrink_to_lo())
132+
} else if let Some(trait_impl_id) = cx.tcx.impl_of_method(i.def_id()) {
133+
// If the type is generic over `T`, implements `Clone` if `T` does
134+
// and the concrete `T` is local, suggest deriving `Clone` for `T` rather than the type.
135+
let predicates = cx.tcx.predicates_of(trait_impl_id);
136+
137+
if predicates.predicates.into_iter().all(|&(predicate, _)| {
138+
let ClauseKind::Trait(trait_predicate) = predicate.kind().skip_binder()
139+
else {
140+
return true;
141+
};
142+
cx.tcx.is_diagnostic_item(sym::Clone, trait_predicate.trait_ref.def_id)
143+
}) {
144+
args.iter().find_map(|arg| {
145+
let did = arg.as_type()?.ty_adt_def()?.did();
146+
if did.is_local() { Some(cx.tcx.def_span(did)) } else { None }
147+
})
148+
} else {
149+
None
150+
}
151+
} else {
152+
None
153+
}
154+
}
129155
_ => None,
130156
};
131157
cx.emit_span_lint(NOOP_METHOD_CALL, span, NoopMethodCallDiag {

‎tests/ui/lint/auxiliary/non_clone_types.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
pub struct NotClone;
2+
3+
pub struct IsClone;
4+
5+
impl Clone for IsClone {
6+
fn clone(&self) -> Self {
7+
Self
8+
}
9+
}
10+
11+
pub struct ConditionalClone<T>(T);
12+
13+
impl<T: Clone> Clone for ConditionalClone<T> {
14+
fn clone(&self) -> Self {
15+
Self(self.0.clone())
16+
}
17+
}
18+
19+
pub struct DifferentlyConditionalClone<T>(T);
20+
21+
impl<T: Default> Clone for DifferentlyConditionalClone<T> {
22+
fn clone(&self) -> Self {
23+
Self(T::default())
24+
}
25+
}

‎tests/ui/lint/noop-method-call.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
//@ check-pass
2+
//@ aux-build:non_clone_types.rs
23

34
#![feature(rustc_attrs)]
45
#![allow(unused)]
56

7+
extern crate non_clone_types;
8+
use non_clone_types::*;
9+
610
use std::borrow::Borrow;
711
use std::ops::Deref;
812

@@ -61,3 +65,27 @@ impl Clone for DiagnosticClone {
6165
fn with_other_diagnostic_item(x: DiagnosticClone) {
6266
x.clone();
6367
}
68+
69+
fn with_foreign_type(v: &NotClone) {
70+
v.clone();
71+
//~^ WARN call to `.clone()` on a reference in this situation does nothing
72+
}
73+
74+
fn with_foreign_generic_type(v: &ConditionalClone<PlainType<u32>>) {
75+
v.clone();
76+
//~^ WARN call to `.clone()` on a reference in this situation does nothing
77+
}
78+
79+
fn with_only_foreign_types_1(v: &ConditionalClone<NotClone>) {
80+
v.clone();
81+
//~^ WARN call to `.clone()` on a reference in this situation does nothing
82+
}
83+
84+
fn with_only_foreign_types_2(v: &ConditionalClone<IsClone>) {
85+
v.clone();
86+
}
87+
88+
fn different_impl_bound(v: &DifferentlyConditionalClone<PlainType<u8>>) {
89+
v.clone();
90+
//~^ WARN call to `.clone()` on a reference in this situation does nothing
91+
}

‎tests/ui/lint/noop-method-call.stderr

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
warning: call to `.clone()` on a reference in this situation does nothing
2-
--> $DIR/noop-method-call.rs:15:25
2+
--> $DIR/noop-method-call.rs:19:25
33
|
44
LL | let _ = &mut encoded.clone();
55
| ^^^^^^^^ help: remove this redundant call
@@ -8,15 +8,15 @@ LL | let _ = &mut encoded.clone();
88
= note: `#[warn(noop_method_call)]` on by default
99

1010
warning: call to `.clone()` on a reference in this situation does nothing
11-
--> $DIR/noop-method-call.rs:17:21
11+
--> $DIR/noop-method-call.rs:21:21
1212
|
1313
LL | let _ = &encoded.clone();
1414
| ^^^^^^^^ help: remove this redundant call
1515
|
1616
= note: the type `[u8]` does not implement `Clone`, so calling `clone` on `&[u8]` copies the reference, which does not do anything and can be removed
1717

1818
warning: call to `.clone()` on a reference in this situation does nothing
19-
--> $DIR/noop-method-call.rs:23:71
19+
--> $DIR/noop-method-call.rs:27:71
2020
|
2121
LL | let non_clone_type_ref_clone: &PlainType<u32> = non_clone_type_ref.clone();
2222
| ^^^^^^^^
@@ -34,7 +34,7 @@ LL | struct PlainType<T>(T);
3434
|
3535

3636
warning: call to `.deref()` on a reference in this situation does nothing
37-
--> $DIR/noop-method-call.rs:31:63
37+
--> $DIR/noop-method-call.rs:35:63
3838
|
3939
LL | let non_deref_type_deref: &PlainType<u32> = non_deref_type.deref();
4040
| ^^^^^^^^
@@ -52,7 +52,7 @@ LL | struct PlainType<T>(T);
5252
|
5353

5454
warning: call to `.borrow()` on a reference in this situation does nothing
55-
--> $DIR/noop-method-call.rs:35:66
55+
--> $DIR/noop-method-call.rs:39:66
5656
|
5757
LL | let non_borrow_type_borrow: &PlainType<u32> = non_borrow_type.borrow();
5858
| ^^^^^^^^^
@@ -70,7 +70,7 @@ LL | struct PlainType<T>(T);
7070
|
7171

7272
warning: call to `.clone()` on a reference in this situation does nothing
73-
--> $DIR/noop-method-call.rs:44:19
73+
--> $DIR/noop-method-call.rs:48:19
7474
|
7575
LL | non_clone_type.clone();
7676
| ^^^^^^^^
@@ -88,7 +88,7 @@ LL | struct PlainType<T>(T);
8888
|
8989

9090
warning: call to `.clone()` on a reference in this situation does nothing
91-
--> $DIR/noop-method-call.rs:49:19
91+
--> $DIR/noop-method-call.rs:53:19
9292
|
9393
LL | non_clone_type.clone();
9494
| ^^^^^^^^
@@ -105,5 +105,57 @@ LL + #[derive(Clone)]
105105
LL | struct PlainType<T>(T);
106106
|
107107

108-
warning: 7 warnings emitted
108+
warning: call to `.clone()` on a reference in this situation does nothing
109+
--> $DIR/noop-method-call.rs:70:6
110+
|
111+
LL | v.clone();
112+
| ^^^^^^^^ help: remove this redundant call
113+
|
114+
= note: the type `non_clone_types::NotClone` does not implement `Clone`, so calling `clone` on `&non_clone_types::NotClone` copies the reference, which does not do anything and can be removed
115+
116+
warning: call to `.clone()` on a reference in this situation does nothing
117+
--> $DIR/noop-method-call.rs:75:6
118+
|
119+
LL | v.clone();
120+
| ^^^^^^^^
121+
|
122+
= note: the type `non_clone_types::ConditionalClone<PlainType<u32>>` does not implement `Clone`, so calling `clone` on `&non_clone_types::ConditionalClone<PlainType<u32>>` copies the reference, which does not do anything and can be removed
123+
help: remove this redundant call
124+
|
125+
LL - v.clone();
126+
LL + v;
127+
|
128+
help: if you meant to clone `non_clone_types::ConditionalClone<PlainType<u32>>`, implement `Clone` for it
129+
|
130+
LL + #[derive(Clone)]
131+
LL | struct PlainType<T>(T);
132+
|
133+
134+
warning: call to `.clone()` on a reference in this situation does nothing
135+
--> $DIR/noop-method-call.rs:80:6
136+
|
137+
LL | v.clone();
138+
| ^^^^^^^^ help: remove this redundant call
139+
|
140+
= note: the type `non_clone_types::ConditionalClone<non_clone_types::NotClone>` does not implement `Clone`, so calling `clone` on `&non_clone_types::ConditionalClone<non_clone_types::NotClone>` copies the reference, which does not do anything and can be removed
141+
142+
warning: call to `.clone()` on a reference in this situation does nothing
143+
--> $DIR/noop-method-call.rs:89:6
144+
|
145+
LL | v.clone();
146+
| ^^^^^^^^
147+
|
148+
= note: the type `non_clone_types::DifferentlyConditionalClone<PlainType<u8>>` does not implement `Clone`, so calling `clone` on `&non_clone_types::DifferentlyConditionalClone<PlainType<u8>>` copies the reference, which does not do anything and can be removed
149+
help: remove this redundant call
150+
|
151+
LL - v.clone();
152+
LL + v;
153+
|
154+
help: if you meant to clone `non_clone_types::DifferentlyConditionalClone<PlainType<u8>>`, implement `Clone` for it
155+
|
156+
LL + #[derive(Clone)]
157+
LL | struct PlainType<T>(T);
158+
|
159+
160+
warning: 11 warnings emitted
109161

0 commit comments

Comments
 (0)