From e2cdfed0ea09e430f1b24351856b224eda3683d9 Mon Sep 17 00:00:00 2001 From: yanglsh Date: Wed, 5 Feb 2025 02:32:18 -0700 Subject: [PATCH] fix: `map_entry` FP on struct member --- clippy_lints/src/entry.rs | 17 ++++++++++++++++- tests/ui/entry.fixed | 23 +++++++++++++++++++++++ tests/ui/entry.rs | 23 +++++++++++++++++++++++ 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/entry.rs b/clippy_lints/src/entry.rs index 99d635aa5cc0..f8a5cf53cda0 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>, allow_insert_closure: bool, diff --git a/tests/ui/entry.fixed b/tests/ui/entry.fixed index 9856fa9b39fd..d52299306fd0 100644 --- a/tests/ui/entry.fixed +++ b/tests/ui/entry.fixed @@ -195,4 +195,27 @@ fn issue12489(map: &mut HashMap) -> Option<()> { Some(()) } +mod issue13934 { + use std::collections::HashMap; + + struct Member {} + + pub struct Foo { + members: HashMap, + } + + 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 bb8ebb4eac08..25cd6eaa4132 100644 --- a/tests/ui/entry.rs +++ b/tests/ui/entry.rs @@ -201,4 +201,27 @@ fn issue12489(map: &mut HashMap) -> Option<()> { Some(()) } +mod issue13934 { + use std::collections::HashMap; + + struct Member {} + + pub struct Foo { + members: HashMap, + } + + 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() {}