Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-27 07:00:55 +00:00
Code Issues Projects Releases Packages Wiki Activity

Auto merge of #5349 - jpospychala:useless_rc, r=Manishearth

Browse source 
useless Rc<Rc<T>>, Rc<Box<T>>, Rc<&T>, Box<&T>

refers to  #2394

changelog: Add lints for Rc<Rc<T>> and Rc<Box<T>> and Rc<&T>, Box<&T>

this is based on top of another change #5310 so probably should go after that one.
This commit is contained in:
bors 2020-04-02 03:11:29 +00:00
commit a840d594cc
10 changed files with 259 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
CHANGELOG.md
View file

@ -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

3
clippy_lints/src/lib.rs
View file

@ -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),
]);

103
clippy_lints/src/types.rs
View file

@ -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,

1
clippy_lints/src/utils/paths.rs
View file

@ -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"];

7
src/lintlist/mod.rs
View file

@ -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",

2
tests/ui/must_use_candidates.fixed
View file

@ -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};

2
tests/ui/must_use_candidates.rs
View file

@ -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
View 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
View 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
View 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