Fix 8128

[surechen]

Fixes #8128

changelog: Fix error suggestion of skip(..).next() for immutable variable.

[rust-highfive]

r? @flip1995

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

[surechen]

I want to add a counter example. It runs correctly in playground. link
But the CI failed

[flip1995]

The tests are failing, because rustfix can only apply suggestions produces by span_lint_and_sugg/span_suggestion. So you probably have to create a separate test file, that doesn't have the // run rustfix comment at the top.

[surechen]

Hi, could you help me see this? Is it consistent with your modification suggestion? Thanks.
r? @xFrednet

[xFrednet]

Hi, could you help me see this? Is it consistent with your modification suggestion?

Sure, this is what I had in mind. I again looked into a way to check this differently, but I don't see a viable solution to get the information from the type of the self argument of skip. And I think this is fine, since @flip1995 didn't mention another solution in his first review.

I have two suggestions how the output could be a bit more readable, but otherwise this looks good to me. Thank you for the work 🙃

[surechen]

Hi, could you help me see this? Is it consistent with your modification suggestion?

Sure, this is what I had in mind. I again looked into a way to check this differently, but I don't see a viable solution to get the information from the type of the self argument of skip. And I think this is fine, since @flip1995 didn't mention another solution in his first review.

I have two suggestions how the output could be a bit more readable, but otherwise this looks good to me. Thank you for the work 🙃

Done. Thanks.

[xFrednet]

Looks good to me! 👍

Could you please squash your commits before the merge? If you want to keep the messages, you can squash the fix #8128 commits into one and all review commits into another one.

A related note: In Rust we usually avoid mentioning the issue in commit messages as that creates a new link in the issue with every rebase. I do see the advantage of mentioning it and use that feature in other projects as well. I think the idea is more to use a descriptive commit message that can be understood without opening an issue. We can also merge commits with such messages, and I'm happy to do so if you want to keep the messages. Just something I wanted to mention, this also depends on your personal style 🙃

[surechen]

Looks good to me! 👍

Could you please squash your commits before the merge? If you want to keep the messages, you can squash the fix #8128 commits into one and all review commits into another one.

A related note: In Rust we usually avoid mentioning the issue in commit messages as that creates a new link in the issue with every rebase. I do see the advantage of mentioning it and use that feature in other projects as well. I think the idea is more to use a descriptive commit message that can be understood without opening an issue. We can also merge commits with such messages, and I'm happy to do so if you want to keep the messages. Just something I wanted to mention, this also depends on your personal style 🙃

Done. Thank you very much.

[xFrednet]

LGTM, thank you for the update and quick replies! 👍

@bors r+

[bors]

📌 Commit 4ffd660 has been approved by xFrednet

[bors]

⌛ Testing commit 4ffd660 with merge bb7b6be...

[bors]

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