Skip to content

Commit e5163a3

Browse files
asdfishasdfish
committed
removed false positive and improved tests
1 parent 67dd19c commit e5163a3

File tree

4 files changed

+86
-31
lines changed

4 files changed

+86
-31
lines changed

‎clippy_lints/src/cloned_ref_to_slice_refs.rs

+5-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ use clippy_config::Conf;
22
use clippy_utils::diagnostics::span_lint_and_sugg;
33
use clippy_utils::msrvs::{self, Msrv};
44
use clippy_utils::sugg::Sugg;
5-
use clippy_utils::{is_in_const_context, is_trait_method};
5+
use clippy_utils::visitors::is_const_evaluatable;
6+
use clippy_utils::{is_in_const_context, is_mutable, is_trait_method};
67
use rustc_errors::Applicability;
78
use rustc_hir::{Expr, ExprKind};
89
use rustc_lint::{LateContext, LateLintPass};
@@ -63,7 +64,6 @@ impl<'tcx> LateLintPass<'tcx> for ClonedRefToSliceRefs<'_> {
6364
msrvs::SLICE_FROM_REF
6465
}
6566
})
66-
6767
// `&[foo.clone()]` expressions
6868
&& let ExprKind::AddrOf(_, mutability, arr) = &expr.kind
6969
// mutable references would have a different meaning
@@ -76,6 +76,9 @@ impl<'tcx> LateLintPass<'tcx> for ClonedRefToSliceRefs<'_> {
7676
&& let ExprKind::MethodCall(_, val, _, _) = item.kind
7777
&& is_trait_method(cx, item, sym::Clone)
7878

