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

Add a visibility suggestion in private-in-public errors #113983

Closed
wants to merge 5 commits into from
Closed
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
1 change: 1 addition & 0 deletions compiler/rustc_privacy/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ privacy_from_private_dep_in_public_interface =
privacy_in_public_interface = {$vis_descr} {$kind} `{$descr}` in public interface
.label = can't leak {$vis_descr} {$kind}
.visibility_label = `{$descr}` declared as {$vis_descr}
.suggestion = consider adding `{$vis_sugg}` in front of it

privacy_item_is_private = {$kind} `{$descr}` is private
.label = private {$kind}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_privacy/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ pub struct UnnamedItemIsPrivate {
}

#[derive(Diagnostic)]
#[help]
#[diag(privacy_in_public_interface, code = E0446)]
pub struct InPublicInterface<'a> {
#[primary_span]
Expand All @@ -58,6 +59,9 @@ pub struct InPublicInterface<'a> {
pub descr: DiagArgFromDisplay<'a>,
#[label(privacy_visibility_label)]
pub vis_span: Span,
#[suggestion(code = "{vis_sugg}", applicability = "maybe-incorrect", style = "verbose")]
pub data: Span,
pub vis_sugg: &'static str,
}

#[derive(Diagnostic)]
Expand Down
19 changes: 19 additions & 0 deletions compiler/rustc_privacy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1337,12 +1337,31 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> {
}
};

// FIXME: this code was adapted from the above `vis_descr` computation,
// but it's not clear if it's correct.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think fn vis_to_string should be usable here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure it will work. I tried to

  • add rustc_ast_pretty = { path = "../rustc_ast_pretty" } to Cargo.toml
  • use it with let vis_sugg = rustc_ast_pretty::pprust::vis_to_string(&self.required_visibility);

But the ty::Visibility that I have here -- enum { Default, Hidden, Protected } is not the same as the one vis_to_string expects -- a struct ast::Visibility with kind/span/tokens. Is there a way i can get the struct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant ty::Visibility::to_string, not vis_to_string from rustc_ast_pretty, sorry.

ty::Visibility::to_string was also called vis_to_string when this comment was written, but it was renamed in #115993.

let vis_sugg = match self.required_visibility {
ty::Visibility::Public => "pub",
ty::Visibility::Restricted(vis_def_id) => {
if vis_def_id
== self.tcx.parent_module_from_def_id(local_def_id).to_local_def_id()
{
"???FIXME???"
} else if vis_def_id.is_top_level_module() {
"pub(crate)"
} else {
"???FIXME???"
}
}
};

