Add a suggestion to add or remove pub on private-in-public error #112540

aryan-debug · 2023-06-12T05:15:37Z

rustbot · 2023-06-12T05:15:44Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @WaffleLapkin (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

compiler-errors

This could use a better PR title -- somethat that describes what it does, like "add a suggestion to add or remove pub on private-in-public error"
this probably needs the UI tests blessed (./x.py test tests/ui --bless) or if no UI tests changed due to this modification, then it needs a new UI test demonstrating the changes.
Should this be a suggestion instead of a help message? Ideally we'd be suggesting where to add the "pub".

compiler/rustc_privacy/messages.ftl

compiler-errors · 2023-06-12T05:24:16Z

compiler/rustc_privacy/messages.ftl

@@ -8,6 +8,8 @@ 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}
+    .help = - Either remove the `pub` from the function


This says "from the function" -- but this isn't always fired on a function, is it?

Thanks for reviewing my changes. I am not sure if it's always fired on a function, but you are most likely correct. In that case, what would be a better suggestion?

WaffleLapkin · 2023-06-12T15:57:35Z

r? compiler-errors

since they already left a review

nyurik · 2023-06-12T19:11:53Z

I agree that the initially proposed help message does not conform to the same style as other help messages. We have a standard way of making code suggestions that modifies the original code in the needed way, which is better than a text-only help message. So if the original code says struct Foo(i32);, the error message should suggest pub struct Foo(i32); - with the pub shown as the addition (green i think, with some underlying?)

aryan-debug · 2023-06-14T07:07:43Z

@nyurik Sure, we can do that, but I am not sure how, since this is my very first contribution to the rust compiler. So, can you guide me on how I would get the error message to suggest the fix with green underline?

nyurik · 2023-06-14T15:13:56Z

@aryan-debug thx for contributing!

My understanding is that the usual process of a lint/error reporting is that you create one or more suggestions - a Vec<(Span, String)>, where the Span represents the original code you want to modify, and the String is the replacement, and pass them to the DiagnosticBuilder instance before you submit that diagnostic builder to the system.

I'm not even close to being an expert on the Rust compiler, so take with a grain of salt :)

nyurik · 2023-06-14T15:16:59Z

P.S. Search *.rs files for "consider (with a space at the end) - there are quiet a few of the ones with suggestions.

compiler-errors · 2023-06-15T18:45:27Z

Marking this as waiting on author, see this comment for instructions how to switch it back when ready to review: #112540 (comment)

@rustbot author

aryan-debug · 2023-06-19T12:13:12Z

@rustbot review

rust-log-analyzer · 2023-06-19T12:26:52Z

The job x86_64-gnu-llvm-14 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

