Skip to content

Commit f784067

Browse files
chansukechansuke
committed
refactor: delete unnecessary comparison
1 parent bc17228 commit f784067

File tree

6 files changed

+53
-50
lines changed

6 files changed

+53
-50
lines changed

‎clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
603603
crate::unnamed_address::FN_ADDRESS_COMPARISONS_INFO,
604604
crate::unnamed_address::VTABLE_ADDRESS_COMPARISONS_INFO,
605605
crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO,
606+
crate::unnecessary_reserve::UNNECESSARY_RESERVE_INFO,
606607
crate::unnecessary_self_imports::UNNECESSARY_SELF_IMPORTS_INFO,
607608
crate::unnecessary_wraps::UNNECESSARY_WRAPS_INFO,
608609
crate::unnested_or_patterns::UNNESTED_OR_PATTERNS_INFO,

‎clippy_lints/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -912,6 +912,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
912912
store.register_late_pass(|_| Box::new(size_of_ref::SizeOfRef));
913913
store.register_late_pass(|_| Box::new(multiple_unsafe_ops_per_block::MultipleUnsafeOpsPerBlock));
914914
store.register_late_pass(move || Box::new(unnecessary_reserve::UnnecessaryReserve::new(msrv)));
915+
store.register_late_pass(move |_| Box::new(unnecessary_reserve::UnnecessaryReserve::new(msrv())));
915916
// add lints here, do not remove this comment, it's used in `new_lint`
916917
}
917918

‎clippy_lints/src/unnecessary_reserve.rs

Lines changed: 20 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
2-
use clippy_utils::{match_def_path, meets_msrv, msrvs, paths, visitors::expr_visitor_no_bodies};
2+
use clippy_utils::msrvs::{self, Msrv};
3+
use clippy_utils::{match_def_path, paths, visitors::for_each_expr, SpanlessEq};
4+
use core::ops::ControlFlow;
35
use rustc_errors::Applicability;
4-
use rustc_hir::{intravisit::Visitor, Block, ExprKind, QPath, StmtKind};
6+
use rustc_hir::{Block, ExprKind, PathSegment, StmtKind};
57
use rustc_lint::{LateContext, LateLintPass};
68
use rustc_middle::ty;
7-
use rustc_semver::RustcVersion;
89
use rustc_session::{declare_tool_lint, impl_lint_pass};
910
use rustc_span::sym;
1011

