Tracking issue for check_invalid_html_tags

[Manishearth]

This is a tracking issue for the rustdoc lint check_invalid_html_tags.
There is no feature gate for this lint, but it is currently nightly-only.

check_invalid_html_tags

This lint warns about unclosed HTML tags and comments. See https://doc.rust-lang.org/nightly/rustdoc/lints.html#invalid_html_tags for more information.

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

• Implementation (Unclosed html tag lint #77119)
• Stabilization PR (see instructions on rustc-dev-guide)

Implementation history

• Initial implementation (Unclosed html tag lint #77119)
• Expanded to HTML comments (Check html comments #77753)
• Stabilization PR (Change default level of INVALID_HTML_TAGS to warning and stabilize it #101720)

Original issue

Rustdoc currently allows HTML in markdown, which is fine. The problem is that it doesn't sanitize the HTML at all, which means you can do wacky stuff like:

/// This is basically a Vec<Script>
pub enum ScriptExtension { /* ... */ }


impl ScriptExtension {
   /// docs
   pub fn some_func() {}
}

in which the Vec<Script> parses as an opening script tag, and the rest of the docs just ... disappear

This isn't great, and it's kind of surprising. Perhaps we should sanitize these, or at least warn when it happens? This may not be easy or cheap, though.

cc @GuillaumeGomez @kinnison

[steveklabnik]

Historically, the decision was to do nothing. That doesn't mean that maybe changing it isn't the right choice, but this feels like internals thread / RFC territorry rather than a bug, personally.

[Manishearth]

@steveklabnik my understanding is that the historical discussion has been mostly about whether rustdoc should allow arbitrary HTML at all, whereas this is about a narrower set of behavior -- allowing unclosed tags (which basically will never do what you want, and are often the result of mistyped generics).

There are really only a few unclosed tags that can cause problems (namely, script, video, object, audio, and some other similar ones), a simpler check would be one that just scans for <script with no corresponding </script> or something

[GuillaumeGomez]

I'd personally do nothing. From my point of view, this is up to users to master what they're doing when using HTML since we're following markdown commonmark spec.

[Manishearth]

Again, the problem is not users "using HTML", it's users accidentally introducing HTML into their code. I am okay with users intentionally using HTML in rustdoc. I know that has been a common complaint, but I am not making that complaint. I'm talking about something different.

Typing Vec<Script> without backticks is enough to break the docs. I've seen plenty of rustdocs that avoid religiously putting backticks on code, and I've made that mistake myself a bunch of times. Unclosed tags are an easy metric for this.

[GuillaumeGomez]

The problem is that a lot of HTML5 tags can be unclosed. Keeping this list up-to-date might be tricky. We can do it but in case a user is using an unclosed HTML5 tag, what should we do in this case? If it's a type we're supposed to not convert it to HTML but what if it's intended to be HTML? This is where I have an issue.

[Manishearth]

I also covered that in a previous comment. We can do this for specific tags only, as a start perhaps only the tags that will make the following content completely disappear (script, object, audio, video, etc).

We can also keep a list of all closeable HTML tags. If it's out of date, it's no big deal, this can just be a lint.

[kornelski]

Yes, it should. This is not the same as forbidding HTML or sanitizing what HTML features are allowed.

Unclosed tags can break the entire page, which is highly undesirable.

It might be used intentionally to open a tag in one doc comment and close it in another, but such uses would couple docs very tightly to a particular ordering of items and specific markup used by rustdoc itself. Allowing to rely on such a hacky thing seems like a bad idea.

Rustdoc could parse and reserialize the markup. It would guarantee the markup is syntactically correct without changing its meaning.

[GuillaumeGomez]

If we keep this as a warning, that could be a big plus. Does it sound OK?

@Manishearth: Do you mind listing the tags which need to be checked please?

[Manishearth]

Yes, I already suggested a warning, I'm fine with that.

script, object, video, audio are the most problematic. We can get an implementation in and figure out the full list later, it should be something that can be found in the HTML spec somewhere.

[GuillaumeGomez]

Ok, I'll try to send something in as soon as possible.

[jyn514]

Mentoring instructions: In src/librustdoc/passes add a new pass (maybe named lint_unclosed_tags). For an example of a pass that looks at doc-comments, see unindent_comments. For an example of a pass that lints, see doc_test_lints.

The pass should check for the following unclosed tags:

• script
• video
• object
• audio

As an extension, you could also look for other unclosed tags; any tag that isn't self-closing should warn here.

You can see the documentation by calling item.attrs.doc_strings.as_str().

For an initial implementation, you don't have to worry about <scri\npt> across newlines. However, you should definitely allow multi-line tags, such as

/// <script>
/// </script>