Skip to content

single_range_in_vec_init stronger than expected, when type is known #11086

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

Open
matzemathics opened this issue Jul 3, 2023 · 10 comments
Open
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have

Comments

@matzemathics
Copy link

Summary

The single_range_in_vec_init warning seems a bit overly cautious, if the type resulting from an invocation of a vec! macro is already known "from the outside", e.g. via a type annotation or when it is used as an argument / return value.

For example

let foo: Vec<Range<usize>> = vec![1..5]; // causes "warning: a `Vec` of `Range` that is only one element"

In this case clippy might infer, that the user indeed wanted a Vec<Range<...>>, because otherwise the program could not even have type checked. The same is true for this example:

fn barify(v: Vec<Range<usize>>) {
  ...
}

let v = vec![3..42]; // causes "warning: a `Vec` of `Range` that is only one element"
barify(v);

One explicit way to remedy this is using explicit lengths:

let foo = vec![1..5; 1]; // OK

although I am slightly worried that tools like cargo-fmt might at some point opt to remove that.

Lint Name

single_range_in_vec_init

Reproducer

I tried this code:

use std::ops::Range;

fn main() {
    let _foo: Vec<Range<usize>> = vec![1..5];
}

I saw this happen:

warning: a `Vec` of `Range` that is only one element
 --> src/main.rs:4:35
  |
4 |     let _foo: Vec<Range<usize>> = vec![1..5];
  |                                   ^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_range_in_vec_init
  = note: `#[warn(clippy::single_range_in_vec_init)]` on by default
help: if you wanted a `Vec` that contains the entire range, try
  |
4 |     let _foo: Vec<Range<usize>> = (1..5).collect::<std::vec::Vec<usize>>();
  |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
help: if you wanted a `Vec` of len 5, try
  |
4 |     let _foo: Vec<Range<usize>> = vec![1; 5];
  |                                        ~~~~

warning: `single_range` (bin "single_range") generated 1 warning

I expected to see this happen:
No warnings produced.

Version

No response

Additional Labels

No response

@matzemathics matzemathics added C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have labels Jul 3, 2023
@Centri3
Copy link
Member

Centri3 commented Jul 3, 2023

Ignoring it for arguments when it's a local would likely be pretty tricky, though type annotations should definitely be checked

@ArthurCose
Copy link

Something not currently covered by the PR:

use std::ops::Range;

struct A {
    ranges: Vec<Range<usize>>,
}

struct B(Vec<Range<usize>>);

fn main() {
    A { ranges: vec![0..5] };
    B(vec![0..5]);
}

The correct type of a struct's field is also known "from the outside".

@Centri3
Copy link
Member

Centri3 commented Jul 16, 2023

Should be simple enough to fix

@Centri3
Copy link
Member

Centri3 commented Jul 16, 2023

It ignores ExprFields now. This also means something like Vec<&dyn Any> will be skipped as well but I think due to the rarity of both that and passing a Range<usize> to it, it should be fine. (It's also debatable whether that should even be linted.)

@djc
Copy link
Contributor

djc commented Aug 25, 2023

I'm also running into this in Quinn, in the tests for the RangeSet::replace() method. This definitely seems like a false positive.

@avantgardnerio
Copy link

This is coming up with a lot of false positives for our project as well. I'm concerned that it is valid to have/want a Vec with a single Range, and that the suggestions to correct it would alter functionality. If anyone goes and applies them blindly, they might cause subtle bugs.

@Centri3
Copy link
Member

Centri3 commented Aug 29, 2023

The help message should probably be altered to make sure this doesn't happen, since right now it acts as though this is almost certainly what they want

@synek317
Copy link

It also causes quite annoying issues in my project. I completely don't understand the intention behind this lint and why is it enabled by default.

@bugadani
Copy link
Contributor

bugadani commented Oct 2, 2024

This lint doesn't tell me what to do if I actually WANT an array/vec of ranges, which makes its strength even more bothersome.

@smalis-msft
Copy link

Agreed with everyone here, in our case it's only triggering on locals that are immediately passed to function calls that wouldn't typecheck otherwise. Should this lint be downgraded to a category that isn't warn-by-default?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have
Projects
None yet
8 participants