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

rustdoc: be more strict about "Methods from Deref" #138574

Merged
merged 1 commit into from
Mar 24, 2025

Conversation

lolbinarycat
Copy link
Contributor

@lolbinarycat lolbinarycat commented Mar 16, 2025

fixes #137083
fixes #24686

Currently done:

Note that this does not yet fix the sidebar logic.

@rustbot
Copy link
Collaborator

rustbot commented Mar 16, 2025

r? @notriddle

rustbot has assigned @notriddle.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels Mar 16, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

pub fn get_u32(&self) -> u32 { self.0 }
}

// Note that the same href is used both on the method itself,
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it an issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, since we're testing that it appears twice, and in any case, we care about the negative case much more, which will trigger if the bad method shows up in the sidebar or on the main page.

@rust-log-analyzer

This comment has been minimized.

@lolbinarycat lolbinarycat force-pushed the rustdoc-deref-24686-v2 branch from fd11eee to 60156ca Compare March 22, 2025 16:15
@GuillaumeGomez
Copy link
Member

Thanks for the fix! Squash your commits then r=me.

hack: is_doc_subtype_of always returns true for TyAlias

it's worth noting that this function is only used in
the handling of "Methods from Deref", and we were previously
assuming all generic parameters were meaningless,
so this is still an improvment from the status quo.

this change means that we will have strictly less false positives
without adding any new false negitives.

Co-authored-by: Guillaume Gomez <guillaume1.gomez@gmail.com>
@lolbinarycat lolbinarycat force-pushed the rustdoc-deref-24686-v2 branch from 60156ca to b46412f Compare March 22, 2025 17:49
@lolbinarycat
Copy link
Contributor Author

r=@GuillaumeGomez

(still don't have bors perms so I doubt the r= is going to do anything)

@notriddle
Copy link
Contributor

r? @GuillaumeGomez

@bors r=GuillaumeGomez

@bors
Copy link
Contributor

bors commented Mar 22, 2025

📌 Commit b46412f has been approved by GuillaumeGomez

It is now in the queue for this repository.

@bors 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 Mar 22, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 24, 2025
Rollup of 10 pull requests

Successful merges:

 - rust-lang#137593 (fix download-llvm logic for subtree sync branches)
 - rust-lang#137736 (Don't attempt to export compiler-builtins symbols from rust dylibs)
 - rust-lang#138135 (Simplify `PartialOrd` on tuples containing primitives)
 - rust-lang#138321 ([bootstrap] Distribute split debuginfo if present)
 - rust-lang#138574 (rustdoc: be more strict about "Methods from Deref")
 - rust-lang#138606 (Fix missing rustfmt in msi installer - cont)
 - rust-lang#138671 (Fix `FileType` `PartialEq` implementation on Windows)
 - rust-lang#138728 (Update `compiler-builtins` to 0.1.152)
 - rust-lang#138783 (Cache current_dll_path output)
 - rust-lang#138846 (Tweaks to writeback and `Obligation -> Goal` conversion)

Failed merges:

 - rust-lang#138755 ([rustdoc] Remove duplicated loop when computing doc cfgs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit abc6784 into rust-lang:master Mar 24, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 24, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 24, 2025
Rollup merge of rust-lang#138574 - lolbinarycat:rustdoc-deref-24686-v2, r=GuillaumeGomez

rustdoc: be more strict about "Methods from Deref"

fixes rust-lang#137083
fixes rust-lang#24686

Currently done:
* [x] fix `render_assoc_items_inner
* [x] fix sidebar logic
* [x] port test from rust-lang#137564
* [x] add test for sidebar items

Note that this does not yet fix the sidebar logic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.
Projects
None yet
6 participants