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

Remove intra doc link items if they are private/hidden and the matching option is not enabled #130278

Conversation

GuillaumeGomez
Copy link
Member

Fixes #130233.

Since such items cannot be linked to, there is no point in keeping them as candidates.

r? @notriddle

@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. labels Sep 12, 2024
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the intra-doc-link-filter-out branch from e0cf6ca to 90a6e01 Compare September 12, 2024 15:34
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the intra-doc-link-filter-out branch from 90a6e01 to e1dd08f Compare September 12, 2024 17:17
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the intra-doc-link-filter-out branch from e1dd08f to c1ec733 Compare September 14, 2024 15:09
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the intra-doc-link-filter-out branch from c1ec733 to c7e2628 Compare September 14, 2024 15:18
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the intra-doc-link-filter-out branch from c7e2628 to 3044e5d Compare October 2, 2024 13:38
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member Author

Some more investigations about how to add this check: I need to have cache.paths and cache.external_paths to be filled. However the Cache::populate code is called after the intra-doc link pass. So I first moved the intra-doc link pass after it, but then some random things break. So seems like I'll need to keep running the intra-doc link pass before, keep all alternatives around and then after the Cache has been populated, I'll be able to filter items.

bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 8, 2024
…notriddle

[rustdoc] Remove intra-doc links dead code

While working on rust-lang#130278, I wondered what `resolve_display_text` was doing. I removed it and ran all rustdoc tests, and nothing failed. Are some intra-doc links tests missing or is it really dead code? Couldn't figure it out.

r? `@notriddle`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 8, 2024
…nup, r=notriddle

Remove unneeded argument of `LinkCollector::verify_disambiguator`

Still working on rust-lang#130278. ^^'

r? `@notriddle`
@GuillaumeGomez GuillaumeGomez force-pushed the intra-doc-link-filter-out branch from 3044e5d to caf905e Compare October 8, 2024 22:00
@rustbot
Copy link
Collaborator

rustbot commented Oct 8, 2024

Some changes occurred in src/librustdoc/clean/types.rs

cc @camelid

@GuillaumeGomez GuillaumeGomez force-pushed the intra-doc-link-filter-out branch from caf905e to e9ea3a9 Compare October 8, 2024 22:01
@GuillaumeGomez
Copy link
Member Author

Implemented the approach I described above. Hopefully should fix all issues now. :)

@rust-log-analyzer

This comment has been minimized.

rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 8, 2024
Rollup merge of rust-lang#131408 - GuillaumeGomez:more-intra-doc-cleanup, r=notriddle

Remove unneeded argument of `LinkCollector::verify_disambiguator`

Still working on rust-lang#130278. ^^'

r? `@notriddle`
@GuillaumeGomez GuillaumeGomez force-pushed the intra-doc-link-filter-out branch from 07da6fe to 353f3fa Compare October 9, 2024 09:42
@GuillaumeGomez
Copy link
Member Author

Ignored the rustc lint. I also realized that in case there were still unresolved intra-doc links, I wasn't emitting an error. It's now fixed with two new ui tests to ensure it. :)

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez GuillaumeGomez force-pushed the intra-doc-link-filter-out branch from 353f3fa to 759c771 Compare October 10, 2024 21:06
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check-tidy failed! Check out the build log: (web) (plain)

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

COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
COPY host-x86_64/mingw-check/validate-error-codes.sh /scripts/

# NOTE: intentionally uses python2 for x.py so we can test it still works.
# validate-toolstate only runs in our CI, so it's ok for it to only support python3.
ENV SCRIPT TIDY_PRINT_DIFF=1 python2.7 ../x.py test \
           --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint,cpp:fmt
