diff --git a/clippy_lints/src/dereference.rs b/clippy_lints/src/dereference.rs index bb0b04b6366d..f1915f16339e 100644 --- a/clippy_lints/src/dereference.rs +++ b/clippy_lints/src/dereference.rs @@ -15,7 +15,7 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow, AutoBorrowMutability}; use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable, TypeckResults}; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::{symbol::sym, Span}; +use rustc_span::{symbol::sym, Span, Symbol}; declare_clippy_lint! { /// ### What it does @@ -181,6 +181,9 @@ enum State { deref_span: Span, deref_hir_id: HirId, }, + ExplicitDerefField { + name: Symbol, + }, Reborrow { deref_span: Span, deref_hir_id: HirId, @@ -243,8 +246,18 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { (None, kind) => { let parent = get_parent_node(cx.tcx, expr.hir_id); let expr_ty = typeck.expr_ty(expr); - match kind { + RefOp::Deref => { + if let Some(Node::Expr(e)) = parent + && let ExprKind::Field(_, name) = e.kind + && !ty_contains_field(typeck.expr_ty(sub_expr), name.name) + { + self.state = Some(( + State::ExplicitDerefField { name: name.name }, + StateData { span: expr.span, hir_id: expr.hir_id }, + )); + } + } RefOp::Method(target_mut) if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id) && is_linted_explicit_deref_position(parent, expr.hir_id, expr.span) => @@ -349,7 +362,7 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { )); } }, - _ => (), + RefOp::Method(..) => (), } }, ( @@ -436,6 +449,11 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing { (state @ Some((State::ExplicitDeref { .. }, _)), RefOp::Deref) => { self.state = state; }, + (Some((State::ExplicitDerefField { name }, data)), RefOp::Deref) + if !ty_contains_field(typeck.expr_ty(sub_expr), name) => + { + self.state = Some((State::ExplicitDerefField { name }, data)); + }, (Some((state, data)), _) => report(cx, expr, state, data), } @@ -879,6 +897,14 @@ fn param_auto_deref_stability(ty: Ty<'_>) -> AutoDerefStability { } } +fn ty_contains_field(ty: Ty<'_>, name: Symbol) -> bool { + if let ty::Adt(adt, _) = *ty.kind() { + adt.is_struct() && adt.non_enum_variant().fields.iter().any(|f| f.name == name) + } else { + false + } +} + #[expect(clippy::needless_pass_by_value)] fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data: StateData) { match state { @@ -968,6 +994,20 @@ fn report<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data }, ); }, + State::ExplicitDerefField { .. } => { + span_lint_hir_and_then( + cx, + EXPLICIT_AUTO_DEREF, + data.hir_id, + data.span, + "deref which would be done by auto-deref", + |diag| { + let mut app = Applicability::MachineApplicable; + let snip = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app).0; + diag.span_suggestion(data.span, "try this", snip.into_owned(), app); + }, + ); + }, State::Borrow | State::Reborrow { .. } => (), } } diff --git a/clippy_lints/src/matches/redundant_pattern_match.rs b/clippy_lints/src/matches/redundant_pattern_match.rs index 0ea3f3b673b7..65cecd333f1c 100644 --- a/clippy_lints/src/matches/redundant_pattern_match.rs +++ b/clippy_lints/src/matches/redundant_pattern_match.rs @@ -362,9 +362,9 @@ fn find_good_method_for_match<'a>( .qpath_res(path_right, arms[1].pat.hir_id) .opt_def_id()?; let body_node_pair = if match_def_path(cx, left_id, expected_left) && match_def_path(cx, right_id, expected_right) { - (&(*arms[0].body).kind, &(*arms[1].body).kind) + (&arms[0].body.kind, &arms[1].body.kind) } else if match_def_path(cx, right_id, expected_left) && match_def_path(cx, right_id, expected_right) { - (&(*arms[1].body).kind, &(*arms[0].body).kind) + (&arms[1].body.kind, &arms[0].body.kind) } else { return None; }; diff --git a/tests/ui/explicit_auto_deref.fixed b/tests/ui/explicit_auto_deref.fixed index 0d3f8b61afc2..91d0379b70e7 100644 --- a/tests/ui/explicit_auto_deref.fixed +++ b/tests/ui/explicit_auto_deref.fixed @@ -164,4 +164,24 @@ fn main() { let ref_s = &s; let _: &String = &*ref_s; // Don't lint reborrow. f_string(&*ref_s); // Don't lint reborrow. + + struct S5 { + foo: u32, + } + let b = Box::new(Box::new(S5 { foo: 5 })); + let _ = b.foo; + let _ = b.foo; + let _ = b.foo; + + struct S6 { + foo: S5, + } + impl core::ops::Deref for S6 { + type Target = S5; + fn deref(&self) -> &Self::Target { + &self.foo + } + } + let s6 = S6 { foo: S5 { foo: 5 } }; + let _ = (*s6).foo; // Don't lint. `S6` also has a field named `foo` } diff --git a/tests/ui/explicit_auto_deref.rs b/tests/ui/explicit_auto_deref.rs index 31c268bcdece..e57553b2a992 100644 --- a/tests/ui/explicit_auto_deref.rs +++ b/tests/ui/explicit_auto_deref.rs @@ -164,4 +164,24 @@ fn main() { let ref_s = &s; let _: &String = &*ref_s; // Don't lint reborrow. f_string(&*ref_s); // Don't lint reborrow. + + struct S5 { + foo: u32, + } + let b = Box::new(Box::new(S5 { foo: 5 })); + let _ = b.foo; + let _ = (*b).foo; + let _ = (**b).foo; + + struct S6 { + foo: S5, + } + impl core::ops::Deref for S6 { + type Target = S5; + fn deref(&self) -> &Self::Target { + &self.foo + } + } + let s6 = S6 { foo: S5 { foo: 5 } }; + let _ = (*s6).foo; // Don't lint. `S6` also has a field named `foo` } diff --git a/tests/ui/explicit_auto_deref.stderr b/tests/ui/explicit_auto_deref.stderr index b53b4b4a200c..54f1a2cd8868 100644 --- a/tests/ui/explicit_auto_deref.stderr +++ b/tests/ui/explicit_auto_deref.stderr @@ -156,5 +156,17 @@ error: deref which would be done by auto-deref LL | let _ = E1::S2 { s: &*s }; | ^^ help: try this: `s` -error: aborting due to 26 previous errors +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:173:13 + | +LL | let _ = (*b).foo; + | ^^^^ help: try this: `b` + +error: deref which would be done by auto-deref + --> $DIR/explicit_auto_deref.rs:174:13 + | +LL | let _ = (**b).foo; + | ^^^^^ help: try this: `b` + +error: aborting due to 28 previous errors