From e6027a42e109fef10f4fc27ebede50d1b3d203f0 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 23 Sep 2020 20:25:56 +0200 Subject: [PATCH 01/12] Add `unclosed_html_tags` lint --- compiler/rustc_lint/src/lib.rs | 7 +- compiler/rustc_session/src/lint/builtin.rs | 11 ++ src/librustdoc/core.rs | 2 + src/librustdoc/html/markdown.rs | 2 +- src/librustdoc/passes/html_tags.rs | 135 +++++++++++++++++++ src/librustdoc/passes/mod.rs | 5 + src/test/rustdoc-ui/intra-link-errors.rs | 1 + src/test/rustdoc-ui/intra-link-errors.stderr | 59 +++++++- 8 files changed, 213 insertions(+), 9 deletions(-) create mode 100644 src/librustdoc/passes/html_tags.rs diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index 33caedfc19826..591be41716196 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -63,8 +63,8 @@ use rustc_middle::ty::query::Providers; use rustc_middle::ty::TyCtxt; use rustc_session::lint::builtin::{ BARE_TRAIT_OBJECTS, BROKEN_INTRA_DOC_LINKS, ELIDED_LIFETIMES_IN_PATHS, - EXPLICIT_OUTLIVES_REQUIREMENTS, INVALID_CODEBLOCK_ATTRIBUTES, MISSING_DOC_CODE_EXAMPLES, - PRIVATE_DOC_TESTS, + EXPLICIT_OUTLIVES_REQUIREMENTS, INVALID_CODEBLOCK_ATTRIBUTES, INVALID_HTML_TAGS, + MISSING_DOC_CODE_EXAMPLES, PRIVATE_DOC_TESTS, }; use rustc_span::symbol::{Ident, Symbol}; use rustc_span::Span; @@ -308,7 +308,8 @@ fn register_builtins(store: &mut LintStore, no_interleave_lints: bool) { PRIVATE_INTRA_DOC_LINKS, INVALID_CODEBLOCK_ATTRIBUTES, MISSING_DOC_CODE_EXAMPLES, - PRIVATE_DOC_TESTS + PRIVATE_DOC_TESTS, + INVALID_HTML_TAGS ); // Register renamed and removed lints. diff --git a/compiler/rustc_session/src/lint/builtin.rs b/compiler/rustc_session/src/lint/builtin.rs index 0cc97fb4541d1..ce2a14b448149 100644 --- a/compiler/rustc_session/src/lint/builtin.rs +++ b/compiler/rustc_session/src/lint/builtin.rs @@ -1881,6 +1881,16 @@ declare_lint! { "detects code samples in docs of private items not documented by rustdoc" } +declare_lint! { + /// The `invalid_html_tags` lint detects invalid HTML tags. This is a + /// `rustdoc` only lint, see the documentation in the [rustdoc book]. + /// + /// [rustdoc book]: ../../../rustdoc/lints.html#invalid_html_tags + pub INVALID_HTML_TAGS, + Warn, + "detects invalid HTML tags in doc comments" +} + declare_lint! { /// The `where_clauses_object_safety` lint detects for [object safety] of /// [where clauses]. @@ -2699,6 +2709,7 @@ declare_lint_pass! { INVALID_CODEBLOCK_ATTRIBUTES, MISSING_CRATE_LEVEL_DOCS, MISSING_DOC_CODE_EXAMPLES, + INVALID_HTML_TAGS, PRIVATE_DOC_TESTS, WHERE_CLAUSES_OBJECT_SAFETY, PROC_MACRO_DERIVE_RESOLUTION_FALLBACK, diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 391859050e8a6..45a84c4fb30d3 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -328,6 +328,7 @@ pub fn run_core( let private_doc_tests = rustc_lint::builtin::PRIVATE_DOC_TESTS.name; let no_crate_level_docs = rustc_lint::builtin::MISSING_CRATE_LEVEL_DOCS.name; let invalid_codeblock_attributes_name = rustc_lint::builtin::INVALID_CODEBLOCK_ATTRIBUTES.name; + let invalid_html_tags = rustc_lint::builtin::INVALID_HTML_TAGS.name; let renamed_and_removed_lints = rustc_lint::builtin::RENAMED_AND_REMOVED_LINTS.name; let unknown_lints = rustc_lint::builtin::UNKNOWN_LINTS.name; @@ -340,6 +341,7 @@ pub fn run_core( private_doc_tests.to_owned(), no_crate_level_docs.to_owned(), invalid_codeblock_attributes_name.to_owned(), + invalid_html_tags.to_owned(), renamed_and_removed_lints.to_owned(), unknown_lints.to_owned(), ]; diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 6c0f1c02ac6da..2fd06d7e5730f 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -43,7 +43,7 @@ use pulldown_cmark::{html, BrokenLink, CodeBlockKind, CowStr, Event, Options, Pa #[cfg(test)] mod tests; -fn opts() -> Options { +pub(crate) fn opts() -> Options { Options::ENABLE_TABLES | Options::ENABLE_FOOTNOTES | Options::ENABLE_STRIKETHROUGH } diff --git a/src/librustdoc/passes/html_tags.rs b/src/librustdoc/passes/html_tags.rs new file mode 100644 index 0000000000000..b177eaeeb7320 --- /dev/null +++ b/src/librustdoc/passes/html_tags.rs @@ -0,0 +1,135 @@ +use super::{span_of_attrs, Pass}; +use crate::clean::*; +use crate::core::DocContext; +use crate::fold::DocFolder; +use crate::html::markdown::opts; +use pulldown_cmark::{Event, Parser}; +use rustc_hir::hir_id::HirId; +use rustc_session::lint; +use rustc_span::Span; + +pub const CHECK_INVALID_HTML_TAGS: Pass = Pass { + name: "check-invalid-html-tags", + run: check_invalid_html_tags, + description: "detects invalid HTML tags in doc comments", +}; + +struct InvalidHtmlTagsLinter<'a, 'tcx> { + cx: &'a DocContext<'tcx>, +} + +impl<'a, 'tcx> InvalidHtmlTagsLinter<'a, 'tcx> { + fn new(cx: &'a DocContext<'tcx>) -> Self { + InvalidHtmlTagsLinter { cx } + } +} + +pub fn check_invalid_html_tags(krate: Crate, cx: &DocContext<'_>) -> Crate { + let mut coll = InvalidHtmlTagsLinter::new(cx); + + coll.fold_crate(krate) +} + +const ALLOWED_UNCLOSED: &[&str] = &[ + "area", "base", "br", "col", "embed", "hr", "img", "input", "keygen", "link", "meta", "param", + "source", "track", "wbr", +]; + +fn drop_tag( + cx: &DocContext<'_>, + tags: &mut Vec, + tag_name: String, + hir_id: HirId, + sp: Span, +) { + if let Some(pos) = tags.iter().position(|t| *t == tag_name) { + for _ in pos + 1..tags.len() { + if ALLOWED_UNCLOSED.iter().find(|&at| at == &tags[pos + 1]).is_some() { + continue; + } + // `tags` is used as a queue, meaning that everything after `pos` is included inside it. + // So `

