Conversation
|
r? @xFrednet (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
the
S-waiting-on-review
|
(xFrednet has picked a reviewer for you, use r? to override) |
blyxyas
left a comment
blyxyas
left a comment
There was a problem hiding this comment.
Very interesting concept for a lint. It has some things I'd like to talk about, but very good overall ❤️
| if !snippet | ||
| .split("->") | ||
| .skip(0) | ||
| .skip(1) |
There was a problem hiding this comment.
Are we sure this change doesn't break anything? Won't this cause wrong suggestions? Probably the safest bet is to create a FIXME pointing to this commit and a brief explaination.
Just tested it, indeed .skip(0) to .skip(1) produce different results. Playground link
There was a problem hiding this comment.
Yep, .skip(0) does nothing. It's meant to skip anything before -> as that's known to not be the return type (and results in FPs if it contains ::std::primitive::u32 or the like). I wrote that code before so :D
| /// ```rust | ||
| /// let v = vec![1, 2, 3]; | ||
| /// let x = v.iter().skip(0).collect::<Vec<_>>(); | ||
| /// let y = v.iter().collect::<Vec<_>>(); |
There was a problem hiding this comment.
Could you re-write this example section so it follows by the general docs. guidelines? (using "Could be written as" and such)
There was a problem hiding this comment.
There's a good few others like that (only example, mostly self-explanatory lints)
|
☔ The latest upstream changes (presumably #11012) made this pull request unmergeable. Please resolve the merge conflicts. |
Centri3
force-pushed
the
iter_skip_zero
branch
2 times, most recently
from
877daff to
2604a06
Compare
|
☔ The latest upstream changes (presumably #10970) made this pull request unmergeable. Please resolve the merge conflicts. |
Centri3
force-pushed
the
iter_skip_zero
branch
2 times, most recently
from
9ce8868 to
a0b76d4
Compare
|
☔ The latest upstream changes (presumably #11052) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@bors r+ |
|
@blyxyas: 🔑 Insufficient privileges: Not in reviewers |
|
Life is hard |
|
@bors r=xFrednet |
|
@blyxyas: 🔑 Insufficient privileges: Not in reviewers |
|
Maybe the second delegation overwrote the first one? @Centri3 could you try? |
|
Maybe, @bors r=blyxyas |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
|
WTF? So picky that boy Hey @bors be nicer to our newest members. Honer their new power!!! ✨ |
|
Just for the record, this lint breaks Alacritty. It has code that skips some non-zero amount of characters in one branch but doesn't skip anything in another. The result is stored in the same structure field of the type
|
|
This lint can be allowed by using an attribute like I'm not seeing how this is an issue, or how a lint can break a project other than generating some new error messages in the CI. |
|
Probably the fix that breaks it? We should call |
|
Thank you for the responses! I didn't mean to say that the issue cannot be ignored in the code. My point is that there are legitimate uses of |
6 participants
Idea came from my contribution to
unnecessary_castrecently. Sorry about that 😅Could be an issue if somebody manually implements
skip, but I don't think anybody will (and this would be an issue with other lints too, afaik)changelog: New lint [
iter_skip_zero]