|
| 1 | +use std::ops::ControlFlow; |
| 2 | + |
| 3 | +use clippy_utils::{ |
| 4 | + diagnostics::span_lint_and_then, |
| 5 | + paths, |
| 6 | + ty::match_type, |
| 7 | + visitors::{for_each_expr, Visitable}, |
| 8 | +}; |
| 9 | +use rustc_ast::LitKind; |
| 10 | +use rustc_data_structures::fx::FxHashSet; |
| 11 | +use rustc_hir::{ |
| 12 | + def::{DefKind, Res}, |
| 13 | + Expr, ImplItemKind, MatchSource, Node, |
| 14 | +}; |
| 15 | +use rustc_hir::{Block, PatKind}; |
| 16 | +use rustc_hir::{ExprKind, Impl, ItemKind, QPath, TyKind}; |
| 17 | +use rustc_hir::{ImplItem, Item, VariantData}; |
| 18 | +use rustc_lint::{LateContext, LateLintPass}; |
| 19 | +use rustc_middle::ty::TypeckResults; |
| 20 | +use rustc_middle::ty::{EarlyBinder, Ty}; |
| 21 | +use rustc_session::{declare_lint_pass, declare_tool_lint}; |
| 22 | +use rustc_span::{sym, Span, Symbol}; |
| 23 | + |
| 24 | +declare_clippy_lint! { |
| 25 | + /// ### What it does |
| 26 | + /// Checks for manual [`core::fmt::Debug`](https://doc.rust-lang.org/core/fmt/trait.Debug.html) implementations that do not use all fields. |
| 27 | + /// |
| 28 | + /// ### Why is this bad? |
| 29 | + /// A common mistake is to forget to update manual `Debug` implementations when adding a new field |
| 30 | + /// to a struct or a new variant to an enum. |
| 31 | + /// |
| 32 | + /// At the same time, it also acts as a style lint to suggest using [`core::fmt::DebugStruct::finish_non_exhaustive`](https://doc.rust-lang.org/core/fmt/struct.DebugStruct.html#method.finish_non_exhaustive) |
| 33 | + /// for the times when the user intentionally wants to leave out certain fields (e.g. to hide implementation details). |
| 34 | + /// |
| 35 | + /// ### Known problems |
| 36 | + /// This lint works based on the `DebugStruct` helper types provided by the `Formatter`, |
| 37 | + /// so this won't detect `Debug` impls that use the `write!` macro. |
| 38 | + /// Oftentimes there is more logic to a `Debug` impl if it uses `write!` macro, so it tries |
| 39 | + /// to be on the conservative side and not lint in those cases in an attempt to prevent false positives. |
| 40 | + /// |
| 41 | + /// This lint also does not look through function calls, so calling `.field(self.as_slice())` for example |
| 42 | + /// does not consider fields used inside of `as_slice()` as used by the `Debug` impl. |
| 43 | + /// |
| 44 | + /// Lastly, it also ignores tuple structs as their `DebugTuple` formatter does not have a `finish_non_exhaustive` |
| 45 | + /// method. |
| 46 | + /// |
| 47 | + /// ### Example |
| 48 | + /// ```rust |
| 49 | + /// use std::fmt; |
| 50 | + /// struct Foo { |
| 51 | + /// data: String, |
| 52 | + /// // implementation detail |
| 53 | + /// hidden_data: i32 |
| 54 | + /// } |
| 55 | + /// impl fmt::Debug for Foo { |
| 56 | + /// fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { |
| 57 | + /// formatter |
| 58 | + /// .debug_struct("Foo") |
| 59 | + /// .field("data", &self.data) |
| 60 | + /// .finish() |
| 61 | + /// } |
| 62 | + /// } |
| 63 | + /// ``` |
| 64 | + /// Use instead: |
| 65 | + /// ```rust |
| 66 | + /// use std::fmt; |
| 67 | + /// struct Foo { |
| 68 | + /// data: String, |
| 69 | + /// // implementation detail |
| 70 | + /// hidden_data: i32 |
| 71 | + /// } |
| 72 | + /// impl fmt::Debug for Foo { |
| 73 | + /// fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { |
| 74 | + /// formatter |
| 75 | + /// .debug_struct("Foo") |
| 76 | + /// .field("data", &self.data) |
| 77 | + /// .finish_non_exhaustive() |
| 78 | + /// } |
| 79 | + /// } |
| 80 | + /// ``` |
| 81 | + #[clippy::version = "1.70.0"] |
| 82 | + pub MISSING_FIELDS_IN_DEBUG, |
| 83 | + pedantic, |
| 84 | + "missing fields in manual `Debug` implementation" |
| 85 | +} |
| 86 | +declare_lint_pass!(MissingFieldsInDebug => [MISSING_FIELDS_IN_DEBUG]); |
| 87 | + |
| 88 | +fn report_lints(cx: &LateContext<'_>, span: Span, span_notes: Vec<(Span, &'static str)>) { |
| 89 | + span_lint_and_then( |
| 90 | + cx, |
| 91 | + MISSING_FIELDS_IN_DEBUG, |
| 92 | + span, |
| 93 | + "manual `Debug` impl does not include all fields", |
| 94 | + |diag| { |
| 95 | + for (span, note) in span_notes { |
| 96 | + diag.span_note(span, note); |
| 97 | + } |
| 98 | + diag.help("consider including all fields in this `Debug` impl") |
| 99 | + .help("consider calling `.finish_non_exhaustive()` if you intend to ignore fields"); |
| 100 | + }, |
| 101 | + ); |
| 102 | +} |
| 103 | + |
| 104 | +/// Checks if we should lint in a block of code |
| 105 | +/// |
| 106 | +/// The way we check for this condition is by checking if there is |
| 107 | +/// a call to `Formatter::debug_struct` but no call to `.finish_non_exhaustive()`. |
| 108 | +fn should_lint<'tcx>( |
| 109 | + cx: &LateContext<'tcx>, |
| 110 | + typeck_results: &TypeckResults<'tcx>, |
| 111 | + block: impl Visitable<'tcx>, |
| 112 | +) -> bool { |
| 113 | + // Is there a call to `DebugStruct::finish_non_exhaustive`? Don't lint if there is. |
| 114 | + let mut has_finish_non_exhaustive = false; |
| 115 | + // Is there a call to `DebugStruct::debug_struct`? Do lint if there is. |
| 116 | + let mut has_debug_struct = false; |
| 117 | + |
| 118 | + for_each_expr(block, |expr| { |
| 119 | + if let ExprKind::MethodCall(path, recv, ..) = &expr.kind { |
| 120 | + let recv_ty = typeck_results.expr_ty(recv).peel_refs(); |
| 121 | + |
| 122 | + if path.ident.name == sym::debug_struct && match_type(cx, recv_ty, &paths::FORMATTER) { |
| 123 | + has_debug_struct = true; |
| 124 | + } else if path.ident.name == sym!(finish_non_exhaustive) && match_type(cx, recv_ty, &paths::DEBUG_STRUCT) { |
| 125 | + has_finish_non_exhaustive = true; |
| 126 | + } |
| 127 | + } |
| 128 | + ControlFlow::<!, _>::Continue(()) |
| 129 | + }); |
| 130 | + |
| 131 | + !has_finish_non_exhaustive && has_debug_struct |
| 132 | +} |
| 133 | + |
| 134 | +/// Checks if the given expression is a call to `DebugStruct::field` |
| 135 | +/// and the first argument to it is a string literal and if so, returns it |
| 136 | +/// |
| 137 | +/// Example: `.field("foo", ....)` returns `Some("foo")` |
| 138 | +fn as_field_call<'tcx>( |
| 139 | + cx: &LateContext<'tcx>, |
| 140 | + typeck_results: &TypeckResults<'tcx>, |
| 141 | + expr: &Expr<'_>, |
| 142 | +) -> Option<Symbol> { |
| 143 | + if let ExprKind::MethodCall(path, recv, [debug_field, _], _) = &expr.kind |
| 144 | + && let recv_ty = typeck_results.expr_ty(recv).peel_refs() |
| 145 | + && match_type(cx, recv_ty, &paths::DEBUG_STRUCT) |
| 146 | + && path.ident.name == sym::field |
| 147 | + && let ExprKind::Lit(lit) = &debug_field.kind |
| 148 | + && let LitKind::Str(sym, ..) = lit.node |
| 149 | + { |
| 150 | + Some(sym) |
| 151 | + } else { |
| 152 | + None |
| 153 | + } |
| 154 | +} |
| 155 | + |
| 156 | +/// Attempts to find unused fields assuming that the item is a struct |
| 157 | +fn check_struct<'tcx>( |
| 158 | + cx: &LateContext<'tcx>, |
| 159 | + typeck_results: &TypeckResults<'tcx>, |
| 160 | + block: &'tcx Block<'tcx>, |
| 161 | + self_ty: Ty<'tcx>, |
| 162 | + item: &'tcx Item<'tcx>, |
| 163 | + data: &VariantData<'_>, |
| 164 | +) { |
| 165 | + // Is there a "direct" field access anywhere (i.e. self.foo)? |
| 166 | + // We don't want to lint if there is not, because the user might have |
| 167 | + // a newtype struct and use fields from the wrapped type only. |
| 168 | + let mut has_direct_field_access = false; |
| 169 | + let mut field_accesses = FxHashSet::default(); |
| 170 | + |
| 171 | + for_each_expr(block, |expr| { |
| 172 | + if let ExprKind::Field(target, ident) = expr.kind |
| 173 | + && let target_ty = typeck_results.expr_ty_adjusted(target).peel_refs() |
| 174 | + && target_ty == self_ty |
| 175 | + { |
| 176 | + field_accesses.insert(ident.name); |
| 177 | + has_direct_field_access = true; |
| 178 | + } else if let Some(sym) = as_field_call(cx, typeck_results, expr) { |
| 179 | + field_accesses.insert(sym); |
| 180 | + } |
| 181 | + ControlFlow::<!, _>::Continue(()) |
| 182 | + }); |
| 183 | + |
| 184 | + let span_notes = data |
| 185 | + .fields() |
| 186 | + .iter() |
| 187 | + .filter_map(|field| { |
| 188 | + let EarlyBinder(field_ty) = cx.tcx.type_of(field.def_id); |
| 189 | + if field_accesses.contains(&field.ident.name) || field_ty.is_phantom_data() { |
| 190 | + None |
| 191 | + } else { |
| 192 | + Some((field.span, "this field is unused")) |
| 193 | + } |
| 194 | + }) |
| 195 | + .collect::<Vec<_>>(); |
| 196 | + |
| 197 | + // only lint if there's also at least one direct field access to allow patterns |
| 198 | + // where one might have a newtype struct and uses fields from the wrapped type |
| 199 | + if !span_notes.is_empty() && has_direct_field_access { |
| 200 | + report_lints(cx, item.span, span_notes); |
| 201 | + } |
| 202 | +} |
| 203 | + |
| 204 | +/// Attempts to find unused fields in variants assuming that |
| 205 | +/// the item is an enum. |
| 206 | +/// |
| 207 | +/// Currently, only simple cases are detected where the user |
| 208 | +/// matches on `self` and calls `debug_struct` inside of the arms |
| 209 | +fn check_enum<'tcx>( |
| 210 | + cx: &LateContext<'tcx>, |
| 211 | + typeck_results: &TypeckResults<'tcx>, |
| 212 | + block: &'tcx Block<'tcx>, |
| 213 | + self_ty: Ty<'tcx>, |
| 214 | + item: &'tcx Item<'tcx>, |
| 215 | +) { |
| 216 | + let Some(arms) = for_each_expr(block, |expr| { |
| 217 | + if let ExprKind::Match(val, arms, MatchSource::Normal) = expr.kind |
| 218 | + && let match_ty = typeck_results.expr_ty_adjusted(val).peel_refs() |
| 219 | + && match_ty == self_ty |
| 220 | + { |
| 221 | + ControlFlow::Break(arms) |
| 222 | + } else { |
| 223 | + ControlFlow::Continue(()) |
| 224 | + } |
| 225 | + }) else { |
| 226 | + return; |
| 227 | + }; |
| 228 | + |
| 229 | + let mut span_notes = Vec::new(); |
| 230 | + |
| 231 | + for arm in arms { |
| 232 | + if !should_lint(cx, typeck_results, arm.body) { |
| 233 | + continue; |
| 234 | + } |
| 235 | + |
| 236 | + arm.pat.walk_always(|pat| match pat.kind { |
| 237 | + PatKind::Wild => span_notes.push((pat.span, "unused field here due to wildcard `_`")), |
| 238 | + PatKind::Tuple(_, rest) | PatKind::TupleStruct(.., rest) if rest.as_opt_usize().is_some() => { |
| 239 | + span_notes.push((pat.span, "more unused fields here due to rest pattern `..`")); |
| 240 | + }, |
| 241 | + PatKind::Struct(.., true) => { |
| 242 | + span_notes.push((pat.span, "more unused fields here due to rest pattern `..`")); |
| 243 | + }, |
| 244 | + _ => {}, |
| 245 | + }); |
| 246 | + |
| 247 | + let mut field_accesses = FxHashSet::default(); |
| 248 | + let mut check_field_access = |sym, expr| { |
| 249 | + if !typeck_results.expr_ty(expr).is_phantom_data() { |
| 250 | + arm.pat.each_binding(|_, _, _, pat_ident| { |
| 251 | + if sym == pat_ident.name { |
| 252 | + field_accesses.insert(pat_ident); |
| 253 | + } |
| 254 | + }); |
| 255 | + } |
| 256 | + }; |
| 257 | + |
| 258 | + for_each_expr(arm.body, |expr| { |
| 259 | + if let ExprKind::Path(QPath::Resolved(_, path)) = expr.kind && let Some(segment) = path.segments.first() |
| 260 | + { |
| 261 | + check_field_access(segment.ident.name, expr); |
| 262 | + } else if let Some(sym) = as_field_call(cx, typeck_results, expr) { |
| 263 | + check_field_access(sym, expr); |
| 264 | + } |
| 265 | + ControlFlow::<!, _>::Continue(()) |
| 266 | + }); |
| 267 | + |
| 268 | + arm.pat.each_binding(|_, _, span, pat_ident| { |
| 269 | + if !field_accesses.contains(&pat_ident) { |
| 270 | + span_notes.push((span, "the field referenced by this binding is unused")); |
| 271 | + } |
| 272 | + }); |
| 273 | + } |
| 274 | + |
| 275 | + if !span_notes.is_empty() { |
| 276 | + report_lints(cx, item.span, span_notes); |
| 277 | + } |
| 278 | +} |
| 279 | + |
| 280 | +impl<'tcx> LateLintPass<'tcx> for MissingFieldsInDebug { |
| 281 | + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx rustc_hir::Item<'tcx>) { |
| 282 | + // is this an `impl Debug for X` block? |
| 283 | + if let ItemKind::Impl(Impl { of_trait: Some(trait_ref), self_ty, items, .. }) = item.kind |
| 284 | + && let Res::Def(DefKind::Trait, trait_def_id) = trait_ref.path.res |
| 285 | + && let TyKind::Path(QPath::Resolved(_, self_path)) = &self_ty.kind |
| 286 | + && cx.match_def_path(trait_def_id, &[sym::core, sym::fmt, sym::Debug]) |
| 287 | + // don't trigger if this impl was derived |
| 288 | + && !cx.tcx.has_attr(item.owner_id, sym::automatically_derived) |
| 289 | + && !item.span.from_expansion() |
| 290 | + // find `Debug::fmt` function |
| 291 | + && let Some(fmt_item) = items.iter().find(|i| i.ident.name == sym::fmt) |
| 292 | + && let ImplItem { kind: ImplItemKind::Fn(_, body_id), .. } = cx.tcx.hir().impl_item(fmt_item.id) |
| 293 | + && let body = cx.tcx.hir().body(*body_id) |
| 294 | + && let ExprKind::Block(block, _) = body.value.kind |
| 295 | + // inspect `self` |
| 296 | + && let self_ty = cx.tcx.type_of(self_path.res.def_id()).0.peel_refs() |
| 297 | + && let Some(self_adt) = self_ty.ty_adt_def() |
| 298 | + && let Some(self_def_id) = self_adt.did().as_local() |
| 299 | + && let Some(Node::Item(self_item)) = cx.tcx.hir().find_by_def_id(self_def_id) |
| 300 | + // NB: can't call cx.typeck_results() as we are not in a body |
| 301 | + && let typeck_results = cx.tcx.typeck_body(*body_id) |
| 302 | + && should_lint(cx, typeck_results, block) |
| 303 | + { |
| 304 | + match &self_item.kind { |
| 305 | + ItemKind::Struct(data, _) => check_struct(cx, typeck_results, block, self_ty, item, data), |
| 306 | + ItemKind::Enum(..) => check_enum(cx, typeck_results, block, self_ty, item), |
| 307 | + _ => {} |
| 308 | + } |
| 309 | + } |
| 310 | + } |
| 311 | +} |
0 commit comments