Skip to content

Commit b960658

Browse files
jieyouxujieyouxu
committed
Add warn-by-default lint for local binding shadowing exported glob re-export item
1 parent 1a5f8bc commit b960658

File tree

12 files changed

+207
-27
lines changed

12 files changed

+207
-27
lines changed

‎compiler/rustc_ast/src/token.rs

+1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
1111
use rustc_data_structures::sync::Lrc;
1212
use rustc_macros::HashStable_Generic;
1313
use rustc_span::symbol::{kw, sym};
14+
#[cfg_attr(not(bootstrap), allow(hidden_glob_reexports))]
1415
use rustc_span::symbol::{Ident, Symbol};
1516
use rustc_span::{self, edition::Edition, Span, DUMMY_SP};
1617
use std::borrow::Cow;

‎compiler/rustc_lint/src/context.rs

+4
Original file line numberDiff line numberDiff line change
@@ -952,6 +952,10 @@ pub trait LintContext: Sized {
952952
db.span_label(first_reexport_span, format!("the name `{}` in the {} namespace is first re-exported here", name, namespace));
953953
db.span_label(duplicate_reexport_span, format!("but the name `{}` in the {} namespace is also re-exported here", name, namespace));
954954
}
955+
BuiltinLintDiagnostics::HiddenGlobReexports { name, namespace, glob_reexport_span, private_item_span } => {
956+
db.span_label(glob_reexport_span, format!("the name `{}` in the {} namespace is supposed to be publicly re-exported here", name, namespace));
957+
db.span_label(private_item_span, "but the private item here shadows it");
958+
}
955959
}
956960
// Rewrap `db`, and pass control to the user.
957961
decorate(db)

‎compiler/rustc_lint_defs/src/builtin.rs

+38
Original file line numberDiff line numberDiff line change
@@ -3272,6 +3272,43 @@ declare_lint! {
32723272
"ambiguous glob re-exports",
32733273
}
32743274

