diff options
| -rw-r--r-- | clippy_lints/src/missing_fields_in_debug.rs | 101 | ||||
| -rw-r--r-- | tests/ui/missing_fields_in_debug.rs | 128 | ||||
| -rw-r--r-- | tests/ui/missing_fields_in_debug.stderr | 42 |
3 files changed, 13 insertions, 258 deletions
diff --git a/clippy_lints/src/missing_fields_in_debug.rs b/clippy_lints/src/missing_fields_in_debug.rs index 389a8d04dc2..8332d638f92 100644 --- a/clippy_lints/src/missing_fields_in_debug.rs +++ b/clippy_lints/src/missing_fields_in_debug.rs @@ -2,22 +2,22 @@ use std::ops::ControlFlow; use clippy_utils::{ diagnostics::span_lint_and_then, - paths, + is_path_lang_item, paths, ty::match_type, visitors::{for_each_expr, Visitable}, }; use rustc_ast::LitKind; use rustc_data_structures::fx::FxHashSet; +use rustc_hir::Block; use rustc_hir::{ def::{DefKind, Res}, - Expr, ImplItemKind, MatchSource, Node, + Expr, ImplItemKind, LangItem, Node, }; -use rustc_hir::{Block, PatKind}; use rustc_hir::{ExprKind, Impl, ItemKind, QPath, TyKind}; use rustc_hir::{ImplItem, Item, VariantData}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::Ty; use rustc_middle::ty::TypeckResults; -use rustc_middle::ty::{EarlyBinder, Ty}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{sym, Span, Symbol}; @@ -38,11 +38,12 @@ declare_clippy_lint! { /// Oftentimes there is more logic to a `Debug` impl if it uses `write!` macro, so it tries /// to be on the conservative side and not lint in those cases in an attempt to prevent false positives. /// - /// This lint also does not look through function calls, so calling `.field(self.as_slice())` for example - /// does not consider fields used inside of `as_slice()` as used by the `Debug` impl. + /// This lint also does not look through function calls, so calling a function does not consider fields + /// used inside of that function as used by the `Debug` impl. /// /// Lastly, it also ignores tuple structs as their `DebugTuple` formatter does not have a `finish_non_exhaustive` - /// method. + /// method, as well as enums because their exhaustiveness is already checked by the compiler when matching on the enum, + /// making it much less likely to accidentally forget to update the `Debug` impl when adding a new variant. /// /// ### Example /// ```rust @@ -185,8 +186,7 @@ fn check_struct<'tcx>( .fields() .iter() .filter_map(|field| { - let EarlyBinder(field_ty) = cx.tcx.type_of(field.def_id); - if field_accesses.contains(&field.ident.name) || field_ty.is_phantom_data() { + if field_accesses.contains(&field.ident.name) || is_path_lang_item(cx, field.ty, LangItem::PhantomData) { None } else { Some((field.span, "this field is unused")) @@ -201,82 +201,6 @@ fn check_struct<'tcx>( } } -/// Attempts to find unused fields in variants assuming that -/// the item is an enum. -/// -/// Currently, only simple cases are detected where the user -/// matches on `self` and calls `debug_struct` inside of the arms -fn check_enum<'tcx>( - cx: &LateContext<'tcx>, - typeck_results: &TypeckResults<'tcx>, - block: &'tcx Block<'tcx>, - self_ty: Ty<'tcx>, - item: &'tcx Item<'tcx>, -) { - let Some(arms) = for_each_expr(block, |expr| { - if let ExprKind::Match(val, arms, MatchSource::Normal) = expr.kind - && let match_ty = typeck_results.expr_ty_adjusted(val).peel_refs() - && match_ty == self_ty - { - ControlFlow::Break(arms) - } else { - ControlFlow::Continue(()) - } - }) else { - return; - }; - - let mut span_notes = Vec::new(); - - for arm in arms { - if !should_lint(cx, typeck_results, arm.body) { - continue; - } - - arm.pat.walk_always(|pat| match pat.kind { - PatKind::Wild => span_notes.push((pat.span, "unused field here due to wildcard `_`")), - PatKind::Tuple(_, rest) | PatKind::TupleStruct(.., rest) if rest.as_opt_usize().is_some() => { - span_notes.push((pat.span, "more unused fields here due to rest pattern `..`")); - }, - PatKind::Struct(.., true) => { - span_notes.push((pat.span, "more unused fields here due to rest pattern `..`")); - }, - _ => {}, - }); - - let mut field_accesses = FxHashSet::default(); - let mut check_field_access = |sym, expr| { - if !typeck_results.expr_ty(expr).is_phantom_data() { - arm.pat.each_binding(|_, _, _, pat_ident| { - if sym == pat_ident.name { - field_accesses.insert(pat_ident); - } - }); - } - }; - - for_each_expr(arm.body, |expr| { - if let ExprKind::Path(QPath::Resolved(_, path)) = expr.kind && let Some(segment) = path.segments.first() - { - check_field_access(segment.ident.name, expr); - } else if let Some(sym) = as_field_call(cx, typeck_results, expr) { - check_field_access(sym, expr); - } - ControlFlow::<!, _>::Continue(()) - }); - - arm.pat.each_binding(|_, _, span, pat_ident| { - if !field_accesses.contains(&pat_ident) { - span_notes.push((span, "the field referenced by this binding is unused")); - } - }); - } - - if !span_notes.is_empty() { - report_lints(cx, item.span, span_notes); - } -} - impl<'tcx> LateLintPass<'tcx> for MissingFieldsInDebug { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx rustc_hir::Item<'tcx>) { // is this an `impl Debug for X` block? @@ -301,10 +225,9 @@ impl<'tcx> LateLintPass<'tcx> for MissingFieldsInDebug { && let typeck_results = cx.tcx.typeck_body(*body_id) && should_lint(cx, typeck_results, block) { - match &self_item.kind { - ItemKind::Struct(data, _) => check_struct(cx, typeck_results, block, self_ty, item, data), - ItemKind::Enum(..) => check_enum(cx, typeck_results, block, self_ty, item), - _ => {} + // we intentionally only lint structs, see lint description + if let ItemKind::Struct(data, _) = &self_item.kind { + check_struct(cx, typeck_results, block, self_ty, item, data); } } } diff --git a/tests/ui/missing_fields_in_debug.rs b/tests/ui/missing_fields_in_debug.rs index 72b9e0e2aae..c156d394ece 100644 --- a/tests/ui/missing_fields_in_debug.rs +++ b/tests/ui/missing_fields_in_debug.rs @@ -97,140 +97,12 @@ impl fmt::Debug for MultiExprDebugImpl { } } -enum SingleVariantEnumUnnamed { - A(u8), -} - -// ok -impl fmt::Debug for SingleVariantEnumUnnamed { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::A(n) => formatter.debug_tuple("A").field(&n).finish(), - } - } -} - -enum MultiVariantEnum { - A(u8), - B { a: u8, b: String }, - C, -} - -impl fmt::Debug for MultiVariantEnum { - // match arm Self::B ignores `b` - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::A(n) => formatter.debug_tuple("A").field(&n).finish(), - Self::B { a, b } => formatter.debug_struct("B").field("a", &a).finish(), - Self::C => formatter.debug_struct("C").finish(), - } - } -} - -enum MultiVariantEnumOk { - A(u8), - B { a: u8, b: String }, - C, -} - -// ok -impl fmt::Debug for MultiVariantEnumOk { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::A(n) => formatter.debug_tuple("A").field(&n).finish(), - Self::B { a, b } => formatter.debug_struct("B").field("a", &a).field("b", &b).finish(), - Self::C => formatter.debug_struct("C").finish(), - } - } -} - -enum MultiVariantEnumNonExhaustive { - A(u8), - B { a: u8, b: String }, - C, -} - -// ok -impl fmt::Debug for MultiVariantEnumNonExhaustive { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::A(n) => formatter.debug_tuple("A").field(&n).finish(), - Self::B { a, b } => formatter.debug_struct("B").field("b", &b).finish_non_exhaustive(), - Self::C => formatter.debug_struct("C").finish(), - } - } -} - -enum MultiVariantRest { - A(u8), - B { a: u8, b: String }, - C, -} - -impl fmt::Debug for MultiVariantRest { - // `a` field ignored due to rest pattern - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::A(n) => formatter.debug_tuple("A").field(&n).finish(), - Self::B { b, .. } => formatter.debug_struct("B").field("b", &b).finish(), - Self::C => formatter.debug_struct("C").finish(), - } - } -} - -enum MultiVariantRestNonExhaustive { - A(u8), - B { a: u8, b: String }, - C, -} - -// ok -impl fmt::Debug for MultiVariantRestNonExhaustive { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::A(n) => formatter.debug_tuple("A").field(&n).finish(), - Self::B { b, .. } => formatter.debug_struct("B").field("b", &b).finish_non_exhaustive(), - Self::C => formatter.debug_struct("C").finish(), - } - } -} - -enum Wildcard { - A(u8), - B(String), -} - -// ok -impl fmt::Debug for Wildcard { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::A(n) => formatter.debug_tuple("A").field(&n).finish(), - _ => todo!(), - } - } -} - -enum Empty {} - -// ok -impl fmt::Debug for Empty { - fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { - match *self {} - } -} - #[derive(Debug)] struct DerivedStruct { a: u8, b: i32, } -#[derive(Debug)] -enum DerivedEnum { - A(i32), - B { a: String }, -} - // https://github.com/rust-lang/rust-clippy/pull/10616#discussion_r1166846953 struct Inner { diff --git a/tests/ui/missing_fields_in_debug.stderr b/tests/ui/missing_fields_in_debug.stderr index 1dc7c0d65e5..ef9d02abab7 100644 --- a/tests/ui/missing_fields_in_debug.stderr +++ b/tests/ui/missing_fields_in_debug.stderr @@ -69,45 +69,5 @@ LL | b: String, = help: consider including all fields in this `Debug` impl = help: consider calling `.finish_non_exhaustive()` if you intend to ignore fields -error: manual `Debug` impl does not include all fields - --> $DIR/missing_fields_in_debug.rs:119:1 - | -LL | / impl fmt::Debug for MultiVariantEnum { -LL | | // match arm Self::B ignores `b` -LL | | fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { -LL | | match self { -... | -LL | | } -LL | | } - | |_^ - | -note: the field referenced by this binding is unused - --> $DIR/missing_fields_in_debug.rs:124:26 - | -LL | Self::B { a, b } => formatter.debug_struct("B").field("a", &a).finish(), - | ^ - = help: consider including all fields in this `Debug` impl - = help: consider calling `.finish_non_exhaustive()` if you intend to ignore fields - -error: manual `Debug` impl does not include all fields - --> $DIR/missing_fields_in_debug.rs:170:1 - | -LL | / impl fmt::Debug for MultiVariantRest { -LL | | // `a` field ignored due to rest pattern -LL | | fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { -LL | | match self { -... | -LL | | } -LL | | } - | |_^ - | -note: more unused fields here due to rest pattern `..` - --> $DIR/missing_fields_in_debug.rs:175:13 - | -LL | Self::B { b, .. } => formatter.debug_struct("B").field("b", &b).finish(), - | ^^^^^^^^^^^^^^^^^ - = help: consider including all fields in this `Debug` impl - = help: consider calling `.finish_non_exhaustive()` if you intend to ignore fields - -error: aborting due to 5 previous errors +error: aborting due to 3 previous errors |