Tweak privacy errors to account for reachable items

Suggest publicly accessible paths for items in private mod:

  When encountering a path in non-import situations that are not reachable
  due to privacy constraints, search for any public re-exports that the
  user could use instead.

Track whether an import suggestion is offering a re-export.

When encountering a path with private segments, mention if the item at
the final path segment is not publicly accessible at all.

Add item visibility metadata to privacy errors from imports:

  On unreachable imports, record the item that was being imported in order
  to suggest publicly available re-exports or to be explicit that the item
  is not available publicly from any path.

  In order to allow this, we add a mode to `resolve_path` that will not
  add new privacy errors, nor return early if it encounters one. This way
  we can get the `Res` corresponding to the final item in the import,
  which is used in the privacy error machinery.
This commit is contained in:
Esteban Küber 2023-06-16 19:44:06 +00:00
parent 717c481739
commit 7dffd24da5
26 changed files with 276 additions and 54 deletions

View File

@ -103,6 +103,7 @@ pub(crate) struct ImportSuggestion {
pub descr: &'static str,
pub path: Path,
pub accessible: bool,
pub via_import: bool,
/// An extra note that should be issued if this item is suggested
pub note: Option<String>,
}
@ -140,9 +141,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
let mut reported_spans = FxHashSet::default();
for error in &self.privacy_errors {
for error in std::mem::take(&mut self.privacy_errors) {
if reported_spans.insert(error.dedup_span) {
self.report_privacy_error(error);
self.report_privacy_error(&error);
}
}
}
@ -1256,6 +1257,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
path,
accessible: child_accessible,
note,
via_import,
});
}
}
@ -1609,8 +1611,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
None
}
fn report_privacy_error(&self, privacy_error: &PrivacyError<'_>) {
let PrivacyError { ident, binding, .. } = *privacy_error;
fn report_privacy_error(&mut self, privacy_error: &PrivacyError<'a>) {
let PrivacyError { ident, binding, outermost_res, parent_scope, dedup_span } =
*privacy_error;
let res = binding.res();
let ctor_fields_span = self.ctor_fields_span(binding);
@ -1627,6 +1630,33 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
struct_span_err!(self.tcx.sess, ident.span, E0603, "{} `{}` is private", descr, ident);
err.span_label(ident.span, format!("private {}", descr));
if let Some((this_res, outer_ident)) = outermost_res {
let import_suggestions = self.lookup_import_candidates(
outer_ident,
this_res.ns().unwrap_or(Namespace::TypeNS),
&parent_scope,
&|res: Res| res == this_res,
);
let point_to_def = !show_candidates(
self.tcx,
&mut err,
Some(dedup_span.until(outer_ident.span.shrink_to_hi())),
&import_suggestions,
Instead::Yes,
FoundUse::Yes,
DiagnosticMode::Import,
vec![],
"",
);
// If we suggest importing a public re-export, don't point at the definition.
if point_to_def && ident.span != outer_ident.span {
err.span_label(
outer_ident.span,
format!("{} `{outer_ident}` is not publicly re-exported", this_res.descr()),
);
}
}
let mut non_exhaustive = None;
// If an ADT is foreign and marked as `non_exhaustive`, then that's
// probably why we have the privacy error.
@ -2455,7 +2485,8 @@ pub(crate) fn import_candidates(
/// When an entity with a given name is not available in scope, we search for
/// entities with that name in all crates. This method allows outputting the
/// results of this search in a programmer-friendly way
/// results of this search in a programmer-friendly way. If any entities are
/// found and suggested, returns `true`, otherwise returns `false`.
fn show_candidates(
tcx: TyCtxt<'_>,
err: &mut Diagnostic,
@ -2467,19 +2498,19 @@ fn show_candidates(
mode: DiagnosticMode,
path: Vec<Segment>,
append: &str,
) {
) -> bool {
if candidates.is_empty() {
return;
return false;
}
let mut accessible_path_strings: Vec<(String, &str, Option<DefId>, &Option<String>)> =
let mut accessible_path_strings: Vec<(String, &str, Option<DefId>, &Option<String>, bool)> =
Vec::new();
let mut inaccessible_path_strings: Vec<(String, &str, Option<DefId>, &Option<String>)> =
let mut inaccessible_path_strings: Vec<(String, &str, Option<DefId>, &Option<String>, bool)> =
Vec::new();
candidates.iter().for_each(|c| {
(if c.accessible { &mut accessible_path_strings } else { &mut inaccessible_path_strings })
.push((path_names_to_string(&c.path), c.descr, c.did, &c.note))
.push((path_names_to_string(&c.path), c.descr, c.did, &c.note, c.via_import))
});
// we want consistent results across executions, but candidates are produced
@ -2493,20 +2524,25 @@ fn show_candidates(
}
if !accessible_path_strings.is_empty() {
let (determiner, kind, name) = if accessible_path_strings.len() == 1 {
("this", accessible_path_strings[0].1, format!(" `{}`", accessible_path_strings[0].0))
} else {
("one of these", "items", String::new())
};
let (determiner, kind, name, through) =
if let [(name, descr, _, _, via_import)] = &accessible_path_strings[..] {
(
"this",
*descr,
format!(" `{name}`"),
if *via_import { " through its public re-export" } else { "" },
)
} else {
("one of these", "items", String::new(), "")
};
let instead = if let Instead::Yes = instead { " instead" } else { "" };
let mut msg = if let DiagnosticMode::Pattern = mode {
format!(
"if you meant to match on {}{}{}, use the full path in the pattern",
kind, instead, name
"if you meant to match on {kind}{instead}{name}, use the full path in the pattern",
)
} else {
format!("consider importing {} {}{}", determiner, kind, instead)
format!("consider importing {determiner} {kind}{through}{instead}")
};
for note in accessible_path_strings.iter().flat_map(|cand| cand.3.as_ref()) {
@ -2522,7 +2558,7 @@ fn show_candidates(
accessible_path_strings.into_iter().map(|a| a.0),
Applicability::MaybeIncorrect,
);
return;
return true;
}
DiagnosticMode::Import => ("", ""),
DiagnosticMode::Normal => ("use ", ";\n"),
@ -2563,6 +2599,7 @@ fn show_candidates(
err.help(msg);
}
true
} else if !matches!(mode, DiagnosticMode::Import) {
assert!(!inaccessible_path_strings.is_empty());
@ -2571,13 +2608,9 @@ fn show_candidates(
} else {
""
};
if inaccessible_path_strings.len() == 1 {
let (name, descr, def_id, note) = &inaccessible_path_strings[0];
if let [(name, descr, def_id, note, _)] = &inaccessible_path_strings[..] {
let msg = format!(
"{}{} `{}`{} exists but is inaccessible",
prefix,
descr,
name,
"{prefix}{descr} `{name}`{} exists but is inaccessible",
if let DiagnosticMode::Pattern = mode { ", which" } else { "" }
);
@ -2594,11 +2627,11 @@ fn show_candidates(
err.note(note.to_string());
}
} else {
let (_, descr_first, _, _) = &inaccessible_path_strings[0];
let (_, descr_first, _, _, _) = &inaccessible_path_strings[0];
let descr = if inaccessible_path_strings
.iter()
.skip(1)
.all(|(_, descr, _, _)| descr == descr_first)
.all(|(_, descr, _, _, _)| descr == descr_first)
{
descr_first
} else {
@ -2611,7 +2644,7 @@ fn show_candidates(
let mut has_colon = false;
let mut spans = Vec::new();
for (name, _, def_id, _) in &inaccessible_path_strings {
for (name, _, def_id, _, _) in &inaccessible_path_strings {
if let Some(local_def_id) = def_id.and_then(|did| did.as_local()) {
let span = tcx.source_span(local_def_id);
let span = tcx.sess.source_map().guess_head_span(span);
@ -2637,6 +2670,9 @@ fn show_candidates(
err.span_note(multi_span, msg);
}
true
} else {
false
}
}

View File

@ -893,6 +893,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
ident,
binding,
dedup_span: path_span,
outermost_res: None,
parent_scope: *parent_scope,
});
} else {
return Err((Determined, Weak::No));
@ -1369,6 +1371,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
let mut allow_super = true;
let mut second_binding = None;
// We'll provide more context to the privacy errors later, up to `len`.
let privacy_errors_len = self.privacy_errors.len();
for (segment_idx, &Segment { ident, id, .. }) in path.iter().enumerate() {
debug!("resolve_path ident {} {:?} {:?}", segment_idx, ident, id);
let record_segment_res = |this: &mut Self, res| {
@ -1506,6 +1511,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
second_binding = Some(binding);
}
let res = binding.res();
// Mark every privacy error in this path with the res to the last element. This allows us
// to detect the item the user cares about and either find an alternative import, or tell
// the user it is not accessible.
for error in &mut self.privacy_errors[privacy_errors_len..] {
error.outermost_res = Some((res, ident));
}
let maybe_assoc = opt_ns != Some(MacroNS) && PathSource::Type.is_expected(res);
if let Some(next_module) = binding.module() {
module = Some(ModuleOrUniformRoot::Module(next_module));

View File

@ -792,6 +792,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
};
let prev_ambiguity_errors_len = self.ambiguity_errors.len();
let finalize = Finalize::with_root_span(import.root_id, import.span, import.root_span);
// We'll provide more context to the privacy errors later, up to `len`.
let privacy_errors_len = self.privacy_errors.len();
let path_res = self.resolve_path(
&import.module_path,
None,
@ -931,6 +935,21 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
_ => unreachable!(),
};
if self.privacy_errors.len() != privacy_errors_len {
// Get the Res for the last element, so that we can point to alternative ways of
// importing it if available.
let mut path = import.module_path.clone();
path.push(Segment::from_ident(ident));
if let PathResult::Module(ModuleOrUniformRoot::Module(module)) =
self.resolve_path(&path, None, &import.parent_scope, Some(finalize), ignore_binding)
{
let res = module.res().map(|r| (r, ident));
for error in &mut self.privacy_errors[privacy_errors_len..] {
error.outermost_res = res;
}
}
}
let mut all_ns_err = true;
self.per_ns(|this, ns| {
if !type_ns_only || ns == TypeNS {

View File

@ -1831,6 +1831,7 @@ impl<'a: 'ast, 'ast, 'tcx> LateResolutionVisitor<'a, '_, 'ast, 'tcx> {
path,
accessible: true,
note: None,
via_import: false,
},
));
} else {

View File

@ -689,10 +689,13 @@ impl<'a> NameBindingKind<'a> {
}
}
#[derive(Debug)]
struct PrivacyError<'a> {
ident: Ident,
binding: &'a NameBinding<'a>,
dedup_span: Span,
outermost_res: Option<(Res, Ident)>,
parent_scope: ParentScope<'a>,
}
#[derive(Debug)]

View File

@ -9,6 +9,10 @@ note: the crate import `core` is defined here
|
LL | extern crate core;
| ^^^^^^^^^^^^^^^^^^
help: consider importing this module instead
|
LL | use std::cell;
| ~~~~~~~~~
error[E0603]: crate import `core` is private
--> $DIR/extern-crate-visibility.rs:9:10
@ -21,6 +25,10 @@ note: the crate import `core` is defined here
|
LL | extern crate core;
| ^^^^^^^^^^^^^^^^^^
help: consider importing this struct instead
|
LL | std::cell::Cell::new(0);
| ~~~~~~~~~~~~~~~
error: aborting due to 2 previous errors

View File

@ -2,7 +2,9 @@ error[E0603]: enum `Foo` is private
--> $DIR/issue-11680.rs:6:21
|
LL | let _b = other::Foo::Bar(1);
| ^^^ private enum
| ^^^ --- tuple variant `Bar` is not publicly re-exported
| |
| private enum
|
note: the enum `Foo` is defined here
--> $DIR/auxiliary/issue-11680.rs:1:1
@ -14,7 +16,9 @@ error[E0603]: enum `Foo` is private
--> $DIR/issue-11680.rs:9:27
|
LL | let _b = other::test::Foo::Bar(1);
| ^^^ private enum
| ^^^ --- tuple variant `Bar` is not publicly re-exported
| |
| private enum
|
note: the enum `Foo` is defined here
--> $DIR/auxiliary/issue-11680.rs:6:5

View File

@ -4,7 +4,7 @@ error: cannot find macro `bla` in this scope
LL | bla!();
| ^^^
|
help: consider importing this macro
help: consider importing this macro through its public re-export
|
LL + use crate::hey::bla;
|
@ -23,7 +23,7 @@ error: cannot find derive macro `Bla` in this scope
LL | #[derive(Bla)]
| ^^^
|
help: consider importing this derive macro
help: consider importing this derive macro through its public re-export
|
LL + use crate::hey::Bla;
|

View File

@ -2,7 +2,9 @@ error[E0603]: enum `Y` is private
--> $DIR/export-tag-variant.rs:7:26
|
LL | fn main() { let z = foo::Y::Y1; }
| ^ private enum
| ^ -- unit variant `Y1` is not publicly re-exported
| |
| private enum
|
note: the enum `Y` is defined here
--> $DIR/export-tag-variant.rs:4:5

View File

@ -2,7 +2,9 @@ error[E0603]: module `bar` is private
--> $DIR/privacy-in-paths.rs:24:16
|
LL | ::foo::bar::baz::f();
| ^^^ private module
| ^^^ - function `f` is not publicly re-exported
| |
| private module
|
note: the module `bar` is defined here
--> $DIR/privacy-in-paths.rs:3:5
@ -21,12 +23,18 @@ note: the module `bar` is defined here
|
LL | mod bar {
| ^^^^^^^
help: consider importing this struct through its public re-export instead
|
LL | foo::S::f();
| ~~~~~~
error[E0603]: trait `T` is private
--> $DIR/privacy-in-paths.rs:26:23
|
LL | <() as ::foo::T>::Assoc::f();
| ^ private trait
| ^ ----- associated type `Assoc` is not publicly re-exported
| |
| private trait
|
note: the trait `T` is defined here
--> $DIR/privacy-in-paths.rs:8:5

View File

@ -2,7 +2,9 @@ error[E0603]: trait `Bar` is private
--> $DIR/privacy-ufcs.rs:12:20
|
LL | <i32 as ::foo::Bar>::baz();
| ^^^ private trait
| ^^^ --- associated function `baz` is not publicly re-exported
| |
| private trait
|
note: the trait `Bar` is defined here
--> $DIR/privacy-ufcs.rs:4:5

View File

@ -50,7 +50,9 @@ error[E0603]: module `baz` is private
--> $DIR/privacy1.rs:104:16
|
LL | ::bar::baz::A::foo();
| ^^^ private module
| ^^^ - struct `A` is not publicly re-exported
| |
| private module
|
note: the module `baz` is defined here
--> $DIR/privacy1.rs:50:5
@ -62,7 +64,9 @@ error[E0603]: module `baz` is private
--> $DIR/privacy1.rs:105:16
|
LL | ::bar::baz::A::bar();
| ^^^ private module
| ^^^ - struct `A` is not publicly re-exported
| |
| private module
|
note: the module `baz` is defined here
--> $DIR/privacy1.rs:50:5
@ -74,7 +78,9 @@ error[E0603]: module `baz` is private
--> $DIR/privacy1.rs:107:16
|
LL | ::bar::baz::A.foo2();
| ^^^ private module
| ^^^ - unit struct `A` is not publicly re-exported
| |
| private module
|
note: the module `baz` is defined here
--> $DIR/privacy1.rs:50:5
@ -86,7 +92,9 @@ error[E0603]: module `baz` is private
--> $DIR/privacy1.rs:108:16
|
LL | ::bar::baz::A.bar2();
| ^^^ private module
| ^^^ - unit struct `A` is not publicly re-exported
| |
| private module
|
note: the module `baz` is defined here
--> $DIR/privacy1.rs:50:5
@ -98,7 +106,9 @@ error[E0603]: trait `B` is private
--> $DIR/privacy1.rs:112:16
|
LL | ::bar::B::foo();
| ^ private trait
| ^ --- associated function `foo` is not publicly re-exported
| |
| private trait
|
note: the trait `B` is defined here
--> $DIR/privacy1.rs:40:5
@ -129,6 +139,10 @@ note: the module `baz` is defined here
|
LL | mod baz {
| ^^^^^^^
help: consider importing this function through its public re-export instead
|
LL | bar::foo();
| ~~~~~~~~
error[E0603]: module `baz` is private
--> $DIR/privacy1.rs:128:16
@ -141,6 +155,10 @@ note: the module `baz` is defined here
|
LL | mod baz {
| ^^^^^^^
help: consider importing this function through its public re-export instead
|
LL | bar::bar();
| ~~~~~~~~
error[E0603]: trait `B` is private
--> $DIR/privacy1.rs:157:17

View File

@ -0,0 +1,4 @@
extern crate core;
use core::slice::index::private_slice_index::Sealed; //~ ERROR module `index` is private
fn main() {
}

View File

@ -0,0 +1,12 @@
error[E0603]: module `index` is private
--> $DIR/private-trait-non-local.rs:2:18
|
LL | use core::slice::index::private_slice_index::Sealed;
| ^^^^^ private module ------ trait `Sealed` is not publicly re-exported
|
note: the module `index` is defined here
--> $SRC_DIR/core/src/slice/mod.rs:LL:COL
error: aborting due to previous error
For more information about this error, try `rustc --explain E0603`.

View File

@ -0,0 +1,10 @@
pub mod a {
mod b {
pub trait Hidden {}
}
}
struct S;
impl a::b::Hidden for S {} //~ ERROR module `b` is private
fn main() {}

View File

@ -0,0 +1,17 @@
error[E0603]: module `b` is private
--> $DIR/private-trait.rs:8:9
|
LL | impl a::b::Hidden for S {}
| ^ ------ trait `Hidden` is not publicly re-exported
| |
| private module
|
note: the module `b` is defined here
--> $DIR/private-trait.rs:2:5
|
LL | mod b {
| ^^^^^
error: aborting due to previous error
For more information about this error, try `rustc --explain E0603`.

View File

@ -0,0 +1,13 @@
// run-rustfix
pub mod a {
pub use self::b::Trait;
mod b {
pub trait Trait {}
}
}
struct S;
impl a::Trait for S {} //~ ERROR module `b` is private
fn main() {}

View File

@ -0,0 +1,13 @@
// run-rustfix
pub mod a {
pub use self::b::Trait;
mod b {
pub trait Trait {}
}
}
struct S;
impl a::b::Trait for S {} //~ ERROR module `b` is private
fn main() {}

View File

@ -0,0 +1,19 @@
error[E0603]: module `b` is private
--> $DIR/re-exported-trait.rs:11:9
|
LL | impl a::b::Trait for S {}
| ^ private module
|
note: the module `b` is defined here
--> $DIR/re-exported-trait.rs:5:5
|
LL | mod b {
| ^^^^^
help: consider importing this trait through its public re-export instead
|
LL | impl a::Trait for S {}
| ~~~~~~~~
error: aborting due to previous error
For more information about this error, try `rustc --explain E0603`.

View File

@ -17,7 +17,7 @@ LL | #[derive(GenHelperUse)]
| ^^^^^^^^^^^^
|
= note: this error originates in the derive macro `GenHelperUse` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider importing this attribute macro
help: consider importing this attribute macro through its public re-export
|
LL + use empty_helper;
|
@ -32,7 +32,7 @@ LL | gen_helper_use!();
| ----------------- in this macro invocation
|
= note: this error originates in the macro `gen_helper_use` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider importing this attribute macro
help: consider importing this attribute macro through its public re-export
|
LL + use crate::empty_helper;
|

View File

@ -2,7 +2,7 @@ error[E0603]: module `super_sekrit` is private
--> $DIR/unreachable-variant.rs:6:21
|
LL | let _x = other::super_sekrit::sooper_sekrit::baz;
| ^^^^^^^^^^^^ private module
| ^^^^^^^^^^^^ private module --- unit variant `baz` is not publicly re-exported
|
note: the module `super_sekrit` is defined here
--> $DIR/auxiliary/unreachable_variant.rs:1:1

View File

@ -228,7 +228,9 @@ error[E0603]: enum `Z` is private
--> $DIR/privacy-enum-ctor.rs:61:22
|
LL | let _: Z = m::n::Z::Fn;
| ^ private enum
| ^ -- tuple variant `Fn` is not publicly re-exported
| |
| private enum
|
note: the enum `Z` is defined here
--> $DIR/privacy-enum-ctor.rs:11:9
@ -252,7 +254,9 @@ error[E0603]: enum `Z` is private
--> $DIR/privacy-enum-ctor.rs:68:22
|
LL | let _: Z = m::n::Z::Unit {};
| ^ private enum
| ^ ---- variant `Unit` is not publicly re-exported
| |
| private enum
|
note: the enum `Z` is defined here
--> $DIR/privacy-enum-ctor.rs:11:9

View File

@ -2,7 +2,9 @@ error[E0603]: module `thread_info` is private
--> $DIR/stability-in-private-module.rs:2:26
|
LL | let _ = std::thread::thread_info::current_thread();
| ^^^^^^^^^^^ private module
| ^^^^^^^^^^^ -------------- function `current_thread` is not publicly re-exported
| |
| private module
|
note: the module `thread_info` is defined here
--> $SRC_DIR/std/src/thread/mod.rs:LL:COL

View File

@ -14,7 +14,9 @@ error[E0603]: enum `Bar` is private
--> $DIR/struct-variant-privacy-xc.rs:7:33
|
LL | struct_variant_privacy::Bar::Baz { a: _a } => {}
| ^^^ private enum
| ^^^ --- variant `Baz` is not publicly re-exported
| |
| private enum
|
note: the enum `Bar` is defined here
--> $DIR/auxiliary/struct_variant_privacy.rs:1:1

View File

@ -14,7 +14,9 @@ error[E0603]: enum `Bar` is private
--> $DIR/struct-variant-privacy.rs:10:14
|
LL | foo::Bar::Baz { a: _a } => {}
| ^^^ private enum
| ^^^ --- variant `Baz` is not publicly re-exported
| |
| private enum
|
note: the enum `Bar` is defined here
--> $DIR/struct-variant-privacy.rs:2:5

View File

@ -62,7 +62,9 @@ error[E0603]: module `foo` is private
--> $DIR/xcrate-private-by-default.rs:35:29
|
LL | static_priv_by_default::foo::a;
| ^^^ private module
| ^^^ - static `a` is not publicly re-exported
| |
| private module
|
note: the module `foo` is defined here
--> $DIR/auxiliary/static_priv_by_default.rs:12:1
@ -74,7 +76,9 @@ error[E0603]: module `foo` is private
--> $DIR/xcrate-private-by-default.rs:37:29
|
LL | static_priv_by_default::foo::b;
| ^^^ private module
| ^^^ - function `b` is not publicly re-exported
| |
| private module
|
note: the module `foo` is defined here
--> $DIR/auxiliary/static_priv_by_default.rs:12:1
@ -86,7 +90,9 @@ error[E0603]: module `foo` is private
--> $DIR/xcrate-private-by-default.rs:39:29
|
LL | static_priv_by_default::foo::c;
| ^^^ private module
| ^^^ - unit struct `c` is not publicly re-exported
| |
| private module
|
note: the module `foo` is defined here
--> $DIR/auxiliary/static_priv_by_default.rs:12:1
@ -98,7 +104,9 @@ error[E0603]: module `foo` is private
--> $DIR/xcrate-private-by-default.rs:41:35
|
LL | foo::<static_priv_by_default::foo::d>();
| ^^^ private module
| ^^^ - enum `d` is not publicly re-exported
| |
| private module
|
note: the module `foo` is defined here
--> $DIR/auxiliary/static_priv_by_default.rs:12:1
@ -110,7 +118,9 @@ error[E0603]: module `foo` is private
--> $DIR/xcrate-private-by-default.rs:43:35
|
LL | foo::<static_priv_by_default::foo::e>();
| ^^^ private module
| ^^^ - type alias `e` is not publicly re-exported
| |
| private module
|
note: the module `foo` is defined here
--> $DIR/auxiliary/static_priv_by_default.rs:12:1