3275+
declare_lint! {
3276+
/// The `hidden_glob_reexports` lint detects cases where glob re-export items are shadowed by
3277+
/// private items.
3278+
///
3279+
/// ### Example
3280+
///
3281+
/// ```rust,compile_fail
3282+
/// #![deny(hidden_glob_reexports)]
3283+
///
3284+
/// pub mod upstream {
3285+
/// mod inner { pub struct Foo {}; pub struct Bar {}; }
3286+
/// pub use self::inner::*;
3287+
/// struct Foo {} // private item shadows `inner::Foo`
3288+
/// }
3289+
///
3290+
/// // mod downstream {
3291+
/// // fn test() {
3292+
/// // let _ = crate::upstream::Foo; // inaccessible
3293+
/// // }
3294+
/// // }
3295+
///
3296+
/// pub fn main() {}
3297+
/// ```
3298+
///
3299+
/// {{produces}}
3300+
///
3301+
/// ### Explanation
3302+
///
3303+
/// This was previously accepted without any errors or warnings but it could silently break a
3304+
/// crate's downstream user code. If the `struct Foo` was added, `dep::inner::Foo` would
3305+
/// silently become inaccessible and trigger a "`struct `Foo` is private`" visibility error at
3306+
/// the downstream use site.
3307+
pub HIDDEN_GLOB_REEXPORTS,
3308+
Warn,
3309+
"name introduced by a private item shadows a name introduced by a public glob re-export",
3310+
}
3311+
32753312
declare_lint_pass! {
32763313
/// Does nothing as a lint pass, but registers some `Lint`s
32773314
/// that are used by other parts of the compiler.
@@ -3304,6 +3341,7 @@ declare_lint_pass! {
33043341
FORBIDDEN_LINT_GROUPS,
33053342
FUNCTION_ITEM_REFERENCES,
33063343
FUZZY_PROVENANCE_CASTS,
3344+
HIDDEN_GLOB_REEXPORTS,
33073345
ILL_FORMED_ATTRIBUTE_INPUT,
33083346
ILLEGAL_FLOATING_POINT_LITERAL_PATTERN,
33093347
IMPLIED_BOUNDS_ENTAILMENT,

‎compiler/rustc_lint_defs/src/lib.rs

+10
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,16 @@ pub enum BuiltinLintDiagnostics {
540540
/// Span where the same name is also re-exported.
541541
duplicate_reexport_span: Span,
542542
},
543+
HiddenGlobReexports {
544+
/// The name of the local binding which shadows the glob re-export.
545+
name: String,
546+
/// The namespace for which the shadowing occurred in.
547+
namespace: String,
548+
/// The glob reexport that is shadowed by the local binding.
549+
glob_reexport_span: Span,
550+
/// The local binding that shadows the glob reexport.
551+
private_item_span: Span,
552+
},
543553
}
544554

545555
/// Lints that are buffered up early on in the `Session` before the

‎compiler/rustc_middle/src/ty/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ use rustc_span::symbol::{kw, sym, Ident, Symbol};
5353
use rustc_span::{ExpnId, ExpnKind, Span};
5454
use rustc_target::abi::{Align, FieldIdx, Integer, IntegerType, VariantIdx};
5555
pub use rustc_target::abi::{ReprFlags, ReprOptions};
56-
use rustc_type_ir::WithCachedTypeInfo;
5756
pub use subst::*;
5857
pub use vtable::*;
5958

@@ -145,6 +144,7 @@ mod opaque_types;
145144
mod parameterized;
146145
mod rvalue_scopes;
147146
mod structural_impls;
147+
#[cfg_attr(not(bootstrap), allow(hidden_glob_reexports))]
148148
mod sty;
149149
mod typeck_results;
150150

‎compiler/rustc_resolve/src/imports.rs

+62-21
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ use rustc_middle::metadata::Reexport;
2121
use rustc_middle::span_bug;
2222
use rustc_middle::ty;
2323
use rustc_session::lint::builtin::{
24-
AMBIGUOUS_GLOB_REEXPORTS, PUB_USE_OF_PRIVATE_EXTERN_CRATE, UNUSED_IMPORTS,
24+
AMBIGUOUS_GLOB_REEXPORTS, HIDDEN_GLOB_REEXPORTS, PUB_USE_OF_PRIVATE_EXTERN_CRATE,
25+
UNUSED_IMPORTS,
2526
};
2627
use rustc_session::lint::BuiltinLintDiagnostics;
2728
use rustc_span::edit_distance::find_best_match_for_name;
@@ -526,31 +527,71 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
526527
}
527528
}
528529

