about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJoshua Nelson <jyn514@gmail.com>2021-02-11 17:59:54 -0500
committerJoshua Nelson <jyn514@gmail.com>2021-04-05 03:57:44 -0400
commit6f89468fc513eb12ef8a1137a70fab8bb9f50f14 (patch)
tree0ab6ad850380becba3504ec236779c92ccd68fa2
parentfe1baa6498c948a48d60ae96cd38181a182051ca (diff)
downloadrust-6f89468fc513eb12ef8a1137a70fab8bb9f50f14.tar.gz
rust-6f89468fc513eb12ef8a1137a70fab8bb9f50f14.zip
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.
-rw-r--r--src/librustdoc/passes/non_autolinks.rs60
-rw-r--r--src/test/rustdoc-ui/url-improvements.rs12
-rw-r--r--src/test/rustdoc-ui/url-improvements.stderr52
3 files changed, 44 insertions, 80 deletions
diff --git a/src/librustdoc/passes/non_autolinks.rs b/src/librustdoc/passes/non_autolinks.rs
index 8378173d68b..0ebd1879db5 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<Regex> = 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<usize>,
         f: &impl Fn(&DocContext<'_>, &str, &str, Range<usize>),
     ) {
+        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 d0b43de2f0e..b06ec438146 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: <http://example.com> 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 f377973656a..404494d6802 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: `<http://aa.com>`
+LL | /// https://somewhere.com
+   |     ^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<https://somewhere.com>`
    |
 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
-  --> $DIR/url-improvements.rs:5:5
-   |
-LL | /// [http://bb.com]
-   |     ^^^^^^^^^^^^^^^ help: use an automatic link instead: `<http://bb.com>`
-
 error: this URL is not a hyperlink
-  --> $DIR/url-improvements.rs:13:5
-   |
-LL | /// https://somewhere.com
-   |     ^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<https://somewhere.com>`
-
-error: this URL is not a hyperlink
-  --> $DIR/url-improvements.rs:15:5
+  --> $DIR/url-improvements.rs:5:5
    |
 LL | /// https://somewhere.com/a
    |     ^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<https://somewhere.com/a>`
 
 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: `<https://www.somewhere.com>`
 
 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: `<https://www.somewhere.com/a>`
 
 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: `<https://subdomain.example.com>`
 
 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: `<https://somewhere.com?>`
 
 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: `<https://somewhere.com/a?>`
 
 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: `<https://somewhere.com?hello=12>`
 
 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: `<https://somewhere.com/a?hello=12>`
 
 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: `<https://example.com?hello=12#xyz>`
 
 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: `<https://example.com/a?hello=12#xyz>`
 
 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: `<https://example.com#xyz>`
 
 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: `<https://example.com/a#xyz>`
 
 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: `<https://somewhere.com?hello=12&bye=11>`
 
 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: `<https://somewhere.com/a?hello=12&bye=11>`
 
 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: `<https://somewhere.com?hello=12&bye=11#xyz>`
 
 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: `<https://somewhere.com/a?hello=12&bye=11#xyz>`
 
-error: aborting due to 19 previous errors
+error: aborting due to 17 previous errors