Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New lint: iter_count #6791

Merged
merged 9 commits into from
Mar 2, 2021
Merged

New lint: iter_count #6791

merged 9 commits into from
Mar 2, 2021

Conversation

TaKO8Ki
Copy link
Member

@TaKO8Ki TaKO8Ki commented Feb 25, 2021

This pull request adds a new lint named iter_count.


closes #6262

changelog: new lint iter_count

@rust-highfive
Copy link

r? @flip1995

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Feb 25, 2021
@TaKO8Ki TaKO8Ki force-pushed the iter-count branch 2 times, most recently from 41814ed to 53cc66c Compare February 25, 2021 14:07
@matthiaskrgr
Copy link
Member

Should this also lint for into_iter().count() ?

@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Feb 25, 2021

@matthewjasper
Oh, I forgot to do this. I'm going to fix the lint.

@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Feb 25, 2021

@flip1995 @matthewjasper @Y-Nak
I'm ready for your review!

Copy link
Member

@matthiaskrgr matthiaskrgr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems there is a false positive with code inside the cargo repo.
Reduced:

needs a cargo.toml with

[dependencies]
im-rc = "*"

main.rs:

fn main() {
    let mut g: Graph<i32, i32> = Graph::new();
    g.iter().count();
}

impl<N: Eq + Ord + Clone, E: Default + Clone> Graph<N, E> {
    pub fn new() -> Graph<N, E> {
        Graph {
            nodes: im_rc::OrdMap::new(),
        }
    }

    pub fn iter(&self) -> impl Iterator<Item = &N> {
        self.nodes.keys()
    }
}

pub struct Graph<N: Clone, E: Clone> {
    nodes: im_rc::OrdMap<N, im_rc::OrdMap<N, E>>,
}

Your lint warns

warning: called `.iter().count()` on a `std::iter::Iterator`
 --> src/main.rs:3:5
  |
3 |     g.iter().count();
  |     ^^^^^^^^^^^^^^^^ help: try: `g.len()`

but there is no corresponding .len() method

error[E0599]: no method named `len` found for struct `Graph<i32, i32>` in the current scope
  --> src/main.rs:3:7
   |
3  |     g.len();
   |       ^^^ method not found in `Graph<i32, i32>`
...
18 | pub struct Graph<N: Clone, E: Clone> {
   | ------------------------------------ method `len` not found for this
   |
   = help: items from traits can only be used if the trait is implemented and in scope
   = note: the following traits define an item `len`, perhaps you need to implement one of them:
           candidate #1: `std::iter::ExactSizeIterator`
           candidate #2: `typenum::type_operators::Len`
           candidate #3: `bitmaps::types::BitOps`

@TaKO8Ki TaKO8Ki requested a review from matthiaskrgr February 25, 2021 19:11
@TaKO8Ki
Copy link
Member Author

TaKO8Ki commented Feb 25, 2021

@matthewjasper
I've just solved this false positive.

clippy_utils/src/lib.rs Outdated Show resolved Hide resolved
clippy_lints/src/methods/iter_count.rs Outdated Show resolved Hide resolved
@TaKO8Ki TaKO8Ki requested a review from giraffate February 26, 2021 07:27
@bors
Copy link
Contributor

bors commented Feb 27, 2021

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

Copy link
Member

@matthiaskrgr matthiaskrgr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@matthiaskrgr
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 2, 2021

📌 Commit 6041365 has been approved by matthiaskrgr

@bors
Copy link
Contributor

bors commented Mar 2, 2021

⌛ Testing commit 6041365 with merge eb04beb...

@bors
Copy link
Contributor

bors commented Mar 2, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: matthiaskrgr
Pushing eb04beb to master...

@bors bors merged commit eb04beb into rust-lang:master Mar 2, 2021
@TaKO8Ki TaKO8Ki deleted the iter-count branch March 2, 2021 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use len() instead of iter().count()
7 participants