` will look like `["h2", "h3"]`. So when closing `h2`, we will still + // have `h3`, meaning the tag wasn't closed as it should have. + cx.tcx.struct_span_lint_hir(lint::builtin::INVALID_HTML_TAGS, hir_id, sp, |lint| { + lint.build(&format!("unclosed HTML tag `{}`", tags[pos + 1])).emit() + }); + tags.remove(pos + 1); + } + tags.remove(pos); + } else { + // It can happen for example in this case: `

` (the `h2` tag isn't required + // but it helps for the visualization). + cx.tcx.struct_span_lint_hir(lint::builtin::INVALID_HTML_TAGS, hir_id, sp, |lint| { + lint.build(&format!("unopened HTML tag `{}`", tag_name)).emit() + }); + } +} + +fn extract_tag(cx: &DocContext<'_>, tags: &mut Vec, text: &str, hir_id: HirId, sp: Span) { + let mut iter = text.chars().peekable(); + + while let Some(c) = iter.next() { + if c == '<' { + let mut tag_name = String::new(); + let mut is_closing = false; + while let Some(&c) = iter.peek() { + // + if c == '/' && tag_name.is_empty() { + is_closing = true; + } else if c.is_ascii_alphanumeric() && !c.is_ascii_uppercase() { + tag_name.push(c); + } else { + break; + } + iter.next(); + } + if tag_name.is_empty() { + // Not an HTML tag presumably... + continue; + } + if is_closing { + drop_tag(cx, tags, tag_name, hir_id, sp); + } else { + tags.push(tag_name); + } + } + } +} + +impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> { + fn fold_item(&mut self, item: Item) -> Option { + let hir_id = match self.cx.as_local_hir_id(item.def_id) { + Some(hir_id) => hir_id, + None => { + // If non-local, no need to check anything. + return None; + } + }; + let dox = item.attrs.collapsed_doc_value().unwrap_or_default(); + if !dox.is_empty() { + let sp = span_of_attrs(&item.attrs).unwrap_or(item.source.span()); + let mut tags = Vec::new(); + + let p = Parser::new_ext(&dox, opts()); + + for event in p { + match event { + Event::Html(text) => extract_tag(self.cx, &mut tags, &text, hir_id, sp), + _ => {} + } + } + + for tag in tags.iter().filter(|t| ALLOWED_UNCLOSED.iter().find(|at| at == t).is_none()) + { + self.cx.tcx.struct_span_lint_hir( + lint::builtin::INVALID_HTML_TAGS, + hir_id, + sp, + |lint| lint.build(&format!("unclosed HTML tag `{}`", tag)).emit(), + ); + } + } + + self.fold_item_recur(item) + } +} diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 75a659666673f..3819aaee56000 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -45,6 +45,9 @@ pub use self::check_code_block_syntax::CHECK_CODE_BLOCK_SYNTAX; mod calculate_doc_coverage; pub use self::calculate_doc_coverage::CALCULATE_DOC_COVERAGE; +mod html_tags; +pub use self::html_tags::CHECK_INVALID_HTML_TAGS; + /// A single pass over the cleaned documentation. /// /// Runs in the compiler context, so it has access to types and traits and the like. @@ -87,6 +90,7 @@ pub const PASSES: &[Pass] = &[ CHECK_CODE_BLOCK_SYNTAX, COLLECT_TRAIT_IMPLS, CALCULATE_DOC_COVERAGE, + CHECK_INVALID_HTML_TAGS, ]; /// The list of passes run by default. @@ -101,6 +105,7 @@ pub const DEFAULT_PASSES: &[ConditionalPass] = &[ ConditionalPass::always(COLLECT_INTRA_DOC_LINKS), ConditionalPass::always(CHECK_CODE_BLOCK_SYNTAX), ConditionalPass::always(PROPAGATE_DOC_CFG), + ConditionalPass::always(CHECK_INVALID_HTML_TAGS), ]; /// The list of default passes run when `--doc-coverage` is passed to rustdoc. diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index 0278caf308776..bd4db6ad61757 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -1,3 +1,4 @@ +#![allow(unclosed_html_tags)] #![deny(broken_intra_doc_links)] //~^ NOTE lint level is defined diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index b63f799535a1f..13ed978d61380 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -1,35 +1,40 @@ error: unresolved link to `path::to::nonexistent::module` - --> $DIR/intra-link-errors.rs:7:6 + --> $DIR/intra-link-errors.rs:8:6 | LL | /// [path::to::nonexistent::module] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the module `intra_link_errors` contains no item named `path` | note: the lint level is defined here - --> $DIR/intra-link-errors.rs:1:9 + --> $DIR/intra-link-errors.rs:2:9 | LL | #![deny(broken_intra_doc_links)] | ^^^^^^^^^^^^^^^^^^^^^^ error: unresolved link to `path::to::nonexistent::macro` - --> $DIR/intra-link-errors.rs:11:6 + --> $DIR/intra-link-errors.rs:12:6 | LL | /// [path::to::nonexistent::macro!] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the module `intra_link_errors` contains no item named `path` error: unresolved link to `path::to::nonexistent::type` - --> $DIR/intra-link-errors.rs:15:6 + --> $DIR/intra-link-errors.rs:16:6 | LL | /// [type@path::to::nonexistent::type] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the module `intra_link_errors` contains no item named `path` error: unresolved link to `std::io::not::here` - --> $DIR/intra-link-errors.rs:19:6 + --> $DIR/intra-link-errors.rs:20:6 | LL | /// [std::io::not::here] | ^^^^^^^^^^^^^^^^^^ the module `io` contains no item named `not` +<<<<<<< HEAD error: unresolved link to `std::io::not::here` --> $DIR/intra-link-errors.rs:23:6 +======= +error: unresolved link to `std::io::Error::x` + --> $DIR/intra-link-errors.rs:24:6 +>>>>>>> Add `unclosed_html_tags` lint | LL | /// [type@std::io::not::here] | ^^^^^^^^^^^^^^^^^^^^^^^ the module `io` contains no item named `not` @@ -41,13 +46,21 @@ LL | /// [std::io::Error::x] | ^^^^^^^^^^^^^^^^^ the struct `Error` has no field or associated item named `x` error: unresolved link to `std::io::ErrorKind::x` +<<<<<<< HEAD --> $DIR/intra-link-errors.rs:31:6 +======= + --> $DIR/intra-link-errors.rs:28:6 +>>>>>>> Add `unclosed_html_tags` lint | LL | /// [std::io::ErrorKind::x] | ^^^^^^^^^^^^^^^^^^^^^ the enum `ErrorKind` has no variant or associated item named `x` error: unresolved link to `f::A` +<<<<<<< HEAD --> $DIR/intra-link-errors.rs:35:6 +======= + --> $DIR/intra-link-errors.rs:32:6 +>>>>>>> Add `unclosed_html_tags` lint | LL | /// [f::A] | ^^^^ `f` is a function, not a module or type, and cannot have associated items @@ -59,25 +72,41 @@ LL | /// [f::A!] | ^^^^^ `f` is a function, not a module or type, and cannot have associated items error: unresolved link to `S::A` +<<<<<<< HEAD --> $DIR/intra-link-errors.rs:43:6 +======= + --> $DIR/intra-link-errors.rs:36:6 +>>>>>>> Add `unclosed_html_tags` lint | LL | /// [S::A] | ^^^^ the struct `S` has no field or associated item named `A` error: unresolved link to `S::fmt` +<<<<<<< HEAD --> $DIR/intra-link-errors.rs:47:6 +======= + --> $DIR/intra-link-errors.rs:40:6 +>>>>>>> Add `unclosed_html_tags` lint | LL | /// [S::fmt] | ^^^^^^ the struct `S` has no field or associated item named `fmt` error: unresolved link to `E::D` +<<<<<<< HEAD --> $DIR/intra-link-errors.rs:51:6 +======= + --> $DIR/intra-link-errors.rs:44:6 +>>>>>>> Add `unclosed_html_tags` lint | LL | /// [E::D] | ^^^^ the enum `E` has no variant or associated item named `D` error: unresolved link to `u8::not_found` +<<<<<<< HEAD --> $DIR/intra-link-errors.rs:55:6 +======= + --> $DIR/intra-link-errors.rs:48:6 +>>>>>>> Add `unclosed_html_tags` lint | LL | /// [u8::not_found] | ^^^^^^^^^^^^^ the builtin type `u8` has no associated item named `not_found` @@ -98,7 +127,11 @@ LL | /// [type@Vec::into_iter] | help: to link to the associated function, add parentheses: `Vec::into_iter()` error: unresolved link to `S` +<<<<<<< HEAD --> $DIR/intra-link-errors.rs:68:6 +======= + --> $DIR/intra-link-errors.rs:52:6 +>>>>>>> Add `unclosed_html_tags` lint | LL | /// [S!] | ^^ @@ -107,7 +140,11 @@ LL | /// [S!] | help: to link to the struct, prefix with `struct@`: `struct@S` error: unresolved link to `T::g` +<<<<<<< HEAD --> $DIR/intra-link-errors.rs:86:6 +======= + --> $DIR/intra-link-errors.rs:70:6 +>>>>>>> Add `unclosed_html_tags` lint | LL | /// [type@T::g] | ^^^^^^^^^ @@ -116,13 +153,21 @@ LL | /// [type@T::g] | help: to link to the associated function, add parentheses: `T::g()` error: unresolved link to `T::h` +<<<<<<< HEAD --> $DIR/intra-link-errors.rs:91:6 +======= + --> $DIR/intra-link-errors.rs:75:6 +>>>>>>> Add `unclosed_html_tags` lint | LL | /// [T::h!] | ^^^^^ the trait `T` has no macro named `h` error: unresolved link to `S::h` +<<<<<<< HEAD --> $DIR/intra-link-errors.rs:78:6 +======= + --> $DIR/intra-link-errors.rs:62:6 +>>>>>>> Add `unclosed_html_tags` lint | LL | /// [type@S::h] | ^^^^^^^^^ @@ -131,7 +176,11 @@ LL | /// [type@S::h] | help: to link to the associated function, add parentheses: `S::h()` error: unresolved link to `m` +<<<<<<< HEAD --> $DIR/intra-link-errors.rs:98:6 +======= + --> $DIR/intra-link-errors.rs:82:6 +>>>>>>> Add `unclosed_html_tags` lint | LL | /// [m()] | ^^^ From 5fcbf4668ecab0ff798ea076757e27760c90aff5 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 24 Sep 2020 13:49:40 +0200 Subject: [PATCH 02/12] Add doc for invalid_html_tag lint --- src/doc/rustdoc/src/lints.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/doc/rustdoc/src/lints.md b/src/doc/rustdoc/src/lints.md index 3e632a0644a73..d6ae665ba05b3 100644 --- a/src/doc/rustdoc/src/lints.md +++ b/src/doc/rustdoc/src/lints.md @@ -250,3 +250,36 @@ warning: unknown attribute `should-panic`. Did you mean `should_panic`? In the example above, the correct form is `should_panic`. This helps detect typo mistakes for some common attributes. + +## invalid_html_tags + +This lint **warns by default**. It detects unclosed or invalid HTML tags. +For example: + +```rust +///

