Auto merge of #9148 - arieluy:then_some_unwrap_or, r=Jarcho

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

diff --git a/clippy_lints/src/redundant_pub_crate.rs b/clippy_lints/src/redundant_pub_crate.rs
index 05f9e01acb44b45154f9c103a07b4821bd830d76..323326381d4079149ce2591abd3c6a402b4e453b 100644 (file)
--- a/clippy_lints/src/redundant_pub_crate.rs
+++ b/clippy_lints/src/redundant_pub_crate.rs
@@ -1,20 +1,23 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use rustc_errors::Applicability;
-use rustc_hir::{Item, ItemKind, VisibilityKind};
+use rustc_hir::def::{DefKind, Res};
+use rustc_hir::{Item, ItemKind};
 use rustc_lint::{LateContext, LateLintPass};
+use rustc_middle::ty;
 use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::def_id::CRATE_DEF_ID;
+use rustc_span::hygiene::MacroKind;
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for items declared `pub(crate)` that are not crate visible because they
+    /// ### What it does
+    /// Checks for items declared `pub(crate)` that are not crate visible because they
     /// are inside a private module.
     ///
-    /// **Why is this bad?** Writing `pub(crate)` is misleading when it's redundant due to the parent
+    /// ### Why is this bad?
+    /// Writing `pub(crate)` is misleading when it's redundant due to the parent
     /// module's visibility.
     ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
-    ///
+    /// ### Example
     /// ```rust
     /// mod internal {
     ///     pub(crate) fn internal_fn() { }
@@ -27,6 +30,7 @@
     ///     pub fn internal_fn() { }
     /// }
     /// ```
+    #[clippy::version = "1.44.0"]
     pub REDUNDANT_PUB_CRATE,
     nursery,
     "Using `pub(crate)` visibility on items that are not crate visible due to the visibility of the module that contains them."
@@ -41,31 +45,32 @@ pub struct RedundantPubCrate {
 
 impl<'tcx> LateLintPass<'tcx> for RedundantPubCrate {
     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
-        if let VisibilityKind::Crate { .. } = item.vis.node {
-            if !cx.access_levels.is_exported(item.hir_id()) {
-                if let Some(false) = self.is_exported.last() {
-                    let span = item.span.with_hi(item.ident.span.hi());
-                    let descr = cx.tcx.def_kind(item.def_id).descr(item.def_id.to_def_id());
-                    span_lint_and_then(
-                        cx,
-                        REDUNDANT_PUB_CRATE,
-                        span,
-                        &format!("pub(crate) {} inside private module", descr),
-                        |diag| {
-                            diag.span_suggestion(
-                                item.vis.span,
-                                "consider using",
-                                "pub".to_string(),
-                                Applicability::MachineApplicable,
-                            );
-                        },
-                    );
-                }
+        if_chain! {
+            if cx.tcx.visibility(item.def_id) == ty::Visibility::Restricted(CRATE_DEF_ID.to_def_id());
+            if !cx.access_levels.is_exported(item.def_id) && self.is_exported.last() == Some(&false);
+            if is_not_macro_export(item);
+            then {
+                let span = item.span.with_hi(item.ident.span.hi());
+                let descr = cx.tcx.def_kind(item.def_id).descr(item.def_id.to_def_id());
+                span_lint_and_then(
+                    cx,
+                    REDUNDANT_PUB_CRATE,
+                    span,
+                    &format!("pub(crate) {} inside private module", descr),
+                    |diag| {
+                        diag.span_suggestion(
+                            item.vis_span,
+                            "consider using",
+                            "pub".to_string(),
+                            Applicability::MachineApplicable,
+                        );
+                    },
+                );
             }
         }
 
         if let ItemKind::Mod { .. } = item.kind {
-            self.is_exported.push(cx.access_levels.is_exported(item.hir_id()));
+            self.is_exported.push(cx.access_levels.is_exported(item.def_id));
         }
     }
 
@@ -75,3 +80,15 @@ fn check_item_post(&mut self, _cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
         }
     }
 }
+
+fn is_not_macro_export<'tcx>(item: &'tcx Item<'tcx>) -> bool {
+    if let ItemKind::Use(path, _) = item.kind {
+        if let Res::Def(DefKind::Macro(MacroKind::Bang), _) = path.res {
+            return false;
+        }
+    } else if let ItemKind::Macro(..) = item.kind {
+        return false;
+    }
+
+    true
+}