diff options
| author | Catherine Flores <catherine.3.flores@gmail.com> | 2025-02-19 09:07:34 +0000 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2025-02-19 09:07:34 +0000 |
| commit | d8ecde0e4308c21708e9b3b2a1d07648d68129c2 (patch) | |
| tree | 7a3835db6bcdf8cde10caeb14610739e3d12db0f | |
| parent | 975a813c5a2137345f009f6b6c2edf16d34573c7 (diff) | |
| parent | e2cdfed0ea09e430f1b24351856b224eda3683d9 (diff) | |
| download | rust-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.rs | 17 | ||||
| -rw-r--r-- | tests/ui/entry.fixed | 23 | ||||
| -rw-r--r-- | tests/ui/entry.rs | 23 |
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() {} |