+/// +pub fn foo() {} +``` + +Which will give: + +```text +warning: unopened HTML tag `script` + --> foo.rs:1:1 + | +1 | / ///

+2 | | /// + | |_____________^ + | + = note: `#[warn(invalid_html_tags)]` on by default + +warning: unclosed HTML tag `h1` + --> foo.rs:1:1 + | +1 | / ///

+2 | | /// + | |_____________^ + +warning: 2 warnings emitted +``` From bc6ec6fe36c5c902da900fea67cba30fad4b0b6b Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 23 Sep 2020 20:26:29 +0200 Subject: [PATCH 03/12] Add test for unclosed_html_tag lint --- src/test/rustdoc-ui/intra-link-errors.rs | 2 +- src/test/rustdoc-ui/intra-link-errors.stderr | 79 ++++---------------- src/test/rustdoc-ui/invalid-html-tags.rs | 19 +++++ src/test/rustdoc-ui/invalid-html-tags.stderr | 54 +++++++++++++ 4 files changed, 89 insertions(+), 65 deletions(-) create mode 100644 src/test/rustdoc-ui/invalid-html-tags.rs create mode 100644 src/test/rustdoc-ui/invalid-html-tags.stderr diff --git a/src/test/rustdoc-ui/intra-link-errors.rs b/src/test/rustdoc-ui/intra-link-errors.rs index bd4db6ad61757..8904828b047ca 100644 --- a/src/test/rustdoc-ui/intra-link-errors.rs +++ b/src/test/rustdoc-ui/intra-link-errors.rs @@ -1,4 +1,4 @@ -#![allow(unclosed_html_tags)] +#![allow(invalid_html_tags)] #![deny(broken_intra_doc_links)] //~^ NOTE lint level is defined diff --git a/src/test/rustdoc-ui/intra-link-errors.stderr b/src/test/rustdoc-ui/intra-link-errors.stderr index 13ed978d61380..798ae0bf01181 100644 --- a/src/test/rustdoc-ui/intra-link-errors.stderr +++ b/src/test/rustdoc-ui/intra-link-errors.stderr @@ -28,97 +28,68 @@ error: unresolved link to `std::io::not::here` LL | /// [std::io::not::here] | ^^^^^^^^^^^^^^^^^^ the module `io` contains no item named `not` -<<<<<<< HEAD error: unresolved link to `std::io::not::here` - --> $DIR/intra-link-errors.rs:23:6 -======= -error: unresolved link to `std::io::Error::x` --> $DIR/intra-link-errors.rs:24:6 ->>>>>>> Add `unclosed_html_tags` lint | LL | /// [type@std::io::not::here] | ^^^^^^^^^^^^^^^^^^^^^^^ the module `io` contains no item named `not` error: unresolved link to `std::io::Error::x` - --> $DIR/intra-link-errors.rs:27:6 + --> $DIR/intra-link-errors.rs:28:6 | LL | /// [std::io::Error::x] | ^^^^^^^^^^^^^^^^^ the struct `Error` has no field or associated item named `x` error: unresolved link to `std::io::ErrorKind::x` -<<<<<<< HEAD - --> $DIR/intra-link-errors.rs:31:6 -======= - --> $DIR/intra-link-errors.rs:28:6 ->>>>>>> Add `unclosed_html_tags` lint + --> $DIR/intra-link-errors.rs:32:6 | LL | /// [std::io::ErrorKind::x] | ^^^^^^^^^^^^^^^^^^^^^ the enum `ErrorKind` has no variant or associated item named `x` error: unresolved link to `f::A` -<<<<<<< HEAD - --> $DIR/intra-link-errors.rs:35:6 -======= - --> $DIR/intra-link-errors.rs:32:6 ->>>>>>> Add `unclosed_html_tags` lint + --> $DIR/intra-link-errors.rs:36:6 | LL | /// [f::A] | ^^^^ `f` is a function, not a module or type, and cannot have associated items error: unresolved link to `f::A` - --> $DIR/intra-link-errors.rs:39:6 + --> $DIR/intra-link-errors.rs:40:6 | LL | /// [f::A!] | ^^^^^ `f` is a function, not a module or type, and cannot have associated items error: unresolved link to `S::A` -<<<<<<< HEAD - --> $DIR/intra-link-errors.rs:43:6 -======= - --> $DIR/intra-link-errors.rs:36:6 ->>>>>>> Add `unclosed_html_tags` lint + --> $DIR/intra-link-errors.rs:44:6 | LL | /// [S::A] | ^^^^ the struct `S` has no field or associated item named `A` error: unresolved link to `S::fmt` -<<<<<<< HEAD - --> $DIR/intra-link-errors.rs:47:6 -======= - --> $DIR/intra-link-errors.rs:40:6 ->>>>>>> Add `unclosed_html_tags` lint + --> $DIR/intra-link-errors.rs:48:6 | LL | /// [S::fmt] | ^^^^^^ the struct `S` has no field or associated item named `fmt` error: unresolved link to `E::D` -<<<<<<< HEAD - --> $DIR/intra-link-errors.rs:51:6 -======= - --> $DIR/intra-link-errors.rs:44:6 ->>>>>>> Add `unclosed_html_tags` lint + --> $DIR/intra-link-errors.rs:52:6 | LL | /// [E::D] | ^^^^ the enum `E` has no variant or associated item named `D` error: unresolved link to `u8::not_found` -<<<<<<< HEAD - --> $DIR/intra-link-errors.rs:55:6 -======= - --> $DIR/intra-link-errors.rs:48:6 ->>>>>>> Add `unclosed_html_tags` lint + --> $DIR/intra-link-errors.rs:56:6 | LL | /// [u8::not_found] | ^^^^^^^^^^^^^ the builtin type `u8` has no associated item named `not_found` error: unresolved link to `std::primitive::u8::not_found` - --> $DIR/intra-link-errors.rs:59:6 + --> $DIR/intra-link-errors.rs:60:6 | LL | /// [std::primitive::u8::not_found] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the builtin type `u8` has no associated item named `not_found` error: unresolved link to `Vec::into_iter` - --> $DIR/intra-link-errors.rs:63:6 + --> $DIR/intra-link-errors.rs:64:6 | LL | /// [type@Vec::into_iter] | ^^^^^^^^^^^^^^^^^^^ @@ -127,11 +98,7 @@ LL | /// [type@Vec::into_iter] | help: to link to the associated function, add parentheses: `Vec::into_iter()` error: unresolved link to `S` -<<<<<<< HEAD - --> $DIR/intra-link-errors.rs:68:6 -======= - --> $DIR/intra-link-errors.rs:52:6 ->>>>>>> Add `unclosed_html_tags` lint + --> $DIR/intra-link-errors.rs:69:6 | LL | /// [S!] | ^^ @@ -140,11 +107,7 @@ LL | /// [S!] | help: to link to the struct, prefix with `struct@`: `struct@S` error: unresolved link to `T::g` -<<<<<<< HEAD - --> $DIR/intra-link-errors.rs:86:6 -======= - --> $DIR/intra-link-errors.rs:70:6 ->>>>>>> Add `unclosed_html_tags` lint + --> $DIR/intra-link-errors.rs:87:6 | LL | /// [type@T::g] | ^^^^^^^^^ @@ -153,21 +116,13 @@ LL | /// [type@T::g] | help: to link to the associated function, add parentheses: `T::g()` error: unresolved link to `T::h` -<<<<<<< HEAD - --> $DIR/intra-link-errors.rs:91:6 -======= - --> $DIR/intra-link-errors.rs:75:6 ->>>>>>> Add `unclosed_html_tags` lint + --> $DIR/intra-link-errors.rs:92:6 | LL | /// [T::h!] | ^^^^^ the trait `T` has no macro named `h` error: unresolved link to `S::h` -<<<<<<< HEAD - --> $DIR/intra-link-errors.rs:78:6 -======= - --> $DIR/intra-link-errors.rs:62:6 ->>>>>>> Add `unclosed_html_tags` lint + --> $DIR/intra-link-errors.rs:79:6 | LL | /// [type@S::h] | ^^^^^^^^^ @@ -176,11 +131,7 @@ LL | /// [type@S::h] | help: to link to the associated function, add parentheses: `S::h()` error: unresolved link to `m` -<<<<<<< HEAD - --> $DIR/intra-link-errors.rs:98:6 -======= - --> $DIR/intra-link-errors.rs:82:6 ->>>>>>> Add `unclosed_html_tags` lint + --> $DIR/intra-link-errors.rs:99:6 | LL | /// [m()] | ^^^ diff --git a/src/test/rustdoc-ui/invalid-html-tags.rs b/src/test/rustdoc-ui/invalid-html-tags.rs new file mode 100644 index 0000000000000..2df7c5435733d --- /dev/null +++ b/src/test/rustdoc-ui/invalid-html-tags.rs @@ -0,0 +1,19 @@ +#![deny(invalid_html_tags)] + +/// +/// +/// < ok +///

