Skip to content

Commit d6ab6f5

Browse files
JarchoJarcho
committed
Add lint ref_binding_to_reference 
1 parent 65cee85 commit d6ab6f5

File tree

8 files changed

+212
-72
lines changed

8 files changed

+212
-72
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2434,6 +2434,7 @@ Released 2018-09-13
24342434
[`redundant_pub_crate`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pub_crate
24352435
[`redundant_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_slicing
24362436
[`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
2437+
[`ref_binding_to_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_binding_to_reference
24372438
[`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
24382439
[`ref_option_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_option_ref
24392440
[`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro

‎clippy_lints/src/lib.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -846,6 +846,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
846846
needless_bool::BOOL_COMPARISON,
847847
needless_bool::NEEDLESS_BOOL,
848848
needless_borrow::NEEDLESS_BORROW,
849+
needless_borrow::REF_BINDING_TO_REFERENCE,
849850
needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE,
850851
needless_continue::NEEDLESS_CONTINUE,
851852
needless_for_each::NEEDLESS_FOR_EACH,
@@ -1629,6 +1630,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16291630
LintId::of(needless_arbitrary_self_type::NEEDLESS_ARBITRARY_SELF_TYPE),
16301631
LintId::of(needless_bool::BOOL_COMPARISON),
16311632
LintId::of(needless_bool::NEEDLESS_BOOL),
1633+
LintId::of(needless_borrow::REF_BINDING_TO_REFERENCE),
16321634
LintId::of(needless_borrowed_ref::NEEDLESS_BORROWED_REFERENCE),
16331635
LintId::of(needless_question_mark::NEEDLESS_QUESTION_MARK),
16341636
LintId::of(needless_update::NEEDLESS_UPDATE),
@@ -1813,6 +1815,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
18131815
LintId::of(misc_early::REDUNDANT_PATTERN),
18141816
LintId::of(mut_mutex_lock::MUT_MUTEX_LOCK),
18151817
LintId::of(mut_reference::UNNECESSARY_MUT_PASSED),
1818+
LintId::of(needless_borrow::REF_BINDING_TO_REFERENCE),
18161819
LintId::of(neg_multiply::NEG_MULTIPLY),
18171820
LintId::of(new_without_default::NEW_WITHOUT_DEFAULT),
18181821
LintId::of(non_copy_const::BORROW_INTERIOR_MUTABLE_CONST),

‎clippy_lints/src/needless_borrow.rs

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,33 @@ declare_clippy_lint! {
3737
"taking a reference that is going to be automatically dereferenced"
3838
}
3939

40-
declare_lint_pass!(NeedlessBorrow => [NEEDLESS_BORROW]);
40+
declare_clippy_lint! {
41+
/// **What it does:** Checks for `ref` bindings which create a reference to a reference.
42+
///
43+
/// **Why is this bad?** The address-of operator at the use site is clearer about the need for a reference.
44+
///
45+
/// **Known problems:** None.
46+
///
47+
/// **Example:**
48+
/// ```rust
49+
/// // Bad
50+
/// let x = Some("");
51+
/// if let Some(ref x) = x {
52+
/// // use `x` here
53+
/// }
54+
///
55+
/// // Good
56+
/// let x = Some("");
57+
/// if let Some(x) = x {
58+
/// // use `&x` here
59+
/// }
60+
/// ```
61+
pub REF_BINDING_TO_REFERENCE,
62+
style,
63+
"`ref` binding to a reference"
64+
}
65+
66+
declare_lint_pass!(NeedlessBorrow => [NEEDLESS_BORROW, REF_BINDING_TO_REFERENCE]);
4167

4268
impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow {
4369
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
@@ -102,6 +128,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow {
102128

103129
let mut app = Applicability::MachineApplicable;
104130
let mut can_fix = true;
131+
let mut lint = NEEDLESS_BORROW;
105132
let mut changes = vec![
106133
(pat.span, snippet_with_context(cx, name.span, ctxt, "..", &mut app).0.into_owned()),
107134
];
@@ -121,6 +148,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow {
121148
}
122149
_ if e.span.ctxt() == ctxt => {
123150
// Double reference might be needed at this point.
151+
lint = REF_BINDING_TO_REFERENCE;
124152
let snip = snippet_with_applicability(cx, e.span, "..", &mut app);
125153
changes.push((e.span, format!("&{}", snip)));
126154
}
@@ -135,7 +163,7 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrow {
135163

136164
span_lint_and_then(
137165
cx,
138-
NEEDLESS_BORROW,
166+
lint,
139167
pat.span,
140168
"this pattern creates a reference to a reference",
141169
|diag| {

‎clippy_utils/src/higher.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ pub fn range<'a>(expr: &'a hir::Expr<'_>) -> Option<Range<'a>> {
7070
limits: ast::RangeLimits::Closed,
7171
})
7272
},
73-
hir::ExprKind::Struct(ref path, ref fields, None) => match path {
73+
hir::ExprKind::Struct(path, ref fields, None) => match path {
7474
hir::QPath::LangItem(hir::LangItem::RangeFull, _) => Some(Range {
7575
start: None,
7676
end: None,

‎tests/ui/needless_borrow_pat.rs

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,6 @@ macro_rules! m1 {
1010
f1($e);
1111
};
1212
}
13-
macro_rules! m2 {
14-
($e:expr) => {
15-
f1(*$e);
16-
};
17-
}
1813
macro_rules! m3 {
1914
($i:ident) => {
2015
Some(ref $i)
@@ -56,13 +51,7 @@ fn main() {
5651
};
5752

5853
// Err, reference to a &String
59-
let _: &&String = match Some(&x) {
60-
Some(ref x) => x,
61-
None => return,
62-
};
63-
64-
// Err, reference to a &String
65-
let _: &&String = match Some(&x) {
54+
let _: &String = match Some(&x) {
6655
Some(ref x) => {
6756
f1(x);
6857
f1(*x);
@@ -77,20 +66,10 @@ fn main() {
7766
None => return,
7867
};
7968

80-
// Err, reference to a &String
81-
match Some(&x) {
82-
Some(ref x) => m2!(x),
83-
None => return,
84-
}
85-
8669
// Err, reference to a &String
8770
let _ = |&ref x: &&String| {
8871
let _: &String = x;
8972
};
90-
// Err, reference to a &String
91-
let _ = |&ref x: &&String| {
92-
let _: &&String = x;
93-
};
9473

9574
// Err, reference to a &String
9675
let (ref y,) = (&x,);
@@ -103,7 +82,7 @@ fn main() {
10382

10483
// Err, reference to a &String
10584
fn f2<'a>(&ref x: &&'a String) -> &'a String {
106-
let _: &&String = x;
85+
let _: &String = x;
10786
*x
10887
}
10988

‎tests/ui/needless_borrow_pat.stderr

Lines changed: 11 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
error: this pattern creates a reference to a reference
2-
--> $DIR/needless_borrow_pat.rs:48:14
2+
--> $DIR/needless_borrow_pat.rs:43:14
33
|
44
LL | Some(ref x) => x,
55
| ^^^^^ help: try this: `x`
66
|
77
= note: `-D clippy::needless-borrow` implied by `-D warnings`
88

99
error: this pattern creates a reference to a reference
10-
--> $DIR/needless_borrow_pat.rs:54:14
10+
--> $DIR/needless_borrow_pat.rs:49:14
1111
|
1212
LL | Some(ref x) => *x,
1313
| ^^^^^
@@ -18,18 +18,7 @@ LL | Some(x) => x,
1818
| ^ ^
1919

2020
error: this pattern creates a reference to a reference
21-
--> $DIR/needless_borrow_pat.rs:60:14
22-
|
23-
LL | Some(ref x) => x,
24-
| ^^^^^
25-
|
26-
help: try this
27-
|
28-
LL | Some(x) => &x,
29-
| ^ ^^
30-
31-
error: this pattern creates a reference to a reference
32-
--> $DIR/needless_borrow_pat.rs:66:14
21+
--> $DIR/needless_borrow_pat.rs:55:14
3322
|
3423
LL | Some(ref x) => {
3524
| ^^^^^
@@ -39,46 +28,22 @@ help: try this
3928
LL | Some(x) => {
4029
LL | f1(x);
4130
LL | f1(x);
42-
LL | &x
4331
|
4432

4533
error: this pattern creates a reference to a reference
46-
--> $DIR/needless_borrow_pat.rs:76:14
34+
--> $DIR/needless_borrow_pat.rs:65:14
4735
|
4836
LL | Some(ref x) => m1!(x),
4937
| ^^^^^ help: try this: `x`
5038

5139
error: this pattern creates a reference to a reference
52-
--> $DIR/needless_borrow_pat.rs:82:14
53-
|
54-
LL | Some(ref x) => m2!(x),
55-
| ^^^^^
56-
|
57-
help: try this
58-
|
59-
LL | Some(x) => m2!(&x),
60-
| ^ ^^
61-
62-
error: this pattern creates a reference to a reference
63-
--> $DIR/needless_borrow_pat.rs:87:15
40+
--> $DIR/needless_borrow_pat.rs:70:15
6441
|
6542
LL | let _ = |&ref x: &&String| {
6643
| ^^^^^ help: try this: `x`
6744

6845
error: this pattern creates a reference to a reference
69-
--> $DIR/needless_borrow_pat.rs:91:15
70-
|
71-
LL | let _ = |&ref x: &&String| {
72-
| ^^^^^
73-
|
74-
help: try this
75-
|
76-
LL | let _ = |&x: &&String| {
77-
LL | let _: &&String = &x;
78-
|
79-
80-
error: this pattern creates a reference to a reference
81-
--> $DIR/needless_borrow_pat.rs:96:10
46+
--> $DIR/needless_borrow_pat.rs:75:10
8247
|
8348
LL | let (ref y,) = (&x,);
8449
| ^^^^^
@@ -90,26 +55,26 @@ LL | let _: &String = y;
9055
|
9156

9257
error: this pattern creates a reference to a reference
93-
--> $DIR/needless_borrow_pat.rs:105:12
58+
--> $DIR/needless_borrow_pat.rs:84:12
9459
|
9560
LL | fn f2<'a>(&ref x: &&'a String) -> &'a String {
9661
| ^^^^^
9762
|
9863
help: try this
9964
|
10065
LL | fn f2<'a>(&x: &&'a String) -> &'a String {
101-
LL | let _: &&String = &x;
66+
LL | let _: &String = x;
10267
LL | x
10368
|
10469

10570
error: this pattern creates a reference to a reference
106-
--> $DIR/needless_borrow_pat.rs:112:11
71+
--> $DIR/needless_borrow_pat.rs:91:11
10772
|
10873
LL | fn f(&ref x: &&String) {
10974
| ^^^^^ help: try this: `x`
11075

11176
error: this pattern creates a reference to a reference
112-
--> $DIR/needless_borrow_pat.rs:120:11
77+
--> $DIR/needless_borrow_pat.rs:99:11
11378
|
11479
LL | fn f(&ref x: &&String) {
11580
| ^^^^^
@@ -120,5 +85,5 @@ LL | fn f(&x: &&String) {
12085
LL | let _: &String = x;
12186
|
12287

123-
error: aborting due to 12 previous errors
88+
error: aborting due to 9 previous errors
12489

‎tests/ui/ref_binding_to_reference.rs

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
// edition:2018
2+
// FIXME: run-rustfix waiting on multi-span suggestions
3+
4+
#![warn(clippy::ref_binding_to_reference)]
5+
#![allow(clippy::needless_borrowed_reference)]
6+
7+
fn f1(_: &str) {}
8+
macro_rules! m2 {
9+
($e:expr) => {
10+
f1(*$e);
11+
};
12+
}
13+
macro_rules! m3 {
14+
($i:ident) => {
15+
Some(ref $i)
16+
};
17+
}
18+
19+
#[allow(dead_code)]
20+
fn main() {
21+
let x = String::new();
22+
23+
// Ok, the pattern is in a different context than the match arm
24+
let _: &&String = match Some(&x) {
25+
m3!(x) => x,
26+
None => return,
27+
};
28+
29+
// Err, reference to a &String
30+
let _: &&String = match Some(&x) {
31+
Some(ref x) => x,
32+
None => return,
33+
};
34+
35+
// Err, reference to a &String
36+
let _: &&String = match Some(&x) {
37+
Some(ref x) => {
38+
f1(x);
39+
f1(*x);
40+
x
41+
},
42+
None => return,
43+
};
44+
45+
// Err, reference to a &String
46+
match Some(&x) {
47+
Some(ref x) => m2!(x),
48+
None => return,
49+
}
50+
51+
// Err, reference to a &String
52+
let _ = |&ref x: &&String| {
53+
let _: &&String = x;
54+
};
55+
}
56+
57+
// Err, reference to a &String
58+
fn f2<'a>(&ref x: &&'a String) -> &'a String {
59+
let _: &&String = x;
60+
*x
61+
}
62+
63+
trait T1 {
64+
// Err, reference to a &String
65+
fn f(&ref x: &&String) {
66+
let _: &&String = x;
67+
}
68+
}
69+
70+
struct S;
71+
impl T1 for S {
72+
// Err, reference to a &String
73+
fn f(&ref x: &&String) {
74+
let _: &&String = x;
75+
}
76+
}

0 commit comments

Comments
 (0)