Skip to content

Commit 063de76

Browse files
jpospychalajpospychala
committed
useless Rc<Rc<T>>, Rc<Box<T>>, Rc<&T>
1 parent cd6eafd commit 063de76

File tree

12 files changed

+283
-25
lines changed

12 files changed

+283
-25
lines changed

‎CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -1433,6 +1433,9 @@ Released 2018-09-13
14331433
[`range_plus_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_plus_one
14341434
[`range_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_step_by_zero
14351435
[`range_zip_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_zip_with_len
1436+
[`rc_borrows`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_borrows
1437+
[`rc_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_box
1438+
[`rc_rc`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_rc
14361439
[`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone
14371440
[`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
14381441
[`redundant_closure_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_call

‎README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
77

8-
[There are 362 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
8+
[There are 366 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
99

1010
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
1111

‎clippy_lints/src/lib.rs

+9
Original file line numberDiff line numberDiff line change
@@ -807,6 +807,9 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
807807
&types::LET_UNIT_VALUE,
808808
&types::LINKEDLIST,
809809
&types::OPTION_OPTION,
810+
&types::RC_BORROWS,
811+
&types::RC_BOX,
812+
&types::RC_RC,
810813
&types::TYPE_COMPLEXITY,
811814
&types::UNIT_ARG,
812815
&types::UNIT_CMP,
@@ -1372,6 +1375,9 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
13721375
LintId::of(&types::IMPLICIT_HASHER),
13731376
LintId::of(&types::LET_UNIT_VALUE),
13741377
LintId::of(&types::OPTION_OPTION),
1378+
LintId::of(&types::RC_BORROWS),
1379+
LintId::of(&types::RC_BOX),
1380+
LintId::of(&types::RC_RC),
13751381
LintId::of(&types::TYPE_COMPLEXITY),
13761382
LintId::of(&types::UNIT_ARG),
13771383
LintId::of(&types::UNIT_CMP),
@@ -1657,6 +1663,9 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
16571663
LintId::of(&trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF),
16581664
LintId::of(&types::BOX_BORROWS),
16591665
LintId::of(&types::BOX_VEC),
1666+
LintId::of(&types::RC_BORROWS),
1667+
LintId::of(&types::RC_BOX),
1668+
LintId::of(&types::RC_RC),
16601669
LintId::of(&vec::USELESS_VEC),
16611670
]);
16621671

‎clippy_lints/src/types.rs

+144-21
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,96 @@ declare_clippy_lint! {
194194
"a box of borrowed type"
195195
}
196196

197+
declare_clippy_lint! {
198+
/// **What it does:** Checks for use of `Rc<&T>` anywhere in the code.
199+
///
200+
/// **Why is this bad?** Any `Rc<&T>` can also be a `&T`, which is more
201+
/// general.
202+
///
203+
/// **Known problems:** None.
204+
///
205+
/// **Example:**
206+
/// ```rust
207+
/// fn foo(bar: Rc<&usize>) {}
208+
/// ```
209+
///
210+
/// Better:
211+
///
212+
/// ```rust
213+
/// fn foo(bar: &usize) {}
214+
/// ```
215+
pub RC_BORROWS,
216+
perf,
217+
"a Rc of borrowed type"
218+
}
219+
220+
declare_clippy_lint! {
221+
/// **What it does:** Checks for use of `Rc<Rc<T>>` anywhere in the code.
222+
///
223+
/// **Why is this bad?** Reference counting pointer of reference counting pointer
224+
/// is an unnecessary level of indirection.
225+
///
226+
/// **Known problems:** None.
227+
///
228+
/// **Example:**
229+
/// ```rust
230+
/// # use std::rc::Rc;
231+
/// fn foo(bar: Rc<Rc<usize>>) {}
232+
/// ```
233+
///
234+
/// Better:
235+
///
236+
/// ```rust
237+
/// # use std::rc::Rc;
238+
/// fn foo(bar: Rc<usize>) {}
239+
/// ```
240+
pub RC_RC,
241+
perf,
242+
"an Rc of Rc"
243+
}
244+
245+
declare_clippy_lint! {
246+
/// **What it does:** Checks for use of `Rc<Box<T>>` anywhere in the code.
247+
///
248+
/// **Why is this bad?** `Rc` already keeps its contents in a separate area on
249+
/// the heap. So if you `Box` its contents, you just add another level of indirection.
250+
///
251+
/// **Known problems:** None.
252+
///
253+
/// **Example:**
254+
/// ```rust
255+
/// # use std::rc::Rc;
256+
/// # use std::boxed::Box;
257+
/// fn foo(bar: Rc<Box<usize>>) {}
258+
/// ```
259+
///
260+
/// Better:
261+
///
262+
/// ```rust
263+
/// # use std::rc::Rc;
264+
/// # use std::boxed::Box;
265+
/// fn foo(bar: Rc<usize>) {}
266+
/// ```
267+
pub RC_BOX,
268+
perf,
269+
"an Rc of Box"
270+
}
271+
197272
pub struct Types {
198273
vec_box_size_threshold: u64,
199274
}
200275

201-
impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, BOX_BORROWS]);
276+
impl_lint_pass!(Types => [
277+
BOX_VEC,
278+
VEC_BOX,
279+
OPTION_OPTION,
280+
LINKEDLIST,
281+
BORROWED_BOX,
282+
BOX_BORROWS,
283+
RC_RC,
284+
RC_BOX,
285+
RC_BORROWS
286+
]);
202287

203288
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Types {
204289
fn check_fn(
@@ -259,6 +344,24 @@ fn match_type_parameter(cx: &LateContext<'_, '_>, qpath: &QPath<'_>, path: &[&st
259344
false
260345
}
261346

347+
fn match_borrows_parameter<'a>(cx: &'a LateContext<'_, '_>, qpath: &QPath<'_>) -> Option<Ty<'a>> {
348+
if_chain! {
349+
let last = last_path_segment(qpath); // why last path segment?
350+
if let Some(ref params) = last.args;
351+
if !params.parenthesized; // what are parenthesized params?
352+
if let Some(ty) = params.args.iter().find_map(|arg| match arg {
353+
GenericArg::Type(ty) => Some(ty),
354+
_ => None,
355+
});
356+
if let TyKind::Rptr(..) = ty.kind;
357+
let ty_ty = hir_ty_to_ty(cx.tcx, ty);
358+
then {
359+
return Some(ty_ty);
360+
}
361+
}
362+
return None;
363+
}
364+
262365
impl Types {
263366
pub fn new(vec_box_size_threshold: u64) -> Self {
264367
Self { vec_box_size_threshold }
@@ -291,26 +394,15 @@ impl Types {
291394
let res = qpath_res(cx, qpath, hir_id);
292395
if let Some(def_id) = res.opt_def_id() {
293396
if Some(def_id) == cx.tcx.lang_items().owned_box() {
294-
if_chain! {
295-
let last = last_path_segment(qpath);
296-
if let Some(ref params) = last.args;
297-
if !params.parenthesized;
298-
if let Some(ty) = params.args.iter().find_map(|arg| match arg {
299-
GenericArg::Type(ty) => Some(ty),
300-
_ => None,
301-
});
302-
if let TyKind::Rptr(..) = ty.kind;
303-
let ty_ty = hir_ty_to_ty(cx.tcx, ty);
304-
then {
305-
span_lint_and_help(
306-
cx,
307-
BOX_BORROWS,
308-
hir_ty.span,
309-
"usage of `Box<&T>`",
310-
format!("try `{}`", ty_ty).as_str(),
311-
);
312-
return; // don't recurse into the type
313-
}
397+
if let Some(ty_ty) = match_borrows_parameter(cx, qpath) {
398+
span_lint_and_help(
399+
cx,
400+
BOX_BORROWS,
401+
hir_ty.span,
402+
"usage of `Box<&T>`",
403+
format!("try `{}`", ty_ty).as_str(),
404+
);
405+
return; // don't recurse into the type
314406
}
315407
if match_type_parameter(cx, qpath, &paths::VEC) {
316408
span_lint_and_help(
@@ -322,6 +414,37 @@ impl Types {
322414
);
323415
return; // don't recurse into the type
324416
}
417+
} else if Some(def_id) == cx.tcx.lang_items().rc() {
418+
if match_type_parameter(cx, qpath, &paths::RC) {
419+
span_lint_and_help(
420+
cx,
421+
RC_RC,
422+
hir_ty.span,
423+
"usage of `Rc<Rc<T>>`",
424+
"Rc<Rc<T>> can be simplified to Rc<T>",
425+
);
426+
return; // don't recurse into the type
427+
}
428+
if match_type_parameter(cx, qpath, &paths::BOX) {
429+
span_lint_and_help(
430+
cx,
431+
RC_BOX,
432+
hir_ty.span,
433+
"usage of `Rc<Box<T>>`",
434+
"Rc<Box<T>> can be simplified to Rc<T>",
435+
);
436+
return; // don't recurse into the type
437+
}
438+
if let Some(ty_ty) = match_borrows_parameter(cx, qpath) {
439+
span_lint_and_help(
440+
cx,
441+
RC_BORROWS,
442+
hir_ty.span,
443+
"usage of `Rc<&T>`",
444+
format!("try `{}`", ty_ty).as_str(),
445+
);
446+
return; // don't recurse into the type
447+
}
325448
} else if cx.tcx.is_diagnostic_item(Symbol::intern("vec_type"), def_id) {
326449
if_chain! {
327450
// Get the _ part of Vec<_>

‎clippy_lints/src/utils/paths.rs

+1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ pub const BEGIN_PANIC: [&str; 3] = ["std", "panicking", "begin_panic"];
99
pub const BEGIN_PANIC_FMT: [&str; 3] = ["std", "panicking", "begin_panic_fmt"];
1010
pub const BINARY_HEAP: [&str; 4] = ["alloc", "collections", "binary_heap", "BinaryHeap"];
1111
pub const BORROW_TRAIT: [&str; 3] = ["core", "borrow", "Borrow"];
12+
pub const BOX: [&str; 3] = ["alloc", "boxed", "Box"];
1213
pub const BTREEMAP: [&str; 5] = ["alloc", "collections", "btree", "map", "BTreeMap"];
1314
pub const BTREEMAP_ENTRY: [&str; 5] = ["alloc", "collections", "btree", "map", "Entry"];
1415
pub const BTREESET: [&str; 5] = ["alloc", "collections", "btree", "set", "BTreeSet"];

‎src/lintlist/mod.rs

+22-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ pub use lint::Lint;
66
pub use lint::LINT_LEVELS;
77

88
// begin lint list, do not remove this comment, it’s used in `update_lints`
9-
pub const ALL_LINTS: [Lint; 362] = [
9+
pub const ALL_LINTS: [Lint; 366] = [
1010
Lint {
1111
name: "absurd_extreme_comparisons",
1212
group: "correctness",
@@ -1722,6 +1722,27 @@ pub const ALL_LINTS: [Lint; 362] = [
17221722
deprecation: None,
17231723
module: "ranges",
17241724
},
1725+
Lint {
1726+
name: "rc_borrows",
1727+
group: "perf",
1728+
desc: "a Rc of borrowed type",
1729+
deprecation: None,
1730+
module: "types",
1731+
},
1732+
Lint {
1733+
name: "rc_box",
1734+
group: "perf",
1735+
desc: "an Rc of Box",
1736+
deprecation: None,
1737+
module: "types",
1738+
},
1739+
Lint {
1740+
name: "rc_rc",
1741+
group: "perf",
1742+
desc: "an Rc of Rc",
1743+
deprecation: None,
1744+
module: "types",
1745+
},
17251746
Lint {
17261747
name: "redundant_clone",
17271748
group: "perf",

‎tests/ui/must_use_candidates.fixed

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// run-rustfix
22
#![feature(never_type)]
3-
#![allow(unused_mut)]
3+
#![allow(unused_mut, clippy::rc_borrows)]
44
#![warn(clippy::must_use_candidate)]
55
use std::rc::Rc;
66
use std::sync::atomic::{AtomicBool, Ordering};

‎tests/ui/must_use_candidates.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// run-rustfix
22
#![feature(never_type)]
3-
#![allow(unused_mut)]
3+
#![allow(unused_mut, clippy::rc_borrows)]
44
#![warn(clippy::must_use_candidate)]
55
use std::rc::Rc;
66
use std::sync::atomic::{AtomicBool, Ordering};

‎tests/ui/rc_borrows.rs

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#![warn(clippy::all)]
2+
#![allow(clippy::boxed_local, clippy::needless_pass_by_value)]
3+
#![allow(clippy::blacklisted_name)]
4+
5+
use std::rc::Rc;
6+
7+
pub struct MyStruct {}
8+
9+
pub struct SubT<T> {
10+
foo: T,
11+
}
12+
13+
pub enum MyEnum {
14+
One,
15+
Two,
16+
}
17+
18+
pub fn test<T>(foo: Rc<&T>) {}
19+
20+
pub fn test1(foo: Rc<&usize>) {}
21+
22+
pub fn test2(foo: Rc<&MyStruct>) {}
23+
24+
pub fn test3(foo: Rc<&MyEnum>) {}
25+
26+
pub fn test4(foo: Rc<&&MyEnum>) {}
27+
28+
pub fn test6(foo: Rc<SubT<&usize>>) {}
29+
30+
fn main() {}

‎tests/ui/rc_borrows.stderr

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
error: usage of `Rc<&T>`
2+
--> $DIR/rc_borrows.rs:18:21
3+
|
4+
LL | pub fn test<T>(foo: Rc<&T>) {}
5+
| ^^^^^^
6+
|
7+
= note: `-D clippy::rc-borrows` implied by `-D warnings`
8+
= help: try `&T`
9+
10+
error: usage of `Rc<&T>`
11+
--> $DIR/rc_borrows.rs:20:19
12+
|
13+
LL | pub fn test1(foo: Rc<&usize>) {}
14+
| ^^^^^^^^^^
15+
|
16+
= help: try `&usize`
17+
18+
error: usage of `Rc<&T>`
19+
--> $DIR/rc_borrows.rs:22:19
20+
|
21+
LL | pub fn test2(foo: Rc<&MyStruct>) {}
22+
| ^^^^^^^^^^^^^
23+
|
24+
= help: try `&MyStruct`
25+
26+
error: usage of `Rc<&T>`
27+
--> $DIR/rc_borrows.rs:24:19
28+
|
29+
LL | pub fn test3(foo: Rc<&MyEnum>) {}
30+
| ^^^^^^^^^^^
31+
|
32+
= help: try `&MyEnum`
33+
34+
error: usage of `Rc<&T>`
35+
--> $DIR/rc_borrows.rs:26:19
36+
|
37+
LL | pub fn test4(foo: Rc<&&MyEnum>) {}
38+
| ^^^^^^^^^^^^
39+
|
40+
= help: try `&&MyEnum`
41+
42+
error: aborting due to 5 previous errors
43+

‎tests/ui/rc_rc.rs

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
use std::boxed::Box;
2+
use std::rc::Rc;
3+
4+
pub fn test(a: Rc<Rc<bool>>) {}
5+
6+
pub fn test2(a: Rc<Box<bool>>) {}
7+
8+
fn main() {}

0 commit comments

Comments
 (0)