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

map_entry makes a suggestion that has different behaviour #5176

Closed
jbg opened this issue Feb 15, 2020 · 1 comment · Fixed by #6937
Closed

map_entry makes a suggestion that has different behaviour #5176

jbg opened this issue Feb 15, 2020 · 1 comment · Fixed by #6937
Labels
C-bug Category: Clippy is not doing the correct thing C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages E-medium Call for participation: Medium difficulty level problem and requires some initial experience. T-async-await Type: Issues related to async/await

Comments

@jbg
Copy link

jbg commented Feb 15, 2020

warning: usage of `contains_key` followed by `insert` on a `HashMap`
  --> src/main.rs:64:5
   |
64 | /     if !things.contains_key(&key) {
65 | |       things.insert(key, fetch_value().await?);
66 | |     }
   | |_____^ help: consider using: `things.entry(key).or_insert(fetch_value().await?);`
   |
   = note: `#[warn(clippy::map_entry)]` on by default
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_entry

The suggestion always calls fetch_value(), whereas the code as written only calls it if things.contains_key(&key) returns false.

Normally, it would be better if the suggestion was .or_insert_with(), but in this case a future is being awaited which won't work inside the closure, so I'm not sure there is any good suggestion that can be made.

@flip1995 flip1995 added E-medium Call for participation: Medium difficulty level problem and requires some initial experience. C-bug Category: Clippy is not doing the correct thing C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages T-async-await Type: Issues related to async/await labels Feb 15, 2020
@llogiq
Copy link
Contributor

llogiq commented Feb 15, 2020

A working suggestion would match on the entry. No closure needed, double lookup averted.

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 C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages E-medium Call for participation: Medium difficulty level problem and requires some initial experience. T-async-await Type: Issues related to async/await
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants