Skip to content

False positive when not *yet* using map value? #6368

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
iago-lito opened this issue Nov 22, 2020 · 4 comments
Closed

False positive when not *yet* using map value? #6368

iago-lito opened this issue Nov 22, 2020 · 4 comments

Comments

@iago-lito
Copy link

The following code

use std::collections::HashMap;

fn main() {
    let mut map = HashMap::new();
    map.insert(5, 8);
    for (key, _value) in &map {
        println!("Temp code not using {} but not _value yet.", key);
    }
}

yields

warning: you seem to want to iterate on a map's keys
 --> src/main.rs:6:26
  |
6 |     for (key, _value) in &map {
  |                          ^^^^
  |
  = note: `#[warn(clippy::for_kv_map)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#for_kv_map
help: use the corresponding method
  |
6 |     for key in map.keys() {
  |         ^^^    ^^^^^^^^^^

warning: 1 warning emitted

    Finished dev [unoptimized + debuginfo] target(s) in 0.01s

But I disagree. I would agree if I had written (key, _) instead, but with (key, _value), I explicitely opted in requesting values but not using them. Is this enough to consider this a false positive?

@camsteffen
Copy link
Contributor

I think it is still more idiomatic to use keys() so this is not a false positive. And I think _ and _name should be regarded as the same thing by Clippy in general.

@iago-lito
Copy link
Author

@camsteffen Well, I'm curious then, what's the supposed use case for the _name syntax in Rust? If both _ and _name are accepted, then they must be intended to address different uses, right?

I personally use _ when I don't need the value when destructuring. On the other hand, I use _name when I do need the value but I haven't used it yet because I am still in the process of developping my program.

As a consequence, this Clippy warning gets in my way. I either need to:

  • change (key, _name) to key and &map to map.keys(), but then when I eventually flesh my code and use the value, I'll need to revert by changing key to (key, name) then map.keys() to &map again.
  • live along with the warning until this time comes, but then the warning clutters my compilation output meanwhile.
  • explicitely disable this clippy warning, and then not forget to enable it again when I'm done.

Alternately, if this was considered false positive, I would just leave my code as is, have no warning. And change _value to value when I eventually use it.

Do you think it is a legitimate workflow, or am I making a wrong use of the _name syntax?

@camsteffen
Copy link
Contributor

As I see it, the general usefulness of the _name syntax is to enhance readability. So it is also useful in completed code. But your way of using it perfectly valid. If your code is not fleshed out, then of course you will have warnings and you should not let them bother you. On the other hand, if you are working on code that is functional, I would not leave an unused binding around just for the possibility of a future enhancement.

@iago-lito
Copy link
Author

Okay I think I get it. Clippy warnings are intended to refer to completed code. If my code is incomplete, then obviously I have warnings. If my code is complete, then clippy is right that I should use .keys() instead.

To conclude: not a false positive. Thank you for your patience @camsteffen :)

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

2 participants