1112
declare_clippy_lint! {
1213
/// ### What it does
14+
///
1315
/// This lint checks for a call to `reserve` before `extend` on a `Vec` or `VecDeque`.
1416
/// ### Why is this bad?
1517
/// Since Rust 1.62, `extend` implicitly calls `reserve`
18+
///
1619
/// ### Example
1720
/// ```rust
1821
/// let mut vec: Vec<usize> = vec![];
@@ -32,31 +35,27 @@ declare_clippy_lint! {
3235
"calling `reserve` before `extend` on a `Vec` or `VecDeque`, when it will be called implicitly"
3336
}
3437

38+
impl_lint_pass!(UnnecessaryReserve => [UNNECESSARY_RESERVE]);
39+
3540
pub struct UnnecessaryReserve {
36-
msrv: Option<RustcVersion>,
41+
msrv: Msrv,
3742
}
38-
3943
impl UnnecessaryReserve {
40-
#[must_use]
41-
pub fn new(msrv: Option<RustcVersion>) -> Self {
44+
pub fn new(msrv: Msrv) -> Self {
4245
Self { msrv }
4346
}
4447
}
4548

46-
impl_lint_pass!(UnnecessaryReserve => [UNNECESSARY_RESERVE]);
47-
4849
impl<'tcx> LateLintPass<'tcx> for UnnecessaryReserve {
4950
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &Block<'tcx>) {
50-
if !meets_msrv(self.msrv, msrvs::UNNECESSARY_RESERVE) {
51+
if !self.msrv.meets(msrvs::CHECK_UNNECESSARY_RESERVE) {
5152
return;
5253
}
5354

5455
for (idx, stmt) in block.stmts.iter().enumerate() {
5556
if let StmtKind::Semi(semi_expr) = stmt.kind
56-
&& let ExprKind::MethodCall(_, [struct_calling_on, _], _) = semi_expr.kind
57-
&& let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(semi_expr.hir_id)
58-
&& (match_def_path(cx, expr_def_id, &paths::VEC_RESERVE) ||
59-
match_def_path(cx, expr_def_id, &paths::VEC_DEQUE_RESERVE))
57+
&& let ExprKind::MethodCall(PathSegment { ident: method, .. }, struct_calling_on, _, _) = semi_expr.kind
58+
&& method.name.as_str() == "reserve"
6059
&& acceptable_type(cx, struct_calling_on)
6160
&& let Some(next_stmt_span) = check_extend_method(cx, block, idx, struct_calling_on)
6261
&& !next_stmt_span.from_expansion()
@@ -102,50 +101,36 @@ fn check_extend_method(
102101
) -> Option<rustc_span::Span> {
103102
let mut read_found = false;
104103
let next_stmt_span;
104+
let mut spanless_eq = SpanlessEq::new(cx);
105105

106-
let mut visitor = expr_visitor_no_bodies(|expr| {
107-
if let ExprKind::MethodCall(_, [struct_calling_on, _], _) = expr.kind
106+
let _: Option<!> = for_each_expr(block, |expr| {
107+
if let ExprKind::MethodCall(_, struct_calling_on, _,_) = expr.kind
108108
&& let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
109109
&& match_def_path(cx, expr_def_id, &paths::ITER_EXTEND)
110110
&& acceptable_type(cx, struct_calling_on)
111-
&& equal_ident(struct_calling_on, struct_expr)
111+
// Check that both expr are equal
112+
&& spanless_eq.eq_expr(struct_calling_on, struct_expr)
112113
{
113114
read_found = true;
114115
}
115-
!read_found
116+
let _ = !read_found;
117+
ControlFlow::Continue(())
116118
});
117119

118120
if idx == block.stmts.len() - 1 {
119121
if let Some(e) = block.expr {
120-
visitor.visit_expr(e);
121122
next_stmt_span = e.span;
122123
} else {
123124
return None;
124125
}
125126
} else {
126127
let next_stmt = &block.stmts[idx + 1];
127-
visitor.visit_stmt(next_stmt);
128128
next_stmt_span = next_stmt.span;
129129
}
130-
drop(visitor);
131130

132131
if read_found {
133132
return Some(next_stmt_span);
134133
}
135134

136135
None
137136
}
138-
139-
#[must_use]
140-
fn equal_ident(left: &rustc_hir::Expr<'_>, right: &rustc_hir::Expr<'_>) -> bool {
141-
fn ident_name(expr: &rustc_hir::Expr<'_>) -> Option<rustc_span::Symbol> {
142-
if let ExprKind::Path(QPath::Resolved(None, inner_path)) = expr.kind
143-
&& let [inner_seg] = inner_path.segments
144-
{
145-
return Some(inner_seg.ident.name);
146-
}
147-
None
148-
}
149-
150-
ident_name(left) == ident_name(right)
151-
}

‎clippy_utils/src/msrvs.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ macro_rules! msrv_aliases {
2020
// names may refer to stabilized feature flags or library items
2121
msrv_aliases! {
2222
1,65,0 { LET_ELSE }
23-
1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE, UNNECESSARY_RESERVE }
23+
1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE, CHECK_UNNECESSARY_RESERVE }
2424
1,58,0 { FORMAT_ARGS_CAPTURE, PATTERN_TRAIT_CHAR_ARRAY }
2525
1,55,0 { SEEK_REWIND }
2626
1,53,0 { OR_PATTERNS, MANUAL_BITS, BTREE_MAP_RETAIN, BTREE_SET_RETAIN, ARRAY_INTO_ITERATOR }

‎tests/ui/unnecessary_reserve.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,27 +6,27 @@ use std::collections::VecDeque;
66

77
fn main() {
88
vec_reserve();
9-
vec_deque_reserve();
10-
hash_map_reserve();
11-
msrv_1_62();
9+
//vec_deque_reserve();
10+
//hash_map_reserve();
11+
//msrv_1_62();
1212
}
1313

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

18-
// do lint
18+
// do not lint
1919
vec.reserve(1);
2020
vec.extend([1]);
2121

22-
// do lint
22+
//// do lint
2323
vec.reserve(array.len());
2424
vec.extend(array);
2525

2626
// do lint
2727
{
28-
vec.reserve(1);
29-
vec.extend([1])
28+
vec.reserve(array.len());
29+
vec.extend(array)
3030
};
3131

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

47-
// do lint
47+
// do not lint
4848
vec_deque.reserve(1);
4949
vec_deque.extend([1]);
5050

5151
// do lint
5252
vec_deque.reserve(array.len());
5353
vec_deque.extend(array);
5454

55-
// do lint
55+
// do not lint
5656
{
5757
vec_deque.reserve(1);
5858
vec_deque.extend([1])

‎tests/ui/unnecessary_reserve.stderr

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,21 @@ LL | vec.reserve(array.len());
1616
LL | vec.extend(array);
1717
| ^^^^^^^^^^^^^^^^^^
1818

19+
error: unnecessary call to `reserve`
20+
--> $DIR/unnecessary_reserve.rs:34:5
21+
|
22+
LL | vec.reserve(array.len());
23+
| ------------------------ help: remove this line
24+
LL | vec.push(1);
25+
| ^^^^^^^^^^^^
26+
1927
error: unnecessary call to `reserve`
2028
--> $DIR/unnecessary_reserve.rs:29:9
2129
|
22-
LL | vec.reserve(1);
23-
| -------------- help: remove this line
24-
LL | vec.extend([1])
25-
| ^^^^^^^^^^^^^^^
30+
LL | vec.reserve(array.len());
31+
| ------------------------ help: remove this line
32+
LL | vec.extend(array)
33+
| ^^^^^^^^^^^^^^^^^
2634

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

51+
error: unnecessary call to `reserve`
52+
--> $DIR/unnecessary_reserve.rs:63:5
53+
|
54+
LL | vec_deque.reserve(array.len() + 1);
55+
| ---------------------------------- help: remove this line
56+
LL | vec_deque.push_back(1);
57+
| ^^^^^^^^^^^^^^^^^^^^^^^
58+
4359
error: unnecessary call to `reserve`
4460
--> $DIR/unnecessary_reserve.rs:58:9
4561
|
@@ -48,5 +64,5 @@ LL | vec_deque.reserve(1);
4864
LL | vec_deque.extend([1])
4965
| ^^^^^^^^^^^^^^^^^^^^^
5066

51-
error: aborting due to 6 previous errors
67+
error: aborting due to 8 previous errors
5268

0 commit comments

Comments
 (0)