Add warn-by-default lint for local binding shadowing exported glob re-export item
This commit is contained in:
parent
1a5f8bce74
commit
b9606589c4
@ -11,6 +11,7 @@
|
||||
use rustc_data_structures::sync::Lrc;
|
||||
use rustc_macros::HashStable_Generic;
|
||||
use rustc_span::symbol::{kw, sym};
|
||||
#[cfg_attr(not(bootstrap), allow(hidden_glob_reexports))]
|
||||
use rustc_span::symbol::{Ident, Symbol};
|
||||
use rustc_span::{self, edition::Edition, Span, DUMMY_SP};
|
||||
use std::borrow::Cow;
|
||||
|
@ -952,6 +952,10 @@ fn lookup_with_diagnostics(
|
||||
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));
|
||||
}
|
||||
BuiltinLintDiagnostics::HiddenGlobReexports { name, namespace, glob_reexport_span, private_item_span } => {
|
||||
db.span_label(glob_reexport_span, format!("the name `{}` in the {} namespace is supposed to be publicly re-exported here", name, namespace));
|
||||
db.span_label(private_item_span, "but the private item here shadows it");
|
||||
}
|
||||
}
|
||||
// Rewrap `db`, and pass control to the user.
|
||||
decorate(db)
|
||||
|
@ -3272,6 +3272,43 @@
|
||||
"ambiguous glob re-exports",
|
||||
}
|
||||
|
||||
declare_lint! {
|
||||
/// The `hidden_glob_reexports` lint detects cases where glob re-export items are shadowed by
|
||||
/// private items.
|
||||
///
|
||||
/// ### Example
|
||||
///
|
||||
/// ```rust,compile_fail
|
||||
/// #![deny(hidden_glob_reexports)]
|
||||
///
|
||||
/// pub mod upstream {
|
||||
/// mod inner { pub struct Foo {}; pub struct Bar {}; }
|
||||
/// pub use self::inner::*;
|
||||
/// struct Foo {} // private item shadows `inner::Foo`
|
||||
/// }
|
||||
///
|
||||
/// // mod downstream {
|
||||
/// // fn test() {
|
||||
/// // let _ = crate::upstream::Foo; // inaccessible
|
||||
/// // }
|
||||
/// // }
|
||||
///
|
||||
/// pub fn main() {}
|
||||
/// ```
|
||||
///
|
||||
/// {{produces}}
|
||||
///
|
||||
/// ### Explanation
|
||||
///
|
||||
/// This was previously accepted without any errors or warnings but it could silently break a
|
||||
/// crate's downstream user code. If the `struct Foo` was added, `dep::inner::Foo` would
|
||||
/// silently become inaccessible and trigger a "`struct `Foo` is private`" visibility error at
|
||||
/// the downstream use site.
|
||||
pub HIDDEN_GLOB_REEXPORTS,
|
||||
Warn,
|
||||
"name introduced by a private item shadows a name introduced by a public glob re-export",
|
||||
}
|
||||
|
||||
declare_lint_pass! {
|
||||
/// Does nothing as a lint pass, but registers some `Lint`s
|
||||
/// that are used by other parts of the compiler.
|
||||
@ -3304,6 +3341,7 @@
|
||||
FORBIDDEN_LINT_GROUPS,
|
||||
FUNCTION_ITEM_REFERENCES,
|
||||
FUZZY_PROVENANCE_CASTS,
|
||||
HIDDEN_GLOB_REEXPORTS,
|
||||
ILL_FORMED_ATTRIBUTE_INPUT,
|
||||
ILLEGAL_FLOATING_POINT_LITERAL_PATTERN,
|
||||
IMPLIED_BOUNDS_ENTAILMENT,
|
||||
|
@ -540,6 +540,16 @@ pub enum BuiltinLintDiagnostics {
|
||||
/// Span where the same name is also re-exported.
|
||||
duplicate_reexport_span: Span,
|
||||
},
|
||||
HiddenGlobReexports {
|
||||
/// The name of the local binding which shadows the glob re-export.
|
||||
name: String,
|
||||
/// The namespace for which the shadowing occurred in.
|
||||
namespace: String,
|
||||
/// The glob reexport that is shadowed by the local binding.
|
||||
glob_reexport_span: Span,
|
||||
/// The local binding that shadows the glob reexport.
|
||||
private_item_span: Span,
|
||||
},
|
||||
}
|
||||
|
||||
/// Lints that are buffered up early on in the `Session` before the
|
||||
|
@ -53,7 +53,6 @@
|
||||
use rustc_span::{ExpnId, ExpnKind, Span};
|
||||
use rustc_target::abi::{Align, FieldIdx, Integer, IntegerType, VariantIdx};
|
||||
pub use rustc_target::abi::{ReprFlags, ReprOptions};
|
||||
use rustc_type_ir::WithCachedTypeInfo;
|
||||
pub use subst::*;
|
||||
pub use vtable::*;
|
||||
|
||||
@ -145,6 +144,7 @@
|
||||
mod parameterized;
|
||||
mod rvalue_scopes;
|
||||
mod structural_impls;
|
||||
#[cfg_attr(not(bootstrap), allow(hidden_glob_reexports))]
|
||||
mod sty;
|
||||
mod typeck_results;
|
||||
|
||||
|
@ -21,7 +21,8 @@
|
||||
use rustc_middle::span_bug;
|
||||
use rustc_middle::ty;
|
||||
use rustc_session::lint::builtin::{
|
||||
AMBIGUOUS_GLOB_REEXPORTS, PUB_USE_OF_PRIVATE_EXTERN_CRATE, UNUSED_IMPORTS,
|
||||
AMBIGUOUS_GLOB_REEXPORTS, HIDDEN_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;
|
||||
@ -526,31 +527,71 @@ pub(crate) fn finalize_imports(&mut self) {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn check_reexport_ambiguities(
|
||||
pub(crate) fn check_hidden_glob_reexports(
|
||||
&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,
|
||||
},
|
||||
);
|
||||
for (key, resolution) in self.resolutions(module).borrow().iter() {
|
||||
let resolution = resolution.borrow();
|
||||
|
||||
if let Some(binding) = resolution.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))
|
||||
{
|
||||
self.lint_buffer.buffer_lint_with_diagnostic(
|
||||
AMBIGUOUS_GLOB_REEXPORTS,
|
||||
import.root_id,
|
||||
import.root_span,
|
||||
"ambiguous glob re-exports",
|
||||
BuiltinLintDiagnostics::AmbiguousGlobReexports {
|
||||
name: key.ident.to_string(),
|
||||
namespace: key.ns.descr().to_string(),
|
||||
first_reexport_span: import.root_span,
|
||||
duplicate_reexport_span: amb_binding.span,
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
if let Some(glob_binding) = resolution.shadowed_glob {
|
||||
let binding_id = match binding.kind {
|
||||
NameBindingKind::Res(res) => {
|
||||
Some(self.def_id_to_node_id[res.def_id().expect_local()])
|
||||
}
|
||||
NameBindingKind::Module(module) => {
|
||||
Some(self.def_id_to_node_id[module.def_id().expect_local()])
|
||||
}
|
||||
NameBindingKind::Import { import, .. } => import.id(),
|
||||
};
|
||||
|
||||
if binding.res() != Res::Err
|
||||
&& glob_binding.res() != Res::Err
|
||||
&& let NameBindingKind::Import { import: glob_import, .. } = glob_binding.kind
|
||||
&& let Some(binding_id) = binding_id
|
||||
&& let Some(glob_import_id) = glob_import.id()
|
||||
&& let glob_import_def_id = self.local_def_id(glob_import_id)
|
||||
&& self.effective_visibilities.is_exported(glob_import_def_id)
|
||||
&& glob_binding.vis.is_public()
|
||||
&& !binding.vis.is_public()
|
||||
{
|
||||
self.lint_buffer.buffer_lint_with_diagnostic(
|
||||
HIDDEN_GLOB_REEXPORTS,
|
||||
binding_id,
|
||||
binding.span,
|
||||
"private item shadows public glob re-export",
|
||||
BuiltinLintDiagnostics::HiddenGlobReexports {
|
||||
name: key.ident.name.to_string(),
|
||||
namespace: key.ns.descr().to_owned(),
|
||||
glob_reexport_span: glob_binding.span,
|
||||
private_item_span: binding.span,
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -1496,8 +1496,8 @@ pub fn resolve_crate(&mut self, krate: &Crate) {
|
||||
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("check_hidden_glob_reexports", || {
|
||||
self.check_hidden_glob_reexports(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));
|
||||
|
@ -14,10 +14,12 @@
|
||||
pub mod outlives_bounds;
|
||||
mod project;
|
||||
pub mod query;
|
||||
#[cfg_attr(not(bootstrap), allow(hidden_glob_reexports))]
|
||||
mod select;
|
||||
mod specialize;
|
||||
mod structural_match;
|
||||
mod structural_normalize;
|
||||
#[cfg_attr(not(bootstrap), allow(hidden_glob_reexports))]
|
||||
mod util;
|
||||
mod vtable;
|
||||
pub mod wf;
|
||||
|
@ -6,6 +6,7 @@ mod parser {
|
||||
pub use options::*;
|
||||
// Private single import shadows public glob import, but arrives too late for initial
|
||||
// resolution of `use parser::ParseOptions` because it depends on that resolution itself.
|
||||
#[allow(hidden_glob_reexports)]
|
||||
use ParseOptions;
|
||||
}
|
||||
|
||||
|
@ -1,16 +1,16 @@
|
||||
error[E0603]: struct import `ParseOptions` is private
|
||||
--> $DIR/issue-55884-2.rs:12:17
|
||||
--> $DIR/issue-55884-2.rs:13:17
|
||||
|
|
||||
LL | pub use parser::ParseOptions;
|
||||
| ^^^^^^^^^^^^ private struct import
|
||||
|
|
||||
note: the struct import `ParseOptions` is defined here...
|
||||
--> $DIR/issue-55884-2.rs:9:9
|
||||
--> $DIR/issue-55884-2.rs:10:9
|
||||
|
|
||||
LL | use ParseOptions;
|
||||
| ^^^^^^^^^^^^
|
||||
note: ...and refers to the struct import `ParseOptions` which is defined here...
|
||||
--> $DIR/issue-55884-2.rs:12:9
|
||||
--> $DIR/issue-55884-2.rs:13:9
|
||||
|
|
||||
LL | pub use parser::ParseOptions;
|
||||
| ^^^^^^^^^^^^^^^^^^^^ consider importing it directly
|
||||
|
52
tests/ui/resolve/hidden_glob_reexports.rs
Normal file
52
tests/ui/resolve/hidden_glob_reexports.rs
Normal file
@ -0,0 +1,52 @@
|
||||
// check-pass
|
||||
|
||||
pub mod upstream_a {
|
||||
mod inner {
|
||||
pub struct Foo {}
|
||||
pub struct Bar {}
|
||||
}
|
||||
|
||||
pub use self::inner::*;
|
||||
|
||||
struct Foo;
|
||||
//~^ WARN private item shadows public glob re-export
|
||||
}
|
||||
|
||||
pub mod upstream_b {
|
||||
mod inner {
|
||||
pub struct Foo {}
|
||||
pub struct Qux {}
|
||||
}
|
||||
|
||||
mod other {
|
||||
pub struct Foo;
|
||||
}
|
||||
|
||||
pub use self::inner::*;
|
||||
|
||||
use self::other::Foo;
|
||||
//~^ WARN private item shadows public glob re-export
|
||||
}
|
||||
|
||||
pub mod upstream_c {
|
||||
mod no_def_id {
|
||||
#![allow(non_camel_case_types)]
|
||||
pub struct u8;
|
||||
pub struct World;
|
||||
}
|
||||
|
||||
pub use self::no_def_id::*;
|
||||
|
||||
use std::primitive::u8;
|
||||
//~^ WARN private item shadows public glob re-export
|
||||
}
|
||||
|
||||
// Downstream crate
|
||||
// mod downstream {
|
||||
// fn proof() {
|
||||
// let _ = crate::upstream_a::Foo;
|
||||
// let _ = crate::upstream_b::Foo;
|
||||
// }
|
||||
// }
|
||||
|
||||
pub fn main() {}
|
31
tests/ui/resolve/hidden_glob_reexports.stderr
Normal file
31
tests/ui/resolve/hidden_glob_reexports.stderr
Normal file
@ -0,0 +1,31 @@
|
||||
warning: private item shadows public glob re-export
|
||||
--> $DIR/hidden_glob_reexports.rs:11:5
|
||||
|
|
||||
LL | pub use self::inner::*;
|
||||
| -------------- the name `Foo` in the type namespace is supposed to be publicly re-exported here
|
||||
LL |
|
||||
LL | struct Foo;
|
||||
| ^^^^^^^^^^^ but the private item here shadows it
|
||||
|
|
||||
= note: `#[warn(hidden_glob_reexports)]` on by default
|
||||
|
||||
warning: private item shadows public glob re-export
|
||||
--> $DIR/hidden_glob_reexports.rs:27:9
|
||||
|
|
||||
LL | pub use self::inner::*;
|
||||
| -------------- the name `Foo` in the type namespace is supposed to be publicly re-exported here
|
||||
LL |
|
||||
LL | use self::other::Foo;
|
||||
| ^^^^^^^^^^^^^^^^ but the private item here shadows it
|
||||
|
||||
warning: private item shadows public glob re-export
|
||||
--> $DIR/hidden_glob_reexports.rs:40:9
|
||||
|
|
||||
LL | pub use self::no_def_id::*;
|
||||
| ------------------ the name `u8` in the type namespace is supposed to be publicly re-exported here
|
||||
LL |
|
||||
LL | use std::primitive::u8;
|
||||
| ^^^^^^^^^^^^^^^^^^ but the private item here shadows it
|
||||
|
||||
warning: 3 warnings emitted
|
||||
|
Loading…
Reference in New Issue
Block a user