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

Do not lint range_plus_one inside implementations of Index<RangeInclusive<_>> #3583

Open
Darksonn opened this issue Dec 26, 2018 · 2 comments

Comments

@Darksonn
Copy link

When implementing indexing on RangeInclusive, we obviously cannot replace start .. end+1 with start ..= end, as that would be an infinite loop.

My situation is something like the following:

struct MyVec {
    inner: Vec<i64>,
}

impl Index<Range<usize>> for MyVec {
    type Output = [i64];
    fn index(&self, i: Range<usize>) -> &[i64] {
        &self.inner[i.start .. i.end]
    }
}

impl Index<RangeInclusive<usize>> for MyVec {
    type Output = [i64];
    fn index(&self, i: RangeInclusive<usize>) -> &[i64] {
        self.index(*i.start()..(*i.end() + 1))
    }
}

Here the impl of RangeInclusive delegates to the impl of Range by turning the inclusive range into an exclusive range.

Of course the same goes for IndexMut.

The warning is:

warning: an inclusive range would be more readable                                                                                                                                             
   --> src/lib.rs:15:20                                                                                                                                                                       
    |                                                                                                                                                                                          
 15 |         self.index(*i.start()..(*i.end() + 1))                                                                                                                                           
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use: `*i.start()..=*i.end()`                                                                                                         
    |                                                                                                                                                                                          
    = note: #[warn(clippy::range_plus_one)] on by default                                                                                                                                      
    = help: for further information visit https://rust-lang-nursery.github.io/rust-clippy/master/index.html#range_plus_one 

Version:

$ cargo clippy -V
clippy 0.0.212 (2e26fdc 2018-11-22)
@detrumi
Copy link
Member

detrumi commented Dec 30, 2018

Just checking whether ..= is used inside a .. impl isn't enough to cover all cases (though it might be good enough), since it could call other functions that use ..= instead. Would be hard to check against that, though.

@KamilaBorowska
Copy link
Contributor

KamilaBorowska commented Dec 30, 2018

Maybe it would make sense to change the lint to detect such ranges being directly passing a function/method expecting an implementation of Iterator/IntoIterator/RangeBounds trait. If a function accepts one of those traits, then barring bugs, ranges should be identical to inclusive ranges. Such an implementation would also fix #3307.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants