From 5c0e62cd3e3df41802b73465e03aeca541aa4d00 Mon Sep 17 00:00:00 2001 From: Jules Bertholet Date: Tue, 19 Dec 2023 22:52:04 -0500 Subject: [PATCH] Hide foreign `#[doc(hidden)]` paths in import suggestions --- compiler/rustc_middle/src/ty/util.rs | 2 +- compiler/rustc_resolve/src/diagnostics.rs | 54 ++++++++++++++----- .../rustc_resolve/src/late/diagnostics.rs | 18 +++++-- .../clippy/tests/ui/crashes/ice-6252.stderr | 2 - .../ui/suggestions/auxiliary/hidden-struct.rs | 17 ++++++ .../dont-suggest-foreign-doc-hidden.rs | 15 ++++++ .../dont-suggest-foreign-doc-hidden.stderr | 25 +++++++++ .../nested-impl-trait-in-tait.rs | 4 +- .../nested-impl-trait-in-tait.stderr | 24 ++++----- 9 files changed, 126 insertions(+), 35 deletions(-) create mode 100644 tests/ui/suggestions/auxiliary/hidden-struct.rs create mode 100644 tests/ui/suggestions/dont-suggest-foreign-doc-hidden.rs create mode 100644 tests/ui/suggestions/dont-suggest-foreign-doc-hidden.stderr diff --git a/compiler/rustc_middle/src/ty/util.rs b/compiler/rustc_middle/src/ty/util.rs index 8b2b76764e646..6845de38ba3fd 100644 --- a/compiler/rustc_middle/src/ty/util.rs +++ b/compiler/rustc_middle/src/ty/util.rs @@ -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()) diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs index 542aff69e3459..0c8e70366b95a 100644 --- a/compiler/rustc_resolve/src/diagnostics.rs +++ b/compiler/rustc_resolve/src/diagnostics.rs @@ -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, @@ -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::::new(), true)]; + let start_did = start_module.def_id(); + let mut worklist = vec![( + start_module, + ThinVec::::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), } { @@ -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 @@ -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() { @@ -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 @@ -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, }); @@ -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)); } } } @@ -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 @@ -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 { diff --git a/compiler/rustc_resolve/src/late/diagnostics.rs b/compiler/rustc_resolve/src/late/diagnostics.rs index d767ed74139ba..54d78cfd47d2b 100644 --- a/compiler/rustc_resolve/src/late/diagnostics.rs +++ b/compiler/rustc_resolve/src/late/diagnostics.rs @@ -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; @@ -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 }; @@ -2222,6 +2229,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { descr: "module", path, accessible: true, + doc_visible, note: None, via_import: false, }, @@ -2229,7 +2237,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> { } 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)); } } } diff --git a/src/tools/clippy/tests/ui/crashes/ice-6252.stderr b/src/tools/clippy/tests/ui/crashes/ice-6252.stderr index f929bec9583c5..f6d0976091c53 100644 --- a/src/tools/clippy/tests/ui/crashes/ice-6252.stderr +++ b/src/tools/clippy/tests/ui/crashes/ice-6252.stderr @@ -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; | diff --git a/tests/ui/suggestions/auxiliary/hidden-struct.rs b/tests/ui/suggestions/auxiliary/hidden-struct.rs new file mode 100644 index 0000000000000..30d69acac2097 --- /dev/null +++ b/tests/ui/suggestions/auxiliary/hidden-struct.rs @@ -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; diff --git a/tests/ui/suggestions/dont-suggest-foreign-doc-hidden.rs b/tests/ui/suggestions/dont-suggest-foreign-doc-hidden.rs new file mode 100644 index 0000000000000..779a0c43c02b2 --- /dev/null +++ b/tests/ui/suggestions/dont-suggest-foreign-doc-hidden.rs @@ -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 diff --git a/tests/ui/suggestions/dont-suggest-foreign-doc-hidden.stderr b/tests/ui/suggestions/dont-suggest-foreign-doc-hidden.stderr new file mode 100644 index 0000000000000..7fb4d95ff9bf5 --- /dev/null +++ b/tests/ui/suggestions/dont-suggest-foreign-doc-hidden.stderr @@ -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`. diff --git a/tests/ui/type-alias-impl-trait/nested-impl-trait-in-tait.rs b/tests/ui/type-alias-impl-trait/nested-impl-trait-in-tait.rs index fec0fdc46fbb6..6a74d1dc4ef38 100644 --- a/tests/ui/type-alias-impl-trait/nested-impl-trait-in-tait.rs +++ b/tests/ui/type-alias-impl-trait/nested-impl-trait-in-tait.rs @@ -1,8 +1,8 @@ #![feature(type_alias_impl_trait)] -pub type Tait = impl Iterator; +pub type Tait = 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 diff --git a/tests/ui/type-alias-impl-trait/nested-impl-trait-in-tait.stderr b/tests/ui/type-alias-impl-trait/nested-impl-trait-in-tait.stderr index d4aeace4ae707..ca15b134a9947 100644 --- a/tests/ui/type-alias-impl-trait/nested-impl-trait-in-tait.stderr +++ b/tests/ui/type-alias-impl-trait/nested-impl-trait-in-tait.stderr @@ -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; +LL | pub type Tait = 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; +LL | pub type Tait = impl for<'db> Iterator; | ++++++++ help: consider introducing lifetime `'db` here | -LL | pub type Tait<'db> = impl Iterator; +LL | pub type Tait<'db> = 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; - | ^^^ not found in this scope +LL | pub type Tait = 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; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | pub type Tait = 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; - | ^^^^^^^^^^^^^ +LL | pub type Tait = impl Iterator; + | ^^^^^^^^^^^^^ | = note: `Tait` must be used in combination with a concrete type within the same module