Skip to content

Commit

Permalink
refactor: delete unnecessary comparison
Browse files Browse the repository at this point in the history
  • Loading branch information
chansuke committed Jan 4, 2023
1 parent 9869bac commit 9b112b2
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 51 deletions.
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::unnamed_address::FN_ADDRESS_COMPARISONS_INFO,
crate::unnamed_address::VTABLE_ADDRESS_COMPARISONS_INFO,
crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO,
crate::unnecessary_reserve::UNNECESSARY_RESERVE_INFO,
crate::unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS_INFO,
crate::unnecessary_wraps::UNNECESSARY_WRAPS_INFO,
crate::unnested_or_patterns::UNNESTED_OR_PATTERNS_INFO,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|_| Box::new(fn_null_check::FnNullCheck));
store.register_late_pass(|_| Box::new(permissions_set_readonly_false::PermissionsSetReadonlyFalse));
store.register_late_pass(|_| Box::new(size_of_ref::SizeOfRef));
store.register_late_pass(move || Box::new(unnecessary_reserve::UnnecessaryReserve::new(msrv)));
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
55 changes: 20 additions & 35 deletions clippy_lints/src/unnecessary_reserve.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::{match_def_path, meets_msrv, msrvs, paths, visitors::expr_visitor_no_bodies};
use clippy_utils::msrvs::{self, Msrv};
use clippy_utils::{match_def_path, paths, visitors::for_each_expr, SpanlessEq};
use core::ops::ControlFlow;
use rustc_errors::Applicability;
use rustc_hir::{intravisit::Visitor, Block, ExprKind, QPath, StmtKind};
use rustc_hir::{Block, ExprKind, PathSegment, 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 a call to `reserve` before `extend` on a `Vec` or `VecDeque`.
/// ### Why is this bad?
/// Since Rust 1.62, `extend` implicitly calls `reserve`
///
/// ### Example
/// ```rust
/// let mut vec: Vec<usize> = vec![];
Expand All @@ -32,31 +35,27 @@ declare_clippy_lint! {
"calling `reserve` before `extend` on a `Vec` or `VecDeque`, when it will be called implicitly"
}

impl_lint_pass!(UnnecessaryReserve => [UNNECESSARY_RESERVE]);

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

impl UnnecessaryReserve {
#[must_use]
pub fn new(msrv: Option<RustcVersion>) -> Self {
pub fn new(msrv: Msrv) -> 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) {
if !self.msrv.meets(msrvs::CHECK_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))
&& let ExprKind::MethodCall(PathSegment { ident: method, .. }, struct_calling_on, _, _) = semi_expr.kind
&& method.name.as_str() == "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()
Expand Down Expand Up @@ -102,50 +101,36 @@ fn check_extend_method(
) -> Option<rustc_span::Span> {
let mut read_found = false;
let next_stmt_span;
let mut spanless_eq = SpanlessEq::new(cx);

let mut visitor = expr_visitor_no_bodies(|expr| {
if let ExprKind::MethodCall(_, [struct_calling_on, _], _) = expr.kind
let _: Option<!> = for_each_expr(block, |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)
// Check that both expr are equal
&& spanless_eq.eq_expr(struct_calling_on, struct_expr)
{
read_found = true;
}
!read_found
let _ = !read_found;
ControlFlow::Continue(())
});

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)
}
2 changes: 1 addition & 1 deletion clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ macro_rules! msrv_aliases {
// names may refer to stabilized feature flags or library items
msrv_aliases! {
1,65,0 { LET_ELSE }
1,62,0 { BOOL_THEN_SOME, UNNECESSARY_RESERVE }
1,62,0 { BOOL_THEN_SOME, CHECK_UNNECESSARY_RESERVE }
1,58,0 { FORMAT_ARGS_CAPTURE, PATTERN_TRAIT_CHAR_ARRAY }
1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN, ARRAY_INTO_ITERATOR }
1,52,0 { STR_SPLIT_ONCE, REM_EUCLID_CONST }
Expand Down
18 changes: 9 additions & 9 deletions tests/ui/unnecessary_reserve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,27 @@ use std::collections::VecDeque;

fn main() {
vec_reserve();
vec_deque_reserve();
hash_map_reserve();
msrv_1_62();
//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
// do not lint
vec.reserve(1);
vec.extend([1]);

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

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

// do not lint
Expand All @@ -44,15 +44,15 @@ fn vec_deque_reserve() {
let mut vec_deque: VecDeque<usize> = [1].into();
let array: &[usize] = &[1, 2];

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

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

// do lint
// do not lint
{
vec_deque.reserve(1);
vec_deque.extend([1])
Expand Down
26 changes: 21 additions & 5 deletions tests/ui/unnecessary_reserve.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,21 @@ LL | vec.reserve(array.len());
LL | vec.extend(array);
| ^^^^^^^^^^^^^^^^^^

error: unnecessary call to `reserve`
--> $DIR/unnecessary_reserve.rs:34:5
|
LL | vec.reserve(array.len());
| ------------------------ help: remove this line
LL | vec.push(1);
| ^^^^^^^^^^^^

error: unnecessary call to `reserve`
--> $DIR/unnecessary_reserve.rs:29:9
|
LL | vec.reserve(1);
| -------------- help: remove this line
LL | vec.extend([1])
| ^^^^^^^^^^^^^^^
LL | vec.reserve(array.len());
| ------------------------ help: remove this line
LL | vec.extend(array)
| ^^^^^^^^^^^^^^^^^

error: unnecessary call to `reserve`
--> $DIR/unnecessary_reserve.rs:49:5
Expand All @@ -40,6 +48,14 @@ LL | vec_deque.reserve(array.len());
LL | vec_deque.extend(array);
| ^^^^^^^^^^^^^^^^^^^^^^^^

error: unnecessary call to `reserve`
--> $DIR/unnecessary_reserve.rs:63:5
|
LL | vec_deque.reserve(array.len() + 1);
| ---------------------------------- help: remove this line
LL | vec_deque.push_back(1);
| ^^^^^^^^^^^^^^^^^^^^^^^

error: unnecessary call to `reserve`
--> $DIR/unnecessary_reserve.rs:58:9
|
Expand All @@ -48,5 +64,5 @@ LL | vec_deque.reserve(1);
LL | vec_deque.extend([1])
| ^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 6 previous errors
error: aborting due to 8 previous errors

0 comments on commit 9b112b2

Please sign in to comment.