Rollup merge of #77753 - GuillaumeGomez:check-html-comments, r=jyn514

Check html comments Part of #67799. cc @ollie27 r? @jyn514
rust-lang · Oct 14, 2020 · 5f5ef05 · 5f5ef05
2 parents 022d207 + dd3be78
commit 5f5ef05
Show file tree

Hide file tree

Showing 3 changed files with 110 additions and 54 deletions.
diff --git a/src/librustdoc/passes/html_tags.rs b/src/librustdoc/passes/html_tags.rs
@@ -7,6 +7,8 @@ use core::ops::Range;
 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",
@@ -75,70 +77,97 @@ fn drop_tag(
     }
 }
 
-fn extract_tag(
+fn extract_html_tag(
     tags: &mut Vec<(String, Range<usize>)>,
     text: &str,
-    range: Range<usize>,
+    range: &Range<usize>,
+    start_pos: usize,
+    iter: &mut Peekable<CharIndices<'_>>,
     f: &impl Fn(&str, &Range<usize>),
 ) {
-    let mut iter = text.char_indices().peekable();
+    let mut tag_name = String::new();
+    let mut is_closing = false;
+    let mut prev_pos = start_pos;
 
-    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 "/// <h3".
-                    None => (prev_pos, '\0'),
-                };
-                prev_pos = pos;
-                // Checking if this is a closing tag (like `</a>` for `<a>`).
-                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;
+    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 "/// <h3".
+            None => (prev_pos, '\0'),
+        };
+        prev_pos = pos;
+        // Checking if this is a closing tag (like `</a>` for `<a>`).
+        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 "</div >" or even "</div         >".
+                    if c != '>' {
+                        if !c.is_whitespace() {
+                            // It seems like it's not a valid HTML tag.
+                            break;
                         }
-                        if is_closing {
-                            // In case we have "</div >" or even "</div         >".
-                            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;
+                        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;
                             }
-                            drop_tag(tags, tag_name, r, f);
-                        } else {
-                            tags.push((tag_name, r));
+                        }
+                        if !found {
+                            break;
                         }
                     }
-                    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<usize>)>,
+    text: &str,
+    range: Range<usize>,
+    is_in_comment: &mut Option<Range<usize>>,
+    f: &impl Fn(&str, &Range<usize>),
+) {
+    let mut iter = text.char_indices().peekable();
+
+    while let Some((start_pos, c)) = iter.next() {
+        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("<!--") {
+                // We skip the "!--" part. (Once `advance_by` is stable, might be nice to use it!)
+                iter.next();
                 iter.next();
+                iter.next();
+                *is_in_comment = Some(Range {
+                    start: range.start + start_pos,
+                    end: range.start + start_pos + 3,
+                });
+            } else {
+                extract_html_tag(tags, text, &range, start_pos, &mut iter, f);
             }
         }
     }
@@ -167,12 +196,15 @@ impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> {
             };
 
             let mut tags = Vec::new();
+            let mut is_in_comment = None;
 
             let p = Parser::new_ext(&dox, opts()).into_offset_iter();
 
             for (event, range) in p {
                 match event {
-                    Event::Html(text) => extract_tag(&mut tags, &text, range, &report_diag),
+                    Event::Html(text) | Event::Text(text) => {
+                        extract_tags(&mut tags, &text, range, &mut is_in_comment, &report_diag)
+                    }
                     _ => {}
                 }
             }
@@ -183,6 +215,10 @@ impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> {
             }) {
                 report_diag(&format!("unclosed HTML tag `{}`", tag), range);
             }
+
+            if let Some(range) = is_in_comment {
+                report_diag("Unclosed HTML comment", &range);
+            }
         }
 
         self.fold_item_recur(item)

diff --git a/src/test/rustdoc-ui/invalid-html-tags.rs b/src/test/rustdoc-ui/invalid-html-tags.rs
@@ -73,3 +73,17 @@ pub fn e() {}
 /// <div></div
 //~^ ERROR unclosed HTML tag `div`
 pub fn f() {}
+
+/// <!---->
+/// <!-- -->
+/// <!-- <div> -->
+/// <!-- <!-- -->
+pub fn g() {}
+
+/// <!--
+/// -->
+pub fn h() {}
+
+/// <!--
+//~^ ERROR Unclosed HTML comment
+pub fn i() {}
diff --git a/src/test/rustdoc-ui/invalid-html-tags.stderr b/src/test/rustdoc-ui/invalid-html-tags.stderr
@@ -76,5 +76,11 @@ error: unclosed HTML tag `div`
 LL | /// <div></div
    |     ^^^^^
 
-error: aborting due to 12 previous errors
+error: Unclosed HTML comment
+  --> $DIR/invalid-html-tags.rs:87:5
+   |
+LL | /// <!--
+   |     ^^^
+
+error: aborting due to 13 previous errors