Skip to content

Commit

Permalink
useless Rc<Rc<T>>, Rc<Box<T>>
Browse files Browse the repository at this point in the history
  • Loading branch information
jpospychala committed Mar 24, 2020
1 parent cd6eafd commit 81a9113
Show file tree
Hide file tree
Showing 8 changed files with 127 additions and 3 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1433,6 +1433,8 @@ Released 2018-09-13
[`range_plus_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_plus_one
[`range_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_step_by_zero
[`range_zip_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_zip_with_len
[`rc_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_box
[`rc_rc`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_rc
[`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone
[`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
[`redundant_closure_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_call
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

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

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

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

Expand Down
6 changes: 6 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&types::LET_UNIT_VALUE,
&types::LINKEDLIST,
&types::OPTION_OPTION,
&types::RC_BOX,
&types::RC_RC,
&types::TYPE_COMPLEXITY,
&types::UNIT_ARG,
&types::UNIT_CMP,
Expand Down Expand Up @@ -1372,6 +1374,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&types::IMPLICIT_HASHER),
LintId::of(&types::LET_UNIT_VALUE),
LintId::of(&types::OPTION_OPTION),
LintId::of(&types::RC_BOX),
LintId::of(&types::RC_RC),
LintId::of(&types::TYPE_COMPLEXITY),
LintId::of(&types::UNIT_ARG),
LintId::of(&types::UNIT_CMP),
Expand Down Expand Up @@ -1657,6 +1661,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF),
LintId::of(&types::BOX_BORROWS),
LintId::of(&types::BOX_VEC),
LintId::of(&types::RC_BOX),
LintId::of(&types::RC_RC),
LintId::of(&vec::USELESS_VEC),
]);

Expand Down
75 changes: 74 additions & 1 deletion clippy_lints/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,63 @@ declare_clippy_lint! {
"a box of borrowed type"
}

declare_clippy_lint! {
/// **What it does:** Checks for use of `Rc<Rc<T>>` anywhere in the code.
///
/// **Why is this bad?** Reference counting pointer of reference counting pointer
/// is an unnecessary level of indirection.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// # use std::rc::Rc;
/// fn foo(bar: Rc<Rc<usize>>) {}
/// ```
///
/// Better:
///
/// ```rust
/// # use std::rc::Rc;
/// fn foo(bar: Rc<usize>) {}
/// ```
pub RC_RC,
perf,
"an Rc of Rc"
}

declare_clippy_lint! {
/// **What it does:** Checks for use of `Rc<Box<T>>` anywhere in the code.
///
/// **Why is this bad?** `Rc` already keeps its contents in a separate area on
/// the heap. So if you `Box` its contents, you just add another level of indirection.
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// # use std::rc::Rc;
/// # use std::boxed::Box;
/// fn foo(bar: Rc<Box<usize>>) {}
/// ```
///
/// Better:
///
/// ```rust
/// # use std::rc::Rc;
/// # use std::boxed::Box;
/// fn foo(bar: Rc<usize>) {}
/// ```
pub RC_BOX,
perf,
"an Rc of Box"
}

pub struct Types {
vec_box_size_threshold: u64,
}

impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, BOX_BORROWS]);
impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, BOX_BORROWS, RC_RC, RC_BOX]);

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Types {
fn check_fn(
Expand Down Expand Up @@ -322,6 +374,27 @@ impl Types {
);
return; // don't recurse into the type
}
} else if Some(def_id) == cx.tcx.lang_items().rc() {
if match_type_parameter(cx, qpath, &paths::RC) {
span_lint_and_help(
cx,
RC_RC,
hir_ty.span,
"usage of `Rc<Rc<T>>`",
"Rc<Rc<T>> can be simplified to Rc<T>",
);
return; // don't recurse into the type
}
if match_type_parameter(cx, qpath, &paths::BOX) {
span_lint_and_help(
cx,
RC_BOX,
hir_ty.span,
"usage of `Rc<Box<T>>`",
"Rc<Box<T>> can be simplified to Rc<T>",
);
return; // don't recurse into the type
}
} else if cx.tcx.is_diagnostic_item(Symbol::intern("vec_type"), def_id) {
if_chain! {
// Get the _ part of Vec<_>
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/utils/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub const BEGIN_PANIC: [&str; 3] = ["std", "panicking", "begin_panic"];
pub const BEGIN_PANIC_FMT: [&str; 3] = ["std", "panicking", "begin_panic_fmt"];
pub const BINARY_HEAP: [&str; 4] = ["alloc", "collections", "binary_heap", "BinaryHeap"];
pub const BORROW_TRAIT: [&str; 3] = ["core", "borrow", "Borrow"];
pub const BOX: [&str; 3] = ["alloc", "boxed", "Box"];
pub const BTREEMAP: [&str; 5] = ["alloc", "collections", "btree", "map", "BTreeMap"];
pub const BTREEMAP_ENTRY: [&str; 5] = ["alloc", "collections", "btree", "map", "Entry"];
pub const BTREESET: [&str; 5] = ["alloc", "collections", "btree", "set", "BTreeSet"];
Expand Down
16 changes: 15 additions & 1 deletion src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;

// begin lint list, do not remove this comment, it’s used in `update_lints`
pub const ALL_LINTS: [Lint; 362] = [
pub const ALL_LINTS: [Lint; 364] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
Expand Down Expand Up @@ -1722,6 +1722,20 @@ pub const ALL_LINTS: [Lint; 362] = [
deprecation: None,
module: "ranges",
},
Lint {
name: "rc_box",
group: "perf",
desc: "an Rc of Box",
deprecation: None,
module: "types",
},
Lint {
name: "rc_rc",
group: "perf",
desc: "an Rc of Rc",
deprecation: None,
module: "types",
},
Lint {
name: "redundant_clone",
group: "perf",
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/rc_rc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
use std::boxed::Box;
use std::rc::Rc;

pub fn test(a: Rc<Rc<bool>>) {}

pub fn test2(a: Rc<Box<bool>>) {}

fn main() {}
20 changes: 20 additions & 0 deletions tests/ui/rc_rc.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: usage of `Rc<Rc<T>>`
--> $DIR/rc_rc.rs:4:16
|
LL | pub fn test(a: Rc<Rc<bool>>) {}
| ^^^^^^^^^^^^
|
= note: `-D clippy::rc-rc` implied by `-D warnings`
= help: Rc<Rc<T>> can be simplified to Rc<T>

error: usage of `Rc<Box<T>>`
--> $DIR/rc_rc.rs:6:17
|
LL | pub fn test2(a: Rc<Box<bool>>) {}
| ^^^^^^^^^^^^^
|
= note: `-D clippy::rc-box` implied by `-D warnings`
= help: Rc<Box<T>> can be simplified to Rc<T>

error: aborting due to 2 previous errors

0 comments on commit 81a9113

Please sign in to comment.