From 82f929cbaf66d8124473d04b408847dca2c94cb0 Mon Sep 17 00:00:00 2001 From: Jacek Pospychala Date: Sun, 29 Mar 2020 22:22:28 +0200 Subject: [PATCH] `unused_self` false positive --- clippy_lints/src/unused_self.rs | 67 ++++++++++++++------------------- tests/ui/unused_self.rs | 11 ++++++ 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/clippy_lints/src/unused_self.rs b/clippy_lints/src/unused_self.rs index a45b4a6869c5..bca73a377cad 100644 --- a/clippy_lints/src/unused_self.rs +++ b/clippy_lints/src/unused_self.rs @@ -2,7 +2,7 @@ use if_chain::if_chain; use rustc::hir::map::Map; use rustc_hir::def::Res; use rustc_hir::intravisit::{walk_path, NestedVisitorMap, Visitor}; -use rustc_hir::{AssocItemKind, HirId, ImplItem, ImplItemKind, ImplItemRef, ItemKind, Path}; +use rustc_hir::{HirId, ImplItem, ImplItemKind, ItemKind, Path}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -45,45 +45,36 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedSelf { return; } let parent = cx.tcx.hir().get_parent_item(impl_item.hir_id); - let item = cx.tcx.hir().expect_item(parent); - if let ItemKind::Impl { - of_trait: None, - items: impl_item_refs, - .. - } = item.kind - { - for impl_item_ref in impl_item_refs { - if_chain! { - if let ImplItemRef { - kind: AssocItemKind::Method { has_self: true }, - .. - } = impl_item_ref; - 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]; - let self_hir_id = self_param.pat.hir_id; - let mut visitor = UnusedSelfVisitor { - cx, - uses_self: false, - self_hir_id: &self_hir_id, - }; - visitor.visit_body(body); - if !visitor.uses_self { - span_lint_and_help( - cx, - UNUSED_SELF, - self_param.span, - "unused `self` argument", - "consider refactoring to a associated function", - ); - return; - } - } + let parent_item = cx.tcx.hir().expect_item(parent); + let def_id = cx.tcx.hir().local_def_id(impl_item.hir_id); + let assoc_item = cx.tcx.associated_item(def_id); + if_chain! { + if let ItemKind::Impl { of_trait: None, .. } = parent_item.kind; + if assoc_item.method_has_self_argument; + 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]; + let self_hir_id = self_param.pat.hir_id; + let mut visitor = UnusedSelfVisitor { + cx, + uses_self: false, + self_hir_id: &self_hir_id, + }; + visitor.visit_body(body); + if !visitor.uses_self { + span_lint_and_help( + cx, + UNUSED_SELF, + self_param.span, + "unused `self` argument", + "consider refactoring to a associated function", + ); + return; } } - }; + } } } diff --git a/tests/ui/unused_self.rs b/tests/ui/unused_self.rs index 42d432a2c9e2..7a4bbdda1ab2 100644 --- a/tests/ui/unused_self.rs +++ b/tests/ui/unused_self.rs @@ -42,6 +42,17 @@ mod unused_self_allow { impl B { fn unused_self_move(self) {} } + + struct C {} + + #[allow(clippy::unused_self)] + impl C { + #[warn(clippy::unused_self)] + fn some_fn((): ()) {} + + // shouldn't trigger + fn unused_self_move(self) {} + } } mod used_self {