Extend automatic_links lint to take into account URLs without link syntax

Author: GuillaumeGomez

Cargo.lock

@@ -4203,6 +4203,7 @@ dependencies = [
  "itertools 0.9.0",
  "minifier",
  "pulldown-cmark 0.8.0",
+ "regex",
  "rustc-rayon",
  "serde",
  "serde_json",

src/doc/rustdoc/src/lints.md

@@ -294,6 +294,7 @@ which could use the "automatic" link syntax. For example:
 ```rust
 #![warn(automatic_links)]
 
+/// http://hello.rs
 /// [http://a.com](http://a.com)
 /// [http://b.com]
 ///
@@ -304,24 +305,27 @@ pub fn foo() {}
 Which will give:
 
 ```text
-error: Unneeded long form for URL
+warning: won't be a link as is
  --> foo.rs:3:5
   |
-3 | /// [http://a.com](http://a.com)
-  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+3 | /// http://hello.rs
+  |     ^^^^^^^^^^^^^^^ help: use an automatic link instead: `<http://hello.rs>`
   |
 note: the lint level is defined here
  --> foo.rs:1:9
   |
-1 | #![deny(automatic_links)]
+1 | #![warn(automatic_links)]
   |         ^^^^^^^^^^^^^^^
-  = help: Try with `<http://a.com>` instead
 
-error: Unneeded long form for URL
+warning: unneeded long form for URL
+ --> foo.rs:4:5
+  |
+4 | /// [http://a.com](http://a.com)
+  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<http://a.com>`
+
+warning: unneeded long form for URL
  --> foo.rs:5:5
   |
 5 | /// [http://b.com]
-  |     ^^^^^^^^^^^^^^
-  |
-  = help: Try with `<http://b.com>` instead
+  |     ^^^^^^^^^^^^^^ help: use an automatic link instead: `<http://b.com>`
 ```

src/librustdoc/Cargo.toml

@@ -16,6 +16,7 @@ serde_json = "1.0"
 smallvec = "1.0"
 tempfile = "3"
 itertools = "0.9"
+regex = "1"
 
 [dev-dependencies]
 expect-test = "1.0"

src/librustdoc/passes/automatic_links.rs

@@ -3,23 +3,55 @@ use crate::clean::*;
 use crate::core::DocContext;
 use crate::fold::DocFolder;
 use crate::html::markdown::opts;
-use pulldown_cmark::{Event, Parser, Tag};
+use core::ops::Range;
+use pulldown_cmark::{Event, LinkType, Parser, Tag};
+use regex::Regex;
+use rustc_errors::Applicability;
 use rustc_feature::UnstableFeatures;
 use rustc_session::lint;
 
 pub const CHECK_AUTOMATIC_LINKS: Pass = Pass {
     name: "check-automatic-links",
     run: check_automatic_links,
-    description: "detects URLS/email addresses that could be written using brackets",
+    description: "detects URLS/email addresses that could be written using angle brackets",
 };
 
+const URL_REGEX: &str =
+    r"https?://(www\.)?[-a-zA-Z0-9@:%._\+~#=]{2,256}\.[a-z]{2,4}\b([-a-zA-Z0-9@:%_\+.~#?&//=]*)";
+
 struct AutomaticLinksLinter<'a, 'tcx> {
     cx: &'a DocContext<'tcx>,
+    regex: Regex,
 }
 
 impl<'a, 'tcx> AutomaticLinksLinter<'a, 'tcx> {
     fn new(cx: &'a DocContext<'tcx>) -> Self {
-        AutomaticLinksLinter { cx }
+        AutomaticLinksLinter { cx, regex: Regex::new(URL_REGEX).expect("failed to build regex") }
+    }
+
+    fn find_raw_urls(
+        &self,
+        text: &str,
+        range: Range<usize>,
+        f: &impl Fn(&DocContext<'_>, &str, &str, Range<usize>),
+    ) {
+        for (pos, c) in text.char_indices() {
+            // For now, we only check "full" URLs.
+            if c == 'h' {
+                let text = &text[pos..];
+                if text.starts_with("http://") || text.starts_with("https://") {
+                    if let Some(m) = self.regex.find(text) {
+                        let url = &text[..m.end()];
+                        f(
+                            self.cx,
+                            "won't be a link as is",
+                            url,
+                            Range { start: range.start + pos, end: range.start + pos + m.end() },
+                        )
+                    }
+                }
+            }
+        }
     }
 }
 
@@ -44,45 +76,48 @@ impl<'a, 'tcx> DocFolder for AutomaticLinksLinter<'a, 'tcx> {
         };
         let dox = item.attrs.collapsed_doc_value().unwrap_or_default();
         if !dox.is_empty() {
-            let cx = &self.cx;
+            let report_diag = |cx: &DocContext<'_>, msg: &str, url: &str, range: Range<usize>| {
+                let sp = super::source_span_for_markdown_range(cx, &dox, &range, &item.attrs)
+                    .or_else(|| span_of_attrs(&item.attrs))
+                    .unwrap_or(item.source.span());
+                cx.tcx.struct_span_lint_hir(lint::builtin::AUTOMATIC_LINKS, hir_id, sp, |lint| {
+                    lint.build(msg)
+                        .span_suggestion(
+                            sp,
+                            "use an automatic link instead",
+                            format!("<{}>", url),
+                            Applicability::MachineApplicable,
+                        )
+                        .emit()
+                });
+            };
 
             let p = Parser::new_ext(&dox, opts()).into_offset_iter();
 
             let mut title = String::new();
             let mut in_link = false;
+            let mut ignore = false;
 
             for (event, range) in p {
                 match event {
-                    Event::Start(Tag::Link(..)) => in_link = true,
+                    Event::Start(Tag::Link(kind, _, _)) => {
+                        in_link = true;
+                        ignore = matches!(kind, LinkType::Autolink | LinkType::Email);
+                    }
                     Event::End(Tag::Link(_, url, _)) => {
                         in_link = false;
-                        if url.as_ref() != title {
-                            continue;
+                        if url.as_ref() == title && !ignore {
+                            report_diag(self.cx, "unneeded long form for URL", &url, range);
                         }
-                        let sp = match super::source_span_for_markdown_range(
-                            cx,
-                            &dox,
-                            &range,
-                            &item.attrs,
-                        ) {
-                            Some(sp) => sp,
-                            None => span_of_attrs(&item.attrs).unwrap_or(item.source.span()),
-                        };
-                        cx.tcx.struct_span_lint_hir(
-                            lint::builtin::AUTOMATIC_LINKS,
-                            hir_id,
-                            sp,
-                            |lint| {
-                                lint.build("Unneeded long form for URL")
-                                    .help(&format!("Try with `<{}>` instead", url))
-                                    .emit()
-                            },
-                        );
                         title.clear();
+                        ignore = false;
                     }
                     Event::Text(s) if in_link => {
-                        title.push_str(&s);
+                        if !ignore {
+                            title.push_str(&s);
+                        }
                     }
+                    Event::Text(s) => self.find_raw_urls(&s, range, &report_diag),
                     _ => {}
                 }
             }

src/test/rustdoc-ui/automatic-links.rs

@@ -1,15 +1,20 @@
 #![deny(automatic_links)]
 
 /// [http://a.com](http://a.com)
-//~^ ERROR Unneeded long form for URL
+//~^ ERROR unneeded long form for URL
 /// [http://b.com]
-//~^ ERROR Unneeded long form for URL
+//~^ ERROR unneeded long form for URL
 ///
 /// [http://b.com]: http://b.com
 ///
 /// [http://c.com][http://c.com]
 pub fn a() {}
 
+/// https://somewhere.com?hello=12
+//~^ ERROR won't be a link as is
+pub fn c() {}
+
+/// <https://somewhere.com>
 /// [a](http://a.com)
 /// [b]
 ///

src/test/rustdoc-ui/automatic-links.stderr

@@ -1,23 +1,26 @@
-error: Unneeded long form for URL
+error: unneeded long form for URL
   --> $DIR/automatic-links.rs:3:5
    |
 LL | /// [http://a.com](http://a.com)
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<http://a.com>`
    |
 note: the lint level is defined here
   --> $DIR/automatic-links.rs:1:9
    |
 LL | #![deny(automatic_links)]
    |         ^^^^^^^^^^^^^^^
-   = help: Try with `<http://a.com>` instead
 
-error: Unneeded long form for URL
+error: unneeded long form for URL
   --> $DIR/automatic-links.rs:5:5
    |
 LL | /// [http://b.com]
-   |     ^^^^^^^^^^^^^^
+   |     ^^^^^^^^^^^^^^ help: use an automatic link instead: `<http://b.com>`
+
+error: won't be a link as is
+  --> $DIR/automatic-links.rs:13:5
    |
-   = help: Try with `<http://b.com>` instead
+LL | /// https://somewhere.com?hello=12
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<https://somewhere.com?hello=12>`
 
-error: aborting due to 2 previous errors
+error: aborting due to 3 previous errors