Improve suggestion in case a bare URL is surrounded by brackets #146413

GuillaumeGomez · 2025-09-10T13:45:54Z

With this change, output looks like this:

  |
1 | //! [https://github.com]
  |     ^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<https://github.com>`
  |
  = note: bare URLs are not automatically turned into clickable links
  = note: `#[warn(rustdoc::bare_urls)]` on by default

cc @fmease
r? @lolbinarycat

GuillaumeGomez · 2025-09-10T14:56:04Z

And fixed too. :)

lolbinarycat

Logic looks solid, i've just got a few minor nitpicks about naming, and an idea for how to make find_raw_urls a bit more readable.

View changes since this review

lolbinarycat · 2025-09-10T15:35:41Z

src/librustdoc/passes/lint/bare_urls.rs


 fn find_raw_urls(
    cx: &DocContext<'_>,
+    whole_text: &str,


nit: this is the concatenation of all doc fragments, right? if so, that is conventionally called dox in rustdoc. unsure which is more important, clarity or consistency.

Not exactly, more like a concatenation of all consecutive text fragments. A bit tricky to name. ^^'

Can two doc fragments be on the same item an not consecutive?

This is literally referred to as dox in visit_item. I think we should stick with that, to try to keep the code uniform.

Well for example, [https://blob.cat] calls visit_item with respectively [, https://blob.cat and ] separately. It depends on the events of pulldown_cmark.

I believe those are the value of text, which seem irrelevant to the naming of a different parameter.

lolbinarycat · 2025-09-10T15:48:28Z

src/librustdoc/passes/lint/bare_urls.rs

-        });
-    };
+    let report_diag =
+        |cx: &DocContext<'_>, msg: &'static str, range: Range<usize>, no_brackets: Option<&str>| {


nit: i think without_brackets makes more since than no_brackets, the latter sounds more like it should be a boolean.

Damn that's what I used originally. Reverting to it then. 😆

lolbinarycat · 2025-09-10T15:55:22Z

src/librustdoc/passes/lint/bare_urls.rs

    trace!("looking for raw urls in {text}");
    // For now, we only check "full" URLs (meaning, starting with "http://" or "https://").
    for match_ in URL_REGEX.find_iter(text) {
        let url_range = match_.range();


Suggested change

let url_range = match_.range();

let mut url_range = match_.range();

url_range.start += range.start;

url_range.end += range.start;

I think this could clean up this function a fair amount, we could get rid of extra_range, and we wouldn't have to do range.start + so many times, which should make the complex condition a bit more readable.

GuillaumeGomez · 2025-09-10T16:55:09Z

Applied suggestions.

lolbinarycat · 2025-09-10T18:00:12Z

Looking good!

r=me with whole_text renamed to dox and CI passing.

GuillaumeGomez · 2025-09-10T19:29:53Z

@bors r=lolbinarycat rollup

bors · 2025-09-10T19:29:56Z

📌 Commit 7966ae0 has been approved by lolbinarycat

It is now in the queue for this repository.

…r=lolbinarycat Improve suggestion in case a bare URL is surrounded by brackets Fixes rust-lang#146162. With this change, output looks like this: ``` | 1 | //! [https://github.com] | ^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<https://github.com>` | = note: bare URLs are not automatically turned into clickable links = note: `#[warn(rustdoc::bare_urls)]` on by default ``` cc `@fmease` r? `@lolbinarycat`

Rollup of 16 pull requests Successful merges: - #145660 (initial implementation of the darwin_objc unstable feature) - #145895 (thread parking: fix docs and examples) - #146308 (support integer literals in `${concat()}`) - #146323 (check before test for hardware capabilites in bits 32~63 of usize) - #146332 (tidy: make behavior of extra-checks more uniform) - #146338 (Extends AArch64 branch protection support to include GCS) - #146374 (Update `browser-ui-test` version to `0.22.2`) - #146413 (Improve suggestion in case a bare URL is surrounded by brackets) - #146426 (Bump miow to 0.60.1) - #146432 (Implement `Socket::take_error` for Hermit) - #146433 (rwlock tests: fix miri macos test regression) - #146435 (Change the default value of `gcc.download-ci-gcc` to `true`) - #146439 (fix cfg for poison test macro) - #146448 ([rustdoc] Correctly handle literal search on paths) - #146449 (Fix `libgccjit` symlink when we build GCC locally) - #146455 (test: remove an outdated normalization for rustc versions) Failed merges: - #146389 (Convert `no_std` and `no_core` to the new attribute infrastructure) r? `@ghost` `@rustbot` modify labels: rollup

…r=lolbinarycat Improve suggestion in case a bare URL is surrounded by brackets Fixes rust-lang#146162. With this change, output looks like this: ``` | 1 | //! [https://github.com] | ^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<https://github.com>` | = note: bare URLs are not automatically turned into clickable links = note: `#[warn(rustdoc::bare_urls)]` on by default ``` cc ``@fmease`` r? ``@lolbinarycat``

Rollup of 16 pull requests Successful merges: - #144549 (match clang's `va_arg` assembly on arm targets) - #145660 (initial implementation of the darwin_objc unstable feature) - #145895 (thread parking: fix docs and examples) - #146308 (support integer literals in `${concat()}`) - #146323 (check before test for hardware capabilites in bits 32~63 of usize) - #146332 (tidy: make behavior of extra-checks more uniform) - #146374 (Update `browser-ui-test` version to `0.22.2`) - #146413 (Improve suggestion in case a bare URL is surrounded by brackets) - #146426 (Bump miow to 0.60.1) - #146432 (Implement `Socket::take_error` for Hermit) - #146433 (rwlock tests: fix miri macos test regression) - #146435 (Change the default value of `gcc.download-ci-gcc` to `true`) - #146439 (fix cfg for poison test macro) - #146448 ([rustdoc] Correctly handle literal search on paths) - #146449 (Fix `libgccjit` symlink when we build GCC locally) - #146455 (test: remove an outdated normalization for rustc versions) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 15 pull requests Successful merges: - #144549 (match clang's `va_arg` assembly on arm targets) - #145895 (thread parking: fix docs and examples) - #146308 (support integer literals in `${concat()}`) - #146323 (check before test for hardware capabilites in bits 32~63 of usize) - #146332 (tidy: make behavior of extra-checks more uniform) - #146374 (Update `browser-ui-test` version to `0.22.2`) - #146413 (Improve suggestion in case a bare URL is surrounded by brackets) - #146426 (Bump miow to 0.60.1) - #146432 (Implement `Socket::take_error` for Hermit) - #146433 (rwlock tests: fix miri macos test regression) - #146435 (Change the default value of `gcc.download-ci-gcc` to `true`) - #146439 (fix cfg for poison test macro) - #146448 ([rustdoc] Correctly handle literal search on paths) - #146449 (Fix `libgccjit` symlink when we build GCC locally) - #146455 (test: remove an outdated normalization for rustc versions) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #146413 - GuillaumeGomez:rustdoc-bare-urls, r=lolbinarycat Improve suggestion in case a bare URL is surrounded by brackets Fixes #146162. With this change, output looks like this: ``` | 1 | //! [https://github.com] | ^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<https://github.com>` | = note: bare URLs are not automatically turned into clickable links = note: `#[warn(rustdoc::bare_urls)]` on by default ``` cc ```@fmease``` r? ```@lolbinarycat```

Rollup of 15 pull requests Successful merges: - rust-lang/rust#144549 (match clang's `va_arg` assembly on arm targets) - rust-lang/rust#145895 (thread parking: fix docs and examples) - rust-lang/rust#146308 (support integer literals in `${concat()}`) - rust-lang/rust#146323 (check before test for hardware capabilites in bits 32~63 of usize) - rust-lang/rust#146332 (tidy: make behavior of extra-checks more uniform) - rust-lang/rust#146374 (Update `browser-ui-test` version to `0.22.2`) - rust-lang/rust#146413 (Improve suggestion in case a bare URL is surrounded by brackets) - rust-lang/rust#146426 (Bump miow to 0.60.1) - rust-lang/rust#146432 (Implement `Socket::take_error` for Hermit) - rust-lang/rust#146433 (rwlock tests: fix miri macos test regression) - rust-lang/rust#146435 (Change the default value of `gcc.download-ci-gcc` to `true`) - rust-lang/rust#146439 (fix cfg for poison test macro) - rust-lang/rust#146448 ([rustdoc] Correctly handle literal search on paths) - rust-lang/rust#146449 (Fix `libgccjit` symlink when we build GCC locally) - rust-lang/rust#146455 (test: remove an outdated normalization for rustc versions) r? `@ghost` `@rustbot` modify labels: rollup

rustbot assigned lolbinarycat Sep 10, 2025

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 10, 2025

This comment has been minimized.

Sign in to view

GuillaumeGomez force-pushed the rustdoc-bare-urls branch from 74a3bb2 to 4a1fffb Compare September 10, 2025 14:52

lolbinarycat reviewed Sep 10, 2025

View reviewed changes

GuillaumeGomez added 2 commits September 10, 2025 18:44

Improve suggestion in case a bare URL is surrounded by brackets

5a36fe2

Add new ui tests for rustdoc::bare_urls

3205e4d

GuillaumeGomez force-pushed the rustdoc-bare-urls branch from 4a1fffb to cf429b0 Compare September 10, 2025 16:54

GuillaumeGomez force-pushed the rustdoc-bare-urls branch from cf429b0 to dfd72e4 Compare September 10, 2025 16:58

rustbot added A-CI Area: Our Github Actions CI A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Sep 10, 2025

GuillaumeGomez force-pushed the rustdoc-bare-urls branch from dfd72e4 to cf429b0 Compare September 10, 2025 16:59

Simplify code for find_raw_urls

7966ae0

GuillaumeGomez force-pushed the rustdoc-bare-urls branch from cf429b0 to 7966ae0 Compare September 10, 2025 18:02

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 Sep 10, 2025

jieyouxu removed T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. A-CI Area: Our Github Actions CI labels Sep 11, 2025

Zalathar mentioned this pull request Sep 12, 2025

Rollup of 16 pull requests #146461

Closed

Zalathar mentioned this pull request Sep 12, 2025

Rollup of 16 pull requests #146462

Closed

Zalathar mentioned this pull request Sep 12, 2025

Rollup of 15 pull requests #146468

Merged

bors merged commit 322f5dc into rust-lang:master Sep 12, 2025
10 checks passed

rustbot added this to the 1.91.0 milestone Sep 12, 2025

GuillaumeGomez deleted the rustdoc-bare-urls branch September 18, 2025 20:28

-        let url_range = match_.range();
+        let mut url_range = match_.range();
+        url_range.start += range.start;
+        url_range.end += range.start;

Improve suggestion in case a bare URL is surrounded by brackets #146413

Improve suggestion in case a bare URL is surrounded by brackets #146413

Conversation

GuillaumeGomez commented Sep 10, 2025

Uh oh!

This comment has been minimized.

GuillaumeGomez commented Sep 10, 2025

Uh oh!

lolbinarycat left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lolbinarycat Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Sep 10, 2025

Uh oh!

lolbinarycat commented Sep 10, 2025

Uh oh!

GuillaumeGomez commented Sep 10, 2025

Uh oh!

bors commented Sep 10, 2025

Uh oh!

Uh oh!

Uh oh!

lolbinarycat left a comment •

edited by rustbot

Loading

lolbinarycat Sep 10, 2025 •

edited

Loading