Rollup merge of #81764 - jyn514:lint-links, r=GuillaumeGomez

Dylan-DPC · web-flow · commit 9aed9c135375 · 2021-04-08T20:29:56.000+02:00
Stabilize `rustdoc::bare_urls` lint Closes #77501. Closes #83598.
diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs
@@ -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);
                     }
                 }
             }
diff --git a/library/core/src/lib.rs b/library/core/src/lib.rs
@@ -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)]
diff --git a/src/doc/rustdoc/src/lints.md b/src/doc/rustdoc/src/lints.md
@@ -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
 ```
diff --git a/src/librustdoc/lint.rs b/src/librustdoc/lint.rs
@@ -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(|| {
@@ -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,
     ]
 });
@@ -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");
 }
diff --git a/src/librustdoc/passes/bare_urls.rs b/src/librustdoc/passes/bare_urls.rs
@@ -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(
@@ -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,
@@ -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,
@@ -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;
+                                }
                                 _ => {}
                             }
                         }
diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs
@@ -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;
@@ -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.
@@ -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.
diff --git a/src/test/rustdoc-ui/renamed-lint-still-applies.rs b/src/test/rustdoc-ui/renamed-lint-still-applies.rs
@@ -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
diff --git a/src/test/rustdoc-ui/renamed-lint-still-applies.stderr b/src/test/rustdoc-ui/renamed-lint-still-applies.stderr
@@ -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
    |
@@ -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
 
diff --git a/src/test/rustdoc-ui/unknown-renamed-lints.rs b/src/test/rustdoc-ui/unknown-renamed-lints.rs
@@ -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.
 
diff --git a/src/test/rustdoc-ui/unknown-renamed-lints.stderr b/src/test/rustdoc-ui/unknown-renamed-lints.stderr
@@ -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
 
diff --git a/src/test/rustdoc-ui/url-improvements.rs b/src/test/rustdoc-ui/url-improvements.rs
@@ -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
@@ -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() {}
diff --git a/src/test/rustdoc-ui/url-improvements.stderr b/src/test/rustdoc-ui/url-improvements.stderr

Original file line number	Diff line number	Diff line change
`@@ -464,15 +464,16 @@ impl<'s> LintLevelsBuilder<'s> {`
`464`	`464`	`// we don't warn about the name change.`
`465`	`465`	`if let CheckLintNameResult::Warning(_, Some(new_name)) = lint_result {`
`466`	`466`	`// Ignore any errors or warnings that happen because the new name is inaccurate`
`467`		`- if let CheckLintNameResult::Ok(ids) =`
`468`		`- store.check_lint_name(&new_name, tool_name)`
`469`		`- {`
	`467`	+ // NOTE: `new_name` already includes the tool name, so we don't have to add it again.
	`468`	`+ if let CheckLintNameResult::Ok(ids) = store.check_lint_name(&new_name, None) {`
`470`	`469`	`let src =`
`471`	`470`	`LintLevelSource::Node(Symbol::intern(&new_name), li.span(), reason);`
`472`	`471`	`for &id in ids {`
`473`	`472`	`self.check_gated_lint(id, attr.span);`
`474`	`473`	`self.insert_spec(&mut specs, id, (level, src));`
`475`	`474`	`}`
	`475`	`+ } else {`
	`476`	`+ panic!("renamed lint does not exist: {}", new_name);`
`476`	`477`	`}`
`477`	`478`	`}`
`478`	`479`	`}`