From ee5551be8597d376b938240a374b8a106d09a17f Mon Sep 17 00:00:00 2001 From: bohan Date: Thu, 30 Nov 2023 22:15:48 +0800 Subject: [PATCH] vis note for no pub reexports glob import --- compiler/rustc_lint/src/context.rs | 4 +++ compiler/rustc_lint_defs/src/lib.rs | 4 +++ compiler/rustc_middle/src/ty/mod.rs | 17 +++++++++++ compiler/rustc_privacy/src/lib.rs | 25 ++++------------ compiler/rustc_resolve/messages.ftl | 3 -- compiler/rustc_resolve/src/imports.rs | 30 ++++++++++++++++--- tests/ui/imports/no-pub-reexports-but-used.rs | 15 ++++++++++ .../imports/no-pub-reexports-but-used.stderr | 20 +++++++++++++ tests/ui/imports/reexports.rs | 2 +- tests/ui/imports/reexports.stderr | 9 +++++- ...46209-private-enum-variant-reexport.stderr | 26 ++++++++++++++-- .../privacy/private-variant-reexport.stderr | 8 ++++- 12 files changed, 130 insertions(+), 33 deletions(-) create mode 100644 tests/ui/imports/no-pub-reexports-but-used.rs create mode 100644 tests/ui/imports/no-pub-reexports-but-used.stderr diff --git a/compiler/rustc_lint/src/context.rs b/compiler/rustc_lint/src/context.rs index 9bc9f88d3f72b..024e542d4afe4 100644 --- a/compiler/rustc_lint/src/context.rs +++ b/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. diff --git a/compiler/rustc_lint_defs/src/lib.rs b/compiler/rustc_lint_defs/src/lib.rs index 878c1a65dbf6e..de0abe04611d7 100644 --- a/compiler/rustc_lint_defs/src/lib.rs +++ b/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 diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 24cba913bb891..9feda4d205e9c 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -296,6 +296,23 @@ pub enum Visibility { 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` diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index b8109d5bb0694..95f631d2bca7c 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -888,21 +888,6 @@ 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) { @@ -910,7 +895,7 @@ impl<'tcx, 'a> TestReachabilityVisitor<'tcx, 'a> { 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), }, ); } diff --git a/compiler/rustc_resolve/messages.ftl b/compiler/rustc_resolve/messages.ftl index 272483d4a9876..3662edcc1c05b 100644 --- a/compiler/rustc_resolve/messages.ftl +++ b/compiler/rustc_resolve/messages.ftl @@ -127,9 +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 .label = used as parameter more than once diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index 3774ae6642004..cc5e4ca0bbd69 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/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}; @@ -248,6 +248,19 @@ struct UnresolvedImportError { candidates: Option>, } +struct RedundantImportVisibilityMsg { + vis: String, +} + +impl RedundantImportVisibilityMsg { + fn to_string(self) -> String { + format!( + "glob import doesn't reexport anything with visibility `{}` because no imported item is public enough", + self.vis + ) + } +} + // Reexports of the form `pub use foo as bar;` where `foo` is `extern crate foo;` // are permitted for backward-compatibility under a deprecation lint. fn pub_use_of_private_extern_crate_hack(import: Import<'_>, binding: NameBinding<'_>) -> bool { @@ -987,13 +1000,22 @@ 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 = RedundantImportVisibilityMsg { + vis: 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; diff --git a/tests/ui/imports/no-pub-reexports-but-used.rs b/tests/ui/imports/no-pub-reexports-but-used.rs new file mode 100644 index 0000000000000..28991bde829f8 --- /dev/null +++ b/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; +} diff --git a/tests/ui/imports/no-pub-reexports-but-used.stderr b/tests/ui/imports/no-pub-reexports-but-used.stderr new file mode 100644 index 0000000000000..b693dea193579 --- /dev/null +++ b/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 + diff --git a/tests/ui/imports/reexports.rs b/tests/ui/imports/reexports.rs index cb1a3ebe18028..2a1a62834ce8b 100644 --- a/tests/ui/imports/reexports.rs +++ b/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::*` } } diff --git a/tests/ui/imports/reexports.stderr b/tests/ui/imports/reexports.stderr index 401e422af0f8e..bf4ba474875bf 100644 --- a/tests/ui/imports/reexports.stderr +++ b/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 diff --git a/tests/ui/privacy/issue-46209-private-enum-variant-reexport.stderr b/tests/ui/privacy/issue-46209-private-enum-variant-reexport.stderr index df5968ba323b3..93a39edbb8232 100644 --- a/tests/ui/privacy/issue-46209-private-enum-variant-reexport.stderr +++ b/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,11 +52,18 @@ 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 @@ -58,11 +71,18 @@ error: unused import: `self::PettyOfficer::*` 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 diff --git a/tests/ui/privacy/private-variant-reexport.stderr b/tests/ui/privacy/private-variant-reexport.stderr index 2f041934a8171..d73bd1a8cc29f 100644 --- a/tests/ui/privacy/private-variant-reexport.stderr +++ b/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 |