Skip to content

New lint: slice.iter().next() can be replaced with slice.get() #5572

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

Closed
lights0123 opened this issue May 6, 2020 · 7 comments
Closed

New lint: slice.iter().next() can be replaced with slice.get() #5572

lights0123 opened this issue May 6, 2020 · 7 comments
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy

Comments

@lights0123
Copy link

As I saw in this code review, someone wrote:

while Some(&instructions[i]) == instructions[i + acc..].iter().next() {
    acc += 1;
}

That should be replaced with:

while Some(&instructions[i]) == instructions.get(i + acc) {
    acc += 1;
}

In general, slice[i..].iter().next() can be replaced with slice.get(i), or 0 if there is no slicing done.

@flip1995 flip1995 added good-first-issue These issues are a good way to get started with Clippy A-lint Area: New lints labels May 6, 2020
@esamudera
Copy link
Contributor

Hello, I'm still new to open source development and I'm interested in working on this issue.

What I understand from reading the description:

  • search all the iter().next()s
  • determine if it's a slice between an index and the end index
  • simplify the code using get(start_index)
  • if it's not a slice, then just replace it with array.get(0)

Is this correct? If yes, I can start working on this.

Thank you!

@lights0123
Copy link
Author

lights0123 commented May 8, 2020

@esamudera Yep. You might want to handle things that Deref to a slice and implement Iterator, like Vec. However, you may want to hardcode just Vec specifically because it's possible to have a type that converts to a slice but doesn't have the same implementation for the iterator. You can also do Option.

For .into_iter(), we can probably just rely on rustc's array_into_iter/clippy::into_iter_on_ref lint.

Here's a sample test case:

fn main() {
    let s = [1, 2, 3];
    let v = vec![1, 2, 3];
    let o = Some(5);
    
    s.iter().next();
    // Should be replaced by s.first()
    
    s.into_iter().next();
    // Probaby can rely on the default `array_into_iter`/`clippy::into_iter_on_ref` lint
    
    s[1..].iter().next();
    // Should be replaced by s.get(1)
    
    s[1..].into_iter().next();
    // Probaby can rely on the default `clippy::into_iter_on_ref` lint
    
    v[1..].iter().next();
    // Should be replaced by v.get(1)
    
    v[1..].into_iter().next();
    // Probaby can rely on the default `clippy::into_iter_on_ref` lint
    
    v.iter().next();
    // Should be replaced by v.first()
    
    v.into_iter().next();
    // Tricky one—I don't think there's any other (short) way to say this (take ownership of first value)
    // So this should probably not get any warnings
    
    o.iter().next();
    // Replace with o.as_ref()
    
    o.into_iter().next();
    // Replace with o
}

@esamudera
Copy link
Contributor

Okay, thank you for the additional contexts, I will start by making a new clippy_lint struct and see where it goes from there :)

@alex-700
Copy link
Contributor

alex-700 commented May 11, 2020

Hi, guys! I think this idea should be split into two lints.

  • slice.iter().next() should be replaced with slice.first(). It can be done easily with adding a case to this match .

  • slice[a..b].first() should be replaced with slice.get(a)

Also the second one can be improved with handling such cases like slice[a..b].get(x) and slice[a..b].last()

@esamudera
Copy link
Contributor

Thanks @alex-700, I didn't know that https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/methods/mod.rs#L1352 exists, and made a new lint struct checking method call expressions and its parents from scratch. I'll use that module instead.

For now I'm trying to implement lint for vector slice with [index..]

I'm still very new to this codebase, is this issue an urgent one? Because I need some time to study the code and learn what rustc_hir library can do, but it's totally fine if I have to pick other issues 👍

bors added a commit that referenced this issue May 31, 2020
New lint: iter_next_slice

Hello, this is a work-in-progress PR for issue: #5572

I have implemented lint to replace `iter().next()` for `slice[index..]` and `array` with `get(index)` and `get(0)` respectively. However since I made a lot of changes, I would like to request some feedback before continuing so that I could fix mistakes.

Thank you!

---

changelog:
- implement iter next slice lint and test
- modify needless_continues, for_loop_over_options_result UI tests since they have `iter().next()`
bors added a commit that referenced this issue Jun 2, 2020
New lint: iter_next_slice

Hello, this is a work-in-progress PR for issue: #5572

I have implemented lint to replace `iter().next()` for `slice[index..]` and `array` with `get(index)` and `get(0)` respectively. However since I made a lot of changes, I would like to request some feedback before continuing so that I could fix mistakes.

Thank you!

---

changelog: implement `iter_next_slice` lint and test, and modify `needless_continues`, `for_loop_over_options_result` UI tests since they have `iter().next()`
@giraffate
Copy link
Contributor

Hi, is this issue already resolved? If so, it would be better to close this.

@JohnTitor
Copy link
Member

Closing in favor of #5597.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints good-first-issue These issues are a good way to get started with Clippy
Projects
None yet
Development

No branches or pull requests

6 participants