Auto merge of rust-lang#115993 - bvanjoi:fix-115966, r=petrochenkov

vis note for no pub reexports glob import

Fixes rust-lang#115966

Only trigger the `unused_import` lint when it's not being used.

r? `@petrochenkov`

Author: bors

compiler/rustc_lint/src/context.rs

@@ -926,6 +926,10 @@ pub trait LintContext {
                         if elided { "'static " } else { "'static" },
                         Applicability::MachineApplicable
                     );
+                },
+                BuiltinLintDiagnostics::RedundantImportVisibility { max_vis, span } => {
+                    db.span_note(span, format!("the most public imported item is `{max_vis}`"));
+                    db.help("reduce the glob import's visibility or increase visibility of imported items");
                 }
             }
             // Rewrap `db`, and pass control to the user.

compiler/rustc_lint_defs/src/lib.rs

@@ -583,6 +583,10 @@ pub enum BuiltinLintDiagnostics {
         elided: bool,
         span: Span,
     },
+    RedundantImportVisibility {
+        span: Span,
+        max_vis: String,
+    },
 }
 
 /// Lints that are buffered up early on in the `Session` before the

compiler/rustc_middle/src/ty/mod.rs

@@ -296,6 +296,23 @@ pub enum Visibility<Id = LocalDefId> {
     Restricted(Id),
 }
 
+impl Visibility {
+    pub fn to_string(self, def_id: LocalDefId, tcx: TyCtxt<'_>) -> String {
+        match self {
+            ty::Visibility::Restricted(restricted_id) => {
+                if restricted_id.is_top_level_module() {
+                    "pub(crate)".to_string()
+                } else if restricted_id == tcx.parent_module_from_def_id(def_id).to_local_def_id() {
+                    "pub(self)".to_string()
+                } else {
+                    format!("pub({})", tcx.item_name(restricted_id.to_def_id()))
+                }
+            }
+            ty::Visibility::Public => "pub".to_string(),
+        }
+    }
+}
+
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable, TyEncodable, TyDecodable)]
 pub enum BoundConstness {
     /// `T: Trait`

compiler/rustc_privacy/src/lib.rs

@@ -888,29 +888,14 @@ pub struct TestReachabilityVisitor<'tcx, 'a> {
     effective_visibilities: &'a EffectiveVisibilities,
 }
 
-fn vis_to_string<'tcx>(def_id: LocalDefId, vis: ty::Visibility, tcx: TyCtxt<'tcx>) -> String {
-    match vis {
-        ty::Visibility::Restricted(restricted_id) => {
-            if restricted_id.is_top_level_module() {
-                "pub(crate)".to_string()
-            } else if restricted_id == tcx.parent_module_from_def_id(def_id).to_local_def_id() {
-                "pub(self)".to_string()
-            } else {
-                format!("pub({})", tcx.item_name(restricted_id.to_def_id()))
-            }
-        }
-        ty::Visibility::Public => "pub".to_string(),
-    }
-}
-
 impl<'tcx, 'a> TestReachabilityVisitor<'tcx, 'a> {
     fn effective_visibility_diagnostic(&mut self, def_id: LocalDefId) {
         if self.tcx.has_attr(def_id, sym::rustc_effective_visibility) {
             let mut error_msg = String::new();
             let span = self.tcx.def_span(def_id.to_def_id());
             if let Some(effective_vis) = self.effective_visibilities.effective_vis(def_id) {
                 for level in Level::all_levels() {
-                    let vis_str = vis_to_string(def_id, *effective_vis.at_level(level), self.tcx);
+                    let vis_str = effective_vis.at_level(level).to_string(def_id, self.tcx);
                     if level != Level::Direct {
                         error_msg.push_str(", ");
                     }
@@ -1506,11 +1491,11 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> {
                         tcx: self.tcx,
                     })
                         .into(),
-                    item_vis_descr: &vis_to_string(self.item_def_id, reachable_at_vis, self.tcx),
+                    item_vis_descr: &reachable_at_vis.to_string(self.item_def_id, self.tcx),
                     ty_span: vis_span,
                     ty_kind: kind,
                     ty_descr: descr.into(),
-                    ty_vis_descr: &vis_to_string(local_def_id, vis, self.tcx),
+                    ty_vis_descr: &vis.to_string(local_def_id, self.tcx),
                 },
             );
         }
@@ -1589,8 +1574,8 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx, '_> {
                     span,
                     kind: self.tcx.def_descr(def_id.to_def_id()),
                     descr: (&LazyDefPathStr { def_id: def_id.to_def_id(), tcx: self.tcx }).into(),
-                    reachable_vis: &vis_to_string(def_id, *reachable_at_vis, self.tcx),
-                    reexported_vis: &vis_to_string(def_id, *reexported_at_vis, self.tcx),
+                    reachable_vis: &reachable_at_vis.to_string(def_id, self.tcx),
+                    reexported_vis: &reexported_at_vis.to_string(def_id, self.tcx),
                 },
             );
         }

