Skip to content

clippy:: map_entry incorrectly warned on code with a contains_key and no insert #14449

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
erickt opened this issue Mar 21, 2025 · 4 comments · Fixed by #14568
Closed

clippy:: map_entry incorrectly warned on code with a contains_key and no insert #14449

erickt opened this issue Mar 21, 2025 · 4 comments · Fixed by #14568
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

@erickt
Copy link
Contributor

erickt commented Mar 21, 2025

Summary

Fuchsia just did a dry run roll of Rust rust-lang/rust@5d85a71, and it's incorrectly flagging the clippy::map_entry lint in at least two bits of our code that use contains_key, but do not have a corresponding insert:

error: usage of `contains_key` followed by `insert` on a `HashMap`
   --> ../../src/connectivity/network/netstack3/src/bindings/filter/conversion.rs:392:17
    |
392 | /                 if self.routine_types.contains_key(&name) {
393 | |                     self.expect_routine_resolved(&name, routine_type, rule_id)
394 | |                 } else {
395 | |                     self.convert_routine(
...   |
401 | |                 }
    | |_________________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
    = note: `-D clippy::map-entry` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::map_entry)]`
error: usage of `contains_key` followed by `insert` on a `HashMap`
   --> ../../src/devices/lib/driver-static-checks/src/checks.rs:150:9
    |
150 | /         if json_category.contains_key(&device_category.subcategory) {
151 | |             Ok(StaticCheckPass {})
152 | |         } else {
153 | |             Err(StaticCheckFail  {
...   |
158 | |             })
159 | |         }
    | |_________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
    = note: `-D clippy::map-entry` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::map_entry)]`

Lint Name

clippy:: map_entry

Reproducer

So far I haven't been able to reproduce it outside our code, I'll update if I figure out one.

Version

This was built in our CI/CQ, so it's a little hard to get `rustc -Vv`, but it's with the nightly rustc version https://github.com/rust-lang/rust/commit/5d85a714b10a9121d2ec5115f39e904174f804e1.

Additional Labels

No response

@erickt erickt 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 Mar 21, 2025
bherrera pushed a commit to misttech/fuchsia that referenced this issue Apr 1, 2025
We're rolling out `clippy::map_entry` across the tree and adding
`#[allow(clippy::map_entry)]` attributes on all current code that
triggers the lint, due to a known bug in the latest Rust compiler (see
[here](rust-lang/rust-clippy#14449) for more
info).

Bug: 407087100
Bug: 407086469
Bug: 407087136

Change-Id: I4962a540e80b1916b2e4e8f4c67bd928165988cb
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1239684
Reviewed-by: Peter Johnston <peterjohnston@google.com>
Reviewed-by: Sarah Chan <spqchan@google.com>
Commit-Queue: Steven Grady <slgrady@google.com>
bherrera pushed a commit to misttech/integration that referenced this issue Apr 3, 2025
We're rolling out `clippy::map_entry` across the tree and adding
`#[allow(clippy::map_entry)]` attributes on all current code that
triggers the lint, due to a known bug in the latest Rust compiler (see
[here](rust-lang/rust-clippy#14449) for more
info).

Original-Bug: 407087100
Original-Bug: 407086469
Original-Bug: 407087136

Original-Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1239684
Original-Revision: 2652deb44c3069064381427157357dd37419d03e
GitOrigin-RevId: 31f9512e215661d3989b6de9d66a7f22252e9753
Change-Id: I081ddf17aab4a046c8fc8cd4e24c80566f82034a
@elenaf9
Copy link

elenaf9 commented Apr 5, 2025

We are running into the same issue:

  • libp2p/rust-libp2p/protocols/gossipsub/src/peer_score.rs:
    (note that it's params.topics and not self.topics in the first statement)

    error: usage of `contains_key` followed by `insert` on a `HashMap`
       --> protocols/gossipsub/src/peer_score.rs:113:9
        |
    113 | /         if params.topics.contains_key(&topic_hash) {
    114 | |             Some(self.topics.entry(topic_hash).or_default())
    115 | |         } else {
    116 | |             self.topics.get_mut(&topic_hash)
    117 | |         }
        | |_________^
        |
  • libp2p/rust-libp2p/protocols/gossipsub/src/behaviour.rs:

    error: usage of `contains_key` followed by `insert` on a `HashMap`
        --> protocols/gossipsub/src/behaviour.rs:1816:9
         |
    1816 | /         if self.mesh.contains_key(&message.topic) {
    1817 | |             tracing::debug!("Sending received message to user");
    1818 | |             self.events
    1819 | |                 .push_back(ToSwarm::GenerateEvent(Event::Message {
    ...    |
    1829 | |             return;
    1830 | |         }
         | |_________^

Version

clippy 0.1.87 (45165c82a4 2025-04-01) (current beta)

@meithecatte
Copy link
Contributor

Minimal example (playground):

use std::collections::BTreeMap;
pub struct Meow {
    map: BTreeMap<String, String>,
}

impl Meow {
    fn pet(&self, _key: &str, _v: u32) -> u32 {
        42
    }
}

pub fn f(meow: &Meow, x: String) {
    if meow.map.contains_key(&x) {
        let _ = meow.pet(&x, 1);
    } else {
        let _ = meow.pet(&x, 0);
    }
}

@meithecatte
Copy link
Contributor

searched nightlies: from nightly-2025-03-17 to nightly-2025-04-08
regressed nightly: nightly-2025-03-22
searched commit range: rust-lang/rust@78948ac...be73c1f
regressed commit: rust-lang/rust@5d85a71

bisected with cargo-bisect-rustc v0.6.9

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --component clippy -- clippy

@meithecatte
Copy link
Contributor

git bisect identifies 18616dc as the first bad commit. cc @profetia

meithecatte added a commit to meithecatte/rust-clippy that referenced this issue Apr 9, 2025
github-merge-queue bot pushed a commit that referenced this issue Apr 9, 2025
…4568)

Fixes #14449, introduced in #14314

changelog: [`map_entry`]: fix a false positive where the lint would
trigger without any insert calls present
primeos-work added a commit to primeos-work/butido that referenced this issue Apr 16, 2025
This is a regression in the lint [0] and it should be fixed again in the
next version. See [1] for more details.

[0]: https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
[1]: rust-lang/rust-clippy#14449

Signed-off-by: Michael Weiss <michael.weiss@eviden.com>
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
Development

Successfully merging a pull request may close this issue.

3 participants