Add `unnecessary_reserve` lint by chansuke · Pull Request #10157 · rust-lang/rust-clippy

chansuke · 2023-01-04T13:56:16Z

This PR has taken over from #9073.

Fixes #8982.

changelog: new lint: [unnecessary_reserve]
#10157

rustbot · 2023-01-04T13:56:21Z

r? @Manishearth

(rustbot has picked a reviewer for you, use r? to override)

Manishearth · 2023-01-04T18:52:17Z

r? @flip1995 since you were reviewing the previous one

bors · 2023-01-07T00:44:40Z

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

chansuke · 2023-01-07T04:06:57Z

Ah, sorry,this is still WIP

bors · 2023-01-19T15:26:03Z

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

flip1995

Mostly style issues and one test seems to be missing.

flip1995 · 2023-01-31T08:21:01Z

README.md

 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

-[There are over 550 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are over 600 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)


🎉 🎉 🎉 🎉 🎉

flip1995 · 2023-01-31T08:22:20Z

clippy_lints/src/declared_lints.rs

    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,


Shouldn't this be

Suggested change

crate::unnecessary_reserve::UNNECESSARY_RESERVE_INFO,

crate::unnecessary_reserve::UNNECESSARY_RESERVE,

@flip1995 Thank you so much for your review. UNNECESSARY_RESERVE is defined in clippy_lints/src/unnecessary_reserve.rs as impl_lint_pass!(UnnecessaryReserve => [UNNECESSARY_RESERVE]); which is a LintPass and thought that need to declare as an another constants.

flip1995 · 2023-01-31T08:23:49Z

clippy_lints/src/unnecessary_reserve.rs

+
+impl<'tcx> LateLintPass<'tcx> for UnnecessaryReserve {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
+        if !self.msrv.meets(msrvs::CHECK_UNNECESSARY_RESERVE) {


Usually we name the MSRV constant after the feature. I don't know if this has a feature name. If not, I would call it EXTEND_IMPLICIT_RESERVE or something along those lines.

flip1995 · 2023-01-31T08:26:18Z

clippy_lints/src/unnecessary_reserve.rs

+        {
+            read_found = true;
+        }
+        let _ = !read_found;


What does this do?

Oh, trying to flip read_found. I will refactor this

flip1995 · 2023-01-31T08:27:15Z

clippy_utils/src/paths.rs

+pub const VEC_RESERVE: [&str; 4] = ["alloc", "vec", "Vec", "reserve"];
+pub const VEC_DEQUE_RESERVE: [&str; 5] = ["alloc", "collections", "vec_deque", "VecDeque", "reserve"];


Those are a lot of paths added here. It may be worth opening a rust-lang/rust PR adding diagnostic items for those.

@flip1995 Thank you so much for your review and suggestion. I will fix these.

flip1995 · 2023-01-31T08:30:50Z

tests/ui/unnecessary_reserve.rs

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


I think one test that is missing is something like

vec.reserve(array1.len()); vec.extend(array2);

This might be covered by this, but I'm not 100% sure.

This should not be linted. Doesn't make much sense to write something like this, but it should be an easy FP to avoid.

bors · 2023-02-03T12:17:04Z

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

bors · 2023-02-23T16:09:19Z

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

bors · 2023-03-07T13:52:42Z

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

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

bors · 2023-04-23T11:09:22Z

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

blyxyas · 2023-10-07T14:06:59Z

@chansuke I'm closing this for inactivity (don't worry, it isn't anyone's fault! Just that Philipp doesn't have time to review anymore ฅ^•ﻌ•^ฅ).

Feel free to re-open this if you're still interested in developing in this branch. But I'd advice you to create a new branch and a new PR.

If you are still interested in this and create another PR, write r? @blyxyas somewhere in the PR body so that it gets assigned to me!

…tems, r=flip1995 [Clippy] Add vec_reserve & vecdeque_reserve diagnostic items I’m currently working on reviving this lint (rust-lang/rust-clippy#10157), and there was [a comment](rust-lang/rust-clippy#10157 (comment)) from `@flip1995` regarding the necessity of adding new diagnostic items.

…tems, r=flip1995 [Clippy] Add vec_reserve & vecdeque_reserve diagnostic items I’m currently working on reviving this lint (rust-lang/rust-clippy#10157), and there was [a comment](rust-lang/rust-clippy#10157 (comment)) from ``@flip1995`` regarding the necessity of adding new diagnostic items.

Rollup merge of rust-lang#136071 - wowinter13:clippy-add-diagnostic-items, r=flip1995 [Clippy] Add vec_reserve & vecdeque_reserve diagnostic items I’m currently working on reviving this lint (rust-lang/rust-clippy#10157), and there was [a comment](rust-lang/rust-clippy#10157 (comment)) from ``@flip1995`` regarding the necessity of adding new diagnostic items.

…lip1995 [Clippy] Add vec_reserve & vecdeque_reserve diagnostic items I’m currently working on reviving this lint (rust-lang/rust-clippy#10157), and there was [a comment](rust-lang/rust-clippy#10157 (comment)) from ``@flip1995`` regarding the necessity of adding new diagnostic items.

…tems, r=flip1995 [Clippy] Add vec_reserve & vecdeque_reserve diagnostic items I’m currently working on reviving this lint (rust-lang/rust-clippy#10157), and there was [a comment](rust-lang/rust-clippy#10157 (comment)) from ``@flip1995`` regarding the necessity of adding new diagnostic items.

rustbot assigned Manishearth Jan 4, 2023

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 4, 2023

rustbot assigned flip1995 and unassigned Manishearth Jan 4, 2023

chansuke changed the title ~~Add unnecessary_reserve lint~~ WIP:Add unnecessary_reserve lint Jan 7, 2023

chansuke force-pushed the issue_8982 branch from a2ef35e to f05a150 Compare January 7, 2023 10:43

chansuke changed the title ~~WIP:Add unnecessary_reserve lint~~ Add unnecessary_reserve lint Jan 7, 2023

chansuke marked this pull request as ready for review January 7, 2023 14:02

chansuke force-pushed the issue_8982 branch 2 times, most recently from 14ca695 to 3f17aa4 Compare January 24, 2023 18:25

flip1995 reviewed Jan 31, 2023

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 Feb 3, 2023

chansuke force-pushed the issue_8982 branch from ab9e51e to 02395c7 Compare February 7, 2023 14:27

chansuke force-pushed the issue_8982 branch 2 times, most recently from f8b338f to c8f604a Compare February 25, 2023 09:46

chansuke requested a review from flip1995 February 25, 2023 20:49

chansuke force-pushed the issue_8982 branch from f44fdba to a942b8b Compare April 11, 2023 13:44

kyoto7250 and others added 4 commits April 11, 2023 22:48

add [unnecessary_reserve] lint

8e73e2a

Update What it dose

ab1a1b3

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

Update description

a980ee8

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

Update Why is this bad

df42bbd

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

kyoto7250 and others added 16 commits April 11, 2023 22:48

use span_lint_and_then

c772ac3

Update warn message

6100e83

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

Update clippy_lints/src/unnecessary_reserve.rs

9a961d5

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

Update clippy_lints/src/unnecessary_reserve.rs

a02df99

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

fix test

47e916f

refactor: delete unnecessary comparison

c332c48

refactor: fix re-daclare logic

390d702

chore: run cargo dev update_lints

6cd1b64

hotfix: replace check_block with check_expr

3ae7b12

hotfix: prevent linting raw int

37fd6b3

refactor: use rustc_span::sym for method name comparison

37eb8c3

hotfix: remove unnecessary path

1f08765

hotfix: update declare_lints name

2da64a9

refactor: update MSRV constant name to follow naming convention

4df0eba

hotfix: rollback declare lints name

452717b

hotfix: add type annotation to let binding

696c11e

chansuke force-pushed the issue_8982 branch from a942b8b to 696c11e Compare April 11, 2023 13:49

hotfix: deduplicate registration

f3dda97

blyxyas closed this Oct 7, 2023

flip1995 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 Oct 9, 2023

wowinter13 mentioned this pull request Jan 25, 2025

[Clippy] Add vec_reserve & vecdeque_reserve diagnostic items rust-lang/rust#136071

Merged

wowinter13 mentioned this pull request Jan 30, 2025

new lint: unnecessary_reserve #14114

Open

	crate::unnecessary_reserve::UNNECESSARY_RESERVE_INFO,
	crate::unnecessary_reserve::UNNECESSARY_RESERVE,

		pub const VEC_RESERVE: [&str; 4] = ["alloc", "vec", "Vec", "reserve"];
		pub const VEC_DEQUE_RESERVE: [&str; 5] = ["alloc", "collections", "vec_deque", "VecDeque", "reserve"];

Conversation

chansuke commented Jan 4, 2023 • edited by xFrednet Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jan 4, 2023

Uh oh!

Manishearth commented Jan 4, 2023

Uh oh!

bors commented Jan 7, 2023

Uh oh!

chansuke commented Jan 7, 2023

Uh oh!

bors commented Jan 19, 2023

Uh oh!

flip1995 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bors commented Feb 3, 2023

Uh oh!

bors commented Feb 23, 2023

Uh oh!

bors commented Mar 7, 2023

Uh oh!

bors commented Apr 23, 2023

Uh oh!

blyxyas commented Oct 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

chansuke commented Jan 4, 2023 •

edited by xFrednet

Loading