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

Don't trigger while_let_on_iterator when the iterator is recreated every iteration #5525

Merged
merged 2 commits into from
Apr 25, 2020

Conversation

flip1995
Copy link
Member

r? @phansch

Fixes #1654

changelog: Fix false positive in [while_let_on_iterator]

@flip1995 flip1995 self-assigned this Apr 25, 2020
@flip1995 flip1995 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 25, 2020
@phansch phansch self-requested a review April 25, 2020 20:06
if_chain! {
if let ExprKind::MethodCall(..) | ExprKind::Call(..) = iter_expr.kind;
if let Some(iter_def_id) = get_trait_def_id(cx, &paths::ITERATOR);
if implements_trait(cx, cx.tables.expr_ty(iter_expr), iter_def_id, &[]);
Copy link
Member

@phansch phansch Apr 25, 2020

Choose a reason for hiding this comment

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

Just to make sure I understand correctly: this is checking that the scrutinee is not a call to an iter method, right?

Copy link
Member Author

@flip1995 flip1995 Apr 25, 2020

Choose a reason for hiding this comment

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

It is checking, that a (method) call before the next call does not return a type, that implements Iterator. Should I add a comment?

This does not cover this:

#[derive(Debug, Clone)]
struct S<'a> {
    chars: std::str::CharIndices<'a>,
}

while let Some(_) = s.clone().chars.next() {
    break;
}

But then again, if the struct is clonable, every field is clonable (in general), and this should be changed to s.chars.clone().next(), which prevents the lint from triggering.

@phansch
Copy link
Member

phansch commented Apr 25, 2020

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 25, 2020

📌 Commit a182622 has been approved by phansch

@bors
Copy link
Collaborator

bors commented Apr 25, 2020

🌲 The tree is currently closed for pull requests below priority 1, this pull request will be tested once the tree is reopened

@phansch
Copy link
Member

phansch commented Apr 25, 2020

@bors treeclosed-

@bors
Copy link
Collaborator

bors commented Apr 25, 2020

⌛ Testing commit a182622 with merge be86cec...

bors added a commit that referenced this pull request Apr 25, 2020
 Don't trigger while_let_on_iterator when the iterator is recreated every iteration

r? @phansch

Fixes #1654

changelog: Fix false positive in [`while_let_on_iterator`]
@bors
Copy link
Collaborator

bors commented Apr 25, 2020

💔 Test failed - checks-action_test

@flip1995
Copy link
Member Author

@bors retry

@bors
Copy link
Collaborator

bors commented Apr 25, 2020

⌛ Testing commit a182622 with merge 6dc4e5c...

bors added a commit that referenced this pull request Apr 25, 2020
 Don't trigger while_let_on_iterator when the iterator is recreated every iteration

r? @phansch

Fixes #1654

changelog: Fix false positive in [`while_let_on_iterator`]
@bors
Copy link
Collaborator

bors commented Apr 25, 2020

💔 Test failed - checks-action_test

@flip1995
Copy link
Member Author

@bors retry

@bors
Copy link
Collaborator

bors commented Apr 25, 2020

⌛ Testing commit a182622 with merge 44eb953...

@bors
Copy link
Collaborator

bors commented Apr 25, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: phansch
Pushing 44eb953 to master...

@bors bors merged commit 44eb953 into rust-lang:master Apr 25, 2020
@flip1995 flip1995 deleted the issue_1654 branch May 4, 2020 15:28
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.

while_let_on_iterator false positive
3 participants