` (the `h2` tag isn't required // but it helps for the visualization). - cx.tcx.struct_span_lint_hir(lint::builtin::INVALID_HTML_TAGS, hir_id, sp, |lint| { - lint.build(&format!("unopened HTML tag `{}`", tag_name)).emit() - }); + f(&format!("unopened HTML tag `{}`", tag_name), &range); } } -fn extract_tag(cx: &DocContext<'_>, tags: &mut Vec, text: &str, hir_id: HirId, sp: Span) { - let mut iter = text.chars().peekable(); +fn extract_tag( + tags: &mut Vec<(String, Range)>, + text: &str, + range: Range, + f: &impl Fn(&str, &Range), +) { + let mut iter = text.chars().enumerate().peekable(); - while let Some(c) = iter.next() { + while let Some((start_pos, c)) = iter.next() { if c == '<' { let mut tag_name = String::new(); let mut is_closing = false; - while let Some(&c) = iter.peek() { - // - if c == '/' && tag_name.is_empty() { + while let Some((pos, c)) = iter.peek() { + // Checking if this is a closing tag (like `` for ``). + if *c == '/' && tag_name.is_empty() { is_closing = true; } else if c.is_ascii_alphanumeric() && !c.is_ascii_uppercase() { - tag_name.push(c); + tag_name.push(*c); } else { + if !tag_name.is_empty() { + let r = Range { start: range.start + start_pos, end: range.start + pos }; + if is_closing { + drop_tag(tags, tag_name, r, f); + } else { + tags.push((tag_name, r)); + } + } break; } iter.next(); } - if tag_name.is_empty() { - // Not an HTML tag presumably... - continue; - } - if is_closing { - drop_tag(cx, tags, tag_name, hir_id, sp); - } else { - tags.push(tag_name); - } } } } @@ -107,26 +107,32 @@ impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> { }; let dox = item.attrs.collapsed_doc_value().unwrap_or_default(); if !dox.is_empty() { - let sp = span_of_attrs(&item.attrs).unwrap_or(item.source.span()); + let cx = &self.cx; + let report_diag = |msg: &str, range: &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::INVALID_HTML_TAGS, hir_id, sp, |lint| { + lint.build(msg).emit() + }); + }; + let mut tags = Vec::new(); - let p = Parser::new_ext(&dox, opts()); + let p = Parser::new_ext(&dox, opts()).into_offset_iter(); - for event in p { + for (event, range) in p { match event { - Event::Html(text) => extract_tag(self.cx, &mut tags, &text, hir_id, sp), + Event::Html(text) => extract_tag(&mut tags, &text, range, &report_diag), _ => {} } } - for tag in tags.iter().filter(|t| ALLOWED_UNCLOSED.iter().find(|at| at == t).is_none()) + for (tag, range) in + tags.iter().filter(|(t, _)| ALLOWED_UNCLOSED.iter().find(|&at| at == t).is_none()) { - self.cx.tcx.struct_span_lint_hir( - lint::builtin::INVALID_HTML_TAGS, - hir_id, - sp, - |lint| lint.build(&format!("unclosed HTML tag `{}`", tag)).emit(), - ); + report_diag(&format!("unclosed HTML tag `{}`", tag), range); } } diff --git a/src/test/rustdoc-ui/invalid-html-tags.rs b/src/test/rustdoc-ui/invalid-html-tags.rs index 2df7c5435733d..b188e16f60513 100644 --- a/src/test/rustdoc-ui/invalid-html-tags.rs +++ b/src/test/rustdoc-ui/invalid-html-tags.rs @@ -1,19 +1,22 @@ #![deny(invalid_html_tags)] +/// /// /// +//~^ ERROR unclosed HTML tag `unknown` /// < ok /// pub fn foo() {} diff --git a/src/librustdoc/passes/html_tags.rs b/src/librustdoc/passes/html_tags.rs index 0cffaee1c4e99..1b48ce622b1f2 100644 --- a/src/librustdoc/passes/html_tags.rs +++ b/src/librustdoc/passes/html_tags.rs @@ -5,9 +5,8 @@ use crate::fold::DocFolder; use crate::html::markdown::opts; use core::ops::Range; use pulldown_cmark::{Event, Parser}; -// use rustc_hir::hir_id::HirId; +use rustc_feature::UnstableFeatures; use rustc_session::lint; -// use rustc_span::Span; pub const CHECK_INVALID_HTML_TAGS: Pass = Pass { name: "check-invalid-html-tags", @@ -26,9 +25,13 @@ impl<'a, 'tcx> InvalidHtmlTagsLinter<'a, 'tcx> { } pub fn check_invalid_html_tags(krate: Crate, cx: &DocContext<'_>) -> Crate { - let mut coll = InvalidHtmlTagsLinter::new(cx); + if !UnstableFeatures::from_environment().is_nightly_build() { + krate + } else { + let mut coll = InvalidHtmlTagsLinter::new(cx); - coll.fold_crate(krate) + coll.fold_crate(krate) + } } const ALLOWED_UNCLOSED: &[&str] = &[ From 4a3746e67b06bfd3bd11c8cae1c3acab944211bf Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 25 Sep 2020 21:53:14 +0200 Subject: [PATCH 06/12] Fix visitor for invalid_html_tag lint --- src/librustdoc/passes/html_tags.rs | 2 +- src/librustdoc/passes/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/passes/html_tags.rs b/src/librustdoc/passes/html_tags.rs index 1b48ce622b1f2..dfc5373886a60 100644 --- a/src/librustdoc/passes/html_tags.rs +++ b/src/librustdoc/passes/html_tags.rs @@ -105,7 +105,7 @@ impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> { Some(hir_id) => hir_id, None => { // If non-local, no need to check anything. - return None; + return self.fold_item_recur(item); } }; let dox = item.attrs.collapsed_doc_value().unwrap_or_default(); diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 3819aaee56000..a32fb10a6a787 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -104,8 +104,8 @@ pub const DEFAULT_PASSES: &[ConditionalPass] = &[ ConditionalPass::new(STRIP_PRIV_IMPORTS, WhenDocumentPrivate), ConditionalPass::always(COLLECT_INTRA_DOC_LINKS), ConditionalPass::always(CHECK_CODE_BLOCK_SYNTAX), - ConditionalPass::always(PROPAGATE_DOC_CFG), ConditionalPass::always(CHECK_INVALID_HTML_TAGS), + ConditionalPass::always(PROPAGATE_DOC_CFG), ]; /// The list of default passes run when `--doc-coverage` is passed to rustdoc. From 6163d892248a2af02b36a0121bb5bf02a5566d2b Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 26 Sep 2020 15:18:38 +0200 Subject: [PATCH 07/12] Improve code --- src/librustdoc/passes/html_tags.rs | 22 +++++++++----- src/test/rustdoc-ui/invalid-html-tags.rs | 19 +++++++++++++ src/test/rustdoc-ui/invalid-html-tags.stderr | 30 ++++++++++++++++---- 3 files changed, 58 insertions(+), 13 deletions(-) diff --git a/src/librustdoc/passes/html_tags.rs b/src/librustdoc/passes/html_tags.rs index dfc5373886a60..79035ba79ffd1 100644 --- a/src/librustdoc/passes/html_tags.rs +++ b/src/librustdoc/passes/html_tags.rs @@ -45,18 +45,20 @@ fn drop_tag( range: Range, f: &impl Fn(&str, &Range), ) { - if let Some(pos) = tags.iter().position(|(t, _)| *t == tag_name) { - for _ in pos + 1..tags.len() { - if ALLOWED_UNCLOSED.iter().find(|&at| at == &tags[pos + 1].0).is_some() { + if let Some(pos) = tags.iter().rev().position(|(t, _)| *t == tag_name) { + // Because this is from a `rev` iterator, the position is reversed as well! + let pos = tags.len() - 1 - pos; + for (last_tag_name, last_tag_span) in tags.drain(pos + 1..) { + if ALLOWED_UNCLOSED.iter().any(|&at| at == &last_tag_name) { continue; } // `tags` is used as a queue, meaning that everything after `pos` is included inside it. // So `

