add `[unnecessary_reserve]` lint #9073

kyoto7250 · 2022-06-30T14:11:01Z

close #8982

This PR adds [unnecessary_reserve] lint.

~~note: This lint targets next stable versions (1.62), so we cannot merge it now.~~

changelog: add [unnecessary_reserve] lint
#9073

rust-highfive · 2022-06-30T14:11:04Z

r? @flip1995

(rust-highfive has picked a reviewer for you, use r? to override)

clippy_lints/src/unnecessary_reserve.rs

Serial-ATA · 2022-06-30T16:29:59Z

clippy_lints/src/unnecessary_reserve.rs

+                    cx,
+                    UNNECESSARY_RESERVE,
+                    next_stmt_span,
+                    "this `reserve` no longer makes sense in rustc version >= 1.62",


This message is shown on the extend call.

I had missed it, Thanks!

Serial-ATA · 2022-06-30T16:32:26Z

clippy_lints/src/unnecessary_reserve.rs

+                && let Some(next_stmt_span) = check_extend_method(cx, block, idx, struct_calling_on)
+                && !next_stmt_span.from_expansion()
+            {
+                span_lint(


Couldn't this have a MaybeIncorrect suggestion?

I fixed in 2c4bca2

Serial-ATA · 2022-06-30T16:33:14Z

clippy_lints/src/unnecessary_reserve.rs

+                && let Some(next_stmt_span) = check_extend_method(cx, block, idx, struct_calling_on)
+                && !next_stmt_span.from_expansion()
+            {
+                span_lint(


It'd be nice to have a note here, so both the reserve and extend calls are visible.

kyoto7250 · 2022-07-01T13:16:06Z

@Serial-ATA

Thanks for your review 👍🏻
I will check and fix.

bors · 2022-07-03T20:22:45Z

☔ The latest upstream changes (presumably #9105) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2022-07-06T09:41:56Z

☔ The latest upstream changes (presumably #9099) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2022-07-15T12:38:28Z

☔ The latest upstream changes (presumably #9103) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2022-07-25T15:38:54Z

☔ The latest upstream changes (presumably #9243) made this pull request unmergeable. Please resolve the merge conflicts.

clippy_lints/src/unnecessary_reserve.rs

bors · 2022-08-02T11:36:34Z

☔ The latest upstream changes (presumably #9264) made this pull request unmergeable. Please resolve the merge conflicts.

Co-authored-by: Alex <69764315+Serial-ATA@users.noreply.github.com>

bors · 2022-08-18T16:42:16Z

☔ The latest upstream changes (presumably #9136) made this pull request unmergeable. Please resolve the merge conflicts.

flip1995

I think this lint would be better placed in the methods module, since you check for a specific method call. That way you also don't have to implement all of the method matching logic.

flip1995 · 2022-08-19T14:28:31Z

clippy_lints/src/unnecessary_reserve.rs

+    /// 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


NIT: readability

Suggested change

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

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

flip1995 · 2022-08-19T14:30:14Z

clippy_utils/src/msrvs.rs

@@ -12,7 +12,7 @@ macro_rules! msrv_aliases {

 // names may refer to stabilized feature flags or library items
 msrv_aliases! {
-    1,62,0 { BOOL_THEN_SOME }
+    1,62,0 { BOOL_THEN_SOME, UNNECESSARY_RESERVE }


This should be named after the feature that was introduced rather than after the lint.

flip1995 · 2022-08-19T14:32:04Z

clippy_lints/src/unnecessary_reserve.rs

+            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) ||


These new paths might not be necessary, if you compare the method name and then check the type of the receiver (struct_calling_on). The type of the receiver can be checked with is_ty_diagnostic_item(..., sym::Vec) and similar for VecDeque.

EDIT: Just saw, that you're already doing this in acceptable_type. So this path comparison is not really necessary. Just compare the method name.

clippy_lints/src/unnecessary_reserve.rs

flip1995 · 2022-08-19T14:36:49Z

clippy_lints/src/unnecessary_reserve.rs

+    struct_expr: &rustc_hir::Expr<'_>,
+) -> Option<rustc_span::Span> {
+    let mut read_found = false;
+    let next_stmt_span;


I would not pre-declare this, but rather use a more Rust like style below with

let next_stmt_span = if idx == block.stmts.len() - 1 {

flip1995 · 2022-08-19T14:38:16Z

clippy_lints/src/unnecessary_reserve.rs

+}
+
+#[must_use]
+fn equal_ident(left: &rustc_hir::Expr<'_>, right: &rustc_hir::Expr<'_>) -> bool {


I'm pretty sure, you can use our SpanlessEq utility for this.

flip1995 · 2022-08-19T14:42:30Z

tests/ui/unnecessary_reserve.rs

+    vec.reserve(1);
+    vec.extend([1]);


I'm unsure if this should lint. I think this should only lint for

vec.reserve(expr.len()); vec.extend(expr);

where expr is just any arbitrary expression where len is called on in the reserve call and then used in the extend call. To check if the expression is the same, you can use SpanlessEq.

Or am I missing something?

flip1995 · 2022-08-19T14:42:50Z

tests/ui/unnecessary_reserve.rs

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


NIT: This should be at the end of the test file.

flip1995 · 2022-08-19T14:43:43Z

tests/ui/unnecessary_reserve.stderr

+  --> $DIR/unnecessary_reserve.rs:29:9
+   |
+LL |         vec.reserve(1);
+   |         -------------- help: remove this line


I think the span of the suggestion is wrong? Shouldn't it include the ; as well?

flip1995 · 2022-08-19T14:48:16Z

clippy_lints/src/unnecessary_reserve.rs

+impl_lint_pass!(UnnecessaryReserve => [UNNECESSARY_RESERVE]);
+
+impl<'tcx> LateLintPass<'tcx> for UnnecessaryReserve {
+    fn check_block(&mut self, cx: &LateContext<'tcx>, block: &Block<'tcx>) {


I'm pretty sure you don't have to check the whole block, but rather just the expression and if you find a reserve call, you can use get_enclosing_block, find the next expression/statement in the block and then just check that. I even doubt that you need a visitor for this.

xFrednet · 2022-12-14T21:54:42Z

Hey @kyoto7250 this is a ping from triage. Are there any updates on this PR? If you're stuck or need assistance, you're always welcome to reach out.

kyoto7250 · 2022-12-16T06:12:20Z

@xFrednet
Sorry, currently I am too busy, so I cannot maintain this PR.
You may close this PR.

I want to re-open and do when I have more time.

xFrednet · 2022-12-17T11:47:30Z

That's totally fine, thank you for the swift response and work, you already put into this!

rust-highfive assigned flip1995 Jun 30, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 30, 2022

kyoto7250 force-pushed the issue_8982 branch 2 times, most recently from 1691b1d to fa604eb Compare June 30, 2022 14:45

Serial-ATA suggested changes Jun 30, 2022

View reviewed changes

kyoto7250 force-pushed the issue_8982 branch from 4f50168 to 2c4bca2 Compare July 4, 2022 13:14

kyoto7250 force-pushed the issue_8982 branch from 2c4bca2 to cee302a Compare July 10, 2022 13:55

kyoto7250 marked this pull request as ready for review July 10, 2022 14:08

kyoto7250 force-pushed the issue_8982 branch from cee302a to 78d5a1d Compare July 19, 2022 01:46

kyoto7250 force-pushed the issue_8982 branch from 78d5a1d to de1622f Compare July 28, 2022 06:37

Serial-ATA suggested changes Jul 28, 2022

View reviewed changes

clippy_lints/src/unnecessary_reserve.rs Outdated Show resolved Hide resolved

clippy_lints/src/unnecessary_reserve.rs Outdated Show resolved Hide resolved

clippy_lints/src/unnecessary_reserve.rs Outdated Show resolved Hide resolved

kyoto7250 and others added 9 commits August 14, 2022 14:07

add [unnecessary_reserve] lint

9c9cbe9

Update What it dose

76c615d

Co-authored-by: Alex <69764315+Serial-ATA@users.noreply.github.com>

Update description

6e10e45

Co-authored-by: Alex <69764315+Serial-ATA@users.noreply.github.com>

Update Why is this bad

404883b

Co-authored-by: Alex <69764315+Serial-ATA@users.noreply.github.com>

use span_lint_and_then

a30e82a

Update warn message

1ec4304

Co-authored-by: Alex <69764315+Serial-ATA@users.noreply.github.com>

Update clippy_lints/src/unnecessary_reserve.rs

380ef8c

Co-authored-by: Alex <69764315+Serial-ATA@users.noreply.github.com>

Update clippy_lints/src/unnecessary_reserve.rs

d0d8a77

Co-authored-by: Alex <69764315+Serial-ATA@users.noreply.github.com>

fix test

42c6bcf

kyoto7250 force-pushed the issue_8982 branch from 1cb9c25 to 42c6bcf Compare August 14, 2022 05:21

flip1995 requested changes Aug 19, 2022

View reviewed changes

flip1995 added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Aug 19, 2022

xFrednet closed this Dec 17, 2022

xFrednet added S-inactive-closed Status: Closed due to inactivity and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Dec 17, 2022

xFrednet mentioned this pull request Dec 17, 2022

Unnecessary Vec/VecDeque reserve lint #8982

Open

chansuke mentioned this pull request Jan 4, 2023

Add unnecessary_reserve lint #10157

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add `[unnecessary_reserve]` lint #9073

add `[unnecessary_reserve]` lint #9073

kyoto7250 commented Jun 30, 2022 •

edited by xFrednet

Loading

rust-highfive commented Jun 30, 2022

Serial-ATA Jun 30, 2022

kyoto7250 Jul 4, 2022

Serial-ATA Jun 30, 2022

kyoto7250 Jul 4, 2022

Serial-ATA Jun 30, 2022

kyoto7250 commented Jul 1, 2022

bors commented Jul 3, 2022

bors commented Jul 6, 2022

bors commented Jul 15, 2022

bors commented Jul 25, 2022

bors commented Aug 2, 2022

bors commented Aug 18, 2022

flip1995 left a comment

flip1995 Aug 19, 2022

flip1995 Aug 19, 2022

flip1995 Aug 19, 2022

flip1995 Aug 19, 2022

flip1995 Aug 19, 2022

flip1995 Aug 19, 2022

flip1995 Aug 19, 2022

flip1995 Aug 19, 2022

flip1995 Aug 19, 2022

xFrednet commented Dec 14, 2022

kyoto7250 commented Dec 16, 2022 •

edited

Loading

xFrednet commented Dec 17, 2022

add [unnecessary_reserve] lint #9073

add [unnecessary_reserve] lint #9073

Conversation

kyoto7250 commented Jun 30, 2022 • edited by xFrednet Loading

rust-highfive commented Jun 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kyoto7250 commented Jul 1, 2022

bors commented Jul 3, 2022

bors commented Jul 6, 2022

bors commented Jul 15, 2022

bors commented Jul 25, 2022

bors commented Aug 2, 2022

bors commented Aug 18, 2022

flip1995 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xFrednet commented Dec 14, 2022

kyoto7250 commented Dec 16, 2022 • edited Loading

xFrednet commented Dec 17, 2022

add `[unnecessary_reserve]` lint #9073

add `[unnecessary_reserve]` lint #9073

kyoto7250 commented Jun 30, 2022 •

edited by xFrednet

Loading

kyoto7250 commented Dec 16, 2022 •

edited

Loading