# This file is autogenerated by pip-compile with Python 3.10
# by the following command:
#
#    pip-compile --allow-unsafe --generate-hashes reuse-requirements.in
---
#13 2.883 Building wheels for collected packages: reuse
#13 2.884   Building wheel for reuse (pyproject.toml): started
#13 3.131   Building wheel for reuse (pyproject.toml): finished with status 'done'
#13 3.132   Created wheel for reuse: filename=reuse-4.0.3-cp310-cp310-manylinux_2_35_x86_64.whl size=132715 sha256=dfa09868353292d98f811d3efdb0d54d07389e808efc71d68e3b93c514bf8bec
#13 3.132   Stored in directory: /tmp/pip-ephem-wheel-cache-1f2y3us5/wheels/3d/8d/0a/e0fc6aba4494b28a967ab5eaf951c121d9c677958714e34532
#13 3.135 Installing collected packages: boolean-py, binaryornot, tomlkit, reuse, python-debian, markupsafe, license-expression, jinja2, chardet, attrs
#13 3.535 Successfully installed attrs-23.2.0 binaryornot-0.4.4 boolean-py-4.0 chardet-5.2.0 jinja2-3.1.4 license-expression-30.3.0 markupsafe-2.1.5 python-debian-0.1.49 reuse-4.0.3 tomlkit-0.13.0
#13 3.536 WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv
#13 4.075 Collecting virtualenv
#13 4.075 Collecting virtualenv
#13 4.125   Downloading virtualenv-20.26.6-py3-none-any.whl (6.0 MB)
#13 4.368      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 6.0/6.0 MB 24.9 MB/s eta 0:00:00
#13 4.415 Collecting distlib<1,>=0.3.7
#13 4.423   Downloading distlib-0.3.9-py2.py3-none-any.whl (468 kB)
#13 4.435      ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 469.0/469.0 KB 49.3 MB/s eta 0:00:00
#13 4.473 Collecting filelock<4,>=3.12.2
#13 4.481   Downloading filelock-3.16.1-py3-none-any.whl (16 kB)
#13 4.521 Collecting platformdirs<5,>=3.9.1
#13 4.528   Downloading platformdirs-4.3.6-py3-none-any.whl (18 kB)
#13 4.611 Installing collected packages: distlib, platformdirs, filelock, virtualenv
#13 4.809 Successfully installed distlib-0.3.9 filelock-3.16.1 platformdirs-4.3.6 virtualenv-20.26.6
#13 DONE 4.9s

#14 [7/8] COPY host-x86_64/mingw-check/validate-toolstate.sh /scripts/
#14 DONE 0.0s
---
DirectMap4k:      227264 kB
DirectMap2M:    10258432 kB
DirectMap1G:     8388608 kB
##[endgroup]
Executing TIDY_PRINT_DIFF=1 python2.7 ../x.py test            --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint,cpp:fmt
+ TIDY_PRINT_DIFF=1 python2.7 ../x.py test --stage 0 src/tools/tidy tidyselftest --extra-checks=py:lint,cpp:fmt
    Finished `dev` profile [unoptimized] target(s) in 0.04s
##[endgroup]
WARNING: `rust.download-rustc` is enabled. The `rust.channel` option will be overridden by the CI rustc's channel.
downloading https://static.rust-lang.org/dist/2024-09-22/rustfmt-nightly-x86_64-unknown-linux-gnu.tar.xz
---
 use crate::passes::Condition::*;
 use crate::passes::collect_intra_doc_links::LinkCollector;
-use crate::passes;
 
 pub(crate) struct DocContext<'tcx> {
     pub(crate) tcx: TyCtxt<'tcx>,
