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

Hide foreign #[doc(hidden)] paths in import suggestions #119151

Merged
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
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1476,7 +1476,7 @@ pub fn reveal_opaque_types_in_bounds<'tcx>(
val.fold_with(&mut visitor)
}

/// Determines whether an item is annotated with `doc(hidden)`.
/// Determines whether an item is directly annotated with `doc(hidden)`.
fn is_doc_hidden(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {
tcx.get_attrs(def_id, sym::doc)
.filter_map(|attr| attr.meta_item_list())
Expand Down
54 changes: 41 additions & 13 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ pub(crate) struct ImportSuggestion {
pub descr: &'static str,
pub path: Path,
pub accessible: bool,
// false if the path traverses a foreign `#[doc(hidden)]` item.
pub doc_visible: bool,
pub via_import: bool,
/// An extra note that should be issued if this item is suggested
pub note: Option<String>,
Expand Down Expand Up @@ -1153,10 +1155,16 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
{
let mut candidates = Vec::new();
let mut seen_modules = FxHashSet::default();
let mut worklist = vec![(start_module, ThinVec::<ast::PathSegment>::new(), true)];
let start_did = start_module.def_id();
let mut worklist = vec![(
start_module,
ThinVec::<ast::PathSegment>::new(),
true,
start_did.is_local() || !self.tcx.is_doc_hidden(start_did),
)];
let mut worklist_via_import = vec![];

while let Some((in_module, path_segments, accessible)) = match worklist.pop() {
while let Some((in_module, path_segments, accessible, doc_visible)) = match worklist.pop() {
None => worklist_via_import.pop(),
Some(x) => Some(x),
} {
Expand Down Expand Up @@ -1199,6 +1207,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}

let res = name_binding.res();
let did = match res {
Res::Def(DefKind::Ctor(..), did) => this.tcx.opt_parent(did),
_ => res.opt_def_id(),
};
let child_doc_visible = doc_visible
&& (did.map_or(true, |did| did.is_local() || !this.tcx.is_doc_hidden(did)));

// collect results based on the filter function
// avoid suggesting anything from the same module in which we are resolving
// avoid suggesting anything with a hygienic name
Expand All @@ -1207,7 +1223,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
&& in_module != parent_scope.module
&& !ident.span.normalize_to_macros_2_0().from_expansion()
{
let res = name_binding.res();
if filter_fn(res) {
// create the path
let mut segms = if lookup_ident.span.at_least_rust_2018() {
Expand All @@ -1221,10 +1236,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {

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

if child_accessible {
// Remove invisible match if exists
Expand Down Expand Up @@ -1264,6 +1275,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
descr: res.descr(),
path,
accessible: child_accessible,
doc_visible: child_doc_visible,
note,
via_import,
});
Expand All @@ -1284,7 +1296,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// add the module to the lookup
if seen_modules.insert(module.def_id()) {
if via_import { &mut worklist_via_import } else { &mut worklist }
.push((module, path_segments, child_accessible));
.push((module, path_segments, child_accessible, child_doc_visible));
}
}
}
Expand Down Expand Up @@ -2694,8 +2706,26 @@ fn show_candidates(
Vec::new();

candidates.iter().for_each(|c| {
(if c.accessible { &mut accessible_path_strings } else { &mut inaccessible_path_strings })
.push((pprust::path_to_string(&c.path), c.descr, c.did, &c.note, c.via_import))
if c.accessible {
// Don't suggest `#[doc(hidden)]` items from other crates
if c.doc_visible {
accessible_path_strings.push((
pprust::path_to_string(&c.path),
c.descr,
c.did,
&c.note,
c.via_import,
))
}
} else {
inaccessible_path_strings.push((
pprust::path_to_string(&c.path),
c.descr,
c.did,
&c.note,
c.via_import,
))
}
});

// we want consistent results across executions, but candidates are produced
Expand Down Expand Up @@ -2794,9 +2824,7 @@ fn show_candidates(
err.help(msg);
}
true
} else if !matches!(mode, DiagnosticMode::Import) {
assert!(!inaccessible_path_strings.is_empty());

} else if !(inaccessible_path_strings.is_empty() || matches!(mode, DiagnosticMode::Import)) {
let prefix = if let DiagnosticMode::Pattern = mode {
"you might have meant to match on "
} else {
Expand Down
18 changes: 13 additions & 5 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2194,15 +2194,20 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
fn find_module(&mut self, def_id: DefId) -> Option<(Module<'a>, ImportSuggestion)> {
let mut result = None;
let mut seen_modules = FxHashSet::default();
let mut worklist = vec![(self.r.graph_root, ThinVec::new())];

while let Some((in_module, path_segments)) = worklist.pop() {
let root_did = self.r.graph_root.def_id();
let mut worklist = vec![(
self.r.graph_root,
ThinVec::new(),
root_did.is_local() || !self.r.tcx.is_doc_hidden(root_did),
)];

while let Some((in_module, path_segments, doc_visible)) = worklist.pop() {
// abort if the module is already found
if result.is_some() {
break;
}

in_module.for_each_child(self.r, |_, ident, _, name_binding| {
in_module.for_each_child(self.r, |r, ident, _, name_binding| {
// abort if the module is already found or if name_binding is private external
if result.is_some() || !name_binding.vis.is_visible_locally() {
return;
Expand All @@ -2212,6 +2217,8 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
let mut path_segments = path_segments.clone();
path_segments.push(ast::PathSegment::from_ident(ident));
let module_def_id = module.def_id();
let doc_visible = doc_visible
&& (module_def_id.is_local() || !r.tcx.is_doc_hidden(module_def_id));
if module_def_id == def_id {
let path =
Path { span: name_binding.span, segments: path_segments, tokens: None };
Expand All @@ -2222,14 +2229,15 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
descr: "module",
path,
accessible: true,
doc_visible,
note: None,
via_import: false,
},
));
} else {
// add the module to the lookup
if seen_modules.insert(module_def_id) {
worklist.push((module, path_segments));
worklist.push((module, path_segments, doc_visible));
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions src/tools/clippy/tests/ui/crashes/ice-6252.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ help: consider importing one of these items
|
LL + use core::marker::PhantomData;
|
LL + use serde::__private::PhantomData;
|
LL + use std::marker::PhantomData;
|

Expand Down
17 changes: 17 additions & 0 deletions tests/ui/suggestions/auxiliary/hidden-struct.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#[doc(hidden)]
pub mod hidden {
pub struct Foo;
}

pub mod hidden1 {
#[doc(hidden)]
pub struct Foo;
}


#[doc(hidden)]
pub(crate) mod hidden2 {
pub struct Bar;
}

pub use hidden2::Bar;
15 changes: 15 additions & 0 deletions tests/ui/suggestions/dont-suggest-foreign-doc-hidden.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// aux-build:hidden-struct.rs
// compile-flags: --crate-type lib

extern crate hidden_struct;

#[doc(hidden)]
mod local {
pub struct Foo;
}

pub fn test(_: Foo) {}
//~^ ERROR cannot find type `Foo` in this scope

pub fn test2(_: Bar) {}
//~^ ERROR cannot find type `Bar` in this scope
25 changes: 25 additions & 0 deletions tests/ui/suggestions/dont-suggest-foreign-doc-hidden.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
error[E0412]: cannot find type `Foo` in this scope
--> $DIR/dont-suggest-foreign-doc-hidden.rs:11:16
|
LL | pub fn test(_: Foo) {}
| ^^^ not found in this scope
|
help: consider importing this struct
|
LL + use local::Foo;
|

error[E0412]: cannot find type `Bar` in this scope
--> $DIR/dont-suggest-foreign-doc-hidden.rs:14:17
|
LL | pub fn test2(_: Bar) {}
| ^^^ not found in this scope
|
help: consider importing this struct
|
LL + use hidden_struct::Bar;
|

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0412`.
4 changes: 2 additions & 2 deletions tests/ui/type-alias-impl-trait/nested-impl-trait-in-tait.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
#![feature(type_alias_impl_trait)]

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

Expand Down
24 changes: 12 additions & 12 deletions tests/ui/type-alias-impl-trait/nested-impl-trait-in-tait.stderr
Original file line number Diff line number Diff line change
@@ -1,43 +1,43 @@
error[E0261]: use of undeclared lifetime name `'db`
--> $DIR/nested-impl-trait-in-tait.rs:3:40
|
LL | pub type Tait = impl Iterator<Item = (&'db Key, impl Iterator)>;
LL | pub type Tait = impl Iterator<Item = (&'db LocalKey, impl Iterator)>;
| ^^^ undeclared lifetime
|
= note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
help: consider making the bound lifetime-generic with a new `'db` lifetime
|
LL | pub type Tait = impl for<'db> Iterator<Item = (&'db Key, impl Iterator)>;
LL | pub type Tait = impl for<'db> Iterator<Item = (&'db LocalKey, impl Iterator)>;
| ++++++++
help: consider introducing lifetime `'db` here
|
LL | pub type Tait<'db> = impl Iterator<Item = (&'db Key, impl Iterator)>;
LL | pub type Tait<'db> = impl Iterator<Item = (&'db LocalKey, impl Iterator)>;
| +++++

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

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

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

Expand Down
Loading