Skip to content

Commit

Permalink
vis note for no pub reexports glob import
Browse files Browse the repository at this point in the history
  • Loading branch information
bvanjoi committed Nov 30, 2023
1 parent 07921b5 commit ee5551b
Show file tree
Hide file tree
Showing 12 changed files with 130 additions and 33 deletions.
4 changes: 4 additions & 0 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
25 changes: 5 additions & 20 deletions compiler/rustc_privacy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(", ");
}
Expand Down Expand Up @@ -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),
},
);
}
Expand Down Expand Up @@ -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),
},
);
}
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_resolve/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 26 additions & 4 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -248,6 +248,19 @@ struct UnresolvedImportError {
candidates: Option<Vec<ImportSuggestion>>,
}

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 {
Expand Down Expand Up @@ -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;
Expand Down
15 changes: 15 additions & 0 deletions tests/ui/imports/no-pub-reexports-but-used.rs
Original file line number Diff line number Diff line change
@@ -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;
}
20 changes: 20 additions & 0 deletions tests/ui/imports/no-pub-reexports-but-used.stderr
Original file line number Diff line number Diff line change
@@ -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

2 changes: 1 addition & 1 deletion tests/ui/imports/reexports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*`
}
}
Expand Down
9 changes: 8 additions & 1 deletion tests/ui/imports/reexports.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 23 additions & 3 deletions tests/ui/privacy/issue-46209-private-enum-variant-reexport.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand All @@ -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
Expand Down
8 changes: 7 additions & 1 deletion tests/ui/privacy/private-variant-reexport.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down

0 comments on commit ee5551b

Please sign in to comment.