Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stabilize rustdoc::bare_urls lint #81764

Merged
merged 5 commits into from
Apr 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions compiler/rustc_lint/src/levels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,15 +464,16 @@ impl<'s> LintLevelsBuilder<'s> {
// we don't warn about the name change.
if let CheckLintNameResult::Warning(_, Some(new_name)) = lint_result {
// Ignore any errors or warnings that happen because the new name is inaccurate
if let CheckLintNameResult::Ok(ids) =
store.check_lint_name(&new_name, tool_name)
{
// NOTE: `new_name` already includes the tool name, so we don't have to add it again.
if let CheckLintNameResult::Ok(ids) = store.check_lint_name(&new_name, None) {
let src =
LintLevelSource::Node(Symbol::intern(&new_name), li.span(), reason);
for &id in ids {
self.check_gated_lint(id, attr.span);
self.insert_spec(&mut specs, id, (level, src));
}
} else {
panic!("renamed lint does not exist: {}", new_name);
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,8 @@ pub mod primitive;
unused_imports,
unsafe_op_in_unsafe_fn
)]
#[allow(rustdoc::non_autolinks)]
#[cfg_attr(bootstrap, allow(rustdoc::non_autolinks))]
#[cfg_attr(not(bootstrap), allow(rustdoc::bare_urls))]
// FIXME: This annotation should be moved into rust-lang/stdarch after clashing_extern_declarations is
// merged. It currently cannot because bootstrap fails as the lint hasn't been defined yet.
#[allow(clashing_extern_declarations)]
Expand Down
23 changes: 8 additions & 15 deletions src/doc/rustdoc/src/lints.md
Original file line number Diff line number Diff line change
Expand Up @@ -294,40 +294,33 @@ warning: unclosed HTML tag `h1`
warning: 2 warnings emitted
```

## non_autolinks
## bare_urls

This lint is **nightly-only** and **warns by default**. It detects links which
could use the "automatic" link syntax. For example:
This lint is **warn-by-default**. It detects URLs which are not links.
For example:

```rust
/// http://example.org
/// [http://example.com](http://example.com)
/// [http://example.net]
///
/// [http://example.com]: http://example.com
pub fn foo() {}
```

Which will give:

```text
warning: this URL is not a hyperlink
--> foo.rs:1:5
--> links.rs:1:5
|
1 | /// http://example.org
| ^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<http://example.org>`
|
= note: `#[warn(rustdoc::non_autolinks)]` on by default

warning: unneeded long form for URL
--> foo.rs:2:5
|
2 | /// [http://example.com](http://example.com)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<http://example.com>`
= note: `#[warn(rustdoc::bare_urls)]` on by default

warning: this URL is not a hyperlink
--> foo.rs:3:6
--> links.rs:3:6
|
3 | /// [http://example.net]
| ^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<http://example.net>`

warning: 2 warnings emitted
```
15 changes: 8 additions & 7 deletions src/librustdoc/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,14 +148,13 @@ declare_rustdoc_lint! {
}

declare_rustdoc_lint! {
/// The `non_autolinks` lint detects when a URL could be written using
/// only angle brackets. This is a `rustdoc` only lint, see the
/// documentation in the [rustdoc book].
/// The `bare_urls` lint detects when a URL is not a hyperlink.
/// This is a `rustdoc` only lint, see the documentation in the [rustdoc book].
///
/// [rustdoc book]: ../../../rustdoc/lints.html#non_autolinks
NON_AUTOLINKS,
/// [rustdoc book]: ../../../rustdoc/lints.html#bare_urls
BARE_URLS,
Warn,
"detects URLs that could be written using only angle brackets"
"detects URLs that are not hyperlinks"
}

crate static RUSTDOC_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
Expand All @@ -166,7 +165,7 @@ crate static RUSTDOC_LINTS: Lazy<Vec<&'static Lint>> = Lazy::new(|| {
PRIVATE_DOC_TESTS,
INVALID_CODEBLOCK_ATTRIBUTES,
INVALID_HTML_TAGS,
NON_AUTOLINKS,
BARE_URLS,
MISSING_CRATE_LEVEL_DOCS,
]
});
Expand All @@ -185,4 +184,6 @@ crate fn register_lints(_sess: &Session, lint_store: &mut LintStore) {
}
lint_store
.register_renamed("intra_doc_link_resolution_failure", "rustdoc::broken_intra_doc_links");
lint_store.register_renamed("non_autolinks", "rustdoc::bare_urls");
lint_store.register_renamed("rustdoc::non_autolinks", "rustdoc::bare_urls");
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,42 @@ 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",
crate const CHECK_BARE_URLS: Pass = Pass {
name: "check-bare-urls",
run: check_bare_urls,
description: "detects URLs that are not hyperlinks",
};

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> {
struct BareUrlsLinter<'a, 'tcx> {
cx: &'a mut DocContext<'tcx>,
regex: Regex,
}

impl<'a, 'tcx> NonAutolinksLinter<'a, 'tcx> {
impl<'a, 'tcx> BareUrlsLinter<'a, 'tcx> {
fn find_raw_urls(
&self,
text: &str,
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 @@ -47,18 +52,11 @@ impl<'a, 'tcx> NonAutolinksLinter<'a, 'tcx> {
}
}

crate fn check_non_autolinks(krate: Crate, cx: &mut DocContext<'_>) -> Crate {
if !cx.tcx.sess.is_nightly_build() {
krate
} else {
let mut coll =
NonAutolinksLinter { cx, regex: Regex::new(URL_REGEX).expect("failed to build regex") };

coll.fold_crate(krate)
}
crate fn check_bare_urls(krate: Crate, cx: &mut DocContext<'_>) -> Crate {
BareUrlsLinter { cx }.fold_crate(krate)
}

impl<'a, 'tcx> DocFolder for NonAutolinksLinter<'a, 'tcx> {
impl<'a, 'tcx> DocFolder for BareUrlsLinter<'a, 'tcx> {
fn fold_item(&mut self, item: Item) -> Option<Item> {
let hir_id = match DocContext::as_local_hir_id(self.cx.tcx, item.def_id) {
Some(hir_id) => hir_id,
Expand All @@ -73,7 +71,7 @@ impl<'a, 'tcx> DocFolder for NonAutolinksLinter<'a, 'tcx> {
let sp = super::source_span_for_markdown_range(cx.tcx, &dox, &range, &item.attrs)
.or_else(|| span_of_attrs(&item.attrs))
.unwrap_or(item.span.inner());
cx.tcx.struct_span_lint_hir(crate::lint::NON_AUTOLINKS, hir_id, sp, |lint| {
cx.tcx.struct_span_lint_hir(crate::lint::BARE_URLS, hir_id, sp, |lint| {
lint.build(msg)
.span_suggestion(
sp,
Expand All @@ -89,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
8 changes: 4 additions & 4 deletions src/librustdoc/passes/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ use crate::core::DocContext;
mod stripper;
crate use stripper::*;

mod non_autolinks;
crate use self::non_autolinks::CHECK_NON_AUTOLINKS;
mod bare_urls;
crate use self::bare_urls::CHECK_BARE_URLS;

mod strip_hidden;
crate use self::strip_hidden::STRIP_HIDDEN;
Expand Down Expand Up @@ -90,7 +90,7 @@ crate const PASSES: &[Pass] = &[
COLLECT_TRAIT_IMPLS,
CALCULATE_DOC_COVERAGE,
CHECK_INVALID_HTML_TAGS,
CHECK_NON_AUTOLINKS,
CHECK_BARE_URLS,
];

/// The list of passes run by default.
Expand All @@ -105,7 +105,7 @@ crate const DEFAULT_PASSES: &[ConditionalPass] = &[
ConditionalPass::always(CHECK_CODE_BLOCK_SYNTAX),
ConditionalPass::always(CHECK_INVALID_HTML_TAGS),
ConditionalPass::always(PROPAGATE_DOC_CFG),
ConditionalPass::always(CHECK_NON_AUTOLINKS),
ConditionalPass::always(CHECK_BARE_URLS),
];

/// The list of default passes run when `--doc-coverage` is passed to rustdoc.
Expand Down
5 changes: 5 additions & 0 deletions src/test/rustdoc-ui/renamed-lint-still-applies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,8 @@
// stable channel.
//! [x]
//~^ ERROR unresolved link

#![deny(rustdoc::non_autolinks)]
//~^ WARNING renamed to `rustdoc::bare_urls`
//! http://example.com
//~^ ERROR not a hyperlink
22 changes: 21 additions & 1 deletion src/test/rustdoc-ui/renamed-lint-still-applies.stderr
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
warning: lint `rustdoc::non_autolinks` has been renamed to `rustdoc::bare_urls`
--> $DIR/renamed-lint-still-applies.rs:8:9
|
LL | #![deny(rustdoc::non_autolinks)]
| ^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `rustdoc::bare_urls`
|
= note: `#[warn(renamed_and_removed_lints)]` on by default

error: unresolved link to `x`
--> $DIR/renamed-lint-still-applies.rs:5:6
|
Expand All @@ -12,5 +20,17 @@ LL | #![deny(broken_intra_doc_links)]
= note: `#[deny(rustdoc::broken_intra_doc_links)]` implied by `#[deny(broken_intra_doc_links)]`
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

error: aborting due to previous error
error: this URL is not a hyperlink
--> $DIR/renamed-lint-still-applies.rs:10:5
|
LL | //! http://example.com
| ^^^^^^^^^^^^^^^^^^ help: use an automatic link instead: `<http://example.com>`
|
note: the lint level is defined here
--> $DIR/renamed-lint-still-applies.rs:8:9
|
LL | #![deny(rustdoc::non_autolinks)]
| ^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 2 previous errors; 1 warning emitted

6 changes: 5 additions & 1 deletion src/test/rustdoc-ui/unknown-renamed-lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,12 @@
//~^ ERROR unknown lint: `rustdoc::x`
#![deny(intra_doc_link_resolution_failure)]
//~^ ERROR renamed to `rustdoc::broken_intra_doc_links`

#![deny(non_autolinks)]
//~^ ERROR renamed to `rustdoc::bare_urls`
#![deny(rustdoc::non_autolinks)]
//~^ ERROR renamed to `rustdoc::bare_urls`

#![deny(private_doc_tests)]
// FIXME: the old names for rustdoc lints should warn by default once `rustdoc::` makes it to the
// stable channel.

Expand Down
18 changes: 15 additions & 3 deletions src/test/rustdoc-ui/unknown-renamed-lints.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,31 @@ note: the lint level is defined here
LL | #![deny(renamed_and_removed_lints)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: lint `non_autolinks` has been renamed to `rustdoc::bare_urls`
--> $DIR/unknown-renamed-lints.rs:11:9
|
LL | #![deny(non_autolinks)]
| ^^^^^^^^^^^^^ help: use the new name: `rustdoc::bare_urls`

error: lint `rustdoc::non_autolinks` has been renamed to `rustdoc::bare_urls`
--> $DIR/unknown-renamed-lints.rs:13:9
|
LL | #![deny(rustdoc::non_autolinks)]
| ^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `rustdoc::bare_urls`

error: lint `rustdoc` has been removed: use `rustdoc::all` instead
--> $DIR/unknown-renamed-lints.rs:16:9
--> $DIR/unknown-renamed-lints.rs:20:9
|
LL | #![deny(rustdoc)]
| ^^^^^^^

error: unknown lint: `rustdoc::intra_doc_link_resolution_failure`
--> $DIR/unknown-renamed-lints.rs:20:9
--> $DIR/unknown-renamed-lints.rs:24:9
|
LL | #![deny(rustdoc::intra_doc_link_resolution_failure)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: Compilation failed, aborting rustdoc

error: aborting due to 6 previous errors
error: aborting due to 8 previous errors

16 changes: 4 additions & 12 deletions src/test/rustdoc-ui/url-improvements.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,4 @@
#![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() {}
#![deny(rustdoc::bare_urls)]

/// https://somewhere.com
//~^ ERROR this URL is not a hyperlink
Expand Down Expand Up @@ -54,12 +44,14 @@ 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)
pub fn everything_is_fine_here() {}

#[allow(rustdoc::non_autolinks)]
#[allow(rustdoc::bare_urls)]
pub mod foo {
/// https://somewhere.com/a?hello=12&bye=11#xyz
pub fn bar() {}
Expand Down
Loading