Skip to content

range_plus_one lint wrongly suggests using RangeInclusive when Range is required and the other way around #3307

@asomers

Description

@asomers

The range_plus_one lint suggests replacing Range syntax with RangeInclusive syntax. That's fine if you're immediately going to loop. But it's invalid if the Range in question is going to be assigned to a variable of type Range. For example, clippy will suggest replacing this code:

struct Foo {
    r: Range<u32>
}

fn bar() {
    let x = 0;
    let foo = Foo{r: x..x+1};
}

with this:

struct Foo {
    r: Range<u32>
}

fn bar() {
    let x = 0;
    let foo = Foo{r: x..=x};
}

But that fails to compile

2263 |     let foo = Foo{r: x..=x};
     |                      ^^^^^ expected struct `std::ops::Range`, found struct `std::ops::RangeInclusive`                                                                
     |
     = note: expected type `std::ops::Range<u32>`
                found type `std::ops::RangeInclusive<{integer}>`
clippy 0.0.212 (32b1d1fc 2018-10-05)

Activity

jonasbb

jonasbb commented on Dec 6, 2018

@jonasbb

Another example using rayon:

The following program compiles, but clippy produces a warning on it:

extern crate rayon;
use rayon::prelude::*;
fn main() {
    (0..1 + 1).into_par_iter();
}

The warning is:

warning: an inclusive range would be more readable
 --> src/main.rs:4:5
  |
4 |     (0..1 + 1).into_par_iter();
  |     ^^^^^^^^^^ help: use: `(0..=1)`
  |
  = note: #[warn(clippy::range_plus_one)] on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#range_plus_one

However, applying the suggestion results in the following error:

error[E0599]: no method named `into_par_iter` found for type `std::ops::RangeInclusive<{integer}>` in the current scope
 --> src/main.rs:4:13
  |
4 |     (0..=1).into_par_iter();
  |             ^^^^^^^^^^^^^
  |
  = note: the method `into_par_iter` exists but the following trait bounds were not satisfied:
          `std::ops::RangeInclusive<{integer}> : rayon::iter::IntoParallelIterator`
          `&std::ops::RangeInclusive<{integer}> : rayon::iter::IntoParallelIterator`
          `&mut std::ops::RangeInclusive<{integer}> : rayon::iter::IntoParallelIterator`

See also the rayon documentation for ranges.

added
C-bugCategory: Clippy is not doing the correct thing
L-suggestionLint: Improving, adding or fixing lint suggestions
on Dec 12, 2018
oxalica

oxalica commented on May 26, 2019

@oxalica
Contributor

+1 for this issue.

I also met it when calling a function requiring Range. Example:

fn foo(_: std::ops::Range<i32>) {}

fn main() {
    let x = 1;
    foo(x..(x + 1));
}

I don't think implementing a generic function for all ranges would be always better.

flip1995

flip1995 commented on Dec 22, 2019

@flip1995
Member

Citing #4898 to keep traack of this in a single issue:

warning: an exclusive range would be more readable
  --> src/main.rs:55:21
   |
55 |         init_range: 0..=(sidx_offset - 1),
   |                     ^^^^^^^^^^^^^^^^^^^^^ help: use: `0..sidx_offset`
   |
   = note: `#[warn(clippy::range_minus_one)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#range_minus_one

However, init_range in the struct in question, is defined as an InclusiveRange. (By design, matching HTTP-Range).

So the same problem exists the other way around.

changed the title [-]range_plus_one lint wrongly suggests using RangeInclusive when Range is required[/-] [+]range_plus_one lint wrongly suggests using RangeInclusive when Range is required and the other way around[/+] on Dec 22, 2019
kvark

kvark commented on Dec 31, 2019

@kvark

We hit this in gfx-rs since the code expects Range specifically and fails upon turning the expression into RangeInclusive.

jyn514

jyn514 commented on Jan 13, 2020

@jyn514
Member

This also happens when returning a value from a function.

pub struct Span {
    offset: usize,
    length: usize,
}

impl From<Span> for RangeInclusive<usize> {
    fn from(span: Span) -> Self {
        // clippy wants me to put span.offset..(span.offset + span.length) here
        span.offset..=(span.offset + span.length - 1)
    }
}
krishna-veerareddy

krishna-veerareddy commented on Jan 24, 2020

@krishna-veerareddy
Contributor

@flip1995 Hey I would like to tackle this one but I am not sure how we would know if the type of an expression can be changed or not. Any suggestions?

22 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-bugCategory: Clippy is not doing the correct thingI-suggestion-causes-errorIssue: The suggestions provided by this Lint cause an ICE/error when appliedL-suggestionLint: Improving, adding or fixing lint suggestions

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      Participants

      @kvark@asomers@jonasbb@flip1995@ThibsG

      Issue actions

        range_plus_one lint wrongly suggests using RangeInclusive when Range is required and the other way around · Issue #3307 · rust-lang/rust-clippy