Skip to content

Commit

Permalink
add [unnecessary_reserve] lint
Browse files Browse the repository at this point in the history
  • Loading branch information
kyoto7250 committed Jun 30, 2022
1 parent 4198013 commit fa604eb
Show file tree
Hide file tree
Showing 9 changed files with 285 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3813,6 +3813,7 @@ Released 2018-09-13
[`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed
[`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation
[`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings
[`unnecessary_reserve`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_reserve
[`unnecessary_self_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_self_imports
[`unnecessary_sort_by`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_sort_by
[`unnecessary_to_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,7 @@ store.register_lints(&[
unnamed_address::FN_ADDRESS_COMPARISONS,
unnamed_address::VTABLE_ADDRESS_COMPARISONS,
unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS,
unnecessary_reserve::UNNECESSARY_RESERVE,
unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS,
unnecessary_sort_by::UNNECESSARY_SORT_BY,
unnecessary_wraps::UNNECESSARY_WRAPS,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_pedantic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
LintId::of(types::LINKEDLIST),
LintId::of(types::OPTION_OPTION),
LintId::of(unicode::UNICODE_NOT_NFC),
LintId::of(unnecessary_reserve::UNNECESSARY_RESERVE),
LintId::of(unnecessary_wraps::UNNECESSARY_WRAPS),
LintId::of(unnested_or_patterns::UNNESTED_OR_PATTERNS),
LintId::of(unused_async::UNUSED_ASYNC),
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ mod unit_return_expecting_ord;
mod unit_types;
mod unnamed_address;
mod unnecessary_owned_empty_strings;
mod unnecessary_reserve;
mod unnecessary_self_imports;
mod unnecessary_sort_by;
mod unnecessary_wraps;
Expand Down Expand Up @@ -913,6 +914,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(move || Box::new(manual_retain::ManualRetain::new(msrv)));
let verbose_bit_mask_threshold = conf.verbose_bit_mask_threshold;
store.register_late_pass(move || Box::new(operators::Operators::new(verbose_bit_mask_threshold)));
store.register_late_pass(move || Box::new(unnecessary_reserve::UnnecessaryReserve::new(msrv)));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
141 changes: 141 additions & 0 deletions clippy_lints/src/unnecessary_reserve.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::{match_def_path, meets_msrv, msrvs, paths, visitors::expr_visitor_no_bodies};
use rustc_hir::{intravisit::Visitor, Block, ExprKind, QPath, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::sym;

declare_clippy_lint! {
/// ### What it does
/// This lint checks for `reserve` before calling the `extend` method.
/// ### Why is this bad?
/// vec::reserve method before vec::extend is no longer makes sense in rustc version >= 1.62
/// ### Example
/// ```rust
/// let mut vec: Vec<usize> = vec![];
/// let array: &[usize] = &[1, 2];
/// vec.reserve(array.len());
/// vec.extend(array);
/// ```
/// Use instead:
/// ```rust
/// let mut vec: Vec<usize> = vec![];
/// let array: &[usize] = &[1, 2];
/// vec.extend(array);
/// ```
#[clippy::version = "1.64.0"]
pub UNNECESSARY_RESERVE,
pedantic,
"`reserve` method before `extend` is no longer makes sense in rustc version >= 1.62"
}

pub struct UnnecessaryReserve {
msrv: Option<RustcVersion>,
}

impl UnnecessaryReserve {
#[must_use]
pub fn new(msrv: Option<RustcVersion>) -> Self {
Self { msrv }
}
}

impl_lint_pass!(UnnecessaryReserve => [UNNECESSARY_RESERVE]);

impl<'tcx> LateLintPass<'tcx> for UnnecessaryReserve {
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &Block<'tcx>) {
if !meets_msrv(self.msrv, msrvs::UNNECESSARY_RESERVE) {
return;
}

for (idx, stmt) in block.stmts.iter().enumerate() {
if let StmtKind::Semi(semi_expr) = stmt.kind
&& let ExprKind::MethodCall(_, [struct_calling_on, _], _) = semi_expr.kind
&& let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(semi_expr.hir_id)
&& (match_def_path(cx, expr_def_id, &paths::VEC_RESERVE) ||
match_def_path(cx, expr_def_id, &paths::VEC_DEQUE_RESERVE))
&& acceptable_type(cx, struct_calling_on)
&& let Some(next_stmt_span) = check_extend_method(cx, block, idx, struct_calling_on)
&& !next_stmt_span.from_expansion()
{
span_lint(
cx,
UNNECESSARY_RESERVE,
next_stmt_span,
"this `reserve` no longer makes sense in rustc version >= 1.62",
);
}
}
}

extract_msrv_attr!(LateContext);
}

#[must_use]
fn acceptable_type(cx: &LateContext<'_>, struct_calling_on: &rustc_hir::Expr<'_>) -> bool {
let acceptable_types = [sym::Vec, sym::VecDeque];
acceptable_types.iter().any(|&acceptable_ty| {
match cx.typeck_results().expr_ty(struct_calling_on).peel_refs().kind() {
ty::Adt(def, _) => cx.tcx.is_diagnostic_item(acceptable_ty, def.did()),
_ => false,
}
})
}

#[must_use]
fn check_extend_method(
cx: &LateContext<'_>,
block: &Block<'_>,
idx: usize,
struct_expr: &rustc_hir::Expr<'_>,
) -> Option<rustc_span::Span> {
let mut read_found = false;
let next_stmt_span;

let mut visitor = expr_visitor_no_bodies(|expr| {
if let ExprKind::MethodCall(_, [struct_calling_on, _], _) = expr.kind
&& let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
&& match_def_path(cx, expr_def_id, &paths::ITER_EXTEND)
&& acceptable_type(cx, struct_calling_on)
&& equal_ident(struct_calling_on, struct_expr)
{
read_found = true;
}
!read_found
});

if idx == block.stmts.len() - 1 {
if let Some(e) = block.expr {
visitor.visit_expr(e);
next_stmt_span = e.span;
} else {
return None;
}
} else {
let next_stmt = &block.stmts[idx + 1];
visitor.visit_stmt(next_stmt);
next_stmt_span = next_stmt.span;
}
drop(visitor);

if read_found {
return Some(next_stmt_span);
}

None
}

#[must_use]
fn equal_ident(left: &rustc_hir::Expr<'_>, right: &rustc_hir::Expr<'_>) -> bool {
fn ident_name(expr: &rustc_hir::Expr<'_>) -> Option<rustc_span::Symbol> {
if let ExprKind::Path(QPath::Resolved(None, inner_path)) = expr.kind
&& let [inner_seg] = inner_path.segments {
return Some(inner_seg.ident.name);
}
None
}

ident_name(left) == ident_name(right)
}
1 change: 1 addition & 0 deletions clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ macro_rules! msrv_aliases {

// names may refer to stabilized feature flags or library items
msrv_aliases! {
1,62,0 { UNNECESSARY_RESERVE }
1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN }
1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST }
1,51,0 { BORROW_AS_PTR, UNSIGNED_ABS }
Expand Down
3 changes: 3 additions & 0 deletions clippy_utils/src/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ pub const IPADDR_V4: [&str; 5] = ["std", "net", "ip", "IpAddr", "V4"];
pub const IPADDR_V6: [&str; 5] = ["std", "net", "ip", "IpAddr", "V6"];
pub const ITER_COUNT: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "count"];
pub const ITER_EMPTY: [&str; 5] = ["core", "iter", "sources", "empty", "Empty"];
pub const ITER_EXTEND: [&str; 6] = ["core", "iter", "traits", "collect", "Extend", "extend"];
pub const ITER_REPEAT: [&str; 5] = ["core", "iter", "sources", "repeat", "repeat"];
#[expect(clippy::invalid_paths)] // internal lints do not know about all external crates
pub const ITERTOOLS_NEXT_TUPLE: [&str; 3] = ["itertools", "Itertools", "next_tuple"];
Expand Down Expand Up @@ -190,6 +191,8 @@ pub const VEC_DEQUE_ITER: [&str; 5] = ["alloc", "collections", "vec_deque", "Vec
pub const VEC_FROM_ELEM: [&str; 3] = ["alloc", "vec", "from_elem"];
pub const VEC_NEW: [&str; 4] = ["alloc", "vec", "Vec", "new"];
pub const VEC_RESIZE: [&str; 4] = ["alloc", "vec", "Vec", "resize"];
pub const VEC_RESERVE: [&str; 4] = ["alloc", "vec", "Vec", "reserve"];
pub const VEC_DEQUE_RESERVE: [&str; 5] = ["alloc", "collections", "vec_deque", "VecDeque", "reserve"];
pub const WEAK_ARC: [&str; 3] = ["alloc", "sync", "Weak"];
pub const WEAK_RC: [&str; 3] = ["alloc", "rc", "Weak"];
pub const PTR_NON_NULL: [&str; 4] = ["core", "ptr", "non_null", "NonNull"];
95 changes: 95 additions & 0 deletions tests/ui/unnecessary_reserve.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
#![warn(clippy::unnecessary_reserve)]
#![feature(custom_inner_attributes)]

use std::collections::HashMap;
use std::collections::VecDeque;

fn main() {
vec_reserve();
vec_deque_reserve();
hash_map_reserve();
msrv_1_62();
}

fn vec_reserve() {
let mut vec: Vec<usize> = vec![];
let array: &[usize] = &[1, 2];

// do lint
vec.reserve(1);
vec.extend([1]);

// do lint
vec.reserve(array.len());
vec.extend(array);

// do lint
{
vec.reserve(1);
vec.extend([1])
};

// do not lint
vec.reserve(array.len());
vec.push(1);
vec.extend(array);

// do not lint
let mut other_vec: Vec<usize> = vec![];
other_vec.reserve(1);
vec.extend([1])
}

fn vec_deque_reserve() {
let mut vec_deque: VecDeque<usize> = [1].into();
let array: &[usize] = &[1, 2];

// do lint
vec_deque.reserve(1);
vec_deque.extend([1]);

// do lint
vec_deque.reserve(array.len());
vec_deque.extend(array);

// do lint
{
vec_deque.reserve(1);
vec_deque.extend([1])
};

// do not lint
vec_deque.reserve(array.len() + 1);
vec_deque.push_back(1);
vec_deque.extend(array);

// do not lint
let mut other_vec_deque: VecDeque<usize> = [1].into();
other_vec_deque.reserve(1);
vec_deque.extend([1])
}

fn hash_map_reserve() {
let mut map: HashMap<usize, usize> = HashMap::new();
let mut other_map: HashMap<usize, usize> = HashMap::new();
// do not lint
map.reserve(other_map.len());
map.extend(other_map);
}

fn msrv_1_62() {
#![clippy::msrv = "1.61"]
let mut vec: Vec<usize> = vec![];
let array: &[usize] = &[1, 2];

// do not lint
vec.reserve(1);
vec.extend([1]);

let mut vec_deque: VecDeque<usize> = [1].into();
let array: &[usize] = &[1, 2];

// do not lint
vec_deque.reserve(1);
vec_deque.extend([1]);
}
40 changes: 40 additions & 0 deletions tests/ui/unnecessary_reserve.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
error: this `reserve` no longer makes sense in rustc version >= 1.62
--> $DIR/unnecessary_reserve.rs:20:5
|
LL | vec.extend([1]);
| ^^^^^^^^^^^^^^^^
|
= note: `-D clippy::unnecessary-reserve` implied by `-D warnings`

error: this `reserve` no longer makes sense in rustc version >= 1.62
--> $DIR/unnecessary_reserve.rs:24:5
|
LL | vec.extend(array);
| ^^^^^^^^^^^^^^^^^^

error: this `reserve` no longer makes sense in rustc version >= 1.62
--> $DIR/unnecessary_reserve.rs:29:9
|
LL | vec.extend([1])
| ^^^^^^^^^^^^^^^

error: this `reserve` no longer makes sense in rustc version >= 1.62
--> $DIR/unnecessary_reserve.rs:49:5
|
LL | vec_deque.extend([1]);
| ^^^^^^^^^^^^^^^^^^^^^^

error: this `reserve` no longer makes sense in rustc version >= 1.62
--> $DIR/unnecessary_reserve.rs:53:5
|
LL | vec_deque.extend(array);
| ^^^^^^^^^^^^^^^^^^^^^^^^

error: this `reserve` no longer makes sense in rustc version >= 1.62
--> $DIR/unnecessary_reserve.rs:58:9
|
LL | vec_deque.extend([1])
| ^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 6 previous errors

0 comments on commit fa604eb

Please sign in to comment.