79+
// check for immutability or purity
80+
&& (!is_mutable(cx, val) || is_const_evaluatable(cx, val))
81+
7982
// get appropriate crate for `slice::from_ref`
8083
&& let Some(builtin_crate) = clippy_utils::std_or_core(cx)
8184
{

‎tests/ui/cloned_ref_to_slice_refs.fixed

+34-11
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,64 @@
11
#![warn(clippy::cloned_ref_to_slice_refs)]
22

3-
use std::sync::Arc;
4-
5-
fn take_slice(_data: &[Data]) {}
6-
fn take_slice_mut(_data: &mut [Data]) {}
7-
fn take_arc(_data: &[Arc<Data>]) {}
8-
93
#[derive(Clone)]
104
struct Data;
115

126
fn main() {
137
{
148
let data = Data;
159
let data_ref = &data;
16-
take_slice(std::slice::from_ref(data_ref)); //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
10+
let _ = std::slice::from_ref(data_ref); //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
11+
}
12+
13+
{
14+
let _ = std::slice::from_ref(&Data); //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
15+
}
16+
17+
{
18+
#[derive(Clone)]
19+
struct Point(i32, i32);
20+
21+
let _ = std::slice::from_ref(&Point(0, 0)); //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
22+
}
23+
24+
// the string was cloned with the intention to not mutate
25+
{
26+
struct BetterString(String);
27+
28+
let mut message = String::from("good");
29+
let sender = BetterString(message.clone());
30+
31+
message.push_str("bye!");
32+
33+
println!("{} {}", message, sender.0)
1734
}
35+
36+
// the string was cloned with the intention to not mutate
1837
{
19-
take_slice(std::slice::from_ref(&Data)); //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
38+
let mut x = String::from("Hello");
39+
let r = &[x.clone()];
40+
x.push('!');
41+
println!("r = `{}', x = `{x}'", r[0]);
2042
}
2143

2244
// mutable borrows may have the intention to clone
2345
{
2446
let data = Data;
2547
let data_ref = &data;
26-
take_slice_mut(&mut [data_ref.clone()]);
48+
let _ = &mut [data_ref.clone()];
2749
}
2850

2951
// `T::clone` is used to denote a clone with side effects
3052
{
53+
use std::sync::Arc;
3154
let data = Arc::new(Data);
32-
take_arc(&[Arc::clone(&data)]);
55+
let _ = &[Arc::clone(&data)];
3356
}
3457

3558
// slices with multiple members can only be made from a singular reference
3659
{
3760
let data_1 = Data;
3861
let data_2 = Data;
39-
take_slice(&[data_1.clone(), data_2.clone()]);
62+
let _ = &[data_1.clone(), data_2.clone()];
4063
}
4164
}

‎tests/ui/cloned_ref_to_slice_refs.rs

+34-11
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,64 @@
11
#![warn(clippy::cloned_ref_to_slice_refs)]
22

3-
use std::sync::Arc;
4-
5-
fn take_slice(_data: &[Data]) {}
6-
fn take_slice_mut(_data: &mut [Data]) {}
7-
fn take_arc(_data: &[Arc<Data>]) {}
8-
93
#[derive(Clone)]
104
struct Data;
115

126
fn main() {
137
{
148
let data = Data;
159
let data_ref = &data;
16-
take_slice(&[data_ref.clone()]); //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
10+
let _ = &[data_ref.clone()]; //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
11+
}
12+
13+
{
14+
let _ = &[Data.clone()]; //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
15+
}
16+
17+
{
18+
#[derive(Clone)]
19+
struct Point(i32, i32);
20+
21+
let _ = &[Point(0, 0).clone()]; //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
22+
}
23+
24+
// the string was cloned with the intention to not mutate
25+
{
26+
struct BetterString(String);
27+
28+
let mut message = String::from("good");
29+
let sender = BetterString(message.clone());
30+
31+
message.push_str("bye!");
32+
33+
println!("{} {}", message, sender.0)
1734
}
35+
36+
// the string was cloned with the intention to not mutate
1837
{
19-
take_slice(&[Data.clone()]); //~ ERROR: this call to `clone` can be replaced with `std::slice::from_ref`
38+
let mut x = String::from("Hello");
39+
let r = &[x.clone()];
40+
x.push('!');
41+
println!("r = `{}', x = `{x}'", r[0]);
2042
}
2143

2244
// mutable borrows may have the intention to clone
2345
{
2446
let data = Data;
2547
let data_ref = &data;
26-
take_slice_mut(&mut [data_ref.clone()]);
48+
let _ = &mut [data_ref.clone()];
2749
}
2850

2951
// `T::clone` is used to denote a clone with side effects
3052
{
53+
use std::sync::Arc;
3154
let data = Arc::new(Data);
32-
take_arc(&[Arc::clone(&data)]);
55+
let _ = &[Arc::clone(&data)];
3356
}
3457

3558
// slices with multiple members can only be made from a singular reference
3659
{
3760
let data_1 = Data;
3861
let data_2 = Data;
39-
take_slice(&[data_1.clone(), data_2.clone()]);
62+
let _ = &[data_1.clone(), data_2.clone()];
4063
}
4164
}

‎tests/ui/cloned_ref_to_slice_refs.stderr

+13-7
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,23 @@
11
error: this call to `clone` can be replaced with `std::slice::from_ref`
2-
--> tests/ui/cloned_ref_to_slice_refs.rs:16:20
2+
--> tests/ui/cloned_ref_to_slice_refs.rs:10:17
33
|
4-
LL | take_slice(&[data_ref.clone()]);
5-
| ^^^^^^^^^^^^^^^^^^^ help: try: `std::slice::from_ref(data_ref)`
4+
LL | let _ = &[data_ref.clone()];
5+
| ^^^^^^^^^^^^^^^^^^^ help: try: `std::slice::from_ref(data_ref)`
66
|
77
= note: `-D clippy::cloned-ref-to-slice-refs` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::cloned_ref_to_slice_refs)]`
99

1010
error: this call to `clone` can be replaced with `std::slice::from_ref`
11-
--> tests/ui/cloned_ref_to_slice_refs.rs:19:20
11+
--> tests/ui/cloned_ref_to_slice_refs.rs:14:17
1212
|
13-
LL | take_slice(&[Data.clone()]);
14-
| ^^^^^^^^^^^^^^^ help: try: `std::slice::from_ref(&Data)`
13+
LL | let _ = &[Data.clone()];
14+
| ^^^^^^^^^^^^^^^ help: try: `std::slice::from_ref(&Data)`
1515

16-
error: aborting due to 2 previous errors
16+
error: this call to `clone` can be replaced with `std::slice::from_ref`
17+
--> tests/ui/cloned_ref_to_slice_refs.rs:21:17
18+
|
19+
LL | let _ = &[Point(0, 0).clone()];
20+
| ^^^^^^^^^^^^^^^^^^^^^^ help: try: `std::slice::from_ref(&Point(0, 0))`
21+
22+
error: aborting due to 3 previous errors
1723

0 commit comments

Comments
 (0)