Stream: t-compiler/rust-analyzer

Topic: visibility inlay hint


pksunkara (Nov 08 2020 at 18:26, on Zulip):

(deleted)

pksunkara (Nov 08 2020 at 18:31, on Zulip):

One of things we do in clap is to put pub(crate) on every private thing. Let me explain why. Assume we have foo.rs

pub fn bar() {}

pub fn baz() {}

We want to expose only baz to end users but still use bar internally in other modules:

mod foo;

pub use foo::baz;

fn uses_bar() {
    foo::bar();
}

In that situation, when we are working on foo.rs, we have no hint about the visibility of the items. Are they visibile to public? Are they visibile to only super? etc.. So, to make it easier for us, we ended up doing pub(crate) on all internal things which we want to use elsewhere in the codebase.

I think this is one of things LSP can do. Provide an inlay hint that can display the visibility reach of each item that has pub.

Florian Diebold (Nov 08 2020 at 18:36, on Zulip):

if I understand you correctly, you might be interested in the unreachable-pub lint

pksunkara (Nov 08 2020 at 18:38, on Zulip):

Yeah, we do have that. But I just don't like it. I think the IDE should display the info of the restriction level of visibility

pksunkara (Nov 08 2020 at 18:38, on Zulip):

Instead of forcing all my contributors to add pub(crate)

bjorn3 (Nov 08 2020 at 18:52, on Zulip):

If you use pub instead of pub(crate) rustc will for all intents and purposes consider it exported. The private-in-public lint won't fire, the missing_docs lint will fire and so on.

matklad (Nov 09 2020 at 09:42, on Zulip):

I do think this is a language-level concern, and that the fix is indeed to use pub only for exported items, and roll with -Dunreacheable_pub otherwise.

matklad (Nov 09 2020 at 09:42, on Zulip):

It is hugely helpful to see pub and immediately know "hey, this is a public API"

matklad (Nov 09 2020 at 09:43, on Zulip):

We recently begin enforcing this on CI: https://github.com/rust-analyzer/rust-analyzer/commit/ba8d6d1e4ea2590b31470171efc175b0301c5e1c

pksunkara (Nov 09 2020 at 14:46, on Zulip):

Thanks for the discussion guys. I am now thinking that we should get clippy to have that lint by default turned on then

matklad (Nov 09 2020 at 14:49, on Zulip):

Its not the clippy lint, it's compiler lint.

The plan was to toggle it in 2018, but that didn't happen, in large part due to crate visibility modifier not happening

Joshua Nelson (Nov 09 2020 at 15:37, on Zulip):

unreachable_pub is buggy though :/

Joshua Nelson (Nov 09 2020 at 15:39, on Zulip):

https://github.com/rust-lang/rust/issues/64762

matklad (Nov 09 2020 at 15:39, on Zulip):

Yeah, I've noticed that it doesn't hanle this case

pub use m::foo;
mod m { pub fn fo0() {}}
Joshua Nelson (Nov 09 2020 at 15:39, on Zulip):

Oh I'm ok with false negatives, it also has false positives

Joshua Nelson (Nov 09 2020 at 15:39, on Zulip):

And those are really annoying

matklad (Nov 09 2020 at 15:39, on Zulip):

I mean, that's false posive

matklad (Nov 09 2020 at 15:40, on Zulip):

And, looking at the issue, I think that actually needs to be m::{foo, bar}.

pksunkara (Nov 10 2020 at 12:19, on Zulip):

Hmm.. I guess I will wait for 2021 edition roadmap and then I can propose turning this to warn

Joshua Nelson (Nov 10 2020 at 12:19, on Zulip):

you don't need an edition break to make lints warn-by-default

Joshua Nelson (Nov 10 2020 at 12:19, on Zulip):

you do however need to make sure there aren't false positives

Joshua Nelson (Nov 10 2020 at 12:19, on Zulip):

so https://github.com/rust-lang/rust/issues/64762 needs to be fixed first

Last update: Jul 24 2021 at 21:30UTC