Lint ambiguous glob re-exports

This commit is contained in:
许杰友 Jieyou Xu (Joe) 2023-03-20 03:11:28 +08:00
parent ab9bb3ea36
commit 1f67949f0e
No known key found for this signature in database
GPG Key ID: C5FD5D32014FDB47
11 changed files with 228 additions and 26 deletions

View File

@ -910,6 +910,10 @@ pub trait LintContext: Sized {
Applicability::MachineApplicable,
);
}
BuiltinLintDiagnostics::AmbiguousGlobReexports { name, namespace, first_reexport_span, duplicate_reexport_span } => {
db.span_label(first_reexport_span, format!("the name `{}` in the {} namespace is first re-exported here", name, namespace));
db.span_label(duplicate_reexport_span, format!("but the name `{}` in the {} namespace is also re-exported here", name, namespace));
}
}
// Rewrap `db`, and pass control to the user.
decorate(db)

View File

@ -3230,6 +3230,45 @@ declare_lint! {
};
}
declare_lint! {
/// The `ambiguous_glob_reexports` lint detects cases where names re-exported via globs
/// collide. Downstream users trying to use the same name re-exported from multiple globs
/// will receive a warning pointing out redefinition of the same name.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![deny(ambiguous_glob_reexports)]
/// pub mod foo {
/// pub type X = u8;
/// }
///
/// pub mod bar {
/// pub type Y = u8;
/// pub type X = u8;
/// }
///
/// pub use foo::*;
/// pub use bar::*;
///
///
/// pub fn main() {}
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// This was previously accepted but it could silently break a crate's downstream users code.
/// For example, if `foo::*` and `bar::*` were re-exported before `bar::X` was added to the
/// re-exports, down stream users could use `this_crate::X` without problems. However, adding
/// `bar::X` would cause compilation errors in downstream crates because `X` is defined
/// multiple times in the same namespace of `this_crate`.
pub AMBIGUOUS_GLOB_REEXPORTS,
Warn,
"ambiguous glob re-exports",
}
declare_lint_pass! {
/// Does nothing as a lint pass, but registers some `Lint`s
/// that are used by other parts of the compiler.
@ -3337,6 +3376,7 @@ declare_lint_pass! {
NAMED_ARGUMENTS_USED_POSITIONALLY,
IMPLIED_BOUNDS_ENTAILMENT,
BYTE_SLICE_IN_PACKED_STRUCT_WITH_DERIVE,
AMBIGUOUS_GLOB_REEXPORTS,
]
}

View File

@ -529,6 +529,16 @@ pub enum BuiltinLintDiagnostics {
vis_span: Span,
ident_span: Span,
},
AmbiguousGlobReexports {
/// The name for which collision(s) have occurred.
name: String,
/// The name space for whihc the collision(s) occurred in.
namespace: String,
/// Span where the name is first re-exported.
first_reexport_span: Span,
/// Span where the same name is also re-exported.
duplicate_reexport_span: Span,
},
}
/// Lints that are buffered up early on in the `Session` before the

View File

