|
| 1 | +use clippy_utils::diagnostics::span_lint; |
| 2 | +use clippy_utils::higher::{FormatArgsArg, FormatArgsExpn}; |
| 3 | +use clippy_utils::{is_diag_trait_item, match_def_path, path_to_local_id, paths}; |
| 4 | +use if_chain::if_chain; |
| 5 | +use rustc_hir::{Expr, ExprKind, HirId, Impl, ImplItem, ImplItemKind, Item, ItemKind, UnOp}; |
| 6 | +use rustc_lint::{LateContext, LateLintPass}; |
| 7 | +use rustc_session::{declare_tool_lint, impl_lint_pass}; |
| 8 | +use rustc_span::{sym, ExpnData, ExpnKind, Symbol}; |
| 9 | + |
| 10 | +const FORMAT_MACRO_PATHS: &[&[&str]] = &[ |
| 11 | + &paths::FORMAT_ARGS_MACRO, |
| 12 | + &paths::ASSERT_EQ_MACRO, |
| 13 | + &paths::ASSERT_MACRO, |
| 14 | + &paths::ASSERT_NE_MACRO, |
| 15 | + &paths::EPRINT_MACRO, |
| 16 | + &paths::EPRINTLN_MACRO, |
| 17 | + &paths::PRINT_MACRO, |
| 18 | + &paths::PRINTLN_MACRO, |
| 19 | + &paths::WRITE_MACRO, |
| 20 | + &paths::WRITELN_MACRO, |
| 21 | +]; |
| 22 | + |
| 23 | +#[derive(Clone, Copy)] |
| 24 | +enum ImplTrait { |
| 25 | + Debug, |
| 26 | + Display, |
| 27 | +} |
| 28 | + |
| 29 | +const FORMAT_MACRO_DIAG_ITEMS: &[Symbol] = &[sym::format_macro, sym::std_panic_macro]; |
| 30 | + |
| 31 | +fn outermost_expn_data(expn_data: ExpnData) -> ExpnData { |
| 32 | + if expn_data.call_site.from_expansion() { |
| 33 | + outermost_expn_data(expn_data.call_site.ctxt().outer_expn_data()) |
| 34 | + } else { |
| 35 | + expn_data |
| 36 | + } |
| 37 | +} |
| 38 | + |
| 39 | +declare_clippy_lint! { |
| 40 | + /// ### What it does |
| 41 | + /// Checks for recursive use of `Display` or `Debug` traits inside their implementation. |
| 42 | + /// |
| 43 | + /// ### Why is this bad? |
| 44 | + /// This is unconditional recursion and so will lead to infinite |
| 45 | + /// recursion and a stack overflow. |
| 46 | + /// |
| 47 | + /// ### Example |
| 48 | + /// |
| 49 | + /// ```rust |
| 50 | + /// use std::fmt; |
| 51 | + /// |
| 52 | + /// struct Structure(i32); |
| 53 | + /// impl fmt::Display for Structure { |
| 54 | + /// fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { |
| 55 | + /// write!(f, "{}", self.to_string()) |
| 56 | + /// } |
| 57 | + /// } |
| 58 | + /// |
| 59 | + /// ``` |
| 60 | + /// Use instead: |
| 61 | + /// ```rust |
| 62 | + /// use std::fmt; |
| 63 | + /// |
| 64 | + /// struct Structure(i32); |
| 65 | + /// impl fmt::Display for Structure { |
| 66 | + /// fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { |
| 67 | + /// write!(f, "{}", self.0) |
| 68 | + /// } |
| 69 | + /// } |
| 70 | + /// ``` |
| 71 | + #[clippy::version = "1.48.0"] |
| 72 | + pub RECURSIVE_FORMAT_TRAIT_IMPL, |
| 73 | + correctness, |
| 74 | + "Format trait method called while implementing the same Format trait" |
| 75 | +} |
| 76 | + |
| 77 | +#[derive(Default)] |
| 78 | +pub struct RecursiveFormatTraitImpl { |
| 79 | + // Whether we are inside Display or Debug trait impl - None for neither |
| 80 | + format_trait_impl: Option<ImplTrait>, |
| 81 | + // hir_id of self parameter of method inside Display Impl - i.e. fmt(&self) |
| 82 | + self_hir_id: Option<HirId>, |
| 83 | +} |
| 84 | + |
| 85 | +impl RecursiveFormatTraitImpl { |
| 86 | + pub fn new() -> Self { |
| 87 | + Self { |
| 88 | + format_trait_impl: None, |
| 89 | + self_hir_id: None, |
| 90 | + } |
| 91 | + } |
| 92 | +} |
| 93 | + |
| 94 | +impl_lint_pass!(RecursiveFormatTraitImpl => [RECURSIVE_FORMAT_TRAIT_IMPL]); |
| 95 | + |
| 96 | +impl LateLintPass<'_> for RecursiveFormatTraitImpl { |
| 97 | + fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { |
| 98 | + if let Some(format_trait_impl) = is_format_trait_impl(cx, item) { |
| 99 | + self.format_trait_impl = Some(format_trait_impl); |
| 100 | + } |
| 101 | + } |
| 102 | + |
| 103 | + fn check_item_post(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { |
| 104 | + // Assume no nested Impl of Debug and Display within eachother |
| 105 | + if is_format_trait_impl(cx, item).is_some() { |
| 106 | + self.format_trait_impl = None; |
| 107 | + self.self_hir_id = None; |
| 108 | + } |
| 109 | + } |
| 110 | + |
| 111 | + fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) { |
| 112 | + if_chain! { |
| 113 | + // If we are in Display or Debug impl, then get hir_id for self in method impl - i.e. fmt(&self) |
| 114 | + if self.format_trait_impl.is_some(); |
| 115 | + if let ImplItemKind::Fn(.., body_id) = &impl_item.kind; |
| 116 | + let body = cx.tcx.hir().body(*body_id); |
| 117 | + if !body.params.is_empty(); |
| 118 | + then { |
| 119 | + let self_param = &body.params[0]; |
| 120 | + self.self_hir_id = Some(self_param.pat.hir_id); |
| 121 | + } |
| 122 | + } |
| 123 | + } |
| 124 | + |
| 125 | + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { |
| 126 | + if let Some(self_hir_id) = self.self_hir_id { |
| 127 | + match self.format_trait_impl { |
| 128 | + Some(ImplTrait::Display) => { |
| 129 | + check_to_string_in_display(cx, expr, self_hir_id); |
| 130 | + check_self_in_format_args(cx, expr, self_hir_id, ImplTrait::Display); |
| 131 | + }, |
| 132 | + Some(ImplTrait::Debug) => { |
| 133 | + check_self_in_format_args(cx, expr, self_hir_id, ImplTrait::Debug); |
| 134 | + }, |
| 135 | + None => {}, |
| 136 | + } |
| 137 | + } |
| 138 | + } |
| 139 | +} |
| 140 | + |
| 141 | +fn check_to_string_in_display(cx: &LateContext<'_>, expr: &Expr<'_>, self_hir_id: HirId) { |
| 142 | + if_chain! { |
| 143 | + // Get the hir_id of the object we are calling the method on |
| 144 | + if let ExprKind::MethodCall(path, _, [ref self_arg, ..], _) = expr.kind; |
| 145 | + // Is the method to_string() ? |
| 146 | + if path.ident.name == sym!(to_string); |
| 147 | + // Is the method a part of the ToString trait? (i.e. not to_string() implemented |
| 148 | + // separately) |
| 149 | + if let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id); |
| 150 | + if is_diag_trait_item(cx, expr_def_id, sym::ToString); |
| 151 | + // Is the method is called on self |
| 152 | + if path_to_local_id(self_arg, self_hir_id); |
| 153 | + then { |
| 154 | + span_lint( |
| 155 | + cx, |
| 156 | + RECURSIVE_FORMAT_TRAIT_IMPL, |
| 157 | + expr.span, |
| 158 | + "using `to_string` in `fmt::Display` implementation might lead to infinite recursion", |
| 159 | + ); |
| 160 | + } |
| 161 | + } |
| 162 | +} |
| 163 | + |
| 164 | +fn check_self_in_format_args(cx: &LateContext<'_>, expr: &Expr<'_>, self_hir_id: HirId, impl_trait: ImplTrait) { |
| 165 | + // Check each arg in format calls - do we ever use Display on self (directly or via deref)? |
| 166 | + if_chain! { |
| 167 | + if let Some(format_args) = FormatArgsExpn::parse(expr); |
| 168 | + let expr_expn_data = expr.span.ctxt().outer_expn_data(); |
| 169 | + let outermost_expn_data = outermost_expn_data(expr_expn_data); |
| 170 | + if let Some(macro_def_id) = outermost_expn_data.macro_def_id; |
| 171 | + if FORMAT_MACRO_PATHS |
| 172 | + .iter() |
| 173 | + .any(|path| match_def_path(cx, macro_def_id, path)) |
| 174 | + || FORMAT_MACRO_DIAG_ITEMS |
| 175 | + .iter() |
| 176 | + .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, macro_def_id)); |
| 177 | + if let ExpnKind::Macro(_, _name) = outermost_expn_data.kind; |
| 178 | + if let Some(args) = format_args.args(); |
| 179 | + then { |
| 180 | + for (_i, arg) in args.iter().enumerate() { |
| 181 | + match impl_trait { |
| 182 | + // In Display, we only care about Display (it is okay to use Debug) |
| 183 | + ImplTrait::Display => { |
| 184 | + if !arg.is_display() { |
| 185 | + continue; |
| 186 | + }}, |
| 187 | + // In Debug, we only care about Debug (it is okay to use Display and ToString) |
| 188 | + ImplTrait::Debug => { |
| 189 | + if !arg.is_debug() { |
| 190 | + continue; |
| 191 | + }}, |
| 192 | + |
| 193 | + }; |
| 194 | + check_format_arg_self(cx, expr, self_hir_id, arg, impl_trait); |
| 195 | + } |
| 196 | + } |
| 197 | + } |
| 198 | +} |
| 199 | + |
| 200 | +fn check_format_arg_self( |
| 201 | + cx: &LateContext<'_>, |
| 202 | + expr: &Expr<'_>, |
| 203 | + self_hir_id: HirId, |
| 204 | + arg: &FormatArgsArg<'_>, |
| 205 | + impl_trait: ImplTrait, |
| 206 | +) { |
| 207 | + // Handle multiple dereferencing of references e.g. &&self |
| 208 | + // Handle single dereference of &self -> self that is equivalent (i.e. via *self in fmt() impl) |
| 209 | + // Since the argument to fmt is itself a reference: &self |
| 210 | + let reference = single_deref(deref_expr(arg.value)); |
| 211 | + if path_to_local_id(reference, self_hir_id) { |
| 212 | + match impl_trait { |
| 213 | + ImplTrait::Display => { |
| 214 | + span_lint( |
| 215 | + cx, |
| 216 | + RECURSIVE_FORMAT_TRAIT_IMPL, |
| 217 | + expr.span, |
| 218 | + "using `self` as Display in `fmt::Display` implementation might lead to infinite recursion", |
| 219 | + ); |
| 220 | + }, |
| 221 | + ImplTrait::Debug => { |
| 222 | + span_lint( |
| 223 | + cx, |
| 224 | + RECURSIVE_FORMAT_TRAIT_IMPL, |
| 225 | + expr.span, |
| 226 | + "using `self` as Debug in `fmt::Debug` implementation might lead to infinite recursion", |
| 227 | + ); |
| 228 | + }, |
| 229 | + } |
| 230 | + } |
| 231 | +} |
| 232 | + |
| 233 | +fn deref_expr<'a, 'b>(expr: &'a Expr<'b>) -> &'a Expr<'b> { |
| 234 | + if let ExprKind::AddrOf(_, _, reference) = expr.kind { |
| 235 | + deref_expr(reference) |
| 236 | + } else { |
| 237 | + expr |
| 238 | + } |
| 239 | +} |
| 240 | + |
| 241 | +fn single_deref<'a, 'b>(expr: &'a Expr<'b>) -> &'a Expr<'b> { |
| 242 | + if let ExprKind::Unary(UnOp::Deref, ex) = expr.kind { |
| 243 | + ex |
| 244 | + } else { |
| 245 | + expr |
| 246 | + } |
| 247 | +} |
| 248 | + |
| 249 | +fn is_format_trait_impl(cx: &LateContext<'_>, item: &'hir Item<'_>) -> Option<ImplTrait> { |
| 250 | + if_chain! { |
| 251 | + // Are we at an Impl? |
| 252 | + if let ItemKind::Impl(Impl { of_trait: Some(trait_ref), .. }) = &item.kind; |
| 253 | + if let Some(did) = trait_ref.trait_def_id(); |
| 254 | + then { |
| 255 | + // Is it for Display trait? |
| 256 | + if match_def_path(cx, did, &paths::DISPLAY_TRAIT) { |
| 257 | + Some(ImplTrait::Display) |
| 258 | + } |
| 259 | + // Is it for Debug trait? |
| 260 | + else if match_def_path(cx, did, &paths::DEBUG_TRAIT) { |
| 261 | + Some(ImplTrait::Debug) |
| 262 | + } else { |
| 263 | + None |
| 264 | + } |
| 265 | + } else { |
| 266 | + None |
| 267 | + } |
| 268 | + } |
| 269 | +} |
0 commit comments