4 LL | type Foo = (impl Sized, u8);
-    | -------- `Foo` declared as private
+    | --------
+    | |
+    | `Foo` declared as private
+    | help: consider adding `pub` in front of it
6 LL | pub fn foo() -> Foo {
7    | ^^^^^^^^^^^^^^^^^^^ can't leak private type alias


The actual stderr differed from the expected stderr.
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/type-alias-impl-trait/privacy/privacy.stderr
Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/type-alias-impl-trait/privacy/privacy.stderr
To update references, rerun the tests and pass the `--bless` flag
To only update this specific test, also pass `--test-args type-alias-impl-trait/privacy.rs`

error: 1 errors occurred comparing output.
status: exit status: 1
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/type-alias-impl-trait/privacy.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "--sysroot" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Cstrip=debuginfo" "--remap-path-prefix=/checkout/tests/ui=fake-test-src-base" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/type-alias-impl-trait/privacy" "-A" "unused" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/type-alias-impl-trait/privacy/auxiliary"
stdout: none
--- stderr -------------------------------
error[E0446]: private type alias `Foo` in public interface
  --> fake-test-src-base/type-alias-impl-trait/privacy.rs:4:1
LL | type Foo = (impl Sized, u8);
   | --------
   | |
   | |
   | `Foo` declared as private
   | help: consider adding `pub` in front of it
LL | pub fn foo() -> Foo {
   | ^^^^^^^^^^^^^^^^^^^ can't leak private type alias
error: aborting due to previous error

For more information about this error, try `rustc --explain E0446`.
------------------------------------------

compiler-errors · 2023-07-10T16:44:33Z

Still needs tests blessed 😅

@rustbot author

compiler-errors · 2023-07-12T14:16:47Z

No idea how to fix your error without trying it locally, @aryan-debug. Try rebasing your code onto the latest master and blessing your tests again?

compiler-errors · 2023-07-12T14:14:09Z

compiler/rustc_privacy/src/errors.rs

@@ -73,6 +73,8 @@ pub struct InPublicInterface<'a> {
    pub descr: DiagnosticArgFromDisplay<'a>,
    #[label(privacy_visibility_label)]
    pub vis_span: Span,
+    #[suggestion(code = "", applicability = "maybe-incorrect")]


This suggestion isn't actually modifying anything

I am not sure what it should modify and how to do it

compiler-errors · 2023-07-12T14:15:38Z

compiler/rustc_privacy/messages.ftl

@@ -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 `pub` in front of it


this wording is a bit awkward, perhaps "consider making the{$descr} public" or something.

i also don't know if this will unilaterally be applicable. have you tested combinations of items that are already pub(crate) but being exported in pub signatures, for example?

I agree with you so I changed the message to what you suggested.

Also, I am not sure what you mean by that example, sorry

nyurik · 2023-07-23T17:13:57Z

I made a few modifications to this PR in #113983 (since I cannot alter the original one), namely addressing @compiler-errors point about suggesting different visibility modifiers in the help message.

bors · 2023-09-01T15:00:05Z

☔ The latest upstream changes (presumably #113126) made this pull request unmergeable. Please resolve the merge conflicts.

JohnCSimon · 2023-11-12T18:19:15Z

@aryan-debug
ping from triage - can you post your status on this PR? There hasn't been an update in a few months. Thanks!

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

nyurik · 2023-11-12T20:02:38Z

@JohnCSimon this PR was updated/refactored a bit in #113983

Dylan-DPC · 2024-02-10T13:53:37Z

Superseded by #113983

Issue#112284

1941eed

rustbot assigned WaffleLapkin Jun 12, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 12, 2023

compiler-errors requested changes Jun 12, 2023

View reviewed changes

aryan-debug changed the title ~~Issue#112284~~ Add a suggestion to add or remove pub on private-in-public error - Issue#112284 Jun 12, 2023

This comment has been minimized.

Sign in to view

compiler-errors changed the title ~~Add a suggestion to add or remove pub on private-in-public error - Issue#112284~~ Add a suggestion to add or remove pub on private-in-public error Jun 12, 2023

rustbot assigned compiler-errors and unassigned WaffleLapkin Jun 12, 2023

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 15, 2023

aryan-debug added 2 commits June 19, 2023 05:06

Added suggestion instead of help

21d9287

fixed formatting issues

a90b9c8

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 19, 2023

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 10, 2023

compiler-errors reviewed Jul 12, 2023

View reviewed changes

nyurik mentioned this pull request Jul 23, 2023

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

Closed

Dylan-DPC closed this Feb 10, 2024

Dylan-DPC removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Feb 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a suggestion to add or remove pub on private-in-public error #112540

Add a suggestion to add or remove pub on private-in-public error #112540

aryan-debug commented Jun 12, 2023 •

edited

Loading

rustbot commented Jun 12, 2023

compiler-errors left a comment •

edited

Loading

compiler-errors Jun 12, 2023

aryan-debug Jun 12, 2023

This comment has been minimized.

WaffleLapkin commented Jun 12, 2023

nyurik commented Jun 12, 2023 •

edited

Loading

aryan-debug commented Jun 14, 2023

nyurik commented Jun 14, 2023

nyurik commented Jun 14, 2023

compiler-errors commented Jun 15, 2023

aryan-debug commented Jun 19, 2023

rust-log-analyzer commented Jun 19, 2023

compiler-errors commented Jul 10, 2023

compiler-errors commented Jul 12, 2023

compiler-errors Jul 12, 2023

aryan-debug Jul 13, 2023

compiler-errors Jul 12, 2023

aryan-debug Jul 13, 2023

nyurik commented Jul 23, 2023

bors commented Sep 1, 2023

JohnCSimon commented Nov 12, 2023

nyurik commented Nov 12, 2023

Dylan-DPC commented Feb 10, 2024

Add a suggestion to add or remove pub on private-in-public error #112540

Add a suggestion to add or remove pub on private-in-public error #112540

Conversation

aryan-debug commented Jun 12, 2023 • edited Loading

rustbot commented Jun 12, 2023

compiler-errors left a comment • edited Loading

Choose a reason for hiding this comment

compiler-errors Jun 12, 2023

Choose a reason for hiding this comment

aryan-debug Jun 12, 2023

Choose a reason for hiding this comment

This comment has been minimized.

WaffleLapkin commented Jun 12, 2023

nyurik commented Jun 12, 2023 • edited Loading

aryan-debug commented Jun 14, 2023

nyurik commented Jun 14, 2023

nyurik commented Jun 14, 2023

compiler-errors commented Jun 15, 2023

aryan-debug commented Jun 19, 2023

rust-log-analyzer commented Jun 19, 2023

compiler-errors commented Jul 10, 2023

compiler-errors commented Jul 12, 2023

compiler-errors Jul 12, 2023

Choose a reason for hiding this comment

aryan-debug Jul 13, 2023

Choose a reason for hiding this comment

compiler-errors Jul 12, 2023

Choose a reason for hiding this comment

aryan-debug Jul 13, 2023

Choose a reason for hiding this comment

nyurik commented Jul 23, 2023

bors commented Sep 1, 2023

JohnCSimon commented Nov 12, 2023

nyurik commented Nov 12, 2023

Dylan-DPC commented Feb 10, 2024

aryan-debug commented Jun 12, 2023 •

edited

Loading

compiler-errors left a comment •

edited

Loading

nyurik commented Jun 12, 2023 •

edited

Loading