@ -4,6 +4,7 @@ use rustc_ast::visit;
use rustc_ast::visit::Visitor;
use rustc_ast::Crate;
use rustc_ast::EnumDef;
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::intern::Interned;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::def_id::CRATE_DEF_ID;
@ -70,11 +71,11 @@ impl Resolver<'_, '_> {
impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
/// Fills the `Resolver::effective_visibilities` table with public & exported items
/// For now, this doesn't resolve macros (FIXME) and cannot resolve Impl, as we
/// need access to a TyCtxt for that.
/// need access to a TyCtxt for that. Returns the set of ambiguous re-exports.
pub(crate) fn compute_effective_visibilities<'c>(
r: &'r mut Resolver<'a, 'tcx>,
krate: &'c Crate,
) {
) -> FxHashSet<Interned<'a, NameBinding<'a>>> {
let mut visitor = EffectiveVisibilitiesVisitor {
r,
def_effective_visibilities: Default::default(),
@ -93,18 +94,26 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
}
visitor.r.effective_visibilities = visitor.def_effective_visibilities;
let mut exported_ambiguities = FxHashSet::default();
// Update visibilities for import def ids. These are not used during the
// `EffectiveVisibilitiesVisitor` pass, because we have more detailed binding-based
// information, but are used by later passes. Effective visibility of an import def id
// is the maximum value among visibilities of bindings corresponding to that def id.
for (binding, eff_vis) in visitor.import_effective_visibilities.iter() {
let NameBindingKind::Import { import, .. } = binding.kind else { unreachable!() };
if let Some(node_id) = import.id() {
r.effective_visibilities.update_eff_vis(r.local_def_id(node_id), eff_vis, r.tcx)
if !binding.is_ambiguity() {
if let Some(node_id) = import.id() {
r.effective_visibilities.update_eff_vis(r.local_def_id(node_id), eff_vis, r.tcx)
}
} else if binding.ambiguity.is_some() && eff_vis.is_public_at_level(Level::Reexported) {
exported_ambiguities.insert(*binding);
}
}
info!("resolve::effective_visibilities: {:#?}", r.effective_visibilities);
exported_ambiguities
}
/// Update effective visibilities of bindings in the given module,
@ -115,21 +124,44 @@ impl<'r, 'a, 'tcx> EffectiveVisibilitiesVisitor<'r, 'a, 'tcx> {
let resolutions = self.r.resolutions(module);
for (_, name_resolution) in resolutions.borrow().iter() {
if let Some(mut binding) = name_resolution.borrow().binding() && !binding.is_ambiguity() {
// Set the given effective visibility level to `Level::Direct` and
// sets the rest of the `use` chain to `Level::Reexported` until
// we hit the actual exported item.
let mut parent_id = ParentId::Def(module_id);
while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind {
let binding_id = ImportId::new_unchecked(binding);
self.update_import(binding_id, parent_id);
if let Some(mut binding) = name_resolution.borrow().binding() {
if !binding.is_ambiguity() {
// Set the given effective visibility level to `Level::Direct` and
// sets the rest of the `use` chain to `Level::Reexported` until
// we hit the actual exported item.
let mut parent_id = ParentId::Def(module_id);
while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind
{
let binding_id = ImportId::new_unchecked(binding);
self.update_import(binding_id, parent_id);
parent_id = ParentId::Import(binding_id);
binding = nested_binding;
}
parent_id = ParentId::Import(binding_id);
binding = nested_binding;
}
if let Some(def_id) = binding.res().opt_def_id().and_then(|id| id.as_local()) {
self.update_def(def_id, binding.vis.expect_local(), parent_id);
if let Some(def_id) = binding.res().opt_def_id().and_then(|id| id.as_local()) {
self.update_def(def_id, binding.vis.expect_local(), parent_id);
}
} else {
// Put the root ambiguity binding and all reexports leading to it into the
// table. They are used by the `ambiguous_glob_reexports` lint. For all
// bindings added to the table here `is_ambiguity` returns true.
let mut parent_id = ParentId::Def(module_id);
while let NameBindingKind::Import { binding: nested_binding, .. } = binding.kind
{
let binding_id = ImportId::new_unchecked(binding);
self.update_import(binding_id, parent_id);
if binding.ambiguity.is_some() {
// Stop at the root ambiguity, further bindings in the chain should not
// be reexported because the root ambiguity blocks any access to them.
// (Those further bindings are most likely not ambiguities themselves.)
break;
}
parent_id = ParentId::Import(binding_id);
binding = nested_binding;
}
}
}
}

View File

@ -19,7 +19,9 @@ use rustc_hir::def::{self, DefKind, PartialRes};
use rustc_middle::metadata::ModChild;
use rustc_middle::span_bug;
use rustc_middle::ty;
use rustc_session::lint::builtin::{PUB_USE_OF_PRIVATE_EXTERN_CRATE, UNUSED_IMPORTS};
use rustc_session::lint::builtin::{
AMBIGUOUS_GLOB_REEXPORTS, PUB_USE_OF_PRIVATE_EXTERN_CRATE, UNUSED_IMPORTS,
};
use rustc_session::lint::BuiltinLintDiagnostics;
use rustc_span::edit_distance::find_best_match_for_name;
use rustc_span::hygiene::LocalExpnId;
@ -506,6 +508,34 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}
pub(crate) fn check_reexport_ambiguities(
&mut self,
exported_ambiguities: FxHashSet<Interned<'a, NameBinding<'a>>>,
) {
for module in self.arenas.local_modules().iter() {
module.for_each_child(self, |this, ident, ns, binding| {
if let NameBindingKind::Import { import, .. } = binding.kind
&& let Some((amb_binding, _)) = binding.ambiguity
&& binding.res() != Res::Err
&& exported_ambiguities.contains(&Interned::new_unchecked(binding))
{
this.lint_buffer.buffer_lint_with_diagnostic(
AMBIGUOUS_GLOB_REEXPORTS,
import.root_id,
import.root_span,
"ambiguous glob re-exports",
BuiltinLintDiagnostics::AmbiguousGlobReexports {
name: ident.to_string(),
namespace: ns.descr().to_string(),
first_reexport_span: import.root_span,
duplicate_reexport_span: amb_binding.span,
},
);
}
});
}
}
fn throw_unresolved_import_error(&self, errors: Vec<(&Import<'_>, UnresolvedImportError)>) {
if errors.is_empty() {
return;

View File

@ -1474,9 +1474,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
pub fn resolve_crate(&mut self, krate: &Crate) {
self.tcx.sess.time("resolve_crate", || {
self.tcx.sess.time("finalize_imports", || self.finalize_imports());
self.tcx.sess.time("compute_effective_visibilities", || {
let exported_ambiguities = self.tcx.sess.time("compute_effective_visibilities", || {
EffectiveVisibilitiesVisitor::compute_effective_visibilities(self, krate)
});
self.tcx.sess.time("check_reexport_ambiguities", || {
self.check_reexport_ambiguities(exported_ambiguities)
});
self.tcx.sess.time("finalize_macro_resolutions", || self.finalize_macro_resolutions());
self.tcx.sess.time("late_resolve_crate", || self.late_resolve_crate(krate));
self.tcx.sess.time("resolve_main", || self.resolve_main());

View File

@ -1,3 +1,5 @@
#![allow(ambiguous_glob_reexports)]
mod m1 {
pub fn f() {}
}

View File

@ -1,4 +1,5 @@
#![feature(decl_macro)]
#![allow(ambiguous_glob_reexports)]
macro_rules! define_exported { () => {
#[macro_export]

View File

@ -1,12 +1,12 @@
error[E0659]: `exported` is ambiguous
--> $DIR/local-modularized-tricky-fail-1.rs:28:1
--> $DIR/local-modularized-tricky-fail-1.rs:29:1
|
LL | exported!();
| ^^^^^^^^ ambiguous name
|
= note: ambiguous because of a conflict between a name from a glob import and a macro-expanded name in the same module during import or macro resolution
note: `exported` could refer to the macro defined here
--> $DIR/local-modularized-tricky-fail-1.rs:5:5
--> $DIR/local-modularized-tricky-fail-1.rs:6:5
|
LL | / macro_rules! exported {
LL | | () => ()
@ -16,7 +16,7 @@ LL | | }
LL | define_exported!();
| ------------------ in this macro invocation
note: `exported` could also refer to the macro imported here
--> $DIR/local-modularized-tricky-fail-1.rs:22:5
--> $DIR/local-modularized-tricky-fail-1.rs:23:5
|
LL | use inner1::*;
| ^^^^^^^^^
@ -24,7 +24,7 @@ LL | use inner1::*;
= note: this error originates in the macro `define_exported` (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0659]: `panic` is ambiguous
--> $DIR/local-modularized-tricky-fail-1.rs:35:5
--> $DIR/local-modularized-tricky-fail-1.rs:36:5
|
LL | panic!();
| ^^^^^ ambiguous name
@ -32,7 +32,7 @@ LL | panic!();
= note: ambiguous because of a conflict between a macro-expanded name and a less macro-expanded name from outer scope during import or macro resolution
= note: `panic` could refer to a macro from prelude
note: `panic` could also refer to the macro defined here
--> $DIR/local-modularized-tricky-fail-1.rs:11:5
--> $DIR/local-modularized-tricky-fail-1.rs:12:5
|
LL | / macro_rules! panic {
LL | | () => ()
@ -45,7 +45,7 @@ LL | define_panic!();
= note: this error originates in the macro `define_panic` (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0659]: `include` is ambiguous
--> $DIR/local-modularized-tricky-fail-1.rs:46:1
--> $DIR/local-modularized-tricky-fail-1.rs:47:1
|
LL | include!();
| ^^^^^^^ ambiguous name
@ -53,7 +53,7 @@ LL | include!();
= note: ambiguous because of a conflict between a macro-expanded name and a less macro-expanded name from outer scope during import or macro resolution
= note: `include` could refer to a macro from prelude
note: `include` could also refer to the macro defined here
--> $DIR/local-modularized-tricky-fail-1.rs:17:5
--> $DIR/local-modularized-tricky-fail-1.rs:18:5
|
LL | / macro_rules! include {
LL | | () => ()

View File

@ -0,0 +1,33 @@
#![deny(ambiguous_glob_reexports)]
pub mod foo {
pub type X = u8;
}
pub mod bar {
pub type X = u8;
pub type Y = u8;
}
pub use foo::*;
//~^ ERROR ambiguous glob re-exports
pub use bar::*;
mod ambiguous {
mod m1 { pub type A = u8; }
mod m2 { pub type A = u8; }
pub use self::m1::*;
//~^ ERROR ambiguous glob re-exports
pub use self::m2::*;
}
pub mod single {
pub use ambiguous::A;
//~^ ERROR `A` is ambiguous
}
pub mod glob {
pub use ambiguous::*;
}
pub fn main() {}

View File

@ -0,0 +1,47 @@
error[E0659]: `A` is ambiguous
--> $DIR/issue-107563-ambiguous-glob-reexports.rs:25:24
|
LL | pub use ambiguous::A;
| ^ ambiguous name
|
= note: ambiguous because of multiple glob imports of a name in the same module
note: `A` could refer to the type alias imported here
--> $DIR/issue-107563-ambiguous-glob-reexports.rs:19:13
|
LL | pub use self::m1::*;
| ^^^^^^^^^^^
= help: consider adding an explicit import of `A` to disambiguate
note: `A` could also refer to the type alias imported here
--> $DIR/issue-107563-ambiguous-glob-reexports.rs:21:13
|
LL | pub use self::m2::*;
| ^^^^^^^^^^^
= help: consider adding an explicit import of `A` to disambiguate
error: ambiguous glob re-exports
--> $DIR/issue-107563-ambiguous-glob-reexports.rs:12:9
|
LL | pub use foo::*;
| ^^^^^^ the name `X` in the type namespace is first re-exported here
LL |
LL | pub use bar::*;
| ------ but the name `X` in the type namespace is also re-exported here
|
note: the lint level is defined here
--> $DIR/issue-107563-ambiguous-glob-reexports.rs:1:9
|
LL | #![deny(ambiguous_glob_reexports)]
| ^^^^^^^^^^^^^^^^^^^^^^^^
error: ambiguous glob re-exports
--> $DIR/issue-107563-ambiguous-glob-reexports.rs:19:13
|
LL | pub use self::m1::*;
| ^^^^^^^^^^^ the name `A` in the type namespace is first re-exported here
LL |
LL | pub use self::m2::*;
| ----------- but the name `A` in the type namespace is also re-exported here
error: aborting due to 3 previous errors
For more information about this error, try `rustc --explain E0659`.