Rollup merge of #83092 - petrochenkov:qspan, r=estebank

[rust.git] / clippy_lints / src / unused_self.rs

diff --git a/clippy_lints/src/unused_self.rs b/clippy_lints/src/unused_self.rs
index 5615dd37cd2e89b3803020d8416745705419893c..812482cf5cfb6638bdbb7eeaa18461c6937a8467 100644 (file)
--- a/clippy_lints/src/unused_self.rs
+++ b/clippy_lints/src/unused_self.rs
@@ -1,16 +1,15 @@
 use if_chain::if_chain;
-use rustc::hir::def::Res;
-use rustc::hir::intravisit::{walk_path, NestedVisitorMap, Visitor};
-use rustc::hir::{AssocItemKind, HirId, ImplItemKind, ImplItemRef, Item, ItemKind, Path};
-use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
-use rustc::{declare_lint_pass, declare_tool_lint};
+use rustc_hir::{Impl, ImplItem, ImplItemKind, ItemKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
 
-use crate::utils::span_help_and_lint;
+use crate::utils::span_lint_and_help;
+use crate::utils::visitors::LocalUsedVisitor;
 
 declare_clippy_lint! {
     /// **What it does:** Checks methods that contain a `self` argument but don't use it
     ///
-    /// **Why is this bad?** It may be clearer to define the method as a static function instead
+    /// **Why is this bad?** It may be clearer to define the method as an associated function instead
     /// of an instance method if it doesn't require `self`.
     ///
     /// **Known problems:** None.
@@ -32,71 +31,41 @@
     /// }
     /// ```
     pub UNUSED_SELF,
-    complexity,
+    pedantic,
     "methods that contain a `self` argument but don't use it"
 }
 
 declare_lint_pass!(UnusedSelf => [UNUSED_SELF]);
 
-impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedSelf {
-    fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &Item) {
-        if item.span.from_expansion() {
+impl<'tcx> LateLintPass<'tcx> for UnusedSelf {
+    fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &ImplItem<'_>) {
+        if impl_item.span.from_expansion() {
             return;
         }
-        if let ItemKind::Impl(_, _, _, _, None, _, ref 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;
-                    let impl_item = cx.tcx.hir().impl_item(impl_item_ref.id);
-                    if let ImplItemKind::Method(_, body_id) = &impl_item.kind;
-                    then {
-                        let body = cx.tcx.hir().body(*body_id);
-                        let self_param = &body.params[0];
-                        let self_hir_id = self_param.pat.hir_id;
-                        let visitor = &mut UnusedSelfVisitor {
-                            cx,
-                            uses_self: false,
-                            self_hir_id: &self_hir_id,
-                        };
-                        visitor.visit_body(body);
-                        if !visitor.uses_self {
-                            span_help_and_lint(
-                                cx,
-                                UNUSED_SELF,
-                                self_param.span,
-                                "unused `self` argument",
-                                "consider refactoring to a static method or function",
-                            )
-                        }
-                    }
+        let parent = cx.tcx.hir().get_parent_item(impl_item.hir_id());
+        let parent_item = cx.tcx.hir().expect_item(parent);
+        let assoc_item = cx.tcx.associated_item(impl_item.def_id);
+        if_chain! {
+            if let ItemKind::Impl(Impl { of_trait: None, .. }) = parent_item.kind;
+            if assoc_item.fn_has_self_parameter;
+            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;
+                if !LocalUsedVisitor::new(cx, self_hir_id).check_body(body) {
+                    span_lint_and_help(
+                        cx,
+                        UNUSED_SELF,
+                        self_param.span,
+                        "unused `self` argument",
+                        None,
+                        "consider refactoring to a associated function",
+                    );
+                    return;
                 }
             }
-        };
-    }
-}
-
-struct UnusedSelfVisitor<'a, 'tcx> {
-    cx: &'a LateContext<'a, 'tcx>,
-    uses_self: bool,
-    self_hir_id: &'a HirId,
-}
-
-impl<'a, 'tcx> Visitor<'tcx> for UnusedSelfVisitor<'a, 'tcx> {
-    fn visit_path(&mut self, path: &'tcx Path, _id: HirId) {
-        if self.uses_self {
-            // This function already uses `self`
-            return;
-        }
-        if let Res::Local(hir_id) = &path.res {
-            self.uses_self = self.self_hir_id == hir_id
         }
-        walk_path(self, path);
-    }
-
-    fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
-        NestedVisitorMap::All(&self.cx.tcx.hir())
     }
 }