fmt error: Running `"/checkout/obj/build/x86_64-unknown-linux-gnu/rustfmt/bin/rustfmt" "--config-path" "/checkout" "--edition" "2021" "--unstable-features" "--skip-children" "--check" "/checkout/src/librustdoc/clean/mod.rs" "/checkout/src/librustdoc/markdown.rs" "/checkout/src/librustdoc/json/import_finder.rs" "/checkout/src/librustdoc/json/conversions.rs" "/checkout/src/librustdoc/json/mod.rs" "/checkout/src/librustdoc/doctest.rs" "/checkout/src/librustdoc/scrape_examples.rs" "/checkout/src/librustdoc/docfs.rs" "/checkout/src/librustdoc/fold.rs" "/checkout/src/librustdoc/config.rs" "/checkout/src/librustdoc/core.rs" "/checkout/src/librustdoc/externalfiles.rs" "/checkout/src/librustdoc/theme.rs" "/checkout/src/librustdoc/visit.rs" "/checkout/src/librustdoc/html/markdown.rs" "/checkout/tests/assembly/x86_64-floating-point-clamp.rs" "/checkout/tests/assembly/nvptx-arch-emit-asm.rs" "/checkout/tests/assembly/sparc-struct-abi.rs" "/checkout/tests/assembly/issue-83585-small-pod-struct-equality.rs" "/checkout/tests/assembly/x86-return-float.rs" "/checkout/tests/assembly/align_offset.rs" "/checkout/tests/assembly/cmse.rs" "/checkout/tests/assembly/nvptx-linking-binary.rs" "/checkout/tests/assembly/pie-relocation-model.rs" "/checkout/tests/assembly/nvptx-c-abi-arg-v7.rs" "/checkout/tests/assembly/nvptx-arch-default.rs" "/checkout/tests/assembly/stack-protector/stack-protector-heuristics-effect-windows-32bit.rs" "/checkout/tests/assembly/stack-protector/stack-protector-target-support.rs" "/checkout/tests/assembly/stack-protector/stack-protector-heuristics-effect.rs" "/checkout/tests/assembly/stack-protector/stack-protector-heuristics-effect-windows-64bit.rs" "/checkout/tests/assembly/nvptx-kernel-abi/nvptx-kernel-args-abi-v7.rs" "/checkout/tests/assembly/libs/issue-115339-zip-arrays.rs" "/checkout/tests/assembly/aarch64-naked-fn-no-bti-prolog.rs" "/checkout/tests/assembly/strict_provenance.rs" "/checkout/tests/assembly/dwarf4.rs" "/checkout/tests/assembly/x86_64-function-return.rs" "/checkout/tests/assembly/x86_64-fortanix-unknown-sgx-lvi-generic-ret.rs" "/checkout/tests/assembly/slice-is_ascii.rs" "/checkout/tests/assembly/nvptx-internalizing.rs" "/checkout/tests/assembly/aarch64-pointer-auth.rs" "/checkout/tests/assembly/nvptx-c-abi-ret-v7.rs" "/checkout/tests/assembly/nvptx-linking-cdylib.rs" "/checkout/tests/assembly/closure-inherit-target-feature.rs" "/checkout/tests/assembly/auxiliary/non-inline-dependency.rs" "/checkout/tests/assembly/auxiliary/breakpoint-panic-handler.rs" "/checkout/tests/assembly/pic-relocation-model.rs" "/checkout/tests/assembly/target-feature-multiple.rs" "/checkout/tests/assembly/is_aligned.rs" "/checkout/tests/assembly/x86_64-fortanix-unknown-sgx-lvi-generic-load.rs" "/checkout/tests/assembly/x86_64-sse_crc.rs" "/checkout/tests/assembly/simd-intrinsic-mask-store.rs" "/checkout/tests/assembly/static-relocation-model.rs" "/checkout/tests/assembly/simd-intrinsic-mask-reduce.rs" "/checkout/tests/assembly/dwarf5.rs" "/checkout/tests/assembly/x86_64-naked-fn-no-cet-prolog.rs" "/checkout/tests/assembly/targets/targets-elf.rs" "/checkout/tests/assembly/targets/targets-nvptx.rs" "/checkout/tests/assembly/targets/targets-pe.rs" "/checkout/tests/assembly/targets/targets-macho.rs" "/checkout/tests/assembly/simd-bitmask.rs" "/checkout/tests/assembly/stack-probes.rs" "/checkout/tests/assembly/niche-prefer-zero.rs" "/checkout/tests/assembly/x86_64-array-pair-load-store-merge.rs" "/checkout/src/librustdoc/clean/cfg/tests.rs"` failed.
If you're running `tidy`, try again with `--bless`. Or, if you just want to format code, run `./x.py fmt` instead.
  local time: Thu Oct 10 21:14:33 UTC 2024
  network time: Thu, 10 Oct 2024 21:14:34 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

RalfJung pushed a commit to RalfJung/miri that referenced this pull request Oct 14, 2024
[rustdoc] Remove intra-doc links dead code

While working on rust-lang/rust#130278, I wondered what `resolve_display_text` was doing. I removed it and ran all rustdoc tests, and nothing failed. Are some intra-doc links tests missing or is it really dead code? Couldn't figure it out.

r? `@notriddle`
@GuillaumeGomez
Copy link
Member Author

Closing in favor of #131691.

@GuillaumeGomez GuillaumeGomez deleted the intra-doc-link-filter-out branch October 14, 2024 16:00
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 16, 2024
…er-out-2, r=notriddle

Delay ambiguous intra-doc link resolution after `Cache` has been populated

Fixes rust-lang#130233.

I was getting nowhere with rust-lang#130278. I took a wrong turn at some point and ended making way too many changes so instead I started again back from 0 and this time it worked out as expected.

r? `@notriddle`
Urgau added a commit to Urgau/rust that referenced this pull request Oct 16, 2024
…er-out-2, r=notriddle

Delay ambiguous intra-doc link resolution after `Cache` has been populated

Fixes rust-lang#130233.

I was getting nowhere with rust-lang#130278. I took a wrong turn at some point and ended making way too many changes so instead I started again back from 0 and this time it worked out as expected.

r? ``@notriddle``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 16, 2024
…er-out-2, r=notriddle

Delay ambiguous intra-doc link resolution after `Cache` has been populated

Fixes rust-lang#130233.

I was getting nowhere with rust-lang#130278. I took a wrong turn at some point and ended making way too many changes so instead I started again back from 0 and this time it worked out as expected.

r? ```@notriddle```
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 16, 2024
Rollup merge of rust-lang#131691 - GuillaumeGomez:intra-doc-link-filter-out-2, r=notriddle

Delay ambiguous intra-doc link resolution after `Cache` has been populated

Fixes rust-lang#130233.

I was getting nowhere with rust-lang#130278. I took a wrong turn at some point and ended making way too many changes so instead I started again back from 0 and this time it worked out as expected.

r? ```@notriddle```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Intra-doc link should rule out hidden items when resolving ambiguous link
4 participants