` will look like `["h2", "h3"]`. So when closing `h2`, we will still // have `h3`, meaning the tag wasn't closed as it should have. - f(&format!("unclosed HTML tag `{}`", tags[pos + 1].0), &tags[pos + 1].1); - tags.remove(pos + 1); + f(&format!("unclosed HTML tag `{}`", last_tag_name), &last_tag_span); } - tags.remove(pos); + // Remove the `tag_name` that was originally closed + tags.pop(); } else { // It can happen for example in this case: `

` (the `h2` tag isn't required // but it helps for the visualization). @@ -84,7 +86,13 @@ fn extract_tag( tag_name.push(*c); } else { if !tag_name.is_empty() { - let r = Range { start: range.start + start_pos, end: range.start + pos }; + let mut r = + Range { start: range.start + start_pos, end: range.start + pos }; + if *c == '>' { + // In case we have a tag without attribute, we can consider the span to + // refer to it fully. + r.end += 1; + } if is_closing { drop_tag(tags, tag_name, r, f); } else { diff --git a/src/test/rustdoc-ui/invalid-html-tags.rs b/src/test/rustdoc-ui/invalid-html-tags.rs index b188e16f60513..51bbae2a0b64e 100644 --- a/src/test/rustdoc-ui/invalid-html-tags.rs +++ b/src/test/rustdoc-ui/invalid-html-tags.rs @@ -20,3 +20,22 @@ pub fn foo() {} /// //~^ ERROR unopened HTML tag `hello` pub fn f() {} + +///
+///

+//~^ ERROR unclosed HTML tag `p` +///

+pub fn a() {} + +///
+///

+///

+///

+///
+pub fn b() {} + +///
+//~^ ERROR unclosed HTML tag `div` +///

+//~^ ERROR unclosed HTML tag `h3` +pub fn c() {} diff --git a/src/test/rustdoc-ui/invalid-html-tags.stderr b/src/test/rustdoc-ui/invalid-html-tags.stderr index f8e67732f63a0..d2e9704a22796 100644 --- a/src/test/rustdoc-ui/invalid-html-tags.stderr +++ b/src/test/rustdoc-ui/invalid-html-tags.stderr @@ -2,7 +2,7 @@ error: unclosed HTML tag `unknown` --> $DIR/invalid-html-tags.rs:7:5 | LL | /// - | ^^^^^^^^ + | ^^^^^^^^^ | note: the lint level is defined here --> $DIR/invalid-html-tags.rs:1:9 @@ -14,25 +14,43 @@ error: unclosed HTML tag `script` --> $DIR/invalid-html-tags.rs:10:5 | LL | /// +/// +pub fn d() {} + +// Unclosed tags shouldn't warn if they are nested inside a +/// +pub fn e() {} From 5bc148957efc0bf4640d3d6b3e1824c00f1156e8 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 28 Sep 2020 11:40:31 +0200 Subject: [PATCH 10/12] Correctly handle unicode characters and tags being open just before the end of the doc comment --- src/librustdoc/passes/html_tags.rs | 58 ++++++++++++++------ src/test/rustdoc-ui/invalid-html-tags.rs | 7 ++- src/test/rustdoc-ui/invalid-html-tags.stderr | 36 ++++++++---- 3 files changed, 72 insertions(+), 29 deletions(-) diff --git a/src/librustdoc/passes/html_tags.rs b/src/librustdoc/passes/html_tags.rs index 490e913fbde8b..f8869a41eb601 100644 --- a/src/librustdoc/passes/html_tags.rs +++ b/src/librustdoc/passes/html_tags.rs @@ -45,14 +45,22 @@ fn drop_tag( range: Range, f: &impl Fn(&str, &Range), ) { - if let Some(pos) = tags.iter().rev().position(|(t, _)| *t == tag_name) { + let tag_name_low = tag_name.to_lowercase(); + if let Some(pos) = tags.iter().rev().position(|(t, _)| t.to_lowercase() == tag_name_low) { // Because this is from a `rev` iterator, the position is reversed as well! let pos = tags.len() - 1 - pos; - // If the tag is nested inside a "