compiler/rustc_resolve/messages.ftl

@@ -127,8 +127,6 @@ resolve_generic_params_from_outer_item_self_ty_param = can't use `Self` here
 
 resolve_generic_params_from_outer_item_ty_param = type parameter from outer item
 
-resolve_glob_import_doesnt_reexport =
-    glob import doesn't reexport anything because no candidate is public enough
 
 resolve_ident_bound_more_than_once_in_parameter_list =
     identifier `{$identifier}` is bound more than once in this parameter list

compiler/rustc_resolve/src/imports.rs

@@ -8,7 +8,7 @@ use crate::errors::{
     ItemsInTraitsAreNotImportable,
 };
 use crate::Determinacy::{self, *};
-use crate::{fluent_generated as fluent, Namespace::*};
+use crate::Namespace::*;
 use crate::{module_to_string, names_to_string, ImportSuggestion};
 use crate::{AmbiguityKind, BindingKey, ModuleKind, ResolutionError, Resolver, Segment};
 use crate::{Finalize, Module, ModuleOrUniformRoot, ParentScope, PerNS, ScopeSet};
@@ -987,13 +987,23 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                     }
                     if !is_prelude
                         && let Some(max_vis) = max_vis.get()
-                        && !max_vis.is_at_least(import.expect_vis(), self.tcx)
+                        && let import_vis = import.expect_vis()
+                        && !max_vis.is_at_least(import_vis, self.tcx)
                     {
-                        self.lint_buffer.buffer_lint(
+                        let def_id = self.local_def_id(id);
+                        let msg = format!(
+                            "glob import doesn't reexport anything with visibility `{}` because no imported item is public enough",
+                            import_vis.to_string(def_id, self.tcx)
+                        );
+                        self.lint_buffer.buffer_lint_with_diagnostic(
                             UNUSED_IMPORTS,
                             id,
                             import.span,
-                            fluent::resolve_glob_import_doesnt_reexport,
+                            msg.to_string(),
+                            BuiltinLintDiagnostics::RedundantImportVisibility {
+                                max_vis: max_vis.to_string(def_id, self.tcx),
+                                span: import.span,
+                            },
                         );
                     }
                     return None;

tests/ui/imports/no-pub-reexports-but-used.rs

@@ -0,0 +1,15 @@
+// check-pass
+// https://github.com/rust-lang/rust/issues/115966
+
+mod m {
+    pub(crate) type A = u8;
+}
+
+#[warn(unused_imports)] //~ NOTE: the lint level is defined here
+pub use m::*;
+//~^ WARNING: glob import doesn't reexport anything with visibility `pub` because no imported item is public enough
+//~| NOTE: the most public imported item is `pub(crate)`
+
+fn main() {
+    let _: A;
+}

tests/ui/imports/no-pub-reexports-but-used.stderr

@@ -0,0 +1,20 @@
+warning: glob import doesn't reexport anything with visibility `pub` because no imported item is public enough
+  --> $DIR/no-pub-reexports-but-used.rs:9:9
+   |
+LL | pub use m::*;
+   |         ^^^^
+   |
+note: the most public imported item is `pub(crate)`
+  --> $DIR/no-pub-reexports-but-used.rs:9:9
+   |
+LL | pub use m::*;
+   |         ^^^^
+   = help: reduce the glob import's visibility or increase visibility of imported items
+note: the lint level is defined here
+  --> $DIR/no-pub-reexports-but-used.rs:8:8
+   |
+LL | #[warn(unused_imports)]
+   |        ^^^^^^^^^^^^^^
+
+warning: 1 warning emitted
+

tests/ui/imports/reexports.rs

@@ -9,7 +9,7 @@ mod a {
         //~^ ERROR cannot be re-exported
         //~| WARNING unused import: `super::foo`
         pub use super::*;
-        //~^ WARNING glob import doesn't reexport anything because no candidate is public enough
+        //~^ WARNING glob import doesn't reexport anything with visibility `pub` because no imported item is public enough
         //~| WARNING unused import: `super::*`
     }
 }

tests/ui/imports/reexports.stderr

@@ -56,11 +56,18 @@ note: the lint level is defined here
 LL | #![warn(unused_imports)]
    |         ^^^^^^^^^^^^^^
 
