Skip to content

Commit 4734ac0

Browse files
borsbors
committed
Auto merge of #111916 - fee1-dead-contrib:noop-method-call-warn, r=compiler-errors
make `noop_method_call` warn by default r? `@compiler-errors`
2 parents ca1f813 + 2a76c57 commit 4734ac0

File tree

17 files changed

+161
-104
lines changed

17 files changed

+161
-104
lines changed

Diff for: ‎compiler/rustc_lint/messages.ftl

+2-2
Original file line numberDiff line numberDiff line change
@@ -410,8 +410,8 @@ lint_non_upper_case_global = {$sort} `{$name}` should have an upper case name
410410
.label = should have an UPPER_CASE name
411411
412412
lint_noop_method_call = call to `.{$method}()` on a reference in this situation does nothing
413-
.label = unnecessary method call
414-
.note = the type `{$receiver_ty}` which `{$method}` is being called on is the same as the type returned from `{$method}`, so the method call does not do anything and can be removed
413+
.suggestion = remove this redundant call
414+
.note = the type `{$orig_ty}` does not implement `{$trait_}`, so calling `{$method}` on `&{$orig_ty}` copies the reference, which does not do anything and can be removed
415415
416416
lint_only_cast_u8_to_char = only `u8` can be cast into `char`
417417
.suggestion = use a `char` literal instead

Diff for: ‎compiler/rustc_lint/src/lints.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -1231,8 +1231,9 @@ pub enum NonUpperCaseGlobalSub {
12311231
#[note]
12321232
pub struct NoopMethodCallDiag<'a> {
12331233
pub method: Symbol,
1234-
pub receiver_ty: Ty<'a>,
1235-
#[label]
1234+
pub orig_ty: Ty<'a>,
1235+
pub trait_: Symbol,
1236+
#[suggestion(code = "", applicability = "machine-applicable")]
12361237
pub label: Span,
12371238
}
12381239

