From 81a91137924b793487563b1c141535cc5cd55cc6 Mon Sep 17 00:00:00 2001 From: Jacek Pospychala Date: Tue, 17 Mar 2020 22:59:16 +0100 Subject: [PATCH] useless Rc>, Rc> --- CHANGELOG.md | 2 + README.md | 2 +- clippy_lints/src/lib.rs | 6 +++ clippy_lints/src/types.rs | 75 ++++++++++++++++++++++++++++++++- clippy_lints/src/utils/paths.rs | 1 + src/lintlist/mod.rs | 16 ++++++- tests/ui/rc_rc.rs | 8 ++++ tests/ui/rc_rc.stderr | 20 +++++++++ 8 files changed, 127 insertions(+), 3 deletions(-) create mode 100644 tests/ui/rc_rc.rs create mode 100644 tests/ui/rc_rc.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 027a73edf86b..7d847b764696 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/README.md b/README.md index 7d6fcbc90986..0c4b524b3424 100644 --- a/README.md +++ b/README.md @@ -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: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index cc84b9138537..8a9f41c6d05b 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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, @@ -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), @@ -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), ]); diff --git a/clippy_lints/src/types.rs b/clippy_lints/src/types.rs index 680035ec3b6a..c256f2b043a5 100644 --- a/clippy_lints/src/types.rs +++ b/clippy_lints/src/types.rs @@ -194,11 +194,63 @@ declare_clippy_lint! { "a box of borrowed type" } +declare_clippy_lint! { + /// **What it does:** Checks for use of `Rc>` 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>) {} + /// ``` + /// + /// Better: + /// + /// ```rust + /// # use std::rc::Rc; + /// fn foo(bar: Rc) {} + /// ``` + pub RC_RC, + perf, + "an Rc of Rc" +} + +declare_clippy_lint! { + /// **What it does:** Checks for use of `Rc>` 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>) {} + /// ``` + /// + /// Better: + /// + /// ```rust + /// # use std::rc::Rc; + /// # use std::boxed::Box; + /// fn foo(bar: Rc) {} + /// ``` + 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( @@ -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> can be simplified to Rc", + ); + 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>`", + "Rc> can be simplified to Rc", + ); + 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<_> diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 6cb1f694fd5e..18f849d6945a 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -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"]; diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 9879234b4418..9258f6ed0ac5 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -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", @@ -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", diff --git a/tests/ui/rc_rc.rs b/tests/ui/rc_rc.rs new file mode 100644 index 000000000000..c6729118f907 --- /dev/null +++ b/tests/ui/rc_rc.rs @@ -0,0 +1,8 @@ +use std::boxed::Box; +use std::rc::Rc; + +pub fn test(a: Rc>) {} + +pub fn test2(a: Rc>) {} + +fn main() {} diff --git a/tests/ui/rc_rc.stderr b/tests/ui/rc_rc.stderr new file mode 100644 index 000000000000..b21d77c276cd --- /dev/null +++ b/tests/ui/rc_rc.stderr @@ -0,0 +1,20 @@ +error: usage of `Rc>` + --> $DIR/rc_rc.rs:4:16 + | +LL | pub fn test(a: Rc>) {} + | ^^^^^^^^^^^^ + | + = note: `-D clippy::rc-rc` implied by `-D warnings` + = help: Rc> can be simplified to Rc + +error: usage of `Rc>` + --> $DIR/rc_rc.rs:6:17 + | +LL | pub fn test2(a: Rc>) {} + | ^^^^^^^^^^^^^ + | + = note: `-D clippy::rc-box` implied by `-D warnings` + = help: Rc> can be simplified to Rc + +error: aborting due to 2 previous errors +