self.tcx.dcx().emit_err(InPublicInterface {
span,
vis_descr,
kind,
descr: descr.into(),
vis_span,
data: vis_span,
vis_sugg,
});
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ LL | type AssocTy = Const<{ my_const_fn(U) }>;
...
LL | const fn my_const_fn(val: u8) -> u8 {
| ----------------------------------- `fn(u8) -> u8 {my_const_fn}` declared as private
|
help: consider adding `pub` in front of it
|
LL | pub {
| ~~~

error: aborting due to 1 previous error

Expand Down
10 changes: 10 additions & 0 deletions tests/ui/error-codes/E0446.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ LL | struct Bar;
...
LL | type Alias1 = Bar;
| ^^^^^^^^^^^ can't leak private type
|
help: consider adding `pub` in front of it
|
LL | pub;
| ~~~

error[E0446]: private trait `PrivTr` in public interface
--> $DIR/E0446.rs:11:5
Expand All @@ -15,6 +20,11 @@ LL | trait PrivTr {}
...
LL | type Alias2 = Box<dyn PrivTr>;
| ^^^^^^^^^^^ can't leak private trait
|
help: consider adding `pub` in front of it
|
LL | pub {}
| ~~~

error: aborting due to 2 previous errors

Expand Down
10 changes: 10 additions & 0 deletions tests/ui/privacy/issue-30079.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ LL | struct Priv;
LL | impl ::std::ops::Deref for ::SemiPriv {
LL | type Target = Priv;
| ^^^^^^^^^^^ can't leak private type
|
help: consider adding `pub(crate)` in front of it
|
LL | pub(crate);
| ~~~~~~~~~~

error[E0446]: private type `m3::Priv` in public interface
--> $DIR/issue-30079.rs:34:9
Expand All @@ -28,6 +33,11 @@ LL | struct Priv;
LL | impl ::SemiPrivTrait for () {
LL | type Assoc = Priv;
| ^^^^^^^^^^ can't leak private type
|
help: consider adding `pub(crate)` in front of it
|
LL | pub(crate);
| ~~~~~~~~~~

error: aborting due to 2 previous errors; 1 warning emitted

Expand Down
20 changes: 20 additions & 0 deletions tests/ui/privacy/private-in-public-assoc-ty.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ LL | struct Priv;
...
LL | type A = Priv;
| ^^^^^^ can't leak private type
|
help: consider adding `pub` in front of it
|
LL | pub;
| ~~~

warning: trait `PrivTr` is more private than the item `PubTr::Alias1`
--> $DIR/private-in-public-assoc-ty.rs:24:9
Expand Down Expand Up @@ -52,6 +57,11 @@ LL | struct Priv;
...
LL | type Alias4 = Priv;
| ^^^^^^^^^^^ can't leak private type
|
help: consider adding `pub` in front of it
|
LL | pub;
| ~~~

error[E0446]: private type `Priv` in public interface
--> $DIR/private-in-public-assoc-ty.rs:38:9
Expand All @@ -61,6 +71,11 @@ LL | struct Priv;
...
LL | type Alias1 = Priv;
| ^^^^^^^^^^^ can't leak private type
|
help: consider adding `pub` in front of it
|
LL | pub;
| ~~~

error[E0446]: private trait `PrivTr` in public interface
--> $DIR/private-in-public-assoc-ty.rs:41:9
Expand All @@ -70,6 +85,11 @@ LL | trait PrivTr {}
...
LL | type Exist = impl PrivTr;
| ^^^^^^^^^^ can't leak private trait
|
help: consider adding `pub` in front of it
|
LL | pub {}
| ~~~

error: aborting due to 4 previous errors; 3 warnings emitted

Expand Down
35 changes: 35 additions & 0 deletions tests/ui/privacy/private-in-public-warn.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ LL | struct Priv;
...
LL | type Alias = Priv;
| ^^^^^^^^^^ can't leak private type
|
help: consider adding `pub` in front of it
|
LL | pub;
| ~~~

error: type `types::Priv` is more private than the item `Tr::f1`
--> $DIR/private-in-public-warn.rs:23:9
Expand Down Expand Up @@ -128,6 +133,11 @@ LL | struct Priv;
...
LL | type Alias = Priv;
| ^^^^^^^^^^ can't leak private type
|
help: consider adding `pub` in front of it
|
LL | pub;
| ~~~

error: trait `traits::PrivTr` is more private than the item `traits::Alias`
--> $DIR/private-in-public-warn.rs:41:5
Expand Down Expand Up @@ -310,6 +320,11 @@ LL | struct Priv;
...
LL | type Alias = Priv;
| ^^^^^^^^^^ can't leak private type
|
help: consider adding `pub` in front of it
|
LL | pub;
| ~~~

error: type `aliases_pub::Priv` is more private than the item `aliases_pub::<impl Pub2>::f`
--> $DIR/private-in-public-warn.rs:180:9
Expand All @@ -331,6 +346,11 @@ LL | struct Priv;
...
LL | type Check = Priv;
| ^^^^^^^^^^ can't leak private type
|
help: consider adding `pub` in front of it
|
LL | pub;
| ~~~

error[E0446]: private type `aliases_pub::Priv` in public interface
--> $DIR/private-in-public-warn.rs:186:9
Expand All @@ -340,6 +360,11 @@ LL | struct Priv;
...
LL | type Check = Priv;
| ^^^^^^^^^^ can't leak private type
|
help: consider adding `pub` in front of it
|
LL | pub;
| ~~~

error[E0446]: private type `aliases_pub::Priv` in public interface
--> $DIR/private-in-public-warn.rs:189:9
Expand All @@ -349,6 +374,11 @@ LL | struct Priv;
...
LL | type Check = Priv;
| ^^^^^^^^^^ can't leak private type
|
help: consider adding `pub` in front of it
|
LL | pub;
| ~~~

error[E0446]: private type `aliases_pub::Priv` in public interface
--> $DIR/private-in-public-warn.rs:192:9
Expand All @@ -358,6 +388,11 @@ LL | struct Priv;
...
LL | type Check = Priv;
| ^^^^^^^^^^ can't leak private type
|
help: consider adding `pub` in front of it
|
LL | pub;
| ~~~

error: trait `PrivTr1` is more private than the item `aliases_priv::Tr1`
--> $DIR/private-in-public-warn.rs:222:5
Expand Down
10 changes: 10 additions & 0 deletions tests/ui/privacy/private-inferred-type.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ LL | struct Priv;
...
LL | impl TraitWithAssocTy for u8 { type AssocTy = Priv; }
| ^^^^^^^^^^^^ can't leak private type
|
help: consider adding `pub` in front of it
|
LL | pub;
| ~~~

error[E0446]: private type `S2` in public interface
--> $DIR/private-inferred-type.rs:83:9
Expand All @@ -15,6 +20,11 @@ LL | struct S2;
...
LL | type Target = S2Alias;
| ^^^^^^^^^^^ can't leak private type
|
help: consider adding `pub` in front of it
|
LL | pub;
| ~~~

error: type `Priv` is private
--> $DIR/private-inferred-type.rs:97:9
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/privacy/where-priv-type.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ LL | type AssocTy = Const<{ my_const_fn(U) }>;
...
LL | const fn my_const_fn(val: u8) -> u8 {
| ----------------------------------- `fn(u8) -> u8 {my_const_fn}` declared as private
|
help: consider adding `pub` in front of it
|
LL | pub {
| ~~~

error: aborting due to 1 previous error; 5 warnings emitted

Expand Down
Loading