make rustdoc::invalid_html_tags more robust

[lolbinarycat]

best reviewed a commit at a time.

I kept finding more edge case so I ended up having to make quite significant changes to the parser in order to make it preserve state across events and handle multiline attributes correctly.

fixes #145529

[rustbot]

r? @notriddle

rustbot has assigned @notriddle.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[klensy]

pulldown-cmark splits html blocks

Used pulldown-cmark is slightly old for reasons, this can be changed in more recent versions.

[notriddle]

This won't work if you wrap an html block in a markdown blockquote.

$ cargo run --release -- --events
    Finished `release` profile [optimized] target(s) in 0.06s
     Running `target/release/pulldown-cmark --events`
> <div
> class="foo">
0..22: Start(BlockQuote(None))
2..22: Start(HtmlBlock)
2..7: Html(Borrowed("<div\n"))
9..22: Html(Borrowed("class=\"foo\">\n"))
2..22: End(HtmlBlock)
0..22: End(BlockQuote(None))
EOF

[lolbinarycat]

What do you think should be done instead, then? Unconditionally coalesce adjacent Html events, using Cow::Owned if needed? That could potentially also break the check that looks for > within the range..

The other option would be fully refactoring the parser to move its state into a struct so that it can be passed multiple strings and doesn't need to have the strings concatenated. If we're doing that we can also refactor the tag parsing a bit so unfinished tags (those missing >) are actually treated as a separate error case from unclosed tags.

The latter would be more robust, but also a fair bit more work.

[notriddle]

Unconditionally coalesce adjacent Html events, using Cow::Owned if needed?

I think that's the easiest way to handle it.

    while let Some((event, range)) = p.next() {
        match event {
            Event::Start(Tag::CodeBlock(_)) => in_code_block = true,
            Event::Html(text) | Event::InlineHtml(text) if !in_code_block => {
                extract_tags(&mut tags, &text, range, dox, &mut is_in_comment, &report_diag)
            }
            // for some reason, pulldown-cmark splits html blocks into seperate events for each line.
            // we undo this, in order to handle multi-line tags.
            Event::Tag(Tag::HtmlBlock) if !in_code_block => {
                let mut html_text = String::new();
                while let Some((event_inner, _)) = p.next() {
                    match event_inner {
                        Event::Html(text) => {
                            html_text.push_str(&text[..]);
                        }
                        Event::End(TagEnd::HtmlBlock) => break,
                        _ => unreachable!("html is supposed to be a leaf block"),
                    }
                }
                extract_tags(&mut tags, &html_text, range, dox, &mut is_in_comment, &report_diag);
            }
            Event::End(TagEnd::CodeBlock) => in_code_block = false,
            _ => {}
        }
    }

[lolbinarycat]

That still doesn't solve the issue of > from backquotes. I think refactoring the parser is probably the correct thing to do here.

[rustbot]

⚠️ Warning ⚠️

• There are issue links (such as #123) in the commit messages of the following commits.
  Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.
  • e50fed7

[GuillaumeGomez]

Seems like it's already a big improvement over the existing so let's merge it.

@notriddle: Based on your comments, you're not completely satisfied with the current fix so I suggest we do iterative improvements in follow-up(s).

Thanks to both of you in any case!

@bors r+ rollup

[bors]

📌 Commit d022089 has been approved by GuillaumeGomez

It is now in the queue for this repository.