From 6f89468fc513eb12ef8a1137a70fab8bb9f50f14 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Thu, 11 Feb 2021 17:59:54 -0500 Subject: [PATCH] Remove 'unnecessary long for for link' warning This also makes the implementation slightly more efficient by only compiling the regex once. See https://github.com/rust-lang/rust/pull/81764#issuecomment-774122759 for why this was removed; essentially the benefit didn't seem great enough to deserve a lint. --- src/librustdoc/passes/non_autolinks.rs | 60 ++++++++------------- src/test/rustdoc-ui/url-improvements.rs | 12 +---- src/test/rustdoc-ui/url-improvements.stderr | 52 +++++++----------- 3 files changed, 44 insertions(+), 80 deletions(-) diff --git a/src/librustdoc/passes/non_autolinks.rs b/src/librustdoc/passes/non_autolinks.rs index 8378173d68b8..0ebd1879db52 100644 --- a/src/librustdoc/passes/non_autolinks.rs +++ b/src/librustdoc/passes/non_autolinks.rs @@ -4,9 +4,11 @@ use crate::core::DocContext; use crate::fold::DocFolder; use crate::html::markdown::opts; use core::ops::Range; -use pulldown_cmark::{Event, LinkType, Parser, Tag}; +use pulldown_cmark::{Event, Parser, Tag}; use regex::Regex; use rustc_errors::Applicability; +use std::lazy::SyncLazy; +use std::mem; crate const CHECK_NON_AUTOLINKS: Pass = Pass { name: "check-non-autolinks", @@ -14,16 +16,18 @@ crate const CHECK_NON_AUTOLINKS: Pass = Pass { description: "detects URLs that could be linkified", }; -const URL_REGEX: &str = concat!( - 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 -); +const URL_REGEX: SyncLazy = SyncLazy::new(|| { + Regex::new(concat!( + 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 + )) + .expect("failed to build regex") +}); struct NonAutolinksLinter<'a, 'tcx> { cx: &'a mut DocContext<'tcx>, - regex: Regex, } impl<'a, 'tcx> NonAutolinksLinter<'a, 'tcx> { @@ -33,8 +37,9 @@ impl<'a, 'tcx> NonAutolinksLinter<'a, 'tcx> { range: Range, f: &impl Fn(&DocContext<'_>, &str, &str, Range), ) { + trace!("looking for raw urls in {}", text); // For now, we only check "full" URLs (meaning, starting with "http://" or "https://"). - for match_ in self.regex.find_iter(&text) { + for match_ in URL_REGEX.find_iter(&text) { let url = match_.as_str(); let url_range = match_.range(); f( @@ -48,7 +53,7 @@ impl<'a, 'tcx> NonAutolinksLinter<'a, 'tcx> { } crate fn check_non_autolinks(krate: Crate, cx: &mut DocContext<'_>) -> Crate { - NonAutolinksLinter::new(cx).fold_crate(krate) + NonAutolinksLinter { cx }.fold_crate(krate) } impl<'a, 'tcx> DocFolder for NonAutolinksLinter<'a, 'tcx> { @@ -82,37 +87,16 @@ impl<'a, 'tcx> DocFolder for NonAutolinksLinter<'a, 'tcx> { while let Some((event, range)) = p.next() { match event { - Event::Start(Tag::Link(kind, _, _)) => { - let ignore = matches!(kind, LinkType::Autolink | LinkType::Email); - let mut title = String::new(); - - while let Some((event, range)) = p.next() { - match event { - Event::End(Tag::Link(_, url, _)) => { - // NOTE: links cannot be nested, so we don't need to - // check `kind` - if url.as_ref() == title && !ignore && self.regex.is_match(&url) - { - report_diag( - self.cx, - "unneeded long form for URL", - &url, - range, - ); - } - break; - } - Event::Text(s) if !ignore => title.push_str(&s), - _ => {} - } - } - } Event::Text(s) => self.find_raw_urls(&s, range, &report_diag), - Event::Start(Tag::CodeBlock(_)) => { - // We don't want to check the text inside the code blocks. + // We don't want to check the text inside code blocks or links. + Event::Start(tag @ (Tag::CodeBlock(_) | Tag::Link(..))) => { while let Some((event, _)) = p.next() { match event { - Event::End(Tag::CodeBlock(_)) => break, + Event::End(end) + if mem::discriminant(&end) == mem::discriminant(&tag) => + { + break; + } _ => {} } } diff --git a/src/test/rustdoc-ui/url-improvements.rs b/src/test/rustdoc-ui/url-improvements.rs index d0b43de2f0e0..b06ec4381469 100644 --- a/src/test/rustdoc-ui/url-improvements.rs +++ b/src/test/rustdoc-ui/url-improvements.rs @@ -1,15 +1,5 @@ #![deny(rustdoc::non_autolinks)] -/// [http://aa.com](http://aa.com) -//~^ ERROR unneeded long form for URL -/// [http://bb.com] -//~^ ERROR unneeded long form for URL -/// -/// [http://bb.com]: http://bb.com -/// -/// [http://c.com][http://c.com] -pub fn a() {} - /// https://somewhere.com //~^ ERROR this URL is not a hyperlink /// https://somewhere.com/a @@ -54,6 +44,8 @@ pub fn c() {} /// /// ``` /// This link should not be linted: http://example.com +/// +/// Nor this one: or this one: [x](http://example.com) /// ``` /// /// [should_not.lint](should_not.lint) diff --git a/src/test/rustdoc-ui/url-improvements.stderr b/src/test/rustdoc-ui/url-improvements.stderr index f377973656a8..404494d68023 100644 --- a/src/test/rustdoc-ui/url-improvements.stderr +++ b/src/test/rustdoc-ui/url-improvements.stderr @@ -1,8 +1,8 @@ -error: unneeded long form for URL +error: this URL is not a hyperlink --> $DIR/url-improvements.rs:3:5 | -LL | /// [http://aa.com](http://aa.com) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` +LL | /// https://somewhere.com + | ^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` | note: the lint level is defined here --> $DIR/url-improvements.rs:1:9 @@ -10,113 +10,101 @@ note: the lint level is defined here LL | #![deny(rustdoc::non_autolinks)] | ^^^^^^^^^^^^^^^^^^^^^^ -error: unneeded long form for URL +error: this URL is not a hyperlink --> $DIR/url-improvements.rs:5:5 | -LL | /// [http://bb.com] - | ^^^^^^^^^^^^^^^ help: use an automatic link instead: `` - -error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:13:5 - | -LL | /// https://somewhere.com - | ^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` - -error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:15:5 - | LL | /// https://somewhere.com/a | ^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:17:5 + --> $DIR/url-improvements.rs:7:5 | LL | /// https://www.somewhere.com | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:19:5 + --> $DIR/url-improvements.rs:9:5 | LL | /// https://www.somewhere.com/a | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:21:5 + --> $DIR/url-improvements.rs:11:5 | LL | /// https://subdomain.example.com | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:23:5 + --> $DIR/url-improvements.rs:13:5 | LL | /// https://somewhere.com? | ^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:25:5 + --> $DIR/url-improvements.rs:15:5 | LL | /// https://somewhere.com/a? | ^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:27:5 + --> $DIR/url-improvements.rs:17:5 | LL | /// https://somewhere.com?hello=12 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:29:5 + --> $DIR/url-improvements.rs:19:5 | LL | /// https://somewhere.com/a?hello=12 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:31:5 + --> $DIR/url-improvements.rs:21:5 | LL | /// https://example.com?hello=12#xyz | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:33:5 + --> $DIR/url-improvements.rs:23:5 | LL | /// https://example.com/a?hello=12#xyz | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:35:5 + --> $DIR/url-improvements.rs:25:5 | LL | /// https://example.com#xyz | ^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:37:5 + --> $DIR/url-improvements.rs:27:5 | LL | /// https://example.com/a#xyz | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:39:5 + --> $DIR/url-improvements.rs:29:5 | LL | /// https://somewhere.com?hello=12&bye=11 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:41:5 + --> $DIR/url-improvements.rs:31:5 | LL | /// https://somewhere.com/a?hello=12&bye=11 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:43:5 + --> $DIR/url-improvements.rs:33:5 | LL | /// https://somewhere.com?hello=12&bye=11#xyz | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` error: this URL is not a hyperlink - --> $DIR/url-improvements.rs:45:10 + --> $DIR/url-improvements.rs:35:10 | LL | /// hey! https://somewhere.com/a?hello=12&bye=11#xyz | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `` -error: aborting due to 19 previous errors +error: aborting due to 17 previous errors