about summary refs log tree commit diff
diff options
context:
space:
mode:
authorCameron Steffen <cam.steffen94@gmail.com>2021-06-17 11:19:33 -0500
committerCameron Steffen <cam.steffen94@gmail.com>2021-06-29 08:14:21 -0500
commit8827e96303a15f9accc7c02ae2a2bd42573b2739 (patch)
treec18407c963bf968ef51c6a823be8de45f43c43d7
parente405c68b3c1265daa9a091ed9b4b5c5a38c0c0ba (diff)
downloadrust-8827e96303a15f9accc7c02ae2a2bd42573b2739.tar.gz
rust-8827e96303a15f9accc7c02ae2a2bd42573b2739.zip
Make ItemKind check dry
-rw-r--r--clippy_lints/src/use_self.rs79
1 files changed, 35 insertions, 44 deletions
diff --git a/clippy_lints/src/use_self.rs b/clippy_lints/src/use_self.rs
index f71dfd02499..9cac3b20ac5 100644
--- a/clippy_lints/src/use_self.rs
+++ b/clippy_lints/src/use_self.rs
@@ -87,59 +87,42 @@ const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element";
 
 impl<'tcx> LateLintPass<'tcx> for UseSelf {
     fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
+        if !is_item_interesting(item) {
+            // This does two things:
+            //  1) Reduce needless churn on `self.stack`
+            //  2) Don't push `StackItem::NoCheck` when entering `ItemKind::OpaqueTy`,
+            //     in order to lint `foo() -> impl <..>`
+            return;
+        }
         // We push the self types of `impl`s on a stack here. Only the top type on the stack is
         // relevant for linting, since this is the self type of the `impl` we're currently in. To
         // avoid linting on nested items, we push `StackItem::NoCheck` on the stack to signal, that
         // we're in an `impl` or nested item, that we don't want to lint
-        //
-        // NB: If you push something on the stack in this method, remember to also pop it in the
-        // `check_item_post` method.
-        match &item.kind {
-            ItemKind::Impl(Impl {
-                self_ty: hir_self_ty,
-                of_trait,
-                ..
-            }) => {
-                let should_check = if let TyKind::Path(QPath::Resolved(_, item_path)) = hir_self_ty.kind {
-                    let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).args;
-                    parameters.as_ref().map_or(true, |params| {
-                        !params.parenthesized && !params.args.iter().any(|arg| matches!(arg, GenericArg::Lifetime(_)))
-                    })
-                } else {
-                    false
-                };
+        let stack_item = if_chain! {
+            if let ItemKind::Impl(Impl { self_ty, ref of_trait, .. }) = item.kind;
+            if let TyKind::Path(QPath::Resolved(_, item_path)) = self_ty.kind;
+            let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).args;
+            if parameters.as_ref().map_or(true, |params| {
+                !params.parenthesized && !params.args.iter().any(|arg| matches!(arg, GenericArg::Lifetime(_)))
+            });
+            then {
                 let impl_trait_ref_def_id = of_trait.as_ref().map(|_| cx.tcx.hir().local_def_id(item.hir_id()));
-                if should_check {
-                    self.stack.push(StackItem::Check {
-                        hir_id: hir_self_ty.hir_id,
-                        impl_trait_ref_def_id,
-                        types_to_lint: Vec::new(),
-                        types_to_skip: Vec::new(),
-                    });
-                } else {
-                    self.stack.push(StackItem::NoCheck);
+                StackItem::Check {
+                    hir_id: self_ty.hir_id,
+                    impl_trait_ref_def_id,
+                    types_to_lint: Vec::new(),
+                    types_to_skip: Vec::new(),
                 }
-            },
-            ItemKind::Static(..)
-            | ItemKind::Const(..)
-            | ItemKind::Fn(..)
-            | ItemKind::Enum(..)
-            | ItemKind::Struct(..)
-            | ItemKind::Union(..)
-            | ItemKind::Trait(..) => {
-                self.stack.push(StackItem::NoCheck);
-            },
-            _ => (),
-        }
+            } else {
+                StackItem::NoCheck
+            }
+        };
+        self.stack.push(stack_item);
     }
 
     fn check_item_post(&mut self, _: &LateContext<'_>, item: &Item<'_>) {
-        use ItemKind::{Const, Enum, Fn, Impl, Static, Struct, Trait, Union};
-        match item.kind {
-            Impl { .. } | Static(..) | Const(..) | Fn(..) | Enum(..) | Struct(..) | Union(..) | Trait(..) => {
-                self.stack.pop();
-            },
-            _ => (),
+        if is_item_interesting(item) {
+            self.stack.pop();
         }
     }
 
@@ -359,6 +342,14 @@ fn lint_path_to_variant(cx: &LateContext<'_>, path: &Path<'_>) {
     }
 }
 
+fn is_item_interesting(item: &Item<'_>) -> bool {
+    use rustc_hir::ItemKind::{Const, Enum, Fn, Impl, Static, Struct, Trait, Union};
+    matches!(
+        item.kind,
+        Impl { .. } | Static(..) | Const(..) | Fn(..) | Enum(..) | Struct(..) | Union(..) | Trait(..)
+    )
+}
+
 fn ty_from_hir_id<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Ty<'tcx> {
     if let Some(Node::Ty(hir_ty)) = cx.tcx.hir().find(hir_id) {
         hir_ty_to_ty(cx.tcx, hir_ty)