Diff for: ‎compiler/rustc_lint/src/noop_method_call.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ declare_lint! {
1818
///
1919
/// ```rust
2020
/// # #![allow(unused)]
21-
/// #![warn(noop_method_call)]
2221
/// struct Foo;
2322
/// let foo = &Foo;
2423
/// let clone: &Foo = foo.clone();
@@ -34,7 +33,7 @@ declare_lint! {
3433
/// calling `clone` on a `&T` where `T` does not implement clone, actually doesn't do anything
3534
/// as references are copy. This lint detects these calls and warns the user about them.
3635
pub NOOP_METHOD_CALL,
37-
Allow,
36+
Warn,
3837
"detects the use of well-known noop methods"
3938
}
4039

@@ -86,10 +85,9 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
8685

8786
let Some(trait_id) = cx.tcx.trait_of_item(did) else { return };
8887

89-
if !matches!(
90-
cx.tcx.get_diagnostic_name(trait_id),
91-
Some(sym::Borrow | sym::Clone | sym::Deref)
92-
) {
88+
let Some(trait_) = cx.tcx.get_diagnostic_name(trait_id) else { return };
89+
90+
if !matches!(trait_, sym::Borrow | sym::Clone | sym::Deref) {
9391
return;
9492
};
9593

@@ -114,11 +112,13 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
114112
let expr_span = expr.span;
115113
let span = expr_span.with_lo(receiver.span.hi());
116114

115+
let orig_ty = expr_ty.peel_refs();
116+
117117
if receiver_ty == expr_ty {
118118
cx.emit_spanned_lint(
119119
NOOP_METHOD_CALL,
120120
span,
121-
NoopMethodCallDiag { method: call.ident.name, receiver_ty, label: span },
121+
NoopMethodCallDiag { method: call.ident.name, orig_ty, trait_, label: span },
122122
);
123123
} else {
124124
match name {

Diff for: ‎compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ use rustc_span::{BytePos, DesugaringKind, ExpnKind, MacroKind, Span, DUMMY_SP};
4141
use rustc_target::spec::abi;
4242
use std::borrow::Cow;
4343
use std::iter;
44-
use std::ops::Deref;
4544

4645
use super::InferCtxtPrivExt;
4746
use crate::infer::InferCtxtExt as _;
@@ -3577,7 +3576,7 @@ impl<'tcx> TypeErrCtxtExt<'tcx> for TypeErrCtxt<'_, 'tcx> {
35773576
// to an associated type (as seen from `trait_pred`) in the predicate. Like in
35783577
// trait_pred `S: Sum<<Self as Iterator>::Item>` and predicate `i32: Sum<&()>`
35793578
let mut type_diffs = vec![];
3580-
if let ObligationCauseCode::ExprBindingObligation(def_id, _, _, idx) = parent_code.deref()
3579+
if let ObligationCauseCode::ExprBindingObligation(def_id, _, _, idx) = parent_code
35813580
&& let Some(node_args) = typeck_results.node_args_opt(call_hir_id)
35823581
&& let where_clauses = self.tcx.predicates_of(def_id).instantiate(self.tcx, node_args)
35833582
&& let Some(where_pred) = where_clauses.predicates.get(*idx)

Diff for: ‎library/core/benches/iter.rs

+1
Original file line numberDiff line numberDiff line change
@@ -473,6 +473,7 @@ fn bench_next_chunk_copied(b: &mut Bencher) {
473473

474474
/// Exercises the TrustedRandomAccess specialization in ArrayChunks
475475
#[bench]
476+
#[allow(noop_method_call)]
476477
fn bench_next_chunk_trusted_random_access(b: &mut Bencher) {
477478
let v = vec![1u8; 1024];
478479

Diff for: ‎src/tools/clippy/tests/ui/explicit_deref_methods.fixed

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#![allow(
55
clippy::borrow_deref_ref,
66
suspicious_double_ref_op,
7+
noop_method_call,
78
clippy::explicit_auto_deref,
89
clippy::needless_borrow,
910
clippy::no_effect,

Diff for: ‎src/tools/clippy/tests/ui/explicit_deref_methods.rs

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#![allow(
55
clippy::borrow_deref_ref,
66
suspicious_double_ref_op,
7+
noop_method_call,
78
clippy::explicit_auto_deref,
89
clippy::needless_borrow,
910
clippy::no_effect,

Diff for: ‎src/tools/clippy/tests/ui/explicit_deref_methods.stderr

+12-12
Original file line numberDiff line numberDiff line change
@@ -1,73 +1,73 @@
11
error: explicit `deref` method call
2-
--> $DIR/explicit_deref_methods.rs:54:19
2+
--> $DIR/explicit_deref_methods.rs:55:19
33
|
44
LL | let b: &str = a.deref();
55
| ^^^^^^^^^ help: try: `&*a`
66
|
77
= note: `-D clippy::explicit-deref-methods` implied by `-D warnings`
88

99
error: explicit `deref_mut` method call
10-
--> $DIR/explicit_deref_methods.rs:56:23
10+
--> $DIR/explicit_deref_methods.rs:57:23
1111
|
1212
LL | let b: &mut str = a.deref_mut();
1313
| ^^^^^^^^^^^^^ help: try: `&mut **a`
1414

1515
error: explicit `deref` method call
16-
--> $DIR/explicit_deref_methods.rs:59:39
16+
--> $DIR/explicit_deref_methods.rs:60:39
1717
|
1818
LL | let b: String = format!("{}, {}", a.deref(), a.deref());
1919
| ^^^^^^^^^ help: try: `&*a`
2020

2121
error: explicit `deref` method call
22-
--> $DIR/explicit_deref_methods.rs:59:50
22+
--> $DIR/explicit_deref_methods.rs:60:50
2323
|
2424
LL | let b: String = format!("{}, {}", a.deref(), a.deref());
2525
| ^^^^^^^^^ help: try: `&*a`
2626

2727
error: explicit `deref` method call
28-
--> $DIR/explicit_deref_methods.rs:61:20
28+
--> $DIR/explicit_deref_methods.rs:62:20
2929
|
3030
LL | println!("{}", a.deref());
3131
| ^^^^^^^^^ help: try: `&*a`
3232

3333
error: explicit `deref` method call
34-
--> $DIR/explicit_deref_methods.rs:64:11
34+
--> $DIR/explicit_deref_methods.rs:65:11
3535
|
3636
LL | match a.deref() {
3737
| ^^^^^^^^^ help: try: `&*a`
3838

3939
error: explicit `deref` method call
40-
--> $DIR/explicit_deref_methods.rs:68:28
40+
--> $DIR/explicit_deref_methods.rs:69:28
4141
|
4242
LL | let b: String = concat(a.deref());
4343
| ^^^^^^^^^ help: try: `&*a`
4444

4545
error: explicit `deref` method call
46-
--> $DIR/explicit_deref_methods.rs:70:13
46+
--> $DIR/explicit_deref_methods.rs:71:13
4747
|
4848
LL | let b = just_return(a).deref();
4949
| ^^^^^^^^^^^^^^^^^^^^^^ help: try: `just_return(a)`
5050

5151
error: explicit `deref` method call
52-
--> $DIR/explicit_deref_methods.rs:72:28
52+
--> $DIR/explicit_deref_methods.rs:73:28
5353
|
5454
LL | let b: String = concat(just_return(a).deref());
5555
| ^^^^^^^^^^^^^^^^^^^^^^ help: try: `just_return(a)`
5656

5757
error: explicit `deref` method call
58-
--> $DIR/explicit_deref_methods.rs:74:19
58+
--> $DIR/explicit_deref_methods.rs:75:19
5959
|
6060
LL | let b: &str = a.deref().deref();
6161
| ^^^^^^^^^^^^^^^^^ help: try: `&**a`
6262

6363
error: explicit `deref` method call
64-
--> $DIR/explicit_deref_methods.rs:77:13
64+
--> $DIR/explicit_deref_methods.rs:78:13
6565
|
6666
LL | let b = opt_a.unwrap().deref();
6767
| ^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*opt_a.unwrap()`
6868

6969
error: explicit `deref` method call
70-
--> $DIR/explicit_deref_methods.rs:114:31
70+
--> $DIR/explicit_deref_methods.rs:115:31
7171
|
7272
LL | let b: &str = expr_deref!(a.deref());
7373
| ^^^^^^^^^ help: try: `&*a`

Diff for: ‎src/tools/compiletest/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1119,7 +1119,7 @@ fn check_overlapping_tests(found_paths: &BTreeSet<PathBuf>) {
11191119
for path in found_paths {
11201120
for ancestor in path.ancestors().skip(1) {
11211121
if found_paths.contains(ancestor) {
1122-
collisions.push((path, ancestor.clone()));
1122+
collisions.push((path, ancestor));
11231123
}
11241124
}
11251125
}

Diff for: ‎tests/ui/issues/issue-11820.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// run-pass
22
// pretty-expanded FIXME #23616
33

4+
#![allow(noop_method_call)]
5+
46
struct NoClone;
57

68
fn main() {

Diff for: ‎tests/ui/lint/noop-method-call.fixed

+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// check-pass
2+
// run-rustfix
3+
4+
#![allow(unused)]
5+
6+
use std::borrow::Borrow;
7+
use std::ops::Deref;
8+
9+
struct PlainType<T>(T);
10+
11+
#[derive(Clone)]
12+
struct CloneType<T>(T);
13+
14+
fn check(mut encoded: &[u8]) {
15+
let _ = &mut encoded;
16+
//~^ WARN call to `.clone()` on a reference in this situation does nothing
17+
let _ = &encoded;
18+
//~^ WARN call to `.clone()` on a reference in this situation does nothing
19+
}
20+
21+
fn main() {
22+
let non_clone_type_ref = &PlainType(1u32);
23+
let non_clone_type_ref_clone: &PlainType<u32> = non_clone_type_ref;
24+
//~^ WARN call to `.clone()` on a reference in this situation does nothing
25+
26+
let clone_type_ref = &CloneType(1u32);
27+
let clone_type_ref_clone: CloneType<u32> = clone_type_ref.clone();
28+
29+
30+
let non_deref_type = &PlainType(1u32);
31+
let non_deref_type_deref: &PlainType<u32> = non_deref_type;
32+
//~^ WARN call to `.deref()` on a reference in this situation does nothing
33+
34+
let non_borrow_type = &PlainType(1u32);
35+
let non_borrow_type_borrow: &PlainType<u32> = non_borrow_type;
36+
//~^ WARN call to `.borrow()` on a reference in this situation does nothing
37+
38+
// Borrowing a &&T does not warn since it has collapsed the double reference
39+
let non_borrow_type = &&PlainType(1u32);
40+
let non_borrow_type_borrow: &PlainType<u32> = non_borrow_type.borrow();
41+
}
42+
43+
fn generic<T>(non_clone_type: &PlainType<T>) {
44+
non_clone_type;
45+
//~^ WARN call to `.clone()` on a reference in this situation does nothing
46+
}
47+
48+
fn non_generic(non_clone_type: &PlainType<u32>) {
49+
non_clone_type;
50+
//~^ WARN call to `.clone()` on a reference in this situation does nothing
51+
}

Diff for: ‎tests/ui/lint/noop-method-call.rs

+13-17
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// check-pass
2+
// run-rustfix
23

34
#![allow(unused)]
4-
#![warn(noop_method_call)]
55

66
use std::borrow::Borrow;
77
use std::ops::Deref;
@@ -11,45 +11,41 @@ struct PlainType<T>(T);
1111
#[derive(Clone)]
1212
struct CloneType<T>(T);
1313

14+
fn check(mut encoded: &[u8]) {
15+
let _ = &mut encoded.clone();
16+
//~^ WARN call to `.clone()` on a reference in this situation does nothing
17+
let _ = &encoded.clone();
18+
//~^ WARN call to `.clone()` on a reference in this situation does nothing
19+
}
20+
1421
fn main() {
1522
let non_clone_type_ref = &PlainType(1u32);
1623
let non_clone_type_ref_clone: &PlainType<u32> = non_clone_type_ref.clone();
17-
//~^ WARNING call to `.clone()` on a reference in this situation does nothing
24+
//~^ WARN call to `.clone()` on a reference in this situation does nothing
1825

1926
let clone_type_ref = &CloneType(1u32);
2027
let clone_type_ref_clone: CloneType<u32> = clone_type_ref.clone();
2128

22-
let clone_type_ref = &&CloneType(1u32);
23-
let clone_type_ref_clone: &CloneType<u32> = clone_type_ref.clone();
24-
//~^ WARNING using `.clone()` on a double reference, which returns `&CloneType<u32>`
2529

2630
let non_deref_type = &PlainType(1u32);
2731
let non_deref_type_deref: &PlainType<u32> = non_deref_type.deref();
28-
//~^ WARNING call to `.deref()` on a reference in this situation does nothing
29-
30-
let non_deref_type = &&PlainType(1u32);
31-
let non_deref_type_deref: &PlainType<u32> = non_deref_type.deref();
32-
//~^ WARNING using `.deref()` on a double reference, which returns `&PlainType<u32>`
32+
//~^ WARN call to `.deref()` on a reference in this situation does nothing
3333

3434
let non_borrow_type = &PlainType(1u32);
3535
let non_borrow_type_borrow: &PlainType<u32> = non_borrow_type.borrow();
36-
//~^ WARNING call to `.borrow()` on a reference in this situation does nothing
36+
//~^ WARN call to `.borrow()` on a reference in this situation does nothing
3737

3838
// Borrowing a &&T does not warn since it has collapsed the double reference
3939
let non_borrow_type = &&PlainType(1u32);
4040
let non_borrow_type_borrow: &PlainType<u32> = non_borrow_type.borrow();
41-
42-
let xs = ["a", "b", "c"];
43-
let _v: Vec<&str> = xs.iter().map(|x| x.clone()).collect(); // could use `*x` instead
44-
//~^ WARNING using `.clone()` on a double reference, which returns `&str`
4541
}
4642

4743
fn generic<T>(non_clone_type: &PlainType<T>) {
4844
non_clone_type.clone();
49-
//~^ WARNING call to `.clone()` on a reference in this situation does nothing
45+
//~^ WARN call to `.clone()` on a reference in this situation does nothing
5046
}
5147

5248
fn non_generic(non_clone_type: &PlainType<u32>) {
5349
non_clone_type.clone();
54-
//~^ WARNING call to `.clone()` on a reference in this situation does nothing
50+
//~^ WARN call to `.clone()` on a reference in this situation does nothing
5551
}

0 commit comments

Comments
 (0)