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 1670ff6 commit 244dd83
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 21 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::InsufficientVisibility { max_vis, span } => {
let msg = format!("the most public imported item is `{max_vis}`");
db.span_note(span, msg);
}
}
// 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,
},
InsufficientVisibility {
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<'tcx>(self, def_id: LocalDefId, tcx: TyCtxt<'tcx>) -> 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
6 changes: 5 additions & 1 deletion compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -989,11 +989,15 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
&& let Some(max_vis) = max_vis.get()
&& !max_vis.is_at_least(import.expect_vis(), self.tcx)
{
self.lint_buffer.buffer_lint(
self.lint_buffer.buffer_lint_with_diagnostic(
UNUSED_IMPORTS,
id,
import.span,
fluent::resolve_glob_import_doesnt_reexport,
BuiltinLintDiagnostics::InsufficientVisibility {
max_vis: max_vis.to_string(self.local_def_id(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 because no candidate is public enough
//~| NOTE: the most public imported item is `pub(crate)`

fn main() {
let _: A;
}
19 changes: 19 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,19 @@
warning: glob import doesn't reexport anything because no candidate 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::*;
| ^^^^
note: the lint level is defined here
--> $DIR/no-pub-reexports-but-used.rs:8:8
|
LL | #[warn(unused_imports)]
| ^^^^^^^^^^^^^^

warning: 1 warning emitted

6 changes: 6 additions & 0 deletions tests/ui/imports/reexports.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,12 @@ LL | #![warn(unused_imports)]
warning: glob import doesn't reexport anything because no candidate 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::*;
| ^^^^^^^^

Expand Down
17 changes: 17 additions & 0 deletions tests/ui/privacy/issue-46209-private-enum-variant-reexport.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ error: glob import doesn't reexport anything because no candidate is public enou
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::*;
| ^^^^^^^^^^^^^^^^^^
note: the lint level is defined here
--> $DIR/issue-46209-private-enum-variant-reexport.rs:1:8
|
Expand All @@ -49,6 +54,12 @@ LL | pub use self::Lieutenant::{JuniorGrade, Full};
error: glob import doesn't reexport anything because no candidate 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::*;
| ^^^^^^^^^^^^^^^^^^^^^

Expand All @@ -61,6 +72,12 @@ LL | pub use self::PettyOfficer::*;
error: glob import doesn't reexport anything because no candidate 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::*;
| ^^^^^^^^^^^^^^^^

Expand Down
5 changes: 5 additions & 0 deletions tests/ui/privacy/private-variant-reexport.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ error: glob import doesn't reexport anything because no candidate is public enou
LL | pub use ::E::*;
| ^^^^^^
|
note: the most public imported item is `pub(crate)`
--> $DIR/private-variant-reexport.rs:15:13
|
LL | pub use ::E::*;
| ^^^^^^
note: the lint level is defined here
--> $DIR/private-variant-reexport.rs:13:8
|
Expand Down

0 comments on commit 244dd83

Please sign in to comment.