From 04bff17668be1305d9efe235665a32727ff3e0b5 Mon Sep 17 00:00:00 2001 From: Takayuki Nakata Date: Thu, 27 Aug 2020 23:37:47 +0900 Subject: [PATCH] Fix FP in `to_string_in_display` Don't emit a lint when `.to_string()` on anything that is not `self` --- clippy_lints/src/to_string_in_display.rs | 32 ++++++++++++++++++++---- tests/ui/to_string_in_display.rs | 14 +++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/to_string_in_display.rs b/clippy_lints/src/to_string_in_display.rs index 4b6a0a6a0c9..006d7a3a12d 100644 --- a/clippy_lints/src/to_string_in_display.rs +++ b/clippy_lints/src/to_string_in_display.rs @@ -1,6 +1,7 @@ -use crate::utils::{match_def_path, match_trait_method, paths, span_lint}; +use crate::utils::{match_def_path, match_trait_method, paths, qpath_res, span_lint}; use if_chain::if_chain; -use rustc_hir::{Expr, ExprKind, Item, ItemKind}; +use rustc_hir::def::Res; +use rustc_hir::{Expr, ExprKind, HirId, ImplItem, ImplItemKind, Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; @@ -45,11 +46,15 @@ declare_clippy_lint! { #[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 { + in_display_impl: false, + self_hir_id: None, + } } } @@ -65,16 +70,33 @@ impl LateLintPass<'_> for ToStringInDisplay { 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 let ExprKind::MethodCall(ref path, _, _, _) = expr.kind; + if let ExprKind::MethodCall(ref path, _, args, _) = expr.kind; if path.ident.name == sym!(to_string); if match_trait_method(cx, expr, &paths::TO_STRING); if self.in_display_impl; - + if let ExprKind::Path(ref qpath) = args[0].kind; + if let Res::Local(hir_id) = qpath_res(cx, qpath, args[0].hir_id); + if let Some(self_hir_id) = self.self_hir_id; + if hir_id == self_hir_id; then { span_lint( cx, diff --git a/tests/ui/to_string_in_display.rs b/tests/ui/to_string_in_display.rs index 3b46324704e..eb8105c6b6d 100644 --- a/tests/ui/to_string_in_display.rs +++ b/tests/ui/to_string_in_display.rs @@ -44,6 +44,20 @@ impl fmt::Display for C { } } +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();