mirror of
https://github.com/rust-lang/rust-clippy
synced 2024-11-22 12:43:18 +00:00
useless Rc<Rc<T>>, Rc<Box<T>>, Rc<&T>, Box<&T>
This commit is contained in:
parent
42796e11c5
commit
f8e892db5e
10 changed files with 259 additions and 8 deletions
|
@ -1433,6 +1433,7 @@ 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
|
||||
[`redundant_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation
|
||||
[`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
|
||||
|
|
|
@ -811,6 +811,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
&types::LET_UNIT_VALUE,
|
||||
&types::LINKEDLIST,
|
||||
&types::OPTION_OPTION,
|
||||
&types::REDUNDANT_ALLOCATION,
|
||||
&types::TYPE_COMPLEXITY,
|
||||
&types::UNIT_ARG,
|
||||
&types::UNIT_CMP,
|
||||
|
@ -1376,6 +1377,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(&types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION),
|
||||
LintId::of(&types::IMPLICIT_HASHER),
|
||||
LintId::of(&types::LET_UNIT_VALUE),
|
||||
LintId::of(&types::REDUNDANT_ALLOCATION),
|
||||
LintId::of(&types::TYPE_COMPLEXITY),
|
||||
LintId::of(&types::UNIT_ARG),
|
||||
LintId::of(&types::UNIT_CMP),
|
||||
|
@ -1660,6 +1662,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||
LintId::of(&slow_vector_initialization::SLOW_VECTOR_INITIALIZATION),
|
||||
LintId::of(&trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF),
|
||||
LintId::of(&types::BOX_VEC),
|
||||
LintId::of(&types::REDUNDANT_ALLOCATION),
|
||||
LintId::of(&vec::USELESS_VEC),
|
||||
]);
|
||||
|
||||
|
|
|
@ -171,11 +171,35 @@ declare_clippy_lint! {
|
|||
"a borrow of a boxed type"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Checks for use of redundant allocations anywhere in the code.
|
||||
///
|
||||
/// **Why is this bad?** Expressions such as `Rc<&T>`, `Rc<Rc<T>>`, `Rc<Box<T>>`, `Box<&T>`
|
||||
/// add an unnecessary level of indirection.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// # use std::rc::Rc;
|
||||
/// fn foo(bar: Rc<&usize>) {}
|
||||
/// ```
|
||||
///
|
||||
/// Better:
|
||||
///
|
||||
/// ```rust
|
||||
/// fn foo(bar: &usize) {}
|
||||
/// ```
|
||||
pub REDUNDANT_ALLOCATION,
|
||||
perf,
|
||||
"redundant allocation"
|
||||
}
|
||||
|
||||
pub struct Types {
|
||||
vec_box_size_threshold: u64,
|
||||
}
|
||||
|
||||
impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX]);
|
||||
impl_lint_pass!(Types => [BOX_VEC, VEC_BOX, OPTION_OPTION, LINKEDLIST, BORROWED_BOX, REDUNDANT_ALLOCATION]);
|
||||
|
||||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Types {
|
||||
fn check_fn(
|
||||
|
@ -217,7 +241,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Types {
|
|||
}
|
||||
|
||||
/// Checks if `qpath` has last segment with type parameter matching `path`
|
||||
fn match_type_parameter(cx: &LateContext<'_, '_>, qpath: &QPath<'_>, path: &[&str]) -> bool {
|
||||
fn match_type_parameter(cx: &LateContext<'_, '_>, qpath: &QPath<'_>, path: &[&str]) -> Option<Span> {
|
||||
let last = last_path_segment(qpath);
|
||||
if_chain! {
|
||||
if let Some(ref params) = last.args;
|
||||
|
@ -230,10 +254,27 @@ fn match_type_parameter(cx: &LateContext<'_, '_>, qpath: &QPath<'_>, path: &[&st
|
|||
if let Some(did) = qpath_res(cx, qpath, ty.hir_id).opt_def_id();
|
||||
if match_def_path(cx, did, path);
|
||||
then {
|
||||
return true;
|
||||
return Some(ty.span);
|
||||
}
|
||||
}
|
||||
false
|
||||
None
|
||||
}
|
||||
|
||||
fn match_borrows_parameter(_cx: &LateContext<'_, '_>, qpath: &QPath<'_>) -> Option<Span> {
|
||||
let last = last_path_segment(qpath);
|
||||
if_chain! {
|
||||
if let Some(ref params) = last.args;
|
||||
if !params.parenthesized;
|
||||
if let Some(ty) = params.args.iter().find_map(|arg| match arg {
|
||||
GenericArg::Type(ty) => Some(ty),
|
||||
_ => None,
|
||||
});
|
||||
if let TyKind::Rptr(..) = ty.kind;
|
||||
then {
|
||||
return Some(ty.span);
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
impl Types {
|
||||
|
@ -257,6 +298,7 @@ impl Types {
|
|||
/// The parameter `is_local` distinguishes the context of the type; types from
|
||||
/// local bindings should only be checked for the `BORROWED_BOX` lint.
|
||||
#[allow(clippy::too_many_lines)]
|
||||
#[allow(clippy::cognitive_complexity)]
|
||||
fn check_ty(&mut self, cx: &LateContext<'_, '_>, hir_ty: &hir::Ty<'_>, is_local: bool) {
|
||||
if hir_ty.span.from_expansion() {
|
||||
return;
|
||||
|
@ -267,7 +309,19 @@ impl Types {
|
|||
let res = qpath_res(cx, qpath, hir_id);
|
||||
if let Some(def_id) = res.opt_def_id() {
|
||||
if Some(def_id) == cx.tcx.lang_items().owned_box() {
|
||||
if match_type_parameter(cx, qpath, &paths::VEC) {
|
||||
if let Some(span) = match_borrows_parameter(cx, qpath) {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
REDUNDANT_ALLOCATION,
|
||||
hir_ty.span,
|
||||
"usage of `Box<&T>`",
|
||||
"try",
|
||||
snippet(cx, span, "..").to_string(),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
return; // don't recurse into the type
|
||||
}
|
||||
if match_type_parameter(cx, qpath, &paths::VEC).is_some() {
|
||||
span_lint_and_help(
|
||||
cx,
|
||||
BOX_VEC,
|
||||
|
@ -277,6 +331,43 @@ impl Types {
|
|||
);
|
||||
return; // don't recurse into the type
|
||||
}
|
||||
} else if Some(def_id) == cx.tcx.lang_items().rc() {
|
||||
if let Some(span) = match_type_parameter(cx, qpath, &paths::RC) {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
REDUNDANT_ALLOCATION,
|
||||
hir_ty.span,
|
||||
"usage of `Rc<Rc<T>>`",
|
||||
"try",
|
||||
snippet(cx, span, "..").to_string(),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
return; // don't recurse into the type
|
||||
}
|
||||
if let Some(span) = match_type_parameter(cx, qpath, &paths::BOX) {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
REDUNDANT_ALLOCATION,
|
||||
hir_ty.span,
|
||||
"usage of `Rc<Box<T>>`",
|
||||
"try",
|
||||
snippet(cx, span, "..").to_string(),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
return; // don't recurse into the type
|
||||
}
|
||||
if let Some(span) = match_borrows_parameter(cx, qpath) {
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
REDUNDANT_ALLOCATION,
|
||||
hir_ty.span,
|
||||
"usage of `Rc<&T>`",
|
||||
"try",
|
||||
snippet(cx, span, "..").to_string(),
|
||||
Applicability::MachineApplicable,
|
||||
);
|
||||
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<_>
|
||||
|
@ -314,7 +405,7 @@ impl Types {
|
|||
}
|
||||
}
|
||||
} else if match_def_path(cx, def_id, &paths::OPTION) {
|
||||
if match_type_parameter(cx, qpath, &paths::OPTION) {
|
||||
if match_type_parameter(cx, qpath, &paths::OPTION).is_some() {
|
||||
span_lint(
|
||||
cx,
|
||||
OPTION_OPTION,
|
||||
|
|
|
@ -10,6 +10,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"];
|
||||
|
|
|
@ -1725,6 +1725,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
|
|||
deprecation: None,
|
||||
module: "ranges",
|
||||
},
|
||||
Lint {
|
||||
name: "redundant_allocation",
|
||||
group: "perf",
|
||||
desc: "redundant allocation",
|
||||
deprecation: None,
|
||||
module: "types",
|
||||
},
|
||||
Lint {
|
||||
name: "redundant_clone",
|
||||
group: "perf",
|
||||
|
|
|
@ -1,6 +1,6 @@
|
|||
// run-rustfix
|
||||
#![feature(never_type)]
|
||||
#![allow(unused_mut)]
|
||||
#![allow(unused_mut, clippy::redundant_allocation)]
|
||||
#![warn(clippy::must_use_candidate)]
|
||||
use std::rc::Rc;
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
|
|
|
@ -1,6 +1,6 @@
|
|||
// run-rustfix
|
||||
#![feature(never_type)]
|
||||
#![allow(unused_mut)]
|
||||
#![allow(unused_mut, clippy::redundant_allocation)]
|
||||
#![warn(clippy::must_use_candidate)]
|
||||
use std::rc::Rc;
|
||||
use std::sync::atomic::{AtomicBool, Ordering};
|
||||
|
|
48
tests/ui/redundant_allocation.fixed
Normal file
48
tests/ui/redundant_allocation.fixed
Normal file
|
@ -0,0 +1,48 @@
|
|||
// run-rustfix
|
||||
#![warn(clippy::all)]
|
||||
#![allow(clippy::boxed_local, clippy::needless_pass_by_value)]
|
||||
#![allow(clippy::blacklisted_name, unused_variables, dead_code)]
|
||||
|
||||
use std::boxed::Box;
|
||||
use std::rc::Rc;
|
||||
|
||||
pub struct MyStruct {}
|
||||
|
||||
pub struct SubT<T> {
|
||||
foo: T,
|
||||
}
|
||||
|
||||
pub enum MyEnum {
|
||||
One,
|
||||
Two,
|
||||
}
|
||||
|
||||
// Rc<&T>
|
||||
|
||||
pub fn test1<T>(foo: &T) {}
|
||||
|
||||
pub fn test2(foo: &MyStruct) {}
|
||||
|
||||
pub fn test3(foo: &MyEnum) {}
|
||||
|
||||
pub fn test4_neg(foo: Rc<SubT<&usize>>) {}
|
||||
|
||||
// Rc<Rc<T>>
|
||||
|
||||
pub fn test5(a: Rc<bool>) {}
|
||||
|
||||
// Rc<Box<T>>
|
||||
|
||||
pub fn test6(a: Box<bool>) {}
|
||||
|
||||
// Box<&T>
|
||||
|
||||
pub fn test7<T>(foo: &T) {}
|
||||
|
||||
pub fn test8(foo: &MyStruct) {}
|
||||
|
||||
pub fn test9(foo: &MyEnum) {}
|
||||
|
||||
pub fn test10_neg(foo: Box<SubT<&usize>>) {}
|
||||
|
||||
fn main() {}
|
48
tests/ui/redundant_allocation.rs
Normal file
48
tests/ui/redundant_allocation.rs
Normal file
|
@ -0,0 +1,48 @@
|
|||
// run-rustfix
|
||||
#![warn(clippy::all)]
|
||||
#![allow(clippy::boxed_local, clippy::needless_pass_by_value)]
|
||||
#![allow(clippy::blacklisted_name, unused_variables, dead_code)]
|
||||
|
||||
use std::boxed::Box;
|
||||
use std::rc::Rc;
|
||||
|
||||
pub struct MyStruct {}
|
||||
|
||||
pub struct SubT<T> {
|
||||
foo: T,
|
||||
}
|
||||
|
||||
pub enum MyEnum {
|
||||
One,
|
||||
Two,
|
||||
}
|
||||
|
||||
// Rc<&T>
|
||||
|
||||
pub fn test1<T>(foo: Rc<&T>) {}
|
||||
|
||||
pub fn test2(foo: Rc<&MyStruct>) {}
|
||||
|
||||
pub fn test3(foo: Rc<&MyEnum>) {}
|
||||
|
||||
pub fn test4_neg(foo: Rc<SubT<&usize>>) {}
|
||||
|
||||
// Rc<Rc<T>>
|
||||
|
||||
pub fn test5(a: Rc<Rc<bool>>) {}
|
||||
|
||||
// Rc<Box<T>>
|
||||
|
||||
pub fn test6(a: Rc<Box<bool>>) {}
|
||||
|
||||
// Box<&T>
|
||||
|
||||
pub fn test7<T>(foo: Box<&T>) {}
|
||||
|
||||
pub fn test8(foo: Box<&MyStruct>) {}
|
||||
|
||||
pub fn test9(foo: Box<&MyEnum>) {}
|
||||
|
||||
pub fn test10_neg(foo: Box<SubT<&usize>>) {}
|
||||
|
||||
fn main() {}
|
52
tests/ui/redundant_allocation.stderr
Normal file
52
tests/ui/redundant_allocation.stderr
Normal file
|
@ -0,0 +1,52 @@
|
|||
error: usage of `Rc<&T>`
|
||||
--> $DIR/redundant_allocation.rs:22:22
|
||||
|
|
||||
LL | pub fn test1<T>(foo: Rc<&T>) {}
|
||||
| ^^^^^^ help: try: `&T`
|
||||
|
|
||||
= note: `-D clippy::redundant-allocation` implied by `-D warnings`
|
||||
|
||||
error: usage of `Rc<&T>`
|
||||
--> $DIR/redundant_allocation.rs:24:19
|
||||
|
|
||||
LL | pub fn test2(foo: Rc<&MyStruct>) {}
|
||||
| ^^^^^^^^^^^^^ help: try: `&MyStruct`
|
||||
|
||||
error: usage of `Rc<&T>`
|
||||
--> $DIR/redundant_allocation.rs:26:19
|
||||
|
|
||||
LL | pub fn test3(foo: Rc<&MyEnum>) {}
|
||||
| ^^^^^^^^^^^ help: try: `&MyEnum`
|
||||
|
||||
error: usage of `Rc<Rc<T>>`
|
||||
--> $DIR/redundant_allocation.rs:32:17
|
||||
|
|
||||
LL | pub fn test5(a: Rc<Rc<bool>>) {}
|
||||
| ^^^^^^^^^^^^ help: try: `Rc<bool>`
|
||||
|
||||
error: usage of `Rc<Box<T>>`
|
||||
--> $DIR/redundant_allocation.rs:36:17
|
||||
|
|
||||
LL | pub fn test6(a: Rc<Box<bool>>) {}
|
||||
| ^^^^^^^^^^^^^ help: try: `Box<bool>`
|
||||
|
||||
error: usage of `Box<&T>`
|
||||
--> $DIR/redundant_allocation.rs:40:22
|
||||
|
|
||||
LL | pub fn test7<T>(foo: Box<&T>) {}
|
||||
| ^^^^^^^ help: try: `&T`
|
||||
|
||||
error: usage of `Box<&T>`
|
||||
--> $DIR/redundant_allocation.rs:42:19
|
||||
|
|
||||
LL | pub fn test8(foo: Box<&MyStruct>) {}
|
||||
| ^^^^^^^^^^^^^^ help: try: `&MyStruct`
|
||||
|
||||
error: usage of `Box<&T>`
|
||||
--> $DIR/redundant_allocation.rs:44:19
|
||||
|
|
||||
LL | pub fn test9(foo: Box<&MyEnum>) {}
|
||||
| ^^^^^^^^^^^^ help: try: `&MyEnum`
|
||||
|
||||
error: aborting due to 8 previous errors
|
||||
|
Loading…
Reference in a new issue