Respect doc(hidden) when suggesting available fields #93214

ibraheemdev · 2022-01-22T20:45:42Z

Resolves #93210

rust-highfive · 2022-01-22T20:45:45Z

(rust-highfive has picked a reviewer for you, use r? to override)

ibraheemdev · 2022-01-22T20:46:41Z

src/test/ui/did_you_mean/issue-93210-ignore-doc-hidden.rs

+fn main() {
+    A::default().hey;
+    //~^ ERROR no field `hey` on type `A`
+    //~| NOTE unknown field


I'm not sure why this note is needed, it doesn't show up in the stderr output 🤔

nagisa · 2022-01-22T21:46:05Z

I’m… not entirely sure this is a holistically a correct thing to do. Compiler diagnostics are not documentation. And the field is accessible either way – it just takes another tool failing to account for doc(hidden) before its front-and-center before the user.

My intuition is that rustc itself should not look at the doc attributes to inform its own behaviour, much like it does not look at e.g. #[rustfmt::*] or #[clippy::*] attributes.

That said it seems like it already does account for doc(hidden) in at least some places such as e.g.

rust/compiler/rustc_mir_build/src/thir/pattern/usefulness.rs

Line 599 in ecf7299

if missing_ctor.is_doc_hidden_variant(pcx)

so I guess my intuition is invalid.

For the Pin case specifically, there's another potential problem – I suspect we suggest unstable fields when user is compiling with a stable compiler, or without a relevant feature enabled. That sounds like something that would want a fix given that the field cannot be accessed in that situation.

m-ou-se · 2022-01-24T11:49:28Z

I’m… not entirely sure this is a holistically a correct thing to do. Compiler diagnostics are not documentation. And the field is accessible either way – it just takes another tool failing to account for doc(hidden) before its front-and-center before the user.

#[doc(hidden)] is the only way and a very common way for libraries to mark something as 'not part of the pubilc api' when it has to be pub for macro-related reasons. In the standard library we combine it with #[unstable], but in libraries that need to work on stable Rust, there's not much more one can do than #[doc(hidden)] and adding a bunch of underscores in the name.

I suspect we suggest unstable fields when user is compiling with a stable compiler, or without a relevant feature enabled. That sounds like something that would want a fix given that the field cannot be accessed in that situation.

It might be good to do the same thing as with enum variants: To also hide them when a field is part of an unstable feature that's not enabled.

danielhenrymantilla · 2022-01-24T12:18:41Z

I've added handling for unstability in my own branch: danielhenrymantilla@issue-93210

_{^{(you can apply it here with git pull https://github.com/danielhenrymantilla/rust issue-93210)}}

danielhenrymantilla · 2022-01-24T12:25:20Z

src/test/ui/did_you_mean/issue-93210-ignore-doc-hidden.rs

+fn main() {
+    A::default().hey;


Suggested change

fn main() {

A::default().hey;

fn main() {

A::default().hallo;

A::default().hey;

.hallo may trigger similarly-named field suggestions, which may also be interesting behavior to test

michaelwoerister · 2022-01-25T12:55:52Z

Looks like there are design questions to be solved here, so r? rust-lang/diagnostics

davidtwco

I agree with @nagisa that it seems less than great that #[doc(hidden)] is used here.

It seems like we could either use a pub(macro) (can only be used in downstream crates when through one of this crate's macro), but I imagine that wouldn't really work with procedural macros; or a new attribute that the user can use to signal that something shouldn't be used in suggestions, rather than #[doc(hidden)].

Regardless, it seems like #[doc(hidden)] is what we've got right now and we're using it for this in other places, so might as well do so here too. Implementation for this looks good - if you want to address @danielhenrymantilla's comment then feel free to, otherwise r=me (I'll do this in a day or two if there aren't any changes).

davidtwco · 2022-01-31T03:42:14Z

@bors r+

bors · 2022-01-31T03:42:16Z

📌 Commit b734abc has been approved by davidtwco

…askrgr Rollup of 8 pull requests Successful merges: - rust-lang#90277 (Improve terminology around "after typeck") - rust-lang#92918 (Allow eliding GATs in expression position) - rust-lang#93039 (Don't suggest inaccessible fields) - rust-lang#93155 (Switch pretty printer to block-based indentation) - rust-lang#93214 (Respect doc(hidden) when suggesting available fields) - rust-lang#93347 (Make `char::DecodeUtf16::size_hist` more precise) - rust-lang#93392 (Clarify documentation on char::MAX) - rust-lang#93444 (Fix some CSS warnings and errors from VS Code) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

danielhenrymantilla · 2022-01-31T12:14:06Z

_{^{Out-of-topic / meta comment: wow, since my commit was originally pushed (to my branch) quite a while ago, now that it has just been integrated in this PR, it nonetheless appears as occurring before I even made the very comment about it! 😆 (more seriously, I was confused by the lack of apparent changes in the "PR feed" near the bottom; I hope they somehow change that 😅).}}

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 22, 2022

rust-highfive assigned michaelwoerister Jan 22, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 22, 2022

ibraheemdev commented Jan 22, 2022

View reviewed changes

respect doc(hidden) when suggesting available fields

32ab0b8

danielhenrymantilla mentioned this pull request Jan 24, 2022

Add a stack-pin!-ning macro to core::pin. #93176

Merged

4 tasks

Handle unstability as well, including "field typo suggestions"

b734abc

danielhenrymantilla reviewed Jan 24, 2022

View reviewed changes

rust-highfive assigned davidtwco and unassigned michaelwoerister Jan 25, 2022

davidtwco approved these changes Jan 26, 2022

View reviewed changes

davidtwco approved these changes Jan 31, 2022

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 31, 2022

matthiaskrgr mentioned this pull request Jan 31, 2022

Rollup of 8 pull requests #93498

Merged

bors merged commit 7de90d5 into rust-lang:master Jan 31, 2022

rustbot added this to the 1.60.0 milestone Jan 31, 2022

dtolnay mentioned this pull request Oct 22, 2024

Hide nonlocal doc(hidden) types and impls from "the following other types implement trait" #132024

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Respect doc(hidden) when suggesting available fields #93214

Respect doc(hidden) when suggesting available fields #93214

ibraheemdev commented Jan 22, 2022

rust-highfive commented Jan 22, 2022

ibraheemdev Jan 22, 2022

nagisa commented Jan 22, 2022

m-ou-se commented Jan 24, 2022 •

edited

Loading

danielhenrymantilla commented Jan 24, 2022

danielhenrymantilla Jan 24, 2022

michaelwoerister commented Jan 25, 2022

davidtwco left a comment

davidtwco commented Jan 31, 2022

bors commented Jan 31, 2022

danielhenrymantilla commented Jan 31, 2022

Respect doc(hidden) when suggesting available fields #93214

Respect doc(hidden) when suggesting available fields #93214

Conversation

ibraheemdev commented Jan 22, 2022

rust-highfive commented Jan 22, 2022

ibraheemdev Jan 22, 2022

Choose a reason for hiding this comment

nagisa commented Jan 22, 2022

m-ou-se commented Jan 24, 2022 • edited Loading

danielhenrymantilla commented Jan 24, 2022

danielhenrymantilla Jan 24, 2022

Choose a reason for hiding this comment

michaelwoerister commented Jan 25, 2022

davidtwco left a comment

Choose a reason for hiding this comment

davidtwco commented Jan 31, 2022

bors commented Jan 31, 2022

danielhenrymantilla commented Jan 31, 2022

m-ou-se commented Jan 24, 2022 •

edited

Loading