From e88ec7189478d85168525080825570594cd1817c Mon Sep 17 00:00:00 2001 From: James McMurray Date: Tue, 28 Dec 2021 19:27:11 +0100 Subject: [PATCH] Add recursive_display_impl lint The to_string_in_display lint is renamed to recursive_display_impl A check is added for the use of self formatted with Display inside any format string in the Display impl The to_string_in_display check is kept as is - like in the format_in_format_args lint --- CHANGELOG.md | 6 +- clippy_lints/src/lib.register_all.rs | 2 +- clippy_lints/src/lib.register_correctness.rs | 2 +- clippy_lints/src/lib.register_lints.rs | 2 +- clippy_lints/src/lib.rs | 4 +- clippy_lints/src/recursive_display_impl.rs | 222 +++++++++++++++++++ clippy_lints/src/to_string_in_display.rs | 123 ---------- tests/ui/recursive_display_impl.rs | 197 ++++++++++++++++ tests/ui/recursive_display_impl.stderr | 50 +++++ tests/ui/to_string_in_display.rs | 69 ------ tests/ui/to_string_in_display.stderr | 10 - 11 files changed, 477 insertions(+), 210 deletions(-) create mode 100644 clippy_lints/src/recursive_display_impl.rs delete mode 100644 clippy_lints/src/to_string_in_display.rs create mode 100644 tests/ui/recursive_display_impl.rs create mode 100644 tests/ui/recursive_display_impl.stderr delete mode 100644 tests/ui/to_string_in_display.rs delete mode 100644 tests/ui/to_string_in_display.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index c09310e55568..e2feaa595c8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1267,7 +1267,7 @@ Released 2020-11-19 * [`manual_strip`] [#6038](https://github.com/rust-lang/rust-clippy/pull/6038) * [`map_err_ignore`] [#5998](https://github.com/rust-lang/rust-clippy/pull/5998) * [`rc_buffer`] [#6044](https://github.com/rust-lang/rust-clippy/pull/6044) -* [`to_string_in_display`] [#5831](https://github.com/rust-lang/rust-clippy/pull/5831) +* `to_string_in_display` [#5831](https://github.com/rust-lang/rust-clippy/pull/5831) * `single_char_push_str` [#5881](https://github.com/rust-lang/rust-clippy/pull/5881) ### Moves and Deprecations @@ -1310,7 +1310,7 @@ Released 2020-11-19 [#5949](https://github.com/rust-lang/rust-clippy/pull/5949) * [`doc_markdown`]: allow using "GraphQL" without backticks [#5996](https://github.com/rust-lang/rust-clippy/pull/5996) -* [`to_string_in_display`]: avoid linting when calling `to_string()` on anything that is not `self` +* `to_string_in_display`: avoid linting when calling `to_string()` on anything that is not `self` [#5971](https://github.com/rust-lang/rust-clippy/pull/5971) * [`indexing_slicing`] and [`out_of_bounds_indexing`] treat references to arrays as arrays [#6034](https://github.com/rust-lang/rust-clippy/pull/6034) @@ -3209,6 +3209,7 @@ Released 2018-09-13 [`range_zip_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_zip_with_len [`rc_buffer`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer [`rc_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex +[`recursive_display_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#recursive_display_impl [`redundant_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation [`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone [`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure @@ -3283,7 +3284,6 @@ Released 2018-09-13 [`tabs_in_doc_comments`]: https://rust-lang.github.io/rust-clippy/master/index.html#tabs_in_doc_comments [`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment [`to_digit_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_digit_is_some -[`to_string_in_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_display [`to_string_in_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_format_args [`todo`]: https://rust-lang.github.io/rust-clippy/master/index.html#todo [`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 3d3999d4cc0d..00791508725e 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -237,6 +237,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(ranges::MANUAL_RANGE_CONTAINS), LintId::of(ranges::RANGE_ZIP_WITH_LEN), LintId::of(ranges::REVERSED_EMPTY_RANGES), + LintId::of(recursive_display_impl::RECURSIVE_DISPLAY_IMPL), LintId::of(redundant_clone::REDUNDANT_CLONE), LintId::of(redundant_closure_call::REDUNDANT_CLOSURE_CALL), LintId::of(redundant_field_names::REDUNDANT_FIELD_NAMES), @@ -265,7 +266,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(tabs_in_doc_comments::TABS_IN_DOC_COMMENTS), LintId::of(temporary_assignment::TEMPORARY_ASSIGNMENT), LintId::of(to_digit_is_some::TO_DIGIT_IS_SOME), - LintId::of(to_string_in_display::TO_STRING_IN_DISPLAY), LintId::of(transmute::CROSSPOINTER_TRANSMUTE), LintId::of(transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS), LintId::of(transmute::TRANSMUTE_BYTES_TO_STR), diff --git a/clippy_lints/src/lib.register_correctness.rs b/clippy_lints/src/lib.register_correctness.rs index 4217fd3a3ea7..2cdf7f4a4bcb 100644 --- a/clippy_lints/src/lib.register_correctness.rs +++ b/clippy_lints/src/lib.register_correctness.rs @@ -52,12 +52,12 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve LintId::of(ptr::INVALID_NULL_PTR_USAGE), LintId::of(ptr::MUT_FROM_REF), LintId::of(ranges::REVERSED_EMPTY_RANGES), + LintId::of(recursive_display_impl::RECURSIVE_DISPLAY_IMPL), LintId::of(regex::INVALID_REGEX), LintId::of(self_assignment::SELF_ASSIGNMENT), LintId::of(serde_api::SERDE_API_MISUSE), LintId::of(size_of_in_element_count::SIZE_OF_IN_ELEMENT_COUNT), LintId::of(swap::ALMOST_SWAPPED), - LintId::of(to_string_in_display::TO_STRING_IN_DISPLAY), LintId::of(transmute::UNSOUND_COLLECTION_TRANSMUTE), LintId::of(transmute::WRONG_TRANSMUTE), LintId::of(transmuting_null::TRANSMUTING_NULL), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 766c5ba1bcb0..a3c920c04450 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -410,6 +410,7 @@ store.register_lints(&[ ranges::RANGE_PLUS_ONE, ranges::RANGE_ZIP_WITH_LEN, ranges::REVERSED_EMPTY_RANGES, + recursive_display_impl::RECURSIVE_DISPLAY_IMPL, redundant_clone::REDUNDANT_CLONE, redundant_closure_call::REDUNDANT_CLOSURE_CALL, redundant_else::REDUNDANT_ELSE, @@ -454,7 +455,6 @@ store.register_lints(&[ tabs_in_doc_comments::TABS_IN_DOC_COMMENTS, temporary_assignment::TEMPORARY_ASSIGNMENT, to_digit_is_some::TO_DIGIT_IS_SOME, - to_string_in_display::TO_STRING_IN_DISPLAY, trailing_empty_array::TRAILING_EMPTY_ARRAY, trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS, trait_bounds::TYPE_REPETITION_IN_BOUNDS, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d1c7956a7a5c..14b9d4a0c773 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -329,6 +329,7 @@ mod ptr_eq; mod ptr_offset_with_cast; mod question_mark; mod ranges; +mod recursive_display_impl; mod redundant_clone; mod redundant_closure_call; mod redundant_else; @@ -360,7 +361,6 @@ mod swap; mod tabs_in_doc_comments; mod temporary_assignment; mod to_digit_is_some; -mod to_string_in_display; mod trailing_empty_array; mod trait_bounds; mod transmute; @@ -703,7 +703,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_early_pass(|| Box::new(reference::DerefAddrOf)); store.register_early_pass(|| Box::new(reference::RefInDeref)); store.register_early_pass(|| Box::new(double_parens::DoubleParens)); - store.register_late_pass(|| Box::new(to_string_in_display::ToStringInDisplay::new())); + store.register_late_pass(|| Box::new(recursive_display_impl::RecursiveDisplayImpl::new())); store.register_early_pass(|| Box::new(unsafe_removed_from_name::UnsafeNameRemoval)); store.register_early_pass(|| Box::new(else_if_without_else::ElseIfWithoutElse)); store.register_early_pass(|| Box::new(int_plus_one::IntPlusOne)); diff --git a/clippy_lints/src/recursive_display_impl.rs b/clippy_lints/src/recursive_display_impl.rs new file mode 100644 index 000000000000..9b630fc3d84b --- /dev/null +++ b/clippy_lints/src/recursive_display_impl.rs @@ -0,0 +1,222 @@ +use clippy_utils::diagnostics::span_lint; +use clippy_utils::higher::{FormatArgsArg, FormatArgsExpn}; +use clippy_utils::{is_diag_trait_item, match_def_path, path_to_local_id, paths}; +use if_chain::if_chain; +use rustc_hir::{Expr, ExprKind, HirId, Impl, ImplItem, ImplItemKind, Item, ItemKind, UnOp}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::{sym, ExpnData, ExpnKind, Symbol}; + +const FORMAT_MACRO_PATHS: &[&[&str]] = &[ + &paths::FORMAT_ARGS_MACRO, + &paths::ASSERT_EQ_MACRO, + &paths::ASSERT_MACRO, + &paths::ASSERT_NE_MACRO, + &paths::EPRINT_MACRO, + &paths::EPRINTLN_MACRO, + &paths::PRINT_MACRO, + &paths::PRINTLN_MACRO, + &paths::WRITE_MACRO, + &paths::WRITELN_MACRO, +]; + +const FORMAT_MACRO_DIAG_ITEMS: &[Symbol] = &[sym::format_macro, sym::std_panic_macro]; + +fn outermost_expn_data(expn_data: ExpnData) -> ExpnData { + if expn_data.call_site.from_expansion() { + outermost_expn_data(expn_data.call_site.ctxt().outer_expn_data()) + } else { + expn_data + } +} + +declare_clippy_lint! { + /// ### What it does + /// Checks for recursive use of `Display` trait inside its implementation. + /// + /// ### Why is this bad? + /// This is unconditional recursion and so will lead to infinite + /// recursion and a stack overflow. + /// + /// ### Example + /// + /// ```rust + /// use std::fmt; + /// + /// struct Structure(i32); + /// impl fmt::Display for Structure { + /// fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + /// write!(f, "{}", self.to_string()) + /// } + /// } + /// + /// ``` + /// Use instead: + /// ```rust + /// use std::fmt; + /// + /// struct Structure(i32); + /// impl fmt::Display for Structure { + /// fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + /// write!(f, "{}", self.0) + /// } + /// } + /// ``` + #[clippy::version = "1.48.0"] + pub RECURSIVE_DISPLAY_IMPL, + correctness, + "`Display` trait method called while implementing `Display` trait" +} + +#[derive(Default)] +pub struct RecursiveDisplayImpl { + in_display_impl: bool, + // hir_id of self parameter of method inside Display Impl - i.e. fmt(&self) + self_hir_id: Option, +} + +impl RecursiveDisplayImpl { + pub fn new() -> Self { + Self { + in_display_impl: false, + self_hir_id: None, + } + } +} + +impl_lint_pass!(RecursiveDisplayImpl => [RECURSIVE_DISPLAY_IMPL]); + +impl LateLintPass<'_> for RecursiveDisplayImpl { + fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { + if is_display_impl(cx, item) { + self.in_display_impl = true; + } + } + + fn check_item_post(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { + if is_display_impl(cx, item) { + self.in_display_impl = false; + self.self_hir_id = None; + } + } + + fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) { + if_chain! { + // If we are in Display impl, then get hir_id for self in method impl - i.e. fmt(&self) + if self.in_display_impl; + if let ImplItemKind::Fn(.., body_id) = &impl_item.kind; + let body = cx.tcx.hir().body(*body_id); + if !body.params.is_empty(); + then { + let self_param = &body.params[0]; + self.self_hir_id = Some(self_param.pat.hir_id); + } + } + } + + fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { + if_chain! { + if self.in_display_impl; + if let Some(self_hir_id) = self.self_hir_id; + then { + check_to_string_in_display(cx, expr, self_hir_id); + check_self_in_format_args(cx, expr, self_hir_id); + } + } + } +} + +fn check_to_string_in_display(cx: &LateContext<'_>, expr: &Expr<'_>, self_hir_id: HirId) { + if_chain! { + // Get the hir_id of the object we are calling the method on + if let ExprKind::MethodCall(path, _, [ref self_arg, ..], _) = expr.kind; + // Is the method to_string() ? + if path.ident.name == sym!(to_string); + // Is the method a part of the ToString trait? (i.e. not to_string() implemented + // separately) + if let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id); + if is_diag_trait_item(cx, expr_def_id, sym::ToString); + // Is the method is called on self + if path_to_local_id(self_arg, self_hir_id); + then { + span_lint( + cx, + RECURSIVE_DISPLAY_IMPL, + expr.span, + "using `to_string` in `fmt::Display` implementation might lead to infinite recursion", + ); + } + } +} + +fn check_self_in_format_args(cx: &LateContext<'_>, expr: &Expr<'_>, self_hir_id: HirId) { + // Check each arg in format calls - do we ever use Display on self (directly or via deref)? + if_chain! { + if let Some(format_args) = FormatArgsExpn::parse(expr); + let expr_expn_data = expr.span.ctxt().outer_expn_data(); + let outermost_expn_data = outermost_expn_data(expr_expn_data); + if let Some(macro_def_id) = outermost_expn_data.macro_def_id; + if FORMAT_MACRO_PATHS + .iter() + .any(|path| match_def_path(cx, macro_def_id, path)) + || FORMAT_MACRO_DIAG_ITEMS + .iter() + .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, macro_def_id)); + if let ExpnKind::Macro(_, _name) = outermost_expn_data.kind; + if let Some(args) = format_args.args(); + then { + for (_i, arg) in args.iter().enumerate() { + // We only care about Display, okay to use Debug here + if !arg.is_display() { + continue; + } + check_format_arg_self(cx, expr, self_hir_id, arg); + } + } + } +} + +fn check_format_arg_self(cx: &LateContext<'_>, expr: &Expr<'_>, self_hir_id: HirId, arg: &FormatArgsArg<'_>) { + // Handle multiple dereferencing of references e.g. &&self + // Handle single dereference of &self -> self that is equivalent (i.e. via *self in fmt() impl) + // Since the argument to fmt is itself a reference: &self + let reference = single_deref(deref_expr(arg.value)); + if path_to_local_id(reference, self_hir_id) { + span_lint( + cx, + RECURSIVE_DISPLAY_IMPL, + expr.span, + "using `self` in `fmt::Display` implementation might lead to infinite recursion", + ); + } +} + +fn deref_expr<'a, 'b>(expr: &'a Expr<'b>) -> &'a Expr<'b> { + if let ExprKind::AddrOf(_, _, reference) = expr.kind { + deref_expr(reference) + } else { + expr + } +} + +fn single_deref<'a, 'b>(expr: &'a Expr<'b>) -> &'a Expr<'b> { + if let ExprKind::Unary(UnOp::Deref, ex) = expr.kind { + ex + } else { + expr + } +} + +fn is_display_impl(cx: &LateContext<'_>, item: &'hir Item<'_>) -> bool { + if_chain! { + // Are we at an Impl? + if let ItemKind::Impl(Impl { of_trait: Some(trait_ref), .. }) = &item.kind; + if let Some(did) = trait_ref.trait_def_id(); + then { + // Is it for Display trait? + match_def_path(cx, did, &paths::DISPLAY_TRAIT) + } else { + false + } + } +} diff --git a/clippy_lints/src/to_string_in_display.rs b/clippy_lints/src/to_string_in_display.rs deleted file mode 100644 index f8b6bdcd3e15..000000000000 --- a/clippy_lints/src/to_string_in_display.rs +++ /dev/null @@ -1,123 +0,0 @@ -use clippy_utils::diagnostics::span_lint; -use clippy_utils::{is_diag_trait_item, match_def_path, path_to_local_id, paths}; -use if_chain::if_chain; -use rustc_hir::{Expr, ExprKind, HirId, Impl, ImplItem, ImplItemKind, Item, ItemKind}; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::symbol::sym; - -declare_clippy_lint! { - /// ### What it does - /// Checks for uses of `to_string()` in `Display` traits. - /// - /// ### Why is this bad? - /// Usually `to_string` is implemented indirectly - /// via `Display`. Hence using it while implementing `Display` would - /// lead to infinite recursion. - /// - /// ### Example - /// - /// ```rust - /// use std::fmt; - /// - /// struct Structure(i32); - /// impl fmt::Display for Structure { - /// fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - /// write!(f, "{}", self.to_string()) - /// } - /// } - /// - /// ``` - /// Use instead: - /// ```rust - /// use std::fmt; - /// - /// struct Structure(i32); - /// impl fmt::Display for Structure { - /// fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - /// write!(f, "{}", self.0) - /// } - /// } - /// ``` - #[clippy::version = "1.48.0"] - pub TO_STRING_IN_DISPLAY, - correctness, - "`to_string` method used while implementing `Display` trait" -} - -#[derive(Default)] -pub struct ToStringInDisplay { - in_display_impl: bool, - self_hir_id: Option, -} - -impl ToStringInDisplay { - pub fn new() -> Self { - Self { - in_display_impl: false, - self_hir_id: None, - } - } -} - -impl_lint_pass!(ToStringInDisplay => [TO_STRING_IN_DISPLAY]); - -impl LateLintPass<'_> for ToStringInDisplay { - fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { - if is_display_impl(cx, item) { - self.in_display_impl = true; - } - } - - fn check_item_post(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { - if is_display_impl(cx, item) { - self.in_display_impl = false; - self.self_hir_id = None; - } - } - - fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &ImplItem<'_>) { - if_chain! { - if self.in_display_impl; - if let ImplItemKind::Fn(.., body_id) = &impl_item.kind; - let body = cx.tcx.hir().body(*body_id); - if !body.params.is_empty(); - then { - let self_param = &body.params[0]; - self.self_hir_id = Some(self_param.pat.hir_id); - } - } - } - - fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { - if_chain! { - if self.in_display_impl; - if let Some(self_hir_id) = self.self_hir_id; - if let ExprKind::MethodCall(path, _, [ref self_arg, ..], _) = expr.kind; - if path.ident.name == sym!(to_string); - if let Some(expr_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id); - if is_diag_trait_item(cx, expr_def_id, sym::ToString); - if path_to_local_id(self_arg, self_hir_id); - then { - span_lint( - cx, - TO_STRING_IN_DISPLAY, - expr.span, - "using `to_string` in `fmt::Display` implementation might lead to infinite recursion", - ); - } - } - } -} - -fn is_display_impl(cx: &LateContext<'_>, item: &Item<'_>) -> bool { - if_chain! { - if let ItemKind::Impl(Impl { of_trait: Some(trait_ref), .. }) = &item.kind; - if let Some(did) = trait_ref.trait_def_id(); - then { - match_def_path(cx, did, &paths::DISPLAY_TRAIT) - } else { - false - } - } -} diff --git a/tests/ui/recursive_display_impl.rs b/tests/ui/recursive_display_impl.rs new file mode 100644 index 000000000000..6cc7a7a24272 --- /dev/null +++ b/tests/ui/recursive_display_impl.rs @@ -0,0 +1,197 @@ +#![warn(clippy::recursive_display_impl)] +#![allow(clippy::inherent_to_string_shadow_display, clippy::to_string_in_format_args)] + +use std::fmt; + +struct A; +impl A { + fn fmt(&self) { + self.to_string(); + } +} + +trait B { + fn fmt(&self) {} +} + +impl B for A { + fn fmt(&self) { + self.to_string(); + } +} + +impl fmt::Display for A { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self.to_string()) + } +} + +fn fmt(a: A) { + a.to_string(); +} + +struct C; + +impl C { + // Doesn't trigger if to_string defined separately + // i.e. not using ToString trait (from Display) + fn to_string(&self) -> String { + String::from("I am C") + } +} + +impl fmt::Display for C { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self.to_string()) + } +} + +enum D { + E(String), + F, +} + +impl std::fmt::Display for D { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match &self { + Self::E(string) => write!(f, "E {}", string.to_string()), + Self::F => write!(f, "F"), + } + } +} + +// Check for use of self as Display, in Display impl +// Triggers on direct use of self +struct G {} + +impl std::fmt::Display for G { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self) + } +} + +// Triggers on reference to self +struct H {} + +impl std::fmt::Display for H { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", &self) + } +} + +// Triggers on multiple reference to self +struct H2 {} + +impl std::fmt::Display for H2 { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", &&&self) + } +} + +// Doesn't trigger on correct deref +struct I {} + +impl std::ops::Deref for I { + type Target = str; + + fn deref(&self) -> &Self::Target { + "test" + } +} + +impl std::fmt::Display for I { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", &**self) + } +} + +// Does trigger when deref resolves to self +struct J {} + +impl std::ops::Deref for J { + type Target = str; + + fn deref(&self) -> &Self::Target { + "test" + } +} + +impl std::fmt::Display for J { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", &*self) + } +} + +struct J2 {} + +impl std::ops::Deref for J2 { + type Target = str; + + fn deref(&self) -> &Self::Target { + "test" + } +} + +impl std::fmt::Display for J2 { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", *self) + } +} + +// Doesn't trigger on Debug +struct K {} + +impl std::fmt::Debug for K { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "test") + } +} + +impl std::fmt::Display for K { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{:?}", self) + } +} + +// Doesn't trigger on struct fields +struct L { + field1: u32, + field2: i32, +} + +impl std::fmt::Display for L { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{},{}", self.field1, self.field2) + } +} + +// Doesn't trigger on nested enum matching +enum Tree { + Leaf, + Node(Vec), +} + +impl std::fmt::Display for Tree { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Tree::Leaf => write!(f, "*"), + Tree::Node(children) => { + write!(f, "(")?; + for child in children.iter() { + write!(f, "{},", child)?; + } + write!(f, ")") + }, + } + } +} + +fn main() { + let a = A; + a.to_string(); + a.fmt(); + fmt(a); + + let c = C; + c.to_string(); +} diff --git a/tests/ui/recursive_display_impl.stderr b/tests/ui/recursive_display_impl.stderr new file mode 100644 index 000000000000..628a2002946d --- /dev/null +++ b/tests/ui/recursive_display_impl.stderr @@ -0,0 +1,50 @@ +error: using `to_string` in `fmt::Display` implementation might lead to infinite recursion + --> $DIR/recursive_display_impl.rs:25:25 + | +LL | write!(f, "{}", self.to_string()) + | ^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::recursive-display-impl` implied by `-D warnings` + +error: using `self` in `fmt::Display` implementation might lead to infinite recursion + --> $DIR/recursive_display_impl.rs:69:9 + | +LL | write!(f, "{}", self) + | ^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `$crate::format_args` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: using `self` in `fmt::Display` implementation might lead to infinite recursion + --> $DIR/recursive_display_impl.rs:78:9 + | +LL | write!(f, "{}", &self) + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `$crate::format_args` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: using `self` in `fmt::Display` implementation might lead to infinite recursion + --> $DIR/recursive_display_impl.rs:87:9 + | +LL | write!(f, "{}", &&&self) + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `$crate::format_args` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: using `self` in `fmt::Display` implementation might lead to infinite recursion + --> $DIR/recursive_display_impl.rs:121:9 + | +LL | write!(f, "{}", &*self) + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `$crate::format_args` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: using `self` in `fmt::Display` implementation might lead to infinite recursion + --> $DIR/recursive_display_impl.rs:137:9 + | +LL | write!(f, "{}", *self) + | ^^^^^^^^^^^^^^^^^^^^^^ + | + = note: this error originates in the macro `$crate::format_args` (in Nightly builds, run with -Z macro-backtrace for more info) + +error: aborting due to 6 previous errors + diff --git a/tests/ui/to_string_in_display.rs b/tests/ui/to_string_in_display.rs deleted file mode 100644 index 3ccdcd1117b5..000000000000 --- a/tests/ui/to_string_in_display.rs +++ /dev/null @@ -1,69 +0,0 @@ -#![warn(clippy::to_string_in_display)] -#![allow(clippy::inherent_to_string_shadow_display, clippy::to_string_in_format_args)] - -use std::fmt; - -struct A; -impl A { - fn fmt(&self) { - self.to_string(); - } -} - -trait B { - fn fmt(&self) {} -} - -impl B for A { - fn fmt(&self) { - self.to_string(); - } -} - -impl fmt::Display for A { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", self.to_string()) - } -} - -fn fmt(a: A) { - a.to_string(); -} - -struct C; - -impl C { - fn to_string(&self) -> String { - String::from("I am C") - } -} - -impl fmt::Display for C { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", self.to_string()) - } -} - -enum D { - E(String), - F, -} - -impl std::fmt::Display for D { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match &self { - Self::E(string) => write!(f, "E {}", string.to_string()), - Self::F => write!(f, "F"), - } - } -} - -fn main() { - let a = A; - a.to_string(); - a.fmt(); - fmt(a); - - let c = C; - c.to_string(); -} diff --git a/tests/ui/to_string_in_display.stderr b/tests/ui/to_string_in_display.stderr deleted file mode 100644 index 5f26ef413e23..000000000000 --- a/tests/ui/to_string_in_display.stderr +++ /dev/null @@ -1,10 +0,0 @@ -error: using `to_string` in `fmt::Display` implementation might lead to infinite recursion - --> $DIR/to_string_in_display.rs:25:25 - | -LL | write!(f, "{}", self.to_string()) - | ^^^^^^^^^^^^^^^^ - | - = note: `-D clippy::to-string-in-display` implied by `-D warnings` - -error: aborting due to previous error -