From a215151cd357945acdb2675010708cb2d10bb4c0 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 5 Oct 2020 14:00:00 +0200 Subject: [PATCH 01/27] Allow ascii whitespace char for doc aliases --- compiler/rustc_passes/src/check_attr.rs | 5 +++-- src/test/rustdoc-ui/check-doc-alias-attr.rs | 1 - src/test/rustdoc-ui/check-doc-alias-attr.stderr | 10 ++-------- src/test/ui/check-doc-alias-attr.rs | 1 - src/test/ui/check-doc-alias-attr.stderr | 10 ++-------- 5 files changed, 7 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 59955b27334..e8e54c3cfaa 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -287,8 +287,9 @@ impl CheckAttrVisitor<'tcx> { self.doc_alias_str_error(meta); return false; } - if let Some(c) = - doc_alias.chars().find(|&c| c == '"' || c == '\'' || c.is_whitespace()) + if let Some(c) = doc_alias + .chars() + .find(|&c| c == '"' || c == '\'' || (c.is_whitespace() && c != ' ')) { self.tcx .sess diff --git a/src/test/rustdoc-ui/check-doc-alias-attr.rs b/src/test/rustdoc-ui/check-doc-alias-attr.rs index c8bec39fad6..d55be9f120c 100644 --- a/src/test/rustdoc-ui/check-doc-alias-attr.rs +++ b/src/test/rustdoc-ui/check-doc-alias-attr.rs @@ -11,6 +11,5 @@ pub struct Bar; #[doc(alias = "\n")] //~ ERROR #[doc(alias = " ")] //~^ ERROR -#[doc(alias = " ")] //~ ERROR #[doc(alias = "\t")] //~ ERROR pub struct Foo; diff --git a/src/test/rustdoc-ui/check-doc-alias-attr.stderr b/src/test/rustdoc-ui/check-doc-alias-attr.stderr index be7d7b3dbea..97444c69ff1 100644 --- a/src/test/rustdoc-ui/check-doc-alias-attr.stderr +++ b/src/test/rustdoc-ui/check-doc-alias-attr.stderr @@ -36,17 +36,11 @@ LL | #[doc(alias = " LL | | ")] | |_^ -error: ' ' character isn't allowed in `#[doc(alias = "...")]` - --> $DIR/check-doc-alias-attr.rs:14:7 - | -LL | #[doc(alias = " ")] - | ^^^^^^^^^^^ - error: '\t' character isn't allowed in `#[doc(alias = "...")]` - --> $DIR/check-doc-alias-attr.rs:15:7 + --> $DIR/check-doc-alias-attr.rs:14:7 | LL | #[doc(alias = "\t")] | ^^^^^^^^^^^^ -error: aborting due to 8 previous errors +error: aborting due to 7 previous errors diff --git a/src/test/ui/check-doc-alias-attr.rs b/src/test/ui/check-doc-alias-attr.rs index c8bec39fad6..d55be9f120c 100644 --- a/src/test/ui/check-doc-alias-attr.rs +++ b/src/test/ui/check-doc-alias-attr.rs @@ -11,6 +11,5 @@ pub struct Bar; #[doc(alias = "\n")] //~ ERROR #[doc(alias = " ")] //~^ ERROR -#[doc(alias = " ")] //~ ERROR #[doc(alias = "\t")] //~ ERROR pub struct Foo; diff --git a/src/test/ui/check-doc-alias-attr.stderr b/src/test/ui/check-doc-alias-attr.stderr index be7d7b3dbea..97444c69ff1 100644 --- a/src/test/ui/check-doc-alias-attr.stderr +++ b/src/test/ui/check-doc-alias-attr.stderr @@ -36,17 +36,11 @@ LL | #[doc(alias = " LL | | ")] | |_^ -error: ' ' character isn't allowed in `#[doc(alias = "...")]` - --> $DIR/check-doc-alias-attr.rs:14:7 - | -LL | #[doc(alias = " ")] - | ^^^^^^^^^^^ - error: '\t' character isn't allowed in `#[doc(alias = "...")]` - --> $DIR/check-doc-alias-attr.rs:15:7 + --> $DIR/check-doc-alias-attr.rs:14:7 | LL | #[doc(alias = "\t")] | ^^^^^^^^^^^^ -error: aborting due to 8 previous errors +error: aborting due to 7 previous errors From accc26abc0e53067d7a97d0fb19800c37a24f844 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 5 Oct 2020 14:00:12 +0200 Subject: [PATCH 02/27] Add test for whitespace in doc alias --- src/test/rustdoc-js/doc-alias-whitespace.js | 19 +++++++++++++++++++ src/test/rustdoc-js/doc-alias-whitespace.rs | 4 ++++ 2 files changed, 23 insertions(+) create mode 100644 src/test/rustdoc-js/doc-alias-whitespace.js create mode 100644 src/test/rustdoc-js/doc-alias-whitespace.rs diff --git a/src/test/rustdoc-js/doc-alias-whitespace.js b/src/test/rustdoc-js/doc-alias-whitespace.js new file mode 100644 index 00000000000..c9fc0c4311f --- /dev/null +++ b/src/test/rustdoc-js/doc-alias-whitespace.js @@ -0,0 +1,19 @@ +// exact-check + +const QUERY = [ + 'Demon Lord', +]; + +const EXPECTED = [ + { + 'others': [ + { + 'path': 'doc_alias_whitespace', + 'name': 'Struct', + 'alias': 'Demon Lord', + 'href': '../doc_alias_whitespace/struct.Struct.html', + 'is_alias': true + }, + ], + }, +]; diff --git a/src/test/rustdoc-js/doc-alias-whitespace.rs b/src/test/rustdoc-js/doc-alias-whitespace.rs new file mode 100644 index 00000000000..bea3e382ae4 --- /dev/null +++ b/src/test/rustdoc-js/doc-alias-whitespace.rs @@ -0,0 +1,4 @@ +#![feature(doc_alias)] + +#[doc(alias = "Demon Lord")] +pub struct Struct; From 11f3476c59ee013c017e669676cfaca00a67b3f1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 6 Oct 2020 14:29:42 +0200 Subject: [PATCH 03/27] Enforce whitespace ascii character check for doc alias --- compiler/rustc_passes/src/check_attr.rs | 10 ++++++++++ src/test/rustdoc-ui/check-doc-alias-attr.rs | 2 ++ src/test/rustdoc-ui/check-doc-alias-attr.stderr | 14 +++++++++++++- src/test/ui/check-doc-alias-attr.rs | 2 ++ src/test/ui/check-doc-alias-attr.stderr | 14 +++++++++++++- 5 files changed, 40 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index e8e54c3cfaa..1acaa4c6eff 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -303,6 +303,16 @@ impl CheckAttrVisitor<'tcx> { .emit(); return false; } + if doc_alias.starts_with(' ') || doc_alias.ends_with(' ') { + self.tcx + .sess + .struct_span_err( + meta.span(), + "`#[doc(alias = \"...\")]` cannot start or end with ' '", + ) + .emit(); + return false; + } if let Some(err) = match target { Target::Impl => Some("implementation block"), Target::ForeignMod => Some("extern block"), diff --git a/src/test/rustdoc-ui/check-doc-alias-attr.rs b/src/test/rustdoc-ui/check-doc-alias-attr.rs index d55be9f120c..0ca2349a43b 100644 --- a/src/test/rustdoc-ui/check-doc-alias-attr.rs +++ b/src/test/rustdoc-ui/check-doc-alias-attr.rs @@ -12,4 +12,6 @@ pub struct Bar; #[doc(alias = " ")] //~^ ERROR #[doc(alias = "\t")] //~ ERROR +#[doc(alias = " hello")] //~ ERROR +#[doc(alias = "hello ")] //~ ERROR pub struct Foo; diff --git a/src/test/rustdoc-ui/check-doc-alias-attr.stderr b/src/test/rustdoc-ui/check-doc-alias-attr.stderr index 97444c69ff1..2c417a3bb65 100644 --- a/src/test/rustdoc-ui/check-doc-alias-attr.stderr +++ b/src/test/rustdoc-ui/check-doc-alias-attr.stderr @@ -42,5 +42,17 @@ error: '\t' character isn't allowed in `#[doc(alias = "...")]` LL | #[doc(alias = "\t")] | ^^^^^^^^^^^^ -error: aborting due to 7 previous errors +error: `#[doc(alias = "...")]` cannot start or end with ' ' + --> $DIR/check-doc-alias-attr.rs:15:7 + | +LL | #[doc(alias = " hello")] + | ^^^^^^^^^^^^^^^^ + +error: `#[doc(alias = "...")]` cannot start or end with ' ' + --> $DIR/check-doc-alias-attr.rs:16:7 + | +LL | #[doc(alias = "hello ")] + | ^^^^^^^^^^^^^^^^ + +error: aborting due to 9 previous errors diff --git a/src/test/ui/check-doc-alias-attr.rs b/src/test/ui/check-doc-alias-attr.rs index d55be9f120c..0ca2349a43b 100644 --- a/src/test/ui/check-doc-alias-attr.rs +++ b/src/test/ui/check-doc-alias-attr.rs @@ -12,4 +12,6 @@ pub struct Bar; #[doc(alias = " ")] //~^ ERROR #[doc(alias = "\t")] //~ ERROR +#[doc(alias = " hello")] //~ ERROR +#[doc(alias = "hello ")] //~ ERROR pub struct Foo; diff --git a/src/test/ui/check-doc-alias-attr.stderr b/src/test/ui/check-doc-alias-attr.stderr index 97444c69ff1..2c417a3bb65 100644 --- a/src/test/ui/check-doc-alias-attr.stderr +++ b/src/test/ui/check-doc-alias-attr.stderr @@ -42,5 +42,17 @@ error: '\t' character isn't allowed in `#[doc(alias = "...")]` LL | #[doc(alias = "\t")] | ^^^^^^^^^^^^ -error: aborting due to 7 previous errors +error: `#[doc(alias = "...")]` cannot start or end with ' ' + --> $DIR/check-doc-alias-attr.rs:15:7 + | +LL | #[doc(alias = " hello")] + | ^^^^^^^^^^^^^^^^ + +error: `#[doc(alias = "...")]` cannot start or end with ' ' + --> $DIR/check-doc-alias-attr.rs:16:7 + | +LL | #[doc(alias = "hello ")] + | ^^^^^^^^^^^^^^^^ + +error: aborting due to 9 previous errors From 861b8921c08e3cbe2ff8176679cc0cb3216bb2e3 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 9 Oct 2020 14:48:45 +0200 Subject: [PATCH 04/27] Clean up rustdoc HTML tags check pass --- src/librustdoc/passes/html_tags.rs | 128 ++++++++++++++++------------- 1 file changed, 71 insertions(+), 57 deletions(-) diff --git a/src/librustdoc/passes/html_tags.rs b/src/librustdoc/passes/html_tags.rs index ae4eac89b45..872543d918c 100644 --- a/src/librustdoc/passes/html_tags.rs +++ b/src/librustdoc/passes/html_tags.rs @@ -4,6 +4,8 @@ use crate::core::DocContext; use crate::fold::DocFolder; use crate::html::markdown::opts; use core::ops::Range; +use std::iter::Peekable; +use std::str::CharIndices; use pulldown_cmark::{Event, Parser}; use rustc_feature::UnstableFeatures; use rustc_session::lint; @@ -75,7 +77,73 @@ fn drop_tag( } } -fn extract_tag( +fn extract_html_tag( + tags: &mut Vec<(String, Range)>, + text: &str, + range: &Range, + start_pos: usize, + iter: &mut Peekable>, + f: &impl Fn(&str, &Range), +) { + let mut tag_name = String::new(); + let mut is_closing = false; + let mut prev_pos = start_pos; + + loop { + let (pos, c) = match iter.peek() { + Some((pos, c)) => (*pos, *c), + // In case we reached the of the doc comment, we want to check that it's an + // unclosed HTML tag. For example "/// (prev_pos, '\0'), + }; + prev_pos = pos; + // Checking if this is a closing tag (like `` for ``). + if c == '/' && tag_name.is_empty() { + is_closing = true; + } else if c.is_ascii_alphanumeric() { + tag_name.push(c); + } else { + if !tag_name.is_empty() { + 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 { + // In case we have "" or even "". + if c != '>' { + if !c.is_whitespace() { + // It seems like it's not a valid HTML tag. + break; + } + let mut found = false; + for (new_pos, c) in text[pos..].char_indices() { + if !c.is_whitespace() { + if c == '>' { + r.end = range.start + new_pos + 1; + found = true; + } + break; + } + } + if !found { + break; + } + } + drop_tag(tags, tag_name, r, f); + } else { + tags.push((tag_name, r)); + } + } + break; + } + iter.next(); + } +} + +fn extract_tags( tags: &mut Vec<(String, Range)>, text: &str, range: Range, @@ -85,61 +153,7 @@ fn extract_tag( while let Some((start_pos, c)) = iter.next() { if c == '<' { - let mut tag_name = String::new(); - let mut is_closing = false; - let mut prev_pos = start_pos; - loop { - let (pos, c) = match iter.peek() { - Some((pos, c)) => (*pos, *c), - // In case we reached the of the doc comment, we want to check that it's an - // unclosed HTML tag. For example "/// (prev_pos, '\0'), - }; - prev_pos = pos; - // Checking if this is a closing tag (like `` for ``). - if c == '/' && tag_name.is_empty() { - is_closing = true; - } else if c.is_ascii_alphanumeric() { - tag_name.push(c); - } else { - if !tag_name.is_empty() { - 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 { - // In case we have "" or even "". - if c != '>' { - if !c.is_whitespace() { - // It seems like it's not a valid HTML tag. - break; - } - let mut found = false; - for (new_pos, c) in text[pos..].char_indices() { - if !c.is_whitespace() { - if c == '>' { - r.end = range.start + new_pos + 1; - found = true; - } - break; - } - } - if !found { - break; - } - } - drop_tag(tags, tag_name, r, f); - } else { - tags.push((tag_name, r)); - } - } - break; - } - iter.next(); - } + extract_html_tag(tags, text, &range, start_pos, &mut iter, f); } } } @@ -172,7 +186,7 @@ impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> { for (event, range) in p { match event { - Event::Html(text) => extract_tag(&mut tags, &text, range, &report_diag), + Event::Html(text) => extract_tags(&mut tags, &text, range, &report_diag), _ => {} } } From 0009cbaabd6f2ba986e7631905612aa83d5970f8 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 9 Oct 2020 15:04:22 +0200 Subject: [PATCH 05/27] Add check for HTML comments --- src/librustdoc/passes/html_tags.rs | 34 +++++++++++++++++--- src/test/rustdoc-ui/invalid-html-tags.rs | 10 ++++++ src/test/rustdoc-ui/invalid-html-tags.stderr | 8 ++++- 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/passes/html_tags.rs b/src/librustdoc/passes/html_tags.rs index 872543d918c..8bb71cef1c7 100644 --- a/src/librustdoc/passes/html_tags.rs +++ b/src/librustdoc/passes/html_tags.rs @@ -4,11 +4,11 @@ use crate::core::DocContext; use crate::fold::DocFolder; use crate::html::markdown::opts; use core::ops::Range; -use std::iter::Peekable; -use std::str::CharIndices; use pulldown_cmark::{Event, Parser}; use rustc_feature::UnstableFeatures; use rustc_session::lint; +use std::iter::Peekable; +use std::str::CharIndices; pub const CHECK_INVALID_HTML_TAGS: Pass = Pass { name: "check-invalid-html-tags", @@ -104,8 +104,7 @@ fn extract_html_tag( tag_name.push(c); } else { if !tag_name.is_empty() { - let mut 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. @@ -143,6 +142,27 @@ fn extract_html_tag( } } +fn extract_html_comment( + text: &str, + range: &Range, + start_pos: usize, + iter: &mut Peekable>, + f: &impl Fn(&str, &Range), +) { + // We first skip the "!--" part. + let mut iter = iter.skip(3); + while let Some((pos, c)) = iter.next() { + if c == '-' && text[pos..].starts_with("-->") { + // All good, we can leave! + return; + } + } + f( + "Unclosed HTML comment", + &Range { start: range.start + start_pos, end: range.start + start_pos + 3 }, + ); +} + fn extract_tags( tags: &mut Vec<(String, Range)>, text: &str, @@ -153,7 +173,11 @@ fn extract_tags( while let Some((start_pos, c)) = iter.next() { if c == '<' { - extract_html_tag(tags, text, &range, start_pos, &mut iter, f); + if text[start_pos..].starts_with(" +/// +/// +/// +pub fn g() {} + +/// $DIR/invalid-html-tags.rs:83:5 + | +LL | /// ") { - // All good, we can leave! - return; - } - } - f( - "Unclosed HTML comment", - &Range { start: range.start + start_pos, end: range.start + start_pos + 3 }, - ); -} - fn extract_tags( tags: &mut Vec<(String, Range)>, text: &str, range: Range, + is_in_comment: &mut Option>, f: &impl Fn(&str, &Range), ) { let mut iter = text.char_indices().peekable(); while let Some((start_pos, c)) = iter.next() { - if c == '<' { + if is_in_comment.is_some() { + if text[start_pos..].starts_with("-->") { + *is_in_comment = None; + } + } else if c == '<' { if text[start_pos..].starts_with(" pub fn h() {} + +/// $DIR/invalid-html-tags.rs:83:5 + --> $DIR/invalid-html-tags.rs:87:5 | LL | ///