about summary refs log tree commit diff
diff options
context:
space:
mode:
authorCatherine Flores <catherine.3.flores@gmail.com>2025-02-19 09:07:34 +0000
committerGitHub <noreply@github.com>2025-02-19 09:07:34 +0000
commitd8ecde0e4308c21708e9b3b2a1d07648d68129c2 (patch)
tree7a3835db6bcdf8cde10caeb14610739e3d12db0f
parent975a813c5a2137345f009f6b6c2edf16d34573c7 (diff)
parente2cdfed0ea09e430f1b24351856b224eda3683d9 (diff)
downloadrust-d8ecde0e4308c21708e9b3b2a1d07648d68129c2.tar.gz
rust-d8ecde0e4308c21708e9b3b2a1d07648d68129c2.zip
fix: `map_entry` FP on struct member (#14151)
fixes #13934

I modified the part for checking if the map is used so that it can check
field and index exprs.

changelog: [`map_entry`]: fix FP on struct member
-rw-r--r--clippy_lints/src/entry.rs17
-rw-r--r--tests/ui/entry.fixed23
-rw-r--r--tests/ui/entry.rs23
3 files changed, 62 insertions, 1 deletions
diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs
index 99d635aa5cc..f8a5cf53cda 100644
--- a/clippy_lints/src/entry.rs
+++ b/clippy_lints/src/entry.rs
@@ -1,5 +1,6 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::{reindent_multiline, snippet_indent, snippet_with_applicability, snippet_with_context};
+use clippy_utils::visitors::for_each_expr;
 use clippy_utils::{
     SpanlessEq, can_move_expr_to_closure_no_visit, higher, is_expr_final_block_expr, is_expr_used_or_unified,
     peel_hir_expr_while,
@@ -12,6 +13,7 @@ use rustc_hir::{Block, Expr, ExprKind, HirId, Pat, Stmt, StmtKind, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
 use rustc_span::{DUMMY_SP, Span, SyntaxContext, sym};
+use std::ops::ControlFlow;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -500,7 +502,7 @@ impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, 'tcx> {
                 self.visit_non_tail_expr(insert_expr.value);
                 self.is_single_insert = is_single_insert;
             },
-            _ if SpanlessEq::new(self.cx).eq_expr(self.map, expr) => {
+            _ if is_any_expr_in_map_used(self.cx, self.map, expr) => {
                 self.is_map_used = true;
             },
             _ => match expr.kind {
@@ -562,6 +564,19 @@ impl<'tcx> Visitor<'tcx> for InsertSearcher<'_, 'tcx> {
     }
 }
 
+/// Check if the given expression is used for each sub-expression in the given map.
+/// For example, in map `a.b.c.my_map`, The expression `a.b.c.my_map`, `a.b.c`, `a.b`, and `a` are
+/// all checked.
+fn is_any_expr_in_map_used<'tcx>(cx: &LateContext<'tcx>, map: &'tcx Expr<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
+    for_each_expr(cx, map, |e| {
+        if SpanlessEq::new(cx).eq_expr(e, expr) {
+            return ControlFlow::Break(());
+        }
+        ControlFlow::Continue(())
+    })
+    .is_some()
+}
+
 struct InsertSearchResults<'tcx> {
     edits: Vec<Edit<'tcx>>,
     allow_insert_closure: bool,
diff --git a/tests/ui/entry.fixed b/tests/ui/entry.fixed
index 9856fa9b39f..d52299306fd 100644
--- a/tests/ui/entry.fixed
+++ b/tests/ui/entry.fixed
@@ -195,4 +195,27 @@ fn issue12489(map: &mut HashMap<u64, u64>) -> Option<()> {
     Some(())
 }
 
+mod issue13934 {
+    use std::collections::HashMap;
+
+    struct Member {}
+
+    pub struct Foo {
+        members: HashMap<u8, Member>,
+    }
+
+    impl Foo {
+        pub fn should_also_not_cause_lint(&mut self, input: u8) {
+            if self.members.contains_key(&input) {
+                todo!();
+            } else {
+                self.other();
+                self.members.insert(input, Member {});
+            }
+        }
+
+        fn other(&self) {}
+    }
+}
+
 fn main() {}
diff --git a/tests/ui/entry.rs b/tests/ui/entry.rs
index bb8ebb4eac0..25cd6eaa413 100644
--- a/tests/ui/entry.rs
+++ b/tests/ui/entry.rs
@@ -201,4 +201,27 @@ fn issue12489(map: &mut HashMap<u64, u64>) -> Option<()> {
     Some(())
 }
 
+mod issue13934 {
+    use std::collections::HashMap;
+
+    struct Member {}
+
+    pub struct Foo {
+        members: HashMap<u8, Member>,
+    }
+
+    impl Foo {
+        pub fn should_also_not_cause_lint(&mut self, input: u8) {
+            if self.members.contains_key(&input) {
+                todo!();
+            } else {
+                self.other();
+                self.members.insert(input, Member {});
+            }
+        }
+
+        fn other(&self) {}
+    }
+}
+
 fn main() {}