Skip to content

Commit 7f822e7

Browse files
committed
needless_borrowed_ref: fix false positive, make rustfixable
1 parent 622b167 commit 7f822e7

File tree

4 files changed

+62
-24
lines changed

4 files changed

+62
-24
lines changed

clippy_lints/src/needless_borrowed_ref.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
//!
33
//! This lint is **warn** by default
44
5-
use crate::utils::{snippet, span_lint_and_then};
5+
use crate::utils::{snippet_with_applicability, span_lint_and_then};
66
use if_chain::if_chain;
7-
use rustc::hir::{BindingAnnotation, MutImmutable, Pat, PatKind};
7+
use rustc::hir::{BindingAnnotation, MutImmutable, Node, Pat, PatKind};
88
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
99
use rustc::{declare_lint_pass, declare_tool_lint};
1010
use rustc_errors::Applicability;
@@ -65,16 +65,25 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessBorrowedRef {
6565

6666
// Check sub_pat got a `ref` keyword (excluding `ref mut`).
6767
if let PatKind::Binding(BindingAnnotation::Ref, .., spanned_name, _) = sub_pat.node;
68+
let parent_id = cx.tcx.hir().get_parent_node(pat.hir_id);
69+
if let Some(parent_node) = cx.tcx.hir().find(parent_id);
6870
then {
71+
// do not recurse within patterns, as they may have other references
72+
// XXXManishearth we can relax this constraint if we only check patterns
73+
// with a single ref pattern inside them
74+
if let Node::Pat(_) = parent_node {
75+
return;
76+
}
77+
let mut applicability = Applicability::MachineApplicable;
6978
span_lint_and_then(cx, NEEDLESS_BORROWED_REFERENCE, pat.span,
7079
"this pattern takes a reference on something that is being de-referenced",
7180
|db| {
72-
let hint = snippet(cx, spanned_name.span, "..").into_owned();
81+
let hint = snippet_with_applicability(cx, spanned_name.span, "..", &mut applicability).into_owned();
7382
db.span_suggestion(
7483
pat.span,
7584
"try removing the `&ref` part and just keep",
7685
hint,
77-
Applicability::MachineApplicable, // snippet
86+
applicability,
7887
);
7988
});
8089
}

tests/ui/needless_borrowed_ref.fixed

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// run-rustfix
2+
3+
#[warn(clippy::needless_borrowed_reference)]
4+
#[allow(unused_variables)]
5+
fn main() {
6+
let mut v = Vec::<String>::new();
7+
let _ = v.iter_mut().filter(|a| a.is_empty());
8+
// ^ should be linted
9+
10+
let var = 3;
11+
let thingy = Some(&var);
12+
if let Some(&ref v) = thingy {
13+
// ^ should be linted
14+
}
15+
16+
let mut var2 = 5;
17+
let thingy2 = Some(&mut var2);
18+
if let Some(&mut ref mut v) = thingy2 {
19+
// ^ should **not** be linted
20+
// v is borrowed as mutable.
21+
*v = 10;
22+
}
23+
if let Some(&mut ref v) = thingy2 {
24+
// ^ should **not** be linted
25+
// here, v is borrowed as immutable.
26+
// can't do that:
27+
//*v = 15;
28+
}
29+
}
30+
31+
#[allow(dead_code)]
32+
enum Animal {
33+
Cat(u64),
34+
Dog(u64),
35+
}
36+
37+
#[allow(unused_variables)]
38+
#[allow(dead_code)]
39+
fn foo(a: &Animal, b: &Animal) {
40+
match (a, b) {
41+
(&Animal::Cat(v), &ref k) | (&ref k, &Animal::Cat(v)) => (), // lifetime mismatch error if there is no '&ref'
42+
// ^ and ^ should **not** be linted
43+
(&Animal::Dog(ref a), &Animal::Dog(_)) => (), // ^ should **not** be linted
44+
}
45+
}

tests/ui/needless_borrowed_ref.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// run-rustfix
2+
13
#[warn(clippy::needless_borrowed_reference)]
24
#[allow(unused_variables)]
35
fn main() {

tests/ui/needless_borrowed_ref.stderr

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,10 @@
11
error: this pattern takes a reference on something that is being de-referenced
2-
--> $DIR/needless_borrowed_ref.rs:5:34
2+
--> $DIR/needless_borrowed_ref.rs:7:34
33
|
44
LL | let _ = v.iter_mut().filter(|&ref a| a.is_empty());
55
| ^^^^^^ help: try removing the `&ref` part and just keep: `a`
66
|
77
= note: `-D clippy::needless-borrowed-reference` implied by `-D warnings`
88

9-
error: this pattern takes a reference on something that is being de-referenced
10-
--> $DIR/needless_borrowed_ref.rs:10:17
11-
|
12-
LL | if let Some(&ref v) = thingy {
13-
| ^^^^^^ help: try removing the `&ref` part and just keep: `v`
14-
15-
error: this pattern takes a reference on something that is being de-referenced
16-
--> $DIR/needless_borrowed_ref.rs:39:27
17-
|
18-
LL | (&Animal::Cat(v), &ref k) | (&ref k, &Animal::Cat(v)) => (), // lifetime mismatch error if there is no '&ref'
19-
| ^^^^^^ help: try removing the `&ref` part and just keep: `k`
20-
21-
error: this pattern takes a reference on something that is being de-referenced
22-
--> $DIR/needless_borrowed_ref.rs:39:38
23-
|
24-
LL | (&Animal::Cat(v), &ref k) | (&ref k, &Animal::Cat(v)) => (), // lifetime mismatch error if there is no '&ref'
25-
| ^^^^^^ help: try removing the `&ref` part and just keep: `k`
26-
27-
error: aborting due to 4 previous errors
9+
error: aborting due to previous error
2810

0 commit comments

Comments
 (0)