Skip to content

Commit

Permalink
Remove 'unnecessary long for for link' warning
Browse files Browse the repository at this point in the history 
This also makes the implementation slightly more efficient by only
compiling the regex once.

See #81764 (comment)
for why this was removed; essentially the benefit didn't seem great
enough to deserve a lint.
  • Loading branch information
@jyn514
jyn514 committed Apr 5, 2021
1 parent fe1baa6 commit 6f89468
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 80 deletions.
60 changes: 22 additions & 38 deletions src/librustdoc/passes/non_autolinks.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,30 @@ 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",
run: check_non_autolinks,
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> {
Expand All @@ -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(
Expand All @@ -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> {
Expand Down Expand Up @@ -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;
}
_ => {}
}
}
Expand Down
12 changes: 2 additions & 10 deletions src/test/rustdoc-ui/url-improvements.rs
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)
Expand Down
52 changes: 20 additions & 32 deletions src/test/rustdoc-ui/url-improvements.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,122 +1,110 @@
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
|
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

0 comments on commit 6f89468

Please sign in to comment.