From 5a36fe23879ee8dde1326bcf08f0016af6ccd896 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 10 Sep 2025 15:44:42 +0200 Subject: [PATCH 1/3] Improve suggestion in case a bare URL is surrounded by brackets --- src/librustdoc/passes/lint/bare_urls.rs | 53 +++++++++++++++++++------ 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/src/librustdoc/passes/lint/bare_urls.rs b/src/librustdoc/passes/lint/bare_urls.rs index f70bdf4e4fe37..ffa160beae30b 100644 --- a/src/librustdoc/passes/lint/bare_urls.rs +++ b/src/librustdoc/passes/lint/bare_urls.rs @@ -17,7 +17,10 @@ use crate::core::DocContext; use crate::html::markdown::main_body_opts; pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: &str) { - let report_diag = |cx: &DocContext<'_>, msg: &'static str, range: Range| { + let report_diag = |cx: &DocContext<'_>, + msg: &'static str, + range: Range, + without_brackets: Option<&str>| { let maybe_sp = source_span_for_markdown_range(cx.tcx, dox, &range, &item.attrs.doc_strings) .map(|(sp, _)| sp); let sp = maybe_sp.unwrap_or_else(|| item.attr_span(cx.tcx)); @@ -27,14 +30,22 @@ pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: & // The fallback of using the attribute span is suitable for // highlighting where the error is, but not for placing the < and > if let Some(sp) = maybe_sp { - lint.multipart_suggestion( - "use an automatic link instead", - vec![ - (sp.shrink_to_lo(), "<".to_string()), - (sp.shrink_to_hi(), ">".to_string()), - ], - Applicability::MachineApplicable, - ); + if let Some(without_brackets) = without_brackets { + lint.multipart_suggestion( + "use an automatic link instead", + vec![(sp, format!("<{without_brackets}>"))], + Applicability::MachineApplicable, + ); + } else { + lint.multipart_suggestion( + "use an automatic link instead", + vec![ + (sp.shrink_to_lo(), "<".to_string()), + (sp.shrink_to_hi(), ">".to_string()), + ], + Applicability::MachineApplicable, + ); + } } }); }; @@ -43,7 +54,7 @@ pub(super) fn visit_item(cx: &DocContext<'_>, item: &Item, hir_id: HirId, dox: & while let Some((event, range)) = p.next() { match event { - Event::Text(s) => find_raw_urls(cx, &s, range, &report_diag), + Event::Text(s) => find_raw_urls(cx, dox, &s, range, &report_diag), // We don't want to check the text inside code blocks or links. Event::Start(tag @ (Tag::CodeBlock(_) | Tag::Link { .. })) => { for (event, _) in p.by_ref() { @@ -67,25 +78,41 @@ static URL_REGEX: LazyLock = LazyLock::new(|| { r"https?://", // url scheme r"([-a-zA-Z0-9@:%._\+~#=]{2,256}\.)+", // one or more subdomains r"[a-zA-Z]{2,63}", // root domain - r"\b([-a-zA-Z0-9@:%_\+.~#?&/=]*)" // optional query or url fragments + r"\b([-a-zA-Z0-9@:%_\+.~#?&/=]*)", // optional query or url fragments )) .expect("failed to build regex") }); fn find_raw_urls( cx: &DocContext<'_>, + whole_text: &str, text: &str, range: Range, - f: &impl Fn(&DocContext<'_>, &'static str, Range), + f: &impl Fn(&DocContext<'_>, &'static str, Range, Option<&str>), ) { 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(); + let mut without_brackets = None; + let mut extra_range = 0; + // If the whole text is contained inside `[]`, then we need to replace the brackets and + // not just add `<>`. + if whole_text[..range.start + url_range.start].ends_with('[') + && range.start + url_range.end <= whole_text.len() + && whole_text[range.start + url_range.end..].starts_with(']') + { + extra_range = 1; + without_brackets = Some(match_.as_str()); + } f( cx, "this URL is not a hyperlink", - Range { start: range.start + url_range.start, end: range.start + url_range.end }, + Range { + start: range.start + url_range.start - extra_range, + end: range.start + url_range.end + extra_range, + }, + without_brackets, ); } } From 3205e4db0f9278ce06240b9de20d0802afef1de9 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 10 Sep 2025 15:44:59 +0200 Subject: [PATCH 2/3] Add new ui tests for `rustdoc::bare_urls` --- tests/rustdoc-ui/lints/bare-urls.fixed | 14 +++++ tests/rustdoc-ui/lints/bare-urls.rs | 14 +++++ tests/rustdoc-ui/lints/bare-urls.stderr | 70 ++++++++++++++++++++++++- 3 files changed, 97 insertions(+), 1 deletion(-) diff --git a/tests/rustdoc-ui/lints/bare-urls.fixed b/tests/rustdoc-ui/lints/bare-urls.fixed index a91573146b823..ac63e291c5b04 100644 --- a/tests/rustdoc-ui/lints/bare-urls.fixed +++ b/tests/rustdoc-ui/lints/bare-urls.fixed @@ -68,3 +68,17 @@ pub mod foo { /// https://somewhere.com/a?hello=12&bye=11#xyz pub fn bar() {} } + +/// +//~^ ERROR this URL is not a hyperlink +/// [ ] +//~^ ERROR this URL is not a hyperlink +/// [ ] +//~^ ERROR this URL is not a hyperlink +/// [ ] +//~^ ERROR this URL is not a hyperlink +/// [ +//~^ ERROR this URL is not a hyperlink +/// ] +//~^ ERROR this URL is not a hyperlink +pub fn lint_with_brackets() {} diff --git a/tests/rustdoc-ui/lints/bare-urls.rs b/tests/rustdoc-ui/lints/bare-urls.rs index 5b008cdafa232..a70a3ec822e4a 100644 --- a/tests/rustdoc-ui/lints/bare-urls.rs +++ b/tests/rustdoc-ui/lints/bare-urls.rs @@ -68,3 +68,17 @@ pub mod foo { /// https://somewhere.com/a?hello=12&bye=11#xyz pub fn bar() {} } + +/// [https://bloob.blob] +//~^ ERROR this URL is not a hyperlink +/// [ https://bloob.blob ] +//~^ ERROR this URL is not a hyperlink +/// [ https://bloob.blob] +//~^ ERROR this URL is not a hyperlink +/// [https://bloob.blob ] +//~^ ERROR this URL is not a hyperlink +/// [https://bloob.blob +//~^ ERROR this URL is not a hyperlink +/// https://bloob.blob] +//~^ ERROR this URL is not a hyperlink +pub fn lint_with_brackets() {} diff --git a/tests/rustdoc-ui/lints/bare-urls.stderr b/tests/rustdoc-ui/lints/bare-urls.stderr index e1108c7e7f84e..fc3c642f5aac4 100644 --- a/tests/rustdoc-ui/lints/bare-urls.stderr +++ b/tests/rustdoc-ui/lints/bare-urls.stderr @@ -243,5 +243,73 @@ help: use an automatic link instead LL | #[doc = ""] | + + -error: aborting due to 20 previous errors +error: this URL is not a hyperlink + --> $DIR/bare-urls.rs:72:5 + | +LL | /// [https://bloob.blob] + | ^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` + | + = note: bare URLs are not automatically turned into clickable links + +error: this URL is not a hyperlink + --> $DIR/bare-urls.rs:74:7 + | +LL | /// [ https://bloob.blob ] + | ^^^^^^^^^^^^^^^^^^ + | + = note: bare URLs are not automatically turned into clickable links +help: use an automatic link instead + | +LL | /// [ ] + | + + + +error: this URL is not a hyperlink + --> $DIR/bare-urls.rs:76:7 + | +LL | /// [ https://bloob.blob] + | ^^^^^^^^^^^^^^^^^^ + | + = note: bare URLs are not automatically turned into clickable links +help: use an automatic link instead + | +LL | /// [ ] + | + + + +error: this URL is not a hyperlink + --> $DIR/bare-urls.rs:78:6 + | +LL | /// [https://bloob.blob ] + | ^^^^^^^^^^^^^^^^^^ + | + = note: bare URLs are not automatically turned into clickable links +help: use an automatic link instead + | +LL | /// [ ] + | + + + +error: this URL is not a hyperlink + --> $DIR/bare-urls.rs:80:6 + | +LL | /// [https://bloob.blob + | ^^^^^^^^^^^^^^^^^^ + | + = note: bare URLs are not automatically turned into clickable links +help: use an automatic link instead + | +LL | /// [ + | + + + +error: this URL is not a hyperlink + --> $DIR/bare-urls.rs:82:5 + | +LL | /// https://bloob.blob] + | ^^^^^^^^^^^^^^^^^^ + | + = note: bare URLs are not automatically turned into clickable links +help: use an automatic link instead + | +LL | /// ] + | + + + +error: aborting due to 26 previous errors From 7966ae0f3352894834a316bea9fd45068f94d937 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 10 Sep 2025 18:54:38 +0200 Subject: [PATCH 3/3] Simplify code for `find_raw_urls` --- src/librustdoc/passes/lint/bare_urls.rs | 28 ++++++++++--------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/src/librustdoc/passes/lint/bare_urls.rs b/src/librustdoc/passes/lint/bare_urls.rs index ffa160beae30b..06256d611512b 100644 --- a/src/librustdoc/passes/lint/bare_urls.rs +++ b/src/librustdoc/passes/lint/bare_urls.rs @@ -85,7 +85,7 @@ static URL_REGEX: LazyLock = LazyLock::new(|| { fn find_raw_urls( cx: &DocContext<'_>, - whole_text: &str, + dox: &str, text: &str, range: Range, f: &impl Fn(&DocContext<'_>, &'static str, Range, Option<&str>), @@ -93,26 +93,20 @@ fn find_raw_urls( 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(); + let mut url_range = match_.range(); + url_range.start += range.start; + url_range.end += range.start; let mut without_brackets = None; - let mut extra_range = 0; - // If the whole text is contained inside `[]`, then we need to replace the brackets and + // If the link is contained inside `[]`, then we need to replace the brackets and // not just add `<>`. - if whole_text[..range.start + url_range.start].ends_with('[') - && range.start + url_range.end <= whole_text.len() - && whole_text[range.start + url_range.end..].starts_with(']') + if dox[..url_range.start].ends_with('[') + && url_range.end <= dox.len() + && dox[url_range.end..].starts_with(']') { - extra_range = 1; + url_range.start -= 1; + url_range.end += 1; without_brackets = Some(match_.as_str()); } - f( - cx, - "this URL is not a hyperlink", - Range { - start: range.start + url_range.start - extra_range, - end: range.start + url_range.end + extra_range, - }, - without_brackets, - ); + f(cx, "this URL is not a hyperlink", url_range, without_brackets); } }