-warning: glob import doesn't reexport anything because no candidate is public enough
+warning: glob import doesn't reexport anything with visibility `pub` because no imported item is public enough
   --> $DIR/reexports.rs:11:17
    |
 LL |         pub use super::*;
    |                 ^^^^^^^^
+   |
+note: the most public imported item is `pub(a)`
+  --> $DIR/reexports.rs:11:17
+   |
+LL |         pub use super::*;
+   |                 ^^^^^^^^
+   = help: reduce the glob import's visibility or increase visibility of imported items
 
 warning: unused import: `super::*`
   --> $DIR/reexports.rs:11:17

tests/ui/privacy/issue-46209-private-enum-variant-reexport.stderr

@@ -22,12 +22,18 @@ note: consider marking `Full` as `pub` in the imported module
 LL |     pub use self::Lieutenant::{JuniorGrade, Full};
    |                                             ^^^^
 
-error: glob import doesn't reexport anything because no candidate is public enough
+error: glob import doesn't reexport anything with visibility `pub` because no imported item is public enough
   --> $DIR/issue-46209-private-enum-variant-reexport.rs:3:13
    |
 LL |     pub use self::Professor::*;
    |             ^^^^^^^^^^^^^^^^^^
    |
+note: the most public imported item is `pub(self)`
+  --> $DIR/issue-46209-private-enum-variant-reexport.rs:3:13
+   |
+LL |     pub use self::Professor::*;
+   |             ^^^^^^^^^^^^^^^^^^
+   = help: reduce the glob import's visibility or increase visibility of imported items
 note: the lint level is defined here
   --> $DIR/issue-46209-private-enum-variant-reexport.rs:1:8
    |
@@ -46,23 +52,37 @@ error: unused imports: `Full`, `JuniorGrade`
 LL |     pub use self::Lieutenant::{JuniorGrade, Full};
    |                                ^^^^^^^^^^^  ^^^^
 
-error: glob import doesn't reexport anything because no candidate is public enough
+error: glob import doesn't reexport anything with visibility `pub` because no imported item is public enough
+  --> $DIR/issue-46209-private-enum-variant-reexport.rs:10:13
+   |
+LL |     pub use self::PettyOfficer::*;
+   |             ^^^^^^^^^^^^^^^^^^^^^
+   |
+note: the most public imported item is `pub(self)`
   --> $DIR/issue-46209-private-enum-variant-reexport.rs:10:13
    |
 LL |     pub use self::PettyOfficer::*;
    |             ^^^^^^^^^^^^^^^^^^^^^
+   = help: reduce the glob import's visibility or increase visibility of imported items
 
 error: unused import: `self::PettyOfficer::*`
   --> $DIR/issue-46209-private-enum-variant-reexport.rs:10:13
    |
 LL |     pub use self::PettyOfficer::*;
    |             ^^^^^^^^^^^^^^^^^^^^^
 
-error: glob import doesn't reexport anything because no candidate is public enough
+error: glob import doesn't reexport anything with visibility `pub` because no imported item is public enough
+  --> $DIR/issue-46209-private-enum-variant-reexport.rs:13:13
+   |
+LL |     pub use self::Crewman::*;
+   |             ^^^^^^^^^^^^^^^^
+   |
+note: the most public imported item is `pub(crate)`
   --> $DIR/issue-46209-private-enum-variant-reexport.rs:13:13
    |
 LL |     pub use self::Crewman::*;
    |             ^^^^^^^^^^^^^^^^
+   = help: reduce the glob import's visibility or increase visibility of imported items
 
 error: unused import: `self::Crewman::*`
   --> $DIR/issue-46209-private-enum-variant-reexport.rs:13:13

tests/ui/privacy/private-variant-reexport.stderr

@@ -30,12 +30,18 @@ LL |     pub use ::E::V::{self};
    |
    = note: consider declaring type or module `V` with `pub`
 
-error: glob import doesn't reexport anything because no candidate is public enough
+error: glob import doesn't reexport anything with visibility `pub` because no imported item is public enough
   --> $DIR/private-variant-reexport.rs:15:13
    |
 LL |     pub use ::E::*;
    |             ^^^^^^
    |
+note: the most public imported item is `pub(crate)`
+  --> $DIR/private-variant-reexport.rs:15:13
+   |
+LL |     pub use ::E::*;
+   |             ^^^^^^
+   = help: reduce the glob import's visibility or increase visibility of imported items
 note: the lint level is defined here
   --> $DIR/private-variant-reexport.rs:13:8
    |