Extend automatic_links lint to take into account URLs without link syntax

This commit is contained in:
Guillaume Gomez 2020-10-13 15:46:34 +02:00
parent 6bc8965c41
commit f467b8d77c
6 changed files with 94 additions and 45 deletions

View File

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

View File

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

View File

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

View File

@ -3,23 +3,55 @@ use crate::clean::*;
use crate::core::DocContext; use crate::core::DocContext;
use crate::fold::DocFolder; use crate::fold::DocFolder;
use crate::html::markdown::opts; 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_feature::UnstableFeatures;
use rustc_session::lint; use rustc_session::lint;
pub const CHECK_AUTOMATIC_LINKS: Pass = Pass { pub const CHECK_AUTOMATIC_LINKS: Pass = Pass {
name: "check-automatic-links", name: "check-automatic-links",
run: 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> { struct AutomaticLinksLinter<'a, 'tcx> {
cx: &'a DocContext<'tcx>, cx: &'a DocContext<'tcx>,
regex: Regex,
} }
impl<'a, 'tcx> AutomaticLinksLinter<'a, 'tcx> { impl<'a, 'tcx> AutomaticLinksLinter<'a, 'tcx> {
fn new(cx: &'a DocContext<'tcx>) -> Self { 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(); let dox = item.attrs.collapsed_doc_value().unwrap_or_default();
if !dox.is_empty() { 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 p = Parser::new_ext(&dox, opts()).into_offset_iter();
let mut title = String::new(); let mut title = String::new();
let mut in_link = false; let mut in_link = false;
let mut ignore = false;
for (event, range) in p { for (event, range) in p {
match event { 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, _)) => { Event::End(Tag::Link(_, url, _)) => {
in_link = false; in_link = false;
if url.as_ref() != title { if url.as_ref() == title && !ignore {
continue; 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(); title.clear();
ignore = false;
} }
Event::Text(s) if in_link => { 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),
_ => {} _ => {}
} }
} }

View File

@ -1,15 +1,20 @@
#![deny(automatic_links)] #![deny(automatic_links)]
/// [http://a.com](http://a.com) /// [http://a.com](http://a.com)
//~^ ERROR Unneeded long form for URL //~^ ERROR unneeded long form for URL
/// [http://b.com] /// [http://b.com]
//~^ ERROR Unneeded long form for URL //~^ ERROR unneeded long form for URL
/// ///
/// [http://b.com]: http://b.com /// [http://b.com]: http://b.com
/// ///
/// [http://c.com][http://c.com] /// [http://c.com][http://c.com]
pub fn a() {} 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) /// [a](http://a.com)
/// [b] /// [b]
/// ///

View File

@ -1,23 +1,26 @@
error: Unneeded long form for URL error: unneeded long form for URL
--> $DIR/automatic-links.rs:3:5 --> $DIR/automatic-links.rs:3:5
| |
LL | /// [http://a.com](http://a.com) LL | /// [http://a.com](http://a.com)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<http://a.com>`
| |
note: the lint level is defined here note: the lint level is defined here
--> $DIR/automatic-links.rs:1:9 --> $DIR/automatic-links.rs:1:9
| |
LL | #![deny(automatic_links)] 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 --> $DIR/automatic-links.rs:5:5
| |
LL | /// [http://b.com] 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