Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vis note for no pub reexports glob import #115993

Merged
merged 1 commit into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
2 changes: 0 additions & 2 deletions compiler/rustc_resolve/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 14 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 @@ -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;
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)`
bvanjoi marked this conversation as resolved.
Show resolved Hide resolved
--> $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
Loading