Skip to content

Commit

Permalink
Unrolled build for rust-lang#119151
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#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
  • Loading branch information
rust-timer authored Jan 5, 2024
2 parents b8c2074 + 5c0e62c commit ed7e1cc
Show file tree
Hide file tree
Showing 9 changed files with 126 additions and 35 deletions.
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 @@ -1483,7 +1483,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 @@ -100,6 +100,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 @@ -1146,10 +1148,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 @@ -1192,6 +1200,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 @@ -1200,7 +1216,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 @@ -1214,10 +1229,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 @@ -1257,6 +1268,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
descr: res.descr(),
path,
accessible: child_accessible,
doc_visible: child_doc_visible,
note,
via_import,
});
Expand All @@ -1277,7 +1289,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 @@ -2687,8 +2699,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 @@ -2787,9 +2817,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 @@ -2191,15 +2191,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 @@ -2209,6 +2214,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 @@ -2219,14 +2226,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

0 comments on commit ed7e1cc

Please sign in to comment.