529-
pub(crate) fn check_reexport_ambiguities(
530+
pub(crate) fn check_hidden_glob_reexports(
530531
&mut self,
531532
exported_ambiguities: FxHashSet<Interned<'a, NameBinding<'a>>>,
532533
) {
533534
for module in self.arenas.local_modules().iter() {
534-
module.for_each_child(self, |this, ident, ns, binding| {
535-
if let NameBindingKind::Import { import, .. } = binding.kind
536-
&& let Some((amb_binding, _)) = binding.ambiguity
537-
&& binding.res() != Res::Err
538-
&& exported_ambiguities.contains(&Interned::new_unchecked(binding))
539-
{
540-
this.lint_buffer.buffer_lint_with_diagnostic(
541-
AMBIGUOUS_GLOB_REEXPORTS,
542-
import.root_id,
543-
import.root_span,
544-
"ambiguous glob re-exports",
545-
BuiltinLintDiagnostics::AmbiguousGlobReexports {
546-
name: ident.to_string(),
547-
namespace: ns.descr().to_string(),
548-
first_reexport_span: import.root_span,
549-
duplicate_reexport_span: amb_binding.span,
550-
},
551-
);
535+
for (key, resolution) in self.resolutions(module).borrow().iter() {
536+
let resolution = resolution.borrow();
537+
538+
if let Some(binding) = resolution.binding {
539+
if let NameBindingKind::Import { import, .. } = binding.kind
540+
&& let Some((amb_binding, _)) = binding.ambiguity
541+
&& binding.res() != Res::Err
542+
&& exported_ambiguities.contains(&Interned::new_unchecked(binding))
543+
{
544+
self.lint_buffer.buffer_lint_with_diagnostic(
545+
AMBIGUOUS_GLOB_REEXPORTS,
546+
import.root_id,
547+
import.root_span,
548+
"ambiguous glob re-exports",
549+
BuiltinLintDiagnostics::AmbiguousGlobReexports {
550+
name: key.ident.to_string(),
551+
namespace: key.ns.descr().to_string(),
552+
first_reexport_span: import.root_span,
553+
duplicate_reexport_span: amb_binding.span,
554+
},
555+
);
556+
}
557+
558+
if let Some(glob_binding) = resolution.shadowed_glob {
559+
let binding_id = match binding.kind {
560+
NameBindingKind::Res(res) => {
561+
Some(self.def_id_to_node_id[res.def_id().expect_local()])
562+
}
563+
NameBindingKind::Module(module) => {
564+
Some(self.def_id_to_node_id[module.def_id().expect_local()])
565+
}
566+
NameBindingKind::Import { import, .. } => import.id(),
567+
};
568+
569+
if binding.res() != Res::Err
570+
&& glob_binding.res() != Res::Err
571+
&& let NameBindingKind::Import { import: glob_import, .. } = glob_binding.kind
572+
&& let Some(binding_id) = binding_id
573+
&& let Some(glob_import_id) = glob_import.id()
574+
&& let glob_import_def_id = self.local_def_id(glob_import_id)
575+
&& self.effective_visibilities.is_exported(glob_import_def_id)
576+
&& glob_binding.vis.is_public()
577+
&& !binding.vis.is_public()
578+
{
579+
self.lint_buffer.buffer_lint_with_diagnostic(
580+
HIDDEN_GLOB_REEXPORTS,
581+
binding_id,
582+
binding.span,
583+
"private item shadows public glob re-export",
584+
BuiltinLintDiagnostics::HiddenGlobReexports {
585+
name: key.ident.name.to_string(),
586+
namespace: key.ns.descr().to_owned(),
587+
glob_reexport_span: glob_binding.span,
588+
private_item_span: binding.span,
589+
},
590+
);
591+
}
592+
}
552593
}
553-
});
594+
}
554595
}
555596
}
556597

‎compiler/rustc_resolve/src/lib.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1496,8 +1496,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
14961496
let exported_ambiguities = self.tcx.sess.time("compute_effective_visibilities", || {
14971497
EffectiveVisibilitiesVisitor::compute_effective_visibilities(self, krate)
14981498
});
1499-
self.tcx.sess.time("check_reexport_ambiguities", || {
1500-
self.check_reexport_ambiguities(exported_ambiguities)
1499+
self.tcx.sess.time("check_hidden_glob_reexports", || {
1500+
self.check_hidden_glob_reexports(exported_ambiguities)
15011501
});
15021502
self.tcx.sess.time("finalize_macro_resolutions", || self.finalize_macro_resolutions());
15031503
self.tcx.sess.time("late_resolve_crate", || self.late_resolve_crate(krate));

‎compiler/rustc_trait_selection/src/traits/mod.rs

+2
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@ mod object_safety;
1414
pub mod outlives_bounds;
1515
mod project;
1616
pub mod query;
17+
#[cfg_attr(not(bootstrap), allow(hidden_glob_reexports))]
1718
mod select;
1819
mod specialize;
1920
mod structural_match;
2021
mod structural_normalize;
22+
#[cfg_attr(not(bootstrap), allow(hidden_glob_reexports))]
2123
mod util;
2224
mod vtable;
2325
pub mod wf;

‎tests/ui/imports/issue-55884-2.rs

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ mod parser {
66
pub use options::*;
77
// Private single import shadows public glob import, but arrives too late for initial
88
// resolution of `use parser::ParseOptions` because it depends on that resolution itself.
9+
#[allow(hidden_glob_reexports)]
910
use ParseOptions;
1011
}
1112

‎tests/ui/imports/issue-55884-2.stderr

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
error[E0603]: struct import `ParseOptions` is private
2-
--> $DIR/issue-55884-2.rs:12:17
2+
--> $DIR/issue-55884-2.rs:13:17
33
|
44
LL | pub use parser::ParseOptions;
55
| ^^^^^^^^^^^^ private struct import
66
|
77
note: the struct import `ParseOptions` is defined here...
8-
--> $DIR/issue-55884-2.rs:9:9
8+
--> $DIR/issue-55884-2.rs:10:9
99
|
1010
LL | use ParseOptions;
1111
| ^^^^^^^^^^^^
1212
note: ...and refers to the struct import `ParseOptions` which is defined here...
13-
--> $DIR/issue-55884-2.rs:12:9
13+
--> $DIR/issue-55884-2.rs:13:9
1414
|
1515
LL | pub use parser::ParseOptions;
1616
| ^^^^^^^^^^^^^^^^^^^^ consider importing it directly

‎tests/ui/resolve/hidden_glob_reexports.rs

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// check-pass
2+
3+
pub mod upstream_a {
4+
mod inner {
5+
pub struct Foo {}
6+
pub struct Bar {}
7+
}
8+
9+
pub use self::inner::*;
10+
11+
struct Foo;
12+
//~^ WARN private item shadows public glob re-export
13+
}
14+
15+
pub mod upstream_b {
16+
mod inner {
17+
pub struct Foo {}
18+
pub struct Qux {}
19+
}
20+
21+
mod other {
22+
pub struct Foo;
23+
}
24+
25+
pub use self::inner::*;
26+
27+
use self::other::Foo;
28+
//~^ WARN private item shadows public glob re-export
29+
}
30+
31+
pub mod upstream_c {
32+
mod no_def_id {
33+
#![allow(non_camel_case_types)]
34+
pub struct u8;
35+
pub struct World;
36+
}
37+
38+
pub use self::no_def_id::*;
39+
40+
use std::primitive::u8;
41+
//~^ WARN private item shadows public glob re-export
42+
}
43+
44+
// Downstream crate
45+
// mod downstream {
46+
// fn proof() {
47+
// let _ = crate::upstream_a::Foo;
48+
// let _ = crate::upstream_b::Foo;
49+
// }
50+
// }
51+
52+
pub fn main() {}

‎tests/ui/resolve/hidden_glob_reexports.stderr

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
warning: private item shadows public glob re-export
2+
--> $DIR/hidden_glob_reexports.rs:11:5
3+
|
4+
LL | pub use self::inner::*;
5+
| -------------- the name `Foo` in the type namespace is supposed to be publicly re-exported here
6+
LL |
7+
LL | struct Foo;
8+
| ^^^^^^^^^^^ but the private item here shadows it
9+
|
10+
= note: `#[warn(hidden_glob_reexports)]` on by default
11+
12+
warning: private item shadows public glob re-export
13+
--> $DIR/hidden_glob_reexports.rs:27:9
14+
|
15+
LL | pub use self::inner::*;
16+
| -------------- the name `Foo` in the type namespace is supposed to be publicly re-exported here
17+
LL |
18+
LL | use self::other::Foo;
19+
| ^^^^^^^^^^^^^^^^ but the private item here shadows it
20+
21+
warning: private item shadows public glob re-export
22+
--> $DIR/hidden_glob_reexports.rs:40:9
23+
|
24+
LL | pub use self::no_def_id::*;
25+
| ------------------ the name `u8` in the type namespace is supposed to be publicly re-exported here
26+
LL |
27+
LL | use std::primitive::u8;
28+
| ^^^^^^^^^^^^^^^^^^ but the private item here shadows it
29+
30+
warning: 3 warnings emitted
31+

0 commit comments

Comments
 (0)