Skip to content

Commit 3a0536a

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #119151 - Jules-Bertholet:no-foreign-doc-hidden-suggest, r=davidtwco
Hide foreign `#[doc(hidden)]` paths in import suggestions Stops the compiler from suggesting to import foreign `#[doc(hidden)]` paths. ```@rustbot``` label A-suggestion-diagnostics
2 parents 11035f9 + 5c0e62c commit 3a0536a

File tree

9 files changed

+126
-35
lines changed

9 files changed

+126
-35
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1483,7 +1483,7 @@ pub fn reveal_opaque_types_in_bounds<'tcx>(
14831483
val.fold_with(&mut visitor)
14841484
}
14851485

1486-
/// Determines whether an item is annotated with `doc(hidden)`.
1486+
/// Determines whether an item is directly annotated with `doc(hidden)`.
14871487
fn is_doc_hidden(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
14881488
tcx.get_attrs(def_id, sym::doc)
14891489
.filter_map(|attr| attr.meta_item_list())

‎compiler/rustc_resolve/src/diagnostics.rs

Lines changed: 41 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ pub(crate) struct ImportSuggestion {
100100
pub descr: &'static str,
101101
pub path: Path,
102102
pub accessible: bool,
103+
// false if the path traverses a foreign `#[doc(hidden)]` item.
104+
pub doc_visible: bool,
103105
pub via_import: bool,
104106
/// An extra note that should be issued if this item is suggested
105107
pub note: Option<String>,
@@ -1146,10 +1148,16 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
11461148
{
11471149
let mut candidates = Vec::new();
11481150
let mut seen_modules = FxHashSet::default();
1149-
let mut worklist = vec![(start_module, ThinVec::<ast::PathSegment>::new(), true)];
1151+
let start_did = start_module.def_id();
1152+
let mut worklist = vec![(
1153+
start_module,
1154+
ThinVec::<ast::PathSegment>::new(),
1155+
true,
1156+
start_did.is_local() || !self.tcx.is_doc_hidden(start_did),
1157+
)];
11501158
let mut worklist_via_import = vec![];
11511159

1152-
while let Some((in_module, path_segments, accessible)) = match worklist.pop() {
1160+
while let Some((in_module, path_segments, accessible, doc_visible)) = match worklist.pop() {
11531161
None => worklist_via_import.pop(),
11541162
Some(x) => Some(x),
11551163
} {
@@ -1192,6 +1200,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
11921200
}
11931201
}
11941202

1203+
let res = name_binding.res();
1204+
let did = match res {
1205+
Res::Def(DefKind::Ctor(..), did) => this.tcx.opt_parent(did),
1206+
_ => res.opt_def_id(),
1207+
};
1208+
let child_doc_visible = doc_visible
1209+
&& (did.map_or(true, |did| did.is_local() || !this.tcx.is_doc_hidden(did)));
1210+
11951211
// collect results based on the filter function
11961212
// avoid suggesting anything from the same module in which we are resolving
11971213
// avoid suggesting anything with a hygienic name
@@ -1200,7 +1216,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
12001216
&& in_module != parent_scope.module
12011217
&& !ident.span.normalize_to_macros_2_0().from_expansion()
12021218
{
1203-
let res = name_binding.res();
12041219
if filter_fn(res) {
12051220
// create the path
12061221
let mut segms = if lookup_ident.span.at_least_rust_2018() {
@@ -1214,10 +1229,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
12141229

12151230
segms.push(ast::PathSegment::from_ident(ident));
12161231
let path = Path { span: name_binding.span, segments: segms, tokens: None };
1217-
let did = match res {
1218-
Res::Def(DefKind::Ctor(..), did) => this.tcx.opt_parent(did),
1219-
_ => res.opt_def_id(),
1220-
};
12211232

12221233
if child_accessible {
12231234
// Remove invisible match if exists
@@ -1257,6 +1268,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
12571268
descr: res.descr(),
12581269
path,
12591270
accessible: child_accessible,
1271+
doc_visible: child_doc_visible,
12601272
note,
12611273
via_import,
12621274
});
@@ -1277,7 +1289,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
12771289
// add the module to the lookup
12781290
if seen_modules.insert(module.def_id()) {
12791291
if via_import { &mut worklist_via_import } else { &mut worklist }
1280-
.push((module, path_segments, child_accessible));
1292+
.push((module, path_segments, child_accessible, child_doc_visible));
12811293
}
12821294
}
12831295
}
@@ -2687,8 +2699,26 @@ fn show_candidates(
26872699
Vec::new();
26882700

26892701
candidates.iter().for_each(|c| {
2690-
(if c.accessible { &mut accessible_path_strings } else { &mut inaccessible_path_strings })
2691-
.push((pprust::path_to_string(&c.path), c.descr, c.did, &c.note, c.via_import))
2702+
if c.accessible {
2703+
// Don't suggest `#[doc(hidden)]` items from other crates
2704+
if c.doc_visible {
2705+
accessible_path_strings.push((
2706+
pprust::path_to_string(&c.path),
2707+
c.descr,
2708+
c.did,
2709+
&c.note,
2710+
c.via_import,
2711+
))
2712+
}
2713+
} else {
2714+
inaccessible_path_strings.push((
2715+
pprust::path_to_string(&c.path),
2716+
c.descr,
2717+
c.did,
2718+
&c.note,
2719+
c.via_import,
2720+
))
2721+
}
26922722
});
26932723

26942724
// we want consistent results across executions, but candidates are produced
@@ -2787,9 +2817,7 @@ fn show_candidates(
27872817
err.help(msg);
27882818
}
27892819
true
2790-
} else if !matches!(mode, DiagnosticMode::Import) {
2791-
assert!(!inaccessible_path_strings.is_empty());
2792-
2820+
} else if !(inaccessible_path_strings.is_empty() || matches!(mode, DiagnosticMode::Import)) {
27932821
let prefix = if let DiagnosticMode::Pattern = mode {
27942822
"you might have meant to match on "
27952823
} else {

‎compiler/rustc_resolve/src/late/diagnostics.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2191,15 +2191,20 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
21912191
fn find_module(&mut self, def_id: DefId) -> Option<(Module<'a>, ImportSuggestion)> {
21922192
let mut result = None;
21932193
let mut seen_modules = FxHashSet::default();
2194-
let mut worklist = vec![(self.r.graph_root, ThinVec::new())];
2195-
2196-
while let Some((in_module, path_segments)) = worklist.pop() {
2194+
let root_did = self.r.graph_root.def_id();
2195+
let mut worklist = vec![(
2196+
self.r.graph_root,
2197+
ThinVec::new(),
2198+
root_did.is_local() || !self.r.tcx.is_doc_hidden(root_did),
2199+
)];
2200+
2201+
while let Some((in_module, path_segments, doc_visible)) = worklist.pop() {
21972202
// abort if the module is already found
21982203
if result.is_some() {
21992204
break;
22002205
}
22012206

2202-
in_module.for_each_child(self.r, |_, ident, _, name_binding| {
2207+
in_module.for_each_child(self.r, |r, ident, _, name_binding| {
22032208
// abort if the module is already found or if name_binding is private external
22042209
if result.is_some() || !name_binding.vis.is_visible_locally() {
22052210
return;
@@ -2209,6 +2214,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
22092214
let mut path_segments = path_segments.clone();
22102215
path_segments.push(ast::PathSegment::from_ident(ident));
22112216
let module_def_id = module.def_id();
2217+
let doc_visible = doc_visible
2218+
&& (module_def_id.is_local() || !r.tcx.is_doc_hidden(module_def_id));
22122219
if module_def_id == def_id {
22132220
let path =
22142221
Path { span: name_binding.span, segments: path_segments, tokens: None };
@@ -2219,14 +2226,15 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
22192226
descr: "module",
22202227
path,
22212228
accessible: true,
2229+
doc_visible,
22222230
note: None,
22232231
via_import: false,
22242232
},
22252233
));
22262234
} else {
22272235
// add the module to the lookup
22282236
if seen_modules.insert(module_def_id) {
2229-
worklist.push((module, path_segments));
2237+
worklist.push((module, path_segments, doc_visible));
22302238
}
22312239
}
22322240
}

‎src/tools/clippy/tests/ui/crashes/ice-6252.stderr

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ help: consider importing one of these items
88
|
99
LL + use core::marker::PhantomData;
1010
|
11-
LL + use serde::__private::PhantomData;
12-
|
1311
LL + use std::marker::PhantomData;
1412
|
1513

‎tests/ui/suggestions/auxiliary/hidden-struct.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#[doc(hidden)]
2+
pub mod hidden {
3+
pub struct Foo;
4+
}
5+
6+
pub mod hidden1 {
7+
#[doc(hidden)]
8+
pub struct Foo;
9+
}
10+
11+
12+
#[doc(hidden)]
13+
pub(crate) mod hidden2 {
14+
pub struct Bar;
15+
}
16+
17+
pub use hidden2::Bar;

‎tests/ui/suggestions/dont-suggest-foreign-doc-hidden.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// aux-build:hidden-struct.rs
2+
// compile-flags: --crate-type lib
3+
4+
extern crate hidden_struct;
5+
6+
#[doc(hidden)]
7+
mod local {
8+
pub struct Foo;
9+
}
10+
11+
pub fn test(_: Foo) {}
12+
//~^ ERROR cannot find type `Foo` in this scope
13+
14+
pub fn test2(_: Bar) {}
15+
//~^ ERROR cannot find type `Bar` in this scope

‎tests/ui/suggestions/dont-suggest-foreign-doc-hidden.stderr

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
error[E0412]: cannot find type `Foo` in this scope
2+
--> $DIR/dont-suggest-foreign-doc-hidden.rs:11:16
3+
|
4+
LL | pub fn test(_: Foo) {}
5+
| ^^^ not found in this scope
6+
|
7+
help: consider importing this struct
8+
|
9+
LL + use local::Foo;
10+
|
11+
12+
error[E0412]: cannot find type `Bar` in this scope
13+
--> $DIR/dont-suggest-foreign-doc-hidden.rs:14:17
14+
|
15+
LL | pub fn test2(_: Bar) {}
16+
| ^^^ not found in this scope
17+
|
18+
help: consider importing this struct
19+
|
20+
LL + use hidden_struct::Bar;
21+
|
22+
23+
error: aborting due to 2 previous errors
24+
25+
For more information about this error, try `rustc --explain E0412`.

‎tests/ui/type-alias-impl-trait/nested-impl-trait-in-tait.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
#![feature(type_alias_impl_trait)]
22

3-
pub type Tait = impl Iterator<Item = (&'db Key, impl Iterator)>;
3+
pub type Tait = impl Iterator<Item = (&'db LocalKey, impl Iterator)>;
44
//~^ ERROR use of undeclared lifetime name `'db`
5-
//~| ERROR cannot find type `Key` in this scope
5+
//~| ERROR cannot find type `LocalKey` in this scope
66
//~| ERROR unconstrained opaque type
77
//~| ERROR unconstrained opaque type
88

‎tests/ui/type-alias-impl-trait/nested-impl-trait-in-tait.stderr

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,43 @@
11
error[E0261]: use of undeclared lifetime name `'db`
22
--> $DIR/nested-impl-trait-in-tait.rs:3:40
33
|
4-
LL | pub type Tait = impl Iterator<Item = (&'db Key, impl Iterator)>;
4+
LL | pub type Tait = impl Iterator<Item = (&'db LocalKey, impl Iterator)>;
55
| ^^^ undeclared lifetime
66
|
77
= note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
88
help: consider making the bound lifetime-generic with a new `'db` lifetime
99
|
10-
LL | pub type Tait = impl for<'db> Iterator<Item = (&'db Key, impl Iterator)>;
10+
LL | pub type Tait = impl for<'db> Iterator<Item = (&'db LocalKey, impl Iterator)>;
1111
| ++++++++
1212
help: consider introducing lifetime `'db` here
1313
|
14-
LL | pub type Tait<'db> = impl Iterator<Item = (&'db Key, impl Iterator)>;
14+
LL | pub type Tait<'db> = impl Iterator<Item = (&'db LocalKey, impl Iterator)>;
1515
| +++++
1616

17-
error[E0412]: cannot find type `Key` in this scope
17+
error[E0412]: cannot find type `LocalKey` in this scope
1818
--> $DIR/nested-impl-trait-in-tait.rs:3:44
1919
|
20-
LL | pub type Tait = impl Iterator<Item = (&'db Key, impl Iterator)>;
21-
| ^^^ not found in this scope
20+
LL | pub type Tait = impl Iterator<Item = (&'db LocalKey, impl Iterator)>;
21+
| ^^^^^^^^ not found in this scope
2222
|
2323
help: consider importing this struct
2424
|
25-
LL + use std::thread::local_impl::Key;
25+
LL + use std::thread::LocalKey;
2626
|
2727

2828
error: unconstrained opaque type
2929
--> $DIR/nested-impl-trait-in-tait.rs:3:17
3030
|
31-
LL | pub type Tait = impl Iterator<Item = (&'db Key, impl Iterator)>;
32-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
31+
LL | pub type Tait = impl Iterator<Item = (&'db LocalKey, impl Iterator)>;
32+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3333
|
3434
= note: `Tait` must be used in combination with a concrete type within the same module
3535

3636
error: unconstrained opaque type
37-
--> $DIR/nested-impl-trait-in-tait.rs:3:49
37+
--> $DIR/nested-impl-trait-in-tait.rs:3:54
3838
|
39-
LL | pub type Tait = impl Iterator<Item = (&'db Key, impl Iterator)>;
40-
| ^^^^^^^^^^^^^
39+
LL | pub type Tait = impl Iterator<Item = (&'db LocalKey, impl Iterator)>;
40+
| ^^^^^^^^^^^^^
4141
|
4242
= note: `Tait` must be used in combination with a concrete type within the same module
4343

0 commit comments

Comments
 (0)